[PATCH net-next v3 0/5] net: cadence: macb: add IEEE 802.3az EEE support

[PATCH net-next v3 0/5] net: cadence: macb: add IEEE 802.3az EEE support

[Nicolai Buchwitz]

Add Energy Efficient Ethernet (IEEE 802.3az) support to the Cadence GEM
(macb) driver using phylink's managed EEE framework. The GEM MAC has
hardware LPI registers but no built-in idle timer, so the driver
implements software-managed TX LPI using a delayed_work timer while
delegating EEE negotiation and ethtool state to phylink.

Changes from v2:
  - macb_tx_lpi_set() now returns bool indicating whether the register
    value actually changed, avoiding redundant writes.
  - Removed tx_lpi_enabled field from struct macb; LPI state is tracked
    entirely within the spinlock-protected register read/modify/write.
  - macb_tx_lpi_wake() uses the return value of macb_tx_lpi_set() to
    skip the cancel/udelay when TXLPIEN was already clear.
  All changes based on feedback from Russell King.

Changes from v1:
  - Rewrote to use phylink managed EEE (mac_enable_tx_lpi /
    mac_disable_tx_lpi callbacks) instead of the obsolete
    phy_init_eee() approach, as recommended by Russell King.
  - ethtool get_eee/set_eee are now pure phylink passthroughs.
  - Removed all manual EEE state tracking from mac_link_up/down;
    phylink handles the lifecycle.

The series is structured as follows:

  1. Register definitions: LPI counter offsets (0x270-0x27c), TXLPIEN
     bitfield (NCR bit 19), and MACB_CAPS_EEE capability flag.

  2. LPI statistics: Expose the four hardware EEE counters (RX/TX LPI
     transitions and time) through ethtool -S, accumulated in software
     since they are clear-on-read.

  3. TX LPI engine: phylink mac_enable_tx_lpi / mac_disable_tx_lpi
     callbacks with a delayed_work-based idle timer. LPI entry is
     deferred 1 second after link-up per IEEE 802.3az. Wake before
     transmit with a conservative 50us PHY wake delay.

  4. ethtool EEE ops: get_eee/set_eee delegating to phylink for PHY
     negotiation and timer management.

  5. RP1 enablement: Set MACB_CAPS_EEE for the Raspberry Pi 5's RP1
     southbridge (Cadence GEM_GXL rev 0x00070109 + BCM54213PE PHY).

Tested on Raspberry Pi 5 (1000BASE-T, BCM54213PE PHY, 250ms LPI timer):

  iperf3 throughput (no regression):
    TCP TX: 937.8 Mbit/s (EEE on) vs 937.0 Mbit/s (EEE off)
    TCP RX: 936.5 Mbit/s both

  Latency (ping RTT, small expected increase from LPI wake):
    1s interval:  0.273 ms (EEE on) vs 0.181 ms (EEE off)
    10ms interval: 0.206 ms (EEE on) vs 0.168 ms (EEE off)
    flood ping:   0.200 ms (EEE on) vs 0.156 ms (EEE off)

  LPI counters (ethtool -S, 1s-interval ping, EEE on):
    tx_lpi_transitions: 112
    tx_lpi_time: 15574651

  Zero packet loss across all tests. Also verified with
  ethtool --show-eee / --set-eee and cable unplug/replug cycling.

Nicolai Buchwitz (5):
  net: cadence: macb: add EEE register definitions and capability flag
  net: cadence: macb: add EEE LPI statistics counters
  net: cadence: macb: implement EEE TX LPI support
  net: cadence: macb: add ethtool EEE support
  net: cadence: macb: enable EEE for Raspberry Pi RP1

 drivers/net/ethernet/cadence/macb.h      |  20 ++++
 drivers/net/ethernet/cadence/macb_main.c | 136 ++++++++++++++++++++++-
 2 files changed, 155 insertions(+), 1 deletion(-)

-- 
2.51.0

[PATCH 1/5] net: cadence: macb: add EEE register definitions and capability flag

[Nicolai Buchwitz]

Add register and bitfield definitions for the Cadence GEM MAC's
IEEE 802.3az Energy Efficient Ethernet (EEE) support:

- LPI statistics counter registers (GEM_RXLPI, GEM_RXLPITIME,
  GEM_TXLPI, GEM_TXLPITIME) at offsets 0x270-0x27c
- TX LPI enable bitfield (GEM_TXLPIEN) in the NCR register (bit 19),
  which directly asserts/deasserts LPI on the transmit path
- MACB_CAPS_EEE capability flag to gate EEE support per platform

These registers are present in all Cadence GEM revisions that support
EEE (verified on SAMA5D2, SAME70, PIC32CZ, and RP1 variants).

No functional change.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/cadence/macb.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 87414a2ddf6e..729751d424c2 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -170,6 +170,10 @@
 #define GEM_PCSANNPTX		0x021c /* PCS AN Next Page TX */
 #define GEM_PCSANNPLP		0x0220 /* PCS AN Next Page LP */
 #define GEM_PCSANEXTSTS		0x023c /* PCS AN Extended Status */
+#define GEM_RXLPI		0x0270 /* RX LPI Transitions */
+#define GEM_RXLPITIME		0x0274 /* RX LPI Time */
+#define GEM_TXLPI		0x0278 /* TX LPI Transitions */
+#define GEM_TXLPITIME		0x027c /* TX LPI Time */
 #define GEM_DCFG1		0x0280 /* Design Config 1 */
 #define GEM_DCFG2		0x0284 /* Design Config 2 */
 #define GEM_DCFG3		0x0288 /* Design Config 3 */
@@ -305,6 +309,8 @@
 #define MACB_IRXFCS_SIZE	1
 
 /* GEM specific NCR bitfields. */
+#define GEM_TXLPIEN_OFFSET		19
+#define GEM_TXLPIEN_SIZE		1
 #define GEM_ENABLE_HS_MAC_OFFSET	31
 #define GEM_ENABLE_HS_MAC_SIZE		1
 
@@ -779,6 +785,7 @@
 #define MACB_CAPS_DMA_PTP			BIT(22)
 #define MACB_CAPS_RSC				BIT(23)
 #define MACB_CAPS_NO_LSO			BIT(24)
+#define MACB_CAPS_EEE				BIT(25)
 
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE			0x01
-- 
2.51.0

[PATCH 2/5] net: cadence: macb: add EEE LPI statistics counters

[Nicolai Buchwitz]

Expose the GEM MAC's EEE Low Power Idle hardware counters through
ethtool -S:

- rx_lpi_transitions: number of RX LPI entry events
- rx_lpi_time: cumulative time spent in RX LPI
- tx_lpi_transitions: number of TX LPI entry events (TXLPIEN 0->1)
- tx_lpi_time: cumulative time in TX LPI

These are clear-on-read hardware registers at offsets 0x270-0x27c,
automatically collected by the existing gem_statistics read loop.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/cadence/macb.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 729751d424c2..e3520e5aec67 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1050,6 +1050,10 @@ struct gem_stats {
 	u64	rx_ip_header_checksum_errors;
 	u64	rx_tcp_checksum_errors;
 	u64	rx_udp_checksum_errors;
+	u64	rx_lpi_transitions;
+	u64	rx_lpi_time;
+	u64	tx_lpi_transitions;
+	u64	tx_lpi_time;
 };
 
 /* Describes the name and offset of an individual statistic register, as
@@ -1149,6 +1153,10 @@ static const struct gem_statistic gem_statistics[] = {
 			    GEM_BIT(NDS_RXERR)),
 	GEM_STAT_TITLE_BITS(RXUDPCCNT, "rx_udp_checksum_errors",
 			    GEM_BIT(NDS_RXERR)),
+	GEM_STAT_TITLE(RXLPI, "rx_lpi_transitions"),
+	GEM_STAT_TITLE(RXLPITIME, "rx_lpi_time"),
+	GEM_STAT_TITLE(TXLPI, "tx_lpi_transitions"),
+	GEM_STAT_TITLE(TXLPITIME, "tx_lpi_time"),
 };
 
 #define GEM_STATS_LEN ARRAY_SIZE(gem_statistics)
-- 
2.51.0

[PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Nicolai Buchwitz]

Implement Energy Efficient Ethernet TX Low Power Idle using phylink's
managed EEE framework. The Cadence GEM MAC has no built-in idle timer
— TXLPIEN (NCR bit 19) immediately blocks all TX when set and the MAC
does NOT auto-wake — so the driver uses a software delayed_work timer
for idle detection.

The TX LPI lifecycle:
  - phylink calls mac_enable_tx_lpi() after link-up with the negotiated
    timer value. The driver defers the first LPI entry by 1 second per
    IEEE 802.3az section 22.7a.
  - macb_tx_complete() reschedules the idle timer after each TX drain.
  - macb_start_xmit() wakes from LPI by clearing TXLPIEN, cancelling
    the pending work, and waiting 50us (conservative Tw_sys) before
    initiating the transmit.
  - phylink calls mac_disable_tx_lpi() before link-down, which cancels
    the work and clears TXLPIEN.

The phylink_config is populated with LPI capabilities (MII, GMII, RGMII
modes; 100FD and 1000FD speeds) and a 250ms default idle timer, gated
on MACB_CAPS_EEE.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/cadence/macb.h      |   5 +
 drivers/net/ethernet/cadence/macb_main.c | 111 +++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index e3520e5aec67..c69828b27dae 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1372,6 +1372,11 @@ struct macb {
 
 	struct work_struct	hresp_err_bh_work;
 
+	/* EEE / LPI state */
+	bool			eee_active;
+	struct delayed_work	tx_lpi_work;
+	u32			tx_lpi_timer;
+
 	int	rx_bd_rd_prefetch;
 	int	tx_bd_rd_prefetch;
 
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 43cd013bb70e..a9ae7ecf9fe9 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -10,6 +10,7 @@
 #include <linux/clk-provider.h>
 #include <linux/clk.h>
 #include <linux/crc32.h>
+#include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/etherdevice.h>
 #include <linux/firmware/xlnx-zynqmp.h>
@@ -589,6 +590,93 @@ static const struct phylink_pcs_ops macb_phylink_pcs_ops = {
 	.pcs_config = macb_pcs_config,
 };
 
+static bool macb_tx_lpi_set(struct macb *bp, bool enable)
+{
+	unsigned long flags;
+	u32 old, ncr;
+
+	spin_lock_irqsave(&bp->lock, flags);
+	old = ncr = macb_readl(bp, NCR);
+	if (enable)
+		ncr |= GEM_BIT(TXLPIEN);
+	else
+		ncr &= ~GEM_BIT(TXLPIEN);
+	if (old != ncr)
+		macb_writel(bp, NCR, ncr);
+	spin_unlock_irqrestore(&bp->lock, flags);
+
+	return old != ncr;
+}
+
+static bool macb_tx_all_queues_idle(struct macb *bp)
+{
+	unsigned int q;
+
+	for (q = 0; q < bp->num_queues; q++) {
+		struct macb_queue *queue = &bp->queues[q];
+
+		if (queue->tx_head != queue->tx_tail)
+			return false;
+	}
+	return true;
+}
+
+static void macb_tx_lpi_work_fn(struct work_struct *work)
+{
+	struct macb *bp = container_of(work, struct macb, tx_lpi_work.work);
+
+	if (bp->eee_active && macb_tx_all_queues_idle(bp))
+		macb_tx_lpi_set(bp, true);
+}
+
+static void macb_tx_lpi_schedule(struct macb *bp)
+{
+	if (bp->eee_active)
+		mod_delayed_work(system_wq, &bp->tx_lpi_work,
+				 usecs_to_jiffies(bp->tx_lpi_timer));
+}
+
+/* Wake from LPI before transmitting. The MAC must deassert TXLPIEN
+ * and wait for the PHY to exit LPI before any frame can be sent.
+ * IEEE 802.3az Tw_sys is ~17us for 1000BASE-T, ~30us for 100BASE-TX;
+ * we use a conservative 50us.
+ */
+static void macb_tx_lpi_wake(struct macb *bp)
+{
+	if (!macb_tx_lpi_set(bp, false))
+		return;
+
+	cancel_delayed_work(&bp->tx_lpi_work);
+	udelay(50);
+}
+
+static void macb_mac_disable_tx_lpi(struct phylink_config *config)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct macb *bp = netdev_priv(ndev);
+
+	bp->eee_active = false;
+	cancel_delayed_work_sync(&bp->tx_lpi_work);
+	macb_tx_lpi_set(bp, false);
+}
+
+static int macb_mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
+				  bool tx_clk_stop)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct macb *bp = netdev_priv(ndev);
+
+	bp->tx_lpi_timer = timer;
+	bp->eee_active = true;
+
+	/* Defer initial LPI entry by 1 second after link-up per
+	 * IEEE 802.3az section 22.7a.
+	 */
+	mod_delayed_work(system_wq, &bp->tx_lpi_work, msecs_to_jiffies(1000));
+
+	return 0;
+}
+
 static void macb_mac_config(struct phylink_config *config, unsigned int mode,
 			    const struct phylink_link_state *state)
 {
@@ -753,6 +841,8 @@ static const struct phylink_mac_ops macb_phylink_ops = {
 	.mac_config = macb_mac_config,
 	.mac_link_down = macb_mac_link_down,
 	.mac_link_up = macb_mac_link_up,
+	.mac_disable_tx_lpi = macb_mac_disable_tx_lpi,
+	.mac_enable_tx_lpi = macb_mac_enable_tx_lpi,
 };
 
 static bool macb_phy_handle_exists(struct device_node *dn)
@@ -848,6 +938,18 @@ static int macb_mii_probe(struct net_device *dev)
 		}
 	}
 
+	/* Configure EEE LPI if supported */
+	if (bp->caps & MACB_CAPS_EEE) {
+		__set_bit(PHY_INTERFACE_MODE_MII,
+			  bp->phylink_config.lpi_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_GMII,
+			  bp->phylink_config.lpi_interfaces);
+		phy_interface_set_rgmii(bp->phylink_config.lpi_interfaces);
+		bp->phylink_config.lpi_capabilities = MAC_100FD | MAC_1000FD;
+		bp->phylink_config.lpi_timer_default = 250000;
+		bp->phylink_config.eee_enabled_default = true;
+	}
+
 	bp->phylink = phylink_create(&bp->phylink_config, bp->pdev->dev.fwnode,
 				     bp->phy_interface, &macb_phylink_ops);
 	if (IS_ERR(bp->phylink)) {
@@ -1244,6 +1346,9 @@ static int macb_tx_complete(struct macb_queue *queue, int budget)
 		netif_wake_subqueue(bp->dev, queue_index);
 	spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
 
+	if (packets)
+		macb_tx_lpi_schedule(bp);
+
 	return packets;
 }
 
@@ -2349,6 +2454,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index),
 			     skb->len);
 
+	macb_tx_lpi_wake(bp);
+
 	spin_lock(&bp->lock);
 	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
 	spin_unlock(&bp->lock);
@@ -3010,6 +3117,8 @@ static int macb_close(struct net_device *dev)
 		netdev_tx_reset_queue(netdev_get_tx_queue(dev, q));
 	}
 
+	cancel_delayed_work_sync(&bp->tx_lpi_work);
+
 	phylink_stop(bp->phylink);
 	phylink_disconnect_phy(bp->phylink);
 
@@ -5616,6 +5725,7 @@ static int macb_probe(struct platform_device *pdev)
 	}
 
 	INIT_WORK(&bp->hresp_err_bh_work, macb_hresp_error_task);
+	INIT_DELAYED_WORK(&bp->tx_lpi_work, macb_tx_lpi_work_fn);
 
 	netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
 		    macb_is_gem(bp) ? "GEM" : "MACB", macb_readl(bp, MID),
@@ -5659,6 +5769,7 @@ static void macb_remove(struct platform_device *pdev)
 		mdiobus_free(bp->mii_bus);
 
 		device_set_wakeup_enable(&bp->pdev->dev, 0);
+		cancel_delayed_work_sync(&bp->tx_lpi_work);
 		cancel_work_sync(&bp->hresp_err_bh_work);
 		pm_runtime_disable(&pdev->dev);
 		pm_runtime_dont_use_autosuspend(&pdev->dev);
-- 
2.51.0

[PATCH 4/5] net: cadence: macb: add ethtool EEE support

[Nicolai Buchwitz]

Implement ethtool get_eee and set_eee operations for the Cadence GEM
MAC, delegating to phylink for PHY-level EEE negotiation state.

The MAC-level LPI control (TXLPIEN) is not manipulated directly in the
ethtool ops - phylink manages the full EEE lifecycle through the
mac_enable_tx_lpi / mac_disable_tx_lpi callbacks.

Both ops are gated on MACB_CAPS_EEE; platforms without the capability
flag return -EOPNOTSUPP.

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

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index a9ae7ecf9fe9..37a5b866e3a5 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4033,6 +4033,26 @@ static const struct ethtool_ops macb_ethtool_ops = {
 	.set_ringparam		= macb_set_ringparam,
 };
 
