| Message ID | 20250226074837.1679988-2-yong.liang.choong@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable SGMII and 2500BASEX interface mode switching for Intel platforms | expand |
On Wed, Feb 26, 2025 at 03:48:32PM +0800, Choong Yong Liang wrote: > The phylink_expects_phy() function allows MAC drivers to check if they are > expecting a PHY to attach. The checking condition in phylink_expects_phy() > aims to achieve the same result as the checking condition in > phylink_attach_phy(). > > However, the checking condition in phylink_expects_phy() uses > pl->link_config.interface, while phylink_attach_phy() uses > pl->link_interface. > > Initially, both pl->link_interface and pl->link_config.interface are set > to SGMII, and pl->cfg_link_an_mode is set to MLO_AN_INBAND. > > When the interface switches from SGMII to 2500BASE-X, > pl->link_config.interface is updated by phylink_major_config(). > At this point, pl->cfg_link_an_mode remains MLO_AN_INBAND, and > pl->link_config.interface is set to 2500BASE-X. > Subsequently, when the STMMAC link goes down and comes up again, > it is blocked by phylink_expects_phy(). I thought we ascertained that it's not "link goes down" but when the interface is taken down administratively. "Link goes down" to most people mean an event such as the network cable being unplugged. Please fix the patch description. > Since phylink_expects_phy() and phylink_attach_phy() aim to achieve the > same result, phylink_expects_phy() should check pl->link_interface, > which never changes, instead of pl->link_config.interface, which is > updated by phylink_major_config(). > > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> With, and *only* with the above fixed: Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Thanks!
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index a3b186ab3854..a3f64b6d2d34 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -2044,7 +2044,7 @@ bool phylink_expects_phy(struct phylink *pl) { if (pl->cfg_link_an_mode == MLO_AN_FIXED || (pl->cfg_link_an_mode == MLO_AN_INBAND && - phy_interface_mode_is_8023z(pl->link_config.interface))) + phy_interface_mode_is_8023z(pl->link_interface))) return false; return true; }
The phylink_expects_phy() function allows MAC drivers to check if they are expecting a PHY to attach. The checking condition in phylink_expects_phy() aims to achieve the same result as the checking condition in phylink_attach_phy(). However, the checking condition in phylink_expects_phy() uses pl->link_config.interface, while phylink_attach_phy() uses pl->link_interface. Initially, both pl->link_interface and pl->link_config.interface are set to SGMII, and pl->cfg_link_an_mode is set to MLO_AN_INBAND. When the interface switches from SGMII to 2500BASE-X, pl->link_config.interface is updated by phylink_major_config(). At this point, pl->cfg_link_an_mode remains MLO_AN_INBAND, and pl->link_config.interface is set to 2500BASE-X. Subsequently, when the STMMAC link goes down and comes up again, it is blocked by phylink_expects_phy(). Since phylink_expects_phy() and phylink_attach_phy() aim to achieve the same result, phylink_expects_phy() should check pl->link_interface, which never changes, instead of pl->link_config.interface, which is updated by phylink_major_config(). Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> --- drivers/net/phy/phylink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)