Re: [PATCH v2] cpsw: ethtool: add support for getting/setting EEE registers

[Florian Fainelli <f.fainelli@gmail.com>]

On 12/02/2016 09:48 AM, Florian Fainelli wrote:
>>> Peppe, any thoughts on this?
>>
>> I share what you say.
>>
>> In sum, the EEE management inside the stmmac is:
>>
>> - the driver looks at own HW cap register if EEE is supported
>>
>>     (indeed the user could keep disable EEE if bugged on some HW
>>      + Alex, Fabrice: we had some patches for this to propose where we
>>              called the phy_ethtool_set_eee to disable feature at phy
>>              level
>>
>> - then the stmmac asks PHY layer to understand if transceiver and
>>   partners are EEE capable.
>>
>> - If all matches the EEE is actually initialized.
>>
>> the logic above should be respected when use ethtool, hmm, I will
>> check the stmmac_ethtool_op_set_eee asap.
>>
>> Hoping this is useful
> 
> This is definitively useful, the only part that I am struggling to
> understand in phy_init_eee() is this:
> 
>                 eee_adv = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV,
>                                                 MDIO_MMD_AN);
>                 if (eee_adv <= 0)
>                         goto eee_exit_err;
> 
> if we are not already advertising EEE in the PHY's MMIO_MMD_AN page, by
> the time we call phy_init_eee(), then we cannot complete the EEE
> configuration at the PHY level, and presumably we should abort the EEE
> configuration at the MAC level.
> 
> While this condition makes sense if e.g: you are re-negotiating the link
> with your partner for instance and if EEE was already advertised, the
> very first time this function is called, it seems to be like we should
> skip the check, because phy_init_eee() should actually tell us if, as a
> result of a successful check, we should be setting EEE as something we
> advertise?
> 
> Do you remember what was the logic behind this check when you added it?

Peppe, can you remember why phy_init_eee() was written in a way that you
need to have already locally advertised EEE for the function to
successfully return? Thank you!
-- 
Florian