net: mdio: improve reset handling of mdio devices

PATCH Superseded Buday Csaba 3 participants 7 messages Nov 17, 2025

Subsystems (3)

Networking Network drivers Ethernet PHY

lore

Share & tools

lore

Buday Csaba

author

2025-11-17 09:34 UTC #0 lore raw

v2 cover net: mdio: improve reset handling of mdio devices

This patchset refactors and slightly improves the reset handling of mdio_device..

The patches were split from a larger series, discussed previously in the links below.

The difference between v1 and v2, is that the leak fix was applied to the base already. See links for v1 and for the now separate leak fix.

Link https://lore.kernel.org/all/[email protected]/ open here

Link https://lore.kernel.org/all/4b419377f8dd7d2f63f919d0f74a336c734f8fff.1762584481.git.buday.csaba@prolan.hu/ open here

Buday Csaba (3):
net: mdio: move device reset functions to mdio_device.c
net: mdio: common handling of phy device reset properties
net: mdio: improve reset handling in mdio_device.c

 drivers/net/mdio/fwnode_mdio.c |  5 ----
 drivers/net/phy/mdio_bus.c     | 39 ++-----------------------
 drivers/net/phy/mdio_device.c  | 53 ++++++++++++++++++++++++++++++++++
 include/linux/mdio.h           |  2 ++
 4 files changed, 57 insertions(+), 42 deletions(-)

base-commit: c9dfb92de0738eb7fe6a591ad1642333793e8b6e

Signature · 1 line

2.39.5

Buday Csaba

author

2025-11-17 09:34 UTC #1 lore raw

PATCH v2 1/3 net: mdio: move device reset functions to mdio_device.c

The functions mdiobus_register_gpiod() and mdiobus_register_reset() handle the mdio device reset initialization, which belong to mdio_device.c.
Move them from mdio_bus.c to mdio_device.c, and rename them to match the corresponding source file: mdio_device_register_gpio() and mdio_device_register_reset().
Remove 'static' qualifiers and declare them in mdio.h.

Signed-off-by Buday Csaba <[email protected]>

---
V1 -> V2: rebase, no changes

---
 drivers/net/phy/mdio_bus.c    | 31 ++-----------------------------
 drivers/net/phy/mdio_device.c | 27 +++++++++++++++++++++++++++
 include/linux/mdio.h          |  2 ++
 3 files changed, 31 insertions(+), 29 deletions(-)

drivers/net/phy/mdio_bus.c +2 ~~-29~~

index 435424113..1ac942102 100644
@@ -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,11 +41,11 @@ 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_gpiod(mdiodev);
 		if (err)
 			return err;
 
-		err = mdiobus_register_reset(mdiodev);
+		err = mdio_device_register_reset(mdiodev);
 		if (err) {
 			gpiod_put(mdiodev->reset_gpio);
 			mdiodev->reset_gpio = NULL;

drivers/net/phy/mdio_device.c +27 -0

index f64176e0e..5a78d8624 100644
@@ -118,6 +118,33 @@ void mdio_device_remove(struct mdio_device *mdiodev)
 }
 EXPORT_SYMBOL(mdio_device_remove);
 
+int mdio_device_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;
+}
+
+int mdio_device_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;
+}
+
 void mdio_device_reset(struct mdio_device *mdiodev, int value)
 {
 	unsigned int d;

include/linux/mdio.h +2 -0

index 42d6d47e4..1322d2623 100644
@@ -92,6 +92,8 @@ void mdio_device_free(struct mdio_device *mdiodev);
 struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr);
 int mdio_device_register(struct mdio_device *mdiodev);
 void mdio_device_remove(struct mdio_device *mdiodev);
+int mdio_device_register_gpiod(struct mdio_device *mdiodev);
+int mdio_device_register_reset(struct mdio_device *mdiodev);
 void mdio_device_reset(struct mdio_device *mdiodev, int value);
 int mdio_driver_register(struct mdio_driver *drv);
 void mdio_driver_unregister(struct mdio_driver *drv);

Signature · 1 line

2.39.5

Russell King (Oracle)

reviewer

2025-11-17 09:35 UTC #2 lore raw

Re: [PATCH net-next v2 1/3] net: mdio: move device reset functions to mdio_device.c

include/linux/mdio.h void mdio_device_free(struct mdio_device *mdiodev); · line 96

Quoted 10 lines · Buday Csaba

include/linux/mdio.h +2 -0

index 42d6d47e4..1322d2623 100644
@@ -92,6 +92,8 @@ void mdio_device_free(struct mdio_device *mdiodev);
 struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr);
 int mdio_device_register(struct mdio_device *mdiodev);
 void mdio_device_remove(struct mdio_device *mdiodev);
+int mdio_device_register_gpiod(struct mdio_device *mdiodev);
+int mdio_device_register_reset(struct mdio_device *mdiodev);

These are private functions to the mdio code living in drivers/net/phy, so I wonder whether we want to have drivers/net/phy/mdio.h for these to discourage other code calling these?

Signature · 2 lines

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

Buday Csaba

author

2025-11-17 12:22 UTC #3 lore raw

Re: [PATCH net-next v2 1/3] net: mdio: move device reset functions to mdio_device.c

Quoted 15 lines · Russell King (Oracle)

include/linux/mdio.h void mdio_device_free(struct mdio_device *mdiodev); · line 96
Quoted 10 lines · Buday Csaba
include/linux/mdio.h +2 -0
index 42d6d47e4..1322d2623 100644
@@ -92,6 +92,8 @@ void mdio_device_free(struct mdio_device *mdiodev);
 struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr);
 int mdio_device_register(struct mdio_device *mdiodev);
 void mdio_device_remove(struct mdio_device *mdiodev);
+int mdio_device_register_gpiod(struct mdio_device *mdiodev);
+int mdio_device_register_reset(struct mdio_device *mdiodev);
These are private functions to the mdio code living in drivers/net/phy, so I wonder whether we want to have drivers/net/phy/mdio.h for these to discourage other code calling these?

I completely agree with that, but that file does not exist yet. Is it worth creating just for the sake of these two functions?

Csaba

Andrew Lunn

maintainer

2025-11-17 13:49 UTC #4 lore raw

Re: [PATCH net-next v2 1/3] net: mdio: move device reset functions to mdio_device.c

Quoted 17 lines · Buday Csaba

Quoted 15 lines · Russell King (Oracle)
include/linux/mdio.h void mdio_device_free(struct mdio_device *mdiodev); · line 96
Quoted 10 lines · Buday Csaba
include/linux/mdio.h +2 -0
index 42d6d47e4..1322d2623 100644
@@ -92,6 +92,8 @@ void mdio_device_free(struct mdio_device *mdiodev);
 struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr);
 int mdio_device_register(struct mdio_device *mdiodev);
 void mdio_device_remove(struct mdio_device *mdiodev);
+int mdio_device_register_gpiod(struct mdio_device *mdiodev);
+int mdio_device_register_reset(struct mdio_device *mdiodev);
These are private functions to the mdio code living in drivers/net/phy, so I wonder whether we want to have drivers/net/phy/mdio.h for these to discourage other code calling these?

Quoted 2 lines

I completely agree with that, but that file does not exist yet. Is it worth creating just for the sake of these two functions?

The effort of creating such a file is much smaller than cleaning up the mess when somebody uses them inappropriately.

Maybe drivers/net/phy/mdio.h is too open, drivers/net/phy/mdio-private.h would be better, since these are supposed to only be used by the core, not PHY drivers.

Andrew

Buday Csaba

author

2025-11-17 09:34 UTC #5 lore raw

PATCH v2 2/3 net: mdio: common handling of phy device reset properties

In reply to Buday Csaba

Unify the handling of the per device reset properties for mdio_device.

Merge mdio_device_register_gpiod() and mdio_device_register_reset() into mdio_device_register_reset(), that handles both reset-controllers and reset-gpios.
Move reading of the reset firmware properties (reset-assert-us, reset-deassert-us) from fwnode_mdio.c to mdio_device_register_reset(), so all reset related initialization code is kept in one place.

Introduce mdio_device_unregister_reset() to release the associated resources.

These changes make tracking the reset properties easier. Added kernel-doc for mdio_device_register/unregister_reset().

Signed-off-by Buday Csaba <[email protected]>

---
V1 -> V2: rebase, no change (leak fix already applied to base)

---
 drivers/net/mdio/fwnode_mdio.c |  5 -----
 drivers/net/phy/mdio_bus.c     | 12 ++--------
 drivers/net/phy/mdio_device.c  | 40 ++++++++++++++++++++++++++--------
 include/linux/mdio.h           |  2 +-
 4 files changed, 34 insertions(+), 25 deletions(-)

drivers/net/mdio/fwnode_mdio.c +0 -5

index 9b41d4697..ba7091518 100644
@@ -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 ~~-10~~

index 1ac942102..748c6a9aa 100644
@@ -41,16 +41,9 @@ int mdiobus_register_device(struct mdio_device *mdiodev)
 		return -EBUSY;
 
 	if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY) {
-		err = mdio_device_register_gpiod(mdiodev);
-		if (err)
-			return err;
-
 		err = mdio_device_register_reset(mdiodev);
-		if (err) {
-			gpiod_put(mdiodev->reset_gpio);
-			mdiodev->reset_gpio = NULL;
+		if (err)
 			return err;
-		}
 
 		/* Assert the reset signal */
 		mdio_device_reset(mdiodev, 1);

@@ -67,8 +60,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 +31 -9

index 5a78d8624..749cf8cdb 100644
@@ -118,8 +118,17 @@ void mdio_device_remove(struct mdio_device *mdiodev)
 }
 EXPORT_SYMBOL(mdio_device_remove);
 
-int mdio_device_register_gpiod(struct mdio_device *mdiodev)
+/**
+ * mdio_device_register_reset - Read and initialize the reset properties of
+ *				an mdio device
+ * @mdiodev: mdio_device structure
+ *
+ * Return: Zero if successful, negative error code on failure
+ */
+int mdio_device_register_reset(struct mdio_device *mdiodev)
 {
+	struct reset_control *reset;
+
 	/* Deassert the optional reset signal */
 	mdiodev->reset_gpio = gpiod_get_optional(&mdiodev->dev,
 						 "reset", GPIOD_OUT_LOW);

@@ -129,22 +138,35 @@ int mdio_device_register_gpiod(struct mdio_device *mdiodev)
 	if (mdiodev->reset_gpio)
 		gpiod_set_consumer_name(mdiodev->reset_gpio, "PHY reset");
 
-	return 0;
-}
-
-int mdio_device_register_reset(struct mdio_device *mdiodev)
-{
-	struct reset_control *reset;
-
 	reset = reset_control_get_optional_exclusive(&mdiodev->dev, "phy");
-	if (IS_ERR(reset))
+	if (IS_ERR(reset)) {
+		gpiod_put(mdiodev->reset_gpio);
+		mdiodev->reset_gpio = NULL;
 		return PTR_ERR(reset);
+	}
 
 	mdiodev->reset_ctrl = reset;
 
+	/* Read optional firmware properties */
+	fwnode_property_read_u32(dev_fwnode(&mdiodev->dev), "reset-assert-us",
+				 &mdiodev->reset_assert_delay);
+	fwnode_property_read_u32(dev_fwnode(&mdiodev->dev), "reset-deassert-us",
+				 &mdiodev->reset_deassert_delay);
+
 	return 0;
 }
 
+/**
+ * mdio_device_unregister_reset - uninitialize the reset properties of
+ *				  an mdio device
+ * @mdiodev: mdio_device structure
+ */
+void mdio_device_unregister_reset(struct mdio_device *mdiodev)
+{
+	gpiod_put(mdiodev->reset_gpio);
+	reset_control_put(mdiodev->reset_ctrl);
+}
+
 void mdio_device_reset(struct mdio_device *mdiodev, int value)
 {
 	unsigned int d;

include/linux/mdio.h +1 -1

index 1322d2623..e76f5a6c2 100644
@@ -92,8 +92,8 @@ void mdio_device_free(struct mdio_device *mdiodev);
 struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr);
 int mdio_device_register(struct mdio_device *mdiodev);
 void mdio_device_remove(struct mdio_device *mdiodev);
-int mdio_device_register_gpiod(struct mdio_device *mdiodev);
 int mdio_device_register_reset(struct mdio_device *mdiodev);
+void mdio_device_unregister_reset(struct mdio_device *mdiodev);
 void mdio_device_reset(struct mdio_device *mdiodev, int value);
 int mdio_driver_register(struct mdio_driver *drv);
 void mdio_driver_unregister(struct mdio_driver *drv);

Signature · 1 line

2.39.5

Buday Csaba

author

2025-11-17 09:34 UTC #6 lore raw

PATCH v2 3/3 net: mdio: improve reset handling in mdio_device.c

In reply to Buday Csaba

Change fwnode_property_read_u32() in mdio_device_register_reset() to device_property_read_u32(), which is more appropriate here.

Make mdio_device_unregister_reset() truly reverse mdio_device_register_reset() by setting the internal fields to their default values.

Signed-off-by Buday Csaba <[email protected]>

---
V1 -> V2: rebase, leak fix removed, since it is already in base

---
 drivers/net/phy/mdio_device.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/net/phy/mdio_device.c +6 -2

index 749cf8cdb..2de401961 100644
@@ -148,9 +148,9 @@ int mdio_device_register_reset(struct mdio_device *mdiodev)
 	mdiodev->reset_ctrl = 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);
 
 	return 0;

@@ -164,7 +164,11 @@ int mdio_device_register_reset(struct mdio_device *mdiodev)
 void mdio_device_unregister_reset(struct mdio_device *mdiodev)
 {
 	gpiod_put(mdiodev->reset_gpio);
+	mdiodev->reset_gpio = NULL;
 	reset_control_put(mdiodev->reset_ctrl);
+	mdiodev->reset_ctrl = NULL;
+	mdiodev->reset_assert_delay = 0;
+	mdiodev->reset_deassert_delay = 0;
 }
 
 void mdio_device_reset(struct mdio_device *mdiodev, int value)

Signature · 1 line

2.39.5