Re: [PATCH net] net: macb: take bp->lock around NCR read-modify-writes

[Théo Lebrun]

Hello Nicolai,

On Fri Sep 18, 2026 at 10:31 PM CEST, Nicolai Buchwitz wrote:
> On 18.9.2026 21:59, Théo Lebrun wrote:
>> NCR is read-modify-written from many contexts:
>> 
>>  - macb_mac_link_down() clears RE|TE,
>>  - macb_mac_link_up() sets RE|TE|PTPUNI,
>>  - macb_hresp_error_task() clears then re-sets RE|TE,
>>  - macb_start_xmit() / macb_tx_restart() / macb_tx_error_task() set
>>    TSTART (already under bp->lock),
>>  - macb_interrupt() might toggle RE (also under bp->lock).
>> 
>> The first three risk concurrent RMW with anyone from the list as they
>> don't grab bp->lock.
>
> There is a fourth, in at91ether_interrupt():
>
>       if (intstatus & MACB_BIT(RXUBR)) {
>               ctl = macb_readl(bp, NCR);
>               macb_writel(bp, NCR, ctl & ~MACB_BIT(RE));
>               wmb();
>               macb_writel(bp, NCR, ctl | MACB_BIT(RE));
>       }
>
> I guess you have skipped this as the emac variants using this are not 
> SMP?

Actually I skipped over this at91ether one because at91ether functions
use zero locking. But as you point out, yes, RMWs are safe from
interrupt context on a non-SMP platform.

> Reviewed-by: Nicolai Buchwitz <nb@xxxxxxxxxxx>

Thanks!

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