netdev - Re: [PATCH net-next] net: genet: Fixup EEE

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: <ZhdycHAooDITV1a3@pengutronix.de>
Date: Thu, 11 Apr 2024 07:17:36 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Florian Fainelli <florian.fainelli@...adcom.com>
Cc: Andrew Lunn <andrew@...n.ch>, Doug Berger <opendmb@...il.com>,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
	"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
Subject: Re: [PATCH net-next] net: genet: Fixup EEE

Hi Florian,

On Wed, Apr 10, 2024 at 10:48:26AM -0700, Florian Fainelli wrote:

> I am seeing a functional difference with and without your patch however, and
> also, there appears to be something wrong within the bcmgenet driver after
> PHYLIB having absorbed the EEE configuration. Both cases we start on boot
> with:
> 
> # ethtool --show-eee eth0
> EEE settings for eth0:
>         EEE status: disabled
>         Tx LPI: disabled
>         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 would expect the EEE status to be enabled, that's how I remember it
> before.

Yes, current default kernel implementation is to use EEE if available.

> Now, with your patch, once I turn on EEE with:
> 
> # ethtool --set-eee eth0 eee on
> # ethtool --show-eee eth0
> EEE settings for eth0:
>         EEE status: enabled - active
>         Tx LPI: disabled
>         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
> #
> 
> there is no change to the EEE_CTRL register to set the EEE_EN, this only
> happens when doing:
> 
> # ethtool --set-eee eth0 eee on tx-lpi on
> 
> which is consistent with the patch, but I don't think this is quite correct
> as I remembered that "eee on" meant enable EEE for the RX path, and "tx-lpi
> on" meant enable EEE for the TX path?

Yes. More precisely, with "eee on" we allow the PHY to advertise EEE
link modes. On link_up, if both sides are agreed to use EEE, MAC is
configured to process LPI opcodes from the PHY and send LPI opcodes to
the PHY if "tx-lpi on" was configured too. tx-lpi will not be enabled in
case of "eee off".

What is exact meaning of EEE_EN on this chip?

Regards,
Oleksij
-- 
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 |