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.
After reading through a few other parts of the driver I believe that
the second check was actually intended to check for 'e1000_i211'
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.
Unfortunately I have no hardware to actually test this on, so it is
compile tested only.
Signed-off-by: Jesper Juhl <redacted>
---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct igb_adapter *adapter, u64 *data)gotoout;}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");
-----Original Message-----
From: Jesper Juhl [mailto:jj@chaosbits.net]
Sent: Wednesday, July 25, 2012 12:06 PM
To: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net; 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
Subject: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
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.
After reading through a few other parts of the driver I believe that the second check was actually intended to check for 'e1000_i211'
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.
Unfortunately I have no hardware to actually test this on, so it is compile tested only.
Signed-off-by: Jesper Juhl <redacted>
---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct igb_adapter *adapter, u64 *data)gotoout;}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
--
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.
ACK.
Good catch. Thanks Jesper!
Carolyn
Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation
------------------------------------------------------------------------------
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
-----Original Message-----
From: Jesper Juhl [mailto:jj@chaosbits.net]
Sent: Wednesday, July 25, 2012 12:06 PM
To: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net; 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
Subject: [PATCH] igb: correct hardware type (i210/i211) check in igb_loopback_test()
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.
After reading through a few other parts of the driver I believe that the second check was actually intended to check for 'e1000_i211'
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.
Unfortunately I have no hardware to actually test this on, so it is compile tested only.
Signed-off-by: Jesper Juhl <redacted>
---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -1783,7 +1783,7 @@ static int igb_loopback_test(struct igb_adapter *adapter, u64 *data)gotoout;}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
From: Jeff Kirsher <hidden> Date: 2012-07-31 21:12:07
On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
quoted
quoted
-----Original Message-----
From: Jesper Juhl [mailto:jj@chaosbits.net]
Sent: Wednesday, July 25, 2012 12:06 PM
To: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net;
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
@@ -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?
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:jj@chaosbits.net]
Sent: Wednesday, July 25, 2012 12:06 PM
To: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net;
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
@@ -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: [off-list 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
From: Jeff Kirsher <hidden> Date: 2012-07-31 21:31:05
On Tue, 2012-07-31 at 23:19 +0200, Jesper Juhl wrote:
On Tue, 31 Jul 2012, Jeff Kirsher wrote:
quoted
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:jj@chaosbits.net]
Sent: Wednesday, July 25, 2012 12:06 PM
To: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net;
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
@@ -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: [off-list ref]