Message-ID: <Z5tRM5TYuMeCPXb-@shell.armlinux.org.uk>
Date: Thu, 30 Jan 2025 10:15:15 +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: [WARNING: ATTACHMENT UNSCANNED]Re: [PATCH RFC net-next 1/2] net:
 pcs: xpcs: Add special code to operate in Microchip KSZ9477 switch

On Thu, Jan 30, 2025 at 04:50:18AM +0000, Tristram.Ha@...rochip.com wrote:
> > 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 sentence reads to me "if we want to use 1000base-X mode, and we
configure the XPCS to use 1000base-X rather than Cisco SGMII then
setting both SGMII_LINK_STS and TX_CONFIG_PHY_SIDE_SGMII bits are
required.

Thanks, that tells me nothing that I don't already know. I know full
well that hardware needs to be configured for 1000base-X mode to use
1000base-X negotiation. I also have been saying for some time that
KSZ9477 documetation states that these two "SGMII" bits need to be
set in 1000base-X mode.

This is not what I'm questioning here. What I am questioning is whether
we can set these two "SGMII" bits _unconditionally_ in the
xpcs_config_aneg_c37_1000basex() path in the driver without impacting
newer XPCS IPs.

> > > 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.

Useful to know, but its not always clear when discussing.

> 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.

I already know this.

> 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.

I already know this.

> 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.

At this point, given all the discussion that has occurred, I'm really
not sure how to take "only link up in 1000base-X mode" - whether you're
talking about using 1000base-X with the other side operating in Cisco
SGMII mode or whether you're talking about e.g. a fibre link.

So I'm going to say it clearly: never operate the link with dis-similar
negotiation protocols. Don't operate the link with 1000base-X at one end
and Cisco SGMII at the other end. It's wrong. It's incorrect. The
configuration words are different formats. The interpretation of the
configuration words are different. Don't do it. Am I clear?

If it's still that 1000base-X mode to another 1000base-X partner doesn't
generate a link-down interrupt, then you will have no option but to use
phylink's polling mode for all protocols with this version of XPCS.

> > 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'm at a loss to work out how to respond to this. You've cut the
context to which I was replying to, and instead included following
context. Confused.

> > > > 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.

The information I've provided is to aid you in understanding "master" and
"slave" mode SGMII. I'm trying to educate you. I'm not questioning
register values or anything like that. I'm giving you information to aid
your understanding of SGMII.

> > > > 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.

Which module does this happen with, and is it still available to buy
(can I get one to test with?)

-- 
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