[PATCH net-next] net: phy: Handle PHY reset during initial PHY ID read

[PATCH net-next] net: phy: Handle PHY reset during initial PHY ID read

[Chen-Yu Tsai]

The mdio device core handles reset GPIOs and controls for PHYs and
MDIOs after the MDIO or PHY device is created. However this does not
cover the initial PHY ID read _before_ the PHY device is created, since
the PHY ID is needed for the PHY device. This causes PHY devices that
have reset GPIOs or controls to not work after a reboot if the GPIO
is left in the reset state; neither will it work if the reset GPIO is
by default (for example, missing a pull-up) in the reset state.

One possible workaround is to place the reset GPIO or control property
under the MDIO bus instead of under the PHY. However the common PHY
device tree bindings already allow a reset for the PHY, so we should
make some effort to support this.

Rework get_phy_device() to allow passing in a fwnode handle for the PHY
device, and use the handle to acquire the reset GPIO, control and delay
timings for it. Before reading the PHY ID, deassert the reset. This
reworked version is renamed to fwnode_get_phy_device(), with
get_phy_device() calling the new version.

Use this new version in fwnode_mdiobus_register_phy() so that PHY reset
is handled. This allows the reset GPIO and reset control in the common
Ethernet PHY device tree to be correctly handled without any gotchas.

Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
---
This work was the result of Russell mentioning [1] that placing the
reset GPIO under the PHY node in the device tree might result in it
not working.

I also talked about this at Plumbers in Tokyo last year during the
Device Tree MC. There a few people mentioned that MDIO reset handling
has been a pain point.

