On 09/13/2016 11:07 AM, John Crispin wrote: > > > On 13/09/2016 19:09, Florian Fainelli wrote: >> On 09/13/2016 08:59 AM, Andrew Lunn wrote: >>>> Hi Andrew, >>>> >>>> this function does indeed duplicate the functionality of >>>> phy_ethtool_get_eee() with the small difference, that e->eee_active is >>>> also set which phy_ethtool_get_eee() does not set. >>>> >>>> dsa_slave_get_eee() will call phy_ethtool_get_eee() right after the >>>> get_eee() op has been called. would it be ok to move the code setting >>>> eee_active to phy_ethtool_get_eee(). >> >> Humm, AFAIR, the reason why eee_active is set outside of >> phy_ethtool_set_eee() is because this is a MAC + PHY thing, both need to >> agree and support that, and so while the PHY may be configured to have >> EEE advertised and enabled, you also need to take care of the MAC >> portion and enable EEE in there as well. Is not there such a thing for >> the qca8k switch where the PHY needs to be configured through the >> standard phylib calls, but the switch's transmitter/receiver also needs >> to have EEE enabled? >> > > Hi Florian, > > the switch needs to enable the eee on a per mac absis, but there is no > way to tell if the autonegotiate worked and eee is enabled without > reading the phys registers.
OK, that does not sound atypical here, most drivers I see do have a way to tell if EEE is active by reading e.g: the LPI indication register, or something that is able to reflect the negotiated result. > > setting the eee_active inside phy_ethtool_get_eee() would break those > dsa drivers that have a register telling if AN worked. if it is ok i > will just call phy_ethtool_get_eee() inside get_eee(). Ok, let's see the code and then we can discuss from there, not very clear on the proposed change here. -- Florian