lkml.org 
[lkml]   [2011]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH net-next v1 3/4] net-e1000e: reworked carrier detection logic
From
David,

I know you asked for a review of this on Friday, before you sent this
out to netdev, and I was planning on doing so, but I see you've sent
it out to a wider audience now.
So, I'll attach comments directly here.

You took my patch (which was for 2.6.34) and split it in two (it was
erroneously marked author zenczykowski@gmail.com instead of
maze@google.com, so that should be fixed) and while you seem to have
reduced the amount of lines-of-code-churn, I'm not sure you've
actually made the resulting code (as a whole) more readable.

I'm not sure how much of these changes were a result of changes
elsewhere in the driver between 2.6.34 and now.

This was very tricky code to get right, and I'm not sure whether the
patch as you posted it gets all the corner cases right...
Indeed I actually think the changes you've made reduce the robustness
wrt. timeouts caused by firmware concurrent accesses. In particular
the first read access you make is very likely to fail.
I think renaming success to link and iterations to intervals was
worthwhile since it improved readability...

For reference, the original patch, cherrypicked onto net-next and
compile (but not anything else) tested will be a reply to this email.


\
 
 \ /
  Last update: 2011-11-22 21:35    [W:1.173 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site