| Message ID | 2cc8012ec538106c6bcf22a40b647ec342e687a8.1689012506.git.daniel@makrotopia.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: ethernet: mtk_eth_soc: add basic support for MT7988 SoC | expand |
On Thu, Jul 13, 2023 at 03:18:52AM +0100, Daniel Golle wrote: > From: Lorenzo Bianconi <lorenzo@kernel.org> > > This is a preliminary patch to add MT7988 SoC support since it runs 3 > macs instead of 2. > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 6 +++++- > drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 ++-- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > index 7014e0d108b27..7f191e4337dd8 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > @@ -4030,8 +4030,12 @@ static void mtk_sgmii_destroy(struct mtk_eth *eth) > { > int i; > > - for (i = 0; i < MTK_MAX_DEVS; i++) > + for (i = 0; i < MTK_MAX_DEVS; i++) { > + if (!eth->sgmii_pcs[i]) > + continue; > + > mtk_pcs_lynxi_destroy(eth->sgmii_pcs[i]); > + } Please instead arrange for mtk_pcs_lynxi_destroy() to be a no-op if it's passed a NULL pointer, which makes it easier to use in error paths (it means mtk_pcs_lynxi_destroy() can be called without checks - like kfree() etc.) Since error paths don't get often tested, we need to do as much as possible to make error paths trivial.
On Thu, Jul 13, 2023 at 08:07:57AM +0100, Russell King (Oracle) wrote: > On Thu, Jul 13, 2023 at 03:18:52AM +0100, Daniel Golle wrote: > > From: Lorenzo Bianconi <lorenzo@kernel.org> > > > > This is a preliminary patch to add MT7988 SoC support since it runs 3 > > macs instead of 2. > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > > --- > > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 6 +++++- > > drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 ++-- > > 2 files changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > > index 7014e0d108b27..7f191e4337dd8 100644 > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > > @@ -4030,8 +4030,12 @@ static void mtk_sgmii_destroy(struct mtk_eth *eth) > > { > > int i; > > > > - for (i = 0; i < MTK_MAX_DEVS; i++) > > + for (i = 0; i < MTK_MAX_DEVS; i++) { > > + if (!eth->sgmii_pcs[i]) > > + continue; > > + > > mtk_pcs_lynxi_destroy(eth->sgmii_pcs[i]); > > + } > > Please instead arrange for mtk_pcs_lynxi_destroy() to be a no-op if it's > passed a NULL pointer, which makes it easier to use in error paths (it > means mtk_pcs_lynxi_destroy() can be called without checks - like > kfree() etc.) > > Since error paths don't get often tested, we need to do as much as > possible to make error paths trivial. This is the case already, so the above if-clause can be dropped. I will do so in v3. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/pcs/pcs-mtk-lynxi.c#n291
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 7014e0d108b27..7f191e4337dd8 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -4030,8 +4030,12 @@ static void mtk_sgmii_destroy(struct mtk_eth *eth) { int i; - for (i = 0; i < MTK_MAX_DEVS; i++) + for (i = 0; i < MTK_MAX_DEVS; i++) { + if (!eth->sgmii_pcs[i]) + continue; + mtk_pcs_lynxi_destroy(eth->sgmii_pcs[i]); + } } static int mtk_cleanup(struct mtk_eth *eth) diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h index c74c3918113a5..b18ce4430d437 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h @@ -1049,8 +1049,8 @@ struct mtk_soc_data { #define MTK_DMA_MONITOR_TIMEOUT msecs_to_jiffies(1000) -/* currently no SoC has more than 2 macs */ -#define MTK_MAX_DEVS 2 +/* currently no SoC has more than 3 macs */ +#define MTK_MAX_DEVS 3 /* struct mtk_eth - This is the main datasructure for holding the state * of the driver