Re: [PATCH RESEND v4 1/8] can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms
From: Enric Balletbo i Serra
Date: Wed Jun 03 2026 - 06:06:38 EST
Hi Ciprian,
Thank you for the patch series.
On Wed, Jun 3, 2026 at 9:14 AM Ciprian Costea
<ciprianmarian.costea@xxxxxxxxxxx> wrote:
>
> From: Ciprian Marian Costea <ciprianmarian.costea@xxxxxxxxxxx>
>
> 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.
>
> Fixes: d9cead75b1c6 ("can: flexcan: add mcf5441x support")
> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@xxxxxxxxxxx>
Adding here because it doesn't seem to have a cover letter. With the
full series applied on top of the current mainline
Tested-by: Enric Balletbo i Serra <eballetb@xxxxxxxxxxx>
Tested on the NXP S32G399A-RDB3 with loopback and high-rate traffic.
No regressions observed: CAN frames transmit and receive correctly,
with no duplicates.
Frame reception showed no errors under stress testing.
Thanks,
Enric
> ---
> drivers/net/can/flexcan/flexcan-core.c | 128 +++++++++++++++++++++----
> 1 file changed, 111 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index f5d22c61503f..f73ff442d530 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -1070,16 +1070,14 @@ static struct sk_buff *flexcan_mailbox_read(struct can_rx_offload *offload,
> return skb;
> }
>
> -static irqreturn_t flexcan_irq(int irq, void *dev_id)
> +/* Process mailbox (RX + TX) events */
> +static irqreturn_t flexcan_do_mb(struct net_device *dev)
> {
> - struct net_device *dev = dev_id;
> struct net_device_stats *stats = &dev->stats;
> struct flexcan_priv *priv = netdev_priv(dev);
> struct flexcan_regs __iomem *regs = priv->regs;
> irqreturn_t handled = IRQ_NONE;
> u64 reg_iflag_tx;
> - u32 reg_esr;
> - enum can_state last_state = priv->can.state;
>
> /* reception interrupt */
> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_USE_RX_MAILBOX) {
> @@ -1131,25 +1129,57 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> netif_wake_queue(dev);
> }
>
> + return handled;
> +}
> +
> +/* Process bus error events */
> +static irqreturn_t flexcan_do_berr(struct net_device *dev)
> +{
> + struct flexcan_priv *priv = netdev_priv(dev);
> + struct flexcan_regs __iomem *regs = priv->regs;
> + irqreturn_t handled = IRQ_NONE;
> + u32 reg_esr;
> +
> reg_esr = priv->read(®s->esr);
>
> - /* ACK all bus error, state change and wake IRQ sources */
> - if (reg_esr & (FLEXCAN_ESR_ALL_INT | FLEXCAN_ESR_WAK_INT)) {
> + /* ACK bus error interrupt source */
> + if (reg_esr & FLEXCAN_ESR_ERR_INT) {
> handled = IRQ_HANDLED;
> - priv->write(reg_esr & (FLEXCAN_ESR_ALL_INT | FLEXCAN_ESR_WAK_INT), ®s->esr);
> + priv->write(FLEXCAN_ESR_ERR_INT, ®s->esr);
> }
>
> - /* state change interrupt or broken error state quirk fix is enabled */
> - if ((reg_esr & FLEXCAN_ESR_ERR_STATE) ||
> - (priv->devtype_data.quirks & (FLEXCAN_QUIRK_BROKEN_WERR_STATE |
> - FLEXCAN_QUIRK_BROKEN_PERR_STATE)))
> - flexcan_irq_state(dev, reg_esr);
> -
> /* bus error IRQ - handle if bus error reporting is activated */
> if ((reg_esr & FLEXCAN_ESR_ERR_BUS) &&
> (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
> flexcan_irq_bus_err(dev, reg_esr);
>
> + return handled;
> +}
> +
> +/* Process device state change events */
> +static irqreturn_t flexcan_do_state(struct net_device *dev)
> +{
> + struct flexcan_priv *priv = netdev_priv(dev);
> + struct flexcan_regs __iomem *regs = priv->regs;
> + irqreturn_t handled = IRQ_NONE;
> + u32 reg_esr;
> + enum can_state last_state = priv->can.state;
> +
> + reg_esr = priv->read(®s->esr);
> +
> + /* ACK state change and wake IRQ sources */
> + if (reg_esr & (FLEXCAN_ESR_ERR_STATE | FLEXCAN_ESR_WAK_INT)) {
> + handled = IRQ_HANDLED;
> + priv->write(reg_esr & (FLEXCAN_ESR_ERR_STATE | FLEXCAN_ESR_WAK_INT),
> + ®s->esr);
> + }
> +
> + /* state change interrupt or broken error state quirk fix is enabled */
> + if ((reg_esr & FLEXCAN_ESR_ERR_STATE) ||
> + (priv->devtype_data.quirks &
> + (FLEXCAN_QUIRK_BROKEN_WERR_STATE | FLEXCAN_QUIRK_BROKEN_PERR_STATE)))
> + flexcan_irq_state(dev, reg_esr);
> +
> /* availability of error interrupt among state transitions in case
> * bus error reporting is de-activated and
> * FLEXCAN_QUIRK_BROKEN_PERR_STATE is enabled:
> @@ -1188,6 +1218,65 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> }
> }
>
> + return handled;
> +}
> +
> +/* Combined IRQ handler for single-IRQ platforms */
> +static irqreturn_t flexcan_irq(int irq, void *dev_id)
> +{
> + struct net_device *dev = dev_id;
> + struct flexcan_priv *priv = netdev_priv(dev);
> + irqreturn_t handled;
> +
> + handled = flexcan_do_mb(dev);
> + handled |= flexcan_do_state(dev);
> + handled |= flexcan_do_berr(dev);
> +
> + if (handled)
> + can_rx_offload_irq_finish(&priv->offload);
> +
> + return handled;
> +}
> +
> +/* Mailbox IRQ handler for multi-IRQ platforms */
> +static irqreturn_t flexcan_irq_mb(int irq, void *dev_id)
> +{
> + struct net_device *dev = dev_id;
> + struct flexcan_priv *priv = netdev_priv(dev);
> + irqreturn_t handled;
> +
> + handled = flexcan_do_mb(dev);
> +
> + if (handled)
> + can_rx_offload_irq_finish(&priv->offload);
> +
> + return handled;
> +}
> +
> +/* Bus error IRQ handler for multi-IRQ platforms */
> +static irqreturn_t flexcan_irq_berr(int irq, void *dev_id)
> +{
> + struct net_device *dev = dev_id;
> + struct flexcan_priv *priv = netdev_priv(dev);
> + irqreturn_t handled;
> +
> + handled = flexcan_do_berr(dev);
> +
> + if (handled)
> + can_rx_offload_irq_finish(&priv->offload);
> +
> + return handled;
> +}
> +
> +/* Device state change IRQ handler for multi-IRQ platforms */
> +static irqreturn_t flexcan_irq_boff(int irq, void *dev_id)
> +{
> + struct net_device *dev = dev_id;
> + struct flexcan_priv *priv = netdev_priv(dev);
> + irqreturn_t handled;
> +
> + handled = flexcan_do_state(dev);
> +
> if (handled)
> can_rx_offload_irq_finish(&priv->offload);
>
> @@ -1761,25 +1850,30 @@ static int flexcan_open(struct net_device *dev)
>
> can_rx_offload_enable(&priv->offload);
>
> - err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev);
> + if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3)
> + err = request_irq(dev->irq, flexcan_irq_mb,
> + IRQF_SHARED, dev->name, dev);
> + else
> + err = request_irq(dev->irq, flexcan_irq,
> + IRQF_SHARED, dev->name, dev);
> if (err)
> goto out_can_rx_offload_disable;
>
> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
> err = request_irq(priv->irq_boff,
> - flexcan_irq, IRQF_SHARED, dev->name, dev);
> + flexcan_irq_boff, IRQF_SHARED, dev->name, dev);
> if (err)
> goto out_free_irq;
>
> err = request_irq(priv->irq_err,
> - flexcan_irq, IRQF_SHARED, dev->name, dev);
> + flexcan_irq_berr, IRQF_SHARED, dev->name, dev);
> if (err)
> goto out_free_irq_boff;
> }
>
> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ) {
> err = request_irq(priv->irq_secondary_mb,
> - flexcan_irq, IRQF_SHARED, dev->name, dev);
> + flexcan_irq_mb, IRQF_SHARED, dev->name, dev);
> if (err)
> goto out_free_irq_err;
> }
> --
> 2.43.0
>