Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net] net: phy: consider AN_RESTART status when reading link status
@ 2019-08-12 19:20 Heiner Kallweit
  2019-08-12 20:24 ` Andrew Lunn
  2019-08-14  2:57 ` Jakub Kicinski
  0 siblings, 2 replies; 3+ messages in thread
From: Heiner Kallweit @ 2019-08-12 19:20 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev, Yonglong Liu

After configuring and restarting aneg we immediately try to read the
link status. On some systems the PHY may not yet have cleared the
"aneg complete" and "link up" bits, resulting in a false link-up
signal. See [0] for a report.
Clause 22 and 45 both require the PHY to keep the AN_RESTART
bit set until the PHY actually starts auto-negotiation.
Let's consider this in the generic functions for reading link status.
The commit marked as fixed is the first one where the patch applies
cleanly.

[0] https://marc.info/?t=156518400300003&r=1&w=2

Fixes: c1164bb1a631 ("net: phy: check PMAPMD link status only in genphy_c45_read_link")
Tested-by: Yonglong Liu <liuyonglong@huawei.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy-c45.c    | 14 ++++++++++++++
 drivers/net/phy/phy_device.c | 12 +++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index b9d414578..58bb25e4a 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -219,6 +219,20 @@ int genphy_c45_read_link(struct phy_device *phydev)
 	int val, devad;
 	bool link = true;
 
+	if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
+		if (val < 0)
+			return val;
+
+		/* Autoneg is being started, therefore disregard current
+		 * link status and report link as down.
+		 */
+		if (val & MDIO_AN_CTRL1_RESTART) {
+			phydev->link = 0;
+			return 0;
+		}
+	}
+
 	while (mmd_mask && link) {
 		devad = __ffs(mmd_mask);
 		mmd_mask &= ~BIT(devad);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b039632de..163295dbc 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1741,7 +1741,17 @@ EXPORT_SYMBOL(genphy_aneg_done);
  */
 int genphy_update_link(struct phy_device *phydev)
 {
-	int status;
+	int status = 0, bmcr;
+
+	bmcr = phy_read(phydev, MII_BMCR);
+	if (bmcr < 0)
+		return bmcr;
+
+	/* Autoneg is being started, therefore disregard BMSR value and
+	 * report link as down.
+	 */
+	if (bmcr & BMCR_ANRESTART)
+		goto done;
 
 	/* The link state is latched low so that momentary link
 	 * drops can be detected. Do not double-read the status
-- 
2.22.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: phy: consider AN_RESTART status when reading link status
  2019-08-12 19:20 [PATCH net] net: phy: consider AN_RESTART status when reading link status Heiner Kallweit
@ 2019-08-12 20:24 ` Andrew Lunn
  2019-08-14  2:57 ` Jakub Kicinski
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Lunn @ 2019-08-12 20:24 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev, Yonglong Liu

On Mon, Aug 12, 2019 at 09:20:02PM +0200, Heiner Kallweit wrote:
> After configuring and restarting aneg we immediately try to read the
> link status. On some systems the PHY may not yet have cleared the
> "aneg complete" and "link up" bits, resulting in a false link-up
> signal. See [0] for a report.
> Clause 22 and 45 both require the PHY to keep the AN_RESTART
> bit set until the PHY actually starts auto-negotiation.
> Let's consider this in the generic functions for reading link status.
> The commit marked as fixed is the first one where the patch applies
> cleanly.
> 
> [0] https://marc.info/?t=156518400300003&r=1&w=2
> 
> Fixes: c1164bb1a631 ("net: phy: check PMAPMD link status only in genphy_c45_read_link")
> Tested-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: phy: consider AN_RESTART status when reading link status
  2019-08-12 19:20 [PATCH net] net: phy: consider AN_RESTART status when reading link status Heiner Kallweit
  2019-08-12 20:24 ` Andrew Lunn
@ 2019-08-14  2:57 ` Jakub Kicinski
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2019-08-14  2:57 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Andrew Lunn, Florian Fainelli, David Miller, netdev, Yonglong Liu

On Mon, 12 Aug 2019 21:20:02 +0200, Heiner Kallweit wrote:
> After configuring and restarting aneg we immediately try to read the
> link status. On some systems the PHY may not yet have cleared the
> "aneg complete" and "link up" bits, resulting in a false link-up
> signal. See [0] for a report.
> Clause 22 and 45 both require the PHY to keep the AN_RESTART
> bit set until the PHY actually starts auto-negotiation.
> Let's consider this in the generic functions for reading link status.
> The commit marked as fixed is the first one where the patch applies
> cleanly.

Queued for 5.1+, then.

> [0] https://marc.info/?t=156518400300003&r=1&w=2
> 
> Fixes: c1164bb1a631 ("net: phy: check PMAPMD link status only in genphy_c45_read_link")
> Tested-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied, thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-12 19:20 [PATCH net] net: phy: consider AN_RESTART status when reading link status Heiner Kallweit
2019-08-12 20:24 ` Andrew Lunn
2019-08-14  2:57 ` Jakub Kicinski

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox