Skip to contentSkip to conversation

e1000e: call pci_save_state() after pci_restore_state()

PATCH Applied Jeff Kirsher 2 participants 12 messages Jan 8, 2010

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Jeff Kirsher

author
#0 lore raw

PATCH 1/5 e1000e: call pci_save_state() after pci_restore_state()

From: Bruce Allan <[email protected]>

Due to a change in pci_restore_state()[1] which clears the saved_state flag, the driver should call pci_save_state() to set the flag once again to avoid issues with EEH (same fix that recently was submitted for ixgbe).

[1] commmit 4b77b0a2ba27d64f58f16d8d4d48d8319dda36ff

Signed-off-by Bruce Allan <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

---

 drivers/net/e1000e/netdev.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/net/e1000e/netdev.c +2 -0
index 762b697..10aa0ec 100644
@@ -4674,6 +4674,7 @@ static int e1000_resume(struct pci_dev *pdev)
 
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
+	pci_save_state(pdev);
 	e1000e_disable_l1aspm(pdev);
 
 	err = pci_enable_device_mem(pdev);

@@ -4825,6 +4826,7 @@ static pci_ers_result_t e1000_io_slot_reset(struct pci_dev *pdev)
 	} else {
 		pci_set_master(pdev);
 		pci_restore_state(pdev);
+		pci_save_state(pdev);
 
 		pci_enable_wake(pdev, PCI_D3hot, 0);
 		pci_enable_wake(pdev, PCI_D3cold, 0);
author
#1 lore raw

PATCH 2/5 e1000e: don't accumulate PHY statistics on PHY read failure

From: Bruce Allan <[email protected]>

Signed-off-by Bruce Allan <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

---

 drivers/net/e1000e/netdev.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)
