[PATCH net-next v2 0/3] net: mdio: improve reset handling of mdio devices

[PATCH net-next v2 0/3] net: mdio: improve reset handling of mdio devices

[Buday Csaba]

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/cover.1761732347.git.buday.csaba@prolan.hu/
Link: https://lore.kernel.org/all/cover.1761909948.git.buday.csaba@prolan.hu/
Link: https://lore.kernel.org/all/4b419377f8dd7d2f63f919d0f74a336c734f8fff.1762584481.git.buday.csaba@prolan.hu/

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
-- 
2.39.5

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

[Buday Csaba]

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 <buday.csaba@prolan.hu>
---
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(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 435424113..1ac942102 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -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;
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index f64176e0e..5a78d8624 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -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;
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 42d6d47e4..1322d2623 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -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);
-- 
2.39.5

[PATCH net-next v2 2/3] net: mdio: common handling of phy device reset properties

[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 <buday.csaba@prolan.hu>
---
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(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 9b41d4697..ba7091518 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -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
 	 */
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 1ac942102..748c6a9aa 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -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;
 
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index 5a78d8624..749cf8cdb 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -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;
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 1322d2623..e76f5a6c2 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -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);
-- 
2.39.5

[PATCH net-next v2 3/3] net: mdio: improve reset handling in mdio_device.c

[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 <buday.csaba@prolan.hu>
---
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(-)

diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index 749cf8cdb..2de401961 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -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)
-- 
2.39.5

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

[Russell King (Oracle)]

On Mon, Nov 17, 2025 at 10:28:51AM +0100, Buday Csaba wrote:
> diff --git a/include/linux/mdio.h b/include/linux/mdio.h
> index 42d6d47e4..1322d2623 100644
> --- a/include/linux/mdio.h
> +++ b/include/linux/mdio.h
> @@ -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?

-- 
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/3] net: mdio: move device reset functions to mdio_device.c

[Buday Csaba]

On Mon, Nov 17, 2025 at 09:34:49AM +0000, Russell King (Oracle) wrote:
> On Mon, Nov 17, 2025 at 10:28:51AM +0100, Buday Csaba wrote:
> > diff --git a/include/linux/mdio.h b/include/linux/mdio.h
> > index 42d6d47e4..1322d2623 100644
> > --- a/include/linux/mdio.h
> > +++ b/include/linux/mdio.h
> > @@ -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

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

[Andrew Lunn]

On Mon, Nov 17, 2025 at 01:22:32PM +0100, Buday Csaba wrote:
> On Mon, Nov 17, 2025 at 09:34:49AM +0000, Russell King (Oracle) wrote:
> > On Mon, Nov 17, 2025 at 10:28:51AM +0100, Buday Csaba wrote:
> > > diff --git a/include/linux/mdio.h b/include/linux/mdio.h
> > > index 42d6d47e4..1322d2623 100644
> > > --- a/include/linux/mdio.h
> > > +++ b/include/linux/mdio.h
> > > @@ -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?

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