Message-ID: <20250129211226.cfrhv4nn3jomooxc@skbuf>
Date: Wed, 29 Jan 2025 23:12:26 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Tristram.Ha@...rochip.com
Cc: 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,
	linux@...linux.org.uk, 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 Wed, Jan 29, 2025 at 12:31:09AM +0000, Tristram.Ha@...rochip.com wrote:
> The default value of DW_VR_MII_AN_CTRL is DW_VR_MII_PCS_MODE_C37_SGMII
> (0x04).  When a SGMII SFP is used the SGMII port works without any
> programming.  So for example network communication can be done in U-Boot
> through the SGMII port.  When a 1000BaseX SFP is used that register needs
> to be programmed (DW_VR_MII_SGMII_LINK_STS |
> DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII | DW_VR_MII_PCS_MODE_C37_1000BASEX)
> (0x18) for it to work.

Can it be that DW_VR_MII_PCS_MODE_C37_1000BASEX is the important setting
when writing 0x18, and the rest is just irrelevant and bogus? If not,
could you please explain what is the role of DW_VR_MII_SGMII_LINK_STS |
DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII for 1000Base-X? The XPCS data book
does not suggest they would be considered for 1000Base-X operation. Are
you suggesting for KSZ9477 that is different? If so, please back that
statement up.

> (DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII has to be used together with
> DW_VR_MII_TX_CONFIG_MASK to mean 0x08.  Likewise for
> DW_VR_MII_PCS_MODE_C37_SGMII and DW_VR_MII_PCS_MODE_MASK to mean 0x04.
> It is a little difficult to just use those names to indicate the actual
> value.)
> 
> DW_VR_MII_DIG_CTRL1 is never touched.  DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW
> does not exist in KSZ9477 implementation.  As setting that bit does not
> have any effect I did not do anything about it.

Never touched by whom? xpcs_config_aneg_c37_sgmii() surely tries to
touch it... Don't you think that the absence of this bit from the
KSZ9477 implementation might have something to do with KSZ9477's unique
need to force the link speed when in in-band mode?

Here's a paragraph about this from the data book:

DWC_xpcs supports automatic reconfiguration of SGMII speed/duplex mode
based on the outcome of auto-negotiation. This feature can be enabled by
programming bit[9] (MAC_AUTO_SW) of VR_MII_DIG_CTRL1 when operating in
SGMII MAC mode. DWC_xpcs is initially configured in the speed/duplex
mode as programmed in the SR_MII_CTRL. If MAC_AUTO_SW bit is enabled,
DWC_xpcs automatically switches to the negotiated speed mode after the
completion of CL37 Auto-negotiation. This eliminates the software
overhead of reading CL37 AN SGMII Status from VR_MII_AN_INTR_STS and
then programming SS13 and SS6 speed-selection bits of SR_MII_CTRL
appropriately.

> It does have the intended effect of separating SGMII and 1000BaseX modes
> in later versions.  And DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL is used along
> with it.  They are mutually exclusive.  For SGMII SFP
> DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW is set; for 1000BaseX SFP
> DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL is set.

It's difficult for me to understand what you are trying to communicate here.

> KSZ9477 errata module 7 indicates the MII_ADVERTISE register needs to be
> set 0x01A0.  This is done with phylink_mii_c22_pcs_encode_advertisement()
> but only for 1000BaseX mode.  I probably need to add that code in SGMII
> configuration.  The default value of this register is 0x20.  This update
> depends on SFP.  So far I did not find a SGMII SFP that requires this
> setting.  This issue is more like the hardware did not set the default
> value properly.  As I said, the SGMII port works with SGMII SFP after
> power up without programming anything.
> 
> I am always confused by the master/slave - phy/mac nature of the SFP.
> The hardware designers seem to think the SGMII module is acting as a
> master then the slave is on the other side, like physically outside the
> chip.  I generally think of the slave is inside the SFP, as every board
> is programmed that way.
> 
> The original instruction was to set 0x18 for SerDes mode, which is used
> for 1000BaseX.  Even though the bits have SGMII names they do not mean
> SGMII operation, although I do not know the exact definition of SGMII.

It sounds like you should run "sgmii spec" by your favorite search
engine and give it a read, it's a freely available PDF only several
pages long, and it will be worth spending your time.

> Note the evaluation board never has SFP cage logic so I never knew there
> is a PHY inside the SGMII SFP.

What kind of SFP cage logic does the evaluation board have?

> For SFPs with label 10/100/1000Base-T
> they start in SGMII mode.  For SFPs with just 1000Base-T they start in
> 1000BaseX mode and needs 0x18 value to work.  In Linux the PHY inside the
> SFP can switch back to SGMII mode and so the SGMII setting is used
> because the EEPROM says SGMII mode is supported.  There are some SFPs
> that will use only 1000BaseX mode.  I wonder why the SFP manufacturers do
> that.  It seems the PHY access is also not reliable as some registers
> always have 0xff value in lower part of the 16-bit value.  That may be
> the reason that there is a special Marvell PHY id just for Finisar.

Powered by blists - more mailing lists