[PATCH net] net: phy: add device link between MAC device and MDIO device

[PATCH net] net: phy: add device link between MAC device and MDIO device

[Wei Fang]

For the shared MDIO bus use case, multiple MACs will share the same MDIO
bus. Therefore, these MACs all depend on this MDIO bus, however, current
PHY driver does not create this dependency relationship between the MACs
and the shared MDIO devices. If this shared MDIO bus is removed, it could
potentially cause a crash. For example, the i.MX6ULL platform has two FEC
ports (fec1 and fec2) and uses phylib interfaces to manage the external
PHY. And fec1 uses the MDIO bus shared by fec2. When the fec2 driver is
removed, users can see the following warning log.

$ echo 20b4000.ethernet > /sys/bus/platform/drivers/fec/unbind
WARNING: drivers/net/phy/phy.c:1352 at _phy_state_machine+0xe8/0x38c, CPU#0: kworker/0:6/35
phy_check_link_status+0x0/0xf8: returned: -13
Call trace:
 unwind_backtrace from show_stack+0x10/0x14
 show_stack from dump_stack_lvl+0x54/0x68
 dump_stack_lvl from __warn+0x84/0xf4
 __warn from warn_slowpath_fmt+0x1a8/0x1bc
 warn_slowpath_fmt from _phy_state_machine+0xe8/0x38c
 _phy_state_machine from phy_state_machine+0x20/0x50
 phy_state_machine from process_one_work+0x168/0x2f4

Another example is the i.MX95-15x15 platform which has two ENETC ports
and uses the phylink interfaces to manage the exteranl PHY. All the
external PHYs can be accessed the EMDIO, it is a different device from
ENETC. When the EMDIO driver is removed, users can see the below crash
log and the console is hanged.

$ echo 0002:01:00.0 > /sys/bus/pci/drivers/fsl_enetc_mdio/unbind
$ ifconfig eth0 down
Unable to handle kernel NULL pointer dereference at virtual address 00000000000001b8
pc : _phy_state_machine+0x230/0x36c
lr : _phy_state_machine+0x2b4/0x36c
Call trace:
 _phy_state_machine+0x230/0x36c (P)
 phy_stop+0x74/0x190
 phylink_stop+0x28/0xb8
 enetc_close+0x28/0x8c
 netif_change_flags+0x24/0x6c
 dev_change_flags+0x48/0x7c
 devinet_ioctl+0x328/0x604
 inet_ioctl+0x204/0x220

The commit bc66fa87d4fd ("net: phy: Add link between phy dev and mac
dev") has created a device link between the MAC and the PHY if the MAC
uses a shared MDIO bus (The MDIO and the MAC are two separate devices).
Sarosh Hasan tried to change the DL_FLAG_STATELESS flag to
DL_FLAG_AUTOREMOVE_SUPPLIER to fix the issue [1]. However, the solution
does not take into account the hot-swappable PHY devices (such as SFP).
so when the PHY device is unplugged, the MAC driver will automatically
be removed, which is not the expected behavior.

Therefore, to solve this issue of the shared MDIO bus, we create the
device link between the MAC device and the MDIO device, rather than
between the MAC device and the PHY device. And when the shared MDIO bus
is removed, all MAC drivers that depend on it will also be removed.

Reported-by: Abhishek Chauhan (ABC) <quic_abchauha@quicinc.com>
Closes: https://lore.kernel.org/all/d696a426-40bb-4c1a-b42d-990fb690de5e@quicinc.com/
Link: https://lore.kernel.org/imx/20250703090041.23137-1-quic_sarohasa@quicinc.com/ # [1]
Fixes: bc66fa87d4fd ("net: phy: Add link between phy dev and mac dev")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/phy/phy_device.c | 24 ++++++++++++++----------
 include/linux/phy.h          |  4 ----
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 81984d4ebb7c..d5ac7506fe39 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1768,12 +1768,21 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 
 	/**
 	 * If the external phy used by current mac interface is managed by
-	 * another mac interface, so we should create a device link between
-	 * phy dev and mac dev.
+	 * another MDIO controller, which means that the MAC and MDIO are
+	 * separated devices, then we should create a device link between
+	 * the MAC device and the MDIO device.
 	 */
-	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) {
+		if (!device_link_add(dev->dev.parent, phydev->mdio.bus->parent,
+				     DL_FLAG_PM_RUNTIME |
+				     DL_FLAG_AUTOREMOVE_SUPPLIER)) {
+			phydev_err(phydev,
+				   "Failed to add devlink between MAC and MDIO\n");
+			err = -EINVAL;
+			goto error;
+		}
+	}
 
 	return err;
 
@@ -1845,11 +1854,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 fbbe028cc4b7..d6f2039a63d3 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -559,8 +559,6 @@ struct phy_oatc14_sqi_capability {
  *
  * @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.
@@ -666,8 +664,6 @@ struct phy_device {
 	/* And management functions */
 	const struct phy_driver *drv;
 
-	struct device_link *devlink;
-
 	u32 phyindex;
 	u32 phy_id;
 
-- 
2.34.1

Re: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Andrew Lunn]

> The commit bc66fa87d4fd ("net: phy: Add link between phy dev and mac
> dev") has created a device link between the MAC and the PHY if the MAC
> uses a shared MDIO bus (The MDIO and the MAC are two separate devices).

> Sarosh Hasan tried to change the DL_FLAG_STATELESS flag to
> DL_FLAG_AUTOREMOVE_SUPPLIER to fix the issue [1]. However, the solution
> does not take into account the hot-swappable PHY devices (such as SFP).
> so when the PHY device is unplugged, the MAC driver will automatically
> be removed, which is not the expected behavior.
> 
> Therefore, to solve this issue of the shared MDIO bus, we create the
> device link between the MAC device and the MDIO device, rather than
> between the MAC device and the PHY device. And when the shared MDIO bus
> is removed, all MAC drivers that depend on it will also be removed.

Shouldn't there be a full chain here? MAC->MDIO->PHY?

For the case of the SFP, maybe it should be

MAC->SFP Cage and I2C MDIO->PHY

So if the PHY inside an SFP disappears, the I2C MDIO bus is
removed. But there is no link upwards from that, so the SFP cage and
the MAC are left alone?

    Andrew

Re: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Russell King (Oracle)]

On Mon, Jan 26, 2026 at 03:13:26PM +0100, Andrew Lunn wrote:
> > The commit bc66fa87d4fd ("net: phy: Add link between phy dev and mac
> > dev") has created a device link between the MAC and the PHY if the MAC
> > uses a shared MDIO bus (The MDIO and the MAC are two separate devices).
> 
> > Sarosh Hasan tried to change the DL_FLAG_STATELESS flag to
> > DL_FLAG_AUTOREMOVE_SUPPLIER to fix the issue [1]. However, the solution
> > does not take into account the hot-swappable PHY devices (such as SFP).
> > so when the PHY device is unplugged, the MAC driver will automatically
> > be removed, which is not the expected behavior.
> > 
> > Therefore, to solve this issue of the shared MDIO bus, we create the
> > device link between the MAC device and the MDIO device, rather than
> > between the MAC device and the PHY device. And when the shared MDIO bus
> > is removed, all MAC drivers that depend on it will also be removed.
> 
> Shouldn't there be a full chain here? MAC->MDIO->PHY?
> 
> For the case of the SFP, maybe it should be
> 
> MAC->SFP Cage and I2C MDIO->PHY
> 
> So if the PHY inside an SFP disappears, the I2C MDIO bus is
> removed.

That's a crazy idea. The SFP cage driver handles the presence of the
I2C-based MDIO bus, and has to because the type of bus created depends
on the module, and the lifetime of the bus is determined by the time
that the module is plugged in, not whether there's a PHY present.

If removing a PHY on a MDIO bus results in the bus to which it was
connected to being removed, that's utterly stupid, because that
means all other PHYs will end up being removed when the bus goes away.

Consider a MDIO bus that has some independent PHYs and a DSA switch
connected. Do we really want removal of the PHY to destroy the MDIO
bus, and thus also take out the DSA switch?

-- 
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] net: phy: add device link between MAC device and MDIO device

[Wei Fang]

> > The commit bc66fa87d4fd ("net: phy: Add link between phy dev and mac
> > dev") has created a device link between the MAC and the PHY if the MAC
> > uses a shared MDIO bus (The MDIO and the MAC are two separate devices).
> 
> > Sarosh Hasan tried to change the DL_FLAG_STATELESS flag to
> > DL_FLAG_AUTOREMOVE_SUPPLIER to fix the issue [1]. However, the solution
> > does not take into account the hot-swappable PHY devices (such as SFP).
> > so when the PHY device is unplugged, the MAC driver will automatically
> > be removed, which is not the expected behavior.
> >
> > Therefore, to solve this issue of the shared MDIO bus, we create the
> > device link between the MAC device and the MDIO device, rather than
> > between the MAC device and the PHY device. And when the shared MDIO bus
> > is removed, all MAC drivers that depend on it will also be removed.
> 
> Shouldn't there be a full chain here? MAC->MDIO->PHY?

The PHY is already a child of the MDIO bus device, so the MDIO->PHY
relationship is already represented.

> 
> For the case of the SFP, maybe it should be
> 
> MAC->SFP Cage and I2C MDIO->PHY
> 
> So if the PHY inside an SFP disappears, the I2C MDIO bus is
> removed. But there is no link upwards from that, so the SFP cage and
> the MAC are left alone?
> 

Sorry, I have no experience with SPF module, but from Russell's reply,
it seems unlikely that the MDIO bus would be removed when removing
the PHY. I also modified Sarosh's patch based on this premise (the MDIO
bus will not be removed due to the removal of the PHY).

Re: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Jakub Kicinski]

On Mon, 26 Jan 2026 18:44:09 +0800 Wei Fang wrote:
> The commit bc66fa87d4fd ("net: phy: Add link between phy dev and mac
> dev") has created a device link between the MAC and the PHY if the MAC
> uses a shared MDIO bus (The MDIO and the MAC are two separate devices).
> Sarosh Hasan tried to change the DL_FLAG_STATELESS flag to
> DL_FLAG_AUTOREMOVE_SUPPLIER to fix the issue [1]. However, the solution
> does not take into account the hot-swappable PHY devices (such as SFP).
> so when the PHY device is unplugged, the MAC driver will automatically
> be removed, which is not the expected behavior.
> 
> Therefore, to solve this issue of the shared MDIO bus, we create the
> device link between the MAC device and the MDIO device, rather than
> between the MAC device and the PHY device. And when the shared MDIO bus
> is removed, all MAC drivers that depend on it will also be removed.

Anyone willing to venture a review tag?

Re: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Maxime Chevallier]

Hi

[...]
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 81984d4ebb7c..d5ac7506fe39 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1768,12 +1768,21 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
>  
>  	/**
>  	 * If the external phy used by current mac interface is managed by
> -	 * another mac interface, so we should create a device link between
> -	 * phy dev and mac dev.
> +	 * another MDIO controller, which means that the MAC and MDIO are
> +	 * separated devices, then we should create a device link between
> +	 * the MAC device and the MDIO device.
>  	 */

I was confused by the use of the "MDIO device" terminology here, which
refers to a device sitting on an mdio bus, such as a PHY. I think though
that what you actually refer to is the MDIO controller itself, right ?

> -	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) {
> +		if (!device_link_add(dev->dev.parent, phydev->mdio.bus->parent,
> +				     DL_FLAG_PM_RUNTIME |
> +				     DL_FLAG_AUTOREMOVE_SUPPLIER)) {

Don't we have the same problem with SFP ? The struct mii_bus for SFP
PHYs also completely disappears when you remove the module.

That being said, I ran some tests with SFP and this patch, and the
netdev actually didn't dissapear under my feet.

I'm a total noob with fw_devlink, but I can see there seems to already
be a link between MAC devices and the associated SFP bus :

on cyclone V :

# ls /sys/class/devlink
 [...]
platform:sfp--platform:ff702000.ethernet

on macchiatobin :

# ls /sys/class/devlink
 [...]
platform:sfp-eth3--platform:f4000000.ethernet

So I guess this is what's preventing the netdev from going away.

Now, these seems to be automagically populated based on DT, but I don't
know if it also works with non-DT platforms. If this is DT-specific,
then I guess we still can't use DL_FLAG_AUTOREMOVE_SUPPLIER.

Maxime

RE: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Wei Fang]

> > @@ -1768,12 +1768,21 @@ int phy_attach_direct(struct net_device *dev,
> struct phy_device *phydev,
> >
> >  	/**
> >  	 * If the external phy used by current mac interface is managed by
> > -	 * another mac interface, so we should create a device link between
> > -	 * phy dev and mac dev.
> > +	 * another MDIO controller, which means that the MAC and MDIO are
> > +	 * separated devices, then we should create a device link between
> > +	 * the MAC device and the MDIO device.
> >  	 */
> 
> I was confused by the use of the "MDIO device" terminology here, which
> refers to a device sitting on an mdio bus, such as a PHY. I think though
> that what you actually refer to is the MDIO controller itself, right ?

Yes, you are right, I was referring to the MDIO controller.

> 
> > -	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) {
> > +		if (!device_link_add(dev->dev.parent, phydev->mdio.bus->parent,
> > +				     DL_FLAG_PM_RUNTIME |
> > +				     DL_FLAG_AUTOREMOVE_SUPPLIER)) {
> 
> Don't we have the same problem with SFP ? The struct mii_bus for SFP
> PHYs also completely disappears when you remove the module.

Sorry, I'm not familiar with SFP and I do not have a board with SPF
module. So I'm not sure whether the MDIO bus will be removed if
the SFP module is unplugged. But if the multiple SPF modules share
the same MDIO controller, unplugging one SFP module will cause the
MDIO bus to be removed. Won't this cause the other SFP modules to
stop working?

> 
> That being said, I ran some tests with SFP and this patch, and the
> netdev actually didn't dissapear under my feet.
> 
> I'm a total noob with fw_devlink, but I can see there seems to already
> be a link between MAC devices and the associated SFP bus :
> 
> on cyclone V :
> 
> # ls /sys/class/devlink
>  [...]
> platform:sfp--platform:ff702000.ethernet
> 
> on macchiatobin :
> 
> # ls /sys/class/devlink
>  [...]
> platform:sfp-eth3--platform:f4000000.ethernet
> 
> So I guess this is what's preventing the netdev from going away.
> 
> Now, these seems to be automagically populated based on DT, but I don't
> know if it also works with non-DT platforms. If this is DT-specific,
> then I guess we still can't use DL_FLAG_AUTOREMOVE_SUPPLIER.
> 

Previously, when Sorash attempted to add
DL_FLAG_AUTOREMOVE_SUPPLIER to the devlink between the MAC
controller and the PHY, you reported that unplugging the PHY would
cause the MAC to be removed as well, resulting in system hangs.

https://lore.kernel.org/imx/20250704142138.3f1a4ec1@fedora.home/

If your guess is correct, removing the SPF module will cause the
MDIO bus to be removed, then I think the MAC driver should also
be removed. Therefore, based on this patch, you should be able
to reproduce the same problem caused by the Sorash's patch.
However, your current results seem different from before. Perhaps
the MDIO bus was not removed along with the SFP module.

Re: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Russell King (Oracle)]

On Wed, Jan 28, 2026 at 08:15:01PM -0800, Jakub Kicinski wrote:
> On Mon, 26 Jan 2026 18:44:09 +0800 Wei Fang wrote:
> > The commit bc66fa87d4fd ("net: phy: Add link between phy dev and mac
> > dev") has created a device link between the MAC and the PHY if the MAC
> > uses a shared MDIO bus (The MDIO and the MAC are two separate devices).
> > Sarosh Hasan tried to change the DL_FLAG_STATELESS flag to
> > DL_FLAG_AUTOREMOVE_SUPPLIER to fix the issue [1]. However, the solution
> > does not take into account the hot-swappable PHY devices (such as SFP).
> > so when the PHY device is unplugged, the MAC driver will automatically
> > be removed, which is not the expected behavior.
> > 
> > Therefore, to solve this issue of the shared MDIO bus, we create the
> > device link between the MAC device and the MDIO device, rather than
> > between the MAC device and the PHY device. And when the shared MDIO bus
> > is removed, all MAC drivers that depend on it will also be removed.
> 
> Anyone willing to venture a review tag?

No, I don't agrew with the patch.

-- 
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] net: phy: add device link between MAC device and MDIO device

[Maxime Chevallier]

On 29/01/2026 11:00, Wei Fang wrote:
>>> @@ -1768,12 +1768,21 @@ int phy_attach_direct(struct net_device *dev,
>> struct phy_device *phydev,
>>>
>>>  	/**
>>>  	 * If the external phy used by current mac interface is managed by
>>> -	 * another mac interface, so we should create a device link between
>>> -	 * phy dev and mac dev.
>>> +	 * another MDIO controller, which means that the MAC and MDIO are
>>> +	 * separated devices, then we should create a device link between
>>> +	 * the MAC device and the MDIO device.
>>>  	 */
>>
>> I was confused by the use of the "MDIO device" terminology here, which
>> refers to a device sitting on an mdio bus, such as a PHY. I think though
>> that what you actually refer to is the MDIO controller itself, right ?
> 
> Yes, you are right, I was referring to the MDIO controller.
> 
>>
>>> -	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) {
>>> +		if (!device_link_add(dev->dev.parent, phydev->mdio.bus->parent,
>>> +				     DL_FLAG_PM_RUNTIME |
>>> +				     DL_FLAG_AUTOREMOVE_SUPPLIER)) {
>>
>> Don't we have the same problem with SFP ? The struct mii_bus for SFP
>> PHYs also completely disappears when you remove the module.
> 
> Sorry, I'm not familiar with SFP and I do not have a board with SPF
> module. So I'm not sure whether the MDIO bus will be removed if
> the SFP module is unplugged. But if the multiple SPF modules share
> the same MDIO controller, unplugging one SFP module will cause the
> MDIO bus to be removed. Won't this cause the other SFP modules to
> stop working?

For SFP, it's not a physical mdiobus that connects to the SFP module, but
rather an i2c bus. When the module is inserted, we create an mdiobus that
will perform the mdio transfers over i2c :

https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/net/mdio/mdio-i2c.c#L461

The mdio bus is created when the SFP module is inserted, and we found a
PHY device in it :

https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/net/phy/sfp.c#L791

It is dynamically created and destroyed, its lifetime isn't correlated to
the netdevice.

> 
>>
>> That being said, I ran some tests with SFP and this patch, and the
>> netdev actually didn't dissapear under my feet.
>>
>> I'm a total noob with fw_devlink, but I can see there seems to already
>> be a link between MAC devices and the associated SFP bus :
>>
>> on cyclone V :
>>
>> # ls /sys/class/devlink
>>  [...]
>> platform:sfp--platform:ff702000.ethernet
>>
>> on macchiatobin :
>>
>> # ls /sys/class/devlink
>>  [...]
>> platform:sfp-eth3--platform:f4000000.ethernet
>>
>> So I guess this is what's preventing the netdev from going away.
>>
>> Now, these seems to be automagically populated based on DT, but I don't
>> know if it also works with non-DT platforms. If this is DT-specific,
>> then I guess we still can't use DL_FLAG_AUTOREMOVE_SUPPLIER.
>>
> 
> Previously, when Sorash attempted to add
> DL_FLAG_AUTOREMOVE_SUPPLIER to the devlink between the MAC
> controller and the PHY, you reported that unplugging the PHY would
> cause the MAC to be removed as well, resulting in system hangs.
> 
> https://lore.kernel.org/imx/20250704142138.3f1a4ec1@fedora.home/
> 
> If your guess is correct, removing the SPF module will cause the
> MDIO bus to be removed, then I think the MAC driver should also
> be removed. Therefore, based on this patch, you should be able
> to reproduce the same problem caused by the Sorash's patch.
> However, your current results seem different from before. Perhaps
> the MDIO bus was not removed along with the SFP module.
> 
It is removed :

https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/net/phy/sfp.c#L2648

This line above is called when we unplug the SFP module.

What I am seeing is that there are other devlink links that apply to
the net_device, unrelated to the MDIO bus, at least on the devices I am 
testing this on, and they prevent the net_device from going away.

One of these devlinks is a link between the sfp cage (compatible "sff,sfp") and
the netdev. This devlink seems to be automatically populated when OF is
parsed.

To solve this, I think we need something more granular that this approach,
and make a distinction between when it's OK for a phy_device to dissapear
when attached to a MAC (SFP phys), and when it's not (PHYs hardwired to
the MAC).

Adding this devlink in phy_attach_direct() is too generic IMO. It's likely
that this link should be populated by phylink then.

Maxime

RE: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Wei Fang]

> It is removed :
>
> https://elixir.boo/
> tlin.com%2Flinux%2Fv6.19-rc5%2Fsource%2Fdrivers%2Fnet%2Fphy%2Fsfp.c%2
> 3L2648&data=05%7C02%7Cwei.fang%40nxp.com%7C2f06a252bfa24532562e0
> 8de5f1fbe42%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6390527
> 87105780228%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlY
> iOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C
> 0%7C%7C%7C&sdata=FtbZ%2FZWqje7ckLwHvVENVlkus7nsTB3hZ3N05jYuflc%3
> D&reserved=0
>
> This line above is called when we unplug the SFP module.
>

Many thanks for your detailed explanation, I really know very little
about SFP. :(

> What I am seeing is that there are other devlink links that apply to
> the net_device, unrelated to the MDIO bus, at least on the devices I am
> testing this on, and they prevent the net_device from going away.
>
> One of these devlinks is a link between the sfp cage (compatible "sff,sfp") and
> the netdev. This devlink seems to be automatically populated when OF is
> parsed.
>
> To solve this, I think we need something more granular that this approach,
> and make a distinction between when it's OK for a phy_device to dissapear
> when attached to a MAC (SFP phys), and when it's not (PHYs hardwired to
> the MAC).

Yes, perhaps we need to add some flags to determine whether to add a
devlink between MAC controller and MDIO controller.

>
> Adding this devlink in phy_attach_direct() is too generic IMO. It's likely
> that this link should be populated by phylink then.
>

Some drivers may not use phylink, like the fec driver.

RE: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Wei Fang]

> On Wed, Jan 28, 2026 at 08:15:01PM -0800, Jakub Kicinski wrote:
> > On Mon, 26 Jan 2026 18:44:09 +0800 Wei Fang wrote:
> > > The commit bc66fa87d4fd ("net: phy: Add link between phy dev and mac
> > > dev") has created a device link between the MAC and the PHY if the
> > > MAC uses a shared MDIO bus (The MDIO and the MAC are two separate
> devices).
> > > Sarosh Hasan tried to change the DL_FLAG_STATELESS flag to
> > > DL_FLAG_AUTOREMOVE_SUPPLIER to fix the issue [1]. However, the
> > > solution does not take into account the hot-swappable PHY devices (such as
> SFP).
> > > so when the PHY device is unplugged, the MAC driver will
> > > automatically be removed, which is not the expected behavior.
> > >
> > > Therefore, to solve this issue of the shared MDIO bus, we create the
> > > device link between the MAC device and the MDIO device, rather than
> > > between the MAC device and the PHY device. And when the shared MDIO
> > > bus is removed, all MAC drivers that depend on it will also be removed.
> >
> > Anyone willing to venture a review tag?
> 
> No, I don't agrew with the patch.
> 

Is it because of SFP? If so, would the following modification be more
reasonable, while SFP still follows the previous logic? I'm not sure if
phydev->sfp_bus_attached can be used to distinguish between SFP
PHYs and non-SFP PHYs.

Anyway, this problem has existed for a long time, and I think we should
find a way to solve it in upstream. If there's a better approach, I can help
verify it.

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 81984d4ebb7c..c48335bfc7b6 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1771,9 +1771,17 @@ 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 && bus->parent && dev->dev.parent != bus->parent) {
+               if (phydev->sfp_bus_attached)
+                       phydev->devlink = device_link_add(dev->dev.parent,
+                                                         bus->parent,
+                                                         DL_FLAG_PM_RUNTIME |
+                                                         DL_FLAG_STATELESS);
+               else
+                       device_link_add(dev->dev.parent, bus->parent,
+                                       DL_FLAG_PM_RUNTIME |
+                                       DL_FLAG_AUTOREMOVE_SUPPLIER);
+       }

Re: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Maxime Chevallier]

Hi,

On 30/01/2026 04:41, Wei Fang wrote:
>> On Wed, Jan 28, 2026 at 08:15:01PM -0800, Jakub Kicinski wrote:
>>> On Mon, 26 Jan 2026 18:44:09 +0800 Wei Fang wrote:
>>>> The commit bc66fa87d4fd ("net: phy: Add link between phy dev and mac
>>>> dev") has created a device link between the MAC and the PHY if the
>>>> MAC uses a shared MDIO bus (The MDIO and the MAC are two separate
>> devices).
>>>> Sarosh Hasan tried to change the DL_FLAG_STATELESS flag to
>>>> DL_FLAG_AUTOREMOVE_SUPPLIER to fix the issue [1]. However, the
>>>> solution does not take into account the hot-swappable PHY devices (such as
>> SFP).
>>>> so when the PHY device is unplugged, the MAC driver will
>>>> automatically be removed, which is not the expected behavior.
>>>>
>>>> Therefore, to solve this issue of the shared MDIO bus, we create the
>>>> device link between the MAC device and the MDIO device, rather than
>>>> between the MAC device and the PHY device. And when the shared MDIO
>>>> bus is removed, all MAC drivers that depend on it will also be removed.
>>>
>>> Anyone willing to venture a review tag?
>>
>> No, I don't agrew with the patch.
>>
> 
> Is it because of SFP? If so, would the following modification be more
> reasonable, while SFP still follows the previous logic? I'm not sure if
> phydev->sfp_bus_attached can be used to distinguish between SFP
> PHYs and non-SFP PHYs.

If you want to know if the PHY is in an SFP module, you should use
phy_on_sfp() instead :

https://elixir.bootlin.com/linux/v6.18.6/source/include/linux/phy.h#L1766

It's set for this kind of setups :


               SFP module
+-----+      +-----------+
| MAC | ---- | PHY       |
+-----+      +--\--------+
                 \
                  \_ phy_on_sfp(phydev) == true


phydev->sfp_bus_attached is for the media converter user-case :

                            SFP Module
+-----+      +-----+      +------------+
| MAC | ---- | PHY | ---- |   ???      |
+-----+      +--\--+      +------------+
                 \
                  \_ phydev->sfp_bus_attached == true
> 
> Anyway, this problem has existed for a long time, and I think we should
> find a way to solve it in upstream. If there's a better approach, I can help
> verify it.

Sorry I wasn't available for most part of yesterday for that discussion.

I'll give a try to this patch today, with the above change and let you know
how this goes.

Maxime

RE: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Wei Fang]

> >>> Anyone willing to venture a review tag?
> >>
> >> No, I don't agrew with the patch.
> >>
> >
> > Is it because of SFP? If so, would the following modification be more
> > reasonable, while SFP still follows the previous logic? I'm not sure
> > if
> > phydev->sfp_bus_attached can be used to distinguish between SFP
> > PHYs and non-SFP PHYs.
>
> If you want to know if the PHY is in an SFP module, you should use
> phy_on_sfp() instead :
>
> https://elixir.boo/
> tlin.com%2Flinux%2Fv6.18.6%2Fsource%2Finclude%2Flinux%2Fphy.h%23L1766
> &data=05%7C02%7Cwei.fang%40nxp.com%7C94dbe2fb454c4a8fd96708de5fd9
> b94a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6390535858824
> 03531%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLj
> AuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%
> 7C%7C&sdata=FqnLEaC8Jt0zIgVQpIkbp0fg7WCrBuJjJFkCExRQKoc%3D&reserved
> =0
>
> It's set for this kind of setups :
>
>
>                SFP module
> +-----+      +-----------+
> | MAC | ---- | PHY       |
> +-----+      +--\--------+
>                  \
>                   \_ phy_on_sfp(phydev) == true
>
>
> phydev->sfp_bus_attached is for the media converter user-case :
>
>                             SFP Module
> +-----+      +-----+      +------------+
> | MAC | ---- | PHY | ---- |   ???      |
> +-----+      +--\--+      +------------+
>                  \
>                   \_ phydev->sfp_bus_attached == true

Thank you very much for your explanation and the diagram. I also noticed
phydev->is_on_sfp_module, which is also used in phy_attach_direct(), but
I didn't know the difference between them.

> >
> > Anyway, this problem has existed for a long time, and I think we
> > should find a way to solve it in upstream. If there's a better
> > approach, I can help verify it.
>
> Sorry I wasn't available for most part of yesterday for that discussion.
>
> I'll give a try to this patch today, with the above change and let you know how
> this goes.
>

Thanks again. This problem has been bothering us for a long time, it would be
great if this problem could be solved.

Re: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Maxime Chevallier]

Hi again,

On 30/01/2026 04:41, Wei Fang wrote:
[...]
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 81984d4ebb7c..c48335bfc7b6 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1771,9 +1771,17 @@ 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 && bus->parent && dev->dev.parent != bus->parent) {
> +               if (phydev->sfp_bus_attached)
                      ^ assuming we replace with phy_on_sfp()
> +                       phydev->devlink = device_link_add(dev->dev.parent,
> +                                                         bus->parent,
> +                                                         DL_FLAG_PM_RUNTIME |
> +                                                         DL_FLAG_STATELESS);
> +               else
> +                       device_link_add(dev->dev.parent, bus->parent,
> +                                       DL_FLAG_PM_RUNTIME |
> +                                       DL_FLAG_AUTOREMOVE_SUPPLIER);
> +       }

I think we should let the phy_on_sfp() case alone for now. The lifetime
of the SFP phydev and the MAC are completely independent here, the PHY
can be probed/removed without the MAC being there, and vice versa.

The MDIO bus that controls that SFP PHY's lifetime is the same as the
PHY itself, the mdio bus in that case exists solely to drive that single
PHY, and in can't be shared between multiple PHYs even if we have
multiple SFP ports in the entire system.

The logical thing to do IMO is not to tie the MAC and the MDIO Bus
itself, but rather the MAC and the PHY as was done before. As you state,
we couldn't set DL_FLAG_AUTOREMOVE_SUPPLIER because of SFP PHYs, but
if we add the check on "phy_on_sfp()", it should now be fine.

Maxime

RE: [PATCH net] net: phy: add device link between MAC device and MDIO device

[Wei Fang]

> On 30/01/2026 04:41, Wei Fang wrote:
> [...]
> >
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index 81984d4ebb7c..c48335bfc7b6 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -1771,9 +1771,17 @@ 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 && bus->parent && dev->dev.parent != bus->parent) {
> > +               if (phydev->sfp_bus_attached)
>                       ^ assuming we replace with phy_on_sfp()
> > +                       phydev->devlink =
> device_link_add(dev->dev.parent,
> > +
> bus->parent,
> > +
> DL_FLAG_PM_RUNTIME |
> > +
> DL_FLAG_STATELESS);
> > +               else
> > +                       device_link_add(dev->dev.parent, bus->parent,
> > +                                       DL_FLAG_PM_RUNTIME |
> > +
> DL_FLAG_AUTOREMOVE_SUPPLIER);
> > +       }
> 
> I think we should let the phy_on_sfp() case alone for now. The lifetime
> of the SFP phydev and the MAC are completely independent here, the PHY
> can be probed/removed without the MAC being there, and vice versa.
> 
> The MDIO bus that controls that SFP PHY's lifetime is the same as the
> PHY itself, the mdio bus in that case exists solely to drive that single
> PHY, and in can't be shared between multiple PHYs even if we have
> multiple SFP ports in the entire system.
> 
> The logical thing to do IMO is not to tie the MAC and the MDIO Bus
> itself, but rather the MAC and the PHY as was done before. As you state,

Okay, I agree. I will only change the devlink flag to
DL_FLAG_AUTOREMOVE_SUPPLIER. And thanks for you prompt feedback. :)


> we couldn't set DL_FLAG_AUTOREMOVE_SUPPLIER because of SFP PHYs, but
> if we add the check on "phy_on_sfp()", it should now be fine.
>