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

From: Antonio Borneo <antonio.borneo@st.com>
To: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, <netdev@vger.kernel.org>,
	Yonglong Liu <liuyonglong@huawei.com>
Cc: Antonio Borneo <antonio.borneo@st.com>, <stable@vger.kernel.org>,
	<linuxarm@huawei.com>, Salil Mehta <salil.mehta@huawei.com>,
	<linux-stm32@st-md-mailman.stormreply.com>,
	<linux-kernel@vger.kernel.org>
Subject: [PATCH] net: phy: fix auto-negotiation in case of 'down-shift'
Date: Tue, 24 Nov 2020 15:38:48 +0100	[thread overview]
Message-ID: <20201124143848.874894-1-antonio.borneo@st.com> (raw)

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.

From commit 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode
in genphy_read_status") the content of MII_CTRL1000 is not checked
anymore at the end of the negotiation, preventing the detection of
phy 'down-shift'.
In case of 'down-shift' phydev->advertising gets out-of-sync wrt
MII_CTRL1000 and still includes modes that the phy have already
dropped. The link partner could still advertise higher speeds,
while the link is established at one of the common lower speeds.
The logic 'and' in phy_resolve_aneg_linkmode() between
phydev->advertising and phydev->lp_advertising will report an
incorrect mode.

Issue detected with a local phy rtl8211f connected with a gigabit
capable router through a two-pairs network cable.

After auto-negotiation, read back MII_CTRL1000 and mask-out from
phydev->advertising the modes that have been eventually discarded
due to the 'down-shift'.

Fixes: 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
Cc: stable@vger.kernel.org # v5.1+
Signed-off-by: Antonio Borneo <antonio.borneo@st.com>
Link: https://lore.kernel.org/r/478f871a-583d-01f1-9cc5-2eea56d8c2a7@huawei.com
---
To: Andrew Lunn <andrew@lunn.ch>
To: Heiner Kallweit <hkallweit1@gmail.com>
To: Russell King <linux@armlinux.org.uk>
To: "David S. Miller" <davem@davemloft.net>
To: Jakub Kicinski <kuba@kernel.org>
To: netdev@vger.kernel.org
To: Yonglong Liu <liuyonglong@huawei.com>
Cc: linuxarm@huawei.com
Cc: Salil Mehta <salil.mehta@huawei.com>
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-kernel@vger.kernel.org
Cc: Antonio Borneo <antonio.borneo@st.com>

 drivers/net/phy/phy_device.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 5dab6be6fc38..5d1060aa1b25 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2331,7 +2331,7 @@ EXPORT_SYMBOL(genphy_read_status_fixed);
  */
 int genphy_read_status(struct phy_device *phydev)
 {
-	int err, old_link = phydev->link;
+	int adv, err, old_link = phydev->link;
 
 	/* Update the link, but return if there was an error */
 	err = genphy_update_link(phydev);
@@ -2356,6 +2356,14 @@ int genphy_read_status(struct phy_device *phydev)
 		return err;
 
 	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
+		if (phydev->is_gigabit_capable) {
+			adv = phy_read(phydev, MII_CTRL1000);
+			if (adv < 0)
+				return adv;
+			/* update advertising in case of 'down-shift' */
+			mii_ctrl1000_mod_linkmode_adv_t(phydev->advertising,
+							adv);
+		}
 		phy_resolve_aneg_linkmode(phydev);
 	} else if (phydev->autoneg == AUTONEG_DISABLE) {
 		err = genphy_read_status_fixed(phydev);

base-commit: d549699048b4b5c22dd710455bcdb76966e55aa3
-- 
2.29.2

