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

Théo Lebrun
2026-04-02T13:57:49Z

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