RE: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit

RE: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit

[Fujinaka, Todd]

SSdtIGxvb2tpbmcgaW50byB0aGlzIGFuZCB3aGlsZSB0aGUgY29kZSBmaXggaXMgY29ycmVjdCwg
SSdtIG5vdCBzdXJlIHRoZSB1bmRlcmx5aW5nIGNvZGUgaXMgY29ycmVjdC4NCg0KVW5mb3J0dW5h
dGVseSwgdGhlIHBlcnNvbiB3aG8ga25vd3MgdGhlIG1vc3QgYWJvdXQgdGhpcyBpcyBvdXQgZm9y
IHNwcmluZyBicmVhay4gQ2FuIHdlIGhvbGQgb2ZmIG9uIHRoaXMgcGF0Y2g/DQoNClRvZGQgRnVq
aW5ha2ENClNvZnR3YXJlIEFwcGxpY2F0aW9uIEVuZ2luZWVyDQpOZXR3b3JraW5nIERpdmlzaW9u
IChORCkNCkludGVsIENvcnBvcmF0aW9uDQp0b2RkLmZ1amluYWthQGludGVsLmNvbQ0KKDUwMykg
NzEyLTQ1NjUNCg=

Re: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

On Thu, 2015-03-19 at 14:36 -0700, Fujinaka, Todd wrote:
> I'm looking into this and while the code fix is correct, I'm not sure
> the underlying code is correct.
> 
> Unfortunately, the person who knows the most about this is out for
> spring break. Can we hold off on this patch?

Understood, I will hold off on pushing this patch upstream until I hear
from Carolyn or you.  I will keep it in my dev-queue branch so that
validation and other testing can be done on it in the meantime.

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

RE: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit

[Wyborny, Carolyn]

[..]
> I'm looking into this and while the code fix is correct, I'm not sure the
> underlying code is correct.
> 
> Unfortunately, the person who knows the most about this is out for spring
> break. Can we hold off on this patch?

Yes, the intended fix is good.  In addition though, I think igb_enable_mas() should be changed to void and the call to it also modified, since there's no way to fail the enable.  A previous implementation had a way to fail, but the current does not.

Thanks,

Carolyn

RE: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit

[Fujinaka, Todd]

I have a fix for this already, if Dan doesn't mind.

Todd Fujinaka
Software Application Engineer
Networking Division (ND)
Intel Corporation
todd.fujinaka@intel.com
(503) 712-4565

-----Original Message-----
From: Wyborny, Carolyn 
Sent: Tuesday, March 31, 2015 11:07 AM
To: Fujinaka, Todd; Kirsher, Jeffrey T; Dan Carpenter
Cc: e1000-devel@lists.sourceforge.net; kernel-janitors@vger.kernel.org; Allan, Bruce W; Brandeburg, Jesse; Linux NICS; Ronciak, John
Subject: RE: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit

[..]
> I'm looking into this and while the code fix is correct, I'm not sure 
> the underlying code is correct.
> 
> Unfortunately, the person who knows the most about this is out for 
> spring break. Can we hold off on this patch?

Yes, the intended fix is good.  In addition though, I think igb_enable_mas() should be changed to void and the call to it also modified, since there's no way to fail the enable.  A previous implementation had a way to fail, but the current does not.

Thanks,

Carolyn

Re: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit

[Dan Carpenter]

On Tue, Mar 31, 2015 at 08:10:31PM +0000, Fujinaka, Todd wrote:
> I have a fix for this already, if Dan doesn't mind.
>

I don't mind.

regards,
dan carpenter