Message-ID: <hdqpsuq7n4aalav7jtzttfksw5ct36alowsc65e72armjt2h67@shph7z32rbc6>
Date: Wed, 3 Jul 2024 19:56:31 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>, andrew@...n.ch
Cc: si.yanteng@...ux.dev, Huacai Chen <chenhuacai@...nel.org>, 
	Yanteng Si <siyanteng@...ngson.cn>, hkallweit1@...il.com, peppe.cavallaro@...com, 
	alexandre.torgue@...s.st.com, joabreu@...opsys.com, Jose.Abreu@...opsys.com, 
	guyinggang@...ngson.cn, netdev@...r.kernel.org, chris.chenfeiyang@...il.com
Subject: Re: [PATCH net-next v13 12/15] net: stmmac: Fixed failure to set
 network speed to 1000.

On Tue, Jul 02, 2024 at 04:08:05PM +0100, Russell King (Oracle) wrote:
> On Tue, Jul 02, 2024 at 01:31:44PM +0300, Serge Semin wrote:
> > On Tue, Jun 04, 2024 at 11:29:43AM +0000, si.yanteng@...ux.dev wrote:
> > > 2024年5月30日 15:22, "Russell King (Oracle)" <linux@...linux.org.uk> 写到:
> > > 
> > > Hi, Russell, Serge,
> > > 
> > > > I would like this patch to be held off until more thought can be put
> > > > 
> > > > into how to handle this without having a hack in the driver (stmmac
> > > > 
> > > > has too many hacks and we're going to have to start saying no to
> > > > 
> > > > these.)
> > > Yeah, you have a point there, but I would also like to hear Serge's opinion.
> > 
> > I would be really glad not to have so many hacks in the STMMAC driver.
> > It would have been awesome if we are able to find a solution without
> > introducing one more quirk in the common driver code.
> > 
> > I started digging into this sometime ago, but failed to come up with
> > any decent hypothesis about the problem nature. One of the glimpse
> > idea was that the loongson_gnet_fix_speed() method code might be somehow
> > connected with the problem. But I didn't have much time to go further
> > than that.
> > 
> > Anyway I guess we'll need to have at least two more patchset
> > re-spins to settle all the found issues. Until then we can keep
> > discussing the problem Yanteng experience on his device. Russell, do
> > you have any suggestion of what info Yanteng should provide to better
> > understand the problem and get closer to it' cause?
> 
> First question: is auto-negotiation required by 802.3 for 1000base-T?
> By "required" I mean "required to be used" not "required to be
> implemented". This is an important distinction, and 802.3 tends to be
> a bit wooly about the exact meaning. However, I think on balance the
> conclusion is that AN is mandatory _to be used_ for 1000base-T links.

One another statement in IEEE 802.3 C40 that implies the AN being
mandatory is that 1000BASE-T PHYs determine their MASTER or SLAVE part
during the Auto-Negotiation process. The part determines the clock
source utilized by the PHYs: "The MASTER PHY uses a local clock to
determine the timing of transmitter operations. The SLAVE PHY recovers
the clock from the received signal and uses it to determine the timing
of transmitter operations, i.e.," (40.1.3 Operation of 1000BASE-T)

So I guess that without Auto-negotiation the link just won't be
established due to the clocks missconfiguration.

> 
> Annex 40C's state diagrams seems to imply that mr_autoneg_enable
> (BMCR AN ENABLE) doesn't affect whether or not the AN state machines
> work for 1000base-T, and some PHY datasheets (e.g. Marvell Alaska)
> state that disabling mr_autoneg_enable leaves AN enabled but forced
> to 1G full duplex.
> 
> So, I'm thinking is that the ethtool interface should reject any
> request where we have a PHY supporting *only* base-T for gigabit
> speeds, where the user is requesting !AN && SPEED_1000 on the basis
> that AN is part of the link setup of 1000base-T links.
> 
> Maybe this should be a property of the struct phy_device so we can
> transition phylib and phylink to return an appropriate error to
> userspace?
> 

> Alternatively, maybe just implement the Marvell Alaska solution
> to this problem (if the user attempts to disable AN on a PHY
> supporting only base-T at gigabit speeds, then we silently force
> AN with SPEED_1000 and DUPLEX_FULL.

I am not that much knowledgable about the PHY-lib and PHY-link
internals, but if we get to establish that the standard indeed
implies the AN being mandatory, then this sounds like the least
harmful solution from the user-space point of view.

-SergeY

> 
> Andrew - seems to be an IEEE 802.3 requirement that's been missed
> in phylib, any thoughts?
> 
> -- 
> 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