RE: [RFC PATCH 06/10] net: pcs: xpcs: improve SGMII AN state handling for Rockchip RK3568

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Fri, Jul 17, 2026 2:29 PM, Coia Prant wrote:
> Jiawen Wu <jiawenwu@trustnetic.com> 于2026年7月17日周五 10:23写道：
> >
> > On Wed, Jul 15, 2026 7:05 AM, Coia Prant wrote:
> > > Andrew Lunn <andrew@lunn.ch> 于2026年7月15日周三 06:44写道：
> > > >
> > > > On Wed, Jul 15, 2026 at 03:08:34AM +0800, Coia Prant wrote:
> > > > > Commit 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> > > >
> > > > You do not appear to Cc: the Wangxun NIC people. It would be good to
> > > > have there comments on this change.
> > >
> > > I apologize; the output from get_maintainer.pl is very long. I’ve
> > > heard that having too many recipients can cause the PATCH to be
> > > rejected by the LKML mail server.
> > >
> > > I have added Wangxun maintainer (Jiawen Wu <jiawenwu@trustnetic.com>
> > > and Mengyuan Lou <mengyuanlou@net-swift.com>) to the CC list.
> > >
> > > > > Fixes: 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> > > > > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> > > >
> > > > Please don't mix fixed and new code. Is this a real fix? Should it be
> > > > back ported to stable?
> > >
> > > I am not sure if this is a specific characteristic of Wangxun NICs, as
> > > I do not have any available for testing.
> > >
> > > The behavior of the Rockchip DW XPCS IP core matches what is described
> > > in the commit message (even though phylink brings the link to down
> > > based on the phydev link status).
> > >
> > > This appears to be a bug (at least on Rockchip platforms) or Wangxun
> > > NICs features.
> > >
> > > However, I cannot confirm whether Wangxun NICs behave the same way.
> > > Therefore, I have kept their code as is for now.
> > >
> > > Could a Wangxun NICs maintainer provide some feedback based on testing?
> > >
> > > If we can confirm that this is indeed a bug, I can submit a separate fix.
> > >
> > > I would greatly appreciate it.
> > >
> > > Thanks.
> >
> > Hi Coia,
> >
> > I have tested this patch and it works on Wangxun NICs, thanks.
> >
> > The commit 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> > is too long ago for me to recall exactly what that thought was.
> >
> > The log shows:
> >
> > "On this device, CL37_ANSGM_STS (bit[4:1] of VR_MII_AN_INTR_STS) indicates
> > the status received from remote link during the auto-negotiation, and
> > self-clear after the auto-negotiation is complete.
> > Meanwhile, CL37_ANCMPLT_INTR will be set to 1, to indicate CL37 AN is
> > complete. So add another way to get the state for CL37 SGMII."
> >
> > I tried to reproduce this issue, but it didn't seem to exist.
> > In the current logic:
> >
> > link up -> read status from CL37_ANSGM_STS -> CL37_ANCMPLT_INTR not clear
> > link down -> read status from BMCR -> CL37_ANCMPLT_INTR clear
> >
> > It also works, although CL37_ANCMPLT_INTR is not cleared every time, and AN
> > restart is absent. But BMCR looks like it only wants to be return as 0, it is
> > weird.
> >
> > So I think Wangxun NICs also can be applied to general code as well, for
> > getting state in C37 SGMII mode.
> 
> Hi Jiawen,
> 
> Thanks so much for testing and confirming that the patch works on Wangxun
> NICs. That's a huge relief.
> 
> You're right to ask about the CL37_ANCMPLT_INTR clear and ANRESTART. Let
> me explain what I observed on RK3568.
> 
> The key difference is that on RK3568's XPCS, we're working in the MAC
> side (TX_CONFIG = 0), whereas on Wangxun it seems you're in the PHY side
> (TX_CONFIG = 1). This is based on the commit 2a22b7ae2fa3 where you set
> TX_CONFIG = 1.

Get it.

> 
> > However, I am wondering whether CL37_ANCMPLT_INTR clear and ANRESTART are
> > necessary.
> 
> On the RK3568 (MAC side), according to the DesignWare XPCS TRM, the
> hardware behavior is as follows:
> 
> - After a software reset (BMCR_RESET), ANENABLE is automatically set to 1
>   (the hardware reset value). So auto-negotiation starts automatically.
> 
> - When the link is up, CL37_ANSGM_STS contains the negotiated speed/duplex
>   and the link status.
> 
> - When the link goes down, CL37_ANSGM_STS becomes all zero, but
>   CL37_ANCMPLT_INTR remains set to 1.
> 
> - Crucially, the PCS does **not** automatically restart AN when the link
>   comes back up. Without an explicit ANRESTART, the link stays down
>   forever.
> 
> So on RK3568, the ANRESTART is mandatory to recover from a link-down
> event. Without it, the link never comes back up after a cable disconnect.
> 
> In my current logic:
> 
> 1. If CL37_ANSGM_STS has valid speed/duplex bits, report the link as up
>    and return.
> 
> 2. If CL37_ANSGM_STS is zero but CL37_ANCMPLT_INTR is set, the link is
>    down. We clear the interrupt and issue an ANRESTART to start a new
>    negotiation.
> 
> This works reliably on RK3568 in SGMII MAC side mode.
> 
> Since you confirmed that the general code path also works on Wangxun, I
> wonder: does the ANRESTART also work on your side? If yes, perhaps we can
> make this the common path for all hardware, not just RK3568.
> 
> If you can test the ANRESTART logic on Wangxun and it works, I'd be happy
> to submit a standalone fix that makes this the common code path.

I've test this patch with removing the restriction on Wangxun PMA. At least
ANRESTART logic does not have a negative impact.

> 
> Otherwise, the current approach (skipping the ANRESTART trigger on
> Wangxun NICs in c37_sgmii_get_state) remains safe.
> 
> If you're comfortable with the change, I'd appreciate it if you could add
> your Tested-by tag to the commit. I'll also make sure to include you on
> the CC list for any future revisions or related fixes.
> 
> Thanks again for testing!

Thanks for the improvements.
Tested-by: Jiawen Wu <jiawenwu@trustnetic.com>