+static int macb_get_eee(struct net_device *dev, struct ethtool_keee *eee)
+{
+	struct macb *bp = netdev_priv(dev);
+
+	if (!(bp->caps & MACB_CAPS_EEE))
+		return -EOPNOTSUPP;
+
+	return phylink_ethtool_get_eee(bp->phylink, eee);
+}
+
+static int macb_set_eee(struct net_device *dev, struct ethtool_keee *eee)
+{
+	struct macb *bp = netdev_priv(dev);
+
+	if (!(bp->caps & MACB_CAPS_EEE))
+		return -EOPNOTSUPP;
+
+	return phylink_ethtool_set_eee(bp->phylink, eee);
+}
+
 static const struct ethtool_ops gem_ethtool_ops = {
 	.get_regs_len		= macb_get_regs_len,
 	.get_regs		= macb_get_regs,
@@ -4054,6 +4074,8 @@ static const struct ethtool_ops gem_ethtool_ops = {
 	.get_rxnfc			= gem_get_rxnfc,
 	.set_rxnfc			= gem_set_rxnfc,
 	.get_rx_ring_count		= gem_get_rx_ring_count,
+	.get_eee		= macb_get_eee,
+	.set_eee		= macb_set_eee,
 };
 
 static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-- 
2.51.0

[PATCH 5/5] net: cadence: macb: enable EEE for Raspberry Pi RP1

[Nicolai Buchwitz]

Enable IEEE 802.3az Energy Efficient Ethernet on the Raspberry Pi 5's
RP1 southbridge by adding MACB_CAPS_EEE to its platform config.

The RP1 contains a Cadence GEM_GXL MAC (revision 0x00070109) paired
with a Broadcom BCM54213PE PHY, both of which support EEE at
1000BASE-T and 100BASE-TX.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 37a5b866e3a5..7fe7e5338a2e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5517,7 +5517,8 @@ static const struct macb_config eyeq5_config = {
 static const struct macb_config raspberrypi_rp1_config = {
 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_CLK_HW_CHG |
 		MACB_CAPS_JUMBO |
-		MACB_CAPS_GEM_HAS_PTP,
+		MACB_CAPS_GEM_HAS_PTP |
+		MACB_CAPS_EEE,
 	.dma_burst_length = 16,
 	.clk_init = macb_clk_init,
 	.init = macb_init,
-- 
2.51.0

Re: [PATCH net-next v3 0/5] net: cadence: macb: add IEEE 802.3az EEE support

[Nicolai Buchwitz]

On 25.2.2026 10:15, Nicolai Buchwitz wrote:
> Add Energy Efficient Ethernet (IEEE 802.3az) support to the Cadence GEM
> (macb) driver using phylink's managed EEE framework. The GEM MAC has
> hardware LPI registers but no built-in idle timer, so the driver
> implements software-managed TX LPI using a delayed_work timer while
> delegating EEE negotiation and ethtool state to phylink.
> 
> Changes from v2:
>   - macb_tx_lpi_set() now returns bool indicating whether the register
>     value actually changed, avoiding redundant writes.
>   - Removed tx_lpi_enabled field from struct macb; LPI state is tracked
>     entirely within the spinlock-protected register read/modify/write.
>   - macb_tx_lpi_wake() uses the return value of macb_tx_lpi_set() to
>     skip the cancel/udelay when TXLPIEN was already clear.
>   All changes based on feedback from Russell King.
> 

Apologies for the missing subject prefix and version tags on this
series - should have been [PATCH net-next v3]. I'm still fairly new
to the kernel mailing list workflow and building up the muscle memory
around git-format-patch/send-email. I'll resend as v4 with the proper
tags. In the meantime, feedback on the code itself is very welcome.

> Changes from v1:
>   - Rewrote to use phylink managed EEE (mac_enable_tx_lpi /
>     mac_disable_tx_lpi callbacks) instead of the obsolete
>     phy_init_eee() approach, as recommended by Russell King.
>   - ethtool get_eee/set_eee are now pure phylink passthroughs.
>   - Removed all manual EEE state tracking from mac_link_up/down;
>     phylink handles the lifecycle.
> 
> The series is structured as follows:
> 
>   1. Register definitions: LPI counter offsets (0x270-0x27c), TXLPIEN
>      bitfield (NCR bit 19), and MACB_CAPS_EEE capability flag.
> 
>   2. LPI statistics: Expose the four hardware EEE counters (RX/TX LPI
>      transitions and time) through ethtool -S, accumulated in software
>      since they are clear-on-read.
> 
>   3. TX LPI engine: phylink mac_enable_tx_lpi / mac_disable_tx_lpi
>      callbacks with a delayed_work-based idle timer. LPI entry is
>      deferred 1 second after link-up per IEEE 802.3az. Wake before
>      transmit with a conservative 50us PHY wake delay.
> 
>   4. ethtool EEE ops: get_eee/set_eee delegating to phylink for PHY
>      negotiation and timer management.
> 
>   5. RP1 enablement: Set MACB_CAPS_EEE for the Raspberry Pi 5's RP1
>      southbridge (Cadence GEM_GXL rev 0x00070109 + BCM54213PE PHY).
> 
> Tested on Raspberry Pi 5 (1000BASE-T, BCM54213PE PHY, 250ms LPI timer):
> 
>   iperf3 throughput (no regression):
>     TCP TX: 937.8 Mbit/s (EEE on) vs 937.0 Mbit/s (EEE off)
>     TCP RX: 936.5 Mbit/s both
> 
>   Latency (ping RTT, small expected increase from LPI wake):
>     1s interval:  0.273 ms (EEE on) vs 0.181 ms (EEE off)
>     10ms interval: 0.206 ms (EEE on) vs 0.168 ms (EEE off)
>     flood ping:   0.200 ms (EEE on) vs 0.156 ms (EEE off)
> 
>   LPI counters (ethtool -S, 1s-interval ping, EEE on):
>     tx_lpi_transitions: 112
>     tx_lpi_time: 15574651
> 
>   Zero packet loss across all tests. Also verified with
>   ethtool --show-eee / --set-eee and cable unplug/replug cycling.
> 
> Nicolai Buchwitz (5):
>   net: cadence: macb: add EEE register definitions and capability flag
>   net: cadence: macb: add EEE LPI statistics counters
>   net: cadence: macb: implement EEE TX LPI support
>   net: cadence: macb: add ethtool EEE support
>   net: cadence: macb: enable EEE for Raspberry Pi RP1
> 
>  drivers/net/ethernet/cadence/macb.h      |  20 ++++
>  drivers/net/ethernet/cadence/macb_main.c | 136 ++++++++++++++++++++++-
>  2 files changed, 155 insertions(+), 1 deletion(-)

Nicolai

Re: [PATCH 1/5] net: cadence: macb: add EEE register definitions and capability flag

[Théo Lebrun]

Hello Nicolai,

On Wed Feb 25, 2026 at 10:15 AM CET, Nicolai Buchwitz wrote:
> Add register and bitfield definitions for the Cadence GEM MAC's
> IEEE 802.3az Energy Efficient Ethernet (EEE) support:

nit: could this patch be squashed into [3/5]?

> - LPI statistics counter registers (GEM_RXLPI, GEM_RXLPITIME,
>   GEM_TXLPI, GEM_TXLPITIME) at offsets 0x270-0x27c
> - TX LPI enable bitfield (GEM_TXLPIEN) in the NCR register (bit 19),
>   which directly asserts/deasserts LPI on the transmit path
> - MACB_CAPS_EEE capability flag to gate EEE support per platform
>
> These registers are present in all Cadence GEM revisions that support
> EEE (verified on SAMA5D2, SAME70, PIC32CZ, and RP1 variants).

Would the capability flag deserve to be added to any other compatible?

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Sai Krishna Gajula]

> -----Original Message-----
> From: Nicolai Buchwitz <nb@tipi-net.de>
> Sent: Wednesday, February 25, 2026 2:46 PM
> To: netdev@vger.kernel.org
> Cc: andrew+netdev@lunn.ch; claudiu.beznea@tuxon.dev;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> nicolas.ferre@microchip.com; pabeni@redhat.com; linux@armlinux.org.uk;
> phil@raspberrypi.com; Nicolai Buchwitz <nb@tipi-net.de>
> Subject: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI
> support
> 
> Implement Energy Efficient Ethernet TX Low Power Idle using phylink's
> managed EEE framework. The Cadence GEM MAC has no built-in idle timer
> — TXLPIEN (NCR bit 19) immediately blocks all TX when set and the MAC does
> NOT auto-wake — so the driver 
> Implement Energy Efficient Ethernet TX Low Power Idle using phylink's
> managed EEE framework. The Cadence GEM MAC has no built-in idle timer
> — TXLPIEN (NCR bit 19) immediately blocks all TX when set and the MAC does
> NOT auto-wake — so the driver uses a software delayed_work timer for idle
> detection.
> 
> The TX LPI lifecycle:
>   - phylink calls mac_enable_tx_lpi() after link-up with the negotiated
>     timer value. The driver defers the first LPI entry by 1 second per
>     IEEE 802.3az section 22.7a.
>   - macb_tx_complete() reschedules the idle timer after each TX drain.
>   - macb_start_xmit() wakes from LPI by clearing TXLPIEN, cancelling
>     the pending work, and waiting 50us (conservative Tw_sys) before
>     initiating the transmit.
>   - phylink calls mac_disable_tx_lpi() before link-down, which cancels
>     the work and clears TXLPIEN.
> 
> The phylink_config is populated with LPI capabilities (MII, GMII, RGMII
> modes; 100FD and 1000FD speeds) and a 250ms default idle timer, gated on
> MACB_CAPS_EEE.
> 
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> ---
>  drivers/net/ethernet/cadence/macb.h      |   5 +
>  drivers/net/ethernet/cadence/macb_main.c | 111
> +++++++++++++++++++++++
>  2 files changed, 116 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h
> b/drivers/net/ethernet/cadence/macb.h
> index e3520e5aec67..c69828b27dae 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1372,6 +1372,11 @@ struct macb {
> 
>  	struct work_struct	hresp_err_bh_work;
> 
> +	/* EEE / LPI state */
> +	bool			eee_active;
> +	struct delayed_work	tx_lpi_work;
> +	u32			tx_lpi_timer;
> +
>  	int	rx_bd_rd_prefetch;
>  	int	tx_bd_rd_prefetch;
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c
> b/drivers/net/ethernet/cadence/macb_main.c
> index 43cd013bb70e..a9ae7ecf9fe9 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -10,6 +10,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/clk.h>
>  #include <linux/crc32.h>
> +#include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/etherdevice.h>
>  #include <linux/firmware/xlnx-zynqmp.h> @@ -589,6 +590,93 @@ static
> const struct phylink_pcs_ops macb_phylink_pcs_ops = {
>  	.pcs_config = macb_pcs_config,
>  };
> 
> +static bool macb_tx_lpi_set(struct macb *bp, bool enable) {
> +	unsigned long flags;
> +	u32 old, ncr;
> +
> +	spin_lock_irqsave(&bp->lock, flags);
> +	old = ncr = macb_readl(bp, NCR);
> +	if (enable)
> +		ncr |= GEM_BIT(TXLPIEN);
> +	else
> +		ncr &= ~GEM_BIT(TXLPIEN);
> +	if (old != ncr)
> +		macb_writel(bp, NCR, ncr);
> +	spin_unlock_irqrestore(&bp->lock, flags);
> +
> +	return old != ncr;
> +}
> +
> +static bool macb_tx_all_queues_idle(struct macb *bp) {
> +	unsigned int q;
> +
> +	for (q = 0; q < bp->num_queues; q++) {
> +		struct macb_queue *queue = &bp->queues[q];
> +
> +		if (queue->tx_head != queue->tx_tail)
> +			return false;
> +	}
> +	return true;
> +}
> +
> +static void macb_tx_lpi_work_fn(struct work_struct *work) {
> +	struct macb *bp = container_of(work, struct macb, tx_lpi_work.work);
> +
> +	if (bp->eee_active && macb_tx_all_queues_idle(bp))
> +		macb_tx_lpi_set(bp, true);
> +}
> +
> +static void macb_tx_lpi_schedule(struct macb *bp) {
> +	if (bp->eee_active)
> +		mod_delayed_work(system_wq, &bp->tx_lpi_work,
> +				 usecs_to_jiffies(bp->tx_lpi_timer));
> +}
> +
> +/* Wake from LPI before transmitting. The MAC must deassert TXLPIEN
> + * and wait for the PHY to exit LPI before any frame can be sent.
> + * IEEE 802.3az Tw_sys is ~17us for 1000BASE-T, ~30us for 100BASE-TX;
> + * we use a conservative 50us.
> + */
> +static void macb_tx_lpi_wake(struct macb *bp) {
> +	if (!macb_tx_lpi_set(bp, false))
> +		return;
> +
> +	cancel_delayed_work(&bp->tx_lpi_work);
> +	udelay(50);
Any reason the 50µs busy-wait in xmit path? Can it be avoided?

> +}
> +
> +static void macb_mac_disable_tx_lpi(struct phylink_config *config) {
> +	struct net_device *ndev = to_net_dev(config->dev);
> +	struct macb *bp = netdev_priv(ndev);
> +
> +	bp->eee_active = false;
> +	cancel_delayed_work_sync(&bp->tx_lpi_work);
> +	macb_tx_lpi_set(bp, false);
> +}
> +
> +static int macb_mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
> +				  bool tx_clk_stop)
> +{
> +	struct net_device *ndev = to_net_dev(config->dev);
> +	struct macb *bp = netdev_priv(ndev);
> +
> +	bp->tx_lpi_timer = timer;
> +	bp->eee_active = true;
> +
> +	/* Defer initial LPI entry by 1 second after link-up per
> +	 * IEEE 802.3az section 22.7a.
> +	 */
> +	mod_delayed_work(system_wq, &bp->tx_lpi_work,
> msecs_to_jiffies(1000));
> +
> +	return 0;
> +}
> +
>  static void macb_mac_config(struct phylink_config *config, unsigned int
> mode,
>  			    const struct phylink_link_state *state)  { @@ -753,6
> +841,8 @@ static const struct phylink_mac_ops macb_phylink_ops = {
>  	.mac_config = macb_mac_config,
>  	.mac_link_down = macb_mac_link_down,
>  	.mac_link_up = macb_mac_link_up,
> +	.mac_disable_tx_lpi = macb_mac_disable_tx_lpi,
> +	.mac_enable_tx_lpi = macb_mac_enable_tx_lpi,
>  };
> 
>  static bool macb_phy_handle_exists(struct device_node *dn) @@ -848,6
> +938,18 @@ static int macb_mii_probe(struct net_device *dev)
>  		}
>  	}
> 
> +	/* Configure EEE LPI if supported */
> +	if (bp->caps & MACB_CAPS_EEE) {
> +		__set_bit(PHY_INTERFACE_MODE_MII,
> +			  bp->phylink_config.lpi_interfaces);
> +		__set_bit(PHY_INTERFACE_MODE_GMII,
> +			  bp->phylink_config.lpi_interfaces);
> +		phy_interface_set_rgmii(bp->phylink_config.lpi_interfaces);
> +		bp->phylink_config.lpi_capabilities = MAC_100FD |
> MAC_1000FD;
> +		bp->phylink_config.lpi_timer_default = 250000;
> +		bp->phylink_config.eee_enabled_default = true;
> +	}
> +
>  	bp->phylink = phylink_create(&bp->phylink_config, bp->pdev-
> >dev.fwnode,
>  				     bp->phy_interface, &macb_phylink_ops);
>  	if (IS_ERR(bp->phylink)) {
> @@ -1244,6 +1346,9 @@ static int macb_tx_complete(struct macb_queue
> *queue, int budget)
>  		netif_wake_subqueue(bp->dev, queue_index);
>  	spin_unlock_irqrestore(&queue->tx_ptr_lock, flags);
> 
> +	if (packets)
> +		macb_tx_lpi_schedule(bp);
> +
>  	return packets;
>  }
> 
> @@ -2349,6 +2454,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff
> *skb, struct net_device *dev)
>  	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev,
> queue_index),
>  			     skb->len);
> 
> +	macb_tx_lpi_wake(bp);
> +
>  	spin_lock(&bp->lock);
>  	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
>  	spin_unlock(&bp->lock);
> @@ -3010,6 +3117,8 @@ static int macb_close(struct net_device *dev)
>  		netdev_tx_reset_queue(netdev_get_tx_queue(dev, q));
>  	}
> 
> +	cancel_delayed_work_sync(&bp->tx_lpi_work);
> +
>  	phylink_stop(bp->phylink);
>  	phylink_disconnect_phy(bp->phylink);
> 
> @@ -5616,6 +5725,7 @@ static int macb_probe(struct platform_device
> *pdev)
>  	}
> 
>  	INIT_WORK(&bp->hresp_err_bh_work, macb_hresp_error_task);
> +	INIT_DELAYED_WORK(&bp->tx_lpi_work, macb_tx_lpi_work_fn);
> 
>  	netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d
> (%pM)\n",
>  		    macb_is_gem(bp) ? "GEM" : "MACB", macb_readl(bp,
> MID), @@ -5659,6 +5769,7 @@ static void macb_remove(struct
> platform_device *pdev)
>  		mdiobus_free(bp->mii_bus);
> 
>  		device_set_wakeup_enable(&bp->pdev->dev, 0);
> +		cancel_delayed_work_sync(&bp->tx_lpi_work);
>  		cancel_work_sync(&bp->hresp_err_bh_work);
>  		pm_runtime_disable(&pdev->dev);
>  		pm_runtime_dont_use_autosuspend(&pdev->dev);
> --
> 2.51.0
> 

