Re: [PATCH net-next v8 1/6] net: phylink: use pl->link_interface in phylink_expects_phy()

["Russell King (Oracle)" <linux@armlinux.org.uk>]

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!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!