Re: [PATCH v2 0/5] can: flexcan: Add NXP S32N79 SoC support

[Marc Kleine-Budde]

On 19.03.2026 16:14:46, Ciprian Marian Costea wrote:
> > Please also have a look at the AI review:
> >
> > https://sashiko.dev/#/patchset/20260318092215.23505-1-ciprianmarian.costea%40oss.nxp.com
> >
> > Especially on patch#3.
> >
> > I think we should split the main IRQ handler into 3 parts, message buff,
> > bus error and state change.

> Thanks for pointing to the AI review.
>
> It raises two concerns:
>
> 1. Duplicate event processing (can be addressed by splitting the handler
> as you've suggested).
>
> This is a pre-existing issue affecting S32G2 (NR_IRQ_3 with 4 IRQ lines
> to the same handler) and MCF5441X (3 IRQ lines on the same handler).
> I'll include this as a preparatory patch in the next version of the series.

Thanks. Until the S32G2 was added multiple IRQ handlers was a niche
problem. But now it's relevant.

> 2. Concurrent skb_irq_queue access (pre-existing, separate scope)
>
> The __skb_queue_add_sort() calls on offload->skb_irq_queue are lockless.
> When the mb and esr handlers run concurrently on different CPUs, both
> can manipulate the list simultaneously.
> This is a valid concern, but it's also pre-existing.
>
> The fix requires changes in CAN core's rx-offload.c rather than in
> flexcan, so I think it would be better handled in a separate series.
>
> Would you agree ?

ACK

One option is to make struct can_rx_offload::skb_irq_queue per CPU.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachment: signature.asc
Description: PGP signature