[PATCH] e1000e: Fix link status in case of error.

Benjamin Poirier
2018-02-28T05:20:29Z

Before commit 19110cfbb34d ("e1000e: Separate signaling for link check/link
up"), errors which happen after "get_link_status = false" in the copper
check_for_link callbacks would be ignored and the link considered up. After
that commit, any error implies that the link is down. Since all
combinations of link up/down and error/no error are possible, do the same
thing as e1000e_phy_has_link_generic() and return the link status in a
separate variable.

Fixes: 19110cfbb34d ("e1000e: Separate signaling for link check/link up")
Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
drivers/net/ethernet/intel/e1000e/82571.c | 6 ++++--
drivers/net/ethernet/intel/e1000e/ethtool.c | 5 +++--
drivers/net/ethernet/intel/e1000e/hw.h | 2 +-
drivers/net/ethernet/intel/e1000e/ich8lan.c | 19 +++++++++++--------
drivers/net/ethernet/intel/e1000e/mac.c | 26 +++++++++++++++-----------
drivers/net/ethernet/intel/e1000e/mac.h | 6 +++---
drivers/net/ethernet/intel/e1000e/netdev.c | 11 +++--------
7 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/82571.c b/drivers/net/ethernet/intel/e1000e/82571.c
index 6b03c8553e59..980ed89e61ea 100644
--- a/drivers/net/ethernet/intel/e1000e/82571.c
+++ b/drivers/net/ethernet/intel/e1000e/82571.c
@@ -40,7 +40,8 @@
static s32 e1000_get_phy_id_82571(struct e1000_hw *hw);
static s32 e1000_setup_copper_link_82571(struct e1000_hw *hw);
static s32 e1000_setup_fiber_serdes_link_82571(struct e1000_hw *hw);
-static s32 e1000_check_for_serdes_link_82571(struct e1000_hw *hw);
+static s32 e1000_check_for_serdes_link_82571(struct e1000_hw *hw,
+ bool *unused);
static s32 e1000_write_nvm_eewr_82571(struct e1000_hw *hw, u16 offset,
u16 words, u16 *data);
static s32 e1000_fix_nvm_checksum_82571(struct e1000_hw *hw);
@@ -1493,6 +1494,7 @@ static s32 e1000_setup_fiber_serdes_link_82571(struct e1000_hw *hw)
/**
* e1000_check_for_serdes_link_82571 - Check for link (Serdes)
* @hw: pointer to the HW structure
+ * @unused: unused for serdes links
*
* Reports the link state as up or down.
*
@@ -1509,7 +1511,7 @@ static s32 e1000_setup_fiber_serdes_link_82571(struct e1000_hw *hw)
* 4) forced_up (the link has been forced up, it did not autonegotiate)
*
**/
-static s32 e1000_check_for_serdes_link_82571(struct e1000_hw *hw)
+static s32 e1000_check_for_serdes_link_82571(struct e1000_hw *hw, bool *unused)
{
struct e1000_mac_info *mac = &hw->mac;
u32 rxcw;
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 003cbd605799..1946ddae06c0 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -1753,6 +1753,7 @@ static int e1000_loopback_test(struct e1000_adapter *adapter, u64 *data)
static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)
{
struct e1000_hw *hw = &adapter->hw;
+ bool link_status;

*data = 0;
if (hw->phy.media_type == e1000_media_type_internal_serdes) {
@@ -1764,7 +1765,7 @@ static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)
* could take as long as 2-3 minutes
*/
do {
- hw->mac.ops.check_for_link(hw);
+ hw->mac.ops.check_for_link(hw, NULL);
if (hw->mac.serdes_has_link)
return *data;
msleep(20);
@@ -1772,7 +1773,7 @@ static int e1000_link_test(struct e1000_adapter *adapter, u64 *data)

*data = 1;
} else {
- hw->mac.ops.check_for_link(hw);
+ hw->mac.ops.check_for_link(hw, &link_status);
if (hw->mac.autoneg)
/* On some Phy/switch combinations, link establishment
* can take a few seconds more than expected.
diff --git a/drivers/net/ethernet/intel/e1000e/hw.h b/drivers/net/ethernet/intel/e1000e/hw.h
index d803b1a12349..4dff6df469bb 100644
--- a/drivers/net/ethernet/intel/e1000e/hw.h
+++ b/drivers/net/ethernet/intel/e1000e/hw.h
@@ -472,7 +472,7 @@ struct e1000_mac_operations {
s32 (*id_led_init)(struct e1000_hw *);
s32 (*blink_led)(struct e1000_hw *);
bool (*check_mng_mode)(struct e1000_hw *);
- s32 (*check_for_link)(struct e1000_hw *);
+ s32 (*check_for_link)(struct e1000_hw *, bool *);
s32 (*cleanup_led)(struct e1000_hw *);
void (*clear_hw_cntrs)(struct e1000_hw *);
void (*clear_vfta)(struct e1000_hw *);
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index ff308b05d68c..3d25255289ff 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -1363,15 +1363,14 @@ static s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force)
/**
* e1000_check_for_copper_link_ich8lan - Check for link (Copper)
* @hw: pointer to the HW structure
+ * @link_status: pointer to whether the link is up or not
*
* Checks to see of the link status of the hardware has changed. If a
* change in link status has been detected, then we read the PHY registers
* to get the current speed/duplex if link exists.
- *
- * Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link
- * up).
**/
-static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
+static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw,
+ bool *link_status)
{
struct e1000_mac_info *mac = &hw->mac;
s32 ret_val, tipg_reg = 0;
@@ -1384,8 +1383,11 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
* get_link_status flag is set upon receiving a Link Status
* Change or Rx Sequence Error interrupt.
*/
- if (!mac->get_link_status)
- return 1;
+ if (!mac->get_link_status) {
+ *link_status = true;
+ return 0;
+ }
+ *link_status = false;

/* First we want to see if the MII Status Register reports
* link. If so, then we want to get the current speed/duplex
@@ -1554,6 +1556,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
if (!link)
return 0; /* No link detected */

+ *link_status = true;
mac->get_link_status = false;

switch (hw->mac.type) {
@@ -1602,7 +1605,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
* we have already determined whether we have link or not.
*/
if (!mac->autoneg)
- return 1;
+ return 0;

/* Auto-Neg is enabled. Auto Speed Detection takes care
* of MAC speed/duplex configuration. So we only need to
@@ -1621,7 +1624,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
return ret_val;
}

- return 1;
+ return 0;
}

static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter)
diff --git a/drivers/net/ethernet/intel/e1000e/mac.c b/drivers/net/ethernet/intel/e1000e/mac.c
index db735644b312..5a5e219fc864 100644
--- a/drivers/net/ethernet/intel/e1000e/mac.c
+++ b/drivers/net/ethernet/intel/e1000e/mac.c
@@ -406,15 +406,13 @@ void e1000e_clear_hw_cntrs_base(struct e1000_hw *hw)
/**
* e1000e_check_for_copper_link - Check for link (Copper)
* @hw: pointer to the HW structure
+ * @link_status: pointer to whether the link is up or not
*
* Checks to see of the link status of the hardware has changed. If a
* change in link status has been detected, then we read the PHY registers
* to get the current speed/duplex if link exists.
- *
- * Returns a negative error code (-E1000_ERR_*) or 0 (link down) or 1 (link
- * up).
**/
-s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
+s32 e1000e_check_for_copper_link(struct e1000_hw *hw, bool *link_status)
{
struct e1000_mac_info *mac = &hw->mac;
s32 ret_val;
@@ -425,8 +423,11 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
* get_link_status flag is set upon receiving a Link Status
* Change or Rx Sequence Error interrupt.
*/
- if (!mac->get_link_status)
- return 1;
+ if (!mac->get_link_status) {
+ *link_status = true;
+ return 0;
+ }
+ *link_status = false;

/* First we want to see if the MII Status Register reports
* link. If so, then we want to get the current speed/duplex
@@ -439,6 +440,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
if (!link)
return 0; /* No link detected */

+ *link_status = true;
mac->get_link_status = false;

/* Check if there was DownShift, must be checked
@@ -450,7 +452,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
* we have already determined whether we have link or not.
*/
if (!mac->autoneg)
- return 1;
+ return 0;

/* Auto-Neg is enabled. Auto Speed Detection takes care
* of MAC speed/duplex configuration. So we only need to
@@ -469,17 +471,18 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
return ret_val;
}

- return 1;
+ return 0;
}

/**
* e1000e_check_for_fiber_link - Check for link (Fiber)
* @hw: pointer to the HW structure
+ * @unused: unused for fiber links
*
* Checks for link up on the hardware. If link is not up and we have
* a signal, then we need to force link up.
**/
-s32 e1000e_check_for_fiber_link(struct e1000_hw *hw)
+s32 e1000e_check_for_fiber_link(struct e1000_hw *hw, bool *unused)
{
struct e1000_mac_info *mac = &hw->mac;
u32 rxcw;
@@ -540,11 +543,12 @@ s32 e1000e_check_for_fiber_link(struct e1000_hw *hw)
/**
* e1000e_check_for_serdes_link - Check for link (Serdes)
* @hw: pointer to the HW structure
+ * @unused: unused for serdes links
*
* Checks for link up on the hardware. If link is not up and we have
* a signal, then we need to force link up.
**/
-s32 e1000e_check_for_serdes_link(struct e1000_hw *hw)
+s32 e1000e_check_for_serdes_link(struct e1000_hw *hw, bool *unused)
{
struct e1000_mac_info *mac = &hw->mac;
u32 rxcw;
@@ -833,7 +837,7 @@ static s32 e1000_poll_fiber_serdes_link_generic(struct e1000_hw *hw)
* link up if we detect a signal. This will allow us to
* communicate with non-autonegotiating link partners.
*/
- ret_val = mac->ops.check_for_link(hw);
+ ret_val = mac->ops.check_for_link(hw, NULL);
if (ret_val) {
e_dbg("Error while checking for link\n");
return ret_val;
diff --git a/drivers/net/ethernet/intel/e1000e/mac.h b/drivers/net/ethernet/intel/e1000e/mac.h
index 8284618af9ff..74299bf1a5bb 100644
--- a/drivers/net/ethernet/intel/e1000e/mac.h
+++ b/drivers/net/ethernet/intel/e1000e/mac.h
@@ -23,9 +23,9 @@
#define _E1000E_MAC_H_

s32 e1000e_blink_led_generic(struct e1000_hw *hw);
-s32 e1000e_check_for_copper_link(struct e1000_hw *hw);
-s32 e1000e_check_for_fiber_link(struct e1000_hw *hw);
-s32 e1000e_check_for_serdes_link(struct e1000_hw *hw);
+s32 e1000e_check_for_copper_link(struct e1000_hw *hw, bool *link_status);
+s32 e1000e_check_for_fiber_link(struct e1000_hw *hw, bool *unused);
+s32 e1000e_check_for_serdes_link(struct e1000_hw *hw, bool *unused);
s32 e1000e_cleanup_led_generic(struct e1000_hw *hw);
s32 e1000e_config_fc_after_link_up(struct e1000_hw *hw);
s32 e1000e_disable_pcie_master(struct e1000_hw *hw);
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 9fd4050a91ca..548250108dc5 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5088,19 +5088,14 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
*/
switch (hw->phy.media_type) {
case e1000_media_type_copper:
- if (hw->mac.get_link_status) {
- ret_val = hw->mac.ops.check_for_link(hw);
- link_active = ret_val > 0;
- } else {
- link_active = true;
- }
+ ret_val = hw->mac.ops.check_for_link(hw, &link_active);
break;
case e1000_media_type_fiber:
- ret_val = hw->mac.ops.check_for_link(hw);
+ ret_val = hw->mac.ops.check_for_link(hw, NULL);
link_active = !!(er32(STATUS) & E1000_STATUS_LU);
break;
case e1000_media_type_internal_serdes:
- ret_val = hw->mac.ops.check_for_link(hw);
+ ret_val = hw->mac.ops.check_for_link(hw, NULL);
link_active = hw->mac.serdes_has_link;
break;
default:
--
2.16.1