Re: [PATCH net] net: stmmac: request the MDIO reset GPIO only once

[Linkui Xiao]

Hi Maxime,

Thanks for the review.

On 2026/9/18 00:10, Maxime Chevallier wrote:
> Hello,
>
> > @@ -389,11 +389,21 @@ int stmmac_mdio_reset(struct mii_bus *bus)
> >   		struct gpio_desc *reset_gpio;
> >   		u32 delays[3] = { 0, 0, 0 };
> >   
> > -		reset_gpio = devm_gpiod_get_optional(priv->device,
> > -						     "snps,reset",
> > -						     GPIOD_OUT_LOW);
> > -		if (IS_ERR(reset_gpio))
> > -			return PTR_ERR(reset_gpio);
> > +		/* Request the reset line only once and reuse the descriptor
> > +		 * afterwards. A second request of the very same line makes
> > +		 * gpiolib fail with -EBUSY, which devm_gpiod_get_optional()
> > +		 * passes through because it only filters out -ENOENT. The
> > +		 * reset would then abort early and leave the PHY un-reset.
> > +		 */
> > +		if (!priv->mdio_reset_gpio) {
> > +			priv->mdio_reset_gpio =
> > +				devm_gpiod_get_optional(priv->device,
> > +							"snps,reset",
> > +							GPIOD_OUT_LOW);
> > +			if (IS_ERR(priv->mdio_reset_gpio))
> > +				return PTR_ERR(priv->mdio_reset_gpio);
> > +		}
> > +		reset_gpio = priv->mdio_reset_gpio;
>
> The problem is real, but this isn't the correct approach to solve that.
>
> Instead of requesting the reset gpio on-the-fly at reset time (good example
> of when not to use devm_xxx) , let's request the gpio at probe time.
>
> I suggest you move the devm_gpiod_get_optional() in stmmac_mdio_register(),
> which is called at probe time. Here, it makes sense to use devm_xxx.

You're right, requesting the GPIO in the reset callback is the wrong
place for devm_xxx.

I'll send a v2 that moves the devm_gpiod_get_optional() call to
stmmac_mdio_register(), which runs at probe time. stmmac_mdio_reset()
will then just use the descriptor stored in stmmac_priv.

Thanks,
Linkui

> Thanks :)
>
> Maxime