Re: [PATCH net-next v4 06/10] net: pcs: xpcs: add ANRESTART support for SGMII link recovery
From: sashiko-bot@kernel.org
Date: 2026-09-08 15:40:51
Also in:
linux-devicetree
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy