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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Apr 2017 15:23:56 +0200
From:   Niklas Cassel <niklas.cassel@...s.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Giuseppe CAVALLARO <peppe.cavallaro@...com>,
        Andrew Lunn <andrew@...n.ch>,
        Yegor Yefremov <yegorslists@...glemail.com>
CC:     netdev <netdev@...r.kernel.org>,
        "linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
        Grygorii Strashko <grygorii.strashko@...com>,
        "N, Mugunthan V" <mugunthanvnm@...com>,
        Rami Rosen <roszenrami@...il.com>,
        Fabrice GASNIER <fabrice.gasnier@...com>
Subject: Re: Re: [PATCH v2] cpsw: ethtool: add support for getting/setting EEE
 registers

On 01/04/2017 03:33 PM, Florian Fainelli wrote:
> 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!
> 

I'm curious about this as well.

I can get EEE to work with stmmac, but to be able to turn EEE on,
I need to set eee advertise via ethtool first.
(Tested with 2 different PHYs from different vendors, with their
PHY specific driver enabled.)

Is this the same for all PHYs or are there certain PHYs/PHY drivers
that actually advertise eee by default?
(From reading this mail thread there seems to be a suggestion that
the broadcom PHY driver might advertise eee by default.)