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:

quoted

On Tue, Jun 04, 2024 at 11:29:43AM +0000, si.yanteng@linux.dev wrote:

quoted

2024年5月30日 15:22, "Russell King (Oracle)" [off-list ref] 写到:

Hi, Russell, Serge,

quoted

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!