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

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

[Jesper Juhl]

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 <jj@chaosbits.net>
---
 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
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)
 		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


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

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

[Wyborny, Carolyn]

-----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 <jj@chaosbits.net>
---
 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
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)
 		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


-- 
Jesper Juhl <jj@chaosbits.net>       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&#174; Ethernet, visit http://communities.intel.com/community/wired

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

[Jesper Juhl]

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

> 
> > -----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 <jj@chaosbits.net>
> > ---
> >  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
> > 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)
> >  		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?


-- 
Jesper Juhl <jj@chaosbits.net>       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&#174; Ethernet, visit http://communities.intel.com/community/wired

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

[Jeff Kirsher]

[-- Attachment #1.1: Type: text/plain, Size: 2778 bytes --]

On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
> On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
> 
> > 
> > > -----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 <jj@chaosbits.net>
> > > ---
> > >  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
> > > 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)
> > >             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?

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 395 bytes --]

------------------------------------------------------------------------------
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/

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
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

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

[Jesper Juhl]

On Tue, 31 Jul 2012, Jeff Kirsher wrote:

> On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
> > On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
> > 
> > > 
> > > > -----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 <jj@chaosbits.net>
> > > > ---
> > > >  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
> > > > 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)
> > > >             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: <alpine.LNX.2.00.1207252115321.11754@swampdragon.chaosbits.net>


-- 
Jesper Juhl <jj@chaosbits.net>       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&#174; Ethernet, visit http://communities.intel.com/community/wired

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

[Jeff Kirsher]

[-- Attachment #1.1: Type: text/plain, Size: 3506 bytes --]

On Tue, 2012-07-31 at 23:19 +0200, Jesper Juhl wrote:
> On Tue, 31 Jul 2012, Jeff Kirsher wrote:
> 
> > On Tue, 2012-07-31 at 22:23 +0200, Jesper Juhl wrote:
> > > On Wed, 25 Jul 2012, Wyborny, Carolyn wrote:
> > > 
> > > > 
> > > > > -----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 <jj@chaosbits.net>
> > > > > ---
> > > > >  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
> > > > > 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)
> > > > >             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: <alpine.LNX.2.00.1207252115321.11754@swampdragon.chaosbits.net>

Done.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 395 bytes --]

------------------------------------------------------------------------------
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/

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
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