netdev - Re: [PATCH net-next v13 12/15] net: stmmac: Fixed failure to set network speed to 1000.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZoWex6T0QbRBmDFE@shell.armlinux.org.uk>
Date: Wed, 3 Jul 2024 19:56:07 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Serge Semin <fancer.lancer@...il.com>
Cc: andrew@...n.ch, 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 Wed, Jul 03, 2024 at 07:56:31PM +0300, Serge Semin wrote:
> 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.

Oh damn, I did a reply, then cocked up sending it (lost it instead!)
So, this is going to be a brief response now.

It seems AN is basically required for 1000base-T.

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

The Atheros PHYs are another PHY where we should not be disabling
AN when wishing to use 1000base-T (so says the datasheet - I did
quote it in my original reply but lost that...)

As has already been mentioned, Marvell Alaska takes an interesting
approach - when BMCR AN enable is cleared but speed is forced to
1000, it internally keeps AN enabled and advertises the appropriate
1G speed + duplex capability bit depending on the BMCR duplex bit.

Rather than erroring out, I think it may be better to just adopt
the Marvell solution to this problem to give consistent behaviour
across all PHYs.

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