net: mdio: implement optional PHY reset before MDIO access
Subsystems (3)
v4 cover net: mdio: implement optional PHY reset before MDIO access
Some Ethernet PHY devices require a hard reset before any MDIO access can be safely performed. This includes the auto-detection of the PHY ID, which is necessary to bind the correct driver to the device.
The kernel currently does not provide a way to assert the reset before reading the ID, making these devices usable only when the ID is hardcoded in the Device Tree 'compatible' string.
(One notable exception is the FEC driver and its now deprecated phy-reset-gpios property).
This patchset implements an optional reset before reading of the PHY ID register, allowing such PHYs to be used with auto-detected ID. The reset is only asserted when the current logic fails to detect the ID, ensuring compatibility with existing systems.
There have been several earlier attempts to implement such functionality, of which I have collected a few in the links section.
The links to my own v1, v2 and v3 versions are also provided. The most notable change compared to v3, is that the DT binding is removed, and the hard-reset works as a fallback logic now.
Buday Csaba (4):
net: mdio: common handling of phy reset properties
net: mdio: change property read from fwnode_property_read_u32() to
device_property_read_u32()
net: mdio: introduce mdio_device_has_reset()
net: mdio: reset PHY before attempting to access registers in
fwnode_mdiobus_register_phy
drivers/net/mdio/fwnode_mdio.c | 46 ++++++++++++++++++++---- drivers/net/phy/mdio_bus.c | 39 ++------------------ drivers/net/phy/mdio_device.c | 65 ++++++++++++++++++++++++++++++++++ include/linux/mdio.h | 3 ++ 4 files changed, 110 insertions(+), 43 deletions(-)
base-commit: 00922eeaca3c5c2001781bcad40e0bd54d0fdbb6
Signature · 1 line
2.39.5
PATCH v4 1/4 net: mdio: common handling of phy reset properties
Reset properties of an mdio_device are initialized in multiple source files and multiple functions:
- reset_assert_delay and reset_deassert_delay are in fwnode_mdio.c
- reset_gpio and reset_ctrl are in mdio_bus.c, but handled by different functions
This patch unifies the handling of all these properties into two functions.mdiobus_register_gpiod() and mdiobus_register_reset() are removed, while mdio_device_register_reset() and mdio_device_unregister_reset() are introduced instead.
These functions handle both reset-controllers and reset-gpios, and also read the corresponding properties from the device tree. These changes should make tracking the reset properties easier.
The reset logic is unaltered, and should work as it did before.
---
V3 -> V4: unmodified
V2 -> V3: fixed kernel-doc warnings
V1 -> V2: the return value of mdio_device_unregister_reset() is made void
--- drivers/net/mdio/fwnode_mdio.c | 5 ---- drivers/net/phy/mdio_bus.c | 39 ++----------------------- drivers/net/phy/mdio_device.c | 52 ++++++++++++++++++++++++++++++++++ include/linux/mdio.h | 2 ++ 4 files changed, 56 insertions(+), 42 deletions(-)
drivers/net/mdio/fwnode_mdio.c +0 -5
@@ -92,11 +92,6 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,if (fwnode_property_read_bool (child,"broken-turn-around" )) mdio->phy_ignore_ta_mask |=1 << addr; -fwnode_property_read_u32 (child,"reset-assert-us" , - &phy->mdio.reset_assert_delay); -fwnode_property_read_u32 (child,"reset-deassert-us" , - &phy->mdio.reset_deassert_delay); -/* Associate the fwnode with the device structure so it * can be looked up later */
drivers/net/phy/mdio_bus.c +2 -37
@@ -33,33 +33,6 @@#define CREATE_TRACE_POINTS #include <trace/events/mdio.h> -static int mdiobus_register_gpiod (struct mdio_device *mdiodev) -{ -/* Deassert the optional reset signal */ - mdiodev->reset_gpio =gpiod_get_optional (&mdiodev->dev, -"reset" ,GPIOD_OUT_LOW ); -if (IS_ERR (mdiodev->reset_gpio)) -return PTR_ERR (mdiodev->reset_gpio); - -if (mdiodev->reset_gpio) -gpiod_set_consumer_name (mdiodev->reset_gpio,"PHY reset" ); - -return 0 ; -} - -static int mdiobus_register_reset (struct mdio_device *mdiodev) -{ -struct reset_control *reset; - - reset =reset_control_get_optional_exclusive (&mdiodev->dev,"phy" ); -if (IS_ERR (reset)) -return PTR_ERR (reset); - - mdiodev->reset_ctrl = reset; - -return 0 ; -} -int mdiobus_register_device (struct mdio_device *mdiodev) {int err;
@@ -68,16 +41,9 @@ int mdiobus_register_device(struct mdio_device *mdiodev)return -EBUSY ;if (mdiodev->flags &MDIO_DEVICE_FLAG_PHY ) { - err =mdiobus_register_gpiod (mdiodev); + err =mdio_device_register_reset (mdiodev);if (err)return err; - - err =mdiobus_register_reset (mdiodev); -if (err) -return err; - -/* Assert the reset signal */ -mdio_device_reset (mdiodev,1 ); } mdiodev->bus->mdio_map[mdiodev->addr] = mdiodev;
@@ -91,8 +57,7 @@ int mdiobus_unregister_device(struct mdio_device *mdiodev)if (mdiodev->bus->mdio_map[mdiodev->addr] != mdiodev)return -EINVAL ; -gpiod_put (mdiodev->reset_gpio); -reset_control_put (mdiodev->reset_ctrl); +mdio_device_unregister_reset (mdiodev); mdiodev->bus->mdio_map[mdiodev->addr] =NULL ;
drivers/net/phy/mdio_device.c +52 -0
58 lines not on this page · raw message
include/linux/mdio.h +2 -0
@@ -90,6 +90,8 @@ static inline void *mdiodev_get_drvdata(struct mdio_device *mdio)void mdio_device_free (struct mdio_device *mdiodev);struct mdio_device *mdio_device_create (struct mii_bus *bus,int addr); +int mdio_device_register_reset (struct mdio_device *mdiodev); +void mdio_device_unregister_reset (struct mdio_device *mdiodev);int mdio_device_register (struct mdio_device *mdiodev);void mdio_device_remove (struct mdio_device *mdiodev);void mdio_device_reset (struct mdio_device *mdiodev,int value);
Signature · 1 line
2.39.5
Jakub Kicinski
maintainerRe: [PATCH net-next v4 1/4] net: mdio: common handling of phy reset properties
Let me give you some nit picky comments, maybe v5 will have more luck attracting area experts :(
Quoted 9 lines · On Wed, 22 Oct 2025 11:08:50 +0200 Buday Csaba wrote:
Reset properties of an
mdio_deviceare initialized in multiple source files and multiple functions:
-reset_assert_delayandreset_deassert_delayare infwnode_mdio.c
-reset_gpioandreset_ctrlare inmdio_bus.c, but handled by different functionsThis patch unifies the handling of all these properties into two functions.
Use imperative mood. "This patch unifies" -> "Unify" Please check all commits msgs for this (eg glancing at patch 2 "Changed" -> "Change")
drivers/net/phy/mdio_device.c struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr) · line 97
Quoted 11 lines
mdiobus_register_gpiod()andmdiobus_register_reset()are removed, whilemdio_device_register_reset()andmdio_device_unregister_reset()are introduced instead.
These functions handle both reset-controllers and reset-gpios, and also read the corresponding properties from the device tree. These changes should make tracking the reset properties easier.drivers/net/phy/mdio_device.c +4
-0+/* reset-gpio, bring up deasserted */ + mdiodev->reset_gpio =gpiod_get_optional (&mdiodev->dev,"reset" , +GPIOD_OUT_LOW ); +
no empty lines between call and its error check pls
Quoted 17 lines
drivers/net/phy/mdio_device.c +17
-0+if (IS_ERR (mdiodev->reset_gpio)) +return PTR_ERR (mdiodev->reset_gpio); + +if (mdiodev->reset_gpio) +gpiod_set_consumer_name (mdiodev->reset_gpio,"PHY reset" ); + + reset =reset_control_get_optional_exclusive (&mdiodev->dev,"phy" ); +if (IS_ERR (reset)) +return PTR_ERR (reset); + + mdiodev->reset_ctrl = reset; + +/* Assert the reset signal */ +mdio_device_reset (mdiodev,1 ); + +return 0 ; +}
PATCH v4 2/4 net: mdio: change property read from fwnode_property_read_u32() to device_property_read_u32()
In reply to Buday CsabaChanged fwnode_property_read_u32() in mdio_device_register_reset() to device_property_read_u32(), which is more appropriate here.
---
V3 -> V4: unmodified
V2 -> V3: unmodified
V1 -> V2: added new patch based on maintainer request
--- drivers/net/phy/mdio_device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/phy/mdio_device.c +2 -2
@@ -86,9 +86,9 @@ int mdio_device_register_reset(struct mdio_device *mdiodev)struct reset_control *reset;/* Read optional firmware properties */ -fwnode_property_read_u32 (dev_fwnode (&mdiodev->dev),"reset-assert-us" , +device_property_read_u32 (&mdiodev->dev,"reset-assert-us" , &mdiodev->reset_assert_delay); -fwnode_property_read_u32 (dev_fwnode (&mdiodev->dev),"reset-deassert-us" , +device_property_read_u32 (&mdiodev->dev,"reset-deassert-us" , &mdiodev->reset_deassert_delay);/* reset-gpio, bring up deasserted */
Signature · 1 line
2.39.5
PATCH v4 3/4 net: mdio: introduce mdio_device_has_reset()
In reply to Buday CsabaIntroduced mdio_device_has_reset() to check if an MDIO device has any reset properties defined.
---
V3 -> V4: new commit
--- drivers/net/phy/mdio_device.c | 13 +++++++++++++ include/linux/mdio.h | 1 + 2 files changed, 14 insertions(+)
drivers/net/phy/mdio_device.c +13 -0
@@ -170,6 +170,19 @@ void mdio_device_remove(struct mdio_device *mdiodev) }EXPORT_SYMBOL (mdio_device_remove); +/** +* mdio_device_has_reset - Check if an MDIO device has reset properties defined +* @mdiodev: mdio_device structure +* +* Return: non-zero if the device has a reset GPIO or reset controller, +* zero otherwise. +*/ +int mdio_device_has_reset (struct mdio_device *mdiodev) +{ +return (mdiodev->reset_gpio || mdiodev->reset_ctrl); +} +EXPORT_SYMBOL (mdio_device_has_reset); +void mdio_device_reset (struct mdio_device *mdiodev,int value) {unsigned int d;
include/linux/mdio.h +1 -0
@@ -92,6 +92,7 @@ void mdio_device_free(struct mdio_device *mdiodev);struct mdio_device *mdio_device_create (struct mii_bus *bus,int addr);int mdio_device_register_reset (struct mdio_device *mdiodev);void mdio_device_unregister_reset (struct mdio_device *mdiodev); +int mdio_device_has_reset (struct mdio_device *mdiodev);int mdio_device_register (struct mdio_device *mdiodev);void mdio_device_remove (struct mdio_device *mdiodev);void mdio_device_reset (struct mdio_device *mdiodev,int value);
Signature · 1 line
2.39.5
PATCH v4 4/4 net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy
In reply to Buday CsabaWhen 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.
---
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(-)
drivers/net/mdio/fwnode_mdio.c +40 -1
@@ -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)) {
Signature · 1 line
2.39.5
Jakub Kicinski
maintainerRe: [PATCH net-next v4 4/4] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy
Quoted 7 lines · On Wed, 22 Oct 2025 11:08:53 +0200 Buday Csaba wrote:
drivers/net/mdio/fwnode_mdio.c +7
-0+/* 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; +
drivers/net/mdio/fwnode_mdio.c int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio, · line 133
Quoted 10 lines
drivers/net/mdio/fwnode_mdio.c +10
-0+ 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.
drivers/net/mdio/fwnode_mdio.c int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio, · line 139
Quoted 6 lines
drivers/net/mdio/fwnode_mdio.c +6
-0+ } + +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
Re: [PATCH net-next v4 4/4] net: mdio: reset PHY before attempting to access registers in fwnode_mdiobus_register_phy
Quoted 14 lines · Jakub Kicinski
Quoted 13 lines · On Wed, 22 Oct 2025 11:08:53 +0200 Buday Csaba wrote:
drivers/net/mdio/fwnode_mdio.c +13
-0+/* 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);
Quoted 8 lines
drivers/net/mdio/fwnode_mdio.c int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio, · line 133
Quoted 4 lines
drivers/net/mdio/fwnode_mdio.c +4
-0+ 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.
Quoted 11 lines
drivers/net/mdio/fwnode_mdio.c int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio, · line 139
Quoted 7 lines
drivers/net/mdio/fwnode_mdio.c +6
-0+ } + +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