[PATCH net-next v2 0/3] Followup fixes for the dwmac and altera lynx conversion

[PATCH net-next v2 0/3] Followup fixes for the dwmac and altera lynx conversion

[Maxime Chevallier]

Following the TSE PCS removal and port of altera_tse and dwmac_socfpga,
this series fixes some issues that slipped through the cracks.

Patch 1 fixes an unitialized struct in altera_tse

Patch 2 uses the correct Kconfig option for altera_tse

Patch 3 makes the Lynx PCS specific to dwmac_socfpga. This patch was
originally written by Russell, my modifications just moves the
#include<linux/pcs-lynx.h> around, to use it only in dwmac_socfpga.

Maxime Chevallier (3):
  net: altera-tse: Initialize the regmap_config struct before using it
  net: altera_tse: Use the correct Kconfig option for the PCS_LYNX
    depenency
  net: stmmac: make the pcs_lynx cleanup sequence specific to
    dwmac_socfpga

 drivers/net/ethernet/altera/Kconfig               |  2 +-
 drivers/net/ethernet/altera/altera_tse_main.c     |  1 +
 drivers/net/ethernet/stmicro/stmmac/common.h      |  1 -
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c   | 15 ++++++++++++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |  3 ---
 5 files changed, 16 insertions(+), 6 deletions(-)

-- 
2.40.1

[PATCH net-next v2 1/3] net: altera-tse: Initialize the regmap_config struct before using it

[Maxime Chevallier]

The regmap_config needs to be zeroed before using it. This will cause
spurious errors at probe time as config->pad_bits is containing random
uninitialized data.

Fixes: db48abbaa18e ("net: ethernet: altera-tse: Convert to mdio-regmap and use PCS Lynx")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V1->V2: No change

 drivers/net/ethernet/altera/altera_tse_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index d866c0f1b503..df509abcd378 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1255,6 +1255,7 @@ static int altera_tse_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_free_netdev;
 
+	memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
 	/* SGMII PCS address space. The location can vary depending on how the
 	 * IP is integrated. We can have a resource dedicated to it at a specific
 	 * address space, but if it's not the case, we fallback to the mdiophy0
-- 
2.40.1

[PATCH net-next v2 2/3] net: altera_tse: Use the correct Kconfig option for the PCS_LYNX depenency

[Maxime Chevallier]

Use the correct Kconfig dependency for altera_tse as PCS_ALTERA_TSE was
replaced by PCS_LYNX.

Fixes: db48abbaa18e ("net: ethernet: altera-tse: Convert to mdio-regmap and use PCS Lynx")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V1->V2 : New patch

 drivers/net/ethernet/altera/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/altera/Kconfig b/drivers/net/ethernet/altera/Kconfig
index 93533ba03429..17985319088c 100644
--- a/drivers/net/ethernet/altera/Kconfig
+++ b/drivers/net/ethernet/altera/Kconfig
@@ -4,7 +4,7 @@ config ALTERA_TSE
 	depends on HAS_DMA
 	select PHYLIB
 	select PHYLINK
-	select PCS_ALTERA_TSE
+	select PCS_LYNX
 	select MDIO_REGMAP
 	select REGMAP_MMIO
 	help
-- 
2.40.1

[PATCH net-next v2 3/3] net: stmmac: make the pcs_lynx cleanup sequence specific to dwmac_socfpga

[Maxime Chevallier]

So far, only the dwmac_socfpga variant of stmmac uses PCS Lynx. Use a
dedicated cleanup sequence for dwmac_socfpga instead of using the
generic stmmac one.

Fixes: 5d1f3fe7d2d5 ("net: stmmac: dwmac-sogfpga: use the lynx pcs driver")
Suggested-By: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V1->V2 : New patch

 drivers/net/ethernet/stmicro/stmmac/common.h      |  1 -
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c   | 15 ++++++++++++++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |  3 ---
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 52c5ec553276..16e67c18b6f7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -16,7 +16,6 @@
 #include <linux/stmmac.h>
 #include <linux/phy.h>
 #include <linux/pcs/pcs-xpcs.h>
-#include <linux/pcs-lynx.h>
 #include <linux/module.h>
 #if IS_ENABLED(CONFIG_VLAN_8021Q)
 #define STMMAC_VLAN_TAG_USED
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index e399fccbafe5..ad66e128bff9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -11,6 +11,7 @@
 #include <linux/phy.h>
 #include <linux/regmap.h>
 #include <linux/mdio/mdio-regmap.h>
+#include <linux/pcs-lynx.h>
 #include <linux/reset.h>
 #include <linux/stmmac.h>
 
@@ -494,6 +495,18 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static void socfpga_dwmac_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct phylink_pcs *pcs = priv->hw->lynx_pcs;
+
+	stmmac_pltfr_remove(pdev);
+
+	lynx_pcs_destroy(pcs);
+}
+
+
 #ifdef CONFIG_PM_SLEEP
 static int socfpga_dwmac_resume(struct device *dev)
 {
@@ -565,7 +578,7 @@ MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
 
 static struct platform_driver socfpga_dwmac_driver = {
 	.probe  = socfpga_dwmac_probe,
-	.remove_new = stmmac_pltfr_remove,
+	.remove_new = socfpga_dwmac_remove,
 	.driver = {
 		.name           = "socfpga-dwmac",
 		.pm		= &socfpga_dwmac_pm_ops,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index c784a6731f08..3db1cb0fd160 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -665,9 +665,6 @@ int stmmac_mdio_unregister(struct net_device *ndev)
 	if (priv->hw->xpcs)
 		xpcs_destroy(priv->hw->xpcs);
 
-	if (priv->hw->lynx_pcs)
-		lynx_pcs_destroy(priv->hw->lynx_pcs);
-
 	mdiobus_unregister(priv->mii);
 	priv->mii->priv = NULL;
 	mdiobus_free(priv->mii);
-- 
2.40.1

Re: [PATCH net-next v2 0/3] Followup fixes for the dwmac and altera lynx conversion

[Russell King (Oracle)]

On Tue, Jun 06, 2023 at 04:21:41PM +0200, Maxime Chevallier wrote:
> Following the TSE PCS removal and port of altera_tse and dwmac_socfpga,
> this series fixes some issues that slipped through the cracks.
> 
> Patch 1 fixes an unitialized struct in altera_tse
> 
> Patch 2 uses the correct Kconfig option for altera_tse
> 
> Patch 3 makes the Lynx PCS specific to dwmac_socfpga. This patch was
> originally written by Russell, my modifications just moves the
> #include<linux/pcs-lynx.h> around, to use it only in dwmac_socfpga.

Hi Maxime,

I'm sorry, but I think you need an extra patch added to this series.
Looking at include/linux/mdio/mdio-regmap.h, that defines:

struct mdio_regmap_config {
        struct device *parent;
        struct regmap *regmap;
        char name[MII_BUS_ID_SIZE];
        u8 valid_addr;
        bool autoscan;
};

In dwmac-socfpga.c, you have:

                struct mdio_regmap_config mrc;

                mrc.regmap = pcs_regmap;
                mrc.parent = &pdev->dev;
                mrc.valid_addr = 0x0;

                snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);

So that's a tick for parent, tick for regmap, tick for name, tick
for valid_addr, but... autoscan is left uninitialised.
devm_mdio_regmap_register() reads this, and uses it to decide
how to set mii->phy_mask, which will be randomly ~0 or ~BIT(0)
depending on the value of mrc.autoscan.

Other than that, the series looks good. Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 0/3] Followup fixes for the dwmac and altera lynx conversion

[Maxime Chevallier]

Hello Russell,

On Tue, 6 Jun 2023 15:32:53 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Tue, Jun 06, 2023 at 04:21:41PM +0200, Maxime Chevallier wrote:
> > Following the TSE PCS removal and port of altera_tse and dwmac_socfpga,
> > this series fixes some issues that slipped through the cracks.
> > 
> > Patch 1 fixes an unitialized struct in altera_tse
> > 
> > Patch 2 uses the correct Kconfig option for altera_tse
> > 
> > Patch 3 makes the Lynx PCS specific to dwmac_socfpga. This patch was
> > originally written by Russell, my modifications just moves the
> > #include<linux/pcs-lynx.h> around, to use it only in dwmac_socfpga.  
> 
> Hi Maxime,
> 
> I'm sorry, but I think you need an extra patch added to this series.

Gosh you're right... The same this also goes for altera_tse... 

> Other than that, the series looks good. Thanks.

I'll followup shortly then. Nice catch !

Thanks,

Maxime