netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Maciej Żenczykowski" <zenczykowski@gmail.com>
To: "Maciej Żenczykowski" <maze@google.com>
Cc: netdev@vger.kernel.org, "Maciej Żenczykowski" <maze@google.com>
Subject: [PATCH] net-e1000e: Report carrier in loopback mode.
Date: Tue, 22 Nov 2011 12:34:51 -0800	[thread overview]
Message-ID: <1321994091-7589-1-git-send-email-zenczykowski@gmail.com> (raw)
In-Reply-To: <CAHo-OozdBn5jBNsRVzbCnmxozquJnTAYa0bNbK5VoaiKaK=7qQ@mail.gmail.com>

From: Maciej Żenczykowski <maze@google.com>

When loopback mode is forced on interface, and if the carrier check returns
negative, then force carrier check positive.  This is useful when interface
does not have carrier and test puts the interface in loopback mode.

While we're at it rework the code to fix other bugs in it.
---
 drivers/net/ethernet/intel/e1000e/e1000.h   |    4 +-
 drivers/net/ethernet/intel/e1000e/ich8lan.c |    4 +-
 drivers/net/ethernet/intel/e1000e/lib.c     |    2 +-
 drivers/net/ethernet/intel/e1000e/phy.c     |   98 +++++++++++++++++----------
 4 files changed, 68 insertions(+), 40 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 9fe18d1..93ffc39 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -620,8 +620,8 @@ extern s32 e1000e_write_kmrn_reg_locked(struct e1000_hw *hw, u32 offset,
 extern s32 e1000e_read_kmrn_reg(struct e1000_hw *hw, u32 offset, u16 *data);
 extern s32 e1000e_read_kmrn_reg_locked(struct e1000_hw *hw, u32 offset,
                                        u16 *data);
-extern s32 e1000e_phy_has_link_generic(struct e1000_hw *hw, u32 iterations,
-			       u32 usec_interval, bool *success);
+extern s32 e1000e_phy_has_link_generic(struct e1000_hw *hw, int intervals,
+			       u32 usec_interval, bool *link);
 extern s32 e1000e_phy_reset_dsp(struct e1000_hw *hw);
 extern void e1000_power_up_phy_copper(struct e1000_hw *hw);
 extern void e1000_power_down_phy_copper(struct e1000_hw *hw);
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index e2a80a2..955cd8c 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -681,7 +681,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
 	 * link.  If so, then we want to get the current speed/duplex
 	 * of the PHY.
 	 */
-	ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
+	ret_val = e1000e_phy_has_link_generic(hw, 0, 0, &link);
 	if (ret_val)
 		goto out;
 
@@ -3527,7 +3527,7 @@ static s32 e1000_kmrn_lock_loss_workaround_ich8lan(struct e1000_hw *hw)
 	 * Attempting this while link is negotiating fouled up link
 	 * stability
 	 */
-	ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
+	ret_val = e1000e_phy_has_link_generic(hw, 0, 0, &link);
 	if (!link)
 		return 0;
 
diff --git a/drivers/net/ethernet/intel/e1000e/lib.c b/drivers/net/ethernet/intel/e1000e/lib.c
index 0893ab1..e9b9e52 100644
--- a/drivers/net/ethernet/intel/e1000e/lib.c
+++ b/drivers/net/ethernet/intel/e1000e/lib.c
@@ -456,7 +456,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
 	 * link.  If so, then we want to get the current speed/duplex
 	 * of the PHY.
 	 */
-	ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
+	ret_val = e1000e_phy_has_link_generic(hw, 0, 0, &link);
 	if (ret_val)
 		return ret_val;
 
diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
index 8666476..f026cba 100644
--- a/drivers/net/ethernet/intel/e1000e/phy.c
+++ b/drivers/net/ethernet/intel/e1000e/phy.c
@@ -1768,46 +1768,74 @@ static s32 e1000_wait_autoneg(struct e1000_hw *hw)
 /**
  *  e1000e_phy_has_link_generic - Polls PHY for link
  *  @hw: pointer to the HW structure
- *  @iterations: number of times to poll for link
+ *  @intervals: number of times to delay between polls for link
  *  @usec_interval: delay between polling attempts
- *  @success: pointer to whether polling was successful or not
+ *  @link: pointer to whether link was present or not
  *
- *  Polls the PHY status register for link, 'iterations' number of times.
+ *  Polls the PHY status register for link, 'intervals + 1' number of times.
+ *  Max run time is approx 'intervals * usec_interval' microseconds.
  **/
-s32 e1000e_phy_has_link_generic(struct e1000_hw *hw, u32 iterations,
-			       u32 usec_interval, bool *success)
+s32 e1000e_phy_has_link_generic(struct e1000_hw *hw, int intervals,
+				u32 usec_interval, bool *link)
 {
-	s32 ret_val = 0;
-	u16 i, phy_status;
+	int good_reads_phy_status = 0;
+	bool loopback_checked = false;
+	s32 err;
+	u16 reg;
+
+	/* Remember that e1e_rphy may fail because of another entity
+	 * (like the firmware) holding the lock, we need to handle
+	 * this gracefully - by waiting and trying again.
+	 *
+	 * Some PHYs require the PHY_STATUS register to be
+	 * read twice due to the link bit being sticky.
+	 * No harm doing it across the board.
+	 *
+	 * This first initial read slightly improves the probability of
+	 * a successful double read of the PHY_STATUS on the first iteration.
+	 * (and thus also whenever this function is called with iterations == 0)
+	 */
+	err = e1e_rphy(hw, PHY_STATUS, &reg);
+	if (!err) ++good_reads_phy_status;
+
+	for (;;) {
+		err = e1e_rphy(hw, PHY_STATUS, &reg);
+		if (!err) {
+			if (++good_reads_phy_status < 2) continue;
+			if (reg & MII_SR_LINK_STATUS) {
+				*link = true;
+				return 0; /* success: link up */
+			}
+		}
+
+		if (!loopback_checked) {
+			/* If the interface is in loopback-mode... */
+			err = e1e_rphy(hw, PHY_CONTROL, &reg);
+			if (!err) {
+				loopback_checked = true;
+				if (reg & MII_CR_LOOPBACK) {
+					/* ... fake link up. */
+					*link = true;
+					return 0; /* success: link up */
+				}
+			}
+		}
+
+		if (--intervals < 0) {
+			/* timeout waiting for link to go up (or only errors) */
+			if (good_reads_phy_status < 2) {
+				if (!err) err = -E1000_ERR_PHY;
+				return err; /* failure */
+			}
+			*link = false;
+			return 0; /* success: link down */
+		}
 
-	for (i = 0; i < iterations; i++) {
-		/*
-		 * Some PHYs require the PHY_STATUS register to be read
-		 * twice due to the link bit being sticky.  No harm doing
-		 * it across the board.
-		 */
-		ret_val = e1e_rphy(hw, PHY_STATUS, &phy_status);
-		if (ret_val)
-			/*
-			 * If the first read fails, another entity may have
-			 * ownership of the resources, wait and try again to
-			 * see if they have relinquished the resources yet.
-			 */
-			udelay(usec_interval);
-		ret_val = e1e_rphy(hw, PHY_STATUS, &phy_status);
-		if (ret_val)
-			break;
-		if (phy_status & MII_SR_LINK_STATUS)
-			break;
 		if (usec_interval >= 1000)
-			mdelay(usec_interval/1000);
+			mdelay(usec_interval / 1000);
 		else
 			udelay(usec_interval);
 	}
-
-	*success = (i < iterations);
-
-	return ret_val;
 }
 
 /**
@@ -1943,7 +1971,7 @@ s32 e1000e_get_phy_info_m88(struct e1000_hw *hw)
 		return -E1000_ERR_CONFIG;
 	}
 
-	ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
+	ret_val = e1000e_phy_has_link_generic(hw, 0, 0, &link);
 	if (ret_val)
 		return ret_val;
 
@@ -2011,7 +2039,7 @@ s32 e1000e_get_phy_info_igp(struct e1000_hw *hw)
 	u16 data;
 	bool link;
 
-	ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
+	ret_val = e1000e_phy_has_link_generic(hw, 0, 0, &link);
 	if (ret_val)
 		return ret_val;
 
@@ -2071,7 +2099,7 @@ s32 e1000_get_phy_info_ife(struct e1000_hw *hw)
 	u16 data;
 	bool link;
 
-	ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
+	ret_val = e1000e_phy_has_link_generic(hw, 0, 0, &link);
 	if (ret_val)
 		goto out;
 
@@ -3298,7 +3326,7 @@ s32 e1000_get_phy_info_82577(struct e1000_hw *hw)
 	u16 data;
 	bool link;
 
-	ret_val = e1000e_phy_has_link_generic(hw, 1, 0, &link);
+	ret_val = e1000e_phy_has_link_generic(hw, 0, 0, &link);
 	if (ret_val)
 		goto out;
 
-- 
1.7.3.1

  reply	other threads:[~2011-11-22 20:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-21 23:19 [PATCH net-next v1 0/4] e1000e: ethtool setfeatures fixes + loopback David Decotigny
2011-11-21 23:19 ` [PATCH net-next v1 1/4] net-e1000e: fix ethtool set_features taking new features into account too late David Decotigny
2011-11-22  0:00   ` Michał Mirosław
2011-11-22  0:04     ` David Decotigny
2011-11-21 23:19 ` [PATCH net-next v1 2/4] net-e1000e: enable ethtool loopback support David Decotigny
2011-11-21 23:19 ` [PATCH net-next v1 3/4] net-e1000e: reworked carrier detection logic David Decotigny
2011-11-22 20:32   ` Maciej Żenczykowski
2011-11-22 20:34     ` Maciej Żenczykowski [this message]
2011-11-21 23:20 ` [PATCH net-next v1 4/4] net-e1000e: Report carrier in loopback mode David Decotigny
2011-11-22 20:49 ` [PATCH net-next v1 0/4] e1000e: ethtool setfeatures fixes + loopback David Miller
2011-11-22 20:49   ` 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=1321994091-7589-1-git-send-email-zenczykowski@gmail.com \
    --to=zenczykowski@gmail.com \
    --cc=maze@google.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).