[PATCH net v3 0/4] net: phy: some cleanups following phy_port SFP

[PATCH net v3 0/4] net: phy: some cleanups following phy_port SFP

[Maxime Chevallier]

While posting the v11 of phy_port netlink, sashiko found some
pre-existing issues, and following the tentative fix, Nicolai found
some more :)

This is V3, with a re-ordering of the port/sfp cleanup, as well as a new
patch (patch 3) that also reorders the phy_remove() path.

V3 :
 - Re-order port cleanup and sfp cleanup
 - patch 3 is new
 - Fix typo wich -> which
 - Add Nicolai's revies

V2 : https://lore.kernel.org/r/20260601084029.815461-1-maxime.chevallier@bootlin.com
 - Add port cleanup - Nicolai
 - Don't probe SFP for genphy drivers - sashiko

V1 : https://lore.kernel.org/r/20260530072706.3167745-1-maxime.chevallier@bootlin.com

Maxime Chevallier (4):
  net: phy: clean the sfp upstream if phy probing fails
  net: phy: remove phy ports upon probe failure
  net: phy: Clean the phy_ports after unregistering the downstream SFP
    bus
  net: phy: don't try to setup PHY-driven SFP cages when using genphy

 drivers/net/phy/phy_device.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

-- 
2.54.0

[PATCH net v3 1/4] net: phy: clean the sfp upstream if phy probing fails

[Maxime Chevallier]

Sashiko reported that we don't call sfp_bus_del_upstream() in the probe
failure path, so let's add it, otherwise the sfp-bus is left with a
dangling 'upstream' field, that may be used later on during SFP events.

This issue existed before the generic phylib sfp support, back when
drivers were calling phy_sfp_probe themselves.

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Fixes: 298e54fa810e ("net: phy: add core phylib sfp support")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/phy_device.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 3370eb822017..6ccbfacf7d1d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1718,6 +1718,9 @@ static int phy_sfp_probe(struct phy_device *phydev)
 
 		ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
 		sfp_bus_put(bus);
+
+		if (ret)
+			phydev->sfp_bus = NULL;
 	}
 
 	if (!ret && phydev->sfp_bus)
@@ -3775,6 +3778,9 @@ static int phy_probe(struct device *dev)
 	return 0;
 
 out:
+	sfp_bus_del_upstream(phydev->sfp_bus);
+	phydev->sfp_bus = NULL;
+
 	if (!phydev->is_on_sfp_module)
 		phy_led_triggers_unregister(phydev);
 
-- 
2.54.0

[PATCH net v3 2/4] net: phy: remove phy ports upon probe failure

[Maxime Chevallier]

When phy_probe fails, let's clean the phy_ports that were successfully
added already.

Suggested-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Fixes: 589e934d2735 ("net: phy: Introduce PHY ports representation")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/phy_device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 6ccbfacf7d1d..4ba880446896 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3781,6 +3781,8 @@ static int phy_probe(struct device *dev)
 	sfp_bus_del_upstream(phydev->sfp_bus);
 	phydev->sfp_bus = NULL;
 
+	phy_cleanup_ports(phydev);
+
 	if (!phydev->is_on_sfp_module)
 		phy_led_triggers_unregister(phydev);
 
-- 
2.54.0

[PATCH net v3 3/4] net: phy: Clean the phy_ports after unregistering the downstream SFP bus

[Maxime Chevallier]

As reported by sashiko when looking a other patches, we need to ensure
that the downstream SFP bus gets unregistered prior to destroying the
phy_ports attached to a phy_device, as the SFP code may reference these
ports. Let's make sure we follow that ordering in phy_remove().

Fixes: 589e934d2735 ("net: phy: Introduce PHY ports representation")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/phy_device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 4ba880446896..b48722589f40 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3806,11 +3806,11 @@ static int phy_remove(struct device *dev)
 
 	phydev->state = PHY_DOWN;
 
-	phy_cleanup_ports(phydev);
-
 	sfp_bus_del_upstream(phydev->sfp_bus);
 	phydev->sfp_bus = NULL;
 
+	phy_cleanup_ports(phydev);
+
 	if (phydev->drv && phydev->drv->remove)
 		phydev->drv->remove(phydev);
 
-- 
2.54.0

[PATCH net v3 4/4] net: phy: don't try to setup PHY-driven SFP cages when using genphy

[Maxime Chevallier]

We don't have support for PHY-driver SFP cages with the genphy code.

On top of that, it was found by sashiko that running
sfp_bus_add_upstream() for genphy deadlocks, as for genphy the PHY
probing runs under RTNL, which isn't the case for non-genphy drivers.

This problem was reproduced, and does lead to a deadlock on RTNL.

Before the blamed commit, the phy_sfp_probe() call was made by
individual PHY drivers, so there was no way to get to the SFP probing
path when using genphy.

Let's therefore only run phy_sfp_probe when not using genphy.

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Fixes: bad869b5e41a ("net: phy: Only rely on phy_port for PHY-driven SFP")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/phy_device.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b48722589f40..1511385b9b36 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3512,9 +3512,15 @@ static int phy_setup_ports(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	ret = phy_sfp_probe(phydev);
-	if (ret)
-		goto out;
+	/* We don't support SFP with genphy drivers. Also, genphy driver
+	 * binding occurs with RTNL help, which will deadlock the call to
+	 * sfp_bus_add_upstream().
+	 */
+	if (!phydev->is_genphy_driven) {
+		ret = phy_sfp_probe(phydev);
+		if (ret)
+			goto out;
+	}
 
 	if (phydev->n_ports < phydev->max_n_ports) {
 		ret = phy_default_setup_single_port(phydev);
-- 
2.54.0

Re: [PATCH net v3 3/4] net: phy: Clean the phy_ports after unregistering the downstream SFP bus

[Nicolai Buchwitz]

HI Maxime

On 4.6.2026 11:28, Maxime Chevallier wrote:
> As reported by sashiko when looking a other patches, we need to ensure
> that the downstream SFP bus gets unregistered prior to destroying the
> phy_ports attached to a phy_device, as the SFP code may reference these
> ports. Let's make sure we follow that ordering in phy_remove().
> 
> Fixes: 589e934d2735 ("net: phy: Introduce PHY ports representation")
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
>  drivers/net/phy/phy_device.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c 
> b/drivers/net/phy/phy_device.c
> index 4ba880446896..b48722589f40 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -3806,11 +3806,11 @@ static int phy_remove(struct device *dev)
> 
>  	phydev->state = PHY_DOWN;
> 
> -	phy_cleanup_ports(phydev);
> -
>  	sfp_bus_del_upstream(phydev->sfp_bus);
>  	phydev->sfp_bus = NULL;
> 
> +	phy_cleanup_ports(phydev);
> +
>  	if (phydev->drv && phydev->drv->remove)
>  		phydev->drv->remove(phydev);

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks,
Nicolai

Re: [PATCH net v3 0/4] net: phy: some cleanups following phy_port SFP

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  4 Jun 2026 11:28:14 +0200 you wrote:
> While posting the v11 of phy_port netlink, sashiko found some
> pre-existing issues, and following the tentative fix, Nicolai found
> some more :)
> 
> This is V3, with a re-ordering of the port/sfp cleanup, as well as a new
> patch (patch 3) that also reorders the phy_remove() path.
> 
> [...]

Here is the summary with links:
  - [net,v3,1/4] net: phy: clean the sfp upstream if phy probing fails
    https://git.kernel.org/netdev/net/c/48774e87bbaa
  - [net,v3,2/4] net: phy: remove phy ports upon probe failure
    https://git.kernel.org/netdev/net/c/b1e780bb37c6
  - [net,v3,3/4] net: phy: Clean the phy_ports after unregistering the downstream SFP bus
    https://git.kernel.org/netdev/net/c/4497f5028675
  - [net,v3,4/4] net: phy: don't try to setup PHY-driven SFP cages when using genphy
    https://git.kernel.org/netdev/net/c/5a0082ec20a0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html