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

[PATCH net-next v5 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 v4:
  - Removed redundant MACB_CAPS_EEE guards from macb_get_eee/set_eee;
    phylink already returns -EOPNOTSUPP when lpi_capabilities and
    lpi_interfaces are not populated. Based on feedback from Russell King.
  - Added patch 5 enabling EEE for Mobileye EyeQ5, tested by Théo Lebrun
    using a hardware loopback.

Changes from v3:
  - Dropped the register-definitions-only patch; LPI counter offsets
    (GEM_RXLPI/RXLPITIME/TXLPI/TXLPITIME) now land in the statistics
    patch, and TXLPIEN + MACB_CAPS_EEE are introduced alongside the TX
    LPI implementation where they are first used. Series is now 4 patches.
  - Add Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com> to all patches.
  - Split chained assignment in macb_tx_lpi_set() (suggested by checkpatch).

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. 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. Adds register offset definitions
     GEM_RXLPI/RXLPITIME/TXLPI/TXLPITIME (0x270-0x27c).

  2. TX LPI engine: Introduces GEM_TXLPIEN (NCR bit 19) and
     MACB_CAPS_EEE alongside the implementation that uses them.
     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 (IEEE 802.3az Tw_sys_tx).

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

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

  5. EyeQ5 enablement: Set MACB_CAPS_EEE for the Mobileye EyeQ5 GEM
     instance, verified with a hardware loopback by Théo Lebrun.

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 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
  net: cadence: macb: enable EEE for Mobileye EyeQ5

 drivers/net/ethernet/cadence/macb.h      |  20 +++++
 drivers/net/ethernet/cadence/macb_main.c | 133 ++++++++++++++++++++++++++++++-
 2 files changed, 151 insertions(+), 2 deletions(-)

-- 
2.51.0

[PATCH net-next v5 1/5] net: cadence: macb: add EEE LPI statistics counters

[Nicolai Buchwitz]

The GEM MAC provides four read-only, clear-on-read LPI statistics
registers at offsets 0x270-0x27c:

  GEM_RXLPI     (0x270): RX LPI transition count (16-bit)
  GEM_RXLPITIME (0x274): cumulative RX LPI time (24-bit)
  GEM_TXLPI     (0x278): TX LPI transition count (16-bit)
  GEM_TXLPITIME (0x27c): cumulative TX LPI time (24-bit)

Add register offset definitions, extend struct gem_stats with
corresponding u64 software accumulators, and register the four
counters in gem_statistics[] so they appear in ethtool -S output.
Because the hardware counters clear on read, the existing
macb_update_stats() path accumulates them into the u64 fields on
every stats poll, preventing loss between userspace reads.

These registers are present on SAMA5D2, SAME70, PIC32CZ, and RP1
variants of the Cadence GEM IP and have been confirmed on RP1 via
devmem reads.

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/cadence/macb.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 87414a2ddf6e..19aa98d01c8c 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 */
@@ -1043,6 +1047,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
@@ -1142,6 +1150,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

Re: [PATCH net-next v5 1/5] net: cadence: macb: add EEE LPI statistics counters

[Claudiu Beznea]

On 2/27/26 17:06, Nicolai Buchwitz wrote:
> The GEM MAC provides four read-only, clear-on-read LPI statistics
> registers at offsets 0x270-0x27c:
> 
>    GEM_RXLPI     (0x270): RX LPI transition count (16-bit)
>    GEM_RXLPITIME (0x274): cumulative RX LPI time (24-bit)
>    GEM_TXLPI     (0x278): TX LPI transition count (16-bit)
>    GEM_TXLPITIME (0x27c): cumulative TX LPI time (24-bit)
> 
> Add register offset definitions, extend struct gem_stats with
> corresponding u64 software accumulators, and register the four
> counters in gem_statistics[] so they appear in ethtool -S output.
> Because the hardware counters clear on read, the existing
> macb_update_stats() path accumulates them into the u64 fields on
> every stats poll, preventing loss between userspace reads.
> 
> These registers are present on SAMA5D2, SAME70, PIC32CZ, and RP1
> variants of the Cadence GEM IP and have been confirmed on RP1 via
> devmem reads.
> 
> Reviewed-by: Théo Lebrun<theo.lebrun@bootlin.com>
> Signed-off-by: Nicolai Buchwitz<nb@tipi-net.de>

Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>

[PATCH net-next v5 2/5] net: cadence: macb: implement EEE TX LPI support

[Nicolai Buchwitz]

The GEM MAC has hardware LPI registers (NCR bit 19: TXLPIEN) but no
built-in idle timer, so asserting TXLPIEN blocks all TX immediately
with no automatic wake. A software idle timer is required, as noted
in Microchip documentation (section 40.6.19): "It is best to use
firmware to control LPI."

Implement phylink managed EEE using the mac_enable_tx_lpi and
mac_disable_tx_lpi callbacks:

- macb_tx_lpi_set(): atomically sets or clears TXLPIEN under the
  existing bp->lock spinlock; returns bool indicating whether the
  register actually changed, avoiding redundant writes.

- macb_tx_lpi_work_fn(): delayed_work handler that enters LPI if all
  TX queues are idle and EEE is still active.

- macb_tx_lpi_schedule(): arms the work timer using the LPI timer
  value provided by phylink (default 250 ms). Called from
  macb_tx_complete() after each TX drain so the idle countdown
  restarts whenever the ring goes quiet.

- macb_tx_lpi_wake(): called from macb_start_xmit() before TSTART.
  Clears TXLPIEN and applies a 50 us udelay for PHY wake (IEEE
  802.3az Tw_sys_tx is 16.5 us for 1000BASE-T / 30 us for
  100BASE-TX; GEM has no hardware enforcement). Only delays when
  TXLPIEN was actually set, avoiding overhead on the common path.
  The delay is placed after tx_head is advanced so the work_fn's
  queue-idle check sees a non-empty ring and cannot race back into
  LPI before the frame is transmitted.

- mac_enable_tx_lpi: stores the timer and sets eee_active, then
  defers the first LPI entry by 1 second per IEEE 802.3az section
  22.7a.

- mac_disable_tx_lpi: clears eee_active, cancels the work, and
  deasserts TXLPIEN.

Populate phylink_config lpi_interfaces (MII, GMII, RGMII variants)
and lpi_capabilities (MAC_100FD | MAC_1000FD) so phylink can
negotiate EEE with the PHY and call the callbacks appropriately.
Set lpi_timer_default to 250000 us and eee_enabled_default to true.

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/cadence/macb.h      |   8 ++
 drivers/net/ethernet/cadence/macb_main.c | 112 +++++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 19aa98d01c8c..c69828b27dae 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -309,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
 
@@ -783,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
@@ -1369,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 02eab26fd98b..c23485f049d3 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>
@@ -621,6 +622,94 @@ 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);
+	ncr = macb_readl(bp, NCR);
+	old = 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)
 {
@@ -769,6 +858,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)
@@ -864,6 +955,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)) {
@@ -1260,6 +1363,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;
 }
 
@@ -2365,6 +2471,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);
@@ -3026,6 +3134,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);
 
@@ -5633,6 +5743,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),
@@ -5676,6 +5787,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 net-next v5 2/5] net: cadence: macb: implement EEE TX LPI support

[Claudiu Beznea]

On 2/27/26 17:06, Nicolai Buchwitz wrote:
> The GEM MAC has hardware LPI registers (NCR bit 19: TXLPIEN) but no
> built-in idle timer, so asserting TXLPIEN blocks all TX immediately
> with no automatic wake. A software idle timer is required, as noted
> in Microchip documentation (section 40.6.19): "It is best to use
> firmware to control LPI."
> 
> Implement phylink managed EEE using the mac_enable_tx_lpi and
> mac_disable_tx_lpi callbacks:
> 
> - macb_tx_lpi_set(): atomically sets or clears TXLPIEN under the
>    existing bp->lock spinlock; returns bool indicating whether the
>    register actually changed, avoiding redundant writes.
> 
> - macb_tx_lpi_work_fn(): delayed_work handler that enters LPI if all
>    TX queues are idle and EEE is still active.
> 
> - macb_tx_lpi_schedule(): arms the work timer using the LPI timer
>    value provided by phylink (default 250 ms). Called from
>    macb_tx_complete() after each TX drain so the idle countdown
>    restarts whenever the ring goes quiet.
> 
> - macb_tx_lpi_wake(): called from macb_start_xmit() before TSTART.
>    Clears TXLPIEN and applies a 50 us udelay for PHY wake (IEEE
>    802.3az Tw_sys_tx is 16.5 us for 1000BASE-T / 30 us for
>    100BASE-TX; GEM has no hardware enforcement). Only delays when
>    TXLPIEN was actually set, avoiding overhead on the common path.
>    The delay is placed after tx_head is advanced so the work_fn's
>    queue-idle check sees a non-empty ring and cannot race back into
>    LPI before the frame is transmitted.
> 
> - mac_enable_tx_lpi: stores the timer and sets eee_active, then
>    defers the first LPI entry by 1 second per IEEE 802.3az section
>    22.7a.
> 
> - mac_disable_tx_lpi: clears eee_active, cancels the work, and
>    deasserts TXLPIEN.
> 
> Populate phylink_config lpi_interfaces (MII, GMII, RGMII variants)
> and lpi_capabilities (MAC_100FD | MAC_1000FD) so phylink can
> negotiate EEE with the PHY and call the callbacks appropriately.
> Set lpi_timer_default to 250000 us and eee_enabled_default to true.
> 
> Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>
> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
> ---
>   drivers/net/ethernet/cadence/macb.h      |   8 ++
>   drivers/net/ethernet/cadence/macb_main.c | 112 +++++++++++++++++++++++
>   2 files changed, 120 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 19aa98d01c8c..c69828b27dae 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -309,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
>   
> @@ -783,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
> @@ -1369,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 02eab26fd98b..c23485f049d3 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>
> @@ -621,6 +622,94 @@ 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);
> +	ncr = macb_readl(bp, NCR);
> +	old = 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];

In case there will be another version, to have a unified approach across the 
driver, this loop can be written as all of the loops on queues in this driver:

     for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
         // ...
     }

Apart from that:

Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>

Thank you,
Claudiu

Re: [PATCH net-next v5 2/5] net: cadence: macb: implement EEE TX LPI support

[Jakub Kicinski]

On Fri, 27 Feb 2026 16:06:07 +0100 Nicolai Buchwitz wrote:
> +static bool macb_tx_lpi_set(struct macb *bp, bool enable)
> +{
> +	unsigned long flags;
> +	u32 old, ncr;
> +
> +	spin_lock_irqsave(&bp->lock, flags);

we should optimize this function for the past path caller.
xmit path does:

+	macb_tx_lpi_wake(bp);
+
 	spin_lock(&bp->lock);

So it immediately takes that lock again, can we move the lpi_wake()
call under the spin_lock, and make sure other callers also take that
lock? I think you can add a lockdep assert to make sure spin lock is
held

> +	ncr = macb_readl(bp, NCR);
> +	old = 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)

Does not not need tx_ptr_lock technically?

> +			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))

Does this lpi_set() not have a relatively high cost, even if eee_active
is disabled? Reading registers is usually pretty slow. Can we add 
a eee_active check here as well to short cut the lpi check? 
If we do we probably want to make sure that the code paths setting
eee_active are also under bp->lock, otherwise this new check will be
racy.
-- 
pw-bot: cr

Re: [PATCH net-next v5 2/5] net: cadence: macb: implement EEE TX LPI support

[Théo Lebrun]

On Tue Mar 3, 2026 at 3:15 AM CET, Jakub Kicinski wrote:
> On Fri, 27 Feb 2026 16:06:07 +0100 Nicolai Buchwitz wrote:
>> +static bool macb_tx_lpi_set(struct macb *bp, bool enable)
>> +{
>> +	unsigned long flags;
>> +	u32 old, ncr;
>> +
>> +	spin_lock_irqsave(&bp->lock, flags);
>
> we should optimize this function for the past path caller.
> xmit path does:
>
> +	macb_tx_lpi_wake(bp);
> +
>  	spin_lock(&bp->lock);
>
> So it immediately takes that lock again, can we move the lpi_wake()
> call under the spin_lock, and make sure other callers also take that
> lock? I think you can add a lockdep assert to make sure spin lock is
> held
>
>> +	ncr = macb_readl(bp, NCR);
>> +	old = 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)
>
> Does not not need tx_ptr_lock technically?
>
>> +			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))
>
> Does this lpi_set() not have a relatively high cost, even if eee_active
> is disabled? Reading registers is usually pretty slow. Can we add 
> a eee_active check here as well to short cut the lpi check? 
> If we do we probably want to make sure that the code paths setting
> eee_active are also under bp->lock, otherwise this new check will be
> racy.

Funny how this discussion keeps coming up! I made the same remark on V3:
https://lore.kernel.org/netdev/DGOXXGNSSMYK.2XNU9AQ6E077P@bootlin.com/

And it had been discussed in a previous iteration before.

In theory I agree, in practice the optimization was statistically
insignificant on my platform. The total time spent in macb_start_xmit()
is tiny, so any optimization inside of it is even more so.

Thanks,

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

Re: [PATCH net-next v5 2/5] net: cadence: macb: implement EEE TX LPI support

[Jakub Kicinski]

