Re: [PATCH net-next 3/6] net: dsa: add support for retrieving the interface mode

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: "Marek Behún" <kabel@kernel.org>, "Andrew Lunn" <andrew@lunn.ch>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Alvin __ipraga" <alsi@bang-olufsen.dk>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Claudiu Manoil" <claudiu.manoil@nxp.com>,
	"Daniel Scally" <djrscally@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	"DENG Qingfang" <dqfext@gmail.com>,
	"Eric Dumazet" <edumazet@google.com>,
	"Florian Fainelli" <f.fainelli@gmail.com>,
	"George McCollister" <george.mccollister@gmail.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Hauke Mehrtens" <hauke@hauke-m.de>,
	"Heikki Krogerus" <heikki.krogerus@linux.intel.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Kurt Kanzenbach" <kurt@linutronix.de>,
	"Landen Chao" <Landen.Chao@mediatek.com>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	netdev@vger.kernel.org, "Paolo Abeni" <pabeni@redhat.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Sean Wang" <sean.wang@mediatek.com>,
	UNGLinuxDriver@microchip.com,
	"Vivien Didelot" <vivien.didelot@gmail.com>,
	"Woojung Huh" <woojung.huh@microchip.com>
Subject: Re: [PATCH net-next 3/6] net: dsa: add support for retrieving the interface mode
Date: Fri, 22 Jul 2022 09:28:08 +0100	[thread overview]
Message-ID: <YtpfmF37FmfY6BV5@shell.armlinux.org.uk> (raw)
In-Reply-To: <20220721213645.57ne2jf7f6try4ec@skbuf>

On Fri, Jul 22, 2022 at 12:36:45AM +0300, Vladimir Oltean wrote:
> On Thu, Jul 21, 2022 at 10:14:00PM +0100, Russell King (Oracle) wrote:
> > > > So currently we try to enable C37 AN in 2500base-x mode, although
> > > > the standard says that it shouldn't be there, and it shouldn't be there
> > > > presumably because they want it to work with C73 AN.
> > > > 
> > > > I don't know how to solve this issue. Maybe declare a new PHY interface
> > > > mode constant, 2500base-x-no-c37-an ?
> > > 
> > > So this is essentially what I'm asking, and you didn't necessarily fully
> > > answer. I take it that there exist Marvell switches which enable in-band
> > > autoneg for 2500base-x and switches which don't, and managed = "in-band-status"
> > > has nothing to do with that decision. Right?
> > 
> > I think we're getting a little too het up over this.
> 
> No, I think it's relevant to this patch set.
> 
> > We have 1000base-X where, when we're not using in-band-status, we don't
> > use autoneg (some drivers that weren't caught in review annoyingly do
> > still use autoneg, but they shouldn't). We ignore the ethtool autoneg
> > bit.
> > 
> > We also have 1000base-X where we're using in-band-status, and we then
> > respect the ethtool autoneg bit.
> > 
> > So, wouldn't it be logical if 2500base-X were implemented the same way,
> > and on setups where 2500base-X does not support clause 37 AN, we
> > clear the ethtool autoneg bit? If we have 2500base-X being used as the
> > media link, surely this is the right behaviour?
> 
> The ethtool autoneg bit is only relevant when the PCS is the last thing
> before the medium. But if the SERDES protocol connects the MAC to the PHY,
> or the MAC to another MAC (such as the case here, CPU or DSA ports),
> there won't be any ethtool bit to take into consideration, and that's
> where my question is. Is there any expected correlation between enabling
> in-band autoneg and the presence or absence of managed = "in-band-status"?

This topic is something I was looking at back in November 2021, trying
to work out what the most sensible way of indicating to a PCS whether
it should enable in-band or not:

http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=e4ea7d035e7e04e87dfd86702f59952e0cecc18d
http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=e454bf101fa457dd5c2cea0b1aaab7ba33048089
http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=e2c57490f205ae7c0e11fcf756675937f933be5e

The intention there was to move the decision about whether a PCS should
enable autoneg out of the PCS and into phylink, but doing that one comes
immediately on the problem of (e.g.) Marvell NETA/PP2 vs Lynx having
different interpretations for 2500base-X. There are also a number of
drivers that do not follow MLO_AN_INBAND-means-use-inband or not for
things such as SGMII or 1000base-X.

This means we have no standard interpretation amongst phylink users
about when in-band signalling should be enabled or disabled, which
means moving that decision into phylink today isn't possible.

The only thing we could do is provide the PCS with an additional bit
of information so it can make the decision - something like a boolean
"pcs_connects_to_medium" flag, and keep the decision making in the
PCS-specific code - sadly keeping the variability between different
PCS implementations.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel