Re: [PATCH net-next v4 06/10] net: pcs: xpcs: add ANRESTART support for SGMII link recovery

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Stale comment contradicts new ANRESTART behavior.
--

commit 553a0aad47033d8800155c135475e03ebbeacf26
Author: Coia Prant <coiaprant@gmail.com>

net: pcs: xpcs: add ANRESTART support for SGMII link recovery

This commit modifies the XPCS driver to trigger ANRESTART alongside ANENABLE
to ensure auto-negotiation correctly restarts on link state changes for
certain hardware. It applies this both when starting AN and when the link
is down but AN completion is detected.

> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> index 0337e2bcc0125..50b9cdf94fdaa 100644
> --- a/drivers/net/pcs/pcs-xpcs.c
> +++ b/drivers/net/pcs/pcs-xpcs.c
> @@ -816,9 +816,14 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
[ ... ]
>  	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
>  		ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MII_BMCR,
> -				 mdio_ctrl | BMCR_ANENABLE);
> +				 mdio_ctrl | BMCR_ANENABLE | BMCR_ANRESTART);

[Severity: Low]
Since this change explicitly triggers AN restart here, does the comment
earlier in xpcs_config_aneg_c37_sgmii() need to be updated?

The block comment at the top of the function currently states:

        /* For AN for C37 SGMII mode, the settings are :-
[ ... ]
         * Note: Since it is MAC side SGMII, there is no need to set
         *       SR_MII_AN_ADV. MAC side SGMII receives AN Tx Config from
         *       PHY about the link state change after C28 AN is completed
         *       between PHY and Link Partner. There is also no need to
         *       trigger AN restart for MAC-side SGMII.
         */
         
It seems the new code logic contradicts the last sentence of this comment.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907153713.2572547-2-coiaprant@gmail.com?part=6