On Tue, 03 Mar 2026 09:14:41 +0100 Théo Lebrun wrote:
> >> +/* 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))  
> >
> > Does this lpi_set() not have a relatively high cost, even if eee_active
> > is disabled? Reading registers is usually pretty slow. Can we add 
> > a eee_active check here as well to short cut the lpi check? 
> > If we do we probably want to make sure that the code paths setting
> > eee_active are also under bp->lock, otherwise this new check will be
> > racy.  
> 
> Funny how this discussion keeps coming up! I made the same remark on V3:
> https://lore.kernel.org/netdev/DGOXXGNSSMYK.2XNU9AQ6E077P@bootlin.com/
> 
> And it had been discussed in a previous iteration before.
> 
> In theory I agree, in practice the optimization was statistically
> insignificant on my platform. The total time spent in macb_start_xmit()
> is tiny, so any optimization inside of it is even more so.

TBH I started looking around because the v5 implementation seems racy.
Probably not in practice but in theory on a multiple queue device you
clear LPI, release the lock, then another queue may schedule LPI again,
if the xmit path is delayed for a long time the LPI work may turn idle
on before xmit rings the doorbell.

The rework I suggested is not only more optimal (dare I say logical to
an experienced developer) but also I think it'd be more correct.

macb has a crazy number of locks so maybe i'm missing something.
But sooner or later someone will hopefully start removing those locks,
cause this driver gotta be dog slow right now :/

[PATCH net-next v5 3/5] net: cadence: macb: add ethtool EEE support

[Nicolai Buchwitz]

Implement get_eee and set_eee ethtool ops for GEM as simple passthroughs
to phylink_ethtool_get_eee() and phylink_ethtool_set_eee().

No MACB_CAPS_EEE guard is needed: phylink returns -EOPNOTSUPP from both
ops when mac_supports_eee is false, which is the case when
lpi_capabilities and lpi_interfaces are not populated. Those fields are
only set when MACB_CAPS_EEE is present (previous patch), so phylink
already handles the unsupported case correctly.

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/cadence/macb_main.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c23485f049d3..3e724417d444 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4050,6 +4050,20 @@ 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);
+
+	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);
+
+	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,
@@ -4072,6 +4086,8 @@ static const struct ethtool_ops gem_ethtool_ops = {
 	.set_rxnfc			= gem_set_rxnfc,
 	.get_rx_ring_count		= gem_get_rx_ring_count,
 	.nway_reset			= phy_ethtool_nway_reset,
+	.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

Re: [PATCH net-next v5 3/5] net: cadence: macb: add ethtool EEE support

[Claudiu Beznea]

On 2/27/26 17:06, Nicolai Buchwitz wrote:
> Implement get_eee and set_eee ethtool ops for GEM as simple passthroughs
> to phylink_ethtool_get_eee() and phylink_ethtool_set_eee().
> 
> No MACB_CAPS_EEE guard is needed: phylink returns -EOPNOTSUPP from both
> ops when mac_supports_eee is false, which is the case when
> lpi_capabilities and lpi_interfaces are not populated. Those fields are
> only set when MACB_CAPS_EEE is present (previous patch), so phylink
> already handles the unsupported case correctly.
> 
> Reviewed-by: Théo Lebrun<theo.lebrun@bootlin.com>
> Signed-off-by: Nicolai Buchwitz<nb@tipi-net.de>

Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>

[PATCH net-next v5 4/5] net: cadence: macb: enable EEE for Raspberry Pi RP1

[Nicolai Buchwitz]

Set MACB_CAPS_EEE for the Raspberry Pi 5 RP1 southbridge
(Cadence GEM_GXL rev 0x00070109 paired with BCM54213PE PHY).

EEE has been verified on RP1 hardware: the LPI counter registers
at 0x270-0x27c return valid data, the TXLPIEN bit in NCR (bit 19)
controls LPI transmission correctly, and ethtool --show-eee reports
the negotiated state after link-up.

Other GEM variants that share the same LPI register layout (SAMA5D2,
SAME70, PIC32CZ) can be enabled by adding MACB_CAPS_EEE to their
respective config entries once tested.

Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>
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 3e724417d444..0196a13c0688 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5529,7 +5529,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 v5 4/5] net: cadence: macb: enable EEE for Raspberry Pi RP1

[Claudiu Beznea]

On 2/27/26 17:06, Nicolai Buchwitz wrote:
> Set MACB_CAPS_EEE for the Raspberry Pi 5 RP1 southbridge
> (Cadence GEM_GXL rev 0x00070109 paired with BCM54213PE PHY).
> 
> EEE has been verified on RP1 hardware: the LPI counter registers
> at 0x270-0x27c return valid data, the TXLPIEN bit in NCR (bit 19)
> controls LPI transmission correctly, and ethtool --show-eee reports
> the negotiated state after link-up.
> 
> Other GEM variants that share the same LPI register layout (SAMA5D2,
> SAME70, PIC32CZ) can be enabled by adding MACB_CAPS_EEE to their
> respective config entries once tested.
> 
> Reviewed-by: Théo Lebrun<theo.lebrun@bootlin.com>
> Signed-off-by: Nicolai Buchwitz<nb@tipi-net.de>

Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>

[PATCH net-next v5 5/5] net: cadence: macb: enable EEE for Mobileye EyeQ5

[Nicolai Buchwitz]

Set MACB_CAPS_EEE for the Mobileye EyeQ5 GEM instance. EEE has been
verified on EyeQ5 hardware using a loopback setup with ethtool
--show-eee confirming EEE active on both ends at 100baseT/Full and
1000baseT/Full.

Tested-by: Théo Lebrun <theo.lebrun@bootlin.com>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 drivers/net/ethernet/cadence/macb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 0196a13c0688..58a265ee9f9e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5518,7 +5518,7 @@ static const struct macb_config versal_config = {
 static const struct macb_config eyeq5_config = {
 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
 		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_QUEUE_DISABLE |
-		MACB_CAPS_NO_LSO,
+		MACB_CAPS_NO_LSO | MACB_CAPS_EEE,
 	.dma_burst_length = 16,
 	.clk_init = macb_clk_init,
 	.init = eyeq5_init,
-- 
2.51.0

Re: [PATCH net-next v5 5/5] net: cadence: macb: enable EEE for Mobileye EyeQ5

[Claudiu Beznea]

On 2/27/26 17:06, Nicolai Buchwitz wrote:
> Set MACB_CAPS_EEE for the Mobileye EyeQ5 GEM instance. EEE has been
> verified on EyeQ5 hardware using a loopback setup with ethtool
> --show-eee confirming EEE active on both ends at 100baseT/Full and
> 1000baseT/Full.
> 
> Tested-by: Théo Lebrun<theo.lebrun@bootlin.com>
> Signed-off-by: Nicolai Buchwitz<nb@tipi-net.de>

Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>