netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] igb: cleanup and code deduplication
@ 2011-03-29 13:09 Stefan Assmann
  2011-03-29 13:09 ` [PATCH 1/3] igb: fix typo in igb_validate_nvm_checksum_82580 Stefan Assmann
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Stefan Assmann @ 2011-03-29 13:09 UTC (permalink / raw)
  To: netdev; +Cc: e1000-devel, jeffrey.t.kirsher, alexander.h.duyck, sassmann

Some cleanups for igb.
First patch is just a simple typo fix.
Second patch turns igb_update_nvm_checksum and igb_validate_nvm_checksum
into wrapper functions to deduplicate code.
Third patch puts the thermal sensor code into a function. This patch could use
some testing as I couldn't really test it due to lack of hardware.

  Stefan

Stefan Assmann (3):
  igb: fix typo in igb_validate_nvm_checksum_82580
  igb: transform igb_{update,validate}_nvm_checksum into wrappers of
    their *_with_offset equivalents
  igb: introduce igb_thermal_sensor_event for sensor checking

 drivers/net/igb/e1000_82575.c |   79 ++--------------------------------------
 drivers/net/igb/e1000_nvm.c   |   38 +++++++++++++++----
 drivers/net/igb/e1000_nvm.h   |    2 +
 drivers/net/igb/igb_main.c    |   67 +++++++++++++++++-----------------
 4 files changed, 70 insertions(+), 116 deletions(-)

-- 
1.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] igb: fix typo in igb_validate_nvm_checksum_82580
  2011-03-29 13:09 [PATCH 0/3] igb: cleanup and code deduplication Stefan Assmann
@ 2011-03-29 13:09 ` Stefan Assmann
  2011-03-29 13:09 ` [PATCH 2/3] igb: transform igb_{update,validate}_nvm_checksum into wrappers of their *_with_offset equivalents Stefan Assmann
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Stefan Assmann @ 2011-03-29 13:09 UTC (permalink / raw)
  To: netdev; +Cc: e1000-devel, jeffrey.t.kirsher, alexander.h.duyck, sassmann

Comment spelling fix.

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/igb/e1000_82575.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/igb/e1000_82575.c b/drivers/net/igb/e1000_82575.c
index 6b256c2..0cd41c4 100644
--- a/drivers/net/igb/e1000_82575.c
+++ b/drivers/net/igb/e1000_82575.c
@@ -1877,7 +1877,7 @@ static s32 igb_validate_nvm_checksum_82580(struct e1000_hw *hw)
 	}
 
 	if (nvm_data & NVM_COMPATIBILITY_BIT_MASK) {
-		/* if chekcsums compatibility bit is set validate checksums
+		/* if checksums compatibility bit is set validate checksums
 		 * for all 4 ports. */
 		eeprom_regions_count = 4;
 	}
@@ -1988,6 +1988,7 @@ static s32 igb_update_nvm_checksum_i350(struct e1000_hw *hw)
 out:
 	return ret_val;
 }
+
 /**
  *  igb_set_eee_i350 - Enable/disable EEE support
  *  @hw: pointer to the HW structure
-- 
1.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] igb: transform igb_{update,validate}_nvm_checksum into wrappers of their *_with_offset equivalents
  2011-03-29 13:09 [PATCH 0/3] igb: cleanup and code deduplication Stefan Assmann
  2011-03-29 13:09 ` [PATCH 1/3] igb: fix typo in igb_validate_nvm_checksum_82580 Stefan Assmann
@ 2011-03-29 13:09 ` Stefan Assmann
  2011-04-04  7:23   ` Stefan Assmann
  2011-03-29 13:09 ` [PATCH 3/3] igb: introduce igb_thermal_sensor_event for sensor checking Stefan Assmann
  2011-03-30  0:00 ` [PATCH 0/3] igb: cleanup and code deduplication Jeff Kirsher
  3 siblings, 1 reply; 6+ messages in thread
From: Stefan Assmann @ 2011-03-29 13:09 UTC (permalink / raw)
  To: netdev; +Cc: e1000-devel, jeffrey.t.kirsher, alexander.h.duyck, sassmann

igb_update_nvm_checksum_with_offset and igb_update_nvm_checksum are similar
except one additionally handles an offset.
Move igb_update_nvm_checksum_with_offset to e1000_nvm.c and transform
igb_update_nvm_checksum to a simple wrapper of
igb_update_nvm_checksum_with_offset.

Exactly the same is done for igb_validate_nvm_checksum.

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/igb/e1000_82575.c |   76 +---------------------------------------
 drivers/net/igb/e1000_nvm.c   |   38 ++++++++++++++++----
 drivers/net/igb/e1000_nvm.h   |    2 +
 3 files changed, 34 insertions(+), 82 deletions(-)

diff --git a/drivers/net/igb/e1000_82575.c b/drivers/net/igb/e1000_82575.c
index 0cd41c4..11f7519 100644
--- a/drivers/net/igb/e1000_82575.c
+++ b/drivers/net/igb/e1000_82575.c
@@ -66,12 +66,8 @@ static s32  igb_set_pcie_completion_timeout(struct e1000_hw *hw);
 static s32  igb_reset_mdicnfg_82580(struct e1000_hw *hw);
 static s32  igb_validate_nvm_checksum_82580(struct e1000_hw *hw);
 static s32  igb_update_nvm_checksum_82580(struct e1000_hw *hw);
-static s32  igb_update_nvm_checksum_with_offset(struct e1000_hw *hw,
-						u16 offset);
-static s32 igb_validate_nvm_checksum_with_offset(struct e1000_hw *hw,
-						u16 offset);
-static s32 igb_validate_nvm_checksum_i350(struct e1000_hw *hw);
-static s32 igb_update_nvm_checksum_i350(struct e1000_hw *hw);
+static s32  igb_validate_nvm_checksum_i350(struct e1000_hw *hw);
+static s32  igb_update_nvm_checksum_i350(struct e1000_hw *hw);
 static const u16 e1000_82580_rxpbs_table[] =
 	{ 36, 72, 144, 1, 2, 4, 8, 16,
 	  35, 70, 140 };
