Re: [PATCH net] net: pcs: xpcs: fix SGMII state reading

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi,

On 7/17/26 09:43, Coia Prant wrote:
> Commit 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> added a path in xpcs_get_state_c37_sgmii() that reads speed/duplex from
> BMCR after AN completes. However, BMCR does not reflect the negotiated
> result on the hardware where this has been tested:
> 
> - On RK3568 (MAC side SGMII), BMCR returns a fixed hardware reset value
> - Wangxun engineer Jiawen Wu confirmed that on their side, "BMCR looks
>   like it only wants to be return as 0" [0]
> 
> The correct information is available in CL37_ANSGM_STS, which contains
> the actual link status and negotiated speed/duplex.
> 
> This bug was previously masked by phylink core, which overrides the PCS
> link state with the PHY state when a PHY is present:
> 
>         /* If we have a phy, the "up" state is the union of both the
>          * PHY and the MAC
>          */
>         if (phy)
>                 link_state.link &= pl->phy_state.link;
> 
> Thus, when the link is down, the PHY's link_down state is applied on top
> of whatever the PCS reports, hiding the broken PCS state reading path.
> 
> Modify xpcs_get_state_c37_sgmii() to:
> 1. Read link state from CL37_ANSGM_STS
> 2. If link is up, report speed/duplex from CL37_ANSGM_STS
> 3. Remove the broken BMCR reading path entirely
> 
> Also properly set state->an_complete to reflect the AN completion status,
> and clear CL37_ANCMPLT_INTR when link is down to avoid stale state.
> 
> [0] https://lore.kernel.org/all/000c01dd1593$2ac0b0f0$804212d0$@trustnetic.com/
> 
> Fixes: 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> Cc: stable@vger.kernel.org
> Tested-by: Jiawen Wu <jiawenwu@trustnetic.com>
> Signed-off-by: Coia Prant <coiaprant@gmail.com>

Give that a test on ksz9477 that has an older XPCS for SGMII, this makes some
sense to me and no regressions were found.

Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime