Skip to contentSkip to conversation

e1000e: disable correctable errors for quad ports while going to D3

PATCH Applied Jeff Kirsher 4 participants 32 messages Nov 21, 2008 – Nov 22, 2008

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

Jeff Kirsher

author
#0 lore raw

PATCH 1/14 e1000e: disable correctable errors for quad ports while going to D3

From: Alexander Duyck <[email protected]>

There has been an issue seen with the pci-e quad port adapters that will cause them to generate a pci-e correctable error on some system while transitioning to D3.

Since no action is needed on this correctable error the simplest solution is to mask off the reporting of correctable errors.

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

---

 drivers/net/e1000e/netdev.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)
drivers/net/e1000e/netdev.c +21 -1
index cc0502b..ebbb9f0 100644
@@ -4462,7 +4462,27 @@ static int e1000_suspend(struct pci_dev *pdev, pm_message_t state)
 
 	pci_disable_device(pdev);
 
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
+	/*
+	 * The pci-e switch on some quad port adapters will report a
+	 * correctable error when the MAC transitions from D0 to D3.  To
+	 * prevent this we need to mask off the correctable errors on the
+	 * downstream port of the pci-e switch.
+	 */
+	if (adapter->flags & FLAG_IS_QUAD_PORT) {
+		struct pci_dev *us_dev = pdev->bus->self;
+		int pos = pci_find_capability(us_dev, PCI_CAP_ID_EXP);
+		u16 devctl;
+
+		pci_read_config_word(us_dev, pos + PCI_EXP_DEVCTL, &devctl);
+		pci_write_config_word(us_dev, pos + PCI_EXP_DEVCTL,
+		                      (devctl & ~PCI_EXP_DEVCTL_CERE));
+
+		pci_set_power_state(pdev, pci_choose_state(pdev, state));
+
+		pci_write_config_word(us_dev, pos + PCI_EXP_DEVCTL, devctl);
+	} else {
+		pci_set_power_state(pdev, pci_choose_state(pdev, state));
+	}
 
 	return 0;
 }
author
#1 lore raw

PATCH 2/14 e1000e: commit speed/duplex changes for m88 PHY

From: Bruce Allan <[email protected]>

Follow the convention used elsewhere in e1000e to 'commit' PHY changes instead of directly writing to the PHY CTRL register to reset it.

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

---

 drivers/net/e1000e/phy.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
drivers/net/e1000e/phy.c +4 -4
index 6cd333a..cb7d71e 100644
@@ -1030,14 +1030,14 @@ s32 e1000e_phy_force_speed_duplex_m88(struct e1000_hw *hw)
 
 	e1000e_phy_force_speed_duplex_setup(hw, &phy_data);
 
-	/* Reset the phy to commit changes. */
-	phy_data |= MII_CR_RESET;
-
 	ret_val = e1e_wphy(hw, PHY_CONTROL, phy_data);
 	if (ret_val)
 		return ret_val;
 
-	udelay(1);
+	/* Reset the phy to commit changes. */
+	ret_val = e1000e_commit_phy(hw);
+	if (ret_val)
+		return ret_val;
 
 	if (phy->autoneg_wait_to_complete) {
 		hw_dbg(hw, "Waiting for forced speed/duplex link on M88 phy.\n");
maintainer Applied
#2 lore raw

Re: [NET-NEXT PATCH 02/14] e1000e: commit speed/duplex changes for m88 PHY

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 10:59:22 -0800

Quoted 7 lines

From: Bruce Allan <[email protected]>

Follow the convention used elsewhere in e1000e to 'commit' PHY changes instead of directly writing to the PHY CTRL register to reset it.

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

Applied.

author
#3 lore raw

PATCH 3/14 e1000e: 82571 check for link fix on 82571 serdes

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Check for link test does not work properly for 82571 parts in a blade environment with an unterminated serdes link partner. Make the test more robust by checking the invalid bit.

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

---

 drivers/net/e1000e/lib.c |   34 ++++++++++++++++++++++++++++------
 1 files changed, 28 insertions(+), 6 deletions(-)
drivers/net/e1000e/lib.c +28 -6
index 089578f..37753d1 100644
@@ -575,20 +575,42 @@ s32 e1000e_check_for_serdes_link(struct e1000_hw *hw)
 		 */
 		/* SYNCH bit and IV bit are sticky. */
 		udelay(10);
-		if (E1000_RXCW_SYNCH & er32(RXCW)) {
+		rxcw = er32(RXCW);
+		if (rxcw & E1000_RXCW_SYNCH) {
 			if (!(rxcw & E1000_RXCW_IV)) {
-				mac->serdes_has_link = 1;
-				hw_dbg(hw, "SERDES: Link is up.\n");
+				mac->serdes_has_link = true;
+				hw_dbg(hw, "SERDES: Link up - forced.\n");
 			}
 		} else {
-			mac->serdes_has_link = 0;
-			hw_dbg(hw, "SERDES: Link is down.\n");
+			mac->serdes_has_link = false;
+			hw_dbg(hw, "SERDES: Link down - force failed.\n");
 		}
 	}
 
 	if (E1000_TXCW_ANE & er32(TXCW)) {
 		status = er32(STATUS);
-		mac->serdes_has_link = (status & E1000_STATUS_LU);
+		if (status & E1000_STATUS_LU) {
+			/* SYNCH bit and IV bit are sticky, so reread rxcw.  */
+			udelay(10);
+			rxcw = er32(RXCW);
+			if (rxcw & E1000_RXCW_SYNCH) {
+				if (!(rxcw & E1000_RXCW_IV)) {
+					mac->serdes_has_link = true;
+					hw_dbg(hw, "SERDES: Link up - autoneg "
+					   "completed sucessfully.\n");
+				} else {
+					mac->serdes_has_link = false;
+					hw_dbg(hw, "SERDES: Link down - invalid"
+					   "codewords detected in autoneg.\n");
+				}
+			} else {
+				mac->serdes_has_link = false;
+				hw_dbg(hw, "SERDES: Link down - no sync.\n");
+			}
+		} else {
+			mac->serdes_has_link = false;
+			hw_dbg(hw, "SERDES: Link down - autoneg failed\n");
+		}
 	}
 
 	return 0;
maintainer Applied
#4 lore raw

Re: [NET-NEXT PATCH 03/14] e1000e: 82571 check for link fix on 82571 serdes

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 10:59:39 -0800

Quoted 8 lines

From: Bruce Allan <[email protected]>

Check for link test does not work properly for 82571 parts in a blade environment with an unterminated serdes link partner. Make the test more robust by checking the invalid bit.

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

Applied.

author
#5 lore raw

PATCH 4/14 e1000e: update comments listing supported parts for each MAC family

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Some branding strings (displayed via lspci) are missing from the comments in various family-specific files in the driver.

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

---

 drivers/net/e1000e/82571.c   |    1 +
 drivers/net/e1000e/ich8lan.c |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/e1000e/82571.c +1 -0
index 3027ad5..cbb1707 100644
@@ -28,6 +28,7 @@
 
 /*
  * 82571EB Gigabit Ethernet Controller
+ * 82571EB Gigabit Ethernet Controller (Copper)
  * 82571EB Gigabit Ethernet Controller (Fiber)
  * 82571EB Dual Port Gigabit Mezzanine Adapter
  * 82571EB Quad Port Gigabit Mezzanine Adapter
drivers/net/e1000e/ich8lan.c +2 -0
index 523b971..11f0dd3 100644
@@ -27,6 +27,7 @@
 *******************************************************************************/
 
 /*
+ * 82562G 10/100 Network Connection
  * 82562G-2 10/100 Network Connection
  * 82562GT 10/100 Network Connection
  * 82562GT-2 10/100 Network Connection

@@ -40,6 +41,7 @@
  * 82566MM Gigabit Network Connection
  * 82567LM Gigabit Network Connection
  * 82567LF Gigabit Network Connection
+ * 82567V Gigabit Network Connection
  * 82567LM-2 Gigabit Network Connection
  * 82567LF-2 Gigabit Network Connection
  * 82567V-2 Gigabit Network Connection
maintainer Applied
#6 lore raw

Re: [NET-NEXT PATCH 04/14] e1000e: update comments listing supported parts for each MAC family

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 10:59:57 -0800

Quoted 7 lines

From: Bruce Allan <[email protected]>

Some branding strings (displayed via lspci) are missing from the comments in various family-specific files in the driver.

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

Applied.

author
#7 lore raw

PATCH 5/14 e1000e: check return of pci_save_state

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Check return of pci_save_state and error out accordingly.

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

---

 drivers/net/e1000e/netdev.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/net/e1000e/netdev.c +4 -1
index ebbb9f0..ae6dce5 100644
@@ -4802,7 +4802,10 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
 		goto err_pci_reg;
 
 	pci_set_master(pdev);
-	pci_save_state(pdev);
+	/* PCI config space info */
+	err = pci_save_state(pdev);
+	if (err)
+		goto err_alloc_etherdev;
 
 	err = -ENOMEM;
 	netdev = alloc_etherdev(sizeof(struct e1000_adapter));
author
#9 lore raw

PATCH 6/14 e1000e: remove unnecessary header file inclusions

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

timer.h and workqueue.h do not need to be explicitly included.

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

---

 drivers/net/e1000e/e1000.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/net/e1000e/e1000.h +0 -2
index c55fd6f..e09a181 100644
@@ -32,8 +32,6 @@
 #define _E1000_H_
 
 #include <linux/types.h>
-#include <linux/timer.h>
-#include <linux/workqueue.h>
 #include <linux/io.h>
 #include <linux/netdevice.h>
maintainer
#10 lore raw

Re: [NET-NEXT PATCH 06/14] e1000e: remove unnecessary header file inclusions

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 11:00:35 -0800

Quoted 6 lines

From: Bruce Allan <[email protected]>

timer.h and workqueue.h do not need to be explicitly included.

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

You are using "struct timer_list" et al. in this header file so you do in fact need to include these header files.

If the idea is to rely upon getting these headers indirectly, that's not how things work.

Not applied.

author
#11 lore raw

PATCH 7/14 e1000e: ESB2 config after link up

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

On ESB2, the MAC-to-PHY (Kumeran) interface must be configured after link is up before any traffic is sent; a new PHY operations function pointer is provided for this. To facilitate read/write of the Kumeran registers without blocking PHY register writes, the driver/firmware synchronization method which previously used a hardware semaphore for both PHY and Kumeran register accesses is now split. New Kumeran register read/write functions utilize this new synchronization method.

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

---

 drivers/net/e1000e/82571.c  |    3 +
 drivers/net/e1000e/es2lan.c |  192 +++++++++++++++++++++++++++++++++++--------
 drivers/net/e1000e/hw.h     |    1 
 drivers/net/e1000e/netdev.c |    8 ++
 4 files changed, 167 insertions(+), 37 deletions(-)
drivers/net/e1000e/82571.c +3 -0
index cbb1707..11e72b6 100644
@@ -1394,6 +1394,7 @@ static struct e1000_phy_operations e82_phy_ops_igp = {
 	.set_d0_lplu_state	= e1000_set_d0_lplu_state_82571,
 	.set_d3_lplu_state	= e1000e_set_d3_lplu_state,
 	.write_phy_reg		= e1000e_write_phy_reg_igp,
+	.cfg_on_link_up      	= NULL,
 };
 
 static struct e1000_phy_operations e82_phy_ops_m88 = {

@@ -1410,6 +1411,7 @@ static struct e1000_phy_operations e82_phy_ops_m88 = {
 	.set_d0_lplu_state	= e1000_set_d0_lplu_state_82571,
 	.set_d3_lplu_state	= e1000e_set_d3_lplu_state,
 	.write_phy_reg		= e1000e_write_phy_reg_m88,
+	.cfg_on_link_up      	= NULL,
 };
 
 static struct e1000_phy_operations e82_phy_ops_bm = {

@@ -1426,6 +1428,7 @@ static struct e1000_phy_operations e82_phy_ops_bm = {
 	.set_d0_lplu_state	= e1000_set_d0_lplu_state_82571,
 	.set_d3_lplu_state	= e1000e_set_d3_lplu_state,
 	.write_phy_reg		= e1000e_write_phy_reg_bm2,
+	.cfg_on_link_up      	= NULL,
 };
 
 static struct e1000_nvm_operations e82571_nvm_ops = {
drivers/net/e1000e/es2lan.c +155 -37

306 lines not on this page · raw message

drivers/net/e1000e/hw.h +1 -0
index c4ffd4b..ac1efb9 100644
@@ -739,6 +739,7 @@ struct e1000_phy_operations {
 	s32  (*set_d0_lplu_state)(struct e1000_hw *, bool);
 	s32  (*set_d3_lplu_state)(struct e1000_hw *, bool);
 	s32  (*write_phy_reg)(struct e1000_hw *, u32, u16);
+	s32  (*cfg_on_link_up)(struct e1000_hw *);
 };
 
 /* Function pointers for the NVM. */
drivers/net/e1000e/netdev.c +8 -0
index ae6dce5..7eb1a36 100644
@@ -3493,6 +3493,7 @@ static void e1000_watchdog_task(struct work_struct *work)
 					struct e1000_adapter, watchdog_task);
 	struct net_device *netdev = adapter->netdev;
 	struct e1000_mac_info *mac = &adapter->hw.mac;
+	struct e1000_phy_info *phy = &adapter->hw.phy;
 	struct e1000_ring *tx_ring = adapter->tx_ring;
 	struct e1000_hw *hw = &adapter->hw;
 	u32 link, tctl;

@@ -3599,6 +3600,13 @@ static void e1000_watchdog_task(struct work_struct *work)
 			tctl |= E1000_TCTL_EN;
 			ew32(TCTL, tctl);
 
+                        /*
+			 * Perform any post-link-up configuration before
+			 * reporting link up.
+			 */
+			if (phy->ops.cfg_on_link_up)
+				phy->ops.cfg_on_link_up(hw);
+
 			netif_carrier_on(netdev);
 			netif_tx_wake_all_queues(netdev);
maintainer Applied
#12 lore raw

Re: [NET-NEXT PATCH 07/14] e1000e: ESB2 config after link up

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 11:00:52 -0800

Quoted 10 lines

From: Bruce Allan <[email protected]>

On ESB2, the MAC-to-PHY (Kumeran) interface must be configured after link is up before any traffic is sent; a new PHY operations function pointer is provided for this. To facilitate read/write of the Kumeran registers without blocking PHY register writes, the driver/firmware synchronization method which previously used a hardware semaphore for both PHY and Kumeran register accesses is now split. New Kumeran register read/write functions utilize this new synchronization method.

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

Applied.

author
#13 lore raw

PATCH 8/14 e1000e: link up/down messages must follow a specific format

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

The system log messages created on a link status change need to follow a specific format to work with tools some customers use.

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

---

 drivers/net/e1000e/netdev.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
drivers/net/e1000e/netdev.c +7 -2
index 7eb1a36..cd6d132 100644
@@ -3408,7 +3408,10 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
 	struct e1000_hw *hw = &adapter->hw;
 	u32 ctrl = er32(CTRL);
 
-	e_info("Link is Up %d Mbps %s, Flow Control: %s\n",
+	/* Link status message must follow this format for user tools */
+	printk(KERN_INFO "e1000e: %s NIC Link is Up %d Mbps %s, "
+	       "Flow Control: %s\n",
+	       adapter->netdev->name,
 	       adapter->link_speed,
 	       (adapter->link_duplex == FULL_DUPLEX) ?
 	                        "Full Duplex" : "Half Duplex",

@@ -3618,7 +3621,9 @@ static void e1000_watchdog_task(struct work_struct *work)
 		if (netif_carrier_ok(netdev)) {
 			adapter->link_speed = 0;
 			adapter->link_duplex = 0;
-			e_info("Link is Down\n");
+			/* Link status message must follow this format */
+			printk(KERN_INFO "e1000e: %s NIC Link is Down\n",
+			       adapter->netdev->name);
 			netif_carrier_off(netdev);
 			netif_tx_stop_all_queues(netdev);
 			if (!test_bit(__E1000_DOWN, &adapter->state))
#14 lore raw

Re: [NET-NEXT PATCH 08/14] e1000e: link up/down messages must follow a specific format

Quoted 4 lines · Jeff Kirsher

From: Bruce Allan <[email protected]>

The system log messages created on a link status change need to follow a specific format to work with tools some customers use.

Um, shouldn't those tools be listening to netlink for carrier events, or are these tools run on a separate machine using on some later date using the logs from the machine with the e1000e?

Dan

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

---

 drivers/net/e1000e/netdev.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
drivers/net/e1000e/netdev.c +7 -3
index 7eb1a36..cd6d132 100644
@@ -3408,7 +3408,10 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
 	struct e1000_hw *hw = &adapter->hw;
 	u32 ctrl = er32(CTRL);
 
-	e_info("Link is Up %d Mbps %s, Flow Control: %s\n",
+	/* Link status message must follow this format for user tools */
+	printk(KERN_INFO "e1000e: %s NIC Link is Up %d Mbps %s, "
+	       "Flow Control: %s\n",
+	       adapter->netdev->name,
 	       adapter->link_speed,
 	       (adapter->link_duplex == FULL_DUPLEX) ?
 	                        "Full Duplex" : "Half Duplex",

@@ -3618,7 +3621,9 @@ static void e1000_watchdog_task(struct work_struct *work)
 		if (netif_carrier_ok(netdev)) {
 			adapter->link_speed = 0;
 			adapter->link_duplex = 0;
-			e_info("Link is Down\n");
+			/* Link status message must follow this format */
+			printk(KERN_INFO "e1000e: %s NIC Link is Down\n",
+			       adapter->netdev->name);
 			netif_carrier_off(netdev);
 			netif_tx_stop_all_queues(netdev);
 			if (!test_bit(__E1000_DOWN, &adapter->state))
 
--

To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#15 lore raw

Re: [NET-NEXT PATCH 08/14] e1000e: link up/down messages must follow a specific format

Quoted 6 lines · Dan Williams
Quoted 4 lines · Jeff Kirsher

From: Bruce Allan <[email protected]>

The system log messages created on a link status change need to follow a specific format to work with tools some customers use.

Quoted 8 lines

Um, shouldn't those tools be listening to netlink for carrier events, or are these tools run on a separate machine using on some later date using the logs from the machine with the e1000e?

Dan

From my understanding these tools are looking at the logs and that is

why we need to have a consistent log message.

Signature · 2 lines
Cheers,
Jeff
#16 lore raw

Re: [NET-NEXT PATCH 08/14] e1000e: link up/down messages must follow a specific format

Quoted 11 lines · On Fri, 21 Nov 2008 11:23:42 -0800 "Jeff Kirsher"…
Quoted 10 lines · Dan Williams
Quoted 4 lines · Jeff Kirsher

From: Bruce Allan <[email protected]>

The system log messages created on a link status change need to follow a specific format to work with tools some customers use.

Um, shouldn't those tools be listening to netlink for carrier events, or are these tools run on a separate machine using on some later date using the logs from the machine with the e1000e?

Quoted 6 lines
Quoted 2 lines

Dan

From my understanding these tools are looking at the logs and that is why we need to have a consistent log message.

These tools are tied to a specific driver (yours), because not all drivers generate a message or the same format message. This may be okay for Intel but is really stupid design...

It would be good if link state transitions generated uevents (online/offline). Then udev, hal and others could use that without netlink.

author
#17 lore raw

Re: [NET-NEXT PATCH 08/14] e1000e: link up/down messages must follow a specific format

Quoted 17 lines · Stephen Hemminger
Quoted 14 lines · On Fri, 21 Nov 2008 11:23:42 -0800 "Jeff Kirsher"…
Quoted 12 lines · Dan Williams
Quoted 4 lines · Jeff Kirsher

From: Bruce Allan <[email protected]>

The system log messages created on a link status change need to follow a specific format to work with tools some customers use.

Um, shouldn't those tools be listening to netlink for carrier events, or are these tools run on a separate machine using on some later date using the logs from the machine with the e1000e?

Dan

Quoted 7 lines
Quoted 3 lines

From my understanding these tools are looking at the logs and that is why we need to have a consistent log message.

These tools are tied to a specific driver (yours), because not all drivers generate a message or the same format message. This may be okay for Intel but is really stupid design...

I have not checked all of the drivers, but I do see that our driver does follow what others driver (e.g. tg3, starfire) are doing. I would be open for standardizing on what messages get generated and in what format.

I am sure that would help third parties generate tools that would work with every driver.

Quoted 3 lines

It would be good if link state transitions generated uevents (online/offline). Then udev, hal and others could use that without netlink.
--

Again, I would be open to any standardization so that all drivers would deal with link up/down messages in the same manner.

Signature · 2 lines
Cheers,
Jeff
maintainer
#18 lore raw

Re: [NET-NEXT PATCH 08/14] e1000e: link up/down messages must follow a specific format

In reply to Jeff Kirsher

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 11:01:10 -0800

Quoted 7 lines

From: Bruce Allan <[email protected]>

The system log messages created on a link status change need to follow a specific format to work with tools some customers use.

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

As stated by others, if anything we should have a global tree wide format for this. Even via a helper function or similar.

But for now, how you print this message is your business, so patch applied :)

author
#19 lore raw

PATCH 9/14 e1000e: fix possible buffer overflow

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Put in missing bounds checking of an array.

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

---

 drivers/net/e1000e/es2lan.c |    5 +++++
 drivers/net/e1000e/phy.c    |    5 +++++
 2 files changed, 10 insertions(+), 0 deletions(-)
drivers/net/e1000e/es2lan.c +5 -0
index db51114..b5250fd 100644
@@ -104,6 +104,8 @@
  */
 static const u16 e1000_gg82563_cable_length_table[] =
 	 { 0, 60, 115, 150, 150, 60, 115, 150, 180, 180, 0xFF };
+#define GG82563_CABLE_LENGTH_TABLE_SIZE \
+		ARRAY_SIZE(e1000_gg82563_cable_length_table)
 
 static s32 e1000_setup_copper_link_80003es2lan(struct e1000_hw *hw);
 static s32 e1000_acquire_swfw_sync_80003es2lan(struct e1000_hw *hw, u16 mask);

@@ -721,6 +723,9 @@ static s32 e1000_get_cable_length_80003es2lan(struct e1000_hw *hw)
 		return ret_val;
 
 	index = phy_data & GG82563_DSPD_CABLE_LENGTH;
+	if (index >= GG82563_CABLE_LENGTH_TABLE_SIZE + 5)
+		return E1000_ERR_PHY;
+
 	phy->min_cable_length = e1000_gg82563_cable_length_table[index];
 	phy->max_cable_length = e1000_gg82563_cable_length_table[index+5];
drivers/net/e1000e/phy.c +5 -0
index cb7d71e..d3aa6b7 100644
@@ -41,6 +41,8 @@ static s32 e1000_access_phy_wakeup_reg_bm(struct e1000_hw *hw, u32 offset,
 /* Cable length tables */
 static const u16 e1000_m88_cable_length_table[] =
 	{ 0, 50, 80, 110, 140, 140, E1000_CABLE_LENGTH_UNDEFINED };
+#define M88E1000_CABLE_LENGTH_TABLE_SIZE \
+		ARRAY_SIZE(e1000_m88_cable_length_table)
 
 static const u16 e1000_igp_2_cable_length_table[] =
 	{ 0, 0, 0, 0, 0, 0, 0, 0, 3, 5, 8, 11, 13, 16, 18, 21, 0, 0, 0, 3,

@@ -1442,6 +1444,9 @@ s32 e1000e_get_cable_length_m88(struct e1000_hw *hw)
 
 	index = (phy_data & M88E1000_PSSR_CABLE_LENGTH) >>
 		M88E1000_PSSR_CABLE_LENGTH_SHIFT;
+	if (index >= M88E1000_CABLE_LENGTH_TABLE_SIZE + 1)
+		return E1000_ERR_PHY;
+
 	phy->min_cable_length = e1000_m88_cable_length_table[index];
 	phy->max_cable_length = e1000_m88_cable_length_table[index+1];
maintainer
#20 lore raw

Re: [NET-NEXT PATCH 09/14] e1000e: fix possible buffer overflow

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 11:01:28 -0800

Quoted 6 lines

From: Bruce Allan <[email protected]>

Put in missing bounds checking of an array.

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

No magic constants, please. What does the "+ 5" mean?

And using a macro that is:

1) Used in exactly one place
2) Gives no more information than the expanded ARRAY_SIZE()

is pretty useless as well.

Patch not applied.

Quoted 10 lines
drivers/net/e1000e/es2lan.c +3 -0
@@ -721,6 +723,9 @@ static s32 e1000_get_cable_length_80003es2lan(struct e1000_hw *hw)
 		return ret_val;
 
 	index = phy_data & GG82563_DSPD_CABLE_LENGTH;
+	if (index >= GG82563_CABLE_LENGTH_TABLE_SIZE + 5)
+		return E1000_ERR_PHY;
+
 	phy->min_cable_length = e1000_gg82563_cable_length_table[index];
 	phy->max_cable_length = e1000_gg82563_cable_length_table[index+5];
author
#21 lore raw

PATCH 10/14 e1000e: sync change flow control variables with ixgbe

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Sync flow control variables and usage model with that found in the ixgbe driver.

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

---

 drivers/net/e1000e/82571.c   |    4 ++--
 drivers/net/e1000e/ethtool.c |   32 +++++++++++++++--------------
 drivers/net/e1000e/hw.h      |    6 +++--
 drivers/net/e1000e/ich8lan.c |   13 ++++++++----
 drivers/net/e1000e/lib.c     |   46 +++++++++++++++++++++---------------------
 drivers/net/e1000e/netdev.c  |    6 +++--
 drivers/net/e1000e/phy.c     |    4 ++--
 7 files changed, 58 insertions(+), 53 deletions(-)
drivers/net/e1000e/82571.c +2 -2
index 11e72b6..60c15cb 100644
@@ -1118,8 +1118,8 @@ static s32 e1000_setup_link_82571(struct e1000_hw *hw)
 	 * set it to full.
 	 */
 	if ((hw->mac.type == e1000_82573 || hw->mac.type == e1000_82574) &&
-	    hw->fc.type == e1000_fc_default)
-		hw->fc.type = e1000_fc_full;
+	    hw->fc.requested_mode == e1000_fc_default)
+		hw->fc.requested_mode = e1000_fc_full;
 
 	return e1000e_setup_link(hw);
 }
drivers/net/e1000e/ethtool.c +16 -16
index 62421ce..875d769 100644
@@ -249,7 +249,7 @@ static int e1000_set_settings(struct net_device *netdev,
 						     ADVERTISED_Autoneg;
 		ecmd->advertising = hw->phy.autoneg_advertised;
 		if (adapter->fc_autoneg)
-			hw->fc.original_type = e1000_fc_default;
+			hw->fc.requested_mode = e1000_fc_default;
 	} else {
 		if (e1000_set_spd_dplx(adapter, ecmd->speed + ecmd->duplex)) {
 			clear_bit(__E1000_RESETTING, &adapter->state);

@@ -279,11 +279,11 @@ static void e1000_get_pauseparam(struct net_device *netdev,
 	pause->autoneg =
 		(adapter->fc_autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE);
 
-	if (hw->fc.type == e1000_fc_rx_pause) {
+	if (hw->fc.current_mode == e1000_fc_rx_pause) {
 		pause->rx_pause = 1;
-	} else if (hw->fc.type == e1000_fc_tx_pause) {
+	} else if (hw->fc.current_mode == e1000_fc_tx_pause) {
 		pause->tx_pause = 1;
-	} else if (hw->fc.type == e1000_fc_full) {
+	} else if (hw->fc.current_mode == e1000_fc_full) {
 		pause->rx_pause = 1;
 		pause->tx_pause = 1;
 	}

@@ -301,19 +301,8 @@ static int e1000_set_pauseparam(struct net_device *netdev,
 	while (test_and_set_bit(__E1000_RESETTING, &adapter->state))
 		msleep(1);
 
-	if (pause->rx_pause && pause->tx_pause)
-		hw->fc.type = e1000_fc_full;
-	else if (pause->rx_pause && !pause->tx_pause)
-		hw->fc.type = e1000_fc_rx_pause;
-	else if (!pause->rx_pause && pause->tx_pause)
-		hw->fc.type = e1000_fc_tx_pause;
-	else if (!pause->rx_pause && !pause->tx_pause)
-		hw->fc.type = e1000_fc_none;
-
-	hw->fc.original_type = hw->fc.type;
-
 	if (adapter->fc_autoneg == AUTONEG_ENABLE) {
-		hw->fc.type = e1000_fc_default;
+		hw->fc.requested_mode = e1000_fc_default;
 		if (netif_running(adapter->netdev)) {
 			e1000e_down(adapter);
 			e1000e_up(adapter);

@@ -321,6 +310,17 @@ static int e1000_set_pauseparam(struct net_device *netdev,
 			e1000e_reset(adapter);
 		}
 	} else {
+		if (pause->rx_pause && pause->tx_pause)
+			hw->fc.requested_mode = e1000_fc_full;
+		else if (pause->rx_pause && !pause->tx_pause)
+			hw->fc.requested_mode = e1000_fc_rx_pause;
+		else if (!pause->rx_pause && pause->tx_pause)
+			hw->fc.requested_mode = e1000_fc_tx_pause;
+		else if (!pause->rx_pause && !pause->tx_pause)
+			hw->fc.requested_mode = e1000_fc_none;
+
+		hw->fc.current_mode = hw->fc.requested_mode;
+
 		retval = ((hw->phy.media_type == e1000_media_type_fiber) ?
 			  hw->mac.ops.setup_link(hw) : e1000e_force_mac_fc(hw));
 	}
drivers/net/e1000e/hw.h +3 -3
index ac1efb9..f25e961 100644
@@ -437,7 +437,7 @@ enum e1000_rev_polarity{
 	e1000_rev_polarity_undefined = 0xFF
 };
 
-enum e1000_fc_type {
+enum e1000_fc_mode {
 	e1000_fc_none = 0,
 	e1000_fc_rx_pause,
 	e1000_fc_tx_pause,

@@ -850,8 +850,8 @@ struct e1000_fc_info {
 	u16 pause_time;          /* Flow control pause timer */
 	bool send_xon;           /* Flow control send XON */
 	bool strict_ieee;        /* Strict IEEE mode */
-	enum e1000_fc_type type; /* Type of flow control */
-	enum e1000_fc_type original_type;
+	enum e1000_fc_mode current_mode; /* FC mode in effect */
+	enum e1000_fc_mode requested_mode; /* FC mode requested by caller */
 };
 
 struct e1000_dev_spec_82571 {
drivers/net/e1000e/ich8lan.c +9 -4
index 11f0dd3..5d85f5b 100644
@@ -2071,12 +2071,17 @@ static s32 e1000_setup_link_ich8lan(struct e1000_hw *hw)
 	 * the default flow control setting, so we explicitly
 	 * set it to full.
 	 */
-	if (hw->fc.type == e1000_fc_default)
-		hw->fc.type = e1000_fc_full;
+	if (hw->fc.requested_mode == e1000_fc_default)
+		hw->fc.requested_mode = e1000_fc_full;
 
-	hw->fc.original_type = hw->fc.type;
+	/*
+	 * Save off the requested flow control mode for use later.  Depending
+	 * on the link partner's capabilities, we may or may not use this mode.
+	 */
+	hw->fc.current_mode = hw->fc.requested_mode;
 
-	hw_dbg(hw, "After fix-ups FlowControl is now = %x\n", hw->fc.type);
+	hw_dbg(hw, "After fix-ups FlowControl is now = %x\n",
+		hw->fc.current_mode);
 
 	/* Continue to configure the copper link. */
 	ret_val = e1000_setup_copper_link_ich8lan(hw);
drivers/net/e1000e/lib.c +23 -23

127 lines not on this page · raw message

drivers/net/e1000e/netdev.c +3 -3
index cd6d132..f10252b 100644
@@ -2785,7 +2785,7 @@ void e1000e_reset(struct e1000_adapter *adapter)
 	else
 		fc->pause_time = E1000_FC_PAUSE_TIME;
 	fc->send_xon = 1;
-	fc->type = fc->original_type;
+	fc->current_mode = fc->requested_mode;
 
 	/* Allow time for pending master requests to run */
 	mac->ops.reset_hw(hw);

@@ -4983,8 +4983,8 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
 	/* Initialize link parameters. User can change them with ethtool */
 	adapter->hw.mac.autoneg = 1;
 	adapter->fc_autoneg = 1;
-	adapter->hw.fc.original_type = e1000_fc_default;
-	adapter->hw.fc.type = e1000_fc_default;
+	adapter->hw.fc.requested_mode = e1000_fc_default;
+	adapter->hw.fc.current_mode = e1000_fc_default;
 	adapter->hw.phy.autoneg_advertised = 0x2f;
 
 	/* ring size defaults */
drivers/net/e1000e/phy.c +2 -2
index d3aa6b7..85f8a74 100644
@@ -746,7 +746,7 @@ static s32 e1000_phy_setup_autoneg(struct e1000_hw *hw)
 	 *  other:  No software override.  The flow control configuration
 	 *	  in the EEPROM is used.
 	 */
-	switch (hw->fc.type) {
+	switch (hw->fc.current_mode) {
 	case e1000_fc_none:
 		/*
 		 * Flow control (Rx & Tx) is completely disabled by a

@@ -1116,7 +1116,7 @@ void e1000e_phy_force_speed_duplex_setup(struct e1000_hw *hw, u16 *phy_ctrl)
 	u32 ctrl;
 
 	/* Turn off flow control when forcing speed/duplex */
-	hw->fc.type = e1000_fc_none;
+	hw->fc.current_mode = e1000_fc_none;
 
 	/* Force speed/duplex on the mac */
 	ctrl = er32(CTRL);
author
#23 lore raw

PATCH 11/14 e1000e: cosmetic newline in debug message

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Add missing newline from debug message.

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

---

 drivers/net/e1000e/ich8lan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/net/e1000e/ich8lan.c +1 -1
index 5d85f5b..33898ea 100644
@@ -1895,7 +1895,7 @@ static s32 e1000_reset_hw_ich8lan(struct e1000_hw *hw)
 		ctrl |= E1000_CTRL_PHY_RST;
 	}
 	ret_val = e1000_acquire_swflag_ich8lan(hw);
-	hw_dbg(hw, "Issuing a global reset to ich8lan");
+	hw_dbg(hw, "Issuing a global reset to ich8lan\n");
 	ew32(CTRL, (ctrl | E1000_CTRL_RST));
 	msleep(20);
author
#25 lore raw

PATCH 12/14 e1000e: store EEPROM version number to prevent unnecessary NVM reads

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Rather than reading the NVM to get the EEPROM version number everytime the ethool get_drvinfo function is called, read it once during probe and save it for future reference.

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

---

 drivers/net/e1000e/e1000.h   |    1 +
 drivers/net/e1000e/ethtool.c |    8 +++-----
 drivers/net/e1000e/netdev.c  |    3 +++
 3 files changed, 7 insertions(+), 5 deletions(-)
drivers/net/e1000e/e1000.h +1 -0
index e09a181..ec94991 100644
@@ -191,6 +191,7 @@ struct e1000_adapter {
 	u16 mng_vlan_id;
 	u16 link_speed;
 	u16 link_duplex;
+	u16 eeprom_vers;
 
 	spinlock_t tx_queue_lock; /* prevent concurrent tail updates */
drivers/net/e1000e/ethtool.c +3 -5
index 875d769..840e8c4 100644
@@ -588,7 +588,6 @@ static void e1000_get_drvinfo(struct net_device *netdev,
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	char firmware_version[32];
-	u16 eeprom_data;
 
 	strncpy(drvinfo->driver,  e1000e_driver_name, 32);
 	strncpy(drvinfo->version, e1000e_driver_version, 32);

@@ -597,11 +596,10 @@ static void e1000_get_drvinfo(struct net_device *netdev,
 	 * EEPROM image version # is reported as firmware version # for
 	 * PCI-E controllers
 	 */
-	e1000_read_nvm(&adapter->hw, 5, 1, &eeprom_data);
 	sprintf(firmware_version, "%d.%d-%d",
-		(eeprom_data & 0xF000) >> 12,
-		(eeprom_data & 0x0FF0) >> 4,
-		eeprom_data & 0x000F);
+		(adapter->eeprom_vers & 0xF000) >> 12,
+		(adapter->eeprom_vers & 0x0FF0) >> 4,
+		(adapter->eeprom_vers & 0x000F));
 
 	strncpy(drvinfo->fw_version, firmware_version, 32);
 	strncpy(drvinfo->bus_info, pci_name(adapter->pdev), 32);
drivers/net/e1000e/netdev.c +3 -0
index f10252b..b3f0b47 100644
@@ -5025,6 +5025,9 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
 	adapter->wol = adapter->eeprom_wol;
 	device_set_wakeup_enable(&adapter->pdev->dev, adapter->wol);
 
+	/* save off EEPROM version number */
+	e1000_read_nvm(&adapter->hw, 5, 1, &adapter->eeprom_vers);
+
 	/* reset the hardware with the new settings */
 	e1000e_reset(adapter);
maintainer Applied
#26 lore raw

Re: [NET-NEXT PATCH 12/14] e1000e: store EEPROM version number to prevent unnecessary NVM reads

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 11:02:21 -0800

Quoted 8 lines

From: Bruce Allan <[email protected]>

Rather than reading the NVM to get the EEPROM version number everytime the ethool get_drvinfo function is called, read it once during probe and save it for future reference.

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

Applied.

author
#27 lore raw

PATCH 13/14 e1000e: fix incorrect link status when switch module pulled

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

On 82571 with SerDes, the true link state is not always correct when read from the STATUS register; use existing e1000_has_link() function instead.

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

---

 drivers/net/e1000e/e1000.h   |    1 +
 drivers/net/e1000e/ethtool.c |    7 ++-----
 drivers/net/e1000e/netdev.c  |    2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)
drivers/net/e1000e/e1000.h +1 -0
index ec94991..da6a799 100644
@@ -387,6 +387,7 @@ extern int e1000e_setup_tx_resources(struct e1000_adapter *adapter);
 extern void e1000e_free_rx_resources(struct e1000_adapter *adapter);
 extern void e1000e_free_tx_resources(struct e1000_adapter *adapter);
 extern void e1000e_update_stats(struct e1000_adapter *adapter);
+extern bool e1000_has_link(struct e1000_adapter *adapter);
 extern void e1000e_set_interrupt_capability(struct e1000_adapter *adapter);
 extern void e1000e_reset_interrupt_capability(struct e1000_adapter *adapter);
drivers/net/e1000e/ethtool.c +2 -5
index 840e8c4..34f1f63 100644
@@ -173,11 +173,8 @@ static int e1000_get_settings(struct net_device *netdev,
 static u32 e1000_get_link(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	struct e1000_hw *hw = &adapter->hw;
-	u32 status;
-	
-	status = er32(STATUS);
-	return (status & E1000_STATUS_LU) ? 1 : 0;
+
+	return e1000_has_link(adapter);
 }
 
 static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u16 spddplx)
drivers/net/e1000e/netdev.c +1 -1
index b3f0b47..b1b534d 100644
@@ -3421,7 +3421,7 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
 	       ((ctrl & E1000_CTRL_TFCE) ? "TX" : "None" )));
 }
 
-static bool e1000_has_link(struct e1000_adapter *adapter)
+bool e1000_has_link(struct e1000_adapter *adapter)
 {
 	struct e1000_hw *hw = &adapter->hw;
 	bool link_active = 0;
maintainer Applied
#28 lore raw

Re: [NET-NEXT PATCH 13/14] e1000e: fix incorrect link status when switch module pulled

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 11:02:39 -0800

Quoted 7 lines

From: Bruce Allan <[email protected]>

On 82571 with SerDes, the true link state is not always correct when read from the STATUS register; use existing e1000_has_link() function instead.

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

Applied.

author
#29 lore raw

PATCH 14/14 e1000e: check return code from NVM accesses and fix bank detection

In reply to Jeff Kirsher

From: Bruce Allan <[email protected]>

Check return code for all NVM accesses[1] and error out accordingly; log a debug message for failed accesses.

For ICH8/9, the valid NVM bank detect function was not checking whether the SEC1VAL (sector 1 valid) bit in the EECD register was itself valid (bits 8 and 9 also have to be set). If invalid, it would have defaulted to the possibly invalid bank 0. Instead, try to use the valid bank detection method used by ICH10 which has been cleaned up a bit.

[1] - reads and updates only; not writes because those are only writing to the Shadow RAM, the update following the write is the only thing actually writing the modified Shadow RAM contents to the NVM.

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

---

 drivers/net/e1000e/82571.c   |    5 +-
 drivers/net/e1000e/defines.h |    1 
 drivers/net/e1000e/ethtool.c |   35 +++++++----
 drivers/net/e1000e/ich8lan.c |  134 ++++++++++++++++++++++++++++--------------
 drivers/net/e1000e/netdev.c  |    4 +
 5 files changed, 116 insertions(+), 63 deletions(-)
drivers/net/e1000e/82571.c +3 -2
index 60c15cb..cf43ee7 100644
@@ -332,8 +332,9 @@ static s32 e1000_get_variants_82571(struct e1000_adapter *adapter)
 
 	case e1000_82573:
 		if (pdev->device == E1000_DEV_ID_82573L) {
-			e1000_read_nvm(&adapter->hw, NVM_INIT_3GIO_3, 1,
-				       &eeprom_data);
+			if (e1000_read_nvm(&adapter->hw, NVM_INIT_3GIO_3, 1,
+				       &eeprom_data) < 0)
+				break;
 			if (eeprom_data & NVM_WORD1A_ASPM_MASK)
 				adapter->flags &= ~FLAG_HAS_JUMBO_FRAMES;
 		}
drivers/net/e1000e/defines.h +1 -0
index 34a68fc..e6caf29 100644
@@ -572,6 +572,7 @@
 #define E1000_EECD_FLUPD     0x00080000 /* Update FLASH */
 #define E1000_EECD_AUPDEN    0x00100000 /* Enable Autonomous FLASH update */
 #define E1000_EECD_SEC1VAL   0x00400000 /* Sector One Valid */
+#define E1000_EECD_SEC1VAL_VALID_MASK (E1000_EECD_AUTO_RD | E1000_EECD_PRES)
 
 #define E1000_NVM_RW_REG_DATA   16   /* Offset to data in NVM read/write registers */
 #define E1000_NVM_RW_REG_DONE   2    /* Offset to READ/WRITE done bit */
drivers/net/e1000e/ethtool.c +21 -14
index 34f1f63..e48956d 100644
@@ -492,18 +492,19 @@ static int e1000_get_eeprom(struct net_device *netdev,
 		for (i = 0; i < last_word - first_word + 1; i++) {
 			ret_val = e1000_read_nvm(hw, first_word + i, 1,
 						      &eeprom_buff[i]);
-			if (ret_val) {
-				/* a read error occurred, throw away the
-				 * result */
-				memset(eeprom_buff, 0xff, sizeof(eeprom_buff));
+			if (ret_val)
 				break;
-			}
 		}
 	}
 
-	/* Device's eeprom is always little-endian, word addressable */
-	for (i = 0; i < last_word - first_word + 1; i++)
-		le16_to_cpus(&eeprom_buff[i]);
+	if (ret_val) {
+		/* a read error occurred, throw away the result */
+		memset(eeprom_buff, 0xff, sizeof(eeprom_buff));
+	} else {
+		/* Device's eeprom is always little-endian, word addressable */
+		for (i = 0; i < last_word - first_word + 1; i++)
+			le16_to_cpus(&eeprom_buff[i]);
+	}
 
 	memcpy(bytes, (u8 *)eeprom_buff + (eeprom->offset & 1), eeprom->len);
 	kfree(eeprom_buff);

@@ -555,6 +556,9 @@ static int e1000_set_eeprom(struct net_device *netdev,
 		ret_val = e1000_read_nvm(hw, last_word, 1,
 				  &eeprom_buff[last_word - first_word]);
 
+	if (ret_val)
+		goto out;
+
 	/* Device's eeprom is always little-endian, word addressable */
 	for (i = 0; i < last_word - first_word + 1; i++)
 		le16_to_cpus(&eeprom_buff[i]);

@@ -567,15 +571,18 @@ static int e1000_set_eeprom(struct net_device *netdev,
 	ret_val = e1000_write_nvm(hw, first_word,
 				  last_word - first_word + 1, eeprom_buff);
 
+	if (ret_val)
+		goto out;
+
 	/*
 	 * Update the checksum over the first part of the EEPROM if needed
-	 * and flush shadow RAM for 82573 controllers
+	 * and flush shadow RAM for applicable controllers
 	 */
-	if ((ret_val == 0) && ((first_word <= NVM_CHECKSUM_REG) ||
-			       (hw->mac.type == e1000_82574) ||
-			       (hw->mac.type == e1000_82573)))
-		e1000e_update_nvm_checksum(hw);
+	if ((first_word <= NVM_CHECKSUM_REG) ||
+	    (hw->mac.type == e1000_82574) || (hw->mac.type == e1000_82573))
+		ret_val = e1000e_update_nvm_checksum(hw);
 
+out:
 	kfree(eeprom_buff);
 	return ret_val;
 }

@@ -860,7 +867,7 @@ static int e1000_eeprom_test(struct e1000_adapter *adapter, u64 *data)
 	for (i = 0; i < (NVM_CHECKSUM_REG + 1); i++) {
 		if ((e1000_read_nvm(&adapter->hw, i, 1, &temp)) < 0) {
 			*data = 1;
-			break;
+			return *data;
 		}
 		checksum += temp;
 	}
drivers/net/e1000e/ich8lan.c +89 -45

233 lines not on this page · raw message

drivers/net/e1000e/netdev.c +2 -2
index b1b534d..ca5d3f5 100644
@@ -4723,14 +4723,14 @@ static void e1000_eeprom_checks(struct e1000_adapter *adapter)
 		return;
 
 	ret_val = e1000_read_nvm(hw, NVM_INIT_CONTROL2_REG, 1, &buf);
-	if (!(le16_to_cpu(buf) & (1 << 0))) {
+	if (!ret_val && (!(le16_to_cpu(buf) & (1 << 0)))) {
 		/* Deep Smart Power Down (DSPD) */
 		dev_warn(&adapter->pdev->dev,
 			 "Warning: detected DSPD enabled in EEPROM\n");
 	}
 
 	ret_val = e1000_read_nvm(hw, NVM_INIT_3GIO_3, 1, &buf);
-	if (le16_to_cpu(buf) & (3 << 2)) {
+	if (!ret_val && (le16_to_cpu(buf) & (3 << 2))) {
 		/* ASPM enable */
 		dev_warn(&adapter->pdev->dev,
 			 "Warning: detected ASPM enabled in EEPROM\n");
maintainer Applied
#30 lore raw

Re: [NET-NEXT PATCH 14/14] e1000e: check return code from NVM accesses and fix bank detection

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 11:02:56 -0800

Quoted 11 lines

From: Bruce Allan <[email protected]>

Check return code for all NVM accesses[1] and error out accordingly; log a debug message for failed accesses.

For ICH8/9, the valid NVM bank detect function was not checking whether the SEC1VAL (sector 1 valid) bit in the EECD register was itself valid (bits 8 and 9 also have to be set). If invalid, it would have defaulted to the possibly invalid bank 0. Instead, try to use the valid bank detection method used by ICH10 which has been cleaned up a bit.

Quoted 5 lines

[1] - reads and updates only; not writes because those are only writing to the Shadow RAM, the update following the write is the only thing actually writing the modified Shadow RAM contents to the NVM.

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

Applied.

Thanks.

maintainer Applied
#31 lore raw
In reply to Jeff Kirsher

From: Jeff Kirsher <[email protected]>
Date: Fri, 21 Nov 2008 10:59:00 -0800

Quoted 6 lines

From: Alexander Duyck <[email protected]>

There has been an issue seen with the pci-e quad port adapters that will cause them to generate a pci-e correctable error on some system while transitioning to D3.

Quoted 6 lines

Since no action is needed on this correctable error the simplest solution is to mask off the reporting of correctable errors.

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

Applied.