Re: [PATCH net-next 03/12] net: pcs: pcs-xpcs: Preserve BMCR_ANENBLE during link up

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditionally forcing BMCR_ANENABLE for SGMII interfaces during out-of-band link configuration breaks forced and fixed links.
--

commit 3fde24f4d6c6c8c6b563418e32cec40953863f8a
Author: Daniel Thompson <daniel@riscstar.com>

net: pcs: pcs-xpcs: Preserve BMCR_ANENBLE during link up

This commit unconditionally sets the BMCR_ANENABLE bit for SGMII interfaces
during link up. This change was introduced to resolve an issue on a specific
hardware platform where the MAC fails to transmit packets despite negotiating
the speed correctly.

> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> index b2c84b7e1e11..1d62d5b31c61 100644
> --- a/drivers/net/pcs/pcs-xpcs.c
> +++ b/drivers/net/pcs/pcs-xpcs.c
> @@ -1263,11 +1263,14 @@ static void xpcs_link_up_sgmii_1000basex(struct dw_xpcs *xpcs,
>  					 phy_interface_t interface,
>  					 int speed, int duplex)
>  {
> +	u16 an_enable;
>  	int ret;
>  
>  	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
>  		return;
>  
> +	an_enable = (interface == PHY_INTERFACE_MODE_SGMII ? BMCR_ANENABLE : 0);
> +

Will this break forced and fixed links by enabling auto-negotiation?

This function handles forcing the link state when in-band auto-negotiation
is explicitly disabled, as indicated by the early return for
PHYLINK_PCS_NEG_INBAND_ENABLED. When this function continues, phylink is
requesting out-of-band negotiation or a fixed link.

By unconditionally setting BMCR_ANENABLE here, the hardware might ignore
the fixed speed and duplex bits, instead waiting for auto-negotiation to
complete. If the system uses a fixed link (e.g., MAC-to-MAC) or connects
to a PHY with in-band signaling disabled, SGMII AN base pages will never
arrive.

Could this cause the MAC PCS to wait indefinitely, failing to establish a
link and dropping all traffic?

[ ... ]

>  	ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MII_BMCR,
> -			 mii_bmcr_encode_fixed(speed, duplex));
> +			 mii_bmcr_encode_fixed(speed, duplex) | an_enable);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260501155421.3329862-1-elder@riscstar.com?part=3