Re: genphy_read_status() vs. 1000bT Pause capability

On Mon, 2017-03-27 at 21:14 -0700, Florian Fainelli wrote:

quoted

Doesn't fix my other problem with Pause in 1000bT land. Do you know if
that way of reflecting the pause capability by hijacking the old
LPA bits is widely implemented enough that we should put it in
genphy_read_status() ?

Not sure I follow you here? The link partner pause capability is
reflected in phydev->pause and phydev->asym_pause (yes, these are
terrible names) when the link is established. 

Right. The problem is that they aren't for some gigabit links :-)

Basically, in my setup with a PHY which uses genphy_read_status()
(like most of them), I never get those advertised despite the fact
that, I *think* they are supported by the other end (even after fixing
my side of the advertising).

I added a printk inside genphy_read_status() to inspect the result
of the negociation, and this is what I read:

lpa=c1e1 lpagb=3800 adv=de1 common_adv=1e1 common_adv_gb=800

As you can see, LPA doesn't have the Pause bits. *However* it does
have bit 0x80 which can mean ADVERTISE_100HALF, but according to
our own mii.h can also mean ADVERTISE_1000XPAUSE. Similarily it 
has bit 0x100 which can mean ADVERTISE_100FULL but also can mean
ADVERTISE_1000XPSE_ASYM.

In fact we appear to have two functions to interpret them as
such inn the non-uapi mii.h:

	ethtool_adv_to_mii_adv_x
	mii_adv_to_ethtool_adv_x

However they aren't used much in the tree and not at all by the
"genphy" code.

So my question is... when we observe that we have a 1000 link
established, should we use these to "interpret" the LPA bits as
above ?

As it is, we never seem to advertise the capability because we never
decode the above (tried with 2 different PHYs, a Realtek and a
Broadcom) while my Cisco switches, I think, do support Pause.

Hence my question ... how "standard" is the re-use of the LPA bits
for these alternate meanings in 1000bT and should we update genphy
to perform that decoding ?

(I'm trying to download the 802.3 document referenced in the phy.txt
to see if it says anything about it but it's taking forever for some
reason).

Cheers,
Ben.

On Tue, 2017-03-28 at 16:17 +1100, Benjamin Herrenschmidt wrote:

Hence my question ... how "standard" is the re-use of the LPA bits
for these alternate meanings in 1000bT and should we update genphy
to perform that decoding ?

And btw, I'm happy to provide patches if we agree on the approach :-)

Cheers,
Ben.

On Tue, Mar 28, 2017 at 04:17:18PM +1100, Benjamin Herrenschmidt wrote:

I added a printk inside genphy_read_status() to inspect the result
of the negociation, and this is what I read:

lpa=c1e1 lpagb=3800 adv=de1 common_adv=1e1 common_adv_gb=800

As you can see, LPA doesn't have the Pause bits. *However* it does
have bit 0x80 which can mean ADVERTISE_100HALF, but according to
our own mii.h can also mean ADVERTISE_1000XPAUSE. Similarily it 
has bit 0x100 which can mean ADVERTISE_100FULL but also can mean
ADVERTISE_1000XPSE_ASYM.

The 1000X definitions are for 1000BaseX (fiber), not for 1000BaseT
(copper).

I have a working setup here with gigabit pause.  The pause bits come
from bits 11 and 10, just like for older copper PHYs.  Your link
parter is indicating that it has no pause capability.  That means
you can't use pause.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

On Tue, 2017-03-28 at 10:42 +0100, Russell King - ARM Linux wrote:

The 1000X definitions are for 1000BaseX (fiber), not for 1000BaseT
(copper).

I have a working setup here with gigabit pause.  The pause bits come
from bits 11 and 10, just like for older copper PHYs.  Your link
parter is indicating that it has no pause capability.  That means
you can't use pause.

Interesting. I tried and it worked :-) Could be something funny in
the config of our switches.

Thanks for the explanation.

Cheers,
Ben.

On Tue, Mar 28, 2017 at 10:11:24PM +1100, Benjamin Herrenschmidt wrote:

On Tue, 2017-03-28 at 10:42 +0100, Russell King - ARM Linux wrote:

quoted

The 1000X definitions are for 1000BaseX (fiber), not for 1000BaseT
(copper).

I have a working setup here with gigabit pause.  The pause bits come
from bits 11 and 10, just like for older copper PHYs.  Your link
parter is indicating that it has no pause capability.  That means
you can't use pause.

Interesting. I tried and it worked :-) Could be something funny in
the config of our switches.

It could be that the switch supports pause frames, but it's not
advertised because there's some corner cases that it doesn't work.  I
know that SolidRun have run into switches that corrupt ethernet frames
when pause is used.  (I don't know off hand which they are, but I could
ask the question.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

On Tue, 2017-03-28 at 12:31 +0100, Russell King - ARM Linux wrote:

quoted

Interesting. I tried and it worked :-) Could be something funny in
the config of our switches.

It could be that the switch supports pause frames, but it's not
advertised because there's some corner cases that it doesn't work.  I
know that SolidRun have run into switches that corrupt ethernet frames
when pause is used.  (I don't know off hand which they are, but I could
ask the question.)

I think this is Cisco gear, I'll check with our lab guy today. In the
meantime I'll test my driver back to back with some other machines, I
should find something that does pause eventually :-)

It's handy when you have a gigabit MAC on a 400Mhz ARM9 to be able to
throttle the peer despite all the other problems with Pause :-)

Cheers,
Ben.