netdev - Re: [PATCH net-next 1/3] net: phy: add phy_disable

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: <950a7738-1308-46ee-b913-018c51db76b8@gmail.com>
Date: Wed, 18 Dec 2024 08:01:40 +0100
From: Heiner Kallweit <hkallweit1@...il.com>
To: Andrew Lunn <andrew@...n.ch>,
 Russell King - ARM Linux <linux@...linux.org.uk>
Cc: Tony Lindgren <tony@...mide.com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
 Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
 David Miller <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Simon Horman <horms@...nel.org>, linux-omap@...r.kernel.org,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/3] net: phy: add phy_disable_eee

On 17.12.2024 23:34, Andrew Lunn wrote:
> On Tue, Dec 17, 2024 at 09:50:12PM +0100, Heiner Kallweit wrote:
>> On 17.12.2024 11:43, Andrew Lunn wrote:
>>>> @@ -2071,6 +2071,7 @@ void phy_advertise_eee_all(struct phy_device *phydev);
>>>>  void phy_support_sym_pause(struct phy_device *phydev);
>>>>  void phy_support_asym_pause(struct phy_device *phydev);
>>>>  void phy_support_eee(struct phy_device *phydev);
>>>> +void phy_disable_eee(struct phy_device *phydev);
>>>
>>> So we have three states:
>>>
>>> MAC tells PHYLIB it does support EEE
>>> MAC tells PHYLIB it does not support EEE
>>> MAC says nothing.
>>>
>>> Do we really want this?
>>>
>>> For phylink, i think we have a nice new clean design and can say, if
>>> the MAC does not indicate it supports EEE, we turn it off in the
>>> PHY. For phylib, we have more of a mess, and there could be MACs
>>> actually doing EEE by default using default setting but with no user
>>> space control. Do we want to keep this, or should we say any MAC which
>>> does not call phy_support_eee() before phy_start() would have EEE
>>> disabled in the PHY?
>>>
>> The case "MAC says nothing" isn't desirable. However, if we did what
>> you mention, we'd silently change the behavior of several drivers,
>> resulting in disabled EEE and higher power consumption.
>> I briefly grepped the kernel source for phy_start() and found about
>> 70 drivers. Some of them have the phylib EEE call, and in some cases
>> like cpsw the MAC doesn't support EEE. But what remains is IMO too
>> many drivers where we'd change the behavior.
> 
> So for phylib, we keep with the three states. But phylink? Can we
> disable EEE when the MAC says nothing?
> 
Looking at patch 5 of Russell's series behavior doesn't change if
pl->mac_supports_eee is false. So I think he also goes with the three
states, at least initially, until all drivers using phylink have
implemented the new phylink ops. AFAICS this affects about 25 drivers.

> 	Andrew
Heiner