Re: [PATCH net-next 10/11] net: macb: use context swapping in .set_ringparam()

Théo Lebrun
2026-04-02T16:34:50Z

On Wed Apr 1, 2026 at 10:17 PM CEST, Maxime Chevallier wrote:
> On 01/04/2026 18:39, Théo Lebrun wrote:
>> ethtool_ops.set_ringparam() is implemented using the primitive close /
>> update ring size / reopen sequence. Under memory pressure this does not
>> fly: we free our buffers at close and cannot reallocate new ones at
>> open. Also, it triggers a slow PHY reinit.
>>
>> Instead, exploit the new context mechanism and improve our sequence to:
>> - allocate a new context (including buffers) first
>> - if it fails, early return without any impact to the interface
>> - stop interface
>> - update global state (bp, netdev, etc)
>> - pass buffer pointers to the hardware
>> - start interface
>> - free old context.
>>
>> The HW disable sequence is inspired by macb_reset_hw() but avoids
>> (1) setting NCR bit CLRSTAT and (2) clearing register PBUFRXCUT.
>>
>> The HW re-enable sequence is inspired by macb_mac_link_up(), skipping
>> over register writes which would be redundant (because values have not
>> changed).
>>
>> The generic context swapping parts are isolated into helper functions
>> macb_context_swap_start|end(), reusable by other operations (change_mtu,
>> set_channels, etc).
>>
>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>> ---
>> drivers/net/ethernet/cadence/macb_main.c | 89 +++++++++++++++++++++++++++++---
>> 1 file changed, 82 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index 42b19b969f3e..543356554c11 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -2905,6 +2905,76 @@ static struct macb_context *macb_context_alloc(struct macb *bp,
>> return ctx;
>> }
>>
>> +static void macb_context_swap_start(struct macb *bp)
>> +{
>> + struct macb_queue *queue;
>> + unsigned int q;
>> + u32 ctrl;
>> +
>> + /* Disable software Tx, disable HW Tx/Rx and disable NAPI. */
>> +
>> + netif_tx_disable(bp->netdev);
>> +
>> + ctrl = macb_readl(bp, NCR);
>> + macb_writel(bp, NCR, ctrl & ~(MACB_BIT(RE) | MACB_BIT(TE)));
>> +
>> + macb_writel(bp, TSR, -1);
>> + macb_writel(bp, RSR, -1);
>> +
>> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
>> + queue_writel(queue, IDR, -1);
>> + queue_readl(queue, ISR);
>> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
>> + queue_writel(queue, ISR, -1);
>> + }
>
> These registers appear to be protected by bp->lock, any chance that this
> may race with an interrupt in the middle of them being configured here ?

The topic is complex! I dug deep this afternoon and replied to the
neighbour thread by Nicolai. It might be of interest to you.

https://lore.kernel.org/netdev/90f843aa3940bdbabadddce27314c1f1@tipi-net.de/
(will appear as a child to this email, it hasn't been indexed yet)

Thanks Maxime,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering