Re: [net-next,12/14] net: stmmac: add support for reading inband SGMII status
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-01-16 02:58:54
Also in:
linux-arm-kernel, linux-arm-msm, linux-phy
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: stmmac: add support for reading inband SGMII status This commit adds support for reading link status, speed, and duplex for SGMII links from the RGSMII status register.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c index 73fc56ce5e55..12fc5038d913 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -97,7 +107,7 @@ static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs, struct phylink_link_state *state) { struct stmmac_pcs *spcs = phylink_pcs_to_stmmac_pcs(pcs); - u32 status, lpa; + u32 status, lpa, rgsmii; status = readl(spcs->base + GMAC_AN_STATUS);@@ -111,7 +121,35 @@ static void dwmac_integrated_pcs_get_state(struct phylink_pcs *pcs, phylink_mii_c22_pcs_decode_state(state, neg_mode, status, lpa); } else { - state->link = false; + rgsmii = field_get(spcs->rgsmii_status_mask, + readl(spcs->rgsmii)); + state->link = !!(status & GMAC_RGSMIII_LNKSTS);
^^^^^^ Should this be checking rgsmii instead of status here? The status variable contains the GMAC_AN_STATUS register which is equivalent to MII_BMSR according to the comment at line 7. In BMSR, the link status bit is BMSR_LSTATUS (bit 2), not bit 3. GMAC_RGSMIII_LNKSTS is defined as BIT(3), which is the correct bit position for the RGSMII status register (matching GMAC_RGSMIIIS_LNKSTS in dwmac1000.h), but it appears to be applied to the wrong register variable. The code later uses rgsmii correctly for speed and duplex extraction. Was the intent to use rgsmii for the link status check as well?
+
+ if (state->link && neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED) {[ ... ]