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 [email protected] instead of [email protected], 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. ------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-novd2d _______________________________________________ E1000-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
