netdev - Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family

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]

Message-ID: <44865cc8-19bf-54dc-ad05-82bdaa648eeb@phrozen.org>
Date:   Tue, 13 Sep 2016 20:07:19 +0200
From:   John Crispin <john@...ozen.org>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>
Cc:     "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, qsdk-review@....qualcomm.com
Subject: Re: [PATCH 3/3] net-next: dsa: add new driver for qca8xxx family



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.

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().

	John