[PATCH net-next v2 1/1] net: mdio: reset PHY before attempting to access ID register

[PATCH net-next v2 1/1] net: mdio: reset PHY before attempting to access ID register

[Buday Csaba]

When the ID of an Ethernet PHY is not provided by the 'compatible'
string in the device tree, its actual ID is read via the MDIO bus.
For some PHYs this could be unsafe, since a hard reset may be
necessary to safely access the MDIO registers.

Add a fallback mechanism for such devices: when reading the ID
fails, the reset will be asserted, and the ID read is retried.

This allows such devices to be used with an autodetected ID.

The fallback mechanism is activated in the error handling path, and
the return code of fwnode_mdiobus_register_phy() is unaltered, except
when the reset fails with -EPROBE_DEFER, which is propagated to the
caller.

Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
---
Patch split from a larger series:
https://lore.kernel.org/all/cover.1761732347.git.buday.csaba@prolan.hu/

The refactoring parts of the previous patchset were already merged,
leaving this one. Functionally identical to:
https://lore.kernel.org/all/5f8d93021a7aa6eeb4fb67ab27ddc7de9101c59f.1761732347.git.buday.csaba@prolan.hu/

Comments were added for clarity.
V1 -> V2: added missing EXPORT_SYMBOL_NS_GPL for
          mdio_device_register/unregister_reset functions
---
 drivers/net/mdio/fwnode_mdio.c | 42 +++++++++++++++++++++++++++++++++-
 drivers/net/phy/mdio_device.c  |  2 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index ba7091518..28daabe63 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -13,9 +13,12 @@
 #include <linux/phy.h>
 #include <linux/pse-pd/pse.h>
 
+#include "../phy/mdio-private.h"
+
 MODULE_AUTHOR("Calvin Johnson <calvin.johnson@oss.nxp.com>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("FWNODE MDIO bus (Ethernet PHY) accessors");
+MODULE_IMPORT_NS("NET_PHY_CORE_ONLY");
 
 static struct pse_control *
 fwnode_find_pse_control(struct fwnode_handle *fwnode,
@@ -67,6 +70,34 @@ fwnode_find_mii_timestamper(struct fwnode_handle *fwnode)
 	return mii_ts;
 }
 
+/* Hard-reset a PHY before registration */
+static int fwnode_reset_phy(struct mii_bus *bus, u32 addr,
+			    struct fwnode_handle *phy_node)
+{
+	struct mdio_device *tmpdev;
+	int rc;
+
+	/* Create a temporary MDIO device to allocate reset resources */
+	tmpdev = mdio_device_create(bus, addr);
+	if (IS_ERR(tmpdev))
+		return PTR_ERR(tmpdev);
+
+	device_set_node(&tmpdev->dev, fwnode_handle_get(phy_node));
+	rc = mdio_device_register_reset(tmpdev);
+	if (rc) {
+		mdio_device_free(tmpdev);
+		return rc;
+	}
+
+	mdio_device_reset(tmpdev, 1);
+	mdio_device_reset(tmpdev, 0);
+
+	mdio_device_unregister_reset(tmpdev);
+	mdio_device_free(tmpdev);
+
+	return 0;
+}
+
 int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 				       struct phy_device *phy,
 				       struct fwnode_handle *child, u32 addr)
@@ -129,8 +160,17 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 		return PTR_ERR(mii_ts);
 
 	is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45");