Re: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Théo Lebrun]

On Wed Feb 25, 2026 at 10:15 AM CET, Nicolai Buchwitz wrote:
> Implement Energy Efficient Ethernet TX Low Power Idle using phylink's
> managed EEE framework. The Cadence GEM MAC has no built-in idle timer
> — TXLPIEN (NCR bit 19) immediately blocks all TX when set and the MAC
> does NOT auto-wake — so the driver uses a software delayed_work timer
> for idle detection.
>
> The TX LPI lifecycle:
>   - phylink calls mac_enable_tx_lpi() after link-up with the negotiated
>     timer value. The driver defers the first LPI entry by 1 second per
>     IEEE 802.3az section 22.7a.
>   - macb_tx_complete() reschedules the idle timer after each TX drain.
>   - macb_start_xmit() wakes from LPI by clearing TXLPIEN, cancelling
>     the pending work, and waiting 50us (conservative Tw_sys) before
>     initiating the transmit.
>   - phylink calls mac_disable_tx_lpi() before link-down, which cancels
>     the work and clears TXLPIEN.
>
> The phylink_config is populated with LPI capabilities (MII, GMII, RGMII
> modes; 100FD and 1000FD speeds) and a 250ms default idle timer, gated
> on MACB_CAPS_EEE.
>
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> ---
>  drivers/net/ethernet/cadence/macb.h      |   5 +
>  drivers/net/ethernet/cadence/macb_main.c | 111 +++++++++++++++++++++++
>  2 files changed, 116 insertions(+)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 43cd013bb70e..a9ae7ecf9fe9 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c

