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

[Jeff Kirsher <jeffrey.t.kirsher@intel.com>]

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