Re: [PATCH v3 1/6] can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms

[Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>]

On 3/24/2026 1:56 PM, Marc Kleine-Budde wrote:
> On 23.03.2026 14:58:22, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>
>> On platforms with multiple IRQ lines (S32G2, MCF5441X), all lines are
>> registered to the same flexcan_irq() handler. Since these are distinct IRQ
>> numbers, they can be dispatched concurrently on different CPUs. Both
>> instances then read the same iflag and ESR registers unconditionally,
>> leading to duplicate frame processing.
>>
>> Fix this by splitting the monolithic handler into focused parts:
>> - flexcan_do_mb(): processes mailbox events
>> - flexcan_do_state(): processes device state change events
>> - flexcan_do_berr(): processes bus error events
>>
>> Introduce dedicated IRQ handlers for multi-IRQ platforms:
>> - flexcan_irq_mb(): mailbox-only, used for mb-0, mb-1 IRQ lines
>> - flexcan_irq_boff(): state-change-only, used for boff/state IRQ line
>> - flexcan_irq_berr(): bus-error-only, used for berr IRQ line
>>
>> The combined flexcan_irq() handler is preserved for single-IRQ
>> platforms with no functional change.
> 
> Thanks for implementing this.
> 
> Can you take care of the S32G2 which has 2 mailbox IRQs, too? Please in
> a separate patch.
> 
> My idea was to take the "irq" argument of the IRQ handler and the quirks
> and figure out if you are the first or second mailbox IRQ handler.
> 
> Convert these
> 
> | struct flexcan_priv {
> | [...]
> | 	u64 rx_mask;
> | 	u64 tx_mask;
> | [...]
> | }
> 
> into a struct and put an array of 2 of these structs into "struct
> flexcan_priv". Use correct mask array depending on IRQ handler.
> 
> regards,
> 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   |

Hello Marc,

Thanks for your review.
I'll add a separate patch implementing per-MB-IRQ mask handling (needed
by S32G2) in V4. And thanks for the implementation suggestion. I'll take
it into account.

Now, unrelated to the per-MB-IRQ refactor, one thing I noticed during 
the IRQ handlers split: dev->stats counters (e.g. rx_fifo_errors) can
be incremented concurrently from different IRQ handlers on different CPUs.

That said, these are just diagnostic counters and lost increments
should be benign. Do you think this warrants any synchronization/locking 
mechanism, or is the current behavior acceptable?

Regards,
Ciprian