Re: [PATCH net-next v5 02/19] net: stmmac: request the MDIO reset GPIO only once

[Linkui Xiao]

On 2026/9/28 07:49, James Hilliard wrote:
> On Sun, Sep 27, 2026 at 5:35 PM Linus Walleij <linusw@xxxxxxxxxx> wrote:
> > Hi James,
> >
> > On Sun, Sep 27, 2026 at 11:59 PM James Hilliard
> > <james.hilliard1@xxxxxxxxx> wrote:
> >
> > > From: Linkui Xiao <xiaolinkui@xxxxxxxxxx>
> > >
> > > stmmac_mdio_reset() calls devm_gpiod_get_optional() every time it runs.
> > > A GPIO line can only be requested once, so from the second call on
> > > gpiod_request_commit() returns -EBUSY. devm_gpiod_get_optional() only
> > > turns -ENOENT into NULL, hence the error is passed straight back and
> > > stmmac_mdio_reset() bails out before pulsing "snps,reset" and before
> > > running the STE101P MDC workaround.
> > >
> > > The first call, made by of_mdiobus_register(), succeeds, so the failure
> > > is only visible later on: every resume that does not use WoL goes
> > > through stmmac_resume() -> stmmac_mdio_reset(), and that caller ignores
> > > the return value, so the PHY silently stays un-reset.
> > >
> > > The descriptor used to be requested exactly once: stmmac_mdio_reset()
> > > resolved "snps,reset-gpio" itself and cached the GPIO number in
> > > stmmac_mdio_bus_data::reset_gpio, and commit ae26c1c6cb9b ("stmmac: fix
> > > PHY reset during resume") relies on that cache to reuse the line on
> > > every call. commit 7c86f20d15b7 ("net: stmmac: use GPIO descriptors in
> > > stmmac_mdio_reset") replaced it with a devm_gpiod_get_optional() that
> > > caches nothing, so the request is repeated on every call and fails from
> > > the second one on.
> > >
> > > Parse the whole reset description, the GPIO and "snps,reset-delays-us",
> > > in stmmac_mdio_register() at probe time, and keep it in struct
> > > stmmac_priv. This is where devm-gpiod is meant to be used: the line is
> > > acquired with the device and released with it, and any failure to
> > > acquire it is reported during probe instead of being ignored by
> > > stmmac_resume(). stmmac_mdio_reset() then only pulses the cached line,
> > > with the delays that were read once and for all at probe time.
> > >
> > > Cache the request and delays for DT devices regardless of
> > > mdio_bus_data->needs_reset. That flag controls the registration-time
> > > bus reset callback, but system resume calls stmmac_mdio_reset()
> > > directly. The reset routine no longer looks at the device tree: where
> > > the description is absent the cached descriptor is NULL and the delays
> > > are zero, so the pulse remains a no-op.
> > >
> > > Keep acquisition conditional on CONFIG_STMMAC_PLATFORM, matching the
> > > reset callback, so non-platform configurations do not request an unused
> > > GPIO. Also skip acquisition for a disabled MDIO child: registering that
> > > bus returns -ENODEV without calling its reset callback, and the driver
> > > must retain the existing disabled-bus success path even if the unused
> > > GPIO is unavailable. Remove the unnecessary gpio_desc forward declaration.
> > >
> > > Fixes: 7c86f20d15b7 ("net: stmmac: use GPIO descriptors in stmmac_mdio_reset")
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Linkui Xiao <xiaolinkui@xxxxxxxxxx>
> > > Co-developed-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> > > Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> >
> > Dostoyevsky commit message, I didn't read it. Ask the agent
> > to be terse.
>
> That commit message style mostly just came from the imported patch,
> I'll change it to be more terse in the next revision:
> https://lore.kernel.org/all/20260921015727.2643540-1-xiaolinkui@xxxxxxx/Hi James,

Thanks for picking this up. Agreed the commit message is too long;
I'm fine with you trimming it for the next revision. Linus already
gave his Reviewed-by on the code, so the logic is settled.

Thanks,
Linkui

> > I looked at the code and from a GPIO PoV it does the right
> > thing: use 1 as asserted and 0 as de-asserted RESET line.
> >
> > Reviewed-by: Linus Walleij <linusw@xxxxxxxxxx>
> >
> > Yours,
> > Linus Walleij