When dealing with PHY-driven SFP, we create a phy_port representing the
SFP bus when we know we have such a bus.

We can move the port creation before registering the sfp upstream ops,
as long as we know the SFP bus is there. This will allow passing the
phy_port along with the upstream information to the SFP bus.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/phy_device.c | 42 ++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8a3eb1839a3d..49696b1a2f13 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1665,13 +1665,13 @@ static void phy_del_port(struct phy_device *phydev, struct phy_port *port)
 	phydev->n_ports--;
 }
 
-static int phy_setup_sfp_port(struct phy_device *phydev)
+static struct phy_port *phy_setup_sfp_port(struct phy_device *phydev)
 {
 	struct phy_port *port = phy_port_alloc();
 	int ret;
 
 	if (!port)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	port->parent_type = PHY_PORT_PHY;
 	port->phy = phydev;
@@ -1686,10 +1686,12 @@ static int phy_setup_sfp_port(struct phy_device *phydev)
 	 * when attaching the port to the phydev.
 	 */
 	ret = phy_add_port(phydev, port);
-	if (ret)
+	if (ret) {
 		phy_port_destroy(port);
+		return ERR_PTR(ret);
+	}
 
-	return ret;
+	return port;
 }
 
 /**
@@ -1698,22 +1700,34 @@ static int phy_setup_sfp_port(struct phy_device *phydev)
  */
 static int phy_sfp_probe(struct phy_device *phydev)
 {
+	struct phy_port *port;
 	struct sfp_bus *bus;
-	int ret = 0;
+	int ret;
 
-	if (phydev->mdio.dev.fwnode) {
-		bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
-		if (IS_ERR(bus))
-			return PTR_ERR(bus);
+	if (!phydev->mdio.dev.fwnode)
+		return 0;
+
+	bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
+	if (IS_ERR(bus))
+		return PTR_ERR(bus);
 
-		phydev->sfp_bus = bus;
+	phydev->sfp_bus = bus;
 
-		ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
-		sfp_bus_put(bus);
+	if (bus) {
+		port = phy_setup_sfp_port(phydev);
+		if (IS_ERR(port)) {
+			sfp_bus_put(bus);
+			return PTR_ERR(port);
+		}
 	}
 
-	if (!ret && phydev->sfp_bus)
-		ret = phy_setup_sfp_port(phydev);
+	ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
+	sfp_bus_put(bus);
+
+	if (ret && port) {
+		phy_del_port(phydev, port);
+		phy_port_destroy(port);
+	}
 
 	return ret;
 }
-- 
2.49.0

On Sun, Feb 01, 2026 at 04:12:42PM +0100, Maxime Chevallier wrote:
> When dealing with PHY-driven SFP, we create a phy_port representing the
> SFP bus when we know we have such a bus.
> 
> We can move the port creation before registering the sfp upstream ops,
> as long as we know the SFP bus is there. This will allow passing the
> phy_port along with the upstream information to the SFP bus.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

...

> @@ -1698,22 +1700,34 @@ static int phy_setup_sfp_port(struct phy_device *phydev)
>   */
>  static int phy_sfp_probe(struct phy_device *phydev)
>  {
> +	struct phy_port *port;
>  	struct sfp_bus *bus;
> -	int ret = 0;
> +	int ret;
>  
> -	if (phydev->mdio.dev.fwnode) {
> -		bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
> -		if (IS_ERR(bus))
> -			return PTR_ERR(bus);
> +	if (!phydev->mdio.dev.fwnode)
> +		return 0;
> +
> +	bus = sfp_bus_find_fwnode(phydev->mdio.dev.fwnode);
> +	if (IS_ERR(bus))
> +		return PTR_ERR(bus);
>  
> -		phydev->sfp_bus = bus;
> +	phydev->sfp_bus = bus;
>  
> -		ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
> -		sfp_bus_put(bus);
> +	if (bus) {
> +		port = phy_setup_sfp_port(phydev);
> +		if (IS_ERR(port)) {
> +			sfp_bus_put(bus);
> +			return PTR_ERR(port);
> +		}
>  	}
>  
> -	if (!ret && phydev->sfp_bus)
> -		ret = phy_setup_sfp_port(phydev);
> +	ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
> +	sfp_bus_put(bus);
> +
> +	if (ret && port) {

Hi Maxime,

If bus is NULL then port will not be set here.

Flagged by Smatch.

> +		phy_del_port(phydev, port);
> +		phy_port_destroy(port);
> +	}
>  
>  	return ret;
>  }
> -- 
> 2.49.0
>

Hi Simon,

On 03/02/2026 18:52, Simon Horman wrote:

>> -	if (!ret && phydev->sfp_bus)
>> -		ret = phy_setup_sfp_port(phydev);
>> +	ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
>> +	sfp_bus_put(bus);
>> +
>> +	if (ret && port) {
> 
> Hi Maxime,
> 
> If bus is NULL then port will not be set here.

Arg... I though smatch was running in the NIPA ingst-mdir script, but I
was wrong :(

I'll address that in V5 as the problme is still in V4, thank you for the
report

Maxime

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: phy: Create SFP phy_port before registering usptream

This commit moves the creation of the SFP phy_port to occur before
registering the SFP upstream ops. This reordering allows passing
the phy_port along with the upstream information to the SFP bus.

> net: phy: Create SFP phy_port before registering usptream

This isn't a bug, but there's a typo in the subject line: "usptream"
should be "upstream".

Hi Simon,

On 03/02/2026 19:03, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: phy: Create SFP phy_port before registering usptream
> 
> This commit moves the creation of the SFP phy_port to occur before
> registering the SFP upstream ops. This reordering allows passing
> the phy_port along with the upstream information to the SFP bus.
> 
>> net: phy: Create SFP phy_port before registering usptream
> 
> This isn't a bug, but there's a typo in the subject line: "usptream"
> should be "upstream".

That's aleady addressed in V4 :)

Maxime