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

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.

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!