Commit 3016e0a0 authored by Benjamin Poirier's avatar Benjamin Poirier Committed by Jeff Kirsher

Revert "e1000e: Separate signaling for link check/link up"

This reverts commit 19110cfb.
This reverts commit 4110e02e.
This reverts commit d3604515c9eda464a92e8e67aae82dfe07fe3c98.

Commit 19110cfb ("e1000e: Separate signaling for link check/link up")
changed what happens to the link status when there is an error which
happens after "get_link_status = false" in the copper check_for_link
callbacks. Previously, such an error would be ignored and the link
considered up. After that commit, any error implies that the link is down.

Revert commit 19110cfb ("e1000e: Separate signaling for link check/link
up") and its followups. After reverting, the race condition described in
the log of commit 19110cfb is reintroduced. It may still be triggered
by LSC events but this should keep the link down in case the link is
electrically unstable, as discussed. The race may no longer be
triggered by RXO events because commit 4aea7a5c ("e1000e: Avoid
receiver overrun interrupt bursts") restored reading icr in the Other
handler.

Link: https://lkml.org/lkml/2018/3/1/789Signed-off-by: default avatarBenjamin Poirier <bpoirier@suse.com>
Acked-by: default avatarAlexander Duyck <alexander.h.duyck@intel.com>
Tested-by: default avatarAaron Brown <aaron.f.brown@intel.com>
Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
parent a2c054a8
...@@ -1367,9 +1367,6 @@ static s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force) ...@@ -1367,9 +1367,6 @@ static s32 e1000_disable_ulp_lpt_lp(struct e1000_hw *hw, bool force)
* Checks to see of the link status of the hardware has changed. If a * 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 * change in link status has been detected, then we read the PHY registers
* to get the current speed/duplex if link exists. * 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)
{ {
...@@ -1385,7 +1382,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw) ...@@ -1385,7 +1382,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
* Change or Rx Sequence Error interrupt. * Change or Rx Sequence Error interrupt.
*/ */
if (!mac->get_link_status) if (!mac->get_link_status)
return 1; return 0;
/* First we want to see if the MII Status Register reports /* First we want to see if the MII Status Register reports
* link. If so, then we want to get the current speed/duplex * link. If so, then we want to get the current speed/duplex
...@@ -1602,7 +1599,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw) ...@@ -1602,7 +1599,7 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
* we have already determined whether we have link or not. * we have already determined whether we have link or not.
*/ */
if (!mac->autoneg) if (!mac->autoneg)
return 1; return -E1000_ERR_CONFIG;
/* Auto-Neg is enabled. Auto Speed Detection takes care /* Auto-Neg is enabled. Auto Speed Detection takes care
* of MAC speed/duplex configuration. So we only need to * of MAC speed/duplex configuration. So we only need to
...@@ -1616,12 +1613,10 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw) ...@@ -1616,12 +1613,10 @@ static s32 e1000_check_for_copper_link_ich8lan(struct e1000_hw *hw)
* different link partner. * different link partner.
*/ */
ret_val = e1000e_config_fc_after_link_up(hw); ret_val = e1000e_config_fc_after_link_up(hw);
if (ret_val) { if (ret_val)
e_dbg("Error configuring flow control\n"); e_dbg("Error configuring flow control\n");
return ret_val;
}
return 1; return ret_val;
} }
static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter) static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter)
......
...@@ -410,9 +410,6 @@ void e1000e_clear_hw_cntrs_base(struct e1000_hw *hw) ...@@ -410,9 +410,6 @@ void e1000e_clear_hw_cntrs_base(struct e1000_hw *hw)
* Checks to see of the link status of the hardware has changed. If a * 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 * change in link status has been detected, then we read the PHY registers
* to get the current speed/duplex if link exists. * 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)
{ {
...@@ -426,7 +423,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw) ...@@ -426,7 +423,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
* Change or Rx Sequence Error interrupt. * Change or Rx Sequence Error interrupt.
*/ */
if (!mac->get_link_status) if (!mac->get_link_status)
return 1; return 0;
/* First we want to see if the MII Status Register reports /* First we want to see if the MII Status Register reports
* link. If so, then we want to get the current speed/duplex * link. If so, then we want to get the current speed/duplex
...@@ -450,7 +447,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw) ...@@ -450,7 +447,7 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
* we have already determined whether we have link or not. * we have already determined whether we have link or not.
*/ */
if (!mac->autoneg) if (!mac->autoneg)
return 1; return -E1000_ERR_CONFIG;
/* Auto-Neg is enabled. Auto Speed Detection takes care /* Auto-Neg is enabled. Auto Speed Detection takes care
* of MAC speed/duplex configuration. So we only need to * of MAC speed/duplex configuration. So we only need to
...@@ -464,12 +461,10 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw) ...@@ -464,12 +461,10 @@ s32 e1000e_check_for_copper_link(struct e1000_hw *hw)
* different link partner. * different link partner.
*/ */
ret_val = e1000e_config_fc_after_link_up(hw); ret_val = e1000e_config_fc_after_link_up(hw);
if (ret_val) { if (ret_val)
e_dbg("Error configuring flow control\n"); e_dbg("Error configuring flow control\n");
return ret_val;
}
return 1; return ret_val;
} }
/** /**
......
...@@ -5090,7 +5090,7 @@ static bool e1000e_has_link(struct e1000_adapter *adapter) ...@@ -5090,7 +5090,7 @@ static bool e1000e_has_link(struct e1000_adapter *adapter)
case e1000_media_type_copper: case e1000_media_type_copper:
if (hw->mac.get_link_status) { if (hw->mac.get_link_status) {
ret_val = hw->mac.ops.check_for_link(hw); ret_val = hw->mac.ops.check_for_link(hw);
link_active = ret_val > 0; link_active = !hw->mac.get_link_status;
} else { } else {
link_active = true; link_active = true;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment