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

[Théo Lebrun]

On Thu Jul 2, 2026 at 12:54 PM CEST, Nicolai Buchwitz wrote:
> On 1.7.2026 17:59, 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() 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@xxxxxxxxxxx>
>> ---
>>  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
>
>> [...]
>
>>  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.

Thanks,

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