Message-ID: <Z5ybJtpOWNyWuF5h@shell.armlinux.org.uk>
Date: Fri, 31 Jan 2025 09:43:02 +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, 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
Subject: Re: [PATCH RFC net-next 1/2] net: pcs: xpcs: Add special code to
 operate in Microchip KSZ9477 switch

On Fri, Jan 31, 2025 at 02:24:23AM +0000, Tristram.Ha@...rochip.com wrote:
> > So the value of 0x2200 for DIG_CTRL1 means that bits 13 and 9 are both
> > set, which are the EN_VSMMD1 and MAC_AUTO_SW bits. So, are you saying
> > that MAC_AUTO_SW can't be cleared in KSZ9477? This is key information
> > that we need.
> > 
> > PHY_MODE_CTRL only has an effect when:
> >         DW_VR_MII_PCS_MODE_C37_SGMII
> >         DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII
> > are both set.
> > 
> > are set, and it determines the source for the bits in the configuration
> > word to be sent to the _MAC_ (since XPCS is acting as a PHY with both
> > these bits set.)
> 
> Sorry, my mistake.  The default value is 0x2400.  XPCS driver tries to
> write 0x2600 and the value stays at 0x2400.

Okay, we can use that if necessary to probe the hardware to see whether
it supports the MAC_AUTO_SW feature, and if it doesn't, it gives us a
clue that it is older IP.

> > If you have a SFP plugged in, then setting PHY-side on the XPCS is
> > wrong. TX_CONFIG_MAC_SIDE_SGMII should be used, and thus PHY_MODE_CTRL
> > is irrelevant as we are not generating a PHY-side Cisco SGMII config
> > word (as I detailed when I described the Cisco SGMII config words which
> > are asymmetric in nature.)

(relevant to the final part of my reply, so leaving this quoted here)

> > That's because if the XPCS acts as a PHY and is talking to another PHY,
> > the only then that the remote PHY (in the SFP module) is looking for is
> > an acknowledgement (bit 14 set.) It's possible that XPCS does this
> > despite operating as a PHY. However, there is another (remote)
> > possibility.

(relevant to the final part of my reply, so leaving this quoted here)

> > ... because the XPCS is operating in the wrong mode, and when operating
> > as a PHY does not expect the other end "the MAC" to be signalling link
> > status to it. One end of a Cisco SGMII link must operate as a PHY and
> > the other end must operate as a MAC to be correct. Other configurations
> > may sort-of work but are incorrect to the Cisco SGMII documentation.

(relevant to the final part of my reply, so leaving this quoted here)

> > Sigh. So you are referring to struct phylink_pcs's boolean neg_mode
> > member here, and fiddling with that is _wrong_. This flag is a
> > property of the driver code. It's "this driver code wants to see the
> > PHYLINK_PCS_NEG_* constants passed into its functions" when set,
> > as opposed to the MLO_AN_* constants that legacy drivers had. This
> > flag _must_ match the driver. It is _not_ to be fiddled with depending
> > on IP versions or other crap like that. It's purely about the code in
> > the driver. Do not touch this boolean. Do not change it in the XPCS
> > driver. It is correct. Setting it to false is incorrect. Am I clear?
> 
> I noticed when neg_mode == NEG_INBAND_ENABLED then auto-negotiation is
> not enabled.  And in the link_up function the code is skipped when this
> is the case.  I wanted to manipulate neg_mode so that it does not equal
> to NEG_INBAND_ENABLED and found it can be done by setting pcs->neg_mode
> to false.  I know the phylink code changes neg_mode to different values
> when pcs->neg_mode is true.  So if pcs->neg_mode is always set to true
> then is there a situation when neg_node is not equal to
> NEG_INBAND_ENABLED?

Yes, several.

- If you're using a fibre SFP and use ethtool to disable autoneg.
- If "inband" mode is not being used (rare with SFPs)
- If the PHY doesn't support inband.

> After executing this code it was the first time I realized that
> 1000BASEX mode works with auto-negotiation disabled.

... which probably happens because the PHY enters "bypass" mode
having given up waiting for the XPCS to send the config word.

> > Right, and its as documented in the KSZ9477 documentation. 1000base-X
> > mode requires TX_CONFIG_PHY_SIDE_SGMII and SGMII_LINK_STS. Likely
> > because of the older IP version.
> > 
> > Let me get back to what I said in my previous email - but word it
> > differently:
> > 
> >    Can we think that setting both TX_CONFIG_PHY_SIDE_SGMII and
> >    SGMII_LINK_STS unconditionally in the 1000base-X path with will
> >    not have any deterimental effects on newer IP versions?
> 
> Using this SGMII_LINK_STS | TX_CONFIG_PHY_SIDE_SGMII combination does not
> have any side effect on the new IP even though it is no longer required
> for 1000BASEX mode.

Let's hope that is the case.



> Note SGMII_LINK_STS | TX_CONFIG_PHY_SIDE_SGMII | C37_SGMII setting even
> works for SGMII mode.  It seems this combination SGMII_LINK_STS |
> TX_CONFIG_PHY_SIDE_SGMII reverts the effect of setting
> TX_CONFIG_PHY_SIDE_SGMII so the SGMII module still acts as a MAC.

No it won't. I've described what happens when you operate both sides
in PHY mode. I've given you the bit layout of the configuration words
that are sent in both directions. I've described how XPCS uses
SGMII_LINK_STS to populate the configuration word it sends in PHY
mode.

Not only will setting PHY mode change the configuration word that is
transmitted, it will change the interpretation of the received
configuration word.

It's broken to configure both ends of the Cisco SGMII link to be
acting as "master" or "PHY" mode. Don't do it.

I've already covered this in my previous reply (see the bits above
that I quoted and stated that they were relevant to this part of the
reply).

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

Powered by blists - more mailing lists