Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode

From: Andrew Lunn <andrew@lunn.ch>
To: Simon Horman <horms@verge.net.au>
Cc: David Miller <davem@davemloft.net>,
	netdev <netdev@vger.kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>
Subject: Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
Date: Thu, 20 Sep 2018 15:25:30 +0200	[thread overview]
Message-ID: <20180920132530.GB13632@lunn.ch> (raw)
In-Reply-To: <20180918105817.z2o5yybcth7diqsu@verge.net.au>

> 1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")
> 
> # mii-tool -vv eth0
> Using SIOCGMIIPHY=0x8947
> eth0: no link
>   registers for MII PHY 0: 
>     1140 7949 0022 1622 0d81 c1e1 000f 0000
>     0000 0300 0000 0000 0000 0000 0000 3000
>     0000 0000 0000 0078 7002 0000 0000 0200
>     0000 0000 0000 0528 0000 0000 0000 0000
>   product info: vendor 00:08:85, model 34 rev 2
>   basic mode:   autonegotiation enabled
>   basic status: no link
>   capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
>   advertising:  100baseTx-FD 100baseTx-HD flow-control
>   link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> 
> 2. net-next with this patch reverted
> 
> # mii-tool -vv eth0
> Using SIOCGMIIPHY=0x8947
> eth0: negotiated 100baseTx-FD, link ok
>   registers for MII PHY 0: 
>     1140 796d 0022 1622 0181 c1e1 000f 0000

Hi Simon

Word 5 is what we are advertising. Bits 10 and 11 are Pause and Asym
Pause. In the good case here, neither are set. In this bad case above,
both bits are set.

The patch i asked you to try only cleared the Pause bit, not the
Asymmetric Pause bit. mii-tool only saying 'flow-control' did not
help.

Word 6 is what the partner is advertising. c1e1 indicates the partner
does not support flow control, both bits are 0. I don't see why this
is preventing auto-net though. But in the bad case, the status
register indicates auto-neg has not completed.

Anyway, please can you try this patch, which also removes Aysm Pause.

Thanks
	Andrew

>From 00a061304af51831ca1dc86bf6ce23d01f724229 Mon Sep 17 00:00:00 2001
From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 18 Sep 2018 18:12:54 -0500
Subject: [PATCH] ravb: Disable Pause Advertisement

The previous commit to ravb had the side effect of making the PHY
advertise Pause. This previously did not happen, and it appears the
MAC does not support Pause. By default, phydev->supported has Pause
enabled, but phydev->advertising does not. Rather than rely on this,
be explicit, and remove the Pause link mode.

Reported-by: Simon Horman <horms@verge.net.au>
Fixes: 41124fa64d4b ("net: ethernet: Add helper to remove a supported link mode")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/renesas/ravb_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index fb2a1125780d..b0f2612ad226 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1073,9 +1073,11 @@ static int ravb_phy_init(struct net_device *ndev)
 		netdev_info(ndev, "limited PHY to 100Mbit/s\n");
 	}
 
-	/* 10BASE is not supported */
+	/* 10BASE, Pause and Asym Pause is not supported */
 	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
 	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Pause_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
 
 	phy_attached_info(phydev);
 
-- 
2.19.0.rc1
