Message-ID: <7c37c518-fb6d-4f78-8e83-663e2518e5fd@bootlin.com>
Date: Thu, 29 Jan 2026 11:18:16 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Wei Fang <wei.fang@....com>
Cc: "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "andrew@...n.ch" <andrew@...n.ch>,
 "hkallweit1@...il.com" <hkallweit1@...il.com>,
 "linux@...linux.org.uk" <linux@...linux.org.uk>,
 "davem@...emloft.net" <davem@...emloft.net>,
 "edumazet@...gle.com" <edumazet@...gle.com>,
 "kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com"
 <pabeni@...hat.com>,
 "florian.fainelli@...adcom.com" <florian.fainelli@...adcom.com>,
 "xiaolei.wang" <xiaolei.wang@...driver.com>,
 "quic_abchauha@...cinc.com" <quic_abchauha@...cinc.com>,
 "quic_sarohasa@...cinc.com" <quic_sarohasa@...cinc.com>
Subject: Re: [PATCH net] net: phy: add device link between MAC device and MDIO
 device



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

Powered by blists - more mailing lists