Messages in this thread | | | From | Vivien Didelot <> | Subject | Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support | Date | Tue, 01 Aug 2017 11:36:13 -0400 |
| |
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> On Mon, Jul 31, 2017 at 06:17:18PM -0400, Vivien Didelot wrote: >> The PHY's EEE settings are already accessed by the DSA layer through the >> Marvell PHY driver and there is nothing to be done for switch's MACs. > > I'm confused, or missing something. Does not patch #1 mean that if the > DSA driver does not have a set_eee function, we always return -ENODEV > in slave.c?
If there is a PHY, phy_init_eee (if eee_enabled is true) and phy_ethtool_set_eee is called. If there is a .set_eee op, it is called. If both are absent, -ENODEV is returned.
> There might be nothing to configure here, but some of the switches do > support EEE. So we need at least a NOP set_eee. Better still it should > return -ENODEV for those switches which don't actually support EEE, > and 0 for those that do?
As I explain in a commit message, I didn't want to make the EEE ops mandatory, because it makes it impossible for the DSA layer to distinguish whether the driver did not update the ethtool_eee structure because there is nothing to do on the port's MAC side (e.g. mv88e6xxx or qca8k) or if it returned EEE disabled. To avoid confusion, I prefered to make the ops optional, making the phy_* calls enough in the first case.
That being said, if you don't share this point of view and prefer to define an inline dsa_set_eee_noop() function, I don't mind, since this allows the DSA layer to make the distinction.
Thanks,
Vivien
| |