netdev - Re: net: ti: weirdness (was Re: [PATCH RFC net-next 00/23] net: phylink managed EEE support)

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] [day] [month] [year] [list]

Message-ID: <46498cdf-3582-4bbc-a00d-c02ff72cf600@lunn.ch>
Date: Thu, 28 Nov 2024 15:21:59 +0100
From: Andrew Lunn <andrew@...n.ch>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Bryan Whitehead <bryan.whitehead@...rochip.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Florian Fainelli <florian.fainelli@...adcom.com>,
	Jakub Kicinski <kuba@...nel.org>, Jose Abreu <joabreu@...opsys.com>,
	linux-arm-kernel@...ts.infradead.org,
	linux-stm32@...md-mailman.stormreply.com,
	Marcin Wojtas <marcin.s.wojtas@...il.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>, netdev@...r.kernel.org,
	Oleksij Rempel <o.rempel@...gutronix.de>,
	Paolo Abeni <pabeni@...hat.com>, UNGLinuxDriver@...rochip.com
Subject: Re: net: ti: weirdness (was Re: [PATCH RFC net-next 00/23] net:
 phylink managed EEE support)

On Wed, Nov 27, 2024 at 11:20:08AM +0000, Russell King (Oracle) wrote:
> On Tue, Nov 26, 2024 at 12:51:36PM +0000, Russell King (Oracle) wrote:
> > In doing this, I came across the fact that the addition of phylib
> > managed EEE support has actually broken a huge number of drivers -
> > phylib will now overwrite all members of struct ethtool_keee whether
> > the netdev driver wants it or not. This leads to weird scenarios where
> > doing a get_eee() op followed by a set_eee() op results in e.g.
> > tx_lpi_timer being zeroed, because the MAC driver doesn't know it needs
> > to initialise phylib's phydev->eee_cfg.tx_lpi_timer member. This mess
> > really needs urgently addressing, and I believe it came about because
> > Andrew's patches were only partly merged via another party - I guess
> > highlighting the inherent danger of "thou shalt limit your patch series
> > to no more than 15 patches" when one has a subsystem who's in-kernel
> > API is changing.
> 
> Looking at the two TI offerings that call phy_ethtool_get_eee(), both
> of them call the phylib functions from their ethtool ops, but it looks
> like the driver does diddly squat with LPI state, which makes me wonder
> why they implemented the calls to phy_ethtool_get_eee() and
> phy_ethtool_set_eee(), since EEE will not be functional unless the PHY
> has been configured with a SmartEEE mode outside the kernel.

Probably because they did not know what they were doing, and it got
past reviewers.

Well, actually:

commit a090994980a15f8cc14fc188b5929bd61d2ae9c3
Author: Yegor Yefremov <yegorslists@...glemail.com>
Date:   Mon Nov 28 09:41:33 2016 +0100

    cpsw: ethtool: add support for getting/setting EEE registers
    
    Add the ability to query and set Energy Efficient Ethernet parameters
    via ethtool for applicable devices.
    
    This patch doesn't activate full EEE support in cpsw driver, but it
    enables reading and writing EEE advertising settings. This way one
    can disable advertising EEE for certain speeds.
    
    Signed-off-by: Yegor Yefremov <yegorslists@...glemail.com>
    Acked-by: Rami Rosen <roszenrami@...il.com>
    Signed-off-by: David S. Miller <davem@...emloft.net>

Seems like somebody had an issue and did the minimum to work around
the issue. This also suggests the hardware is doing EEE by default,
hopefully with some sort of sensible hardware defaults.

	Andrew