-	if (is_c45 || fwnode_get_phy_id(child, &phy_id))
+	if (is_c45 || fwnode_get_phy_id(child, &phy_id)) {
 		phy = get_phy_device(bus, addr, is_c45);
+		if (IS_ERR(phy)) {
+			/* get_phy_device() failed, retry after a reset */
+			rc = fwnode_reset_phy(bus, addr, child);
+			if (rc == -EPROBE_DEFER)
+				goto clean_mii_ts;
+			else if (!rc)
+				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/mdio_device.c b/drivers/net/phy/mdio_device.c
index fd0e16dbc..1125c89e4 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -156,6 +156,7 @@ int mdio_device_register_reset(struct mdio_device *mdiodev)
 
 	return 0;
 }
+EXPORT_SYMBOL_NS_GPL(mdio_device_register_reset, "NET_PHY_CORE_ONLY");
 
 /**
  * mdio_device_unregister_reset - uninitialize the reset properties of
@@ -171,6 +172,7 @@ void mdio_device_unregister_reset(struct mdio_device *mdiodev)
 	mdiodev->reset_assert_delay = 0;
 	mdiodev->reset_deassert_delay = 0;
 }
+EXPORT_SYMBOL_NS_GPL(mdio_device_unregister_reset, "NET_PHY_CORE_ONLY");
 
 void mdio_device_reset(struct mdio_device *mdiodev, int value)
 {

base-commit: e2c20036a8879476c88002730d8a27f4e3c32d4b
-- 
2.39.5

Re: [PATCH net-next v2 1/1] net: mdio: reset PHY before attempting to access ID register

[Paolo Abeni]

On 11/28/25 2:53 PM, Buday Csaba wrote:
> When the ID of an Ethernet PHY is not provided by the 'compatible'
> string in the device tree, its actual ID is read via the MDIO bus.
> For some PHYs this could be unsafe, since a hard reset may be
> necessary to safely access the MDIO registers.
> 
> Add a fallback mechanism for such devices: when reading the ID
> fails, the reset will be asserted, and the ID read is retried.
> 
> This allows such devices to be used with an autodetected ID.
> 
> The fallback mechanism is activated in the error handling path, and
> the return code of fwnode_mdiobus_register_phy() is unaltered, except
> when the reset fails with -EPROBE_DEFER, which is propagated to the
> caller.
> 
> Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>

IMHO this deserves an explicit ack from phy maintainers. Unless such ack
is going to land on the ML very soon, I suggest to defer this patch to
the next cycle, as Jakub is wrapping the net-next PR.

Thanks,

Paolo

Re: [PATCH net-next v2 1/1] net: mdio: reset PHY before attempting to access ID register

[Russell King (Oracle)]

On Tue, Dec 02, 2025 at 01:38:21PM +0100, Paolo Abeni wrote:
> On 11/28/25 2:53 PM, Buday Csaba wrote:
> > When the ID of an Ethernet PHY is not provided by the 'compatible'
> > string in the device tree, its actual ID is read via the MDIO bus.
> > For some PHYs this could be unsafe, since a hard reset may be
> > necessary to safely access the MDIO registers.
> > 
> > Add a fallback mechanism for such devices: when reading the ID
> > fails, the reset will be asserted, and the ID read is retried.
> > 
> > This allows such devices to be used with an autodetected ID.
> > 
> > The fallback mechanism is activated in the error handling path, and
> > the return code of fwnode_mdiobus_register_phy() is unaltered, except
> > when the reset fails with -EPROBE_DEFER, which is propagated to the
> > caller.
> > 
> > Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
> 
> IMHO this deserves an explicit ack from phy maintainers. Unless such ack
> is going to land on the ML very soon, I suggest to defer this patch to
> the next cycle, as Jakub is wrapping the net-next PR.

No time to do anything quickly, sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 1/1] net: mdio: reset PHY before attempting to access ID register

[Andrew Lunn]

> @@ -13,9 +13,12 @@
>  #include <linux/phy.h>
>  #include <linux/pse-pd/pse.h>
>  
> +#include "../phy/mdio-private.h"

Relative imports are generally not allowed. It sometimes suggests
layering violations.

I'm not convinced the complexity and ugliness of partially registering
a device in order to be able to reset it, is worth the effort when we
have a simpler solution of just using the ID values in DT.

Give the merge window, i will mark this as change request for the
moment.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next v2 1/1] net: mdio: reset PHY before attempting to access ID register

[Russell King (Oracle)]

On Tue, Dec 02, 2025 at 02:46:42PM +0100, Andrew Lunn wrote:
> > @@ -13,9 +13,12 @@
> >  #include <linux/phy.h>
> >  #include <linux/pse-pd/pse.h>
> >  
> > +#include "../phy/mdio-private.h"
> 
> Relative imports are generally not allowed. It sometimes suggests
> layering violations.
> 
> I'm not convinced the complexity and ugliness of partially registering
> a device in order to be able to reset it, is worth the effort when we
> have a simpler solution of just using the ID values in DT.
> 
> Give the merge window, i will mark this as change request for the
> moment.

It seems to me that PHY clocks and resets are a never-ending source of
problems, so I'm wondering whether we need to consider a radically
different solution, rather than keeping trying to put sticky plasters
over this in various different forms.

__mdiobus_register() is a path where deferred probing is possible,
and this is responsible for doing the bus scan, which is one source
of the problems.

If we add a new ops structure for firmware methods:

struct mii_bus_fw_ops {
	int (*init)(struct mii_bus *bus);
	int (*prescan)(struct mii_bus *bus);
	void (*postscan)(struct mii_bus *bus);
	void (*release)(struct mii_bus *bus);
};

This would be provided when we have e.g. a MII bus described in DT.

The init() method would be called by __mdiobus_register() before:

        err = device_register(&bus->dev);

and this method would:
- walk the sub-nodes of the bus, looking for reset and clock resources.
- it gets the reset and clock resources that are found.
- any that return deferred probe can safely propagate that error code.

__mdiobus_register() would then do its reset stuff, and then before
doing the scans, call the prescan() method. This will:

- walk the resources found in the init() method, noting the initial
  state of resets, then enabling clocks and finally releasing any
  resets.

During the scan, if a device is probed, then we need to consider how
to handle a potential updated reset state. Unlike clocks, which are
nestable, resets aren't. We don't want to re-assert a reset signal
if the PHY driver has specifically asked for it to be deasserted in
its probe() method.

After the bus scans have completed, then the postscan() op would be
called, and this will:

- walk the resources again, restoring the reset state (note the
  comment above concerning PHY driver probe()) and disabling
  any clocks (if the clock has already been "got" by something, e.g.
  a driver that probed as a result of successful scan, this doesn't
  affect it.)

The release() method would be called whenever we are unregistering
the bus (after device_del(&bus->dev) in __mdiobus_register() and
mdiobus_unregister()).

Maybe we should also consider whether phylib should have functions
for PHY drivers to call to control these resources that it has
obtained for each PHY device too?

Yes, this is a bigger change, but I think it should put a stop to
the dribble of problems that seem to be cropping up around the bus
scanning. It can be easily extended for other resources (e.g.
regulators) that are needed to scan and probe, and should also
avoid the need for relative-path includes.

If there are buses that we need to actively assert the reset, then
I feel that this should itself be a firmware property of the device,
as there are likely platforms that come up with the PHY already
released from reset and in a functional state, and causing their
link to drop by forcing a reset of the PHY could be undesirable.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!