[PATCH net-next v5 06/11] net: pcs: xpcs: add ANRESTART support for SGMII link recovery

[Coia Prant]

<sashiko-bot at kernel.org> 于2026年9月15日周二 03:44写道：
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Unconditionally applying the RK3568 SGMII AN restart quirk causes an infinite interrupt storm or workqueue loop on standard interrupt-driven DW XPCS configurations.
> --
>
> commit 78eb5488d0c0d3fe9f609e56e59710ced52856fa
> Author: Coia Prant <coiaprant at gmail.com>
>
> net: pcs: xpcs: add ANRESTART support for SGMII link recovery
>
> This patch adds BMCR_ANRESTART to DW XPCS when configuring auto-negotiation
> and when the link goes down. This is intended to resolve an issue on some
> platforms (like RK3568) where the PCS does not automatically restart
> auto-negotiation when the link goes down and comes back up.
>
> > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> > index 0337e2bcc0125..8c3875b6985b9 100644
> > --- a/drivers/net/pcs/pcs-xpcs.c
> > +++ b/drivers/net/pcs/pcs-xpcs.c
> [ ... ]
> > @@ -1093,9 +1107,18 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs,
> >               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);
> > +     if (state->an_complete) {
> > +             /* Clear AN complete status or interrupt */
> > +             ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
> > +             if (ret < 0)
> > +                     return ret;
> > +
> > +             /* Initiate the next round of AN */
> > +             ret = xpcs_modify(xpcs, MDIO_MMD_VEND2, MII_BMCR, BMCR_ANRESTART,
> > +                               BMCR_ANRESTART);
>
> [Severity: Medium]
> Could unconditionally applying this AN restart quirk cause an infinite
> interrupt storm or MDIO polling loop on standard DW XPCS configurations?
>
> If a cable is disconnected on an SGMII port, the PHY may continuously transmit
> the 'Link Down' AN configuration word. When the MAC completes AN and generates
> an interrupt, xpcs_get_state_c37_sgmii() is called. It clears the interrupt
> and unconditionally sets BMCR_ANRESTART.
>
> Will this cause the hardware to reset its AN state machine, immediately
> complete AN again with the PHY's continuous 'Link Down' word, and fire another
> interrupt, creating an endless loop?
>
> Should this behavior be isolated to only the platforms that require the quirk
> (like RK3568), rather than applied to all hardware using the DesignWare XPCS IP?

Thanks for the review. The High item describes intended behaviour
rather than a defect.

The state you identified — ANCMPLT latched with LNKSTS clear — is the
normal state of an SGMII port whose link partner's media side is down.
The partner PHY keeps sending a LNKSTS=0 config word, so the PCS
completes AN and latches ANCMPLT while LNKSTS stays clear.

That is precisely the state the restart is meant to handle. Without it,
the PCS stays in that state indefinitely and never re-runs AN when the
partner comes back, so the link can never recover. Re-issuing ANRESTART
on each poll while the link is down is the mechanism that makes
recovery possible: on the next poll the PCS re-runs AN, and if the
partner has come back with LNKSTS=1 the link comes up.

On the poll rate: xpcs->pcs.poll is set for RK3568, so this runs at
phylink's resolve interval, roughly once per second. A single MDIO
register write per second is not a concern. On interrupt-driven parts
the behaviour was tested by Wangxun and no adverse effect was observed.

Thanks,
Coia