[PATCH net-next v2 0/3] net: mdio: support dynamic OF device changes

[PATCH net-next v2 0/3] net: mdio: support dynamic OF device changes

[James Hilliard]

MDIO buses enumerate firmware children only when the bus is registered. A
later devicetree overlay or status transition is ignored, unlike on I2C and
SPI buses. Supporting live reconfiguration also means that population and
removal can race address scanning, PHY attachment and bus teardown.

Patch 1 factors fixed-address child registration and legacy PHY scanning
into helpers shared by initial and dynamic population.

Patch 2 serializes MDIO device-map changes, reserves addresses while
registration is in progress, and coordinates scans, attachment, removal
and bus teardown. Dynamically removed devices remain pinned until bus
teardown, preserving the borrowed-pointer convention of
mdiobus_get_phy().

Patch 3 adds the OF reconfiguration notifier. It supports fixed-address
PHYs and generic MDIO devices, scanned PHY addresses, and Ethernet PHY
packages. It also preflights overlay removal and rejects removal of
attached or in-flight PHYs before their firmware nodes can disappear.

Build-tested on net-next with arm64 defconfig and x86_64 allmodconfig and
allyesconfig configurations under W=1. No warning was emitted from a
changed file. Runtime-tested on Allwinner H616 hardware with
notifier-driven addition of an AC300 control endpoint at a fixed MDIO
address, 20 relay-driven cold boots, 20 complete MAC/MDIO teardown and
recreation cycles, and link traffic after each topology was populated.

Changes v1 -> v2:
  - split the generic MDIO work from the ACx00 series
  - rebase onto current net-next
  - move touched declarations to function scope for netdev style
  - Link to v1:
    https://patch.msgid.link/20260802-submit-acx00-of-dynamic-v1-v1-0-0a53cd9e21cc@gmail.com

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
James Hilliard (3):
      net: mdio: factor out OF child registration helpers
      net: mdio: make device map changes hotplug-safe
      net: mdio: support dynamic OF device changes

 drivers/net/mdio/of_mdio.c          | 508 +++++++++++++++++++++++++++++++++---
 drivers/net/phy/mdio_bus.c          |  14 +-
 drivers/net/phy/mdio_bus_provider.c | 141 +++++++---
 drivers/net/phy/mdio_device.c       | 235 +++++++++++++++--
 drivers/net/phy/phy_device.c        | 102 ++++++--
 drivers/net/phy/phylib-internal.h   |   4 +-
 include/linux/mdio.h                |   6 +
 include/linux/phy.h                 |  18 ++
 8 files changed, 908 insertions(+), 120 deletions(-)
---
base-commit: d661abdc30c254649c32ef6e0aa1e621e04ff0a7
change-id: 20260803-submit-mdio-of-dynamic-v2-90560ca159b9

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

[PATCH net-next v2 1/3] net: mdio: factor out OF child registration helpers

[James Hilliard]

Live OF reconfiguration needs to create the same PHY and generic MDIO
devices as initial bus population, including legacy PHY nodes that need
address scanning.

Factor fixed-address child registration and PHY address scanning into
helpers. The old scan loop also visited every free address for a child
without a reg property even when the child was not a PHY. Reject that case
before scanning while preserving enumeration of valid PHY and generic MDIO
children.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes v1 -> v2:
  - split from the ACx00 series without code changes
---
 drivers/net/mdio/of_mdio.c | 66 ++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index b8d298c04d3f..051e449bbe7c 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -77,6 +77,15 @@ static int of_mdiobus_register_device(struct mii_bus *mdio,
 	return 0;
 }
 
+static int of_mdiobus_register_child(struct mii_bus *mdio,
+				     struct device_node *child, u32 addr)
+{
+	if (of_mdiobus_child_is_phy(child))
+		return of_mdiobus_register_phy(mdio, child, addr);
+
+	return of_mdiobus_register_device(mdio, child, addr);
+}
+
 /* The following is a list of PHY compatible strings which appear in
  * some DTBs. The compatible string is never matched against a PHY
  * driver, so is pointless. We only expect devices which are not PHYs
@@ -136,6 +145,32 @@ bool of_mdiobus_child_is_phy(struct device_node *child)
 }
 EXPORT_SYMBOL(of_mdiobus_child_is_phy);
 
+static int of_mdiobus_scan_phy(struct mii_bus *mdio,
+			       struct device_node *child)
+{
+	int addr, rc;
+
+	if (!of_mdiobus_child_is_phy(child))
+		return -ENODEV;
+
+	for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
+		if (mdiobus_is_registered_device(mdio, addr))
+			continue;
+
+		dev_info(&mdio->dev, "scan phy %pOFn at address %i\n",
+			 child, addr);
+
+		/* -ENODEV means that scanning should continue. */
+		rc = of_mdiobus_register_child(mdio, child, addr);
+		if (!rc)
+			return 0;
+		if (rc != -ENODEV)
+			return rc;
+	}
+
+	return -ENODEV;
+}
+
 static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
 				   bool *scanphys)
 {
@@ -164,10 +199,7 @@ static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
 			continue;
 		}
 
-		if (of_mdiobus_child_is_phy(child))
-			rc = of_mdiobus_register_phy(mdio, child, addr);
-		else
-			rc = of_mdiobus_register_device(mdio, child, addr);
+		rc = of_mdiobus_register_child(mdio, child, addr);
 
 		if (rc == -ENODEV)
 			dev_err(&mdio->dev,
@@ -197,7 +229,7 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
 {
 	struct device_node *child;
 	bool scanphys = false;
-	int addr, rc;
+	int rc;
 
 	if (!np)
 		return __mdiobus_register(mdio, owner);
@@ -238,27 +270,9 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
 		    of_node_name_eq(child, "ethernet-phy-package"))
 			continue;
 
-		for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
-			/* skip already registered PHYs */
-			if (mdiobus_is_registered_device(mdio, addr))
-				continue;
-
-			/* be noisy to encourage people to set reg property */
-			dev_info(&mdio->dev, "scan phy %pOFn at address %i\n",
-				 child, addr);
-
-			if (of_mdiobus_child_is_phy(child)) {
-				/* -ENODEV is the return code that PHYLIB has
-				 * standardized on to indicate that bus
-				 * scanning should continue.
-				 */
-				rc = of_mdiobus_register_phy(mdio, child, addr);
-				if (!rc)
-					break;
-				if (rc != -ENODEV)
-					goto put_unregister;
-			}
-		}
+		rc = of_mdiobus_scan_phy(mdio, child);
+		if (rc && rc != -ENODEV)
+			goto put_unregister;
 	}
 
 	return 0;

-- 
2.53.0

[PATCH net-next v2 2/3] net: mdio: make device map changes hotplug-safe

[James Hilliard]

MDIO device registration currently publishes directly into mdio_map
without serializing address ownership, and removal frees the object
immediately. That is sufficient while bus population is static, but
dynamic firmware changes can race registration, PHY attachment and bus
teardown. It also cannot safely preserve mdiobus_get_phy()'s
borrowed-pointer convention.

Serialize device-map state and reserve addresses while registration is
in progress. Track active scans, registrations and firmware changes so
bus teardown first blocks new work and waits for existing work before
consuming the final map. Block new registrations and PHY attachment while
a firmware removal owns the map.

Publish initialized devices with release ordering and make PHY attachment
atomic with map removal, including standalone PHY users. Have address
scans atomically claim a matching OF node as well, so explicit OF
population cannot instantiate the same device concurrently.

Keep dynamically removed devices pinned until the MDIO bus is torn down
so existing borrowed pointers cannot become use-after-free references.
Drop their firmware-node reference after device_del() so this does not
keep an overlay node alive. Normal MDIO removal and driver APIs remain
unchanged, and consumers do not need to adopt a new refcounting API.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes v1 -> v2:
  - split from the ACx00 series
  - rebase onto net-next after <linux/mdio.h> stopped including
    <linux/mod_devicetable.h>
  - move touched declarations to function scope for netdev style
---
 drivers/net/phy/mdio_bus.c          |  14 ++-
 drivers/net/phy/mdio_bus_provider.c | 141 ++++++++++++++++------
 drivers/net/phy/mdio_device.c       | 235 ++++++++++++++++++++++++++++++++----
 drivers/net/phy/phy_device.c        | 102 ++++++++++++----
 drivers/net/phy/phylib-internal.h   |   4 +-
 include/linux/mdio.h                |   6 +
 include/linux/phy.h                 |  18 +++
 7 files changed, 433 insertions(+), 87 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 00d0e4159e9b..f45c936fe5aa 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -34,7 +34,8 @@ static struct mdio_device *mdiobus_find_device(struct mii_bus *bus, int addr)
 	if (WARN_ONCE(!addr_valid, "addr %d out of range\n", addr))
 		return NULL;
 
-	return bus->mdio_map[addr];
+	/* Pair with map publication in mdiobus_register_device(). */
+	return smp_load_acquire(&bus->mdio_map[addr]);
 }
 
 struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr)
@@ -54,7 +55,16 @@ EXPORT_SYMBOL(mdiobus_get_phy);
 
 bool mdiobus_is_registered_device(struct mii_bus *bus, int addr)
 {
-	return mdiobus_find_device(bus, addr) != NULL;
+	bool addr_valid = addr >= 0 && addr < ARRAY_SIZE(bus->mdio_map);
+	bool registered;
+
+	if (WARN_ONCE(!addr_valid, "addr %d out of range\n", addr))
+		return false;
+
+	registered = READ_ONCE(bus->mdio_map[addr]) ||
+		     (READ_ONCE(bus->mdio_map_pending) & BIT(addr));
+
+	return registered;
 }
 EXPORT_SYMBOL(mdiobus_is_registered_device);
 
