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

From: sashiko-bot@kernel.org
To: "Coia Prant" <coiaprant@gmail.com>
Cc: neil.armstrong@linaro.org, devicetree@vger.kernel.org,
	robh@kernel.org, linux-phy@lists.infradead.org,
	wsa+renesas@sang-engineering.com, conor+dt@kernel.org,
	vkoul@kernel.org, olteanv@gmail.com
Subject: Re: [PATCH v2 06/10] net: pcs: xpcs: add ANRESTART support for SGMII link recovery
Date: Sun, 02 Aug 2026 14:25:36 +0000	[thread overview]
Message-ID: <20260802142537.173511F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260801142244.679437-8-coiaprant@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Stale documentation: the patch introduces BMCR_ANRESTART for MAC-side SGMII but fails to update the block comment that explicitly states this is not needed.
--

--- Patch [6]: [PATCH v2 06/10] net: pcs: xpcs: add ANRESTART support for SGMII link recovery ---
commit 2bc2528d92a030c16f9b0b3ce9ca602d98573c26
Author: Coia Prant <coiaprant@gmail.com>

net: pcs: xpcs: add ANRESTART support for SGMII link recovery

This commit adds ANRESTART support for SGMII link recovery on hardware
using the DesignWare XPCS IP. It sets BMCR_ANRESTART when starting
auto-negotiation and triggers it again if the link goes down after
completion to ensure the negotiation restarts properly.

> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> --- 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]
This isn't a bug, but does this change make the large block comment earlier
in xpcs_config_aneg_c37_sgmii() contradictory?

The existing comment explicitly states:
"There is also no need to trigger AN restart for MAC-side SGMII."

Since this patch adds the BMCR_ANRESTART flag to the MAC-side SGMII path,
should the comment be updated to reflect the new behavior?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801142244.679437-2-coiaprant@gmail.com?part=6

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy