Message-ID: <20181215103736.GG26090@n2100.armlinux.org.uk>
Date:   Sat, 15 Dec 2018 10:37:36 +0000
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Yunsheng Lin <linyunsheng@...wei.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Weiwei Deng <dengweiwei@...wei.com>,
        "Yisen.Zhuang@...wei.com" <Yisen.Zhuang@...wei.com>,
        "huangdaode@...ilicon.com" <huangdaode@...ilicon.com>,
        "lipeng321@...wei.com" <lipeng321@...wei.com>,
        "salil.mehta@...wei.com" <salil.mehta@...wei.com>,
        lijianhua 00216010 <lijianhua@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Question: pause mode disabled for marvell 88e151x phy

On Sat, Dec 15, 2018 at 04:07:42PM +0800, Yunsheng Lin wrote:
> There seems to be some problem with pause subsequent negotiation.
> We reverted the above patch and tried to reproduce the above problem
> by triggering another negotiation by reconnection of the cable, using
> ethtool -a cmd shows both still have tx and rx pause enable.

That's where the problem is - as far as the network device and Linux
is concerned, pause was successfully negotiated.  However, as the
advertisment register has ended up with the pause mode bits cleared,
Linux doesn't realise that what we conveyed to the partner was an
advertisment containing no pause mode bits.

ethtool doesn't read the PHY advertisment register when displaying
what we advertised, it returns what's in phydev->advertising - it
gives you the cached value not the this-is-what-the-hardware-is-doing
value.

> 1. Does all the 88e151x supporting SGMII-to-Copper have the above problem?

Unknown.

> 2. If not, can we use revision id field in phydev->phy_id to only disable
>    the pause support for specific 88e151x phy? We can not find some useful
>    revision info in datasheet, and by printing the phy id when phy init, we
>    are able to find that the phy we are using has a phy id as 0x1d10dd1,
>    which has revision id as 0x1.

0x01d10dd1 doesn't look to be a Marvell part - Marvell parts generally
start with 0x0141....  Is your 0x1d1 a typo?  My device is 0x01410dd1.

> 3. Does this problem only happen marvel 88e1512 phy with some specific partner
>    phy? We are unable to reproduce this problem, so any suggestion to reproduce
>    this would be very helpful to us too.

I don't think you've proven that you do not have a problem (see below
for how to do this.)

> 4. Also the commit disables the pause support completely, if using revision id
>    can not aviod this problem, can we only disable pause support when negotiation
>    by only clearing pause support in phydev->advertising, but not phydev->supported?

No comment at present.


I think you first need to ensure that your observations are correct.
You are basing your assumptions on ethtool -a's output, which is
definitely wrong as I've mentioned above.

You need to read directly from the hardware using mii-diag -v ethN
and manually decode the advertisment register (register 4) checking
bits 11 and 10 (the pause mode bits).  My observation is that Linux
can set these bits, but then both bits clear during the negotiation
process.

If you then find that these bits do not clear, then we need further
research to work out why there's a different behaviour between the
device I've tested here and the device that you are testing.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Powered by blists - more mailing lists