| 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
| ||
|
Message-ID: <aP96RVclYQaoBxSO@pengutronix.de>
Date: Mon, 27 Oct 2025 14:57:25 +0100
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Andrew Lunn <andrew@...n.ch>
Cc: Emanuele Ghidoli <ghidoliemanuele@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Emanuele Ghidoli <emanuele.ghidoli@...adex.com>,
Russell King <linux@...linux.org.uk>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org
Subject: Re: [PATCH v1] net: phy: dp83867: Disable EEE support as not
implemented
On Mon, Oct 27, 2025 at 02:25:12PM +0100, Andrew Lunn wrote:
> On Mon, Oct 27, 2025 at 01:57:48PM +0100, Emanuele Ghidoli wrote:
> >
> >
> > On 27/10/2025 00:45, Andrew Lunn wrote:
> > >> Since the introduction of phylink-managed EEE support in the stmmac driver,
> > >> EEE is now enabled by default, leading to issues on systems using the
> > >> DP83867 PHY.
> > >
> > > Did you do a bisect to prove this?
> > Yes, I have done a bisect and the commit that introduced the behavior on our
> > board is 4218647d4556 ("net: stmmac: convert to phylink managed EEE support").
> >
> > >
> > >> Fixes: 2a10154abcb7 ("net: phy: dp83867: Add TI dp83867 phy")
> > >
> > > What has this Fixes: tag got to do with phylink?
> > I think that the phylink commit is just enabling by default the EEE support,
> > and my commit is not really fixing that. It is why I didn't put a Fixes: tag
> > pointing to that.
> >
> > I’ve tried to trace the behavior, but it’s quite complex. From my testing, I
> > can summarize the situation as follows:
> >
> > - ethtool, after that patch, returns:
> > ethtool --show-eee end0
> > EEE settings for end0:
> > EEE status: enabled - active
> > Tx LPI: 1000000 (us)
> > Supported EEE link modes: 100baseT/Full
> > 1000baseT/Full
> > Advertised EEE link modes: 100baseT/Full
> > 1000baseT/Full
> > Link partner advertised EEE link modes: 100baseT/Full
> > 1000baseT/Full
> > - before that patch returns, after boot:
> > EEE settings for end0:
> > EEE status: disabled
> > Tx LPI: disabled
> > Supported EEE link modes: 100baseT/Full
> > 1000baseT/Full
> > Advertised EEE link modes: Not reported
> > Link partner advertised EEE link modes: 100baseT/Full
> > 1000baseT/Full
> > - Enabling EEE manually using ethtool, triggers the problem too (and ethtool
> > -show-eee report eee status enabled):
> > ethtool --set-eee end0 eee on tx-lpi on
> > ethtool --show-eee end0
> > EEE settings for end0:
> > EEE status: enabled - active
> > Tx LPI: 1000000 (us)
> > Supported EEE link modes: 100baseT/Full
> > 1000baseT/Full
> > Advertised EEE link modes: 100baseT/Full
> > 1000baseT/Full
> > Link partner advertised EEE link modes: 100baseT/Full
> > 1000baseT/Full
> >
> > I understand Russell point of view but from my point of view EEE is now
> > enabled by default, and before it wasn't, at least on my setup.
>
> We like to try to understand what is going on, and give accurate
> descriptions. You have given us important information here, which at
> minimum should go into the commit message, but more likely, it will
> help lead us to the correct fix.
>
> So, two things here. You say:
>
> > I think that the phylink commit is just enabling by default the EEE support,
>
> That needs confirming, because you are blaming the conversion to
> phylink, not that phylink now enabled EEE by default. Russell also
> tries to avoid behaviour change, which this clearly is. We want a
> better understanding what caused this behaviour change.
>
> Also:
>
> > - Enabling EEE manually using ethtool, triggers the problem too (and ethtool
> > -show-eee report eee status enabled):
>
> This indicates EEE has always been broken. This brokenness has been
> somewhat hidden in the past, and it is the change in behaviour in
> phylink which exposed this brokenness. A commit message using these
> words would be much more factually correct, and it would also fit with
> the Fixes: tag you used.
>
> So, please work with Russell. I see two things which would be good to
> understand before a new version of the patch is submitted:
>
> What cause the behaviour change such that EEE is now enabled? Was it
> deliberate? Should something be change to revert that behaviour
> change?
>
> Given that EEE has always been broken, do we understand it
> sufficiently to say it is not fixable? Is there an errata?
None of following TI Gbit PHYs claim EEE support:
dp83867cr/ir https://www.ti.com/de/lit/gpn/dp83867cr
dp83867e/cs/is https://www.ti.com/de/lit/gpn/dp83867cs
dp83869hm https://www.ti.com/lit/gpn/dp83869hm
For comparison, TI 100Mbit PHYs list EEE as supported:
dp83826a* https://www.ti.com/de/lit/gpn/dp83826ae
dp83826i https://www.ti.com/de/lit/gpn/dp83826i
If vendor do not see it as selling point, or it is just broken beyond
repair, there is nothing we can do here. I guess it is ok to sync the
driver with vendors claim.
> Are we sure it is the PHY and not the MAC which is broken?
I personally still do not have suitable reference board for testing.
There are some with Realtek or TI PHYs. It will be good to find board
with iMX8MP + KSZ9131 on both MACs (FEC and STMMAC).
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Powered by blists - more mailing lists