Re: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()

On Tue, 31 Jul 2012, Jeff Kirsher wrote:

On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:

quoted

On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:

quoted

quoted

-----Original Message-----
From: Jesper Juhl [mailto:<redacted>] 
Sent: Wednesday, July 25, 2012 12:06 PM
To: <redacted>
Cc: <redacted>; <redacted>;

Wyborny, Carolyn; Pieper, Jeffrey E; Kirsher, Jeffrey T; Rick Jones;
Ronciak, John; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C;
Rose, Gregory V; Waskiewicz Jr, Peter P; Duyck, Alexander H; David S.
Miller

quoted

quoted

Subject: [PATCH] igb: correct hardware type (i210/i211) check in

igb_loopback_test()

quoted

quoted

In the original code
...
    if ((adapter->hw.mac.type == e1000_i210)
            || (adapter->hw.mac.type == e1000_i210)) { ...
the second check of 'adapter->hw.mac.type' is pointless since it

tests for the exact same value as the first.

quoted

quoted

After reading through a few other parts of the driver I believe

that the second check was actually intended to check for 'e1000_i211'

quoted

quoted

rather than 'e1000_i210', but I admit that I'm not certain so

someone with more knowledge about this driver should ACK the patch
before it gets merged.

quoted

quoted

Unfortunately I have no hardware to actually test this on, so it

is compile tested only.

quoted

quoted

Signed-off-by: Jesper Juhl <redacted>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c

b/drivers/net/ethernet/intel/igb/igb_ethtool.c

quoted

quoted

index a19c84c..ad489b7 100644

--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c

@@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct

igb_adapter *adapter, u64 *data)

quoted

quoted

            goto out;
    }
    if ((adapter->hw.mac.type == e1000_i210)
-           || (adapter->hw.mac.type == e1000_i210)) {
+           || (adapter->hw.mac.type == e1000_i211)) {
            dev_err(&adapter->pdev->dev,
                    "Loopback test not supported "
                    "on this part at this time.\n");
--
1.7.11.3

ACK.

Good catch.  Thanks Jesper!

Carolyn

Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation

Thank you for the ack Carolyn.

David: will you take this in the networking tree? 

I have a few igb patches already, so I will send a pull request for Dave
with this patch added to the igb patches I already have.  Ok?

That's perfectly fine with me.

Perhaps you could consider also picking up the other one I sent a few 
minutes after this one ?
  Subject: [PATCH] igb: don't break user visible strings over multiple lines in igb_ethtool.c
  Message-ID: [ref]


-- 
Jesper Juhl [off-list ref]       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired