On 04/27/2012 05:11 PM, Giuseppe CAVALLARO wrote:

On 4/26/2012 7:17 PM, Ben Hutchings wrote:

quoted

On Thu, 2012-04-26 at 09:48 +0200, Giuseppe CAVALLARO wrote:

quoted

Hello Ben

On 4/19/2012 5:30 PM, Ben Hutchings wrote:
[snip]

quoted

quoted

I'm changing the code for getting/setting the EEE capability and trying
to follow your suggestions.

The "get" will show the following things; this is a bit different of the
points "a" "b" and "c" we had discussed. Maybe, this could also be a
more complete (*) .
The ethtool (see output below as example) could report the phy
(supported/advertised/lp_advertised) and mac eee capabilities separately.

Sounds reasonable.

quoted

The "set" will be useful for some eth devices (like the stmmac) that can
stop/enable internally the eee capability (at mac level).

I don't know much about EEE, but shouldn't the driver take care of
configuring the MAC for this whenever the PHY is set to advertise EEE
capability?

Yes indeed this can be done at driver level. So could I definitely
remove it from ethtool? What do you suggest?

In case of the stmmac I could add a specific driver option via sys to
enable/disable the eee and set timer.

Generally, ethtool doesn't distinguish MAC and PHY settings because they
have to be configured consistently for the device to do anything useful.
If there is some good use for enabling EEE in the MAC and not the PHY,
or vice versa, then this should be exposed in the ethtool interface.
But if not then I don't believe it needs to be in either an ethtool or a
driver-specific interface.

Thanks Ben for this clarification: in case of the stmmac the option is
useful to stop a timer to enter in lpi state for the tx.
So it's worth having that and from ethtool.

How will a user turn off EEE support using this implementation?

Are you suggesting a "set" that works similarly to the control of the pause
parameters - that is, a user could either shutdown EEE or only Tx, which
will mean to the driver "don't enter Tx LPI mode"?

Keep in mind that if later an interface controlling the LPI timers would be
added (as a measure of user control to the power saving vs. latency issue),
it could make this 'partial' closure interface redundant.

Perhaps "set" should only turn the EEE feature on/off entirely (adv. them or
not, since clearly the link will have to be re-established afterwards), and
we should have a different function that prevents entry into LPI mode in Tx
- one whose functionality could later on be extended.

Regards,
Yuval