[PATCH] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy

[PATCH] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy

[Buday Csaba]

Some PHYs (e.g. LAN8710A) require a reset after power-on,even for
MDIO register access.
The current implementation of fwnode_mdiobus_register_phy() and
get_phy_device() attempt to read the id registers without ensuring
that the PHY had a reset before, which can fail on these devices.

This patch addresses that shortcoming, by always resetting the PHY
(when such property is given in the device tree). To keep the code
impact minimal, a change was also needed in phy_device_remove() to
prevent asserting the reset on device removal.

According to the documentation of phy_device_remove(), it should
reverse the effect of phy_device_register(). Since the reset GPIO
is in undefined state before that, it should be acceptable to leave
it unchanged during removal.

Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
---
 drivers/net/mdio/fwnode_mdio.c | 20 ++++++++++++++++++--
 drivers/net/phy/phy_device.c   |  3 ---
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index aea0f03575689..36b60544327b6 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -139,8 +139,24 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 	}
 
 	is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45");
-	if (is_c45 || fwnode_get_phy_id(child, &phy_id))
-		phy = get_phy_device(bus, addr, is_c45);
+	if (is_c45 || fwnode_get_phy_id(child, &phy_id)) {
+		/* get_phy_device is NOT SAFE HERE, since the PHY may need a HW RESET.
+		 * First create a dummy PHY device, reset the PHY, then call
+		 * get_phy_device.
+		 */
+		phy = phy_device_create(bus, addr, 0, 0, NULL);
+		if (!IS_ERR(phy)) {
+			if (is_of_node(child)) {
+				/* fwnode_mdiobus_phy_device_register performs the reset */
+				rc = fwnode_mdiobus_phy_device_register(bus, phy, child, addr);
+				if (!rc)
+					phy_device_remove(phy);
+				/* PHY has been reset at this point. */
+			}
+			phy_device_free(phy);
+			phy = get_phy_device(bus, addr, is_c45);
+		}
+	}
 	else
 		phy = phy_device_create(bus, addr, phy_id, 0, NULL);
 	if (IS_ERR(phy)) {
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 13dea33d86ffa..da4ddce04e5fb 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1102,9 +1102,6 @@ void phy_device_remove(struct phy_device *phydev)
 
 	device_del(&phydev->mdio.dev);
 
-	/* Assert the reset signal */
-	phy_device_reset(phydev, 1);
-
 	mdiobus_unregister_device(&phydev->mdio);
 }
 EXPORT_SYMBOL(phy_device_remove);
-- 
2.39.5

Re: [PATCH] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy

[Andrew Lunn]

On Wed, Jul 09, 2025 at 03:24:24PM +0200, Buday Csaba wrote:
> Some PHYs (e.g. LAN8710A) require a reset after power-on,even for
> MDIO register access.
> The current implementation of fwnode_mdiobus_register_phy() and
> get_phy_device() attempt to read the id registers without ensuring
> that the PHY had a reset before, which can fail on these devices.

This is specific to this device, so the driver for this device should
take care of the reset.

To solve the chicken/egg, you need to put a compatible in the PHY node
listing the ID of the PHY. That will cause the correct PHY driver to
load, and probe. The probe can then reset it.

We have to be careful about changing the reset behaviour, it is likely
to break PHYs which currently work, but stop working when they get an
unexpected reset.

    Andrew

---
pw-bot: cr