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

[Marc Kleine-Budde]

On 24.03.2026 14:30:50, Ciprian Marian Costea wrote:
> > 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   |
>
> 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.

Maybe you can come up with a improved/better solution.

With the potential concurrent mailbox IRQ handlers on the s32g we have a
problem with the rx-offload infrastructure. After converting it to
per-CPU lists, we still have to merge the lists to the global one.

The current supported use case is a single IRQ handler that adds all
received CAN frames (and CAN error frames) sorted to the skb_irq_queue
list. At the end of the IRQ handler the list is appended to the
skb_queue and NAPI pushes the CAN frames into the networking stack.

This is done to preserve the order of CAN frames, which is crucial for
some CAN protocols.

If we have 2 IRQ handlers adding the per-cpu skb_irq_queue to the
skb_queue might lead to out-of-order reception.

I see 2 options to workaround this:
- Lock the skb_queue and sort each packet into it.
  This makes the lock longer and
  if an IRQ is delayed old packets might already been processed, so
  out-of-order reception is still possible.
- Disable mailboxes 1...8 for the s32g and only use the 2nd IRQ handler.
  This obviously reduces the number of RX mailboxes, this increases the
  change of RX buffer overflows on systems under high load.
  On the other hand the driver can be extended to use the mailboxes
  64...127.

As I don't have a s32g SoC nor a customer using it, I have no time to
implement this. Maybe you or someone from NXP is willing to do this.

> 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?

There are better ways to do stats, but IMHO for now we can live with the
problem.

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   |

Attachment: signature.asc
Description: PGP signature