Message-ID:
 <DM3PR11MB8736B524E983153188A531ADECE82@DM3PR11MB8736.namprd11.prod.outlook.com>
Date: Fri, 31 Jan 2025 02:24:23 +0000
From: <Tristram.Ha@...rochip.com>
To: <linux@...linux.org.uk>
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

> > As I mentioned before, the IP used in KSZ9477 is old and so may not match
> > the behavior in current DesignWare specifications.  I experimented with
> > different settings and observed these behaviors.
> >
> > DW_VR_MII_DIG_CTRL1 has default value 0x2200.  Setting MAC_AUTO_SW (9)
> > has no effect as the value read back does not retain the bit.  Setting
> > PHY_MODE_CTRL (0) retains the bit but does not seem to have any effect
> > and it is not required for operation.  So we can ignore this register
> > for KSZ9477.
> 
> 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.

> > DW_VR_MII_AN_CTRL has default value 0x0004, which means C37_SGMII is
> > enabled.  Plugging in a 10/100/1000Base-T SFP will work without doing
> > anything.
> >
> > Setting TX_CONFIG_PHY_SIDE_SGMII (3) requires auto-negotiation to be
> > disabled in MII_BMCR for the port to work.
> 
> 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.)
> 
> > SGMII_LINK_STS (4) depends on TX_CONFIG_PHY_SIDE_SGMII.  If that bit is
> > not set then this bit has no effect.  The result of setting this bit is
> > auto-negotiation can be enabled in MII_BMCR.
> 
> "that bit" and "this bit" makes this difficult to follow as I'm not
> sure which of SGMII_LINK_STS or TX_CONFIG_PHY_SIDE_SGMII that "this"
> and "that" are referring to. Please be explicit to avoid confusion.
> 
> Since in later documentation, SGMII_LINK_STS is used to populate bit
> 15 of the Cisco SGMII configuration word when operating in PHY mode
> when these bits are set:
> 
>         TX_CONFIG_PHY_SIDE_SGMII
>         PHY_MODE_CTRL
> 
> then, as PHY_MODE_CTRL is not supported by your hardware (it's marked
> as reserved) I suggest that this older version of XPCS either has this
> PHY_MODE_CTRL as an integration-time option, or the logic of taking
> the values from registers was not implemented in that revision. Thus
> why it only depends on TX_CONFIG_PHY_SIDE_SGMII in your case.
> 
> > So C37_SGMII mode can still work with TX_CONFIG_PHY_SIDE_SGMII on and
> > auto-negotiation disabled.
> 
> 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.
> 
> > But the problem is when the cable is
> > unplugged and plugged the port does not work as the module cannot detect
> > the link.  Enabling auto-negotiation and then disabling it will cause
> > the port to work again.
> 
> ... 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.
> 
> > Now for 1000BaseX mode C37_1000BASEX is used and when auto-negotiation
> > is disabled the port works.
> 
> There's something which can complicate "works" - some implementations
> have a "bypass" mode for negotiation. If they only receive idles
> without any sign of negotiation, after a timeout expires, they enter
> "bypass" mode and bring the data link up anyway.

I do not have the knowledge to check that all PHY register bits like
acknowledgment and next-page happen as should be during negotiation.  
I can only verify the link status is correct and the SGMII port can send
traffic.  If that works then the setup works, isn't it?

The only concern is there may be corruption in the data line or it
completely breaks other IP.

> > For the XPCS driver this can be done by setting neg_mode to false at
> > the beginning. Problem is this flag can > only be set once at driver
> > initialization.
> 
> 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?

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

This was my observation during experimentation with the XPCS driver and I
am not trying to change neg_mode.

> > Note this behavior for 1000BaseX mode only occurs in KSZ9477, so we can
> > stop finding the reasons with current specs.
> 
> 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.

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.

So even though you said not to setup the module as a PHY it may not have
that effect.

Powered by blists - more mailing lists