public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Russell King - ARM Linux admin <linux@armlinux•org.uk>
To: Heiner Kallweit <hkallweit1@gmail•com>
Cc: Andrew Lunn <andrew@lunn•ch>,
	Florian Fainelli <f.fainelli@gmail•com>,
	David Miller <davem@davemloft•net>,
	"netdev@vger•kernel.org" <netdev@vger•kernel.org>
Subject: Re: [PATCH net-next 3/3] net: phy: move handling latched link-down to phylib state machine
Date: Tue, 28 May 2019 14:15:24 +0100	[thread overview]
Message-ID: <20190528131524.unl7uvgzurcppu7s@shell.armlinux.org.uk> (raw)
In-Reply-To: <b79f49f8-a42b-11c1-f83e-c198fee49dab@gmail.com>

On Mon, May 27, 2019 at 08:29:45PM +0200, Heiner Kallweit wrote:
> Especially with fibre links there may be very short link drops. And if
> interrupt handling is slow we may miss such a link drop. To deal with
> this we remove the double link status read from the generic link status
> read functions, and call the state machine twice instead.
> The flag for double-reading link status can be set by phy_mac_interrupt
> from hard irq context, therefore we have to use an atomic operation.

I came up with a different solution to this - I haven't extensively
tested it yet though:

 drivers/net/phy/phy-c45.c    | 12 ------------
 drivers/net/phy/phy.c        | 29 +++++++++++++++++++----------
 drivers/net/phy/phy_device.c | 14 --------------
 3 files changed, 19 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 9e24d9569424..756d7711cbc5 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -222,18 +222,6 @@ int genphy_c45_read_link(struct phy_device *phydev)
 		devad = __ffs(mmd_mask);
 		mmd_mask &= ~BIT(devad);
 
-		/* The link state is latched low so that momentary link
-		 * drops can be detected. Do not double-read the status
-		 * in polling mode to detect such short link drops.
-		 */
-		if (!phy_polling_mode(phydev)) {
-			val = phy_read_mmd(phydev, devad, MDIO_STAT1);
-			if (val < 0)
-				return val;
-			else if (val & MDIO_STAT1_LSTATUS)
-				continue;
-		}
-
 		val = phy_read_mmd(phydev, devad, MDIO_STAT1);
 		if (val < 0)
 			return val;
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7b3c5eec0129..2e7f0428e8fa 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -507,20 +507,29 @@ static int phy_config_aneg(struct phy_device *phydev)
  */
 static int phy_check_link_status(struct phy_device *phydev)
 {
-	int err;
+	int err, i;
 
 	WARN_ON(!mutex_is_locked(&phydev->lock));
 
-	err = phy_read_status(phydev);
-	if (err)
-		return err;
+	/* The link state is latched low so that momentary link drops can
+	 * be detected. If the link has failed, re-read the link status
+	 * to ensure that we are up to date with the current link state,
+	 * while notifying that the link status has changed.
+	 */
+	for (i = 0; i < 2; i++) {
+		err = phy_read_status(phydev);
+		if (err)
+			return err;
 
-	if (phydev->link && phydev->state != PHY_RUNNING) {
-		phydev->state = PHY_RUNNING;
-		phy_link_up(phydev);
-	} else if (!phydev->link && phydev->state != PHY_NOLINK) {
-		phydev->state = PHY_NOLINK;
-		phy_link_down(phydev, true);
+		if (phydev->link && phydev->state != PHY_RUNNING) {
+			phydev->state = PHY_RUNNING;
+			phy_link_up(phydev);
+		} else if (!phydev->link && phydev->state != PHY_NOLINK) {
+			phydev->state = PHY_NOLINK;
+			phy_link_down(phydev, true);
+		}
+		if (phydev->link)
+			break;
 	}
 
 	return 0;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 77068c545de0..ccc292c0f585 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1704,20 +1704,6 @@ int genphy_update_link(struct phy_device *phydev)
 {
 	int status;
 
-	/* The link state is latched low so that momentary link
-	 * drops can be detected. Do not double-read the status
-	 * in polling mode to detect such short link drops.
-	 */
-	if (!phy_polling_mode(phydev)) {
-		status = phy_read(phydev, MII_BMSR);
-		if (status < 0) {
-			return status;
-		} else if (status & BMSR_LSTATUS) {
-			phydev->link = 1;
-			return 0;
-		}
-	}
-
 	/* Read link and autonegotiation status */
 	status = phy_read(phydev, MII_BMSR);
 	if (status < 0)
-- 
2.7.4


-- 
RMK's Patch system: https://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

  reply	other threads:[~2019-05-28 13:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-27 18:26 [PATCH net-next 0/3] net: phy: improve handling of more complex C45 PHY's Heiner Kallweit
2019-05-27 18:28 ` [PATCH net-next 1/3] net: phy: export phy_queue_state_machine Heiner Kallweit
2019-05-27 19:19   ` Florian Fainelli
2019-05-28 13:10   ` Russell King - ARM Linux admin
2019-05-27 18:28 ` [PATCH net-next 2/3] net: phy: add callback for custom interrupt handler to struct phy_driver Heiner Kallweit
2019-05-27 19:25   ` Florian Fainelli
2019-05-27 19:36     ` Heiner Kallweit
2019-05-28 19:37       ` Florian Fainelli
2019-05-28 20:08         ` Heiner Kallweit
2019-05-28 13:12   ` Russell King - ARM Linux admin
2019-05-27 18:29 ` [PATCH net-next 3/3] net: phy: move handling latched link-down to phylib state machine Heiner Kallweit
2019-05-28 13:15   ` Russell King - ARM Linux admin [this message]
2019-05-28 18:22     ` Heiner Kallweit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190528131524.unl7uvgzurcppu7s@shell.armlinux.org.uk \
    --to=linux@armlinux$(echo .)org.uk \
    --cc=andrew@lunn$(echo .)ch \
    --cc=davem@davemloft$(echo .)net \
    --cc=f.fainelli@gmail$(echo .)com \
    --cc=hkallweit1@gmail$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox