[PATCH] net: designware: Claim the reset GPIO in probe and free it on failure

[PATCH] net: designware: Claim the reset GPIO in probe and free it on failure

[Yuho Choi]

designware_eth_of_to_plat() requests the "snps,reset-gpio" PHY reset
GPIO, but the driver never frees it: not when of_to_plat() itself fails
afterwards (no "snps,reset-delays-us" property), not when the probe
fails and not in designware_eth_remove().

When DM_DEVICE_REMOVE is enabled, a failed probe ends in device_free(),
which clears DM_FLAG_PLATDATA_VALID, so the next probe attempt runs
of_to_plat() again. The GPIO is still claimed from the first attempt, so
gpio_request_by_name() returns -EBUSY and every later probe attempt
fails. The eth uclass retries the probe on later network commands, so
one failure, e.g. dw_phy_init() finding no PHY, leaves the interface
failing with -EBUSY until the next reset. The same happens when
eth_post_probe() rejects the device because it has no valid MAC
address: device_probe() removes the device again, and once ethaddr is
set, the next probe fails with -EBUSY.

While the GPIO is claimed in of_to_plat() the driver cannot release it
on all of these paths: failures in the glue drivers and in
device_probe() before the probe method runs never reach driver code.
Parse the GPIO in of_to_plat() with gpio_parse_by_name() and request it
in designware_eth_probe() with gpio_request_parsed() right before the
MDIO bus is set up, as doc/develop/driver-model/design.rst asks. Free it
on the probe error path and in designware_eth_remove().

The parsed spec is kept in priv next to the descriptor rather than in
plat, because dwmac_s700 allocates only a struct eth_pdata as its plat.
The glue drivers do not use designware_eth_remove() as their remove
method, so removing a glue device still leaves the GPIO claimed.

The GPIO is now claimed after the clocks, the PHY supply and the default
pinctrl state are set up. The PHY is still reset through it before the
bus is scanned for the PHY.

Fixes: 90b7fc924adf ("net: designware: support phy reset device-tree bindings")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
 drivers/net/designware.c | 22 ++++++++++++++++++++--
 drivers/net/designware.h |  1 +
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index 9470804ebd1..378586c708e 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -972,6 +972,15 @@ int designware_eth_probe(struct udevice *dev)
 	priv->interface = pdata->phy_interface;
 	priv->max_speed = pdata->max_speed;
 
+#if CONFIG_IS_ENABLED(DM_GPIO)
+	ret = gpio_request_parsed(dev, &priv->reset_gpio_spec,
+				  &priv->reset_gpio);
+	if (ret && ret != -ENOENT) {
+		err = ret;
+		goto gpio_err;
+	}
+#endif
+
 #if IS_ENABLED(CONFIG_BITBANGMII) && IS_ENABLED(CONFIG_DM_GPIO)
 	if (dev_read_bool(dev, "snps,bitbang-mii")) {
 		ret = dw_bb_mdio_init(dev->name, dev);
@@ -1003,6 +1012,11 @@ int designware_eth_probe(struct udevice *dev)
 	mdio_unregister(priv->bus);
 	mdio_free(priv->bus);
 mdio_err:
+#if CONFIG_IS_ENABLED(DM_GPIO)
+	if (dm_gpio_is_valid(&priv->reset_gpio))
+		dm_gpio_free(dev, &priv->reset_gpio);
+gpio_err:
+#endif
 
 #ifdef CONFIG_CLK
 clk_err:
@@ -1021,6 +1035,10 @@ static int designware_eth_remove(struct udevice *dev)
 	free(priv->phydev);
 	mdio_unregister(priv->bus);
 	mdio_free(priv->bus);
+#if CONFIG_IS_ENABLED(DM_GPIO)
+	if (dm_gpio_is_valid(&priv->reset_gpio))
+		dm_gpio_free(dev, &priv->reset_gpio);
+#endif
 
 #ifdef CONFIG_CLK
 	return clk_release_all(priv->clocks, priv->clock_count);
@@ -1061,8 +1079,8 @@ int designware_eth_of_to_plat(struct udevice *dev)
 	if (dev_read_bool(dev, "snps,reset-active-low"))
 		reset_flags |= GPIOD_ACTIVE_LOW;
 
-	ret = gpio_request_by_name(dev, "snps,reset-gpio", 0,
-		&priv->reset_gpio, reset_flags);
+	ret = gpio_parse_by_name(dev, "snps,reset-gpio", 0, reset_flags,
+				 &priv->reset_gpio_spec);
 	if (ret == 0) {
 		ret = dev_read_u32_array(dev, "snps,reset-delays-us",
 					 dw_pdata->reset_delays, 3);
diff --git a/drivers/net/designware.h b/drivers/net/designware.h
index 3618300f4c5..072e97ff89e 100644
--- a/drivers/net/designware.h
+++ b/drivers/net/designware.h
@@ -237,6 +237,7 @@ struct dw_eth_dev {
 	struct eth_mac_regs *mac_regs_p;
 	struct eth_dma_regs *dma_regs_p;
 #if CONFIG_IS_ENABLED(DM_GPIO)
+	struct gpio_dt_spec reset_gpio_spec;
 	struct gpio_desc reset_gpio;
 #endif
 #ifdef CONFIG_CLK
-- 
2.43.0