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

Hi Coia,

On 8/1/26 16:22, Coia Prant wrote:
> On some hardware using the DesignWare XPCS IP (e.g., RK3568 MAC side
> SGMII), the PCS does not automatically restart auto-negotiation when the
> link goes down and comes back up. Without an explicit ANRESTART, the link
> stays down forever.
> 
> Add BMCR_ANRESTART in two places:
> 1. In xpcs_config_aneg_c37_sgmii(), when starting AN, set ANRESTART
>    alongside ANENABLE to initiate a fresh negotiation.
> 2. In xpcs_get_state_c37_sgmii(), when link is down and AN completion is
>    detected, clear the interrupt and trigger ANRESTART to restart the
>    negotiation process.
> 
> This change has been verified on:
> - RK3568 (Photonicat board)
> - Wangxun NICs (no regression)
> 
> Tested-by: Jiawen Wu <[email protected]>
> Signed-off-by: Coia Prant <[email protected]>

KSZ9477's old XPCS doesn't break with that code, tested in SGMII
mode :)

Tested-by: Maxime Chevallier <[email protected]>

Maxime

> ---
>  drivers/net/pcs/pcs-xpcs.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> index 0337e2bcc0125..28e98449e8ac2 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 (ret < 0)
>  		return ret;
>  
> +	/* Clear CL37 AN complete status */
> +	ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
> +	if (ret < 0)
> +		return ret;
> +
>  	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);
>  
>  	return ret;
>  }
> @@ -1089,14 +1094,18 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs,
>  			state->duplex = DUPLEX_FULL;
>  		else
>  			state->duplex = DUPLEX_HALF;
> -
>  		return 0;
>  	}
>  
> -	/* Clear AN complete status or interrupt */
> -	if (state->an_complete)
> +	if (state->an_complete) {
> +		/* Clear AN complete status or interrupt */
>  		xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
>  
> +		/* Initiate the next round of AN */
> +		xpcs_modify(xpcs, MDIO_MMD_VEND2, MII_BMCR, BMCR_ANRESTART,
> +				    BMCR_ANRESTART);
> +	}
> +
>  	return 0;
>  }
>