diff --git a/drivers/net/phy/mdio_bus_provider.c b/drivers/net/phy/mdio_bus_provider.c
index ce3a607a40cb..df80125eeec4 100644
--- a/drivers/net/phy/mdio_bus_provider.c
+++ b/drivers/net/phy/mdio_bus_provider.c
@@ -330,6 +330,9 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
 		return NULL;
 
 	bus->state = MDIOBUS_ALLOCATED;
+	mutex_init(&bus->mdio_map_lock);
+	init_waitqueue_head(&bus->mdio_map_wait);
+	INIT_LIST_HEAD(&bus->mdio_map_retired);
 	if (size)
 		bus->priv = (void *)bus + aligned_size;
 
@@ -355,10 +358,9 @@ static int of_mdiobus_find_phy(struct device *dev, struct mdio_device *mdiodev,
 			       struct device_node *np)
 {
 	struct device_node *child;
+	int addr, ret;
 
 	for_each_available_child_of_node(np, child) {
-		int addr;
-
 		if (of_node_name_eq(child, "ethernet-phy-package")) {
 			/* Validate PHY package reg presence */
 			if (!of_property_present(child, "reg")) {
@@ -366,12 +368,13 @@ static int of_mdiobus_find_phy(struct device *dev, struct mdio_device *mdiodev,
 				return -EINVAL;
 			}
 
-			if (!of_mdiobus_find_phy(dev, mdiodev, child)) {
+			ret = of_mdiobus_find_phy(dev, mdiodev, child);
+			if (ret != -ENODEV) {
 				/* The refcount for the PHY package will be
 				 * incremented later when PHY join the Package.
 				 */
 				of_node_put(child);
-				return 0;
+				return ret;
 			}
 
 			continue;
@@ -382,6 +385,11 @@ static int of_mdiobus_find_phy(struct device *dev, struct mdio_device *mdiodev,
 			continue;
 
 		if (addr == mdiodev->addr) {
+			if (of_node_test_and_set_flag(child, OF_POPULATED)) {
+				of_node_put(child);
+				return -EBUSY;
+			}
+
 			device_set_node(dev, of_fwnode_handle(child));
 			/* The refcount on "child" is passed to the mdio
 			 * device. Do _not_ use of_node_put(child) here.
@@ -393,22 +401,26 @@ static int of_mdiobus_find_phy(struct device *dev, struct mdio_device *mdiodev,
 	return -ENODEV;
 }
 
-static void of_mdiobus_link_mdiodev(struct mii_bus *bus,
-				    struct mdio_device *mdiodev)
+static int of_mdiobus_link_mdiodev(struct mii_bus *bus,
+				   struct mdio_device *mdiodev)
 {
 	struct device *dev = &mdiodev->dev;
 
 	if (dev->of_node || !bus->dev.of_node)
-		return;
+		return 0;
 
-	of_mdiobus_find_phy(dev, mdiodev, bus->dev.of_node);
+	return of_mdiobus_find_phy(dev, mdiodev, bus->dev.of_node);
 }
 #endif
 
-static struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr, bool c45)
+static struct phy_device *__mdiobus_scan(struct mii_bus *bus, int addr,
+					 bool c45)
 {
 	struct phy_device *phydev = ERR_PTR(-ENODEV);
 	struct fwnode_handle *fwnode;
+#if IS_ENABLED(CONFIG_OF_MDIO)
+	bool of_node_populated = false;
+#endif
 	char node_name[16];
 	int err;
 
@@ -420,7 +432,12 @@ static struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr, bool c45)
 	/* For DT, see if the auto-probed phy has a corresponding child
 	 * in the bus node, and set the of_node pointer in this case.
 	 */
-	of_mdiobus_link_mdiodev(bus, &phydev->mdio);
+	err = of_mdiobus_link_mdiodev(bus, &phydev->mdio);
+	if (err == -EBUSY) {
+		phy_device_free(phydev);
+		return ERR_PTR(-ENODEV);
+	}
+	of_node_populated = !!phydev->mdio.dev.of_node;
 #endif
 
 	/* Search for a swnode for the phy in the swnode hierarchy of the bus.
@@ -437,6 +454,11 @@ static struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr, bool c45)
 
 	err = phy_device_register(phydev);
 	if (err) {
+#if IS_ENABLED(CONFIG_OF_MDIO)
+		if (of_node_populated)
+			of_node_clear_flag(phydev->mdio.dev.of_node,
+					   OF_POPULATED);
+#endif
 		phy_device_free(phydev);
 		return ERR_PTR(-ENODEV);
 	}
@@ -458,7 +480,17 @@ static struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr, bool c45)
  */
 struct phy_device *mdiobus_scan_c22(struct mii_bus *bus, int addr)
 {
-	return mdiobus_scan(bus, addr, false);
+	struct phy_device *phydev;
+	int err;
+
+	err = mdiobus_device_change_begin(bus, false);
+	if (err)
+		return ERR_PTR(err);
+
+	phydev = __mdiobus_scan(bus, addr, false);
+	mdiobus_device_change_end(bus, false);
+
+	return phydev;
 }
 EXPORT_SYMBOL(mdiobus_scan_c22);
 
@@ -476,7 +508,7 @@ EXPORT_SYMBOL(mdiobus_scan_c22);
  */
 static struct phy_device *mdiobus_scan_c45(struct mii_bus *bus, int addr)
 {
-	return mdiobus_scan(bus, addr, true);
+	return __mdiobus_scan(bus, addr, true);
 }
 
 static int mdiobus_scan_bus_c22(struct mii_bus *bus)
@@ -487,7 +519,7 @@ static int mdiobus_scan_bus_c22(struct mii_bus *bus)
 		if ((bus->phy_mask & BIT(i)) == 0) {
 			struct phy_device *phydev;
 
-			phydev = mdiobus_scan_c22(bus, i);
+			phydev = __mdiobus_scan(bus, i, false);
 			if (IS_ERR(phydev) && (PTR_ERR(phydev) != -ENODEV))
 				return PTR_ERR(phydev);
 		}
@@ -504,7 +536,7 @@ static int mdiobus_scan_bus_c45(struct mii_bus *bus)
 			struct phy_device *phydev;
 
 			/* Don't scan C45 if we already have a C22 device */
-			if (bus->mdio_map[i])
+			if (mdiobus_is_registered_device(bus, i))
 				continue;
 
 			phydev = mdiobus_scan_c45(bus, i);
@@ -536,6 +568,44 @@ static bool mdiobus_prevent_c45_scan(struct mii_bus *bus)
 	return false;
 }
 
+static void mdiobus_stop_device_changes(struct mii_bus *bus)
+{
+	mutex_lock(&bus->mdio_map_lock);
+	bus->state = MDIOBUS_UNREGISTERING;
+	mutex_unlock(&bus->mdio_map_lock);
+
+	wait_event(bus->mdio_map_wait, !READ_ONCE(bus->mdio_map_ops));
+}
+
+static void mdiobus_remove_devices(struct mii_bus *bus)
+{
+	LIST_HEAD(removed);
+	struct mdio_device *mdiodev, *next;
+	int i;
+
+	for (i = 0; i < PHY_MAX_ADDR; i++) {
+		mdiodev = bus->mdio_map[i];
+		if (!mdiodev)
+			continue;
+
+		mdiodev->device_remove(mdiodev);
+		mdiodev->device_free(mdiodev);
+	}
+
+	mutex_lock(&bus->mdio_map_lock);
+	list_splice_init(&bus->mdio_map_retired, &removed);
+	mutex_unlock(&bus->mdio_map_lock);
+
+	list_for_each_entry_safe(mdiodev, next, &removed, retired_node) {
+		list_del_init(&mdiodev->retired_node);
+		mdio_device_put(mdiodev);
+	}
+
+	mutex_lock(&bus->mdio_map_lock);
+	bus->state = MDIOBUS_UNREGISTERED;
+	mutex_unlock(&bus->mdio_map_lock);
+}
+
 /**
  * __mdiobus_register - bring up all the PHYs on a given bus and attach them to bus
  * @bus: target mii_bus
@@ -552,10 +622,9 @@ static bool mdiobus_prevent_c45_scan(struct mii_bus *bus)
  */
 int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 {
-	struct mdio_device *mdiodev;
 	struct gpio_desc *gpiod;
 	bool prevent_c45_scan;
-	int i, err;
+	int err;
 
 	if (!bus || !bus->name)
 		return -EINVAL;
@@ -596,7 +665,9 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 	 *
 	 * State will be updated later in this function in case of success
 	 */
+	mutex_lock(&bus->mdio_map_lock);
 	bus->state = MDIOBUS_UNREGISTERED;
+	mutex_unlock(&bus->mdio_map_lock);
 
 	err = device_register(&bus->dev);
 	if (err) {
@@ -613,8 +684,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 		err = dev_err_probe(&bus->dev, PTR_ERR(gpiod),
 				    "mii_bus %s couldn't get reset GPIO\n",
 				    bus->id);
-		device_del(&bus->dev);
-		return err;
+		goto error_reset_gpiod;
 	} else	if (gpiod) {
 		bus->reset_gpiod = gpiod;
 		fsleep(bus->reset_delay_us);
@@ -629,6 +699,10 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 			goto error_reset_gpiod;
 	}
 
+	mutex_lock(&bus->mdio_map_lock);
+	bus->state = MDIOBUS_REGISTERING;
+	mutex_unlock(&bus->mdio_map_lock);
+
 	if (bus->read) {
 		err = mdiobus_scan_bus_c22(bus);
 		if (err)
@@ -643,20 +717,17 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 			goto error;
 	}
 
+	mutex_lock(&bus->mdio_map_lock);
 	bus->state = MDIOBUS_REGISTERED;
+	mutex_unlock(&bus->mdio_map_lock);
 	dev_dbg(&bus->dev, "probed\n");
 	return 0;
 
 error:
-	for (i = 0; i < PHY_MAX_ADDR; i++) {
-		mdiodev = bus->mdio_map[i];
-		if (!mdiodev)
-			continue;
-
-		mdiodev->device_remove(mdiodev);
-		mdiodev->device_free(mdiodev);
-	}
 error_reset_gpiod:
+	mdiobus_stop_device_changes(bus);
+	mdiobus_remove_devices(bus);
+
 	/* Put PHYs in RESET to save power */
 	if (bus->reset_gpiod)
 		gpiod_set_value_cansleep(bus->reset_gpiod, 1);
@@ -668,21 +739,11 @@ EXPORT_SYMBOL(__mdiobus_register);
 
 void mdiobus_unregister(struct mii_bus *bus)
 {
-	struct mdio_device *mdiodev;
-	int i;
-
 	if (WARN_ON_ONCE(bus->state != MDIOBUS_REGISTERED))
 		return;
-	bus->state = MDIOBUS_UNREGISTERED;
-
-	for (i = 0; i < PHY_MAX_ADDR; i++) {
-		mdiodev = bus->mdio_map[i];
-		if (!mdiodev)
-			continue;
 
-		mdiodev->device_remove(mdiodev);
-		mdiodev->device_free(mdiodev);
-	}
+	mdiobus_stop_device_changes(bus);
+	mdiobus_remove_devices(bus);
 
 	/* Put PHYs in RESET to save power */
 	if (bus->reset_gpiod)
@@ -702,8 +763,11 @@ EXPORT_SYMBOL(mdiobus_unregister);
  */
 void mdiobus_free(struct mii_bus *bus)
 {
+	mutex_lock(&bus->mdio_map_lock);
+
 	/* For compatibility with error handling in drivers. */
 	if (bus->state == MDIOBUS_ALLOCATED) {
+		mutex_unlock(&bus->mdio_map_lock);
 		kfree(bus);
 		return;
 	}
@@ -711,6 +775,7 @@ void mdiobus_free(struct mii_bus *bus)
 	WARN(bus->state != MDIOBUS_UNREGISTERED,
 	     "%s: not in UNREGISTERED state\n", bus->id);
 	bus->state = MDIOBUS_RELEASED;
+	mutex_unlock(&bus->mdio_map_lock);
 
 	put_device(&bus->dev);
 }
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index 06151f207134..3b19e8315f61 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -15,6 +15,7 @@
 #include <linux/mdio.h>
 #include <linux/mii.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
@@ -33,12 +34,16 @@
 static int mdio_device_register_reset(struct mdio_device *mdiodev)
 {
 	struct reset_control *reset;
+	int err;
 
 	/* 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 (IS_ERR(mdiodev->reset_gpio)) {
+		err = PTR_ERR(mdiodev->reset_gpio);
+		mdiodev->reset_gpio = NULL;
+		return err;
+	}
 
 	if (mdiodev->reset_gpio)
 		gpiod_set_consumer_name(mdiodev->reset_gpio, "PHY reset");
@@ -116,6 +121,13 @@ static void mdio_device_release(struct device *dev)
 	kfree(to_mdio_device(dev));
 }
 
+static int __mdio_device_remove(struct mdio_device *mdiodev, bool dynamic);
+
+static int mdio_device_remove_dynamic(struct mdio_device *mdiodev)
+{
+	return __mdio_device_remove(mdiodev, true);
+}
+
 struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr)
 {
 	struct mdio_device *mdiodev;
@@ -130,9 +142,11 @@ struct mdio_device *mdio_device_create(struct mii_bus *bus, int addr)
 	mdiodev->dev.bus = &mdio_bus_type;
 	mdiodev->device_free = mdio_device_free;
 	mdiodev->device_remove = mdio_device_remove;
+	mdiodev->device_remove_dynamic = mdio_device_remove_dynamic;
 	mdiodev->bus = bus;
 	mdiodev->addr = addr;
 	mdiodev->reset_state = -1;
+	INIT_LIST_HEAD(&mdiodev->retired_node);
 
 	dev_set_name(&mdiodev->dev, PHY_ID_FMT, bus->id, addr);
 
@@ -159,19 +173,27 @@ int mdio_device_register(struct mdio_device *mdiodev)
 		return err;
 
 	err = device_add(&mdiodev->dev);
-	if (err) {
+	if (err)
 		pr_err("MDIO %d failed to add\n", mdiodev->addr);
-		goto out;
-	}
-
-	return 0;
 
- out:
-	mdiobus_unregister_device(mdiodev);
-	return err;
+	return mdiobus_registration_done(mdiodev, err);
 }
 EXPORT_SYMBOL(mdio_device_register);
 
+static int __mdio_device_remove(struct mdio_device *mdiodev, bool dynamic)
+{
+	int err;
+
+	err = mdiobus_begin_remove(mdiodev, dynamic);
+	if (err)
+		return err;
+
+	device_del(&mdiodev->dev);
+	mdiobus_finish_remove(mdiodev, dynamic);
+
+	return 0;
+}
+
 /**
  * mdio_device_remove - Remove a previously registered mdio device from the
  *			MDIO bus
@@ -183,42 +205,211 @@ EXPORT_SYMBOL(mdio_device_register);
  */
 void mdio_device_remove(struct mdio_device *mdiodev)
 {
-	device_del(&mdiodev->dev);
-	mdiobus_unregister_device(mdiodev);
+	__mdio_device_remove(mdiodev, false);
 }
 EXPORT_SYMBOL(mdio_device_remove);
 
 int mdiobus_register_device(struct mdio_device *mdiodev)
 {
+	struct mii_bus *bus = mdiodev->bus;
 	int err;
 
-	if (mdiodev->bus->mdio_map[mdiodev->addr])
-		return -EBUSY;
+	mutex_lock(&bus->mdio_map_lock);
+	if (bus->state != MDIOBUS_REGISTERING &&
+	    bus->state != MDIOBUS_REGISTERED) {
+		err = -ENODEV;
+		goto out_unlock;
+	}
+	if (bus->mdio_map_removals) {
+		err = -EBUSY;
+		goto out_unlock;
+	}
+
+	if (bus->mdio_map[mdiodev->addr] ||
+	    bus->mdio_map_pending & BIT(mdiodev->addr)) {
+		err = -EBUSY;
+		goto out_unlock;
+	}
+
+	bus->mdio_map_pending |= BIT(mdiodev->addr);
+	bus->mdio_map_ops++;
+	mutex_unlock(&bus->mdio_map_lock);
 
 	if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY) {
 		err = mdio_device_register_reset(mdiodev);
-		if (err)
+		if (err) {
+			mdiobus_registration_done(mdiodev, err);
 			return err;
+		}
 
 		/* Assert the reset signal */
 		mdio_device_reset(mdiodev, 1);
 	}
 
-	mdiodev->bus->mdio_map[mdiodev->addr] = mdiodev;
+	mutex_lock(&bus->mdio_map_lock);
+	/* Teardown waits for this registration before consuming the map. */
+	smp_store_release(&bus->mdio_map[mdiodev->addr], mdiodev);
+	mutex_unlock(&bus->mdio_map_lock);
 
 	return 0;
+
+out_unlock:
+	mutex_unlock(&bus->mdio_map_lock);
+	return err;
 }
 
-int mdiobus_unregister_device(struct mdio_device *mdiodev)
+/**
+ * mdiobus_device_change_begin - start changing devices on a registered bus
+ * @bus: MDIO bus that will be scanned or changed
+ * @removing: whether PHY attachment must be blocked during the change
+ *
+ * Return: zero on success or a negative error code when the bus is unavailable
+ */
+int mdiobus_device_change_begin(struct mii_bus *bus, bool removing)
 {
-	if (mdiodev->bus->mdio_map[mdiodev->addr] != mdiodev)
-		return -EINVAL;
+	int err = 0;
+
+	mutex_lock(&bus->mdio_map_lock);
+	if (bus->state != MDIOBUS_REGISTERED) {
+		err = -ENODEV;
+	} else {
+		bus->mdio_map_ops++;
+		if (removing)
+			bus->mdio_map_removals++;
+	}
+	mutex_unlock(&bus->mdio_map_lock);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(mdiobus_device_change_begin);
 
-	mdio_device_unregister_reset(mdiodev);
+static void mdiobus_operation_done_locked(struct mii_bus *bus)
+{
+	lockdep_assert_held(&bus->mdio_map_lock);
 
-	mdiodev->bus->mdio_map[mdiodev->addr] = NULL;
+	if (WARN_ON_ONCE(!bus->mdio_map_ops))
+		return;
+	bus->mdio_map_ops--;
+	if (!bus->mdio_map_ops)
+		wake_up_all(&bus->mdio_map_wait);
+}
 
-	return 0;
+/**
+ * mdiobus_device_change_end - finish changing devices on an MDIO bus
+ * @bus: MDIO bus previously passed to mdiobus_device_change_begin()
+ * @removing: value passed to mdiobus_device_change_begin()
+ */
+void mdiobus_device_change_end(struct mii_bus *bus, bool removing)
+{
+	mutex_lock(&bus->mdio_map_lock);
+	if (removing) {
+		if (!WARN_ON_ONCE(!bus->mdio_map_removals))
+			bus->mdio_map_removals--;
+	}
+	mdiobus_operation_done_locked(bus);
+	mutex_unlock(&bus->mdio_map_lock);
+}
+EXPORT_SYMBOL_GPL(mdiobus_device_change_end);
+
+static void mdiobus_operation_done(struct mii_bus *bus)
+{
+	mutex_lock(&bus->mdio_map_lock);
+	mdiobus_operation_done_locked(bus);
+	mutex_unlock(&bus->mdio_map_lock);
+}
+
+static void mdiobus_unpublish_device(struct mdio_device *mdiodev)
+{
+	struct mii_bus *bus = mdiodev->bus;
+
+	lockdep_assert_held(&bus->mdio_map_lock);
+
+	if (bus->mdio_map[mdiodev->addr] == mdiodev)
+		WRITE_ONCE(bus->mdio_map[mdiodev->addr], NULL);
+	if (mdiodev->dev.of_node)
+		of_node_clear_flag(mdiodev->dev.of_node, OF_POPULATED);
+}
+
+int mdiobus_registration_done(struct mdio_device *mdiodev, int err)
+{
+	struct mii_bus *bus = mdiodev->bus;
+
+	mutex_lock(&bus->mdio_map_lock);
+	if (WARN_ON_ONCE(!(bus->mdio_map_pending & BIT(mdiodev->addr))))
+		goto out_unlock;
+
+	if (err)
+		mdiobus_unpublish_device(mdiodev);
+	else
+		WARN_ON_ONCE(bus->mdio_map[mdiodev->addr] != mdiodev);
+
+	bus->mdio_map_pending &= ~BIT(mdiodev->addr);
+
+out_unlock:
+	mutex_unlock(&bus->mdio_map_lock);
+	if (err) {
+		if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY) {
+			mdio_device_reset(mdiodev, 1);
+			mdio_device_unregister_reset(mdiodev);
+		}
+	}
+	mdiobus_operation_done(bus);
+
+	return err;
+}
+
+int mdiobus_begin_remove(struct mdio_device *mdiodev, bool dynamic)
+{
+	struct mii_bus *bus = mdiodev->bus;
+	int err = 0;
+
+	mutex_lock(&bus->mdio_map_lock);
+	if (dynamic && (bus->state == MDIOBUS_UNREGISTERED ||
+			bus->state == MDIOBUS_RELEASED)) {
+		err = -ENODEV;
+		goto out_unlock;
+	}
+	if (bus->mdio_map_pending & BIT(mdiodev->addr)) {
+		err = -EBUSY;
+		goto out_unlock;
+	}
+
+	if (bus->mdio_map[mdiodev->addr] != mdiodev) {
+		err = -ENODEV;
+		goto out_unlock;
+	}
+
+	if (dynamic && mdiodev->flags & MDIO_DEVICE_FLAG_PHY &&
+	    to_phy_device(&mdiodev->dev)->attached) {
+		err = -EBUSY;
+		goto out_unlock;
+	}
+
+	mdiobus_unpublish_device(mdiodev);
+
+	if (dynamic) {
+		mdio_device_get(mdiodev);
+		list_add_tail(&mdiodev->retired_node, &bus->mdio_map_retired);
+	}
+
+out_unlock:
+	mutex_unlock(&bus->mdio_map_lock);
+	return err;
+}
+
+void mdiobus_finish_remove(struct mdio_device *mdiodev, bool dynamic)
+{
+	struct fwnode_handle *fwnode;
+
+	if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY)
+		mdio_device_unregister_reset(mdiodev);
+
+	/* Do not keep an overlay node alive with the retired device. */
+	if (dynamic) {
+		fwnode = dev_fwnode(&mdiodev->dev);
+		device_set_node(&mdiodev->dev, NULL);
+		fwnode_handle_put(fwnode);
+	}
 }
 
 /**
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 94b2e85e00a3..3b182957c3de 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -227,6 +227,8 @@ static void phy_device_release(struct device *dev)
 	kfree(to_phy_device(dev));
 }
 
+static int __phy_device_remove(struct phy_device *phydev, bool dynamic);
+
 static void phy_mdio_device_remove(struct mdio_device *mdiodev)
 {
 	struct phy_device *phydev;
@@ -235,6 +237,14 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)
 	phy_device_remove(phydev);
 }
 
+static int phy_mdio_device_remove_dynamic(struct mdio_device *mdiodev)
+{
+	struct phy_device *phydev;
+
+	phydev = container_of(mdiodev, struct phy_device, mdio);
+	return __phy_device_remove(phydev, true);
+}
+
 static struct phy_driver genphy_driver;
 
 static LIST_HEAD(phy_fixup_list);
@@ -768,7 +778,9 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
 	mdiodev->flags = MDIO_DEVICE_FLAG_PHY;
 	mdiodev->device_free = phy_mdio_device_free;
 	mdiodev->device_remove = phy_mdio_device_remove;
+	mdiodev->device_remove_dynamic = phy_mdio_device_remove_dynamic;
 	mdiodev->reset_state = -1;
+	INIT_LIST_HEAD(&mdiodev->retired_node);
 
 	dev->speed = SPEED_UNKNOWN;
 	dev->duplex = DUPLEX_UNKNOWN;
@@ -1121,25 +1133,40 @@ int phy_device_register(struct phy_device *phydev)
 	err = phy_scan_fixups(phydev);
 	if (err) {
 		phydev_err(phydev, "failed to initialize\n");
-		goto out;
+		return mdiobus_registration_done(&phydev->mdio, err);
 	}
 
 	err = device_add(&phydev->mdio.dev);
-	if (err) {
+	if (err)
 		phydev_err(phydev, "failed to add\n");
-		goto out;
-	}
 
-	return 0;
+	return mdiobus_registration_done(&phydev->mdio, err);
+}
+EXPORT_SYMBOL(phy_device_register);
+
+static int __phy_device_remove(struct phy_device *phydev, bool dynamic)
+{
+	int err;
+
+	err = mdiobus_begin_remove(&phydev->mdio, dynamic);
+	if (dynamic && err == -EBUSY)
+		dev_warn(&phydev->mdio.dev,
+			 "cannot remove a PHY while it is attached or being registered\n");
+	if (err)
+		return err;
+
+	unregister_mii_timestamper(phydev->mii_ts);
+	pse_control_put(phydev->psec);
+
+	device_del(&phydev->mdio.dev);
 
- out:
 	/* Assert the reset signal */
 	phy_device_reset(phydev, 1);
 
-	mdiobus_unregister_device(&phydev->mdio);
-	return err;
+	mdiobus_finish_remove(&phydev->mdio, dynamic);
+
+	return 0;
 }
-EXPORT_SYMBOL(phy_device_register);
 
 /**
  * phy_device_remove - Remove a previously registered phy device from the MDIO bus
@@ -1151,15 +1178,7 @@ EXPORT_SYMBOL(phy_device_register);
  */
 void phy_device_remove(struct phy_device *phydev)
 {
-	unregister_mii_timestamper(phydev->mii_ts);
-	pse_control_put(phydev->psec);
-
-	device_del(&phydev->mdio.dev);
-
-	/* Assert the reset signal */
-	phy_device_reset(phydev, 1);
-
-	mdiobus_unregister_device(&phydev->mdio);
+	__phy_device_remove(phydev, false);
 }
 EXPORT_SYMBOL(phy_device_remove);
 
@@ -1734,6 +1753,36 @@ static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
+static int phy_claim(struct phy_device *phydev)
+{
+	struct mdio_device *mdiodev = &phydev->mdio;
+	struct mii_bus *bus = mdiodev->bus;
+	int err = 0;
+
+	mutex_lock(&bus->mdio_map_lock);
+	if (bus->state != MDIOBUS_REGISTERED ||
+	    bus->mdio_map_removals ||
+	    bus->mdio_map[mdiodev->addr] != mdiodev ||
+	    (bus->mdio_map_pending & BIT(mdiodev->addr)))
+		err = -ENODEV;
+	else if (phydev->attached)
+		err = -EBUSY;
+	else
+		phydev->attached = true;
+	mutex_unlock(&bus->mdio_map_lock);
+
+	return err;
+}
+
+static void phy_release(struct phy_device *phydev)
+{
+	struct mii_bus *bus = phydev->mdio.bus;
+
+	mutex_lock(&bus->mdio_map_lock);
+	phydev->attached = false;
+	mutex_unlock(&bus->mdio_map_lock);
+}
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1755,6 +1804,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	struct mii_bus *bus = phydev->mdio.bus;
 	struct device *d = &phydev->mdio.dev;
 	struct module *ndev_owner = NULL;
+	bool claimed = false;
 	int err;
 
 	/* For Ethernet device drivers that register their own MDIO bus, we
@@ -1770,6 +1820,13 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	}
 
 	get_device(d);
+	err = phy_claim(phydev);
+	if (err == -EBUSY)
+		phydev_err(phydev, "PHY already attached\n");
+	if (!err)
+		claimed = true;
+	if (err)
+		goto error_put_device;
 
 	/* Assume that if there is no driver, that it doesn't
 	 * exist, and we should use the genphy driver.
@@ -1798,12 +1855,6 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 			goto error_module_put;
 	}
 
-	if (phydev->attached_dev) {
-		dev_err(&dev->dev, "PHY already attached\n");
-		err = -EBUSY;
-		goto error;
-	}
-
 	phydev->phy_link_change = phy_link_change;
 	if (dev) {
 		phydev->attached_dev = dev;
@@ -1899,6 +1950,8 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	phydev->is_genphy_driven = 0;
 	d->driver = NULL;
 error_put_device:
+	if (claimed)
+		phy_release(phydev);
 	put_device(d);
 	if (ndev_owner != bus->owner)
 		module_put(bus->owner);
@@ -1977,6 +2030,7 @@ void phy_detach(struct phy_device *phydev)
 	 * a use-after-free bug by reading the underlying bus first.
 	 */
 	bus = phydev->mdio.bus;
+	phy_release(phydev);
 
 	put_device(&phydev->mdio.dev);
 	if (dev)
diff --git a/drivers/net/phy/phylib-internal.h b/drivers/net/phy/phylib-internal.h
index 664ed7faa518..7e9161fcdd90 100644
--- a/drivers/net/phy/phylib-internal.h
+++ b/drivers/net/phy/phylib-internal.h
@@ -25,7 +25,9 @@ int phy_speed_down_core(struct phy_device *phydev);
 void phy_check_downshift(struct phy_device *phydev);
 
 int mdiobus_register_device(struct mdio_device *mdiodev);
-int mdiobus_unregister_device(struct mdio_device *mdiodev);
+int mdiobus_registration_done(struct mdio_device *mdiodev, int err);
+int mdiobus_begin_remove(struct mdio_device *mdiodev, bool dynamic);
+void mdiobus_finish_remove(struct mdio_device *mdiodev, bool dynamic);
 
 int genphy_c45_read_eee_adv(struct phy_device *phydev, unsigned long *adv);
 
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index a7d9e3ae362a..e19963af8e13 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -8,6 +8,7 @@
 
 #include <uapi/linux/mdio.h>
 #include <linux/bitfield.h>
+#include <linux/list.h>
 
 struct gpio_desc;
 struct mii_bus;
@@ -32,6 +33,9 @@ struct mdio_device {
 	int (*bus_match)(struct device *dev, const struct device_driver *drv);
 	void (*device_free)(struct mdio_device *mdiodev);
 	void (*device_remove)(struct mdio_device *mdiodev);
+	int (*device_remove_dynamic)(struct mdio_device *mdiodev);
+	/* Entry in mii_bus::mdio_map_retired. */
+	struct list_head retired_node;
 
 	/* Bus address of the MDIO device (0-31) */
 	int addr;
@@ -694,6 +698,8 @@ static inline int mdiodev_c45_write(struct mdio_device *mdiodev, u32 devad,
 
 bool mdiobus_is_registered_device(struct mii_bus *bus, int addr);
 struct phy_device *mdiobus_get_phy(struct mii_bus *bus, int addr);
+int mdiobus_device_change_begin(struct mii_bus *bus, bool removing);
+void mdiobus_device_change_end(struct mii_bus *bus, bool removing);
 
 /**
  * mdio_module_driver() - Helper macro for registering mdio drivers
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5f8d65868e0f..0500f80e0fac 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -22,6 +22,7 @@
 #include <linux/mii_timestamper.h>
 #include <linux/module.h>
 #include <linux/timer.h>
+#include <linux/wait.h>
 #include <linux/workqueue.h>
 #include <linux/device-id/mdio.h>
 #include <linux/u64_stats_sync.h>
@@ -391,7 +392,9 @@ struct mii_bus {
 	/** @state: State of bus structure */
 	enum {
 		MDIOBUS_ALLOCATED = 1,
+		MDIOBUS_REGISTERING,
 		MDIOBUS_REGISTERED,
+		MDIOBUS_UNREGISTERING,
 		MDIOBUS_UNREGISTERED,
 		MDIOBUS_RELEASED,
 	} state;
@@ -401,6 +404,18 @@ struct mii_bus {
 
 	/** @mdio_map: list of all MDIO devices on bus */
 	struct mdio_device *mdio_map[PHY_MAX_ADDR];
+	/** @mdio_map_pending: addresses with registration in progress */
+	u32 mdio_map_pending;
+	/** @mdio_map_lock: protects the MDIO device map and bus state */
+	struct mutex mdio_map_lock;
+	/** @mdio_map_wait: wait for active map operations during teardown */
+	wait_queue_head_t mdio_map_wait;
+	/** @mdio_map_ops: active registrations and firmware changes */
+	unsigned int mdio_map_ops;
+	/** @mdio_map_removals: firmware removals blocking PHY attachment */
+	unsigned int mdio_map_removals;
+	/** @mdio_map_retired: removed devices pinned until bus teardown */
+	struct list_head mdio_map_retired;
 
 	/** @phy_mask: PHY addresses to be ignored when probing */
 	u32 phy_mask;
@@ -652,6 +667,8 @@ struct phy_oatc14_sqi_capability {
  * @n_ports: Number of ports currently attached to the PHY
  * @max_n_ports: Max number of ports this PHY can expose
  * @lock:  Mutex for serialization access to PHY
+ * @attached: Whether a network device or standalone user attached the PHY;
+ *	protected by the MDIO bus map lock
  * @state_queue: Work queue for state machine
  * @link_down_events: Number of times link was lost
  * @shared: Pointer to private data shared by phys in one package
@@ -781,6 +798,7 @@ struct phy_device {
 	struct delayed_work state_queue;
 
 	struct mutex lock;
+	bool attached;
 
 	/* This may be modified under the rtnl lock */
 	bool sfp_bus_attached;

-- 
2.53.0

[PATCH net-next v2 3/3] net: mdio: support dynamic OF device changes

[James Hilliard]

MDIO buses enumerate firmware children only when the bus is registered.
A later DT overlay or status transition is ignored, unlike I2C and SPI.

Add an OF reconfiguration notifier. On addition, create fixed-address
PHYs and generic MDIO devices, scan free addresses for legacy PHY nodes
without reg, and recurse into Ethernet PHY packages. Track populated
nodes so static enumeration, address scanning and notifier-driven
enumeration cannot create duplicates. Walk every package child and report
the first population error without undoing devices which may already have
acquired consumers.

Serialize static and notifier-driven population while allowing a PHY
probe to enable another node on the same bus. Check the node's current
state after taking that lock so an older notification cannot override a
newer status change.

On removal, reject attached or in-flight PHYs and unregister the object
found by its OF node. This also covers PHYs found by address scanning.
Treat all PHYs in a package as one removal scope, so removing one member
cannot invalidate state used by an attached peer. Preflight overlay
removal while it can still be refused; normal changeset errors are
propagated so the caller can restore the previous state.

Hold a core MDIO change transaction across discovery and registration so
bus teardown cannot race the MDIO reads used to identify a PHY. Compile
the notifier only with CONFIG_OF_DYNAMIC and the overlay preflight only
with CONFIG_OF_OVERLAY.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes v1 -> v2:
  - split from the ACx00 series
  - move the PHY declaration to function scope for netdev style
---
 drivers/net/mdio/of_mdio.c | 448 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 438 insertions(+), 10 deletions(-)

diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index 051e449bbe7c..c89f88aacd8d 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -13,6 +13,7 @@
 #include <linux/fwnode_mdio.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/netdevice.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
@@ -20,6 +21,8 @@
 #include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
+#include <linux/sched.h>
+#include <linux/string.h>
 
 #define DEFAULT_GPIO_RESET_DELAY	10	/* in microseconds */
 
@@ -27,6 +30,52 @@ MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("OpenFirmware MDIO bus (Ethernet PHY) accessors");
 
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
+/*
+ * OF changes can nest when probing one MDIO device enables another node on
+ * the same bus. Serialize independent changes while allowing that nesting.
+ */
+static DEFINE_MUTEX(of_mdio_reconfig_mutex);
+static struct task_struct *of_mdio_reconfig_owner;
+static unsigned int of_mdio_reconfig_depth;
+
+static void of_mdio_reconfig_lock(void)
+{
+	if (!mutex_trylock(&of_mdio_reconfig_mutex)) {
+		if (of_mdio_reconfig_owner == current) {
+			of_mdio_reconfig_depth++;
+			return;
+		}
+		mutex_lock(&of_mdio_reconfig_mutex);
+	}
+
+	WARN_ON_ONCE(of_mdio_reconfig_owner);
+	WARN_ON_ONCE(of_mdio_reconfig_depth);
+	of_mdio_reconfig_owner = current;
+	of_mdio_reconfig_depth = 1;
+}
+
+static void of_mdio_reconfig_unlock(void)
+{
+	WARN_ON_ONCE(of_mdio_reconfig_owner != current);
+	WARN_ON_ONCE(!of_mdio_reconfig_depth);
+
+	if (--of_mdio_reconfig_depth)
+		return;
+
+	of_mdio_reconfig_owner = NULL;
+	mutex_unlock(&of_mdio_reconfig_mutex);
+}
+#else
+static inline void of_mdio_reconfig_lock(void)
+{
+}
+
+static inline void of_mdio_reconfig_unlock(void)
+{
+}
+#endif
+
 /* Extract the clause 22 phy ID from the compatible string of the form
  * ethernet-phy-idAAAA.BBBB */
 static int of_get_phy_id(struct device_node *device, u32 *phy_id)
@@ -44,7 +93,7 @@ int of_mdiobus_phy_device_register(struct mii_bus *mdio, struct phy_device *phy,
 EXPORT_SYMBOL(of_mdiobus_phy_device_register);
 
 static int of_mdiobus_register_phy(struct mii_bus *mdio,
-				    struct device_node *child, u32 addr)
+				   struct device_node *child, u32 addr)
 {
 	return fwnode_mdiobus_register_phy(mdio, of_fwnode_handle(child), addr);
 }
@@ -80,10 +129,20 @@ static int of_mdiobus_register_device(struct mii_bus *mdio,
 static int of_mdiobus_register_child(struct mii_bus *mdio,
 				     struct device_node *child, u32 addr)
 {
+	int rc;
+
+	if (of_node_test_and_set_flag(child, OF_POPULATED))
+		return 0;
+
 	if (of_mdiobus_child_is_phy(child))
-		return of_mdiobus_register_phy(mdio, child, addr);
+		rc = of_mdiobus_register_phy(mdio, child, addr);
+	else
+		rc = of_mdiobus_register_device(mdio, child, addr);
 
-	return of_mdiobus_register_device(mdio, child, addr);
+	if (rc)
+		of_node_clear_flag(child, OF_POPULATED);
+
+	return rc;
 }
 
 /* The following is a list of PHY compatible strings which appear in
@@ -255,13 +314,21 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
 	if (rc)
 		return rc;
 
+	of_mdio_reconfig_lock();
+	rc = mdiobus_device_change_begin(mdio, false);
+	if (rc) {
+		of_mdio_reconfig_unlock();
+		mdiobus_unregister(mdio);
+		return rc;
+	}
+
 	/* Loop over the child nodes and register a phy_device for each phy */
 	rc = __of_mdiobus_parse_phys(mdio, np, &scanphys);
 	if (rc)
-		goto unregister;
+		goto out_change;
 
 	if (!scanphys)
-		return 0;
+		goto out_change;
 
 	/* auto scan for PHYs with empty reg property */
 	for_each_available_child_of_node(np, child) {
@@ -272,19 +339,380 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
 
 		rc = of_mdiobus_scan_phy(mdio, child);
 		if (rc && rc != -ENODEV)
-			goto put_unregister;
+			goto put_child;
+		rc = 0;
 	}
 
-	return 0;
+out_change:
+	mdiobus_device_change_end(mdio, false);
+	of_mdio_reconfig_unlock();
+	if (!rc)
+		return 0;
 
-put_unregister:
-	of_node_put(child);
-unregister:
 	mdiobus_unregister(mdio);
 	return rc;
+
+put_child:
+	of_node_put(child);
+	goto out_change;
 }
 EXPORT_SYMBOL(__of_mdiobus_register);
 
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
+static bool of_mdiobus_node_is_available(struct device_node *node)
+{
+	return !of_node_check_flag(node, OF_DETACHED) &&
+	       of_device_is_available(node);
+}
+
+static int of_mdiobus_add_node(struct mii_bus *mdio,
+			       struct device_node *node)
+{
+	struct device_node *child;
+	int addr, rc, ret = 0;
+
+	if (!of_mdiobus_node_is_available(node))
+		return 0;
+
+	if (of_node_name_eq(node, "ethernet-phy-package")) {
+		if (!of_property_present(node, "reg"))
+			return 0;
+
+		for_each_available_child_of_node(node, child) {
+			rc = of_mdiobus_add_node(mdio, child);
+			if (rc && rc != -ENODEV && !ret)
+				ret = rc;
+		}
+
+		return ret;
+	}
+
+	addr = of_mdio_parse_addr(&mdio->dev, node);
+	if (addr < 0)
+		rc = of_mdiobus_scan_phy(mdio, node);
+	else
+		rc = of_mdiobus_register_child(mdio, node, addr);
+
+	if (rc == -ENODEV && addr >= 0)
+		dev_err(&mdio->dev,
+			"MDIO device at address %d is missing.\n", addr);
+
+	return rc;
+}
+
+static bool of_mdiobus_node_is_busy(struct mii_bus *mdio,
+				    struct device_node *node)
+{
+	struct mdio_device *mdiodev;
+	struct phy_device *phydev;
+	bool busy = false;
+
+	if (of_node_name_eq(node, "ethernet-phy-package")) {
+		for_each_child_of_node_scoped(node, child) {
+			if (of_mdiobus_node_is_busy(mdio, child))
+				return true;
+		}
+
+		return false;
+	}
+
+	if (!of_node_check_flag(node, OF_POPULATED))
+		return false;
+
+	mdiodev = of_mdio_find_device(node);
+	if (!mdiodev)
+		return true;
+	if (mdiodev->bus != mdio) {
+		put_device(&mdiodev->dev);
+		return false;
+	}
+
+	mutex_lock(&mdio->mdio_map_lock);
+	busy = mdio->mdio_map[mdiodev->addr] != mdiodev ||
+	       (mdio->mdio_map_pending & BIT(mdiodev->addr));
+	if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY) {
+		phydev = to_phy_device(&mdiodev->dev);
+		if (phydev->attached) {
+			busy = true;
+			dev_warn(&mdiodev->dev,
+				 "cannot remove an attached PHY; remove its consumer first\n");
+		}
+	}
+	mutex_unlock(&mdio->mdio_map_lock);
+	put_device(&mdiodev->dev);
+
+	return busy;
+}
+
+static struct device_node *
+of_mdiobus_get_removal_scope(struct device_node *node)
+{
+	struct device_node *parent;
+
+	if (of_node_name_eq(node, "ethernet-phy-package"))
+		return of_node_get(node);
+
+	parent = of_get_parent(node);
+	if (of_node_name_eq(parent, "ethernet-phy-package"))
+		return parent;
+
+	of_node_put(parent);
+	return of_node_get(node);
+}
+
+static int of_mdiobus_remove_node(struct mii_bus *mdio,
+				  struct device_node *node)
+{
+	struct mdio_device *mdiodev;
+	struct device_node *child;
+	int ret;
+
+	if (of_node_name_eq(node, "ethernet-phy-package")) {
+		for_each_child_of_node(node, child) {
+			ret = of_mdiobus_remove_node(mdio, child);
+			if (ret) {
+				of_node_put(child);
+				return ret;
+			}
+		}
+		return 0;
+	}
+
+	if (!of_node_check_flag(node, OF_POPULATED))
+		return 0;
+
+	/* The OF node lookup also covers PHYs found by address scanning. */
+	mdiodev = of_mdio_find_device(node);
+	if (!mdiodev)
+		return -EBUSY;
+	if (mdiodev->bus != mdio) {
+		put_device(&mdiodev->dev);
+		return -ENODEV;
+	}
+
+	ret = mdiodev->device_remove_dynamic(mdiodev);
+	if (!ret)
+		mdiodev->device_free(mdiodev);
+	put_device(&mdiodev->dev);
+
+	return ret == -ENODEV ? 0 : ret;
+}
+
+static struct mii_bus *of_mdiobus_find_parent(struct device_node *node)
+{
+	struct device_node *parent, *bus_node;
+	struct mii_bus *mdio;
+
+	parent = of_get_parent(node);
+	if (!parent)
+		return NULL;
+
+	if (of_node_name_eq(parent, "ethernet-phy-package")) {
+		if (!of_device_is_available(parent) ||
+		    !of_property_present(parent, "reg")) {
+			of_node_put(parent);
+			return NULL;
+		}
+
+		bus_node = of_get_parent(parent);
+		of_node_put(parent);
+	} else {
+		bus_node = parent;
+	}
+
+	mdio = of_mdio_find_bus(bus_node);
+	of_node_put(bus_node);
+
+	return mdio;
+}
+
+#if IS_ENABLED(CONFIG_OF_OVERLAY)
+/* Overlay entry notifier errors cannot stop removal after the tree changed. */
+static bool of_mdiobus_live_node_is_busy(struct device_node *node)
+{
+	struct device_node *scope;
+	struct mii_bus *mdio;
+	bool busy = true;
+
+	scope = of_mdiobus_get_removal_scope(node);
+	mdio = of_mdiobus_find_parent(scope);
+	if (!mdio)
+		goto out_put_scope;
+
+	if (!mdiobus_device_change_begin(mdio, true)) {
+		busy = of_mdiobus_node_is_busy(mdio, scope);
+		mdiobus_device_change_end(mdio, true);
+	}
+	put_device(&mdio->dev);
+
+out_put_scope:
+	of_node_put(scope);
+	return busy;
+}
+
+static struct device_node *
+of_mdiobus_overlay_target_child(struct device_node *target,
+				struct device_node *overlay_child)
+{
+	const char *name = kbasename(overlay_child->full_name);
+	struct device_node *child;
+
+	for_each_child_of_node(target, child) {
+		if (!of_node_cmp(kbasename(child->full_name), name))
+			return child;
+	}
+
+	return NULL;
+}
+
+static bool of_mdiobus_overlay_node_is_busy(struct device_node *overlay,
+					    struct device_node *target,
+					    bool added)
+{
+	struct device_node *overlay_child, *target_child;
+	bool busy, child_added;
+
+	if ((added || of_property_present(overlay, "status")) &&
+	    of_mdiobus_live_node_is_busy(target))
+		return true;
+
+	for_each_child_of_node(overlay, overlay_child) {
+		target_child = of_mdiobus_overlay_target_child(target,
+							       overlay_child);
+		if (!target_child)
+			continue;
+
+		child_added = of_node_check_flag(target_child, OF_OVERLAY);
+		busy = of_mdiobus_overlay_node_is_busy(overlay_child,
+						       target_child, child_added);
+		of_node_put(target_child);
+		if (busy) {
+			of_node_put(overlay_child);
+			return true;
+		}
+	}
+
+	return false;
+}
+
+static int of_mdiobus_overlay_notify(struct notifier_block *nb,
+				     unsigned long action, void *arg)
+{
+	struct of_overlay_notify_data *nd = arg;
+	bool busy;
+
+	if (action != OF_OVERLAY_PRE_REMOVE)
+		return NOTIFY_OK;
+
+	of_mdio_reconfig_lock();
+	busy = of_mdiobus_overlay_node_is_busy(nd->overlay, nd->target,
+					       false);
+	of_mdio_reconfig_unlock();
+
+	return busy ? notifier_from_errno(-EBUSY) : NOTIFY_OK;
+}
+
+static struct notifier_block of_mdio_overlay_notifier = {
+	.notifier_call = of_mdiobus_overlay_notify,
+};
+#endif
+
+static int of_mdiobus_notify(struct notifier_block *nb, unsigned long action,
+			     void *arg)
+{
+	struct of_reconfig_data *rd = arg;
+	struct device_node *scope;
+	struct mii_bus *mdio;
+	enum of_reconfig_change change;
+	bool removing;
+	int rc, ret = NOTIFY_OK;
+
+	of_mdio_reconfig_lock();
+	change = of_reconfig_get_state_change(action, rd);
+	switch (change) {
+	case OF_RECONFIG_CHANGE_ADD:
+		/* A newer change may have made this notification stale. */
+		if (!of_mdiobus_node_is_available(rd->dn))
+			goto out_unlock;
+		removing = false;
+		break;
+	case OF_RECONFIG_CHANGE_REMOVE:
+		/* A newer change may have made this notification stale. */
+		if (of_mdiobus_node_is_available(rd->dn))
+			goto out_unlock;
+		removing = true;
+		break;
+	default:
+		goto out_unlock;
+	}
+
+	mdio = of_mdiobus_find_parent(rd->dn);
+	if (!mdio)
+		goto out_unlock;
+
+	rc = mdiobus_device_change_begin(mdio, removing);
+	if (rc) {
+		if (!removing)
+			ret = notifier_from_errno(-EPROBE_DEFER);
+		goto out_put_mdio;
+	}
+
+	if (!removing) {
+		rc = of_mdiobus_add_node(mdio, rd->dn);
+	} else {
+		/* The node may already be detached from its parent hierarchy. */
+		scope = of_mdiobus_get_removal_scope(rd->dn);
+		if (of_mdiobus_node_is_busy(mdio, scope))
+			rc = -EBUSY;
+		else
+			rc = of_mdiobus_remove_node(mdio, rd->dn);
+		of_node_put(scope);
+	}
+
+	mdiobus_device_change_end(mdio, removing);
+	if (rc && (removing || rc != -ENODEV))
+		ret = notifier_from_errno(rc);
+
+out_put_mdio:
+	put_device(&mdio->dev);
+out_unlock:
+	of_mdio_reconfig_unlock();
+
+	return ret;
+}
+
+static struct notifier_block of_mdio_notifier = {
+	.notifier_call = of_mdiobus_notify,
+};
+
+static int __init of_mdio_init(void)
+{
+	int ret;
+
+	ret = of_reconfig_notifier_register(&of_mdio_notifier);
+	if (ret)
+		return ret;
+
+#if IS_ENABLED(CONFIG_OF_OVERLAY)
+	ret = of_overlay_notifier_register(&of_mdio_overlay_notifier);
+	if (ret)
+		of_reconfig_notifier_unregister(&of_mdio_notifier);
+#endif
+
+	return ret;
+}
+module_init(of_mdio_init);
+
+static void __exit of_mdio_exit(void)
+{
+#if IS_ENABLED(CONFIG_OF_OVERLAY)
+	of_overlay_notifier_unregister(&of_mdio_overlay_notifier);
+#endif
+	of_reconfig_notifier_unregister(&of_mdio_notifier);
+}
+module_exit(of_mdio_exit);
+#endif /* CONFIG_OF_DYNAMIC */
+
 /**
  * of_mdio_find_device - Given a device tree node, find the mdio_device
  * @np: pointer to the mdio_device's device tree node

-- 
2.53.0

Re: [PATCH net-next v2 0/3] net: mdio: support dynamic OF device changes

[Andrew Lunn]

On Mon, Aug 03, 2026 at 05:48:09PM -0600, James Hilliard wrote:
> MDIO buses enumerate firmware children only when the bus is registered. A
> later devicetree overlay or status transition is ignored, unlike on I2C and
> SPI buses. Supporting live reconfiguration also means that population and
> removal can race address scanning, PHY attachment and bus teardown.
> 
> Patch 1 factors fixed-address child registration and legacy PHY scanning
> into helpers shared by initial and dynamic population.
> 
> Patch 2 serializes MDIO device-map changes, reserves addresses while
> registration is in progress, and coordinates scans, attachment, removal
> and bus teardown. Dynamically removed devices remain pinned until bus
> teardown, preserving the borrowed-pointer convention of
> mdiobus_get_phy().
> 
> Patch 3 adds the OF reconfiguration notifier. It supports fixed-address
> PHYs and generic MDIO devices, scanned PHY addresses, and Ethernet PHY
> packages. It also preflights overlay removal and rejects removal of
> attached or in-flight PHYs before their firmware nodes can disappear.

This is a lot of complexity which i think should be avoided by just
getting the bootloader to put the correct nodes in DT.

If this was a hat on top of a SBC, with an EEPROM indicating what the
hat was, then maybe DT overlays would make sense. But from what you
have said, this is a fixed PCB design, nothing hot/cold plugable.

     Andrew

Re: [PATCH net-next v2 0/3] net: mdio: support dynamic OF device changes

[James Hilliard]

On Mon, Aug 3, 2026 at 8:06 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Aug 03, 2026 at 05:48:09PM -0600, James Hilliard wrote:
> > MDIO buses enumerate firmware children only when the bus is registered. A
> > later devicetree overlay or status transition is ignored, unlike on I2C and
> > SPI buses. Supporting live reconfiguration also means that population and
> > removal can race address scanning, PHY attachment and bus teardown.
> >
> > Patch 1 factors fixed-address child registration and legacy PHY scanning
> > into helpers shared by initial and dynamic population.
> >
> > Patch 2 serializes MDIO device-map changes, reserves addresses while
> > registration is in progress, and coordinates scans, attachment, removal
> > and bus teardown. Dynamically removed devices remain pinned until bus
> > teardown, preserving the borrowed-pointer convention of
> > mdiobus_get_phy().
> >
> > Patch 3 adds the OF reconfiguration notifier. It supports fixed-address
> > PHYs and generic MDIO devices, scanned PHY addresses, and Ethernet PHY
> > packages. It also preflights overlay removal and rejects removal of
> > attached or in-flight PHYs before their firmware nodes can disappear.
>
> This is a lot of complexity which i think should be avoided by just
> getting the bootloader to put the correct nodes in DT.

This should all be generic OF_DYNAMIC support for MDIO, so the
changes shouldn't really be tied to a specific PHY driver in any case.

> If this was a hat on top of a SBC, with an EEPROM indicating what the
> hat was, then maybe DT overlays would make sense. But from what you
> have said, this is a fixed PCB design, nothing hot/cold plugable.

Some H616/H618 boards have the ethernet port on an expansion board:
http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/2W-expansion-board.html

>
>      Andrew
>

Re: [PATCH net-next v2 0/3] net: mdio: support dynamic OF device changes

[Andrew Lunn]

> > If this was a hat on top of a SBC, with an EEPROM indicating what the
> > hat was, then maybe DT overlays would make sense. But from what you
> > have said, this is a fixed PCB design, nothing hot/cold plugable.
> 
> Some H616/H618 boards have the ethernet port on an expansion board:
> http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/2W-expansion-board.html

I don't see a list of pins which go across the connector. Does the
MDIO bus go to the expansion board? Is the PHY on the expansion
board. It all looks passive to me.

	Andrew

Re: [PATCH net-next v2 0/3] net: mdio: support dynamic OF device changes

[James Hilliard]

On Tue, Aug 4, 2026 at 6:54 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > If this was a hat on top of a SBC, with an EEPROM indicating what the
> > > hat was, then maybe DT overlays would make sense. But from what you
> > > have said, this is a fixed PCB design, nothing hot/cold plugable.
> >
> > Some H616/H618 boards have the ethernet port on an expansion board:
> > http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/2W-expansion-board.html
>
> I don't see a list of pins which go across the connector. Does the
> MDIO bus go to the expansion board? Is the PHY on the expansion
> board. It all looks passive to me.

Pins are listed under "24Pin expansion board interface pin description":
https://orangepi.net/wp-content/uploads/2023/10/OrangePi_Zero2w_H618_User-Manual_v1.1.pdf

I think it's just the connector part that's on the expansion board?

>
>         Andrew

Re: [PATCH net-next v2 0/3] net: mdio: support dynamic OF device changes

[Andrew Lunn]

On Tue, Aug 04, 2026 at 09:10:30AM -0600, James Hilliard wrote:
> On Tue, Aug 4, 2026 at 6:54 AM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > > If this was a hat on top of a SBC, with an EEPROM indicating what the
> > > > hat was, then maybe DT overlays would make sense. But from what you
> > > > have said, this is a fixed PCB design, nothing hot/cold plugable.
> > >
> > > Some H616/H618 boards have the ethernet port on an expansion board:
> > > http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/2W-expansion-board.html
> >
> > I don't see a list of pins which go across the connector. Does the
> > MDIO bus go to the expansion board? Is the PHY on the expansion
> > board. It all looks passive to me.
> 
> Pins are listed under "24Pin expansion board interface pin description":
> https://orangepi.net/wp-content/uploads/2023/10/OrangePi_Zero2w_H618_User-Manual_v1.1.pdf

So just the lines after the PHY, basically the UTP lines without the
twist.

So please stop wasting my time by making invalid points.

   Andrew

Re: [PATCH net-next v2 0/3] net: mdio: support dynamic OF device changes

[James Hilliard]

On Tue, Aug 4, 2026 at 12:57 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Tue, Aug 04, 2026 at 09:10:30AM -0600, James Hilliard wrote:
> > On Tue, Aug 4, 2026 at 6:54 AM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > > If this was a hat on top of a SBC, with an EEPROM indicating what the
> > > > > hat was, then maybe DT overlays would make sense. But from what you
> > > > > have said, this is a fixed PCB design, nothing hot/cold plugable.
> > > >
> > > > Some H616/H618 boards have the ethernet port on an expansion board:
> > > > http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/2W-expansion-board.html
> > >
> > > I don't see a list of pins which go across the connector. Does the
> > > MDIO bus go to the expansion board? Is the PHY on the expansion
> > > board. It all looks passive to me.
> >
> > Pins are listed under "24Pin expansion board interface pin description":
> > https://orangepi.net/wp-content/uploads/2023/10/OrangePi_Zero2w_H618_User-Manual_v1.1.pdf
>
> So just the lines after the PHY, basically the UTP lines without the
> twist.
>
> So please stop wasting my time by making invalid points.

I'm confused how this is an invalid point.

Presumably one may want to use OF_DYNAMIC to dynamically
enable the PHY only when the expansion board is installed.

Whether or not the MDIO bus is wired to the expansion board or
physically integrated into the SoC doesn't seem like it would make
a difference in regards to whether or not one would want to dynamically
enable the PHY using OF_DYNAMIC as that decision would presumably
be made based on the actual configured hardware setup. If the PHY is
not physically connected to an ethernet port then presumably it should
remain disabled due to that being a non-functional hardware configuration.

Or am I missing something here?

>
>    Andrew

Re: [PATCH net-next v2 2/3] net: mdio: make device map changes hotplug-safe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] `mdiobus_remove_devices()` can race with concurrent non-dynamic device removals, leading to a double free of the device reference count.
- [Medium] The dynamic removal safety mechanisms introduced in the patch are dead code because `device_remove_dynamic` is never invoked.

Pre-existing issues:
- [High] Accessing `bus->owner` in `phy_detach()` and the error path of `phy_attach_direct()` after potentially freeing the `bus`.
--

--- Patch [2]: [PATCH net-next v2 2/3] net: mdio: make device map changes hotplug-safe ---
Note: The format of this review has been altered to a free-form summary due
to recitation restrictions. A standard inline diff has been omitted.

commit 22975fc157973334c571ff5ec87c6baf39897f09
Author: James Hilliard <james.hilliard1@gmail.com>
Subject: net: mdio: make device map changes hotplug-safe

This commit adds serialization and safety mechanisms to MDIO device
registration and removal. It aims to protect the device map from concurrent
dynamic firmware changes and bus teardowns by pinning dynamically removed
devices.

[Severity: High]
Does mdiobus_remove_devices() have a race condition with concurrent
non-dynamic device removals?

Looking at mdiobus_remove_devices() in drivers/net/phy/mdio_bus_provider.c,
the loop reads the device pointer from the map and subsequently calls
device_free() without taking a reference or checking for a concurrent
removal's success.

If a concurrent thread performs a non-dynamic removal (like an SFP unplug)
calling phy_device_remove() on the same device, it could successfully
unpublish the device and call phy_device_free().

Then mdiobus_remove_devices() continues and calls
mdiodev->device_remove(mdiodev). This fails internally with -ENODEV in
mdiobus_begin_remove(). However, __phy_device_remove() and device_remove()
swallow this error and return early without indicating failure to the void
returning caller.

Because of this, mdiobus_remove_devices() unconditionally proceeds to call
mdiodev->device_free(mdiodev), resulting in a double free of the device
reference count.

[Severity: Medium]
Are the dynamic removal protections reachable in practice?

The patch introduces mdio_device_remove_dynamic() and
phy_mdio_device_remove_dynamic() intended to pass dynamic = true to the
removal routines.

However, the function pointer device_remove_dynamic declared in
include/linux/mdio.h is never invoked anywhere. The standard APIs continue
to call phy_device_remove() in drivers/net/phy/phy_device.c, which hardcodes
__phy_device_remove(phydev, false). This appears to leave the dynamic
removal safety mechanisms inactive.

[Severity: High]
This is a pre-existing issue, but can accessing bus->owner cause a
use-after-free?

In phy_detach() in drivers/net/phy/phy_device.c, the code calls
put_device(&phydev->mdio.dev) which drops the network driver's reference to
the PHY device.

If the MDIO bus is a separate device that was already unregistered and freed
by its driver, dropping the child PHY device removes the last reference to
the parent bus kobject, causing the bus structure to be freed immediately.

The very next line checks if (ndev_owner != bus->owner), reading from
potentially freed memory.

The same sequence exists in the error_put_device path of
phy_attach_direct(), which calls put_device(d) followed by checking
bus->owner.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-submit-mdio-of-dynamic-v2-v2-0-f8841f3124d7@gmail.com?part=2

Re: [PATCH net-next v2 3/3] net: mdio: support dynamic OF device changes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The global OF overlay notifier inadvertently blocks the removal of any overlay containing non-MDIO nodes, and any overlay whose underlying MDIO bus is unregistered.
- [Low] Data race on `of_mdio_reconfig_owner` in the recursive lock implementation.
--

--- Patch [3]: [PATCH net-next v2 3/3] net: mdio: support dynamic OF device changes ---
commit c037ead180706f6895d9e354b94ae56ca7628104
Author: James Hilliard <james.hilliard1@gmail.com>

net: mdio: support dynamic OF device changes

This commit adds dynamic OF device support for MDIO buses. It enumerates PHYs
on OF reconfiguration addition events and handles removals by unregistering
devices attached to removed nodes. It also introduces serialization and overlay
preflight checks.

> diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
> index 051e449bbe7c0..c89f88aacd8df 100644
> --- a/drivers/net/mdio/of_mdio.c
> +++ b/drivers/net/mdio/of_mdio.c
[ ... ]
> @@ -27,6 +30,52 @@ MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("OpenFirmware MDIO bus (Ethernet PHY) accessors");
>  
> +#if IS_ENABLED(CONFIG_OF_DYNAMIC)
> +/*
> + * OF changes can nest when probing one MDIO device enables another node on
> + * the same bus. Serialize independent changes while allowing that nesting.
> + */
> +static DEFINE_MUTEX(of_mdio_reconfig_mutex);
> +static struct task_struct *of_mdio_reconfig_owner;
> +static unsigned int of_mdio_reconfig_depth;
> +
> +static void of_mdio_reconfig_lock(void)
> +{
> +	if (!mutex_trylock(&of_mdio_reconfig_mutex)) {
> +		if (of_mdio_reconfig_owner == current) {

[Severity: Low]
Since `of_mdio_reconfig_owner` can be updated to NULL in
`of_mdio_reconfig_unlock()` concurrently, does this lockless read trigger a
KCSAN data race?

Would it be safer to use `READ_ONCE()` here and `WRITE_ONCE()` in the unlock
path to satisfy concurrency rules?

> +			of_mdio_reconfig_depth++;
> +			return;
> +		}
> +		mutex_lock(&of_mdio_reconfig_mutex);
> +	}
> +
> +	WARN_ON_ONCE(of_mdio_reconfig_owner);
> +	WARN_ON_ONCE(of_mdio_reconfig_depth);
> +	of_mdio_reconfig_owner = current;
> +	of_mdio_reconfig_depth = 1;
> +}
> +
> +static void of_mdio_reconfig_unlock(void)
> +{
> +	WARN_ON_ONCE(of_mdio_reconfig_owner != current);
> +	WARN_ON_ONCE(!of_mdio_reconfig_depth);
> +
> +	if (--of_mdio_reconfig_depth)
> +		return;
> +
> +	of_mdio_reconfig_owner = NULL;
> +	mutex_unlock(&of_mdio_reconfig_mutex);
> +}
[ ... ]
> +/* Overlay entry notifier errors cannot stop removal after the tree changed. */
> +static bool of_mdiobus_live_node_is_busy(struct device_node *node)
> +{
> +	struct device_node *scope;
> +	struct mii_bus *mdio;
> +	bool busy = true;
> +
> +	scope = of_mdiobus_get_removal_scope(node);
> +	mdio = of_mdiobus_find_parent(scope);
> +	if (!mdio)
> +		goto out_put_scope;

[Severity: High]
If a removed overlay contains non-MDIO nodes (for example, I2C devices),
`of_mdiobus_find_parent()` will return NULL.

Since `busy` is initialized to `true`, doesn't jumping to `out_put_scope`
return `true` in this case?

Because this overlay notifier is registered globally, does this inadvertently
block the removal of any overlay containing non-MDIO nodes system-wide by
falsely claiming the non-MDIO node is a busy MDIO device?

> +
> +	if (!mdiobus_device_change_begin(mdio, true)) {
> +		busy = of_mdiobus_node_is_busy(mdio, scope);
> +		mdiobus_device_change_end(mdio, true);
> +	}
> +	put_device(&mdio->dev);
> +
> +out_put_scope:
> +	of_node_put(scope);
> +	return busy;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-submit-mdio-of-dynamic-v2-v2-0-f8841f3124d7@gmail.com?part=3