[PATCH net-next v2 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios

[PATCH net-next v2 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios

[Joseph Telaak via B4 Relay]

The GSW1xx driver never resets the switch, and the MDIO core does not
handle reset-gpios for non-PHY devices, so the driver works with whatever
the bootloader configured. On the GL.iNet GL-SFT1200 (Siflower SF19A2890 +
GSW150) the bootloader leaves the internal PHYs one MDIO address too high,
and the WAN port has no PHY.

Patch 1 allows reset-gpios in the binding for the standalone switches.
Patch 2 pulses it at probe.

This is separate from commit 82e2c7db0830 ("net: dsa: mxl-gsw1xx: force
internal PHYs into a known reset state"), which only programs the PHY
bits of RST_REQ. On this board that commit is applied, and the PHYs still
sit at the shifted addresses; only a hardware reset of the switch moves
them back.

Tested on a GL.iNet GL-SFT1200 (Intel GSW150 / PEB7084 at SMI address
0x1f, reset on an active-low SoC GPIO), on v7.4-rc1 with
CONFIG_RESET_GPIO=y: after a warm reboot and after a cold power-on, all
three ports find their PHYs at addresses 0 to 2 and link at 1 Gbit/s.
Without the patch, port 0 fails with -ENODEV after every boot.

There is no in-tree user yet: the GL-SFT1200's device tree is under
review on linux-mips without the switch, which follows once the SoC's
GMAC is supported.

---
Changes in v2:
- Use the reset controller API, so reset-gpios goes through reset-gpio,
  instead of driving the GPIO from the driver (Daniel).
- Use the vendor driver's reset timing, 250 ms asserted and 1 s before
  the first access. v1 gave 2 ms and 100 ms as the vendor's, which they
  are not.
- Patch 1: add Rob's Ack.
- Link to v1: https://patch.msgid.link/20261104-mxl-gsw1xx-reset-v1-0-53839bf5c88e@telaak.dev

---
Joseph Telaak (2):
      dt-bindings: net: dsa: lantiq,gswip: add reset-gpios
      net: dsa: mxl-gsw1xx: reset the switch through reset-gpios

 .../devicetree/bindings/net/dsa/lantiq,gswip.yaml  |  6 ++++++
 drivers/net/dsa/lantiq/mxl-gsw1xx.c                | 25 ++++++++++++++++++++++
 drivers/net/dsa/lantiq/mxl-gsw1xx.h                |  4 ++++
 3 files changed, 35 insertions(+)
---
base-commit: 4dd16e31135ea8437a720222df34e8745f385d6d
change-id: 20261102-mxl-gsw1xx-reset-37ac836b31fe

Best regards,
--  
Joseph Telaak <joseph@telaak.dev>

[PATCH net-next v2 1/2] dt-bindings: net: dsa: lantiq,gswip: add reset-gpios

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

The standalone GSW1xx switches have a hardware reset input, and boards
wire it to a SoC GPIO. Allow describing it. The SoC-integrated GSWIP
variants have no such pin.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml b/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml
index 49af5573e4..9a74ea2a6d 100644
--- a/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml
@@ -40,6 +40,11 @@ properties:
       - const: mdio
       - const: mii
 
+  reset-gpios:
+    maxItems: 1
+    description:
+      GPIO connected to the hardware reset input of a standalone switch.
+
   mdio:
     $ref: /schemas/net/mdio.yaml#
     unevaluatedProperties: false
@@ -173,6 +178,7 @@ allOf:
         mdio:
           required:
             - compatible
+        reset-gpios: false
     else:
       properties:
         reg:

-- 
Git-155)

[PATCH net-next v2 2/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios

[Joseph Telaak via B4 Relay]

From: Joseph Telaak <joseph@telaak.dev>

The switch keeps the configuration the bootloader left in it, and the
driver never resets it. The MDIO core asserts and releases reset-gpios
only for PHY devices, so a reset line described on the switch node has
no effect either.

On the GL.iNet GL-SFT1200 the bootloader leaves each internal PHY of
the GSW150 one MDIO address above its port. Port 0 then fails with
"failed to connect to PHY: -ENODEV", and the PHY that answers at a
port's address belongs to the neighboring port. After a hardware reset
every PHY is back at its port's address and all ports work.

Get the reset through the reset controller API and pulse it before the
first register access. The reset core hands a reset-gpios property to
the reset-gpio driver, so a board that wires the reset this way needs
CONFIG_RESET_GPIO; without it the reset is skipped. The timing follows
the vendor's driver for the GSW150: asserted for 250 ms, then 1 s before
the switch is accessed.

Signed-off-by: Joseph Telaak <joseph@telaak.dev>
---
 drivers/net/dsa/lantiq/mxl-gsw1xx.c | 25 +++++++++++++++++++++++++
 drivers/net/dsa/lantiq/mxl-gsw1xx.h |  4 ++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
index a26f78e1a4..94a7eff5e0 100644
--- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
+++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
@@ -18,6 +18,7 @@
 #include <linux/phy/phy-common-props.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/workqueue.h>
 #include <net/dsa.h>
 
@@ -719,6 +720,7 @@ static int gsw1xx_serdes_pcs_init(struct gsw1xx_priv *priv)
 static int gsw1xx_probe(struct mdio_device *mdiodev)
 {
 	struct device *dev = &mdiodev->dev;
+	struct reset_control *reset;
 	struct gsw1xx_priv *priv;
 	u32 version, val;
 	u8 shellver;
@@ -737,6 +739,29 @@ static int gsw1xx_probe(struct mdio_device *mdiodev)
 	if (!priv->gswip.hw_info)
 		return -EINVAL;
 
+	/* The MDIO core handles reset-gpios only for PHYs. Reset the switch
+	 * here, so it does not run with whatever the bootloader configured.
+	 */
+	reset = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (IS_ERR(reset))
+		return dev_err_probe(dev, PTR_ERR(reset),
+				     "failed to get reset control\n");
+
+	if (reset) {
+		ret = reset_control_assert(reset);
+		if (ret)
+			return dev_err_probe(dev, ret, "failed to assert reset\n");
+
+		fsleep(GSW1XX_RESET_ASSERT_US);
+
+		ret = reset_control_deassert(reset);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to deassert reset\n");
+
+		fsleep(GSW1XX_RESET_DEASSERT_US);
+	}
+
 	priv->gswip.gswip = gsw1xx_regmap_init(priv, "switch",
 					       GSW1XX_SWITCH_BASE, 0xfff);
 	if (IS_ERR(priv->gswip.gswip))
diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.h b/drivers/net/dsa/lantiq/mxl-gsw1xx.h
index 81bc00eabb..bca8e3223c 100644
--- a/drivers/net/dsa/lantiq/mxl-gsw1xx.h
+++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.h
@@ -19,6 +19,10 @@
 
 #define GSW1XX_SYS_CLK_FREQ			340000000
 
+/* Hardware reset timing of the vendor's driver for the GSW150 */
+#define GSW1XX_RESET_ASSERT_US			250000
+#define GSW1XX_RESET_DEASSERT_US		1000000
+
 /* SMDIO switch register base address */
 #define GSW1XX_SMDIO_BADR			0x1f
 #define  GSW1XX_SMDIO_BADR_UNKNOWN		-1

-- 
Git-155)

Re: [PATCH net-next v2 2/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The hardware reset only happens when CONFIG_RESET_CONTROLLER
  and CONFIG_RESET_GPIO are both enabled, and NET_DSA_MXL_GSW1XX does…
- [Medium] The new full-chip hardware reset in gsw1xx_probe()
  (drivers/net/dsa/lantiq/mxl-gsw1xx.c:750-763) contradicts the binding…
- [Low] gsw1xx_probe() (drivers/net/dsa/lantiq/mxl-gsw1xx.c:750-763)
  asserts the reset, sleeps 250 ms, deasserts and sleeps another 1 s…

--

net: dsa: mxl-gsw1xx: reset the switch through reset-gpios

The commit adds a hardware reset pulse to gsw1xx_probe() through the reset
controller API. The reset is held for 250 ms, followed by a 1 s wait before
the first register access. The goal is to clear the bootloader state that
leaves the internal PHYs of the GSW150 at shifted MDIO addresses on the
GL.iNet GL-SFT1200.

> diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> index a26f78e1a44ce..94a7eff5e0077 100644
> --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
> +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c

[ ... ]

> @@ -737,6 +739,29 @@ static int gsw1xx_probe(struct mdio_device *mdiodev)
>  	if (!priv->gswip.hw_info)
>  		return -EINVAL;
>  
> +	/* The MDIO core handles reset-gpios only for PHYs. Reset the switch
> +	 * here, so it does not run with whatever the bootloader configured.
> +	 */
> +	reset = devm_reset_control_get_optional_exclusive(dev, NULL);
> +	if (IS_ERR(reset))
> +		return dev_err_probe(dev, PTR_ERR(reset),
> +				     "failed to get reset control\n");

[Severity: Medium]
If CONFIG_RESET_GPIO is disabled, does this lookup silently return NULL even
when the switch node has reset-gpios?

drivers/reset/core.c:__fwnode_reset_control_get() {
    ...
	if (!IS_ENABLED(CONFIG_RESET_GPIO))
		return optional ? NULL : ERR_PTR(ret);
    ...
}

With CONFIG_RESET_CONTROLLER=n, the optional stub also returns NULL. In
both cases the if (reset) block is skipped and nothing is logged. The switch
then keeps the bootloader state, and on the GL-SFT1200 port 0 would still
fail with "failed to connect to PHY: -ENODEV".

NET_DSA_MXL_GSW1XX in drivers/net/dsa/lantiq/Kconfig does not select, imply
or depend on RESET_CONTROLLER or RESET_GPIO. The reset-gpios entry added to
lantiq,gswip.yaml in this series ("dt-bindings: net: dsa: lantiq,gswip: add
reset-gpios") describes the property with no such condition. The commit
message mentions the requirement, but neither Kconfig nor the driver at
runtime does.

No Kconfig in the tree selects RESET_GPIO, and it sits inside the
RESET_CONTROLLER menuconfig, so selecting it is not straightforward.

Other DSA drivers (mv88e6xxx, rtl83xx, qca8k, ksz_common, mt7530-mdio,
lan9303, vitesse-vsc73xx) read reset-gpios with
devm_gpiod_get_optional(dev, "reset", ...), which only needs GPIOLIB.

Would a gpiod based lookup, or a gpiod fallback, make the documented
property work whatever the reset framework config is?

> +
> +	if (reset) {
> +		ret = reset_control_assert(reset);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "failed to assert reset\n");

[Severity: Medium]
Does this full chip reset conflict with the slew rate binding? For
maxlinear,slew-rate-txc and maxlinear,slew-rate-txd, lantiq,gswip.yaml
says:

  If not present, the configuration made by the switch bootloader is
  preserved.

gsw1xx_rmii_slew_rate() relies on that and skips the write when the
property is absent:

	ret = of_property_read_u32(np, prop, &rate);
	/* Optional property */
	if (ret == -EINVAL)
		return 0;

When a board adds reset-gpios, the bootloader's GSW1XX_SHELL_RGMII_SLEW_CFG
setup would be replaced by power-on defaults before the driver touches the
switch.

RGMII_SLEW_CFG_RX_2_5_V and RGMII_SLEW_CFG_TX_2_5_V are defined in
mxl-gsw1xx.h but never written anywhere in drivers/net/dsa/lantiq/. Suppose
a bootloader set them for 2.5 V RGMII signalling. Could the RGMII CPU link
be degraded or stop working after this reset?

The actual hardware impact depends on the GSW1xx reset defaults and straps.
The GSWIP_SWRES_R0 reset in gswip_setup() only covers the switch core
register block, so this pad state was not reset before this patch.

> +
> +		fsleep(GSW1XX_RESET_ASSERT_US);
> +
> +		ret = reset_control_deassert(reset);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "failed to deassert reset\n");
> +
> +		fsleep(GSW1XX_RESET_DEASSERT_US);
> +	}

[Severity: Low]
This runs before gswip_probe_common()->dsa_register_switch(), which can
return -EPROBE_DEFER when the conduit netdev is not available yet:

drivers/net/dsa/lantiq/lantiq_gswip_common.c:gswip_probe_common() {
    ...
	err = dsa_register_switch(priv->ds);
	if (err)
		return dev_err_probe(priv->dev, err, "dsa switch registration failed\n");
    ...
}

Does each deferred probe then repeat the full 250 ms + 1 s blocking pulse?

If the conduit Ethernet driver probes late or is built as a module, each
retry would add about 1.25 s to boot and reset the switch again.

> +
>  	priv->gswip.gswip = gsw1xx_regmap_init(priv, "switch",
>  					       GSW1XX_SWITCH_BASE, 0xfff);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261108-mxl-gsw1xx-reset-v2-0-46c1da093893%40telaak.dev

Re: [PATCH net-next v2 2/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios

[Joseph Telaak]

> If CONFIG_RESET_GPIO is disabled, does this lookup silently return NULL even
> when the switch node has reset-gpios?

Yes, as for every driver that gets its reset-gpios through the
reset-gpio fallback. A platform that uses reset-gpios enables
RESET_GPIO.

> Would a gpiod based lookup, or a gpiod fallback, make the documented
> property work whatever the reset framework config is?

v1 did that; Daniel asked for reset-gpio instead, so v2 uses the reset
controller API.

> Does this full chip reset conflict with the slew rate binding?

It does: a board that adds reset-gpios gets the power-on defaults for
the slew rates it does not set. v3 will say so in the binding.

> Does each deferred probe then repeat the full 250 ms + 1 s blocking pulse?

Yes. The switch is not registered at that point, so the cost is the
delay, and only when the conduit defers.

pw-bot: cr