Message-ID: <ZGLCAfbUjexCJ2+v@shell.armlinux.org.uk>
Date: Tue, 16 May 2023 00:36:33 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Daniel Machon <daniel.machon@...rochip.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Felix Fietkau <nbd@....name>,
	Florian Fainelli <f.fainelli@...il.com>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Horatiu Vultur <horatiu.vultur@...rochip.com>,
	Ioana Ciornei <ioana.ciornei@....com>,
	Jakub Kicinski <kuba@...nel.org>, John Crispin <john@...ozen.org>,
	Jose Abreu <Jose.Abreu@...opsys.com>,
	Lars Povlsen <lars.povlsen@...rochip.com>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Marcin Wojtas <mw@...ihalf.com>,
	Mark Lee <Mark-MC.Lee@...iatek.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>,
	Paolo Abeni <pabeni@...hat.com>, Sean Wang <sean.wang@...iatek.com>,
	Steen Hegelund <Steen.Hegelund@...rochip.com>,
	Taras Chornyi <taras.chornyi@...ision.eu>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	UNGLinuxDriver@...rochip.com, linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org, netdev@...r.kernel.org
Subject: Re: [PATCH RFC] Providing a helper for PCS inband negotiation

On Tue, May 16, 2023 at 01:08:33AM +0300, Vladimir Oltean wrote:
> On Mon, May 15, 2023 at 10:45:21PM +0100, Russell King (Oracle) wrote:
> > Clause 73.1:
> > 
> > So, my reading of these statements is that the _user_ should be
> > able to control via ethtool whether Clause 73 negotiation is
> > performed on a 10GBASE-KR (or any other backplane link that
> > uses clause 73 negotiation.) Having extracted that from 802.3,
> > I now believe it should be treated the same as 1000BASE-X, and
> > the Autoneg bit in ethtool should determine whether Clause 73
> > negotiation is used for 10GBASE-KR (and any other Clause 73
> > using protocol.)
> 
> Having said that copper backplane link modes should be treated "the
> same" as fiber link modes w.r.t. ethtool -s autoneg, it should also be
> said that there are significant differences between clause 37 and 73
> autoneg too.
> 
> Clause 73 negotiates the actual use of 10GBase-KR as a SERDES protocol
> through the copper backplane in favor of other "Base-K*" alternative
> link modes, so it's not quite proper to say that 10GBase-KR is a clause
> 73 using protocol.

I believe it is correct to state it as such, because:

72.1: Table 72–1—Physical Layer clauses associated with the 10GBASE-KR PMD

	73—Auto-Negotiation for Backplane Ethernet              Required

Essentially, 802.3 doesn't permit 10GBASE-KR without hardware support
for Clause 73 AN (but that AN doesn't have to be enabled by management
software.)

> To me, the goals of clause 73 autoneg are much more similar to those of
> the twisted pair autoneg process - clause 28, which similarly selects
> between different media side protocols in the PHY, using a priority
> resolution function. For those, we use phylib and the phy_device
> structure. What are the merits of using phylink_pcs for copper backplanes
> and not phylib?

I agree with you on that, because not only does that fit better with
our ethernet PHY model, but it also means PHY_INTERFACE_MODE_XLGMII
makes sense.

However, by that same token, 1000BASE-X should never have been a
PHY_INTERFACE_MODE_xxx, and this should also have been treated purely
as a PHY.

Taking that still further, this means SGMII, which is 1000BASE-X but
modified for Cisco's purposes, would effectively be a media converting
PHY sat between the MAC/RS and the "real" ethernet PHY. In this case,
PHY_INTERFACE_MODE_SGMII might make sense because the "real" ethernet
PHY needs to know that.

Then there's 1000BASE-X used to connect a "real" ethernet PHY to the
MAC/RS, which means 1000BASE-X can't really be any different from
SGMII.

This all makes the whole thing extremely muddy, but this deviates away
from the original topic, because we're now into a "what should we call
a PCS" vs "what should we call a PHY" discussion. Then we'll get into
a discussion about phylib, difficulties with net_device only being
able to have one phylib device, stacked PHYs, and phylib not being
able to cope with non-MDIO based devices that we find on embedded
platforms (some which don't even offer anything that approximates the
802.3 register set, so could never be a phylib driver.)

It even impacts on the DT description, since what does "managed =
"in-band-status";" mean if we start considering 1000base-X the same
way as 1000base-T and the "PHY" protocol for 1000base-X becomes GMII.
A GMII link has no inband AN, so "managed = "in-band-status";" at
that point makes no sense.

That is definitely a can of worms I do *not* want to open with this
discussion - and much of the above has a long history and considerably
pre-dates phylink.

My original question was more around: how do we decide what we
currently have as a PCS should use inband negotiation.

For SGMII and close relatives, and 1000BASE-X it's been obvious. For
2500BASE-X less so (due to vendors coming up with it before its been
standardised.)

We have implementations using this for other protocols, so it's
a question that needs answering for these other protocols.


However, if we did want to extend this topic, then there are a number
of questions that really need to be asked is about the XPCS driver.
Such as - what does 1000BASE-KX, 10000BASE-KX4, 10000BASE-KR and
2500BASE-X have to do with USXGMII, and why are there no copper
ethtool modes listed when a USXGMII link can definitely support
being connected to a copper PHY? (See xpcs_usxgmii_features[]).

Why does XPCS think that USXGMII uses Clause 73 AN? (see the first
entry in synopsys_xpcs_compat[].)

xpcs_sgmii_features[] only mentions copper linkmodes, but as we know,
it's common for copper PHYs to also support fibre with an auto-
selection mechanism. So, 1000BASE-X is definitely possible if SGMII
is supported, so why isn't it listed.

As previously said, 1000BASE-X can be connected to a PHY that does
1000BASE-T, so why does xpcs_1000basex_features[] not mention
1000baseT_Full... there's probably more here as well.

Interestingly, xpcs_2500basex_features[] _does_ mention both
2500BASE-X and 2500BASE-T, but I think that only does because I
happened to comment on it during a review.

I think xpcs is another can of worms, but is an easier can of worms
to solve than trying to sort out that "what's an ethernet PHY"
question we seem to be heading towards (which I think would be a
mammoth task, even back when phylink didn't exist, to sort out.)

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