drivers/net/e1000e/netdev.c +14 -14
index 10aa0ec..c45965a 100644
@@ -3315,24 +3315,24 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
 	if ((hw->phy.type == e1000_phy_82578) ||
 	    (hw->phy.type == e1000_phy_82577)) {
 		e1e_rphy(hw, HV_SCC_UPPER, &phy_data);
-		e1e_rphy(hw, HV_SCC_LOWER, &phy_data);
-		adapter->stats.scc += phy_data;
+		if (!e1e_rphy(hw, HV_SCC_LOWER, &phy_data))
+			adapter->stats.scc += phy_data;
 
 		e1e_rphy(hw, HV_ECOL_UPPER, &phy_data);
-		e1e_rphy(hw, HV_ECOL_LOWER, &phy_data);
-		adapter->stats.ecol += phy_data;
+		if (!e1e_rphy(hw, HV_ECOL_LOWER, &phy_data))
+			adapter->stats.ecol += phy_data;
 
 		e1e_rphy(hw, HV_MCC_UPPER, &phy_data);
-		e1e_rphy(hw, HV_MCC_LOWER, &phy_data);
-		adapter->stats.mcc += phy_data;
+		if (!e1e_rphy(hw, HV_MCC_LOWER, &phy_data))
+			adapter->stats.mcc += phy_data;
 
 		e1e_rphy(hw, HV_LATECOL_UPPER, &phy_data);
-		e1e_rphy(hw, HV_LATECOL_LOWER, &phy_data);
-		adapter->stats.latecol += phy_data;
+		if (!e1e_rphy(hw, HV_LATECOL_LOWER, &phy_data))
+			adapter->stats.latecol += phy_data;
 
 		e1e_rphy(hw, HV_DC_UPPER, &phy_data);
-		e1e_rphy(hw, HV_DC_LOWER, &phy_data);
-		adapter->stats.dc += phy_data;
+		if (!e1e_rphy(hw, HV_DC_LOWER, &phy_data))
+			adapter->stats.dc += phy_data;
 	} else {
 		adapter->stats.scc += er32(SCC);
 		adapter->stats.ecol += er32(ECOL);

@@ -3360,8 +3360,8 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
 	if ((hw->phy.type == e1000_phy_82578) ||
 	    (hw->phy.type == e1000_phy_82577)) {
 		e1e_rphy(hw, HV_COLC_UPPER, &phy_data);
-		e1e_rphy(hw, HV_COLC_LOWER, &phy_data);
-		hw->mac.collision_delta = phy_data;
+		if (!e1e_rphy(hw, HV_COLC_LOWER, &phy_data))
+			hw->mac.collision_delta = phy_data;
 	} else {
 		hw->mac.collision_delta = er32(COLC);
 	}

@@ -3372,8 +3372,8 @@ void e1000e_update_stats(struct e1000_adapter *adapter)
 	if ((hw->phy.type == e1000_phy_82578) ||
 	    (hw->phy.type == e1000_phy_82577)) {
 		e1e_rphy(hw, HV_TNCRS_UPPER, &phy_data);
-		e1e_rphy(hw, HV_TNCRS_LOWER, &phy_data);
-		adapter->stats.tncrs += phy_data;
+		if (!e1e_rphy(hw, HV_TNCRS_LOWER, &phy_data))
+			adapter->stats.tncrs += phy_data;
 	} else {
 		if ((hw->mac.type != e1000_82574) &&
 		    (hw->mac.type != e1000_82583))
author
#3 lore raw

PATCH 3/5 e1000e: perform 10/100 adaptive IFS only on parts that support it

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Adaptive IFS which involves writing to the Adaptive IFS Throttle register was being done for all devices supported by the driver even though it is not supported (i.e. the register doesn't even exist) on some devices. The feature is supported on 8257x/82583 and ICH/PCH based devices, but not on ESB2.

Signed-off-by Bruce Allan <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

---

 drivers/net/e1000e/82571.c   |    2 ++
 drivers/net/e1000e/es2lan.c  |    2 ++
 drivers/net/e1000e/hw.h      |    1 +
 drivers/net/e1000e/ich8lan.c |    2 ++
 drivers/net/e1000e/lib.c     |   14 ++++++++++++++
 5 files changed, 21 insertions(+), 0 deletions(-)
drivers/net/e1000e/82571.c +2 -0
index b979464..02d67d0 100644
@@ -237,6 +237,8 @@ static s32 e1000_init_mac_params_82571(struct e1000_adapter *adapter)
 	/* Set if manageability features are enabled. */
 	mac->arc_subsystem_valid = (er32(FWSM) & E1000_FWSM_MODE_MASK)
 	                ? true : false;
+	/* Adaptive IFS supported */
+	mac->adaptive_ifs = true;
 
 	/* check for link */
 	switch (hw->phy.media_type) {
drivers/net/e1000e/es2lan.c +2 -0
index 3028f23..e2aa3b7 100644
@@ -224,6 +224,8 @@ static s32 e1000_init_mac_params_80003es2lan(struct e1000_adapter *adapter)
 	/* Set if manageability features are enabled. */
 	mac->arc_subsystem_valid = (er32(FWSM) & E1000_FWSM_MODE_MASK)
                         ? true : false;
+	/* Adaptive IFS not supported */
+	mac->adaptive_ifs = false;
 
 	/* check for link */
 	switch (hw->phy.media_type) {
drivers/net/e1000e/hw.h +1 -0
index 2784cf4..eccf29b 100644
@@ -818,6 +818,7 @@ struct e1000_mac_info {
 
 	u8  forced_speed_duplex;
 
+	bool adaptive_ifs;
 	bool arc_subsystem_valid;
 	bool autoneg;
 	bool autoneg_failed;
drivers/net/e1000e/ich8lan.c +2 -0
index 9b09246..ad08cf3 100644
@@ -454,6 +454,8 @@ static s32 e1000_init_mac_params_ich8lan(struct e1000_adapter *adapter)
 		mac->rar_entry_count--;
 	/* Set if manageability features are enabled. */
 	mac->arc_subsystem_valid = true;
+	/* Adaptive IFS supported */
+	mac->adaptive_ifs = true;
 
 	/* LED operations */
 	switch (mac->type) {
drivers/net/e1000e/lib.c +14 -0
index a86c175..56b59e4 100644
@@ -1609,6 +1609,11 @@ void e1000e_reset_adaptive(struct e1000_hw *hw)
 {
 	struct e1000_mac_info *mac = &hw->mac;
 
+	if (!mac->adaptive_ifs) {
+		e_dbg("Not in Adaptive IFS mode!\n");
+		goto out;
+	}
+
 	mac->current_ifs_val = 0;
 	mac->ifs_min_val = IFS_MIN;
 	mac->ifs_max_val = IFS_MAX;

@@ -1617,6 +1622,8 @@ void e1000e_reset_adaptive(struct e1000_hw *hw)
 
 	mac->in_ifs_mode = false;
 	ew32(AIT, 0);
+out:
+	return;
 }
 
 /**

@@ -1630,6 +1637,11 @@ void e1000e_update_adaptive(struct e1000_hw *hw)
 {
 	struct e1000_mac_info *mac = &hw->mac;
 
+	if (!mac->adaptive_ifs) {
+		e_dbg("Not in Adaptive IFS mode!\n");
+		goto out;
+	}
+
 	if ((mac->collision_delta * mac->ifs_ratio) > mac->tx_packet_delta) {
 		if (mac->tx_packet_delta > MIN_NUM_XMITS) {
 			mac->in_ifs_mode = true;

@@ -1650,6 +1662,8 @@ void e1000e_update_adaptive(struct e1000_hw *hw)
 			ew32(AIT, 0);
 		}
 	}
+out:
+	return;
 }
 
 /**
maintainer Applied
#4 lore raw

Re: [net-2.6 PATCH 3/5] e1000e: perform 10/100 adaptive IFS only on parts that support it

From: Jeff Kirsher <[email protected]>
Date: Thu, 07 Jan 2010 18:31:35 -0800

Quoted 10 lines

From: Bruce Allan <[email protected]>

Adaptive IFS which involves writing to the Adaptive IFS Throttle register was being done for all devices supported by the driver even though it is not supported (i.e. the register doesn't even exist) on some devices. The feature is supported on 8257x/82583 and ICH/PCH based devices, but not on ESB2.

Signed-off-by Bruce Allan <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

Applied.

author
#5 lore raw

PATCH 4/5 e1000e: e1000e_enable_tx_pkt_filtering() returns wrong value

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

e1000e_enable_tx_pkt_filtering() will return a non-zero value if the driver fails to enable the manageability interface on the host for any reason; instead it should retun zero to indicate filtering has been disabled. Also provide a single exit point for the function.

Signed-off-by Bruce Allan <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

---

 drivers/net/e1000e/lib.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)
drivers/net/e1000e/lib.c +9 -7
index 56b59e4..97649bf 100644
@@ -2301,10 +2301,12 @@ bool e1000e_enable_tx_pkt_filtering(struct e1000_hw *hw)
 	s32 ret_val, hdr_csum, csum;
 	u8 i, len;
 
+	hw->mac.tx_pkt_filtering = true;
+
 	/* No manageability, no filtering */
 	if (!e1000e_check_mng_mode(hw)) {
 		hw->mac.tx_pkt_filtering = false;
-		return 0;
+		goto out;
 	}
 
 	/*

@@ -2312,9 +2314,9 @@ bool e1000e_enable_tx_pkt_filtering(struct e1000_hw *hw)
 	 * reason, disable filtering.
 	 */
 	ret_val = e1000_mng_enable_host_if(hw);
-	if (ret_val != 0) {
+	if (ret_val) {
 		hw->mac.tx_pkt_filtering = false;
-		return ret_val;
+		goto out;
 	}
 
 	/* Read in the header.  Length and offset are in dwords. */

@@ -2333,17 +2335,17 @@ bool e1000e_enable_tx_pkt_filtering(struct e1000_hw *hw)
 	 */
 	if ((hdr_csum != csum) || (hdr->signature != E1000_IAMT_SIGNATURE)) {
 		hw->mac.tx_pkt_filtering = true;
-		return 1;
+		goto out;
 	}
 
 	/* Cookie area is valid, make the final check for filtering. */
 	if (!(hdr->status & E1000_MNG_DHCP_COOKIE_STATUS_PARSING)) {
 		hw->mac.tx_pkt_filtering = false;
-		return 0;
+		goto out;
 	}
 
-	hw->mac.tx_pkt_filtering = true;
-	return 1;
+out:
+	return hw->mac.tx_pkt_filtering;
 }
 
 /**
maintainer Applied
#6 lore raw

Re: [net-2.6 PATCH 4/5] e1000e: e1000e_enable_tx_pkt_filtering() returns wrong value

From: Jeff Kirsher <[email protected]>
Date: Thu, 07 Jan 2010 18:31:54 -0800

Quoted 9 lines

From: Bruce Allan <[email protected]>

e1000e_enable_tx_pkt_filtering() will return a non-zero value if the driver fails to enable the manageability interface on the host for any reason; instead it should retun zero to indicate filtering has been disabled. Also provide a single exit point for the function.

Signed-off-by Bruce Allan <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

Applied.

author
#7 lore raw

PATCH 5/5 e1000e: fix and commonize code for setting the receive address registers

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Fix e1000e_rar_set() to flush consecutive register writes to avoid write combining which some parts cannot handle. Update e1000e_init_rx_addrs() to call the fixed e1000e_rar_set() instead of duplicating code.

Also change e1000e_rar_set() to _not_ set the Address Valid bit if the MAC address is all zeros.

Signed-off-by Bruce Allan <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

---

 drivers/net/e1000e/lib.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)
drivers/net/e1000e/lib.c +15 -9
index 97649bf..2fa9b36 100644
@@ -125,6 +125,7 @@ void e1000_write_vfta_generic(struct e1000_hw *hw, u32 offset, u32 value)
 void e1000e_init_rx_addrs(struct e1000_hw *hw, u16 rar_count)
 {
 	u32 i;
+	u8 mac_addr[ETH_ALEN] = {0};
 
 	/* Setup the receive address */
 	e_dbg("Programming MAC Address into RAR[0]\n");

@@ -133,12 +134,8 @@ void e1000e_init_rx_addrs(struct e1000_hw *hw, u16 rar_count)
 
 	/* Zero out the other (rar_entry_count - 1) receive addresses */
 	e_dbg("Clearing RAR[1-%u]\n", rar_count-1);
-	for (i = 1; i < rar_count; i++) {
-		E1000_WRITE_REG_ARRAY(hw, E1000_RA, (i << 1), 0);
-		e1e_flush();
-		E1000_WRITE_REG_ARRAY(hw, E1000_RA, ((i << 1) + 1), 0);
-		e1e_flush();
-	}
+	for (i = 1; i < rar_count; i++)
+		e1000e_rar_set(hw, mac_addr, i);
 }
 
 /**

@@ -164,10 +161,19 @@ void e1000e_rar_set(struct e1000_hw *hw, u8 *addr, u32 index)
 
 	rar_high = ((u32) addr[4] | ((u32) addr[5] << 8));
 
-	rar_high |= E1000_RAH_AV;
+	/* If MAC address zero, no need to set the AV bit */
+	if (rar_low || rar_high)
+		rar_high |= E1000_RAH_AV;
 
-	E1000_WRITE_REG_ARRAY(hw, E1000_RA, (index << 1), rar_low);
-	E1000_WRITE_REG_ARRAY(hw, E1000_RA, ((index << 1) + 1), rar_high);
+	/*
+	 * Some bridges will combine consecutive 32-bit writes into
+	 * a single burst write, which will malfunction on some parts.
+	 * The flushes avoid this.
+	 */
+	ew32(RAL(index), rar_low);
+	e1e_flush();
+	ew32(RAH(index), rar_high);
+	e1e_flush();
 }
 
 /**
maintainer Applied
#8 lore raw

Re: [net-2.6 PATCH 5/5] e1000e: fix and commonize code for setting the receive address registers

From: Jeff Kirsher <[email protected]>
Date: Thu, 07 Jan 2010 18:32:13 -0800

Quoted 11 lines

From: Bruce Allan <[email protected]>

Fix e1000e_rar_set() to flush consecutive register writes to avoid write combining which some parts cannot handle. Update e1000e_init_rx_addrs() to call the fixed e1000e_rar_set() instead of duplicating code.

Also change e1000e_rar_set() to _not_ set the Address Valid bit if the MAC address is all zeros.

Signed-off-by Bruce Allan <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

Applied.

author
#10 lore raw
Quoted 4 lines · David Miller

Aren't the fragment overrun patches for e1000 and e1000e 1,000 times more important that thumb twiddling fixes like these? :-( --

I would agree, and Jesse's updated patches which were made 1 day ago are currently under test and review, where as these patches were produced over Christmas break and testing was completed before Jesse completed his rework of the fragment overrun patches (and are based on a driver which has these patches applied).

Signature · 2 lines
Cheers,
Jeff
maintainer Applied
#11 lore raw
In reply to Jeff Kirsher

From: Jeff Kirsher <[email protected]>
Date: Thu, 07 Jan 2010 18:30:56 -0800

Quoted 10 lines

From: Bruce Allan <[email protected]>

Due to a change in pci_restore_state()[1] which clears the saved_state flag, the driver should call pci_save_state() to set the flag once again to avoid issues with EEH (same fix that recently was submitted for ixgbe).

[1] commmit 4b77b0a2ba27d64f58f16d8d4d48d8319dda36ff

Signed-off-by Bruce Allan <[email protected]>
Signed-off-by Jeff Kirsher <[email protected]>

Applied.