On Thu, 2012-04-19 at 14:58 +0200, Giuseppe CAVALLARO wrote:

Hello Ben,

On 4/16/2012 7:41 AM, Giuseppe CAVALLARO wrote:
[snip]

quoted

quoted

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.

Sorry Ben but I believed that (c) was enough.

quoted

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.

yes! I'll rework this and provide new patches asap.

sorry if I disturb you but I want to be sure to avoid to forget
something else in the next EEE patches (avoiding to continuously disturb
you).

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.

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?

What do you think?

Regards
Peppe

----

# ./ethtool  eth0
Settings for eth0:

[snip]

	Current message level: 0x0000003f (63)
			       drv probe link timer ifdown ifup
	Link detected: yes
	Energy-Efficient Ethernet:           -------------------------
		MAC supports: yes            |-> related to MAC side  |
		phy supports modes: ...      |-> from MMD 3.20        |
		phy advertising modes: ...   |-> from MMD 7.60        |
		LP advertising modes: ...    |-> from MMD 7.61        |
					     --------------------------
                                                    (*)
PS. The "..." above means that we can actually dump: 100BASE-TX EEE etc
for each advertising modes and also for phy support (reg 3.20).

Yes, that's the sort of information I would expect to see (but try to be
consistent with the wording used for AN).

The EEE advertising mask presumably should be changeable similarly to
the AN advertising mask ('ethtool -s <devname> eeeadv <mask>').  But I
don't know how the two advertising masks interact.  Is one supposed to
be a subset of the other?  Currently ethtool automatically changes the
AN advertising mask in response to a speed/duplex change; should it also
try to change the EEE advertising mask?

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.