[PATCH net v1] net: phy: Change flag to autoremove the consumer

[PATCH net v1] net: phy: Change flag to autoremove the consumer

[Sarosh Hasan]

phy_detach() is not called when the MDIO controller driver is
removed. So phydev->devlink is not cleared, but actually the device
link has been removed by phy_device_remove()--> device_del().Therefore,
it will cause the crash when the MAC controller driver is removed.
In such case delete link between phy dev and mac dev. Change the 
DL_FLAG_STATELESS flag to DL_FLAG_AUTOREMOVE_SUPPLIER,so that the
consumer (MAC controller) driver will be automatically removed
when the link is removed.

Fixes: bc66fa87d4fd ("net: phy: Add link between phy dev and mac dev")
Link: https://lore.kernel.org/all/e6824f1a-c1a9-4c2e-9b46-6fe224877bfc@quicinc.com/
Suggested-by: Wei Fang <wei.fang@nxp.com>
Signed-off-by: Abhishek Chauhan <quic_abchauha@quicinc.com>
Signed-off-by: Sarosh Hasan <quic_sarohasa@quicinc.com>
---
 drivers/net/phy/phy_device.c | 17 +++++++++--------
 include/linux/phy.h          |  4 ----
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 90951681523c..f3db3dd93c74 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1630,6 +1630,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;
+	struct device_link *devlink;
 	int err;
 
 	/* For Ethernet device drivers that register their own MDIO bus, we
@@ -1760,9 +1761,14 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	 * another mac interface, so we should create a device link between
 	 * phy dev and mac dev.
 	 */
-	if (dev && phydev->mdio.bus->parent && dev->dev.parent != phydev->mdio.bus->parent)
-		phydev->devlink = device_link_add(dev->dev.parent, &phydev->mdio.dev,
-						  DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
+	if (dev && phydev->mdio.bus->parent && dev->dev.parent != phydev->mdio.bus->parent) {
+		devlink = device_link_add(dev->dev.parent, &phydev->mdio.dev,
+					  DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_SUPPLIER);
+		if (!devlink) {
+			err = -ENOMEM;
+			goto error;
+		}
+	}
 
 	return err;
 
@@ -1834,11 +1840,6 @@ void phy_detach(struct phy_device *phydev)
 	struct module *ndev_owner = NULL;
 	struct mii_bus *bus;
 
-	if (phydev->devlink) {
-		device_link_del(phydev->devlink);
-		phydev->devlink = NULL;
-	}
-
 	if (phydev->sysfs_links) {
 		if (dev)
 			sysfs_remove_link(&dev->dev.kobj, "phydev");
diff --git a/include/linux/phy.h b/include/linux/phy.h
index b037aab7b71d..e20643fb6f41 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -507,8 +507,6 @@ struct macsec_ops;
  *
  * @mdio: MDIO bus this PHY is on
  * @drv: Pointer to the driver for this PHY instance
- * @devlink: Create a link between phy dev and mac dev, if the external phy
- *           used by current mac interface is managed by another mac interface.
  * @phyindex: Unique id across the phy's parent tree of phys to address the PHY
  *	      from userspace, similar to ifindex. A zero index means the PHY
  *	      wasn't assigned an id yet.
@@ -613,8 +611,6 @@ struct phy_device {
 	/* And management functions */
 	const struct phy_driver *drv;
 
-	struct device_link *devlink;
-
 	u32 phyindex;
 	u32 phy_id;
 
-- 
2.17.1

Re: [PATCH net v1] net: phy: Change flag to autoremove the consumer

[Maxime Chevallier]

On Thu, 3 Jul 2025 14:30:41 +0530
Sarosh Hasan <quic_sarohasa@quicinc.com> wrote:

> phy_detach() is not called when the MDIO controller driver is
> removed. So phydev->devlink is not cleared, but actually the device
> link has been removed by phy_device_remove()--> device_del().Therefore,
> it will cause the crash when the MAC controller driver is removed.
> In such case delete link between phy dev and mac dev. Change the 
> DL_FLAG_STATELESS flag to DL_FLAG_AUTOREMOVE_SUPPLIER,so that the
> consumer (MAC controller) driver will be automatically removed
> when the link is removed.

This doesn't work unfortunately, PHY devices can be hot-swappable, e.g.
when the PHY is in an SFP module. In that case, we must not
automatically remove the MAC controller driver when the PHY goes away.

I gave this patch a quick test on a Macchiatobin, which has an SFP
module, and indeed when you unplug the module while the link is up, the
system hangs completely when running a command like "ip a" afterwards.

Maxime

RE: [PATCH net v1] net: phy: Change flag to autoremove the consumer

[Wei Fang]

> > phy_detach() is not called when the MDIO controller driver is
> > removed. So phydev->devlink is not cleared, but actually the device
> > link has been removed by phy_device_remove()--> device_del().Therefore,
> > it will cause the crash when the MAC controller driver is removed.
> > In such case delete link between phy dev and mac dev. Change the
> > DL_FLAG_STATELESS flag to DL_FLAG_AUTOREMOVE_SUPPLIER,so that the
> > consumer (MAC controller) driver will be automatically removed
> > when the link is removed.
> 
> This doesn't work unfortunately, PHY devices can be hot-swappable, e.g.
> when the PHY is in an SFP module. In that case, we must not
> automatically remove the MAC controller driver when the PHY goes away.
> 
> I gave this patch a quick test on a Macchiatobin, which has an SFP
> module, and indeed when you unplug the module while the link is up, the
> system hangs completely when running a command like "ip a" afterwards.
> 

Hi Sarosh,

Sorry, I didn't realize there are hot-swappable PHY devices. Given this
situation, there are two proposals.

Solution 1 is to set phydev->devlink to NULL after device_del() in
phy_device_remove(). But I think this solution is not perfect. For the use
case that multiple MAC controllers share the same MDIO bus, if the driver
of the main MAC controller (which has the shared MDIO bus) is removed,
other MAC controller drivers will not be able to use the MDIO bus, which
may introduce some problems. For example, the 'ifconfig ethx down/up'
command may cause a crash.

Solution 2 is to add a devlink between the MAC controller and the MDIO
controller instead of adding a devlink between the MAC controller and
the PHY device. This solution needs to be verified whether it will introduce
other problems.