Thread (41 messages) 41 messages, 4 authors, 2d ago

Re: [PATCH net-next v3 06/15] net: macb: allocate tieoff descriptor once across device lifetime

From: Théo Lebrun <theo.lebrun@bootlin.com>
Date: 2026-07-15 13:48:27
Also in: lkml

On Mon Jul 13, 2026 at 2:00 PM CEST, Théo Lebrun wrote:
On Thu Jul 2, 2026 at 12:54 PM CEST, Nicolai Buchwitz wrote:
quoted
On 1.7.2026 17:59, Théo Lebrun wrote:
quoted
The tieoff descriptor is a RX DMA descriptor ring of size one. It gets
configured onto queues for Wake-on-LAN during system-wide suspend when
hardware does not support disabling individual queues
(MACB_CAPS_QUEUE_DISABLE).

MACB/GEM driver allocates it alongside the main RX ring
inside macb_alloc() at open. Free is done by macb_free() at close.

Change to allocate once at probe and free on probe failure or device
removal. This makes the tieoff descriptor lifetime much longer,
avoiding repeating coherent buffer allocation on each open/close cycle.

Main benefit: we dissociate its lifetime from the main ring's lifetime.
That way there is less work to be doing on resources (re)alloc. This
currently happens on close/open, but will soon also happen on context
swap operations (set_ringparam, change_mtu, set_channels, etc).

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 75 
+++++++++++++++++---------------
 1 file changed, 41 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index 8b52122bc134..951a7f080225 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
quoted
[...]
quoted
 static void macb_init_rings(struct macb *bp)
@@ -2832,8 +2801,6 @@ static void macb_init_rings(struct macb *bp)
 	bp->queues[0].tx_head = 0;
 	bp->queues[0].tx_tail = 0;
 	desc->ctrl |= MACB_BIT(TX_WRAP);
-
-	macb_init_tieoff(bp);
 }

 static void macb_reset_hw(struct macb *bp)
@@ -5518,6 +5485,38 @@ static int eyeq5_init(struct platform_device 
*pdev)
 	return ret;
 }

+static int macb_alloc_tieoff(struct macb *bp)
+{
+	/* Tieoff is a workaround in case HW cannot disable queues, for PM. 
*/
+	if (bp->caps & MACB_CAPS_QUEUE_DISABLE)
+		return 0;
Before, the tieoff was allocated in macb_alloc(), which the at91ether 
path
never called. Now it's allocated from macb_probe() for all variants, 
gated only
on MACB_CAPS_QUEUE_DISABLE, so EMAC gets a coherent descriptor it never 
uses.

Add MACB_CAPS_MACB_IS_EMAC to the if statement?
Clearly. That EMAC distinction keeps being annoying.
Well well well. Under AT91 with WOL active there is nothing preventing
the tieoff from being used in macb_suspend(). Meaning the tieoff is
being used zero-initialised. Not advisable.

So I withdraw my previous comment: we won't shield macb_alloc_tieoff()
from IS_EMAC and maybe it will fix a bug.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help