* [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios
@ 2026-11-12 21:39 Joseph Telaak via B4 Relay
2026-11-12 21:39 ` [PATCH net-next v3 1/2] dt-bindings: net: dsa: lantiq,gswip: add reset-gpios Joseph Telaak via B4 Relay
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Joseph Telaak via B4 Relay @ 2026-11-12 21:39 UTC (permalink / raw)
To: Hauke Mehrtens, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Daniel Golle, Philipp Zabel, netdev, devicetree, linux-kernel,
Joseph Telaak
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 v3:
- Patch 1: say in the slew-rate descriptions that, without the
properties, a reset through reset-gpios leaves the power-on defaults.
Rob's Ack is kept: reset-gpios itself is unchanged.
- Patch 2: say the same in the commit message, and wrap a line over 80
columns.
- Cc Philipp Zabel for the reset controller API.
- Link to v2: https://patch.msgid.link/20261108-mxl-gsw1xx-reset-v2-0-46c1da093893@telaak.dev
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 | 12 ++++++++--
drivers/net/dsa/lantiq/mxl-gsw1xx.c | 26 ++++++++++++++++++++++
drivers/net/dsa/lantiq/mxl-gsw1xx.h | 4 ++++
3 files changed, 40 insertions(+), 2 deletions(-)
---
base-commit: 4dd16e31135ea8437a720222df34e8745f385d6d
change-id: 20261102-mxl-gsw1xx-reset-37ac836b31fe
Best regards,
--
Joseph Telaak <joseph@telaak.dev>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH net-next v3 1/2] dt-bindings: net: dsa: lantiq,gswip: add reset-gpios 2026-11-12 21:39 [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios Joseph Telaak via B4 Relay @ 2026-11-12 21:39 ` Joseph Telaak via B4 Relay 2026-11-13 21:40 ` sashiko-bot 2026-11-12 21:39 ` [PATCH net-next v3 2/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios Joseph Telaak via B4 Relay 2026-11-13 14:08 ` [PATCH net-next v3 0/2] " Andrew Lunn 2 siblings, 1 reply; 10+ messages in thread From: Joseph Telaak via B4 Relay @ 2026-11-12 21:39 UTC (permalink / raw) To: Hauke Mehrtens, Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Daniel Golle, Philipp Zabel, netdev, devicetree, linux-kernel, Joseph Telaak 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. A reset through it returns the RGMII slew rates to their power-on defaults, so say so where the slew-rate properties say that the bootloader's configuration is otherwise preserved. Acked-by: Rob Herring (Arm) <robh@kernel.org> Signed-off-by: Joseph Telaak <joseph@telaak.dev> --- Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml b/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml index 49af5573e4..89f78a1bc1 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 @@ -120,7 +125,8 @@ patternProperties: 1: Slow If not present, the configuration made by the switch bootloader is - preserved. + preserved, or the power-on default after a reset through + reset-gpios. maxlinear,slew-rate-txd: $ref: /schemas/types.yaml#/definitions/uint32 enum: [0, 1] @@ -131,7 +137,8 @@ patternProperties: 1: Slow If not present, the configuration made by the switch bootloader is - preserved. + preserved, or the power-on default after a reset through + reset-gpios. maxlinear,rmii-refclk-out: type: boolean description: @@ -173,6 +180,7 @@ allOf: mdio: required: - compatible + reset-gpios: false else: properties: reg: -- Git-155) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 1/2] dt-bindings: net: dsa: lantiq,gswip: add reset-gpios 2026-11-12 21:39 ` [PATCH net-next v3 1/2] dt-bindings: net: dsa: lantiq,gswip: add reset-gpios Joseph Telaak via B4 Relay @ 2026-11-13 21:40 ` sashiko-bot 0 siblings, 0 replies; 10+ messages in thread From: sashiko-bot @ 2026-11-13 21:40 UTC (permalink / raw) To: Joseph Telaak; +Cc: devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Low] dt-bindings: net: dsa: lantiq,gswip: split GSW1xx into separate schema -- commit 3590cf6c71e15fdb259e4e21a87a5fb546f5d150 Author: Joseph Telaak <joseph@telaak.dev> dt-bindings: net: dsa: lantiq,gswip: add reset-gpios This commit adds the reset-gpios property to the lantiq,gswip binding for standalone GSW1xx switches and updates the RGMII slew-rate descriptions to note that a hardware reset restores power-on defaults. > diff --git a/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml b/Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml > index 49af5573e452d..89f78a1bc113d 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 +180,7 @@ allOf: > mdio: > required: > - compatible > + reset-gpios: false > else: > properties: > reg: [Severity: Low] This isn't a bug, but would it make sense to split the standalone MDIO-based GSW1xx switches into a separate binding schema instead of adding more per-compatible exclusions here? The SoC-integrated GSWIP variants and standalone GSW1xx switches sit on different buses and already override or disable almost every top-level property outside compatible and ports (reg, reg-names, mdio, gphy-fw, and now reset-gpios) in the allOf if-then-else block. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261112-mxl-gsw1xx-reset-v3-0-863e82074893@telaak.dev?part=1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net-next v3 2/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios 2026-11-12 21:39 [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios Joseph Telaak via B4 Relay 2026-11-12 21:39 ` [PATCH net-next v3 1/2] dt-bindings: net: dsa: lantiq,gswip: add reset-gpios Joseph Telaak via B4 Relay @ 2026-11-12 21:39 ` Joseph Telaak via B4 Relay 2026-11-13 21:56 ` netdev-bot+sashiko 2026-11-13 14:08 ` [PATCH net-next v3 0/2] " Andrew Lunn 2 siblings, 1 reply; 10+ messages in thread From: Joseph Telaak via B4 Relay @ 2026-11-12 21:39 UTC (permalink / raw) To: Hauke Mehrtens, Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Daniel Golle, Philipp Zabel, netdev, devicetree, linux-kernel, Joseph Telaak 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. The reset also returns the RGMII slew rates to their power-on defaults, so a board that uses reset-gpios sets the ones it needs in its device tree. Signed-off-by: Joseph Telaak <joseph@telaak.dev> --- drivers/net/dsa/lantiq/mxl-gsw1xx.c | 26 ++++++++++++++++++++++++++ drivers/net/dsa/lantiq/mxl-gsw1xx.h | 4 ++++ 2 files changed, 30 insertions(+) diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c index a26f78e1a4..f69856a65b 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,30 @@ 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) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 2/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios 2026-11-12 21:39 ` [PATCH net-next v3 2/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios Joseph Telaak via B4 Relay @ 2026-11-13 21:56 ` netdev-bot+sashiko 0 siblings, 0 replies; 10+ messages in thread From: netdev-bot+sashiko @ 2026-11-13 21:56 UTC (permalink / raw) To: joseph Cc: hauke, andrew, olteanv, davem, edumazet, kuba, pabeni, robh, krzk+dt, conor+dt, daniel, p.zabel, netdev, devicetree, linux-kernel Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider. Critical: 0 · High: 0 · Medium: 1 · Low: 3 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] In gsw1xx_probe() (drivers/net/dsa/lantiq/mxl-gsw1xx.c:745), the switch reset is fetched with… - [Low] gsw1xx_probe() (drivers/net/dsa/lantiq/mxl-gsw1xx.c:750-764) pulses the reset and sleeps for a fixed 1.25 s on every probe attempt:… - [Low] The new comment in gsw1xx_probe() says the driver handles the reset itself because "The MDIO core handles reset-gpios only for… - [Low] GSW1XX_RESET_ASSERT_US (250000) and GSW1XX_RESET_DEASSERT_US (1000000) in drivers/net/dsa/lantiq/mxl-gsw1xx.h:22-24 are documented… -- net: dsa: mxl-gsw1xx: reset the switch through reset-gpios This patch makes gsw1xx_probe() pulse the switch reset before the first register access. The reset is obtained through the reset controller API. Without it, the bootloader's configuration stays in place, and on the GL-SFT1200 the internal PHYs end up at the wrong MDIO addresses. The timing follows the GSW150 vendor driver: 250 ms asserted, then 1 s before the switch is accessed. > diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c > index a26f78e1a44ce..f69856a65befa 100644 > --- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c > +++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c [ ... ] > @@ -737,6 +739,30 @@ 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) { [Severity: Medium] What happens here when the kernel is built with CONFIG_RESET_GPIO=n (or CONFIG_RESET_CONTROLLER=n) and the switch node only has reset-gpios? In drivers/reset/core.c, the reset-gpios fallback is only tried when RESET_GPIO is enabled. Otherwise an optional get returns NULL: drivers/reset/core.c:__fwnode_reset_control_get() { ... if (!IS_ENABLED(CONFIG_RESET_GPIO)) return optional ? NULL : ERR_PTR(ret); ... } The !CONFIG_RESET_CONTROLLER stubs in include/linux/reset.h also return NULL for optional gets. gsw1xx_probe() treats NULL as "no reset wired". It skips the if (reset) block and logs nothing. NET_DSA_MXL_GSW1XX in drivers/net/dsa/lantiq/Kconfig also has no select or imply for RESET_CONTROLLER or RESET_GPIO. On such a kernel, wouldn't a GL-SFT1200 whose DT describes reset-gpios silently keep the bootloader configuration? Port 0 would then fail again with "failed to connect to PHY: -ENODEV". The binding text "or the power-on default after a reset through reset-gpios" would not hold on those kernels either. The commit message mentions the CONFIG_RESET_GPIO requirement, but there is no Kconfig link and no runtime message. The MDIO core in mdio_device_register_reset() and other DSA drivers call gpiod_get_optional(dev, "reset", ...) directly, which only needs GPIOLIB. Would it work to use gpiod here, or add an imply/select? At minimum, could the driver warn when reset-gpios is present but no reset control was obtained? > + 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"); [Severity: Low] The comment above gives "The MDIO core handles reset-gpios only for PHYs" as the reason for doing this here. Does this cover everything the core does with the reset line? In drivers/net/phy/mdio_device.c, mdio_probe() asserts the reset again if the driver's probe fails, and mdio_remove() always asserts it: drivers/net/phy/mdio_device.c:mdio_probe() { ... err = mdiodrv->probe(mdiodev); if (err) { /* Assert the reset signal */ mdio_device_reset(mdiodev, 1); } ... } The core only sets up reset_ctrl and reset_gpio for MDIO_DEVICE_FLAG_PHY, so those calls do nothing for this switch. In gsw1xx_probe(), none of the later error returns assert the reset again. That covers regmap init, -ENODEV on a chip ID mismatch, gsw1xx_serdes_pcs_init(), the GSWIP_VERSION read, and gswip_probe_common(), including -EPROBE_DEFER. gsw1xx_remove() and gsw1xx_shutdown() never touch the reset either. Should the switch be put back into reset on probe failure and on remove? lan9303-core.c, for example, asserts reset on removal "to prevent it from doing anything". The switch was already left running after unbind before this patch, so the change is mostly the state after a failed probe. > + > + fsleep(GSW1XX_RESET_DEASSERT_US); > + } [Severity: Low] Is the reset pulse, with 1.25 s of sleeping, repeated on every deferred probe attempt? Nothing records that the reset already happened. Later in the same probe, gswip_probe_common()->dsa_register_switch() can return -EPROBE_DEFER, for example when the conduit netdev is not registered yet: net/dsa/dsa.c:dsa_port_parse_of() { ... if (!conduit) { rtnl_unlock(); return -EPROBE_DEFER; } ... } It can also defer when the tagger is missing. gswip_probe_common() passes the error back through dev_err_probe(). Each retry would reset the switch again and block the deferred probe worker for another 250 ms + 1 s, so boot time grows with every deferral. The commit message does not mention this. > + > 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 81bc00eabbfb3..bca8e3223cd60 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 [Severity: Low] These values are described as the vendor's timing for the GSW150. However, gsw1xx_probe() applies them to every compatible in gsw1xx_of_match, including the GSW120, GSW125, GSW140, GSW141 and GSW145. Are these timings right for the other variants? The reset-assert-us and reset-deassert-us properties are also never read, so a board cannot adjust the timing. These properties are defined for MDIO bus children in Documentation/devicetree/bindings/net/mdio.yaml, and the MDIO core reads them: drivers/net/phy/mdio_device.c:mdio_device_register_reset() { ... device_property_read_u32(&mdiodev->dev, "reset-assert-us", &mdiodev->reset_assert_delay); device_property_read_u32(&mdiodev->dev, "reset-deassert-us", &mdiodev->reset_deassert_delay); ... } As a result, every variant with a reset line waits at least 1.25 s per probe attempt, and firmware cannot override it. The dependency on CONFIG_RESET_GPIO raised above has the same cause: mdio_device_register_reset() uses gpiod directly, while this patch goes through the reset_control API. > /* SMDIO switch register base address */ > #define GSW1XX_SMDIO_BADR 0x1f > #define GSW1XX_SMDIO_BADR_UNKNOWN -1 -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261112-mxl-gsw1xx-reset-v3-0-863e82074893%40telaak.dev ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios 2026-11-12 21:39 [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios Joseph Telaak via B4 Relay 2026-11-12 21:39 ` [PATCH net-next v3 1/2] dt-bindings: net: dsa: lantiq,gswip: add reset-gpios Joseph Telaak via B4 Relay 2026-11-12 21:39 ` [PATCH net-next v3 2/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios Joseph Telaak via B4 Relay @ 2026-11-13 14:08 ` Andrew Lunn 2026-11-13 15:10 ` Daniel Golle 2 siblings, 1 reply; 10+ messages in thread From: Andrew Lunn @ 2026-11-13 14:08 UTC (permalink / raw) To: joseph Cc: Hauke Mehrtens, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Daniel Golle, Philipp Zabel, netdev, devicetree, linux-kernel > 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. I don't think 82e2c7db0830 is relevant here. The commit message says nothing about the address of the PHY. How does the bootloader change the address? I assume there is a register it writes? Is there a comment about why it changes the address? Does the driver have an implicit assumption about the addresses of the PHYs? Should the driver actually be writing this register to put the PHYs at the expected address? Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios 2026-11-13 14:08 ` [PATCH net-next v3 0/2] " Andrew Lunn @ 2026-11-13 15:10 ` Daniel Golle 2026-11-13 21:05 ` Andrew Lunn 0 siblings, 1 reply; 10+ messages in thread From: Daniel Golle @ 2026-11-13 15:10 UTC (permalink / raw) To: Andrew Lunn Cc: joseph, Hauke Mehrtens, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, netdev, devicetree, linux-kernel On Fri, Nov 13, 2026 at 03:08:10PM +0100, Andrew Lunn wrote: > > 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. > > I don't think 82e2c7db0830 is relevant here. The commit message says > nothing about the address of the PHY. > > How does the bootloader change the address? I assume there is a > register it writes? Yes. PHY_ADDR_{0,1,2,3,4,5} at offsets 0xF415, 0xF414, 0xF413, 0xF412, 0xF411, 0xF410 (ie. reverse order). Bits 4:0 are the address of the respective built-in PHY. > > Is there a comment about why it changes the address? I *guess* that in this case the vendor wants to avoid using PHY address 0 which is widely misunderstood as being the "broadcast address" (which it is, but only in scope of AUI attachments and SFP modules). I know many people who get upset about using PHY address 0 for an actual PHY rather than reserving it as a broadcast address all PHYs on the bus are listening on... The switch also has it's own MDIO master interface exposed on external pins which is shared with the internal PHYs. Hence programming the addresses of the built-in PHYs can be done in order to accomodate external PHYs on the same bus which may not be free to use arbitrary addresses. > > Does the driver have an implicit assumption about the addresses of the > PHYs? Should the driver actually be writing this register to put the > PHYs at the expected address? The driver does what is described in the device tree. Ie. boot firmware applying a non-default PHY addresses should just say so in the DT passed to Linux imho. See also Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios 2026-11-13 15:10 ` Daniel Golle @ 2026-11-13 21:05 ` Andrew Lunn 2026-11-13 22:16 ` Joseph Telaak 0 siblings, 1 reply; 10+ messages in thread From: Andrew Lunn @ 2026-11-13 21:05 UTC (permalink / raw) To: Daniel Golle Cc: joseph, Hauke Mehrtens, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, netdev, devicetree, linux-kernel > I *guess* that in this case the vendor wants to avoid using PHY > address 0 which is widely misunderstood as being the "broadcast > address" (which it is, but only in scope of AUI attachments and SFP > modules). There are no standard for copper SFP modules, so i find it hard to accept that SFP modules expect 0 to work as a broadcast address. > > Does the driver have an implicit assumption about the addresses of the > > PHYs? Should the driver actually be writing this register to put the > > PHYs at the expected address? > > The driver does what is described in the device tree. Ie. boot firmware > applying a non-default PHY addresses should just say so in the DT passed > to Linux imho. And this is what i was wondering about. Is it the DT which is wrong, or the driver? Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios 2026-11-13 21:05 ` Andrew Lunn @ 2026-11-13 22:16 ` Joseph Telaak 2026-11-13 22:35 ` Joseph Telaak 0 siblings, 1 reply; 10+ messages in thread From: Joseph Telaak @ 2026-11-13 22:16 UTC (permalink / raw) To: Andrew Lunn Cc: Daniel Golle, Hauke Mehrtens, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, netdev, devicetree, linux-kernel On Fri, Nov 13, 2026 at 11:05:55PM +0100, Andrew Lunn wrote: > > The driver does what is described in the device tree. Ie. boot firmware > > applying a non-default PHY addresses should just say so in the DT passed > > to Linux imho. > > And this is what i was wondering about. Is it the DT which is wrong, > or the driver? The DT. The driver assumes nothing about the addresses; the board's device tree described the PHYs at their power-on addresses, 0 to 2. The bootloader is GL.iNet's U-Boot, which is only available as a binary, so I can't say why it moves them. On every boot it pulses the switch's reset GPIO, sets up the switch, and leaves the GPHY of port N at MDIO address N + 1, through the PHY_ADDR registers Daniel listed. GL's own firmware deals with it the same way this series does: its Linux driver pulses the reset GPIO at boot, and under that firmware PHY_ADDR_0 to PHY_ADDR_3 read back 0 to 3. I'm testing the other way now, with the PHYs described at 1 to 3 and no reset from Linux. If that works on warm and cold boots, this series isn't needed and I'll drop it. You were also right that 82e2c7db0830 had no place in the cover letter. pw-bot: cr ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios 2026-11-13 22:16 ` Joseph Telaak @ 2026-11-13 22:35 ` Joseph Telaak 0 siblings, 0 replies; 10+ messages in thread From: Joseph Telaak @ 2026-11-13 22:35 UTC (permalink / raw) To: Andrew Lunn Cc: Daniel Golle, Hauke Mehrtens, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel, netdev, devicetree, linux-kernel On Fri, Nov 13, 2026 at 05:16:51PM -0500, Joseph Telaak wrote: > I'm testing the other way now, with the PHYs described at 1 to 3 and no > reset from Linux. If that works on warm and cold boots, this series > isn't needed and I'll drop it. It doesn't work. With the PHYs described at 1 to 3 and no reset from Linux, neither LAN port passes traffic, after the flash or after a cold power-on. The same board with this series works. So the addresses aren't the whole story: U-Boot leaves the switch in some other state that the DT can't describe, and the reset clears it. On your earlier question, the driver already writes PHY_ADDR from the DT, in gswip_port_enable(), but only once the PHYs have been probed. Since GL's own firmware resets the switch from Linux too, I still think a hardware reset at probe is the right fix. I'll send a v4 with the cover letter corrected, this explained, and Sashiko's points addressed (the silent skip with CONFIG_RESET_GPIO=n, and the repeated pulse on probe deferral). ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-11-13 22:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-12 21:39 [PATCH net-next v3 0/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios Joseph Telaak via B4 Relay 2026-11-12 21:39 ` [PATCH net-next v3 1/2] dt-bindings: net: dsa: lantiq,gswip: add reset-gpios Joseph Telaak via B4 Relay 2026-11-13 21:40 ` sashiko-bot 2026-11-12 21:39 ` [PATCH net-next v3 2/2] net: dsa: mxl-gsw1xx: reset the switch through reset-gpios Joseph Telaak via B4 Relay 2026-11-13 21:56 ` netdev-bot+sashiko 2026-11-13 14:08 ` [PATCH net-next v3 0/2] " Andrew Lunn 2026-11-13 15:10 ` Daniel Golle 2026-11-13 21:05 ` Andrew Lunn 2026-11-13 22:16 ` Joseph Telaak 2026-11-13 22:35 ` Joseph Telaak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox