MII phy_ports are not meant to be connected directly to a link partner.
They are meant to feed into some media converter devices, so far we only
support SFP modules for that.

We have information about what MII they can handle, however we don't
store anything about whether they are currently connected to an SFP
module or not. As phy_port aims at listing the front-facing ports, let's
store an "vacant" bit to know whether or not a MII port is currently
vacant (i.e. there's no module in the SFP cage), or not (i.e.
there's an SFP module).

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/phy_device.c | 17 +++++++++++++----
 drivers/net/phy/phylink.c    |  7 +++++++
 include/linux/phy_port.h     |  4 ++++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8bff64040540..5d26d75b5f6b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1550,6 +1550,7 @@ static int phy_sfp_module_insert(void *upstream, const struct sfp_eeprom_id *id)
 	struct phy_device *phydev = upstream;
 	const struct sfp_module_caps *caps;
 	struct phy_port *port;
+	int ret = 0;
 
 	phy_interface_t iface;
 
@@ -1578,9 +1579,12 @@ static int phy_sfp_module_insert(void *upstream, const struct sfp_eeprom_id *id)
 		phydev->port = caps->port;
 
 	if (port->ops && port->ops->configure_mii)
-		return port->ops->configure_mii(port, true, iface);
+		ret = port->ops->configure_mii(port, true, iface);
 
-	return 0;
+	if (!ret)
+		port->vacant = false;
+
+	return ret;
 }
 
 static void phy_sfp_module_remove(void *upstream)
@@ -1588,8 +1592,12 @@ static void phy_sfp_module_remove(void *upstream)
 	struct phy_device *phydev = upstream;
 	struct phy_port *port = phy_get_sfp_port(phydev);
 
-	if (port && port->ops && port->ops->configure_mii)
-		port->ops->configure_mii(port, false, PHY_INTERFACE_MODE_NA);
+	if (port) {
+		if (port->ops && port->ops->configure_mii)
+			port->ops->configure_mii(port, false,
+						 PHY_INTERFACE_MODE_NA);
+		port->vacant = true;
+	}
 
 	if (phydev->n_ports == 1)
 		phydev->port = PORT_NONE;
@@ -1756,6 +1764,7 @@ static struct phy_port *phy_setup_sfp_port(struct phy_device *phydev)
 	 */
 	port->is_mii = true;
 	port->is_sfp = true;
+	port->vacant = true;
 
 	/* The port->supported and port->interfaces list will be populated
 	 * when attaching the port to the phydev.
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 79f7811dec22..b575f5db9ed6 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1777,6 +1777,7 @@ static int phylink_create_sfp_port(struct phylink *pl)
 	port->is_sfp = true;
 	port->is_mii = true;
 	port->active = true;
+	port->vacant = true;
 
 	phy_interface_and(port->interfaces, pl->config->supported_interfaces,
 			  phylink_sfp_interfaces);
@@ -3917,6 +3918,9 @@ static int phylink_sfp_module_insert(void *upstream,
 	pl->sfp_may_have_phy = caps->may_have_phy;
 	pl->sfp_port = caps->port;
 
+	if (pl->sfp_bus_port)
+		pl->sfp_bus_port->vacant = false;
+
 	/* If this module may have a PHY connecting later, defer until later */
 	if (pl->sfp_may_have_phy)
 		return 0;
@@ -3928,6 +3932,9 @@ static void phylink_sfp_module_remove(void *upstream)
 {
 	struct phylink *pl = upstream;
 
+	if (pl->sfp_bus_port)
+		pl->sfp_bus_port->vacant = true;
+
 	phy_interface_zero(pl->sfp_interfaces);
 }
 
diff --git a/include/linux/phy_port.h b/include/linux/phy_port.h
index 4e2a3fdd2f2e..b0b1fa6a67a9 100644
--- a/include/linux/phy_port.h
+++ b/include/linux/phy_port.h
@@ -51,6 +51,9 @@ struct phy_port_ops {
  * @is_mii: Indicates if this port is MII (Media Independent Interface),
  *          or MDI (Media Dependent Interface).
  * @is_sfp: Indicates if this port drives an SFP cage.
+ * @vacant: For MII ports, indicates whether or not the port has a connection to
+ *	    another device (e.g. for SFP ports, indicates the absence or presence
+ *	    of an SFP module)
  */
 struct phy_port {
 	u32 id;
@@ -71,6 +74,7 @@ struct phy_port {
 	unsigned int active:1;
 	unsigned int is_mii:1;
 	unsigned int is_sfp:1;
+	unsigned int vacant:1;
 };
 
 struct phy_port *phy_port_alloc(void);
-- 
2.49.0

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: phy_port: Store information about a MII port's vacant state

This patch adds a 'vacant' flag to phy_port to track whether an MII port
currently has an SFP module installed. The flag is set to true when the
port is created or when a module is removed, and set to false when a
module is successfully inserted.

> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 8bff64040540..5d26d75b5f6b 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1550,6 +1550,7 @@ static int phy_sfp_module_insert(void *upstream, const struct sfp_eeprom_id *id)
>  	struct phy_device *phydev = upstream;
>  	const struct sfp_module_caps *caps;
>  	struct phy_port *port;
> +	int ret = 0;
>
>  	phy_interface_t iface;
>
> @@ -1578,9 +1579,12 @@ static int phy_sfp_module_insert(void *upstream, const struct sfp_eeprom_id *id)
>  		phydev->port = caps->port;
>
>  	if (port->ops && port->ops->configure_mii)
> -		return port->ops->configure_mii(port, true, iface);
> +		ret = port->ops->configure_mii(port, true, iface);
>
> -	return 0;
> +	if (!ret)
> +		port->vacant = false;
> +
> +	return ret;
>  }

[ ... ]

> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 79f7811dec22..b575f5db9ed6 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c

[ ... ]

> @@ -3917,6 +3918,9 @@ static int phylink_sfp_module_insert(void *upstream,
>  	pl->sfp_may_have_phy = caps->may_have_phy;
>  	pl->sfp_port = caps->port;
>
> +	if (pl->sfp_bus_port)
> +		pl->sfp_bus_port->vacant = false;
> +
>  	/* If this module may have a PHY connecting later, defer until later */
>  	if (pl->sfp_may_have_phy)
>  		return 0;

Can the 'vacant' flag remain incorrectly set to false when the module is
removed?

In phylink_sfp_module_insert(), the vacant flag is set to false before
phylink_sfp_config_optical() is called. If phylink_sfp_config_optical()
returns an error (for example, no common interface between MAC and SFP
module), the error propagates back to sfp_sm_module(), which transitions
the state machine to SFP_MOD_ERROR (enum value 1).

When the module is later physically removed, sfp_sm_mod_remove() checks
'sfp->sm_mod_state > SFP_MOD_WAITDEV' (enum value 3). Since SFP_MOD_ERROR
is 1, which is less than 3, sfp_module_remove() is NOT called. This means
phylink_sfp_module_remove() never executes and 'vacant' remains false even
though the module was physically removed.

The PHY-side function phy_sfp_module_insert() in phy_device.c avoids this
by gating the assignment:

    if (!ret)
        port->vacant = false;

Should phylink_sfp_module_insert() use the same pattern, setting vacant to
false only after phylink_sfp_config_optical() succeeds?

Hi

On 06/03/2026 15: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: phy_port: Store information about a MII port's vacant state
> 
> This patch adds a 'vacant' flag to phy_port to track whether an MII port
> currently has an SFP module installed. The flag is set to true when the
> port is created or when a module is removed, and set to false when a
> module is successfully inserted.
> 
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 8bff64040540..5d26d75b5f6b 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -1550,6 +1550,7 @@ static int phy_sfp_module_insert(void *upstream, const struct sfp_eeprom_id *id)
>>  	struct phy_device *phydev = upstream;
>>  	const struct sfp_module_caps *caps;
>>  	struct phy_port *port;
>> +	int ret = 0;
>>
>>  	phy_interface_t iface;
>>
>> @@ -1578,9 +1579,12 @@ static int phy_sfp_module_insert(void *upstream, const struct sfp_eeprom_id *id)
>>  		phydev->port = caps->port;
>>
>>  	if (port->ops && port->ops->configure_mii)
>> -		return port->ops->configure_mii(port, true, iface);
>> +		ret = port->ops->configure_mii(port, true, iface);
>>
>> -	return 0;
>> +	if (!ret)
>> +		port->vacant = false;
>> +
>> +	return ret;
>>  }
> 
> [ ... ]
> 
>> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
>> index 79f7811dec22..b575f5db9ed6 100644
>> --- a/drivers/net/phy/phylink.c
>> +++ b/drivers/net/phy/phylink.c
> 
> [ ... ]
> 
>> @@ -3917,6 +3918,9 @@ static int phylink_sfp_module_insert(void *upstream,
>>  	pl->sfp_may_have_phy = caps->may_have_phy;
>>  	pl->sfp_port = caps->port;
>>
>> +	if (pl->sfp_bus_port)
>> +		pl->sfp_bus_port->vacant = false;
>> +
>>  	/* If this module may have a PHY connecting later, defer until later */
>>  	if (pl->sfp_may_have_phy)
>>  		return 0;
> 
> Can the 'vacant' flag remain incorrectly set to false when the module is
> removed?
> 
> In phylink_sfp_module_insert(), the vacant flag is set to false before
> phylink_sfp_config_optical() is called. If phylink_sfp_config_optical()
> returns an error (for example, no common interface between MAC and SFP
> module), the error propagates back to sfp_sm_module(), which transitions
> the state machine to SFP_MOD_ERROR (enum value 1).
> 
> When the module is later physically removed, sfp_sm_mod_remove() checks
> 'sfp->sm_mod_state > SFP_MOD_WAITDEV' (enum value 3). Since SFP_MOD_ERROR
> is 1, which is less than 3, sfp_module_remove() is NOT called. This means
> phylink_sfp_module_remove() never executes and 'vacant' remains false even
> though the module was physically removed.
> 
> The PHY-side function phy_sfp_module_insert() in phy_device.c avoids this
> by gating the assignment:
> 
>     if (!ret)
>         port->vacant = false;
> 
> Should phylink_sfp_module_insert() use the same pattern, setting vacant to
> false only after phylink_sfp_config_optical() succeeds?

I'll rather reset-it upon failure, but that's correct and I tested this
error case to be indeed showing-up.

I'll address for the next revision

Maxime