Messages in this thread | | | Date | Sun, 12 Apr 2020 19:03:36 +0200 | From | Clemens Gruber <> | Subject | Re: [PATCH] net: phy: marvell: Fix pause frame negotiation |
| |
On Sat, Apr 11, 2020 at 02:43:44PM +0100, Russell King - ARM Linux admin wrote: > The fiber code is IMHO very suspect; the decoding of the pause status > seems to be completely broken. However, I'm not sure whether anyone > actually uses that or not, so I've been trying not to touch it.
If the following table for the link partner advertisement is correct.. PAUSE ASYM_PAUSE MEANING 0 0 Link partner has no pause frame support 0 1 <- Link partner can TX pause frames 1 0 <-> Link partner can RX and TX pauses 1 1 -> Link partner can RX pause frames
..then I think both pause and asym_pause have to be assigned independently, like this: phydev->pause = !!(lpa & LPA_1000XPAUSE); phydev->asym_pause = !!(lpa & LPA_1000XPAUSE_ASYM);
(Using the defines from uapi mii.h instead of the redundant/combined LPA_PAUSE_FIBER etc. which can then be removed from marvell.c)
Currently, if LPA_1000XPAUSE_ASYM is set we do pause=1 and asym_pause=1 no matter if LPA_1000XPAUSE is set. This could lead us to mistake a link partner who can only send for one who can only receive pause frames. ^ Was this the problem you meant?
I saw that for the copper case and in other drivers, we first set the ETHTOOL_LINK_MODE_(Asym_)Pause_BIT bit in lp_advertising and then set phydev->(asym_)pause depending on the ETHTOOL_LINK_MODE_... bit. Do you agree that we should also set the ETHTOOL_ bits in the fiber case?
Does anybody have access to a Marvell PHY with 1000base-X Ethernet? (I only have a 88E1510 + 1000Base-T at the home office)
Thanks, Clemens
| |