netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Oros <poros@redhat.com>
To: netdev@vger.kernel.org
Cc: andrew@lunn.ch, f.fainelli@gmail.com, hkallweit1@gmail.com,
	ivecera@redhat.com
Subject: [PATCH net-next] phy: avoid unnecessary link-up delay in polling mode
Date: Tue, 18 Feb 2020 10:35:55 +0100	[thread overview]
Message-ID: <20200218093555.948922-1-poros@redhat.com> (raw)
In-Reply-To: <20200129101308.74185-1-poros@redhat.com>

commit 93c0970493c71f ("net: phy: consider latched link-down status in
polling mode") removed double-read of latched link-state register for
polling mode from genphy_update_link(). This added extra ~1s delay into
sequence link down->up.
Following scenario:
 - After boot link goes up
 - phy_start() is called triggering an aneg restart, hence link goes
   down and link-down info is latched.
 - After aneg has finished link goes up. In phy_state_machine is checked
   link state but it is latched "link is down". The state machine is
   scheduled after one second and there is detected "link is up". This
   extra delay can be avoided when we keep link-state register double read
   in case when link was down previously.

With this solution we don't miss a link-down event in polling mode and
link-up is faster.

Details about this quirky behavior on Realtek phy:
Without patch:
T0:    aneg is started, link goes down, link-down status is latched
T0+3s: state machine runs, up-to-date link-down is read
T0+4s: state machine runs, aneg is finished (BMSR_ANEGCOMPLETE==1),
       here i read link-down (BMSR_LSTATUS==0),
T0+5s: state machine runs, aneg is finished (BMSR_ANEGCOMPLETE==1),
       up-to-date link-up is read (BMSR_LSTATUS==1),
       phydev->link goes up, state change PHY_NOLINK to PHY_RUNNING

With patch:
T0:    aneg is started, link goes down, link-down status is latched
T0+3s: state machine runs, up-to-date link-down is read
T0+4s: state machine runs, aneg is finished (BMSR_ANEGCOMPLETE==1),
       first BMSR read: BMSR_ANEGCOMPLETE==1 and BMSR_LSTATUS==0,
       second BMSR read: BMSR_ANEGCOMPLETE==1 and BMSR_LSTATUS==1,
       phydev->link goes up, state change PHY_NOLINK to PHY_RUNNING

Signed-off-by: Petr Oros <poros@redhat.com>
---
 drivers/net/phy/phy-c45.c    | 5 +++--
 drivers/net/phy/phy_device.c | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index a1caeee1223617..bceb0dcdecbd61 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -239,9 +239,10 @@ int genphy_c45_read_link(struct phy_device *phydev)
 
 		/* 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.
+		 * in polling mode to detect such short link drops except
+		 * the link was already down.
 		 */
-		if (!phy_polling_mode(phydev)) {
+		if (!phy_polling_mode(phydev) || !phydev->link) {
 			val = phy_read_mmd(phydev, devad, MDIO_STAT1);
 			if (val < 0)
 				return val;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2a973265de8095..be0129231c2a1c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1930,9 +1930,10 @@ int genphy_update_link(struct phy_device *phydev)
 
 	/* 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.
+	 * in polling mode to detect such short link drops except
+	 * the link was already down.
 	 */
-	if (!phy_polling_mode(phydev)) {
+	if (!phy_polling_mode(phydev) || !phydev->link) {
 		status = phy_read(phydev, MII_BMSR);
 		if (status < 0)
 			return status;
-- 
2.24.1


  parent reply	other threads:[~2020-02-18  9:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-29 10:13 [PATCH net] phy: avoid unnecessary link-up delay in polling mode Petr Oros
2020-01-29 11:18 ` Ivan Vecera
2020-01-29 12:06 ` Ivan Vecera
2020-01-29 12:15   ` David Miller
2020-01-29 12:23     ` Petr Oros
2020-01-29 12:19 ` [PATCH net v2] " Petr Oros
2020-01-29 21:01   ` Heiner Kallweit
2020-01-31 15:09     ` Petr Oros
2020-01-31 20:50       ` Heiner Kallweit
2020-02-01 10:25         ` Ivan Vecera
2020-02-01 20:26           ` Heiner Kallweit
2020-02-02 14:49             ` Ivan Vecera
2020-02-18  9:35 ` Petr Oros [this message]
2020-02-18 20:45   ` [PATCH net-next] " Heiner Kallweit
2020-02-20  0:17   ` David Miller

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=20200218093555.948922-1-poros@redhat.com \
    --to=poros@redhat.com \
    --cc=andrew@lunn.ch \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=ivecera@redhat.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).