Re: [net-next,v6,07/10] net: phy: phy_port: Store information about a MII port's vacant state
From: Simon Horman <horms@kernel.org>
Date: 2026-03-06 14:03:39
Also in:
lkml
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.
quoted hunk ↗ jump to hunk
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; }
[ ... ]
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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?