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

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

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.

Thanks :)

Maxime