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.

This patch adds a fallback mechanism for these devices:
when reading the ID fails, the reset will be asserted, and the
ID read is retried.
When the reset is asserted, an info level message is printed.

Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
---
V3 -> V4: dropped DT property `phy-id-read-needs-reset` to control
	  the reset behaviour, asserting reset as a fallback
	  mechanism instead. Added info level message for resetting.
V2 -> V3: kernel-doc replaced with a comment (fixed warning)
V1 -> V2:
 - renamed fwnode_reset_phy_before_probe() to
   fwnode_reset_phy()
 - added kernel-doc for fwnode_reset_phy()
 - improved error handling in fwnode_reset_phy()
---
 drivers/net/mdio/fwnode_mdio.c | 41 +++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index ba7091518..623120e3b 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -114,6 +114,40 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
 }
 EXPORT_SYMBOL(fwnode_mdiobus_phy_device_register);
 
+/* 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 err;
+
+	tmpdev = mdio_device_create(bus, addr);
+	if (IS_ERR(tmpdev))
+		return PTR_ERR(tmpdev);
+
+	fwnode_handle_get(phy_node);
+	device_set_node(&tmpdev->dev, phy_node);
+	err = mdio_device_register_reset(tmpdev);
+	if (err) {
+		mdio_device_free(tmpdev);
+		return err;
+	}
+
+	if (mdio_device_has_reset(tmpdev)) {
+		dev_info(&bus->dev,
+			 "PHY device at address %d not detected, resetting PHY.",
+			 addr);
+		mdio_device_reset(tmpdev, 1);
+		mdio_device_reset(tmpdev, 0);
+	}
+
+	mdio_device_unregister_reset(tmpdev);
+	mdio_device_free(tmpdev);
+	fwnode_handle_put(phy_node);
+
+	return 0;
+}
+
 int fwnode_mdiobus_register_phy(struct mii_bus *bus,
 				struct fwnode_handle *child, u32 addr)
 {
@@ -129,8 +163,13 @@ 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)) {
+			fwnode_reset_phy(bus, addr, child);
+			phy = get_phy_device(bus, addr, is_c45);
+		}
+	}
 	else
 		phy = phy_device_create(bus, addr, phy_id, 0, NULL);
 	if (IS_ERR(phy)) {
-- 
2.39.5

On Wed, 22 Oct 2025 11:08:53 +0200 Buday Csaba wrote:
> +/* 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 err;
> +
> +	tmpdev = mdio_device_create(bus, addr);
> +	if (IS_ERR(tmpdev))
> +		return PTR_ERR(tmpdev);
> +
> +	fwnode_handle_get(phy_node);
> +	device_set_node(&tmpdev->dev, phy_node);
> +	err = mdio_device_register_reset(tmpdev);
> +	if (err) {
> +		mdio_device_free(tmpdev);
> +		return err;

Should we worry about -EPROBE_DEFER on any of the error paths here?
If not maybe consider making this function void? We can add errors
back if the caller starts to care.

> +	}
> +
> +	if (mdio_device_has_reset(tmpdev)) {
> +		dev_info(&bus->dev,
> +			 "PHY device at address %d not detected, resetting PHY.",
> +			 addr);

IDK if this is still strictly necessary but I guess \n at the end of
the info msg would be idiomatic

On Tue, Oct 28, 2025 at 06:26:05PM -0700, Jakub Kicinski wrote:
> On Wed, 22 Oct 2025 11:08:53 +0200 Buday Csaba wrote:
> > +/* 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 err;
> > +
> > +	tmpdev = mdio_device_create(bus, addr);
> > +	if (IS_ERR(tmpdev))
> > +		return PTR_ERR(tmpdev);
> > +
> > +	fwnode_handle_get(phy_node);
> > +	device_set_node(&tmpdev->dev, phy_node);
> > +	err = mdio_device_register_reset(tmpdev);
> > +	if (err) {
> > +		mdio_device_free(tmpdev);
> > +		return err;
> 
> Should we worry about -EPROBE_DEFER on any of the error paths here?
> If not maybe consider making this function void? We can add errors
> back if the caller starts to care.

That is a very valid point, thanks! I think I can handle that correctly,
by propagating that (or any other) error codes to the caller of
fwnode_mdiobus_register_phy(). fwnode_reset_phy() does nothing that
would not be expected to occur during the normal registration, so if
it fails here, it would fail later as well.

> 
> > +	}
> > +
> > +	if (mdio_device_has_reset(tmpdev)) {
> > +		dev_info(&bus->dev,
> > +			 "PHY device at address %d not detected, resetting PHY.",
> > +			 addr);
> 
> IDK if this is still strictly necessary but I guess \n at the end of
> the info msg would be idiomatic
> 

I will separate the error message to another patch, then the maintainers
can decide wheter to merge it or not.

Thanks for the feedback!
I should be able to send v5 soon.

Csaba