Re: [PATCH] net: phy: fix auto-negotiation in case of 'down-shift'

On Tue, 2020-11-24 at 15:17 +0000, Russell King - ARM Linux admin wrote:

On Tue, Nov 24, 2020 at 04:03:40PM +0100, Heiner Kallweit wrote:

quoted

Am 24.11.2020 um 15:38 schrieb Antonio Borneo:

quoted

If the auto-negotiation fails to establish a gigabit link, the phy
can try to 'down-shift': it resets the bits in MII_CTRL1000 to
stop advertising 1Gbps and retries the negotiation at 100Mbps.

I see that Russell answered already. My 2cts:

Are you sure all PHY's supporting downshift adjust the
advertisement bits? IIRC an Aquantia PHY I dealt with does not.
And if a PHY does so I'd consider this problematic:
Let's say you have a broken cable and the PHY downshifts to
100Mbps. If you change the cable then the PHY would still negotiate
100Mbps only.

From what I've seen, that is not how downshift works, at least on
the PHYs I've seen.

When the PHY downshifts, it modifies the advertisement registers,
but it also remembers the original value. When the cable is
unplugged, it restores the setting to what was previously set.

In fact, at least rtl8211f is able to recover the original settings and
returns to 1Gbps once a decent cable gets plugged-in.

It is _far_ from nice, but the fact is that your patch that Antonio
identified has broken previously working support, something that I
brought up when I patched one of the PHY drivers that was broken by
this very same problem by your patch.

The idea to fix it for a general case was indeed triggered by the fact that
before commit 5502b218e001 this was the norm. I considered it as a
regression.

That said, _if_ the PHY has a way to read the resolved state rather
than reading the advertisement registers, that is what should be
used (as I said previously) rather than trying to decode the
advertisement registers ourselves. That is normally more reliable
for speed and duplex.

Wrt rtl8211f I don't have info other then the public datasheet, and there I
didn't found any way other than reading the advertisement register.

I have read the latest comment from Heiner. I will check aqr107!

Thanks
Antonio