Date: Fri, 23 Feb 2024 06:58:55 +0000
From: "Voon, Weifeng" <weifeng.voon@...el.com>
To: Russell King <linux@...linux.org.uk>, Choong Yong Liang
	<yong.liang.choong@...ux.intel.com>
CC: Rajneesh Bhardwaj <irenic.rajneesh@...il.com>, David E Box
	<david.e.box@...ux.intel.com>, Hans de Goede <hdegoede@...hat.com>, "Mark
 Gross" <markgross@...nel.org>, Alexandre Torgue
	<alexandre.torgue@...s.st.com>, Jose Abreu <Jose.Abreu@...opsys.com>, "David
 S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, "Jakub
 Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Maxime Coquelin
	<mcoquelin.stm32@...il.com>, Richard Cochran <richardcochran@...il.com>,
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
	Jesper Dangaard Brouer <hawk@...nel.org>, John Fastabend
	<john.fastabend@...il.com>, Andrew Lunn <andrew@...n.ch>, Heiner Kallweit
	<hkallweit1@...il.com>, Philipp Zabel <p.zabel@...gutronix.de>, "Andrew
 Halaney" <ahalaney@...hat.com>, Serge Semin <fancer.lancer@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-stm32@...md-mailman.stormreply.com"
	<linux-stm32@...md-mailman.stormreply.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "platform-driver-x86@...r.kernel.org"
	<platform-driver-x86@...r.kernel.org>, "linux-hwmon@...r.kernel.org"
	<linux-hwmon@...r.kernel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
	"Sit, Michael Wei Hong" <michael.wei.hong.sit@...el.com>, "Lai, Peter Jun
 Ann" <peter.jun.ann.lai@...el.com>, "Abdul Rahim, Faizal"
	<faizal.abdul.rahim@...el.com>
Subject: RE: [PATCH net-next v5 1/9] net: phylink: provide
 mac_get_pcs_neg_mode() function

> > For instance, if the interface switches from 2500baseX to SGMII mode,
> > and the current link mode is MLO_AN_PHY, calling
> 'phylink_pcs_neg_mode'
> > would yield PHYLINK_PCS_NEG_OUTBAND. Since the MAC and PCS driver
> > require PHYLINK_PCS_NEG_INBAND_ENABLED, the
> 'mac_get_pcs_neg_mode'
> > function will calculate the mode based on the interface, current link
> > negotiation mode, and advertising link mode, returning
> > PHYLINK_PCS_NEG_OUTBAND to enable the PCS to configure the correct
> settings.
> 
> This paragraph doesn't make sense - at least to me. It first talks about
> requiring PHYLINK_PCS_NEG_INBAND_ENABLED when in SGMII mode. On
> this:

The example given here is a very specific condition and that probably why there are some confusions here. Basically, this patch provides an optional function for MAC driver to change the phy interface on-the-fly without the need of reinitialize the Ethernet driver. As we know that the 2500base-x is messy, in our case the 2500base-x does not support inband. To complete the picture, we are using SGMII c37 to handle speed 10/100/1000. Hence, to enable user to switch link speed from 2500 to 1000/100/10 and vice versa on-the-fly, the phy interface need to be configured to inband SGMII for speed 10/100/1000, and outband 2500base-x for speed 2500. Lastly, the newly introduced "mac_get_pcs_neg_mode"callback function enables MAC driver to reconfigure pcs negotiation mode to inband or outband based on the interface mode, current link negotiation mode, and advertising link mode.

> 
> 1) are you sure that the hardware can't be programmed for the SGMII
> symbol repititions?
> 

No, the HW can be program for SGMII symbol repetitions.

> 2) what happens if you're paired with a PHY (e.g. on a SFP module) which
> uses SGMII but has no capability of providing the inband data?
> (They do exist.) If your hardware truly does require inband data, it is going to
> be fundamentally inoperative with these modules.
> 

Above explanation should have already cleared your doubts. Inband or outband capability is configured based on the phy interface.

> Next, you then talk about returning PHYLINK_PCS_NEG_OUTBAND for the
> "correct settings". How does this relate to the first part where you basically
> describe the problem as SGMII requring inband? Basically the two don't
> follow.

It should be a typo mistake. SGMII should return PHYLINK_PCS_NEG_INBAND_ENABLED.

> 
> How, from a design point of view, because this fundamentally allows drivers
> to change how the system behaves, it will allow radically different behaviours
> for the same parameters between different drivers.
> I am opposed to that - I want to see a situation where we have uniform
> behaviour for the same configuration, and where hardware doesn't support
> something, we have some way to indicate that via some form of capabilities.
> 

Hi Russell,
If I understand you correctly, MAC driver should not interfere with pcs negotiation mode and it should be standardized in the generic function, e.g., phylink_pcs_neg_mode()?

> The issue of whether 2500base-X has inband or not is a long standing issue,
> and there are arguments (and hardware) that take totally opposing views on
> this. There is hardware where 2500base-X inband _must_ be used or the link
> doesn't come up. There is also hardware where 2500base-X inband is not
> "supported" in documentation but works in practice. There is also hardware
> where 2500base-X inband doesn't work. The whole thing is a total mess
> (thanks IEEE 802.3 for not getting on top of this early enough... and what's
> now stated in 802.3 for 2500base-X is now irrelevant because they were too
> late to the
> party.)
> 

Agreed. And I have also seen some of your comments regarding the 2500SGMII and 2500BASEX.

Powered by blists - more mailing lists