From 377d66fa86654085be1f48906c1d88b7ca721c78 Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 9 Dec 2025 01:28:20 +0000 Subject: [PATCH 1/4] net: dsa: lantiq_gswip: fix order in .remove operation Russell King pointed out that disabling the switch by clearing GSWIP_MDIO_GLOB_ENABLE before calling dsa_unregister_switch() is problematic, as it violates a Golden Rule of driver development to always first unpublish userspace interfaces and then disable the hardware. Fix this, and also simplify the probe() function, by introducing a dsa_switch_ops teardown() operation which takes care of clearing the GSWIP_MDIO_GLOB_ENABLE bit. Fixes: 14fceff4771e5 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200") Suggested-by: "Russell King (Oracle)" Signed-off-by: Daniel Golle Link: https://patch.msgid.link/4ebd72a29edc1e4059b9666a26a0bb5d906a829a.1765241054.git.daniel@makrotopia.org Reviewed-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- drivers/net/dsa/lantiq/lantiq_gswip.c | 3 --- drivers/net/dsa/lantiq/lantiq_gswip_common.c | 13 ++++++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/net/dsa/lantiq/lantiq_gswip.c b/drivers/net/dsa/lantiq/lantiq_gswip.c index 57dd063c0740..b094001a7c80 100644 --- a/drivers/net/dsa/lantiq/lantiq_gswip.c +++ b/drivers/net/dsa/lantiq/lantiq_gswip.c @@ -444,9 +444,6 @@ static void gswip_remove(struct platform_device *pdev) if (!priv) return; - /* disable the switch */ - gswip_disable_switch(priv); - dsa_unregister_switch(priv->ds); for (i = 0; i < priv->num_gphy_fw; i++) diff --git a/drivers/net/dsa/lantiq/lantiq_gswip_common.c b/drivers/net/dsa/lantiq/lantiq_gswip_common.c index 9da39edf8f57..6b171d58e186 100644 --- a/drivers/net/dsa/lantiq/lantiq_gswip_common.c +++ b/drivers/net/dsa/lantiq/lantiq_gswip_common.c @@ -752,6 +752,13 @@ static int gswip_setup(struct dsa_switch *ds) return 0; } +static void gswip_teardown(struct dsa_switch *ds) +{ + struct gswip_priv *priv = ds->priv; + + regmap_clear_bits(priv->mdio, GSWIP_MDIO_GLOB, GSWIP_MDIO_GLOB_ENABLE); +} + static enum dsa_tag_protocol gswip_get_tag_protocol(struct dsa_switch *ds, int port, enum dsa_tag_protocol mp) @@ -1629,6 +1636,7 @@ static const struct phylink_mac_ops gswip_phylink_mac_ops = { static const struct dsa_switch_ops gswip_switch_ops = { .get_tag_protocol = gswip_get_tag_protocol, .setup = gswip_setup, + .teardown = gswip_teardown, .port_setup = gswip_port_setup, .port_enable = gswip_port_enable, .port_disable = gswip_port_disable, @@ -1718,15 +1726,14 @@ int gswip_probe_common(struct gswip_priv *priv, u32 version) err = gswip_validate_cpu_port(priv->ds); if (err) - goto disable_switch; + goto unregister_switch; dev_info(priv->dev, "probed GSWIP version %lx mod %lx\n", GSWIP_VERSION_REV(version), GSWIP_VERSION_MOD(version)); return 0; -disable_switch: - gswip_disable_switch(priv); +unregister_switch: dsa_unregister_switch(priv->ds); return err; From 8e4c0f08f6bedeb885515c5ec5a6388a45d768ec Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 9 Dec 2025 01:28:49 +0000 Subject: [PATCH 2/4] net: dsa: mxl-gsw1xx: fix order in .remove operation The driver's .remove operation was calling gswip_disable_switch() which clears the GSWIP_MDIO_GLOB_ENABLE bit before calling dsa_unregister_switch() and thereby violating a Golden Rule of driver development to always unpublish userspace interfaces before disabling hardware, as pointed out by Russell King. Fix this by relying in GSWIP_MDIO_GLOB_ENABLE being cleared by the .teardown operation introduced by the previous commit ("net: dsa: lantiq_gswip: fix teardown order"). Fixes: 22335939ec907 ("net: dsa: add driver for MaxLinear GSW1xx switch family") Suggested-by: "Russell King (Oracle)" Signed-off-by: Daniel Golle Link: https://patch.msgid.link/63f882eeb910cf24503c35a443b541cc54a930f2.1765241054.git.daniel@makrotopia.org Reviewed-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- drivers/net/dsa/lantiq/mxl-gsw1xx.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c index cf33a16fd183..cda966d71e88 100644 --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c @@ -652,8 +652,6 @@ static void gsw1xx_remove(struct mdio_device *mdiodev) if (!priv) return; - gswip_disable_switch(priv); - dsa_unregister_switch(priv->ds); } From 651b253b80379b0eb3669405fcf50d4039dc7a0e Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 9 Dec 2025 01:29:05 +0000 Subject: [PATCH 3/4] net: dsa: mxl-gsw1xx: fix .shutdown driver operation The .shutdown operation should call dsa_switch_shutdown() just like it is done also by the sibling lantiq_gswip driver. Not doing that results in shutdown or reboot hanging and waiting for the CPU port becoming free, which introduces a longer delay and a WARNING before shutdown or reboot in case the driver is built-into the kernel. Fix this by calling dsa_switch_shutdown() in the driver's shutdown operation, harmonizing it with what is done in the lantiq_gswip driver. As a side-effect this now allows to remove the previously exported gswip_disable_switch() function which no longer got any users. Fixes: 22335939ec907 ("net: dsa: add driver for MaxLinear GSW1xx switch family") Signed-off-by: Daniel Golle Link: https://patch.msgid.link/77ed91a5206e5dbf5d3e83d7e364ebfda90d31fd.1765241054.git.daniel@makrotopia.org Reviewed-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- drivers/net/dsa/lantiq/lantiq_gswip.h | 2 -- drivers/net/dsa/lantiq/lantiq_gswip_common.c | 6 ------ drivers/net/dsa/lantiq/mxl-gsw1xx.c | 4 ++-- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/drivers/net/dsa/lantiq/lantiq_gswip.h b/drivers/net/dsa/lantiq/lantiq_gswip.h index 9c38e51a75e8..2e0f2afbadbb 100644 --- a/drivers/net/dsa/lantiq/lantiq_gswip.h +++ b/drivers/net/dsa/lantiq/lantiq_gswip.h @@ -294,8 +294,6 @@ struct gswip_priv { u16 version; }; -void gswip_disable_switch(struct gswip_priv *priv); - int gswip_probe_common(struct gswip_priv *priv, u32 version); #endif /* __LANTIQ_GSWIP_H */ diff --git a/drivers/net/dsa/lantiq/lantiq_gswip_common.c b/drivers/net/dsa/lantiq/lantiq_gswip_common.c index 6b171d58e186..e790f2ef7588 100644 --- a/drivers/net/dsa/lantiq/lantiq_gswip_common.c +++ b/drivers/net/dsa/lantiq/lantiq_gswip_common.c @@ -1664,12 +1664,6 @@ static const struct dsa_switch_ops gswip_switch_ops = { .port_hsr_leave = dsa_port_simple_hsr_leave, }; -void gswip_disable_switch(struct gswip_priv *priv) -{ - regmap_clear_bits(priv->mdio, GSWIP_MDIO_GLOB, GSWIP_MDIO_GLOB_ENABLE); -} -EXPORT_SYMBOL_GPL(gswip_disable_switch); - static int gswip_validate_cpu_port(struct dsa_switch *ds) { struct gswip_priv *priv = ds->priv; diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c index cda966d71e88..4dc287ad141e 100644 --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c @@ -662,9 +662,9 @@ static void gsw1xx_shutdown(struct mdio_device *mdiodev) if (!priv) return; - dev_set_drvdata(&mdiodev->dev, NULL); + dsa_switch_shutdown(priv->ds); - gswip_disable_switch(priv); + dev_set_drvdata(&mdiodev->dev, NULL); } static const struct gswip_hw_info gsw12x_data = { From 7b103aaf0d564b83ee1d4bb532ee7ae36ed001ed Mon Sep 17 00:00:00 2001 From: Daniel Golle Date: Tue, 9 Dec 2025 01:29:34 +0000 Subject: [PATCH 4/4] net: dsa: mxl-gsw1xx: manually clear RANEG bit Despite being documented as self-clearing, the RANEG bit sometimes remains set, preventing auto-negotiation from happening. Manually clear the RANEG bit after 10ms as advised by MaxLinear. In order to not hold RTNL during the 10ms of waiting schedule delayed work to take care of clearing the bit asynchronously, which is similar to the self-clearing behavior. Fixes: 22335939ec90 ("net: dsa: add driver for MaxLinear GSW1xx switch family") Reported-by: Rasmus Villemoes Signed-off-by: Daniel Golle Link: https://patch.msgid.link/76745fceb5a3f53088110fb7a96acf88434088ca.1765241054.git.daniel@makrotopia.org Reviewed-by: Vladimir Oltean Signed-off-by: Paolo Abeni --- drivers/net/dsa/lantiq/mxl-gsw1xx.c | 34 ++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c index 4dc287ad141e..f8ff8a604bf5 100644 --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c @@ -11,10 +11,12 @@ #include #include +#include #include #include #include #include +#include #include #include "lantiq_gswip.h" @@ -29,6 +31,7 @@ struct gsw1xx_priv { struct regmap *clk; struct regmap *shell; struct phylink_pcs pcs; + struct delayed_work clear_raneg; phy_interface_t tbi_interface; struct gswip_priv gswip; }; @@ -145,7 +148,9 @@ static void gsw1xx_pcs_disable(struct phylink_pcs *pcs) { struct gsw1xx_priv *priv = pcs_to_gsw1xx(pcs); - /* Assert SGMII shell reset */ + cancel_delayed_work_sync(&priv->clear_raneg); + + /* Assert SGMII shell reset (will also clear RANEG bit) */ regmap_set_bits(priv->shell, GSW1XX_SHELL_RST_REQ, GSW1XX_RST_REQ_SGMII_SHELL); @@ -428,12 +433,29 @@ static int gsw1xx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode, return 0; } +static void gsw1xx_pcs_clear_raneg(struct work_struct *work) +{ + struct gsw1xx_priv *priv = + container_of(work, struct gsw1xx_priv, clear_raneg.work); + + regmap_clear_bits(priv->sgmii, GSW1XX_SGMII_TBI_ANEGCTL, + GSW1XX_SGMII_TBI_ANEGCTL_RANEG); +} + static void gsw1xx_pcs_an_restart(struct phylink_pcs *pcs) { struct gsw1xx_priv *priv = pcs_to_gsw1xx(pcs); + cancel_delayed_work_sync(&priv->clear_raneg); + regmap_set_bits(priv->sgmii, GSW1XX_SGMII_TBI_ANEGCTL, GSW1XX_SGMII_TBI_ANEGCTL_RANEG); + + /* despite being documented as self-clearing, the RANEG bit + * sometimes remains set, preventing auto-negotiation from happening. + * MaxLinear advises to manually clear the bit after 10ms. + */ + schedule_delayed_work(&priv->clear_raneg, msecs_to_jiffies(10)); } static void gsw1xx_pcs_link_up(struct phylink_pcs *pcs, @@ -636,6 +658,8 @@ static int gsw1xx_probe(struct mdio_device *mdiodev) if (ret) return ret; + INIT_DELAYED_WORK(&priv->clear_raneg, gsw1xx_pcs_clear_raneg); + ret = gswip_probe_common(&priv->gswip, version); if (ret) return ret; @@ -648,16 +672,21 @@ static int gsw1xx_probe(struct mdio_device *mdiodev) static void gsw1xx_remove(struct mdio_device *mdiodev) { struct gswip_priv *priv = dev_get_drvdata(&mdiodev->dev); + struct gsw1xx_priv *gsw1xx_priv; if (!priv) return; dsa_unregister_switch(priv->ds); + + gsw1xx_priv = container_of(priv, struct gsw1xx_priv, gswip); + cancel_delayed_work_sync(&gsw1xx_priv->clear_raneg); } static void gsw1xx_shutdown(struct mdio_device *mdiodev) { struct gswip_priv *priv = dev_get_drvdata(&mdiodev->dev); + struct gsw1xx_priv *gsw1xx_priv; if (!priv) return; @@ -665,6 +694,9 @@ static void gsw1xx_shutdown(struct mdio_device *mdiodev) dsa_switch_shutdown(priv->ds); dev_set_drvdata(&mdiodev->dev, NULL); + + gsw1xx_priv = container_of(priv, struct gsw1xx_priv, gswip); + cancel_delayed_work_sync(&gsw1xx_priv->clear_raneg); } static const struct gswip_hw_info gsw12x_data = {