[1] https://lore.kernel.org/linux-sunxi/aJy_qUbmqoOG-GBC@shell.armlinux.org.uk/

 drivers/net/mdio/fwnode_mdio.c |  2 +-
 drivers/net/phy/phy_device.c   | 47 +++++++++++++++++++++++++++++++---
 include/linux/phy.h            | 10 +++++++-
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index ba7091518265..f62a48583404 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -130,7 +130,7 @@ 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);
+		phy = fwnode_get_phy_device(bus, addr, child, 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 3bd415710bf3..62304fcacc7b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -15,6 +15,7 @@
 #include <linux/errno.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/gpio/consumer.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -34,6 +35,7 @@
 #include <linux/pse-pd/pse.h>
 #include <linux/property.h>
 #include <linux/ptp_clock_kernel.h>
+#include <linux/reset.h>
 #include <linux/rtnetlink.h>
 #include <linux/sfp.h>
 #include <linux/skbuff.h>
@@ -1050,14 +1052,17 @@ int fwnode_get_phy_id(struct fwnode_handle *fwnode, u32 *phy_id)
 EXPORT_SYMBOL(fwnode_get_phy_id);
 
 /**
- * get_phy_device - reads the specified PHY device and returns its @phy_device
- *		    struct
+ * fwnode_get_phy_device - reads the specified PHY device and returns its
+ *			   @phy_device struct
  * @bus: the target MII bus
  * @addr: PHY address on the MII bus
+ * @fwnode: PHY fwnode handle
  * @is_c45: If true the PHY uses the 802.3 clause 45 protocol
  *
  * Probe for a PHY at @addr on @bus.
  *
+ * Transparently handle any reset GPIOs.
+ *
  * When probing for a clause 22 PHY, then read the ID registers. If we find
  * a valid ID, allocate and return a &struct phy_device.
  *
@@ -1068,21 +1073,55 @@ EXPORT_SYMBOL(fwnode_get_phy_id);
  * Returns an allocated &struct phy_device on success, %-ENODEV if there is
  * no PHY present, or %-EIO on bus access error.
  */
-struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
+struct phy_device *fwnode_get_phy_device(struct mii_bus *bus, int addr,
+					 struct fwnode_handle *fwnode, bool is_c45)
 {
 	struct phy_c45_device_ids c45_ids;
+	struct gpio_desc *gpiod = NULL;
+	struct reset_control *rstc = NULL;
 	u32 phy_id = 0;
+	u32 delay = 0;
 	int r;
 
 	c45_ids.devices_in_package = 0;
 	c45_ids.mmds_present = 0;
 	memset(c45_ids.device_ids, 0xff, sizeof(c45_ids.device_ids));
 
+	if (fwnode) {
+		/* Deassert the optional reset signal */
+		gpiod = fwnode_gpiod_get_index(fwnode, "reset", 0,
+					       GPIOD_OUT_LOW, "PHY reset");
+		if (IS_ERR(gpiod)) {
+			if (PTR_ERR(gpiod) == -ENOENT)
+				gpiod = NULL;
+			else if (PTR_ERR(gpiod) == -ENOSYS)
+				gpiod = NULL;
+			else
+				return ERR_CAST(gpiod);
+		}
+
+		if (is_of_node(fwnode)) {
+			rstc = of_reset_control_get_optional_exclusive(to_of_node(fwnode), "phy");
+			if (IS_ERR(rstc))
+				return ERR_CAST(rstc);
+			reset_control_deassert(rstc);
+		}
+
+		/* Wait for PHY to come out of reset if needed */
+		if (!fwnode_property_read_u32(fwnode, "reset-deassert-us", &delay))
+			fsleep(delay);
+	}
+
 	if (is_c45)
 		r = get_phy_c45_ids(bus, addr, &c45_ids);
 	else
 		r = get_phy_c22_id(bus, addr, &phy_id);
 
+	if (!IS_ERR_OR_NULL(rstc))
+		reset_control_put(rstc);
+	if (!IS_ERR_OR_NULL(gpiod))
+		gpiod_put(gpiod);
+
 	if (r)
 		return ERR_PTR(r);
 
@@ -1100,7 +1139,7 @@ struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
 
 	return phy_device_create(bus, addr, phy_id, is_c45, &c45_ids);
 }
-EXPORT_SYMBOL(get_phy_device);
+EXPORT_SYMBOL(fwnode_get_phy_device);
 
 /**
  * phy_device_register - Register the phy device on the MDIO bus
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 6f9979a26892..3bd3bbc1b281 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -2142,7 +2142,15 @@ int fwnode_get_phy_id(struct fwnode_handle *fwnode, u32 *phy_id);
 struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode);
 struct phy_device *fwnode_phy_find_device(struct fwnode_handle *phy_fwnode);
 struct fwnode_handle *fwnode_get_phy_node(const struct fwnode_handle *fwnode);
-struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45);
+struct phy_device *fwnode_get_phy_device(struct mii_bus *bus, int addr,
+					 struct fwnode_handle *fwnode, bool is_c45);
+
+static inline
+struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
+{
+	return fwnode_get_phy_device(bus, addr, NULL, is_c45);
+}
+
 int phy_device_register(struct phy_device *phy);
 void phy_device_free(struct phy_device *phydev);
 void phy_device_remove(struct phy_device *phydev);
-- 
2.47.3

Re: [PATCH net-next] net: phy: Handle PHY reset during initial PHY ID read

[Andrew Lunn]

On Mon, Mar 02, 2026 at 10:44:57PM +0800, Chen-Yu Tsai wrote:
> The mdio device core handles reset GPIOs and controls for PHYs and
> MDIOs after the MDIO or PHY device is created. However this does not
> cover the initial PHY ID read _before_ the PHY device is created, since
> the PHY ID is needed for the PHY device. This causes PHY devices that
> have reset GPIOs or controls to not work after a reboot if the GPIO
> is left in the reset state; neither will it work if the reset GPIO is
> by default (for example, missing a pull-up) in the reset state.
> 
> One possible workaround is to place the reset GPIO or control property
> under the MDIO bus instead of under the PHY. However the common PHY
> device tree bindings already allow a reset for the PHY, so we should
> make some effort to support this.

Please either:

1) Add the ID to the DT

2) Work on driver/base, and add generic power on sequencing code which
can handle reset GPIOs, reset controllers, regulators, clocks etc. A
solution which works for PCIe, USB, SATA, and other enumerable
devices.  And then modify phylib to use this generic code.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next] net: phy: Handle PHY reset during initial PHY ID read

[Andrew Lunn]

> This work was the result of Russell mentioning [1] that placing the
> reset GPIO under the PHY node in the device tree might result in it
> not working.
> 
> I also talked about this at Plumbers in Tokyo last year during the
> Device Tree MC. There a few people mentioned that MDIO reset handling
> has been a pain point.

It is definitely a pain point. I wish we never added support for
resets, just left it to the bootloader to get the hardware into a
usable state before starting linux.

Anyway, i don't really see this being a MDIO/PHY problem. It is a
generic device driver model problem. We need the core to have generic
code used for any enumerable bus, PCIe, USB, SDIO, MDIO etc to ensure
the clocks, resets, regulators, etc are in the correct state that the
probe can work.

Once the core has this facility, we can modify MDIO to make use of it,
if needed.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next] net: phy: Handle PHY reset during initial PHY ID read

[Chen-Yu Tsai]

On Thu, Mar 12, 2026 at 3:05 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > This work was the result of Russell mentioning [1] that placing the
> > reset GPIO under the PHY node in the device tree might result in it
> > not working.
> >
> > I also talked about this at Plumbers in Tokyo last year during the
> > Device Tree MC. There a few people mentioned that MDIO reset handling
> > has been a pain point.
>
> It is definitely a pain point. I wish we never added support for
> resets, just left it to the bootloader to get the hardware into a
> usable state before starting linux.
>
> Anyway, i don't really see this being a MDIO/PHY problem. It is a
> generic device driver model problem. We need the core to have generic
> code used for any enumerable bus, PCIe, USB, SDIO, MDIO etc to ensure
> the clocks, resets, regulators, etc are in the correct state that the
> probe can work.

I agree it's not just a MDIO/PHY problem. I am looking into it for all
the common enumerable bus types. Currently SDIO, USB, and PCIe all have
some solution, though the SDIO one is deprecated, and the USB one is
incomplete.

That said, this doesn't fit into the driver model well: to create a struct
device the device needs to be enumerated; to enumerate the device, its
resources need to be provided and enabled, but at this point there's no
device to tie the resources to.

All the existing solutions scan the DT and create extra platform devices
to handle the power sequencing. Some bus specific code is needed for this.
USB is simpler, since once the USB device powers up it will initiate a
handshake with the host. All the other bus types rely on the host controller
doing a scan to enumerate the devices, so the power sequencing code needs
to be inserted into the bus scanning code. The PCI implementation is
extra complicated: some controllers have dedicated PERST# functions
that are toggled from the controller's registers instead of using
GPIOs for the device reset.

> Once the core has this facility, we can modify MDIO to make use of it,
> if needed.

We already have the power sequencing framework (drivers/power/sequencing).
So instead of the open coded reset GPIO I did in this patch, it would
just be calls to pwrseq_get() and pwrseq_power_on(). I would need to
figure out how to actually work them into the MDIO subsystem, and also
how to model a generic PHY power sequencing provider. But the recent
M.2 slot work [1] provides some good examples.

Does that sound more acceptable?


Thanks
ChenYu

[1] https://lore.kernel.org/all/20260107-pci-m2-v5-0-8173d8a72641@oss.qualcomm.com/
[2] https://lore.kernel.org/all/20260112-pci-m2-e-v4-0-eff84d2c6d26@oss.qualcomm.com/

Re: [PATCH net-next] net: phy: Handle PHY reset during initial PHY ID read

[Andrew Lunn]

> I agree it's not just a MDIO/PHY problem. I am looking into it for all
> the common enumerable bus types.

Great.

> We already have the power sequencing framework (drivers/power/sequencing).
> So instead of the open coded reset GPIO I did in this patch, it would
> just be calls to pwrseq_get() and pwrseq_power_on(). I would need to
> figure out how to actually work them into the MDIO subsystem, and also
> how to model a generic PHY power sequencing provider. But the recent
> M.2 slot work [1] provides some good examples.

You need to be careful with "generic PHY". That generally means
drivers/phy and the config symbol GENERIC_PHY. Ethernet PHYs have been
around a long time, so PHY generally means Ethernet PHY. But
"recently", devices with SERDES interfaces, PCIe, SATA, USB, Ethernet
PCS have a 'phy' which needs configuring, so we gained the Generic PHY
subsystem. And we gained the confusion of PHY vs Generic PHY.

> Does that sound more acceptable?

Yes it does.

Thanks
	Andrew