@@ -1788,74 +1784,6 @@ u16 igb_rxpbs_adjust_82580(u32 data)
 }
 
 /**
- *  igb_validate_nvm_checksum_with_offset - Validate EEPROM
- *  checksum
- *  @hw: pointer to the HW structure
- *  @offset: offset in words of the checksum protected region
- *
- *  Calculates the EEPROM checksum by reading/adding each word of the EEPROM
- *  and then verifies that the sum of the EEPROM is equal to 0xBABA.
- **/
-s32 igb_validate_nvm_checksum_with_offset(struct e1000_hw *hw, u16 offset)
-{
-	s32 ret_val = 0;
-	u16 checksum = 0;
-	u16 i, nvm_data;
-
-	for (i = offset; i < ((NVM_CHECKSUM_REG + offset) + 1); i++) {
-		ret_val = hw->nvm.ops.read(hw, i, 1, &nvm_data);
-		if (ret_val) {
-			hw_dbg("NVM Read Error\n");
-			goto out;
-		}
-		checksum += nvm_data;
-	}
-
-	if (checksum != (u16) NVM_SUM) {
-		hw_dbg("NVM Checksum Invalid\n");
-		ret_val = -E1000_ERR_NVM;
-		goto out;
-	}
-
-out:
-	return ret_val;
-}
-
-/**
- *  igb_update_nvm_checksum_with_offset - Update EEPROM
- *  checksum
- *  @hw: pointer to the HW structure
- *  @offset: offset in words of the checksum protected region
- *
- *  Updates the EEPROM checksum by reading/adding each word of the EEPROM
- *  up to the checksum.  Then calculates the EEPROM checksum and writes the
- *  value to the EEPROM.
- **/
-s32 igb_update_nvm_checksum_with_offset(struct e1000_hw *hw, u16 offset)
-{
-	s32 ret_val;
-	u16 checksum = 0;
-	u16 i, nvm_data;
-
-	for (i = offset; i < (NVM_CHECKSUM_REG + offset); i++) {
-		ret_val = hw->nvm.ops.read(hw, i, 1, &nvm_data);
-		if (ret_val) {
-			hw_dbg("NVM Read Error while updating checksum.\n");
-			goto out;
-		}
-		checksum += nvm_data;
-	}
-	checksum = (u16) NVM_SUM - checksum;
-	ret_val = hw->nvm.ops.write(hw, (NVM_CHECKSUM_REG + offset), 1,
-				&checksum);
-	if (ret_val)
-		hw_dbg("NVM Write Error while updating checksum.\n");
-
-out:
-	return ret_val;
-}
-
-/**
  *  igb_validate_nvm_checksum_82580 - Validate EEPROM checksum
  *  @hw: pointer to the HW structure
  *
diff --git a/drivers/net/igb/e1000_nvm.c b/drivers/net/igb/e1000_nvm.c
index 75bf36a..0f1ec3f 100644
--- a/drivers/net/igb/e1000_nvm.c
+++ b/drivers/net/igb/e1000_nvm.c
@@ -648,19 +648,21 @@ s32 igb_read_mac_addr(struct e1000_hw *hw)
 }
 
 /**
- *  igb_validate_nvm_checksum - Validate EEPROM checksum
+ *  igb_validate_nvm_checksum_with_offset - Validate EEPROM
+ *  checksum
  *  @hw: pointer to the HW structure
+ *  @offset: offset in words of the checksum protected region
  *
  *  Calculates the EEPROM checksum by reading/adding each word of the EEPROM
  *  and then verifies that the sum of the EEPROM is equal to 0xBABA.
  **/
-s32 igb_validate_nvm_checksum(struct e1000_hw *hw)
+s32 igb_validate_nvm_checksum_with_offset(struct e1000_hw *hw, u16 offset)
 {
 	s32 ret_val = 0;
 	u16 checksum = 0;
 	u16 i, nvm_data;
 
-	for (i = 0; i < (NVM_CHECKSUM_REG + 1); i++) {
+	for (i = offset; i < ((NVM_CHECKSUM_REG + offset) + 1); i++) {
 		ret_val = hw->nvm.ops.read(hw, i, 1, &nvm_data);
 		if (ret_val) {
 			hw_dbg("NVM Read Error\n");
@@ -680,20 +682,31 @@ out:
 }
 
 /**
- *  igb_update_nvm_checksum - Update EEPROM checksum
+ *  igb_validate_nvm_checksum - Validate EEPROM checksum
+ *  @hw: pointer to the HW structure
+ **/
+s32 igb_validate_nvm_checksum(struct e1000_hw *hw)
+{
+	return igb_validate_nvm_checksum_with_offset(hw, 0);
+}
+
+/**
+ *  igb_update_nvm_checksum_with_offset - Update EEPROM
+ *  checksum
  *  @hw: pointer to the HW structure
+ *  @offset: offset in words of the checksum protected region
  *
  *  Updates the EEPROM checksum by reading/adding each word of the EEPROM
  *  up to the checksum.  Then calculates the EEPROM checksum and writes the
  *  value to the EEPROM.
  **/
-s32 igb_update_nvm_checksum(struct e1000_hw *hw)
+s32 igb_update_nvm_checksum_with_offset(struct e1000_hw *hw, u16 offset)
 {
-	s32  ret_val;
+	s32 ret_val;
 	u16 checksum = 0;
 	u16 i, nvm_data;
 
-	for (i = 0; i < NVM_CHECKSUM_REG; i++) {
+	for (i = offset; i < (NVM_CHECKSUM_REG + offset); i++) {
 		ret_val = hw->nvm.ops.read(hw, i, 1, &nvm_data);
 		if (ret_val) {
 			hw_dbg("NVM Read Error while updating checksum.\n");
@@ -702,7 +715,8 @@ s32 igb_update_nvm_checksum(struct e1000_hw *hw)
 		checksum += nvm_data;
 	}
 	checksum = (u16) NVM_SUM - checksum;
-	ret_val = hw->nvm.ops.write(hw, NVM_CHECKSUM_REG, 1, &checksum);
+	ret_val = hw->nvm.ops.write(hw, (NVM_CHECKSUM_REG + offset), 1,
+				&checksum);
 	if (ret_val)
 		hw_dbg("NVM Write Error while updating checksum.\n");
 
@@ -710,3 +724,11 @@ out:
 	return ret_val;
 }
 
+/**
+ *  igb_update_nvm_checksum - Update EEPROM checksum
+ *  @hw: pointer to the HW structure
+ **/
+s32 igb_update_nvm_checksum(struct e1000_hw *hw)
+{
+	return igb_update_nvm_checksum_with_offset(hw, 0);
+}
diff --git a/drivers/net/igb/e1000_nvm.h b/drivers/net/igb/e1000_nvm.h
index 7f43564..b13b405 100644
--- a/drivers/net/igb/e1000_nvm.h
+++ b/drivers/net/igb/e1000_nvm.h
@@ -38,6 +38,8 @@ s32  igb_read_nvm_eerd(struct e1000_hw *hw, u16 offset, u16 words, u16 *data);
 s32  igb_read_nvm_spi(struct e1000_hw *hw, u16 offset, u16 words, u16 *data);
 s32  igb_write_nvm_spi(struct e1000_hw *hw, u16 offset, u16 words, u16 *data);
 s32  igb_validate_nvm_checksum(struct e1000_hw *hw);
+s32  igb_validate_nvm_checksum_with_offset(struct e1000_hw *hw, u16 offset);
 s32  igb_update_nvm_checksum(struct e1000_hw *hw);
+s32  igb_update_nvm_checksum_with_offset(struct e1000_hw *hw, u16 offset);
 
 #endif
-- 
1.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] igb: introduce igb_thermal_sensor_event for sensor checking
  2011-03-29 13:09 [PATCH 0/3] igb: cleanup and code deduplication Stefan Assmann
  2011-03-29 13:09 ` [PATCH 1/3] igb: fix typo in igb_validate_nvm_checksum_82580 Stefan Assmann
  2011-03-29 13:09 ` [PATCH 2/3] igb: transform igb_{update,validate}_nvm_checksum into wrappers of their *_with_offset equivalents Stefan Assmann
@ 2011-03-29 13:09 ` Stefan Assmann
  2011-03-30  0:00 ` [PATCH 0/3] igb: cleanup and code deduplication Jeff Kirsher
  3 siblings, 0 replies; 6+ messages in thread
From: Stefan Assmann @ 2011-03-29 13:09 UTC (permalink / raw)
  To: netdev; +Cc: e1000-devel, jeffrey.t.kirsher, alexander.h.duyck, sassmann

The code for thermal sensor checking should be wrapped into a function.

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/igb/igb_main.c |   67 ++++++++++++++++++++++---------------------
 1 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 3d850af..cea2f7f 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -3532,6 +3532,25 @@ bool igb_has_link(struct igb_adapter *adapter)
 	return link_active;
 }
 
+static bool igb_thermal_sensor_event(struct e1000_hw *hw, u32 event)
+{
+	bool ret = false;
+	u32 ctrl_ext, thstat;
+
+	/* check for thermal sensor event on i350, copper only */
+	if (hw->mac.type == e1000_i350) {
+		thstat = rd32(E1000_THSTAT);
+		ctrl_ext = rd32(E1000_CTRL_EXT);
+
+		if ((hw->phy.media_type == e1000_media_type_copper) &&
+		    !(ctrl_ext & E1000_CTRL_EXT_LINK_MODE_SGMII)) {
+			ret = !!(thstat & event);
+		}
+	}
+
+	return ret;
+}
+
 /**
  * igb_watchdog - Timer Call-back
  * @data: pointer to adapter cast into an unsigned long
@@ -3550,7 +3569,7 @@ static void igb_watchdog_task(struct work_struct *work)
                                                    watchdog_task);
 	struct e1000_hw *hw = &adapter->hw;
 	struct net_device *netdev = adapter->netdev;
-	u32 link, ctrl_ext, thstat;
+	u32 link;
 	int i;
 
 	link = igb_has_link(adapter);
@@ -3574,25 +3593,14 @@ static void igb_watchdog_task(struct work_struct *work)
 			       ((ctrl & E1000_CTRL_RFCE) ?  "RX" :
 			       ((ctrl & E1000_CTRL_TFCE) ?  "TX" : "None")));
 
-			/* check for thermal sensor event on i350,
-			 * copper only */
-			if (hw->mac.type == e1000_i350) {
-				thstat = rd32(E1000_THSTAT);
-				ctrl_ext = rd32(E1000_CTRL_EXT);
-				if ((hw->phy.media_type ==
-				     e1000_media_type_copper) && !(ctrl_ext &
-				     E1000_CTRL_EXT_LINK_MODE_SGMII)) {
-					if (thstat &
-					    E1000_THSTAT_LINK_THROTTLE) {
-						printk(KERN_INFO "igb: %s The "
-						       "network adapter link "
-						       "speed was downshifted "
-						       "because it "
-						       "overheated.\n",
-						       netdev->name);
-					}
-				}
+			/* check for thermal sensor event */
+			if (igb_thermal_sensor_event(hw, E1000_THSTAT_LINK_THROTTLE)) {
+				printk(KERN_INFO "igb: %s The network adapter "
+						 "link speed was downshifted "
+						 "because it overheated.\n",
+						 netdev->name);
 			}
+
 			/* adjust timeout factor according to speed/duplex */
 			adapter->tx_timeout_factor = 1;
 			switch (adapter->link_speed) {
@@ -3618,22 +3626,15 @@ static void igb_watchdog_task(struct work_struct *work)
 		if (netif_carrier_ok(netdev)) {
 			adapter->link_speed = 0;
 			adapter->link_duplex = 0;
-			/* check for thermal sensor event on i350
-			 * copper only*/
-			if (hw->mac.type == e1000_i350) {
-				thstat = rd32(E1000_THSTAT);
-				ctrl_ext = rd32(E1000_CTRL_EXT);
-				if ((hw->phy.media_type ==
-				     e1000_media_type_copper) && !(ctrl_ext &
-				     E1000_CTRL_EXT_LINK_MODE_SGMII)) {
-					if (thstat & E1000_THSTAT_PWR_DOWN) {
-						printk(KERN_ERR "igb: %s The "
-						"network adapter was stopped "
-						"because it overheated.\n",
+
+			/* check for thermal sensor event */
+			if (igb_thermal_sensor_event(hw, E1000_THSTAT_PWR_DOWN)) {
+				printk(KERN_ERR "igb: %s The network adapter "
+						"was stopped because it "
+						"overheated.\n",
 						netdev->name);
-					}
-				}
 			}
+
 			/* Links status message must follow this format */
 			printk(KERN_INFO "igb: %s NIC Link is Down\n",
 			       netdev->name);
-- 
1.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/3] igb: cleanup and code deduplication
  2011-03-29 13:09 [PATCH 0/3] igb: cleanup and code deduplication Stefan Assmann
                   ` (2 preceding siblings ...)
  2011-03-29 13:09 ` [PATCH 3/3] igb: introduce igb_thermal_sensor_event for sensor checking Stefan Assmann
@ 2011-03-30  0:00 ` Jeff Kirsher
  3 siblings, 0 replies; 6+ messages in thread
From: Jeff Kirsher @ 2011-03-30  0:00 UTC (permalink / raw)
  To: Stefan Assmann; +Cc: e1000-devel, netdev

On Tue, Mar 29, 2011 at 06:09, Stefan Assmann <sassmann@kpanic.de> wrote:
> Some cleanups for igb.
> First patch is just a simple typo fix.
> Second patch turns igb_update_nvm_checksum and igb_validate_nvm_checksum
> into wrapper functions to deduplicate code.
> Third patch puts the thermal sensor code into a function. This patch could use
> some testing as I couldn't really test it due to lack of hardware.
>
>  Stefan
>
> Stefan Assmann (3):
>  igb: fix typo in igb_validate_nvm_checksum_82580
>  igb: transform igb_{update,validate}_nvm_checksum into wrappers of
>    their *_with_offset equivalents
>  igb: introduce igb_thermal_sensor_event for sensor checking
>
>  drivers/net/igb/e1000_82575.c |   79 ++--------------------------------------
>  drivers/net/igb/e1000_nvm.c   |   38 +++++++++++++++----
>  drivers/net/igb/e1000_nvm.h   |    2 +
>  drivers/net/igb/igb_main.c    |   67 +++++++++++++++++-----------------
>  4 files changed, 70 insertions(+), 116 deletions(-)
>
>

Thanks Stefan, I have added these patches to my igb-queue.

-- 
Cheers,
Jeff

------------------------------------------------------------------------------
Enable your software for Intel(R) Active Management Technology to meet the
growing manageability and security demands of your customers. Businesses
are taking advantage of Intel(R) vPro (TM) technology - will your software 
be a part of the solution? Download the Intel(R) Manageability Checker 
today! http://p.sf.net/sfu/intel-dev2devmar
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] igb: transform igb_{update,validate}_nvm_checksum into wrappers of their *_with_offset equivalents
  2011-03-29 13:09 ` [PATCH 2/3] igb: transform igb_{update,validate}_nvm_checksum into wrappers of their *_with_offset equivalents Stefan Assmann
@ 2011-04-04  7:23   ` Stefan Assmann
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Assmann @ 2011-04-04  7:23 UTC (permalink / raw)
  To: netdev; +Cc: e1000-devel, jeffrey.t.kirsher, alexander.h.duyck

On 29.03.2011 15:09, Stefan Assmann wrote:
> igb_update_nvm_checksum_with_offset and igb_update_nvm_checksum are similar
> except one additionally handles an offset.
> Move igb_update_nvm_checksum_with_offset to e1000_nvm.c and transform
> igb_update_nvm_checksum to a simple wrapper of
> igb_update_nvm_checksum_with_offset.
> 
> Exactly the same is done for igb_validate_nvm_checksum.
> 
> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>

Please ignore this for now, Intel will send a replacement patch.

  Stefan

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-04-04  7:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-29 13:09 [PATCH 0/3] igb: cleanup and code deduplication Stefan Assmann
2011-03-29 13:09 ` [PATCH 1/3] igb: fix typo in igb_validate_nvm_checksum_82580 Stefan Assmann
2011-03-29 13:09 ` [PATCH 2/3] igb: transform igb_{update,validate}_nvm_checksum into wrappers of their *_with_offset equivalents Stefan Assmann
2011-04-04  7:23   ` Stefan Assmann
2011-03-29 13:09 ` [PATCH 3/3] igb: introduce igb_thermal_sensor_event for sensor checking Stefan Assmann
2011-03-30  0:00 ` [PATCH 0/3] igb: cleanup and code deduplication Jeff Kirsher

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).