Re: [net-next 1/4 (V3)] net: ethtool: add the EEE support

[Ben Hutchings <bhutchings@xxxxxxxxxxxxxx>]

On Fri, 2012-04-06 at 11:29 +0200, Giuseppe CAVALLARO wrote:
> This patch adds two new functions to detect if Energy-Efficient
> Ethernet (EEE) is supported and the way to enable/disable it.
> 
> As Ben said, it is certainly necessary to distinguish:
> 
> a. PHY is advertising EEE from
> b. Link partner is advertising EEE
> or
> c. EEE will be used (= a && b)
> 
> The logic behind this code, is that .get_eee will pass
> to the user-space if the EEE is actually used and available (so point c).
> The .set_eee should used to force the MAC to disable/enable the EEE (if
> actually supported by MAC+PHY).
[...]

What I meant is that userland should be able to find out (a), and,
*separately*, either (b) or (c).  That is, there must be at least 2
separate flags for this.  In fact, I explicitly requested you define
supported/advertising/lp_advertising bitmasks for EEE link modes just
like we have for autonegotiation.  But you've made no substantive
changes in response to my review, aside from dropping the added field in
ethtool_cmd.

What you're submitting just isn't good enough for a generic interface,
as the ethtool API is supposed to be.  It's not even a good interface to
your driver.

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.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html