Re: [net-next 4/6] e1000: configure and read MDI settings

On Fri, 2012-07-20 at 18:17 -0700, Brandeburg, Jesse wrote:

On Fri, 20 Jul 2012, Ben Hutchings wrote:

quoted

Why don't you set ecmd->eth_tp_mdix_ctrl here?

If you also leave it as 0, it's impossible for userland to tell whether
the current mode was forced or automatically selected.

Thanks for the review, right now the get interface (and ethtool display) 
doesn't support any way to report if the setting was forced or not.  I 
didn't think about changing the get because I didn't want to modify the 
userland reporting (I also figured it was a simple interface right now, 
and didn't need changing, and was focused on the _set_ which is the part 
fixing the users' reported bugs.)

Everything else you can change with ETHTOOL_SSET is also reported by
ETHTOOL_GSET; why would this be any different?

I think the patches as they currently stand are okay, do you agree? I 
would be glad to submit a followon to implement the new "get" interface if 
we can hash out the interface changes, but I see no reason to gate these 
patches.

You left this for 20 months, what's the hurry now?

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.