public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] net: bcmgenet: enable EEE TX LPI by default
@ 2026-03-03 16:02 Nicolai Buchwitz
  2026-03-16 19:54 ` Florian Fainelli
  0 siblings, 1 reply; 5+ messages in thread
From: Nicolai Buchwitz @ 2026-03-03 16:02 UTC (permalink / raw)
  To: Doug Berger, Florian Fainelli, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: bcm-kernel-feedback-list, netdev, linux-kernel, Nicolai Buchwitz

GENET never initializes priv->eee.eee_enabled or
priv->eee.tx_lpi_enabled to true, so the link-change callback in
bcmgenet_mac_config() always calls bcmgenet_eee_enable_set() with
enable=false.  The result is that EEE is negotiated at the PHY level
but the MAC never enters Low Power Idle, wasting the power savings
that EEE is designed to provide.

Users can work around this with 'ethtool --set-eee eth0 eee on tx-lpi
on tx-timer 250', but the MAC should enable TX LPI out of the box when
the hardware supports it, consistent with how phylink-managed MACs
behave (phylink calls phy_support_eee() which sets eee_enabled=true and
tx_lpi_enabled=true by default).

Initialize EEE software state during bcmgenet_open() on GENET v2+ so
that the link-change path activates EEE in hardware once the PHY
negotiates it.  Use an LPI timer of 250 us, matching the default used
by other MAC drivers (mvneta, mvpp2).

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 33fece36a95e..00ce93b8a6fa 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3375,6 +3375,16 @@ static int bcmgenet_open(struct net_device *dev)
 
 	bcmgenet_phy_pause_set(dev, priv->rx_pause, priv->tx_pause);
 
+	/* Enable EEE by default on hardware that supports it.  This sets the
+	 * software state so that the link-change callback will activate EEE
+	 * in hardware once EEE is negotiated with the link partner.
+	 */
+	if (!GENET_IS_V1(priv)) {
+		priv->eee.eee_enabled = true;
+		priv->eee.tx_lpi_enabled = true;
+		bcmgenet_umac_writel(priv, 250, UMAC_EEE_LPI_TIMER);
+	}
+
 	bcmgenet_netif_start(dev);
 
 	netif_tx_start_all_queues(dev);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: bcmgenet: enable EEE TX LPI by default
  2026-03-03 16:02 [PATCH net-next] net: bcmgenet: enable EEE TX LPI by default Nicolai Buchwitz
@ 2026-03-16 19:54 ` Florian Fainelli
  2026-03-16 20:59   ` Jakub Kicinski
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2026-03-16 19:54 UTC (permalink / raw)
  To: Nicolai Buchwitz, Doug Berger, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: bcm-kernel-feedback-list, netdev, linux-kernel

On 3/3/26 08:02, Nicolai Buchwitz wrote:
> GENET never initializes priv->eee.eee_enabled or
> priv->eee.tx_lpi_enabled to true, so the link-change callback in
> bcmgenet_mac_config() always calls bcmgenet_eee_enable_set() with
> enable=false.  The result is that EEE is negotiated at the PHY level
> but the MAC never enters Low Power Idle, wasting the power savings
> that EEE is designed to provide.
> 
> Users can work around this with 'ethtool --set-eee eth0 eee on tx-lpi
> on tx-timer 250', but the MAC should enable TX LPI out of the box when
> the hardware supports it, consistent with how phylink-managed MACs
> behave (phylink calls phy_support_eee() which sets eee_enabled=true and
> tx_lpi_enabled=true by default).
> 
> Initialize EEE software state during bcmgenet_open() on GENET v2+ so
> that the link-change path activates EEE in hardware once the PHY
> negotiates it.  Use an LPI timer of 250 us, matching the default used
> by other MAC drivers (mvneta, mvpp2).
> 
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: bcmgenet: enable EEE TX LPI by default
  2026-03-16 19:54 ` Florian Fainelli
@ 2026-03-16 20:59   ` Jakub Kicinski
  2026-03-16 21:21     ` Florian Fainelli
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2026-03-16 20:59 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Nicolai Buchwitz, Doug Berger, Andrew Lunn, David S . Miller,
	Eric Dumazet, Paolo Abeni, bcm-kernel-feedback-list, netdev,
	linux-kernel

On Mon, 16 Mar 2026 12:54:46 -0700 Florian Fainelli wrote:
> On 3/3/26 08:02, Nicolai Buchwitz wrote:
> > GENET never initializes priv->eee.eee_enabled or
> > priv->eee.tx_lpi_enabled to true, so the link-change callback in
> > bcmgenet_mac_config() always calls bcmgenet_eee_enable_set() with
> > enable=false.  The result is that EEE is negotiated at the PHY level
> > but the MAC never enters Low Power Idle, wasting the power savings
> > that EEE is designed to provide.
> > 
> > Users can work around this with 'ethtool --set-eee eth0 eee on tx-lpi
> > on tx-timer 250', but the MAC should enable TX LPI out of the box when
> > the hardware supports it, consistent with how phylink-managed MACs
> > behave (phylink calls phy_support_eee() which sets eee_enabled=true and
> > tx_lpi_enabled=true by default).
> > 
> > Initialize EEE software state during bcmgenet_open() on GENET v2+ so
> > that the link-change path activates EEE in hardware once the PHY
> > negotiates it.  Use an LPI timer of 250 us, matching the default used
> > by other MAC drivers (mvneta, mvpp2).
> > 
> > Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>  
> 
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>

FWIW this patch is set to Changes Requested, I _think_ based on this
reply from Andrew:

https://lore.kernel.org/all/d352039f-4cbb-41e6-9aeb-0b4f3941b54c@lunn.ch/

LMK if the patch is good as is.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: bcmgenet: enable EEE TX LPI by default
  2026-03-16 20:59   ` Jakub Kicinski
@ 2026-03-16 21:21     ` Florian Fainelli
  2026-03-17  7:51       ` Nicolai Buchwitz
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2026-03-16 21:21 UTC (permalink / raw)
  To: Jakub Kicinski, Florian Fainelli
  Cc: Nicolai Buchwitz, Doug Berger, Andrew Lunn, David S . Miller,
	Eric Dumazet, Paolo Abeni, bcm-kernel-feedback-list, netdev,
	linux-kernel

On 3/16/26 13:59, Jakub Kicinski wrote:
> On Mon, 16 Mar 2026 12:54:46 -0700 Florian Fainelli wrote:
>> On 3/3/26 08:02, Nicolai Buchwitz wrote:
>>> GENET never initializes priv->eee.eee_enabled or
>>> priv->eee.tx_lpi_enabled to true, so the link-change callback in
>>> bcmgenet_mac_config() always calls bcmgenet_eee_enable_set() with
>>> enable=false.  The result is that EEE is negotiated at the PHY level
>>> but the MAC never enters Low Power Idle, wasting the power savings
>>> that EEE is designed to provide.
>>>
>>> Users can work around this with 'ethtool --set-eee eth0 eee on tx-lpi
>>> on tx-timer 250', but the MAC should enable TX LPI out of the box when
>>> the hardware supports it, consistent with how phylink-managed MACs
>>> behave (phylink calls phy_support_eee() which sets eee_enabled=true and
>>> tx_lpi_enabled=true by default).
>>>
>>> Initialize EEE software state during bcmgenet_open() on GENET v2+ so
>>> that the link-change path activates EEE in hardware once the PHY
>>> negotiates it.  Use an LPI timer of 250 us, matching the default used
>>> by other MAC drivers (mvneta, mvpp2).
>>>
>>> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
>>
>> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> 
> FWIW this patch is set to Changes Requested, I _think_ based on this
> reply from Andrew:
> 
> https://lore.kernel.org/all/d352039f-4cbb-41e6-9aeb-0b4f3941b54c@lunn.ch/
> 
> LMK if the patch is good as is.

It is good after 908c344d5cfac4160f49715da9efacdf5b6a28bd ("net: 
bcmgenet: fix broken EEE by converting to phylib-managed state") reaches 
net/next which is what Andrew was commenting on.
-- 
Florian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next] net: bcmgenet: enable EEE TX LPI by default
  2026-03-16 21:21     ` Florian Fainelli
@ 2026-03-17  7:51       ` Nicolai Buchwitz
  0 siblings, 0 replies; 5+ messages in thread
From: Nicolai Buchwitz @ 2026-03-17  7:51 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Jakub Kicinski, Florian Fainelli, Doug Berger, Andrew Lunn,
	David S . Miller, Eric Dumazet, Paolo Abeni,
	bcm-kernel-feedback-list, netdev, linux-kernel

On 16.3.2026 22:21, Florian Fainelli wrote:
> On 3/16/26 13:59, Jakub Kicinski wrote:
>> On Mon, 16 Mar 2026 12:54:46 -0700 Florian Fainelli wrote:
>>> On 3/3/26 08:02, Nicolai Buchwitz wrote:
>>>> GENET never initializes priv->eee.eee_enabled or
>>>> priv->eee.tx_lpi_enabled to true, so the link-change callback in
>>>> bcmgenet_mac_config() always calls bcmgenet_eee_enable_set() with
>>>> enable=false.  The result is that EEE is negotiated at the PHY level
>>>> but the MAC never enters Low Power Idle, wasting the power savings
>>>> that EEE is designed to provide.
>>>> 
>>>> Users can work around this with 'ethtool --set-eee eth0 eee on 
>>>> tx-lpi
>>>> on tx-timer 250', but the MAC should enable TX LPI out of the box 
>>>> when
>>>> the hardware supports it, consistent with how phylink-managed MACs
>>>> behave (phylink calls phy_support_eee() which sets eee_enabled=true 
>>>> and
>>>> tx_lpi_enabled=true by default).
>>>> 
>>>> Initialize EEE software state during bcmgenet_open() on GENET v2+ so
>>>> that the link-change path activates EEE in hardware once the PHY
>>>> negotiates it.  Use an LPI timer of 250 us, matching the default 
>>>> used
>>>> by other MAC drivers (mvneta, mvpp2).
>>>> 
>>>> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
>>> 
>>> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
>> 
>> FWIW this patch is set to Changes Requested, I _think_ based on this
>> reply from Andrew:
>> 
>> https://lore.kernel.org/all/d352039f-4cbb-41e6-9aeb-0b4f3941b54c@lunn.ch/
>> 
>> LMK if the patch is good as is.
> 
> It is good after 908c344d5cfac4160f49715da9efacdf5b6a28bd ("net: 
> bcmgenet: fix broken EEE by converting to phylib-managed state") 
> reaches net/next which is what Andrew was commenting on.

908c344d5cfac4160f49715da9efacdf5b6a28bd ("net: bcmgenet: fix broken EEE 
by converting to phylib-managed state") should cover it - phylib handles 
EEE activation automatically and sets the LPI tx timer from the phy 
default. I noted in the new patch that both earlier submissions are 
superseded, but forgot to mention it in the patches themselves. 
Apologies, still getting used to the list-based workflow!

Nicolai

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-03-17  7:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03 16:02 [PATCH net-next] net: bcmgenet: enable EEE TX LPI by default Nicolai Buchwitz
2026-03-16 19:54 ` Florian Fainelli
2026-03-16 20:59   ` Jakub Kicinski
2026-03-16 21:21     ` Florian Fainelli
2026-03-17  7:51       ` Nicolai Buchwitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox