Message-ID:
 <DM3PR11MB873652D36F1FC20999F45772ECE92@DM3PR11MB8736.namprd11.prod.outlook.com>
Date: Thu, 30 Jan 2025 04:50:18 +0000
From: <Tristram.Ha@...rochip.com>
To: <linux@...linux.org.uk>, <olteanv@...il.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>,
	<UNGLinuxDriver@...rochip.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: RE: [WARNING: ATTACHMENT UNSCANNED]Re: [PATCH RFC net-next 1/2] net:
 pcs: xpcs: Add special code to operate in Microchip KSZ9477 switch

> (To Tristram as well - I've added a workaround for your company mail
> sewers that don't accept bounces from emails that have left your
> organisation - in other words, once they have left your companies
> mail servers, you have no idea whether they reached their final
> recipient. You only get to know if your email servers can't send it
> to the very _next_ email server.)
> 
> On Wed, Jan 29, 2025 at 11:12:26PM +0200, Vladimir Oltean wrote:
> > 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.
> 
> Agreed. I know that the KSZ9477 information says differently, but if
> the implementation is the Synopsys DesignWare XPCS, then surely the
> register details in Synopsys' documentation should apply... so I'm
> also interested to know why KSZ9477 seems to deviate from Synopsys'
> implementation on this need.
> 
> I've been wondering whether setting DW_VR_MII_SGMII_LINK_STS and
> DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII in 1000base-X mode is something
> that should be done anyway, but from what Vladimir is saying, there's
> nothing in Synopsys' documentation that supports it.
> 
> The next question would be whether it's something that we _could_
> always do - if it has no effect for anyone else, then removing a
> conditional simplifies the code.

I explained in the other email that this SGMII_LINK_STS |
TX_CONFIG_PHY_SIDE_SGMII setting is only required for 1000BASEX where
C37_1000BASEX is used instead of C37_SGMII and auto-negotiation is
enabled.

This behavior only occurs in KSZ9477 with old IP and so may not reflect
in current specs.  If neg_mode can be set in certain way that disables
auto-negotiation in 1000BASEX mode but enables auto-negotiation in SGMII
mode then this setting is not required.

> > 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?
> 
> I think Tristram is talking about xpcs_config_aneg_c37_1000basex()
> here, not SGMII.
> 
> Tristram, as a general note: there is a reason I utterly hate the term
> "SGMII" - and the above illustrates exactly why. There is Cisco SGMII
> (the modified 1000base-X protocol for use with PHYs.) Then there is the
> "other" SGMII that manufacturers like to band about because they want
> to describe their "Serial Gigabit Media Independent Interface" and they
> use it to describe an interface that supports both 1000base-X and Cisco
> SGMII.
> 
> This overloading of "SGMII" leads to nothing but confusion - please be
> specific about whether you are talking about 1000base-X or Cisco SGMII,
> and please please please avoid using "SGMII".
> 
> However, in the kernel code, we use "SGMII" exclusively to mean Cisco
> SGMII.

I use the terms SGMII and 1000BASEX just like in Linux driver where there
are PHY_INTERFACE_MODE_SGMII and PHY_INTERFACE_MODE_1000BASEX, and it is
also how the SGMII module operates where there are two register settings
to use on these modes.  What is confusing is how to call the SFPs using
which mode.

All the fiber transceivers like 1000Base-SX and 1000Base-LX operate in
1000BASEX mode.

All 10/100/1000Base-T SFPs I tested operate in SGMII mode.

All 1000Base-T SFPs with RJ45 connector operate in 1000BASEX mode at the
beginning.  If a PHY is found inside (typically Marvell) that PHY driver
can change the mode to SGMII.  If that PHY driver is forced to not change
it to SGMII mode then 1000BASEX mode can still be used.

The major difference between 1000BASEX and SGMII modes in KSZ9477 is
there are link up and link down interrupts in SGMII mode but only link up
interrupt in 1000BASEX mode.  The phylink code can use the SFP cage logic
to detect the fiber cable is unplugged and say the link is down, so that
may be why the implementation behaves like that, but that does not work
for 1000Base-T SFP that operates in 1000BASEX mode.
 
> > > 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.
> 
> Hang on one moment... I think we're going off to another problem.
> 
> For 1000base-X, we do use phylink_mii_c22_pcs_encode_advertisement()
> which will generate the advertisement word for 1000base-X.
> 
> For Cisco SGMII, it will generate the tx_config word for a MAC-side
> setup (which is basically the fixed 0x4001 value.) From what I read
> in KSZ9477, this value would be unsuitable for a case where the
> following register values are:
> 
>         DW_VR_MII_PCS_MODE_C37_SGMII set
>         DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII set
>         DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL clear
> 
> meaning that we're generating a SGMII PHY-side word indicating the
> parameters to be used from the registers rather than hardware signals.
> 
> > > 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.

TX_CONFIG_PHY_SIDE_SGMII is never set for C37_SGMII mode.
Again I am not sure how this problem can be triggered.  I was just told
to set this value.  And a different chip with new IP has this value by
default.

> > > 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.
> 
> I think you're getting confused by microchip terminology.
> 
> Cisco SGMII is an asymmetric protocol. Cisco invented it as a way of:
> 1. supporting 10M and 100M speeds over a single data pair in each
>    direction.
> 2. sending the parameters of that link from the PHY to the MAC/PCS over
>    that single data pair.
> 
> They took the IEEE 1000base-X specification as a basis (which is
> symmetric negotiation via a 16-bit word).
> 
> The Cisco SGMII configuration word from the PHY to the PCS/MAC
> contains:
> 
>         bit 15 - link status
>         bit 14 - (reserved for AN acknowledge as per 1000base-X)
>         bit 13 - reserved (zero)
>         bit 12 - duplex mode
>         bit 11, 10 - speed
>         bits 9..1 - reserved (zero)
>         bit 0 - set to 1
> 
> This is "PHY" mode, or in Microchip parlence "master" mode - because
> the PHY is dictating what the other end should be doing.
> 
> When the PCS/MAC receives this, the PCS/MAC is expected to respond
> with a configuration word containing:
> 
>         bit 15 - zero
>         bit 14 - set to 1 (indicating acknowledge)
>         bit 13..1 - zero
>         bit 0 - set to 1
> 
> This is MAC mode, or in Microchip parlence "slave" mode - because the
> MAC is being told what it should do.
> 
> So, for a Cisco SGMII link with a SFP module which has a PHY embedded
> inside, you definitely want to be using MAC mode, because the PHY on
> the SFP module will be dictating the speed and duplex to the components
> outside of the SFP - in other words the PCS and MAC.

I do not know the internal working in the SGMII module where the
registers may have incorrect values.  I only verify the SGMII port is
working by sending and receiving traffic after changing some registers.

> > > 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.
> 
> I don't have any modules that have a Finisar PHY rather than a Marvell
> PHY. I wonder if the problem is that the Finisar module doesn't like
> multi-byte I2C accesses to the PHY.
> 
> Another thing - make sure that the I2C bus to the SFP cage is running
> at 100kHz, not 400kHz.

What I meant is this Marvell PHY ID 0x01ff0cc0.  Basically it is
0x01410cc0 for Marvell 88E1111 with 0x41 replaced with 0xff.  Some
registers like the status register even has 0xff value all the time so
the PHY can never report the link is down.

Powered by blists - more mailing lists