From 2efa9f57d089ad1793b8a9baecbcfd6e8392e333 Mon Sep 17 00:00:00 2001 From: Coia Prant Date: Thu, 30 Jul 2026 03:11:08 +0800 Subject: [PATCH] net: pcs: xpcs: fix SGMII state reading commit def9a4745e105145133e442dd8a1c126caf0f553 upstream. 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 Signed-off-by: Coia Prant Tested-by: Maxime Chevallier Reviewed-by: Maxime Chevallier Link: https://patch.msgid.link/20260717074324.3250043-2-coiaprant@gmail.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- drivers/net/pcs/pcs-xpcs.c | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c index f0f41e86a4fb..680e2b261086 100644 --- a/drivers/net/pcs/pcs-xpcs.c +++ b/drivers/net/pcs/pcs-xpcs.c @@ -994,6 +994,7 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs, /* Reset link_state */ state->link = false; + state->an_complete = false; state->speed = SPEED_UNKNOWN; state->duplex = DUPLEX_UNKNOWN; state->pause = 0; @@ -1005,6 +1006,8 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs, if (ret < 0) return ret; + state->an_complete = ret & DW_VR_MII_AN_STS_C37_ANCMPLT_INTR; + if (ret & DW_VR_MII_C37_ANSGM_SP_LNKSTS) { int speed_value; @@ -1023,35 +1026,14 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs, state->duplex = DUPLEX_FULL; else state->duplex = DUPLEX_HALF; - } else if (ret == DW_VR_MII_AN_STS_C37_ANCMPLT_INTR) { - int speed, duplex; - state->link = true; - - speed = xpcs_read(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1); - if (speed < 0) - return speed; - - speed &= SGMII_SPEED_SS13 | SGMII_SPEED_SS6; - if (speed == SGMII_SPEED_SS6) - state->speed = SPEED_1000; - else if (speed == SGMII_SPEED_SS13) - state->speed = SPEED_100; - else if (speed == 0) - state->speed = SPEED_10; - - duplex = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE); - if (duplex < 0) - return duplex; - - if (duplex & DW_FULL_DUPLEX) - state->duplex = DUPLEX_FULL; - else if (duplex & DW_HALF_DUPLEX) - state->duplex = DUPLEX_HALF; - - xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0); + return 0; } + /* Clear AN complete status or interrupt */ + if (state->an_complete) + xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0); + return 0; }