Re: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit
On Tue, Mar 31, 2015 at 08:10:31PM +, Fujinaka, Todd wrote: I have a fix for this already, if Dan doesn't mind. I don't mind. regards, dan carpenter -- Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ 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: [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 -- Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ 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: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit
I have a fix for this already, if Dan doesn't mind. Todd Fujinaka Software Application Engineer Networking Division (ND) Intel Corporation todd.fujin...@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-janit...@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 -- Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ 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: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit
On Thu, 2015-03-19 at 16:31 +0300, Dan Carpenter wrote:
Static checkers complain about this because we do:
if (!(connsw E1000_CONNSW_SERDESD)) {
...
} else if (connsw E1000_CONNSW_SERDESD) {
...
} else {
...
}
Once you eliminate that E1000_CONNSW_SERDESD is set and not set then
there aren't any other possibilities so the else statement is dead
code.
This function always returns zero so if you delete the ret_val
variable, the code is shorter and more clear.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
Thanks Dan, surprised we did not see this earlier. I have added your
patch to my queue.
--
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git
dev-queue
signature.asc
Description: This is a digitally signed message part
--
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/___
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: [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? Todd Fujinaka Software Application Engineer Networking Division (ND) Intel Corporation todd.fujin...@intel.com (503) 712-4565 -- Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ 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: [E1000-devel] [patch] igb: cleanup igb_enable_mas() a bit
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. signature.asc Description: This is a digitally signed message part -- Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/___ 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
