linux-kernel - Re: [WARNING: ATTACHMENT UNSCANNED]Re: [WARNING: ATTACHMENT UNSCANNED]Re: [PATCH RFC net-next 1/2] net: pcs: xpcs: Add special code to operate in Microchip KSZ9477 switch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z53nSexf2BaUQiCY@shell.armlinux.org.uk>
Date: Sat, 1 Feb 2025 09:20:09 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Tristram.Ha@...rochip.com
Cc: olteanv@...il.com, Woojung.Huh@...rochip.com, andrew@...n.ch,
	hkallweit1@...il.com, Jose.Abreu@...opsys.com,
	maxime.chevallier@...tlin.com, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	UNGLinuxDriver@...rochip.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, Sadhan.Rudresh@...opsys.com,
	Siddhant.Kumar@...opsys.com
Subject: Re: [WARNING: ATTACHMENT UNSCANNED]Re: [WARNING: ATTACHMENT
 UNSCANNED]Re: [PATCH RFC net-next 1/2] net: pcs: xpcs: Add special code to
 operate in Microchip KSZ9477 switch

On Sat, Feb 01, 2025 at 01:12:00AM +0000, Tristram.Ha@...rochip.com wrote:
> > On Fri, Jan 31, 2025 at 02:36:49PM +0000, Jose Abreu wrote:
> > > From: Russell King (Oracle) <linux@...linux.org.uk>
> > > Date: Thu, Jan 30, 2025 at 11:02:00
> > >
> > > > Would it be safe to set these two bits with newer XPCS hardware when
> > > > programming it for 1000base-X mode, even though documentation e.g.
> > > > for SJA1105 suggests that these bits do not apply when operating in
> > > > 1000base-X mode?
> > >
> > > It's hard to provide a clear answer because our products can all be modified
> > > by final customer. I hope this snippet below can help:
> > >
> > > "Nothing has changed in "AN control register" ever since at least for a decade.
> > > Having said that, bit[4] and bit[3] are valid for SGMII mode and not valid
> > > for 1000BASE-X mode (I don't know why customer says 'serdes' mode.
> > > There is no such mode in ethernet standard). So, customer shall
> > > leave this bits at default value of 0.  Even if they set to 1, there is no
> > > impact (as those bits are not used in 1000BASE-X mode)."
> > 
> > Thanks for the reply Jose, that's useful.
> > 
> > Tristram, I think you need to talk to your hardware people to find out
> > where this requirement to set these two bits comes from as it seems it
> > isn't a property that comes from Synopsys' IP (I suppose unless your
> > IP is older than ten years.)
> > 
> > That said, Jose's response indicates that we can set these two bits
> > with impunity provided another of Synopsys's customers hasn't modified
> > their integration of XPCS to require these bits to be set to zero. So,
> > while I think we can do that unconditionally (as per the patch
> > attached) I think we need a clearer comment to state why it's being
> > done (and I probably need to now modify the commit message - this was
> > created before Jose's reply.)
> > 
> > So, I think given the last two patches I've sent, I believe I've
> > covered both of the issues that you have with XPCS:
> > 
> > 1) the need to set bits 4 and 3 in AN control for 1000base-X in KSZ9477
> >    (subject to a better commit message and code comment, which will be
> >     dependent on your research as to where this requirement has come
> >     from.)
> > 
> > 2) the lack of MAC_AUTO_SW support in KSZ9477 which can be enabled by
> >    writing DW_XPCS_SGMII_MODE_MAC_MANUAL to xpcs->sgmii_mode.
> > 
> > We now need to work out a way to identify this older IP. I think for
> > (2) we could potentially do something like (error handling omitted for
> > clarity):
> > 
> >         if (xpcs->sgmii_mode == DW_XPCS_SGMII_MODE_MAC_AUTO) {
> >                 xpcs_modify(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL,
> >                             DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW,
> >                             DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW);
> > 
> >                 ret = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL);
> > 
> >                 /* If MAC_AUTO_SW doesn't retain the MAC_AUTO_SW bit, then the
> >                  * XPCS implementation does not support this feature, and we
> >                  * have to manually configure the BMCR for the link parameters.
> >                  */
> >                 if (ret >= 0 && !(ret & DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW))
> >                         xpcs->sgmii_mode = DW_XPCS_SGMII_MODE_MAC_MANUAL;
> >         }
> 
> The IP document says version 3.10a and has date August 2013.
> 
> Indeed it does not make sense to use SGMII_LINK_STS and
> TX_CONFIG_PHY_SIDE_SGMII in 1000BASEX mode.
> 
> My thinking is there may be a hardware bug to prevent 1000BASEX mode to
> work when auto-negotiation is enabled.  And somehow setting those bits
> workaround that, and those bits have different meanings.
> 
> In order not to use those bits auto-negotiation needs to be disabled.  Is
> there a way to do that in normal driver activation?

You do not want to do that. While you may negotiate with a PHY using
1000base-X that has "bypass" mode enabled, anything that doesn't have
a "bypass" mode or has it disabled won't work.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!