[...]

> +/* Wake from LPI before transmitting. The MAC must deassert TXLPIEN
> + * and wait for the PHY to exit LPI before any frame can be sent.
> + * IEEE 802.3az Tw_sys is ~17us for 1000BASE-T, ~30us for 100BASE-TX;
> + * we use a conservative 50us.
> + */
> +static void macb_tx_lpi_wake(struct macb *bp)
> +{
> +	if (!macb_tx_lpi_set(bp, false))
> +		return;
> +
> +	cancel_delayed_work(&bp->tx_lpi_work);
> +	udelay(50);
> +}

[...]

> @@ -2349,6 +2454,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index),
>  			     skb->len);
>  
> +	macb_tx_lpi_wake(bp);

Should this be protected by a bp->eee_active condition? It could go
in macb_tx_lpi_wake(). We avoid a spinlock acquire per xmit for most
platforms. Probably negligeable though.

For the full series:

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>

Thanks!

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Russell King (Oracle)]

On Wed, Feb 25, 2026 at 06:42:08PM +0100, Théo Lebrun wrote:
> On Wed Feb 25, 2026 at 10:15 AM CET, Nicolai Buchwitz wrote:
> > @@ -2349,6 +2454,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
> >  	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index),
> >  			     skb->len);
> >  
> > +	macb_tx_lpi_wake(bp);
> 
> Should this be protected by a bp->eee_active condition? It could go
> in macb_tx_lpi_wake(). We avoid a spinlock acquire per xmit for most
> platforms. Probably negligeable though.

It will read the register, find the bit clear, and then return if
EEE is already disabled.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 1/5] net: cadence: macb: add EEE register definitions and capability flag

[Jakub Kicinski]

On Wed, 25 Feb 2026 18:26:18 +0100 Théo Lebrun wrote:
> On Wed Feb 25, 2026 at 10:15 AM CET, Nicolai Buchwitz wrote:
> > Add register and bitfield definitions for the Cadence GEM MAC's
> > IEEE 802.3az Energy Efficient Ethernet (EEE) support:  
> 
> nit: could this patch be squashed into [3/5]?

Maybe split into two in fact, the relevant part squashed into patch 2
and the rest in patch 3?

Re: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Nicolai Buchwitz]

On 25.2.2026 18:32, Sai Krishna Gajula wrote:
>> -----Original Message-----
>> From: Nicolai Buchwitz <nb@tipi-net.de>
>> Sent: Wednesday, February 25, 2026 2:46 PM
>> To: netdev@vger.kernel.org
>> Cc: andrew+netdev@lunn.ch; claudiu.beznea@tuxon.dev;
>> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
>> nicolas.ferre@microchip.com; pabeni@redhat.com; linux@armlinux.org.uk;
>> phil@raspberrypi.com; Nicolai Buchwitz <nb@tipi-net.de>
>> Subject: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI
>> support
>> 

[...]

>> +
>> +static void macb_tx_lpi_schedule(struct macb *bp) {
>> +	if (bp->eee_active)
>> +		mod_delayed_work(system_wq, &bp->tx_lpi_work,
>> +				 usecs_to_jiffies(bp->tx_lpi_timer));
>> +}
>> +
>> +/* Wake from LPI before transmitting. The MAC must deassert TXLPIEN
>> + * and wait for the PHY to exit LPI before any frame can be sent.
>> + * IEEE 802.3az Tw_sys is ~17us for 1000BASE-T, ~30us for 100BASE-TX;
>> + * we use a conservative 50us.
>> + */
>> +static void macb_tx_lpi_wake(struct macb *bp) {
>> +	if (!macb_tx_lpi_set(bp, false))
>> +		return;
>> +
>> +	cancel_delayed_work(&bp->tx_lpi_work);
>> +	udelay(50);
> Any reason the 50µs busy-wait in xmit path? Can it be avoided?
> 

The 50us delay is required by IEEE 802.3az. After clearing TXLPIEN the
PHY needs time to exit LPI and re-establish normal operation before the
MAC can transmit. This is defined as Tw_sys_tx - approximately 16.5us
for 1000BASE-T and 30us for 100BASE-TX. We use a conservative 50us to
cover both speeds without needing to track the current link rate.

Testing without the delay confirmed packet loss on the first frame after
waking from LPI, as the PHY had not yet fully transitioned back to
normal mode.

This delay is specific to the GEM MAC because TXLPIEN directly and
immediately controls LPI with no hardware enforcement of Tw_sys_tx - the
software must enforce it. Other phylink-managed EEE drivers (mvneta,
stmmac, lan743x, mtk) don't need an explicit delay because their
hardware handles the wake timing automatically.

The cost is also bounded: the delay only fires when TXLPIEN was
actually asserted, i.e. on the first transmit after an idle period.

[...]

Nicolai

Re: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Théo Lebrun]

On Wed Feb 25, 2026 at 6:50 PM CET, Russell King (Oracle) wrote:
> On Wed, Feb 25, 2026 at 06:42:08PM +0100, Théo Lebrun wrote:
>> On Wed Feb 25, 2026 at 10:15 AM CET, Nicolai Buchwitz wrote:
>> > @@ -2349,6 +2454,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>> >  	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index),
>> >  			     skb->len);
>> >  
>> > +	macb_tx_lpi_wake(bp);
>> 
>> Should this be protected by a bp->eee_active condition? It could go
>> in macb_tx_lpi_wake(). We avoid a spinlock acquire per xmit for most
>> platforms. Probably negligeable though.
>
> It will read the register, find the bit clear, and then return if
> EEE is already disabled.

Yes I agree with your sentence, sorry my point was unclear. I was not
describing a bug but rather a performance optimisation.

We would look up bp->eee_active to know if we can avoid calling
macb_tx_lpi_set(), to avoid grabbing bp->lock once per xmit.
That spinlock is interface-wide.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Nicolai Buchwitz]

On 26.2.2026 10:52, Théo Lebrun wrote:
> On Wed Feb 25, 2026 at 6:50 PM CET, Russell King (Oracle) wrote:
>> On Wed, Feb 25, 2026 at 06:42:08PM +0100, Théo Lebrun wrote:
>>> On Wed Feb 25, 2026 at 10:15 AM CET, Nicolai Buchwitz wrote:
>>> > @@ -2349,6 +2454,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>> >  	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index),
>>> >  			     skb->len);
>>> >
>>> > +	macb_tx_lpi_wake(bp);
>>> 
>>> Should this be protected by a bp->eee_active condition? It could go
>>> in macb_tx_lpi_wake(). We avoid a spinlock acquire per xmit for most
>>> platforms. Probably negligeable though.
>> 
>> It will read the register, find the bit clear, and then return if
>> EEE is already disabled.
> 
> Yes I agree with your sentence, sorry my point was unclear. I was not
> describing a bug but rather a performance optimisation.
> 
> We would look up bp->eee_active to know if we can avoid calling
> macb_tx_lpi_set(), to avoid grabbing bp->lock once per xmit.
> That spinlock is interface-wide.

So you're suggesting to revert to the v2 approach?
https://patchwork.kernel.org/project/netdevbpf/patch/20260224091821.47671-4-nb@tipi-net.de/
> 
> Thanks,
> 
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Nicolai

Re: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Théo Lebrun]

On Thu Feb 26, 2026 at 11:49 AM CET, Nicolai Buchwitz wrote:
> On 26.2.2026 10:52, Théo Lebrun wrote:
>> On Wed Feb 25, 2026 at 6:50 PM CET, Russell King (Oracle) wrote:
>>> On Wed, Feb 25, 2026 at 06:42:08PM +0100, Théo Lebrun wrote:
>>>> On Wed Feb 25, 2026 at 10:15 AM CET, Nicolai Buchwitz wrote:
>>>> > @@ -2349,6 +2454,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>>> >  	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index),
>>>> >  			     skb->len);
>>>> >
>>>> > +	macb_tx_lpi_wake(bp);
>>>> 
>>>> Should this be protected by a bp->eee_active condition? It could go
>>>> in macb_tx_lpi_wake(). We avoid a spinlock acquire per xmit for most
>>>> platforms. Probably negligeable though.
>>> 
>>> It will read the register, find the bit clear, and then return if
>>> EEE is already disabled.
>> 
>> Yes I agree with your sentence, sorry my point was unclear. I was not
>> describing a bug but rather a performance optimisation.
>> 
>> We would look up bp->eee_active to know if we can avoid calling
>> macb_tx_lpi_set(), to avoid grabbing bp->lock once per xmit.
>> That spinlock is interface-wide.
>
> So you're suggesting to revert to the v2 approach?
> https://patchwork.kernel.org/project/netdevbpf/patch/20260224091821.47671-4-nb@tipi-net.de/

I discovered this series at its V3.
It seems like I was suggesting a mix of both V2 & V3 approaches.

// V2 was:
static void macb_tx_lpi_wake(struct macb *bp)
{
	if (!bp->eee_active)
		return;

	macb_tx_lpi_set(bp, false);
	cancel_delayed_work(&bp->tx_lpi_work);
	udelay(50);
}

// V3 & V3 are:
static void macb_tx_lpi_wake(struct macb *bp)
{
	if (!macb_tx_lpi_set(bp, false))
		return;

	cancel_delayed_work(&bp->tx_lpi_work);
	udelay(50);
}

// I was suggesting:
static void macb_tx_lpi_wake(struct macb *bp)
{
	if (!eee_active || !macb_tx_lpi_set(bp, false))
		return;

	cancel_delayed_work(&bp->tx_lpi_work);
	udelay(50);
}

But talking about perf, let's measure.

I don't get any measurable performance improvement on average/max time
spent in macb_start_xmit() between net-next and this series. We spend
<3µs per call anyway. 10s iperf3 1G TCP test on EyeQ5, ftrace function
graph tracer on macb_start_xmit().

Summary: you can ignore my remark.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Nicolai Buchwitz]

On 26.2.2026 14:50, Théo Lebrun wrote:
> On Thu Feb 26, 2026 at 11:49 AM CET, Nicolai Buchwitz wrote:
>> On 26.2.2026 10:52, Théo Lebrun wrote:
>>> On Wed Feb 25, 2026 at 6:50 PM CET, Russell King (Oracle) wrote:
>>>> On Wed, Feb 25, 2026 at 06:42:08PM +0100, Théo Lebrun wrote:
>>>>> On Wed Feb 25, 2026 at 10:15 AM CET, Nicolai Buchwitz wrote:
>>>>> > @@ -2349,6 +2454,8 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>>>>> >  	netdev_tx_sent_queue(netdev_get_tx_queue(bp->dev, queue_index),
>>>>> >  			     skb->len);
>>>>> >
>>>>> > +	macb_tx_lpi_wake(bp);
>>>>> 
>>>>> Should this be protected by a bp->eee_active condition? It could go
>>>>> in macb_tx_lpi_wake(). We avoid a spinlock acquire per xmit for 
>>>>> most
>>>>> platforms. Probably negligeable though.
>>>> 
>>>> It will read the register, find the bit clear, and then return if
>>>> EEE is already disabled.
>>> 
>>> Yes I agree with your sentence, sorry my point was unclear. I was not
>>> describing a bug but rather a performance optimisation.
>>> 
>>> We would look up bp->eee_active to know if we can avoid calling
>>> macb_tx_lpi_set(), to avoid grabbing bp->lock once per xmit.
>>> That spinlock is interface-wide.
>> 
>> So you're suggesting to revert to the v2 approach?
>> https://patchwork.kernel.org/project/netdevbpf/patch/20260224091821.47671-4-nb@tipi-net.de/
> 
> I discovered this series at its V3.
> It seems like I was suggesting a mix of both V2 & V3 approaches.
> 
> // V2 was:
> static void macb_tx_lpi_wake(struct macb *bp)
> {
> 	if (!bp->eee_active)
> 		return;
> 
> 	macb_tx_lpi_set(bp, false);
> 	cancel_delayed_work(&bp->tx_lpi_work);
> 	udelay(50);
> }
> 
> // V3 & V3 are:
> static void macb_tx_lpi_wake(struct macb *bp)
> {
> 	if (!macb_tx_lpi_set(bp, false))
> 		return;
> 
> 	cancel_delayed_work(&bp->tx_lpi_work);
> 	udelay(50);
> }
> 
> // I was suggesting:
> static void macb_tx_lpi_wake(struct macb *bp)
> {
> 	if (!eee_active || !macb_tx_lpi_set(bp, false))
> 		return;
> 
> 	cancel_delayed_work(&bp->tx_lpi_work);
> 	udelay(50);
> }
> 
> But talking about perf, let's measure.

Tthanks for testing! Since you have the hardware at hand, would you be 
willing to add MACB_CAPS_EEE to eyeq5_config as well? I'm happy to 
include it in the series if you can confirm it works correctly.

> 
> I don't get any measurable performance improvement on average/max time
> spent in macb_start_xmit() between net-next and this series. We spend
> <3µs per call anyway. 10s iperf3 1G TCP test on EyeQ5, ftrace function
> graph tracer on macb_start_xmit().
> 
> Summary: you can ignore my remark.
> 
> Thanks,
> 
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Nicolai

Re: [PATCH 3/5] net: cadence: macb: implement EEE TX LPI support

[Théo Lebrun]

Hello Nicolai,

On Fri Feb 27, 2026 at 10:00 AM CET, Nicolai Buchwitz wrote:
> On 26.2.2026 14:50, Théo Lebrun wrote:
>> On Thu Feb 26, 2026 at 11:49 AM CET, Nicolai Buchwitz wrote:
>> But talking about perf, let's measure.
>
> Tthanks for testing! Since you have the hardware at hand, would you be 
> willing to add MACB_CAPS_EEE to eyeq5_config as well? I'm happy to 
> include it in the series if you can confirm it works correctly.

Tested-by: Théo Lebrun <theo.lebrun@bootlin.com>

Tested using a hardware loopback and a net namespace.

# ip netns add netns_eth1
# ip link set eth1 netns netns_eth1
# ip -n netns_eth1 a add 10.0.0.2/24 dev eth1
# ip -n netns_eth1 link set eth1 up
# ip a add 10.0.0.1/24 dev eth0
# ip link set eth0 up

... ping and iperf3 ...

# ethtool --show-eee eth0
EEE settings for eth0:
enabled - active
250000 (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

# ip netns exec netns_eth1 ethtool --show-eee eth1
EEE settings for eth1:
enabled - active
250000 (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

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com