Re: [PATCH net-next 05/11] net: macb: allocate tieoff descriptor once across device lifetime

["Théo Lebrun" <theo.lebrun@bootlin.com>]

On Thu Apr 2, 2026 at 1:14 PM CEST, Nicolai Buchwitz wrote:
> On 1.4.2026 18:39, Théo Lebrun wrote:
>> 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_consistent() at open. Free is done by
>> macb_free_consistent() 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 | 70 
>> ++++++++++++++++----------------
>
>> [...]
>
>> 
>> +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;
>> +
>> +	bp->rx_ring_tieoff = dma_alloc_coherent(&bp->pdev->dev,
>> +						macb_dma_desc_get_size(bp),
>> +						&bp->rx_ring_tieoff_dma,
>> +						GFP_KERNEL);
>> +	if (!bp->rx_ring_tieoff)
>> +		return -ENOMEM;
>> +
>> +	return 0;
>> +}
>
> The old macb_init_tieoff() that wrote WRAP+USED into the
> descriptor is deleted but its work is not replicated here.
> dma_alloc_coherent zeroes the memory, so RX_USED=0 and the
> hardware will treat it as a valid receive buffer pointing to
> DMA address 0 during suspend.
>
> Shouldn't this have a macb_set_addr() + ctrl=0 after the
> allocation?

Clearly! This V1 uses tieoff uninitialised. The two instructions from
old macb_init_tieoff() have been appended to macb_alloc_tieoff().

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;

	bp->rx_ring_tieoff = dma_alloc_coherent(&bp->pdev->dev,
						macb_dma_desc_get_size(bp),
						&bp->rx_ring_tieoff_dma,
						GFP_KERNEL);
	if (!bp->rx_ring_tieoff)
		return -ENOMEM;

	macb_set_addr(bp, bp->rx_ring_tieoff,
		      MACB_BIT(RX_WRAP) | MACB_BIT(RX_USED));

	bp->rx_ring_tieoff->ctrl = 0;

	return 0;
}

Thanks,

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