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

From: Linkui Xiao <xiaolinkui@kylinos.cn>

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@vger.kernel.org
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
Co-developed-by: James Hilliard <james.hilliard1@gmail.com>
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |  2 +
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 50 +++++++++++------------
 2 files changed, 27 insertions(+), 25 deletions(-)
  

Hi James,

On Sun, Sep 27, 2026 at 11:59 PM James Hilliard
<james.hilliard1@gmail.com> wrote:

> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>
> 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@vger.kernel.org
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> Co-developed-by: James Hilliard <james.hilliard1@gmail.com>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Dostoyevsky commit message, I didn't read it. Ask the agent
to be terse.

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@kernel.org>

Yours,
Linus Walleij
  

On Sun, Sep 27, 2026 at 5:35 PM Linus Walleij <linusw@kernel.org> wrote:
>
> Hi James,
>
> On Sun, Sep 27, 2026 at 11:59 PM James Hilliard
> <james.hilliard1@gmail.com> wrote:
>
> > From: Linkui Xiao <xiaolinkui@kylinos.cn>
> >
> > 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@vger.kernel.org
> > Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> > Co-developed-by: James Hilliard <james.hilliard1@gmail.com>
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>
> 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@126.com/

> 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@kernel.org>
>
> Yours,
> Linus Walleij
  

On 2026/9/28 07:49, James Hilliard wrote:
> On Sun, Sep 27, 2026 at 5:35 PM Linus Walleij <linusw@kernel.org> wrote:
>>
>> Hi James,
>>
>> On Sun, Sep 27, 2026 at 11:59 PM James Hilliard
>> <james.hilliard1@gmail.com> wrote:
>>
>>> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>>>
>>> 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@vger.kernel.org
>>> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
>>> Co-developed-by: James Hilliard <james.hilliard1@gmail.com>
>>> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>>
>> 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@126.com/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@kernel.org>
>>
>> Yours,
>> Linus Walleij