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

[Linkui Xiao <xiaolinkui@126.com>]

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
>