* [PATCH v6 1/6] can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms
2026-07-13 8:53 [PATCH v6 0/6] can: flexcan: Add NXP S32N79 SoC support Ciprian Costea
@ 2026-07-13 8:53 ` Ciprian Costea
2026-07-13 9:07 ` sashiko-bot
2026-07-13 8:53 ` [PATCH v6 2/6] can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable() Ciprian Costea
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Ciprian Costea @ 2026-07-13 8:53 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Angelo Dureghello
Cc: linux-can, devicetree, linux-kernel, imx, NXP S32 Linux Team,
Christophe Lizzi, Alberto Ruiz, Enric Balletbo, Eric Chanudet,
Ciprian Marian Costea, Haibo Chen
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.
Fixes: d9cead75b1c6 ("can: flexcan: add mcf5441x support")
Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Reviewed-and-tested-by: Haibo Chen <haibo.chen@nxp.com>
Tested-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v6 1/6] can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms
2026-07-13 8:53 ` [PATCH v6 1/6] can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms Ciprian Costea
@ 2026-07-13 9:07 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-07-13 9:07 UTC (permalink / raw)
To: Ciprian Costea
Cc: Marc Kleine-Budde, devicetree, Frank.Li, imx, linux-can, robh,
conor+dt, Oliver Hartkopp, Vincent Mailhol, Oleksij Rempel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Splitting the IRQ handler breaks the FLEXCAN_QUIRK_BROKEN_PERR_STATE workaround on multi-IRQ platforms.
Pre-existing issues:
- [Critical] Concurrent lockless modifications of `skb_irq_queue` by multiple IRQ handlers causes list corruption.
- [High] Incomplete fix for duplicate frame processing on platforms with secondary mailbox IRQs.
--
--- Patch [1]: [PATCH v6 1/6] can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms ---
commit b6f6f5d373c333a28a06c2ed84ea386f9dc5b122
Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms
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@oss.nxp.com>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Reviewed-and-tested-by: Haibo Chen <haibo.chen@nxp.com>
Tested-by: Enric Balletbo i Serra <eballetb@redhat.com>
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index f5d22c61503fa..f73ff442d5304 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -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);
[Severity: High]
Does isolating mailbox handling into flexcan_irq_mb() break the
FLEXCAN_QUIRK_BROKEN_PERR_STATE workaround?
On multi-IRQ platforms with this quirk (such as MCF5441X and S32G2), the
hardware fails to trigger a state change interrupt, meaning flexcan_irq_boff()
won't fire. The baseline driver worked around this by polling the state via
flexcan_do_state() during every mailbox interrupt in the monolithic handler.
Since flexcan_irq_mb() completely bypasses flexcan_do_state(), could this cause
the driver to silently miss CAN error state transitions (e.g., Error Passive)
when the error rate increases?
> +
> + 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);
[Severity: Critical]
This is a pre-existing issue, but does having dedicated IRQ handlers
running concurrently on different CPUs cause a race condition in the rx offload
framework?
If flexcan_irq_mb() and flexcan_irq_berr() fire simultaneously, both will
eventually call into the CAN rx offload framework (e.g., via
can_rx_offload_queue_timestamp()):
drivers/net/can/dev/rx-offload.c:can_rx_offload_queue_timestamp() {
...
__skb_queue_add_sort(&offload->skb_irq_queue, skb,
can_rx_offload_compare);
...
}
Since __skb_queue_add_sort() locklessly modifies skb_irq_queue, could
concurrent execution of these handlers corrupt the sk_buff linked lists?
> +
> + 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)
> +{
[ ... ]
> @@ -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 (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);
[Severity: High]
This isn't a bug introduced by this patch, but does registering the same
flexcan_irq_mb() handler for both primary and secondary MB IRQ lines leave the
duplicate frame processing bug unfixed on platforms with secondary mailboxes?
If both IRQ lines fire concurrently on S32G2, flexcan_irq_mb() is dispatched to
both CPUs. Since flexcan_do_mb() doesn't check which IRQ line triggered it,
won't both CPUs unconditionally process all mailboxes, resulting in every
frame being processed twice?
> if (err)
> goto out_free_irq_err;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713085306.2643794-1-ciprianmarian.costea@oss.nxp.com?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 2/6] can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable()
2026-07-13 8:53 [PATCH v6 0/6] can: flexcan: Add NXP S32N79 SoC support Ciprian Costea
2026-07-13 8:53 ` [PATCH v6 1/6] can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms Ciprian Costea
@ 2026-07-13 8:53 ` Ciprian Costea
2026-07-13 9:07 ` sashiko-bot
2026-07-13 8:53 ` [PATCH v6 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line Ciprian Costea
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Ciprian Costea @ 2026-07-13 8:53 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Angelo Dureghello
Cc: linux-can, devicetree, linux-kernel, imx, NXP S32 Linux Team,
Christophe Lizzi, Alberto Ruiz, Enric Balletbo, Eric Chanudet,
Ciprian Marian Costea, Haibo Chen
From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
flexcan_chip_interrupts_enable() disables only the primary IRQ line while
writing to the IMASK and CTRL registers.
On multi-IRQ platforms (S32G2, MCF5441X), the additional IRQ lines (boff,
err, secondary-mb) remain active so their handlers can fire while
registers are inconsistent.
Disable all registered IRQ lines around the IMASK/CTRL writes. This
also fixes the resume path, which calls this function.
Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Reviewed-and-tested-by: Haibo Chen <haibo.chen@nxp.com>
Tested-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
drivers/net/can/flexcan/flexcan-core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index f73ff442d530..7dde2e623def 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -1519,14 +1519,28 @@ static void flexcan_chip_interrupts_enable(const struct net_device *dev)
{
const struct flexcan_priv *priv = netdev_priv(dev);
struct flexcan_regs __iomem *regs = priv->regs;
+ u32 quirks = priv->devtype_data.quirks;
u64 reg_imask;
disable_irq(dev->irq);
+ if (quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
+ disable_irq(priv->irq_boff);
+ disable_irq(priv->irq_err);
+ }
+ if (quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
+ disable_irq(priv->irq_secondary_mb);
+
priv->write(priv->reg_ctrl_default, ®s->ctrl);
reg_imask = priv->rx_mask | priv->tx_mask;
priv->write(upper_32_bits(reg_imask), ®s->imask2);
priv->write(lower_32_bits(reg_imask), ®s->imask1);
enable_irq(dev->irq);
+ if (quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
+ enable_irq(priv->irq_secondary_mb);
+ if (quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
+ enable_irq(priv->irq_boff);
+ enable_irq(priv->irq_err);
+ }
}
static void flexcan_chip_interrupts_disable(const struct net_device *dev)
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v6 2/6] can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable()
2026-07-13 8:53 ` [PATCH v6 2/6] can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable() Ciprian Costea
@ 2026-07-13 9:07 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-07-13 9:07 UTC (permalink / raw)
To: Ciprian Costea
Cc: linux-can, imx, robh, Frank.Li, Marc Kleine-Budde,
Vincent Mailhol, conor+dt, Oleksij Rempel, Oliver Hartkopp,
devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] On multi-IRQ platforms, Wake-on-CAN is broken because `flexcan_suspend` only arms the primary IRQ (`dev->irq`) for wakeup, completely ignoring the state/BOFF interrupt (`priv->irq_boff`) which actually handles the wakeup event.
- [High] In `flexcan_resume`, the network queue is restarted (`netif_device_attach` and `netif_start_queue`) before the hardware is initialized (`flexcan_chip_start`). This creates a race condition where a packet can be queued for transmission just before the hardware undergoes a soft reset, leading to a permanent TX stall.
--
--- Patch [2]: [PATCH v6 2/6] can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable() ---
commit ca7ea75d45df1ecf13f11dcec0bd19377eccf3c6
Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable()
flexcan_chip_interrupts_enable() disables only the primary IRQ line while
writing to the IMASK and CTRL registers. On multi-IRQ platforms, the
additional IRQ lines remain active so their handlers can fire while registers
are inconsistent. This commit disables all registered IRQ lines around the
IMASK/CTRL writes.
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index f73ff442d5304..7dde2e623defe 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -1519,14 +1519,28 @@ static void flexcan_chip_interrupts_enable(const struct net_device *dev)
> {
> const struct flexcan_priv *priv = netdev_priv(dev);
> struct flexcan_regs __iomem *regs = priv->regs;
> + u32 quirks = priv->devtype_data.quirks;
> u64 reg_imask;
>
> disable_irq(dev->irq);
> + if (quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
> + disable_irq(priv->irq_boff);
[Severity: High]
This is a pre-existing issue, but on multi-IRQ platforms, is Wake-on-CAN
broken because we only arm the primary IRQ during suspend?
In drivers/net/can/flexcan/flexcan-core.c:flexcan_suspend(), we see:
if (device_may_wakeup(device)) {
enable_irq_wake(dev->irq);
err = flexcan_enter_stop_mode(priv);
Since the hardware asserts irq_boff (which handles FLEXCAN_ESR_WAK_INT) for
wakeup events, doesn't irq_boff need to be armed with enable_irq_wake() as
well? Otherwise, the system might fail to wake from suspend since the
interrupt controller won't treat the asserted IRQ line as a wakeup source.
[Severity: High]
This is also a pre-existing issue, but does flexcan_resume() restart the
network queue prematurely?
In drivers/net/can/flexcan/flexcan-core.c:flexcan_resume(), the queue is
restarted before hardware initialization:
if (netif_running(dev)) {
netif_device_attach(dev);
netif_start_queue(dev);
if (device_may_wakeup(device)) {
...
} else {
...
err = flexcan_chip_start(dev);
If netif_start_queue() is called before flexcan_chip_start() reinitializes
the hardware, could the networking stack concurrently call ndo_start_xmit
(flexcan_start_xmit) and queue a packet?
If so, would the subsequent flexcan_chip_start() soft reset wipe out the
freshly populated message buffers, aborting the transmission and leading to a
permanent TX stall since the completion interrupt will never fire?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713085306.2643794-1-ciprianmarian.costea@oss.nxp.com?part=2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line
2026-07-13 8:53 [PATCH v6 0/6] can: flexcan: Add NXP S32N79 SoC support Ciprian Costea
2026-07-13 8:53 ` [PATCH v6 1/6] can: flexcan: use dedicated IRQ handlers for multi-IRQ platforms Ciprian Costea
2026-07-13 8:53 ` [PATCH v6 2/6] can: flexcan: disable all IRQ lines in flexcan_chip_interrupts_enable() Ciprian Costea
@ 2026-07-13 8:53 ` Ciprian Costea
2026-07-13 9:08 ` sashiko-bot
2026-07-13 8:53 ` [PATCH v6 4/6] dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support Ciprian Costea
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Ciprian Costea @ 2026-07-13 8:53 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Angelo Dureghello
Cc: linux-can, devicetree, linux-kernel, imx, NXP S32 Linux Team,
Christophe Lizzi, Alberto Ruiz, Enric Balletbo, Eric Chanudet,
Ciprian Marian Costea, Haibo Chen
From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
On S32G2, which has two mailbox IRQ lines (mb-0 for MBs 0-7, mb-1
for MBs 8-63), both handlers currently process the full rx_mask/tx_mask
range.
Introduce FLEXCAN_SECONDARY_MB_IRQ_MB0_MASK and
FLEXCAN_SECONDARY_MB_IRQ_MB1_MASK to describe the split, and pass
the selected mask to flexcan_do_mb() via a new mb_mask parameter.
In flexcan_irq_mb(), the irq argument selects the correct mask: the
primary MB IRQ uses MB0_MASK and the secondary uses MB1_MASK.
For single-IRQ platforms, mb_mask is ~0ULL with no functional change.
Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Reviewed-and-tested-by: Haibo Chen <haibo.chen@nxp.com>
Tested-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
drivers/net/can/flexcan/flexcan-core.c | 39 ++++++++++++++++++--------
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index 7dde2e623def..8b70952f7f05 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -182,6 +182,12 @@
#define FLEXCAN_IFLAG_RX_FIFO_WARN BIT(6)
#define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE BIT(5)
+/* On platforms with FLEXCAN_QUIRK_SECONDARY_MB_IRQ, the MB IRQ lines are
+ * split.
+ */
+#define FLEXCAN_SECONDARY_MB_IRQ_MB0_MASK GENMASK_U64(7, 0)
+#define FLEXCAN_SECONDARY_MB_IRQ_MB1_MASK GENMASK_U64(63, 8)
+
/* FLEXCAN message buffers */
#define FLEXCAN_MB_CODE_MASK (0xf << 24)
#define FLEXCAN_MB_CODE_RX_BUSY_BIT (0x1 << 24)
@@ -957,14 +963,16 @@ static inline void flexcan_write64(struct flexcan_priv *priv, u64 val, void __io
priv->write(lower_32_bits(val), addr);
}
-static inline u64 flexcan_read_reg_iflag_rx(struct flexcan_priv *priv)
+static inline u64 flexcan_read_reg_iflag_rx(struct flexcan_priv *priv,
+ u64 rx_mask)
{
- return flexcan_read64_mask(priv, &priv->regs->iflag1, priv->rx_mask);
+ return flexcan_read64_mask(priv, &priv->regs->iflag1, rx_mask);
}
-static inline u64 flexcan_read_reg_iflag_tx(struct flexcan_priv *priv)
+static inline u64 flexcan_read_reg_iflag_tx(struct flexcan_priv *priv,
+ u64 tx_mask)
{
- return flexcan_read64_mask(priv, &priv->regs->iflag1, priv->tx_mask);
+ return flexcan_read64_mask(priv, &priv->regs->iflag1, tx_mask);
}
static inline struct flexcan_priv *rx_offload_to_priv(struct can_rx_offload *offload)
@@ -1071,12 +1079,14 @@ static struct sk_buff *flexcan_mailbox_read(struct can_rx_offload *offload,
}
/* Process mailbox (RX + TX) events */
-static irqreturn_t flexcan_do_mb(struct net_device *dev)
+static irqreturn_t flexcan_do_mb(struct net_device *dev, u64 mb_mask)
{
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 rx_mask = priv->rx_mask & mb_mask;
+ u64 tx_mask = priv->tx_mask & mb_mask;
u64 reg_iflag_tx;
/* reception interrupt */
@@ -1084,7 +1094,8 @@ static irqreturn_t flexcan_do_mb(struct net_device *dev)
u64 reg_iflag_rx;
int ret;
- while ((reg_iflag_rx = flexcan_read_reg_iflag_rx(priv))) {
+ while ((reg_iflag_rx = flexcan_read_reg_iflag_rx(priv,
+ rx_mask))) {
handled = IRQ_HANDLED;
ret = can_rx_offload_irq_offload_timestamp(&priv->offload,
reg_iflag_rx);
@@ -1110,10 +1121,10 @@ static irqreturn_t flexcan_do_mb(struct net_device *dev)
}
}
- reg_iflag_tx = flexcan_read_reg_iflag_tx(priv);
+ reg_iflag_tx = flexcan_read_reg_iflag_tx(priv, tx_mask);
/* transmission complete interrupt */
- if (reg_iflag_tx & priv->tx_mask) {
+ if (reg_iflag_tx & tx_mask) {
u32 reg_ctrl = priv->read(&priv->tx_mb->can_ctrl);
handled = IRQ_HANDLED;
@@ -1125,7 +1136,7 @@ static irqreturn_t flexcan_do_mb(struct net_device *dev)
/* after sending a RTR frame MB is in RX mode */
priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
&priv->tx_mb->can_ctrl);
- flexcan_write64(priv, priv->tx_mask, ®s->iflag1);
+ flexcan_write64(priv, tx_mask, ®s->iflag1);
netif_wake_queue(dev);
}
@@ -1228,7 +1239,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
struct flexcan_priv *priv = netdev_priv(dev);
irqreturn_t handled;
- handled = flexcan_do_mb(dev);
+ handled = flexcan_do_mb(dev, ~0ULL);
handled |= flexcan_do_state(dev);
handled |= flexcan_do_berr(dev);
@@ -1244,8 +1255,14 @@ 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;
+ u64 mb_mask = ~0ULL;
+
+ if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
+ mb_mask = (irq == priv->irq_secondary_mb) ?
+ FLEXCAN_SECONDARY_MB_IRQ_MB1_MASK :
+ FLEXCAN_SECONDARY_MB_IRQ_MB0_MASK;
- handled = flexcan_do_mb(dev);
+ handled = flexcan_do_mb(dev, mb_mask);
if (handled)
can_rx_offload_irq_finish(&priv->offload);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v6 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line
2026-07-13 8:53 ` [PATCH v6 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line Ciprian Costea
@ 2026-07-13 9:08 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-07-13 9:08 UTC (permalink / raw)
To: Ciprian Costea
Cc: robh, conor+dt, Frank.Li, Oleksij Rempel, linux-can,
Marc Kleine-Budde, imx, Oliver Hartkopp, Vincent Mailhol,
devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent execution of multiple flexcan IRQ handlers causes lockless list corruption in the can_rx_offload queue.
--
--- Patch [3]: [PATCH v6 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line ---
commit 9f143e05fb5957d463ce1d05d4126d168798b0d8
Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
can: flexcan: split rx/tx masks per mailbox IRQ line
This commit updates flexcan to process split rx/tx masks based on the
mailbox IRQ line for platforms like S32G2. It introduces MB0 and MB1 masks
and passes them to flexcan_do_mb() to only process the relevant mailbox
range for each IRQ.
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
[ ... ]
> @@ -1244,8 +1255,14 @@ 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;
> + u64 mb_mask = ~0ULL;
> +
> + if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
> + mb_mask = (irq == priv->irq_secondary_mb) ?
> + FLEXCAN_SECONDARY_MB_IRQ_MB1_MASK :
> + FLEXCAN_SECONDARY_MB_IRQ_MB0_MASK;
>
> - handled = flexcan_do_mb(dev);
> + handled = flexcan_do_mb(dev, mb_mask);
>
> if (handled)
> can_rx_offload_irq_finish(&priv->offload);
[Severity: High]
This is a pre-existing issue, but does this concurrent execution risk
corrupting the lockless rx-offload queues?
Since the primary and secondary mailbox IRQs are separate lines (e.g., on
S32G2), they can be dispatched concurrently on different CPUs when frames
arrive simultaneously for both mailbox groups.
If both IRQs execute at the same time:
CPU1
flexcan_irq_mb()
flexcan_do_mb()
can_rx_offload_irq_offload_timestamp()
__skb_queue_add_sort(&offload->skb_irq_queue, ...)
CPU2
flexcan_irq_mb()
flexcan_do_mb()
can_rx_offload_irq_offload_timestamp()
__skb_queue_add_sort(&offload->skb_irq_queue, ...)
Or if CPU2 reaches the finish step while CPU1 is still adding:
CPU2
flexcan_irq_mb()
can_rx_offload_irq_finish()
skb_queue_splice_tail_init(&offload->skb_irq_queue, ...)
Since __skb_queue_add_sort() and skb_queue_splice_tail_init() operate
locklessly on offload->skb_irq_queue, could this lead to corrupted linked
list next/prev pointers or infinite loops during list traversal?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713085306.2643794-1-ciprianmarian.costea@oss.nxp.com?part=3
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 4/6] dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support
2026-07-13 8:53 [PATCH v6 0/6] can: flexcan: Add NXP S32N79 SoC support Ciprian Costea
` (2 preceding siblings ...)
2026-07-13 8:53 ` [PATCH v6 3/6] can: flexcan: split rx/tx masks per mailbox IRQ line Ciprian Costea
@ 2026-07-13 8:53 ` Ciprian Costea
2026-07-13 9:18 ` sashiko-bot
2026-07-13 8:53 ` [PATCH v6 5/6] can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk Ciprian Costea
2026-07-13 8:53 ` [PATCH v6 6/6] can: flexcan: add NXP S32N79 SoC support Ciprian Costea
5 siblings, 1 reply; 12+ messages in thread
From: Ciprian Costea @ 2026-07-13 8:53 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Angelo Dureghello
Cc: linux-can, devicetree, linux-kernel, imx, NXP S32 Linux Team,
Christophe Lizzi, Alberto Ruiz, Enric Balletbo, Eric Chanudet,
Ciprian Marian Costea, Andra-Teodora Ilie, Larisa Grigore,
Conor Dooley, Haibo Chen
From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Add NXP S32N79 SoC compatible string and interrupt properties.
On S32N79, FlexCAN IP is integrated with two interrupt lines:
one for the mailbox interrupts (0-127) and one for signaling
bus errors and device state changes.
Co-developed-by: Andra-Teodora Ilie <andra.ilie@nxp.com>
Signed-off-by: Andra-Teodora Ilie <andra.ilie@nxp.com>
Co-developed-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-and-tested-by: Haibo Chen <haibo.chen@nxp.com>
---
.../bindings/net/can/fsl,flexcan.yaml | 30 ++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
index f81d56f7c12a..d098a44c2b9c 100644
--- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
+++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
@@ -26,6 +26,7 @@ properties:
- fsl,ls1021ar2-flexcan
- fsl,lx2160ar1-flexcan
- nxp,s32g2-flexcan
+ - nxp,s32n79-flexcan
- items:
- enum:
- fsl,imx53-flexcan
@@ -173,11 +174,38 @@ allOf:
- const: mb-1
required:
- interrupt-names
- else:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: nxp,s32n79-flexcan
+ then:
+ properties:
+ interrupts:
+ items:
+ - description: Message Buffer interrupt for mailboxes 0-127
+ - description: Bus Error and Device state change interrupt
+ interrupt-names:
+ items:
+ - const: mb-0
+ - const: berr
+ required:
+ - interrupt-names
+
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nxp,s32g2-flexcan
+ - nxp,s32n79-flexcan
+ then:
properties:
interrupts:
maxItems: 1
interrupt-names: false
+
- if:
required:
- xceiver-supply
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v6 4/6] dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support
2026-07-13 8:53 ` [PATCH v6 4/6] dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support Ciprian Costea
@ 2026-07-13 9:18 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-07-13 9:18 UTC (permalink / raw)
To: Ciprian Costea
Cc: Vincent Mailhol, robh, devicetree, Frank.Li, linux-can,
Marc Kleine-Budde, Oleksij Rempel, imx, Oliver Hartkopp, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The flexcan driver fails to claim the newly added 'nxp,s32n79-flexcan' compatible string.
--
--- Patch [4]: [PATCH v6 4/6] dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support ---
commit 81a73119cf0179abcfa99d350e69491bfd8303b6
Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support
This commit adds the NXP S32N79 SoC compatible string and interrupt properties
to the FlexCAN device tree binding. On this platform, the IP uses separate
interrupt lines for mailboxes and bus errors.
> diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
> index f81d56f7c12a5..d098a44c2b9c2 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
> +++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
> @@ -26,6 +26,7 @@ properties:
> - fsl,ls1021ar2-flexcan
> - fsl,lx2160ar1-flexcan
> - nxp,s32g2-flexcan
> + - nxp,s32n79-flexcan
[Severity: Medium]
Does the flexcan driver need to be updated to claim this new compatible
string?
Because the binding does not provide a fallback compatible string, devices
using nxp,s32n79-flexcan will fail to probe unless flexcan_of_match in
drivers/net/can/flexcan/flexcan-core.c is also updated to include it. Is there
a companion patch planned to update the driver's match table?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713085306.2643794-1-ciprianmarian.costea@oss.nxp.com?part=4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 5/6] can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk
2026-07-13 8:53 [PATCH v6 0/6] can: flexcan: Add NXP S32N79 SoC support Ciprian Costea
` (3 preceding siblings ...)
2026-07-13 8:53 ` [PATCH v6 4/6] dt-bindings: can: fsl,flexcan: add NXP S32N79 SoC support Ciprian Costea
@ 2026-07-13 8:53 ` Ciprian Costea
2026-07-13 9:19 ` sashiko-bot
2026-07-13 8:53 ` [PATCH v6 6/6] can: flexcan: add NXP S32N79 SoC support Ciprian Costea
5 siblings, 1 reply; 12+ messages in thread
From: Ciprian Costea @ 2026-07-13 8:53 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Angelo Dureghello
Cc: linux-can, devicetree, linux-kernel, imx, NXP S32 Linux Team,
Christophe Lizzi, Alberto Ruiz, Enric Balletbo, Eric Chanudet,
Ciprian Marian Costea, Larisa Grigore, Haibo Chen
From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Introduce FLEXCAN_QUIRK_IRQ_BERR quirk to handle hardware integration
where the FlexCAN module has a dedicated interrupt line for signaling
bus errors and device state changes.
This adds the flexcan_irq_esr() handler which composes
flexcan_do_state() and flexcan_do_berr() to handle platforms where
these events share a single IRQ line.
Also extend flexcan_chip_interrupts_enable() to disable/enable the
new IRQ line during IMASK register writes.
This is required for NXP S32N79 SoC support.
Co-developed-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Reviewed-and-tested-by: Haibo Chen <haibo.chen@nxp.com>
Tested-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
drivers/net/can/flexcan/flexcan-core.c | 53 +++++++++++++++++++++-----
drivers/net/can/flexcan/flexcan.h | 2 +
2 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index 8b70952f7f05..e7e3fb41f55e 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -1300,6 +1300,22 @@ static irqreturn_t flexcan_irq_boff(int irq, void *dev_id)
return handled;
}
+/* Combined bus error and state change IRQ handler */
+static irqreturn_t flexcan_irq_esr(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);
+ handled |= flexcan_do_berr(dev);
+
+ if (handled)
+ can_rx_offload_irq_finish(&priv->offload);
+
+ return handled;
+}
+
static void flexcan_set_bittiming_ctrl(const struct net_device *dev)
{
const struct flexcan_priv *priv = netdev_priv(dev);
@@ -1540,10 +1556,10 @@ static void flexcan_chip_interrupts_enable(const struct net_device *dev)
u64 reg_imask;
disable_irq(dev->irq);
- if (quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
+ if (quirks & FLEXCAN_QUIRK_NR_IRQ_3)
disable_irq(priv->irq_boff);
+ if (quirks & (FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_IRQ_BERR))
disable_irq(priv->irq_err);
- }
if (quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
disable_irq(priv->irq_secondary_mb);
@@ -1554,10 +1570,10 @@ static void flexcan_chip_interrupts_enable(const struct net_device *dev)
enable_irq(dev->irq);
if (quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
enable_irq(priv->irq_secondary_mb);
- if (quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
- enable_irq(priv->irq_boff);
+ if (quirks & (FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_IRQ_BERR))
enable_irq(priv->irq_err);
- }
+ if (quirks & FLEXCAN_QUIRK_NR_IRQ_3)
+ enable_irq(priv->irq_boff);
}
static void flexcan_chip_interrupts_disable(const struct net_device *dev)
@@ -1881,7 +1897,8 @@ static int flexcan_open(struct net_device *dev)
can_rx_offload_enable(&priv->offload);
- if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3)
+ if (priv->devtype_data.quirks &
+ (FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_IRQ_BERR))
err = request_irq(dev->irq, flexcan_irq_mb,
IRQF_SHARED, dev->name, dev);
else
@@ -1902,6 +1919,13 @@ static int flexcan_open(struct net_device *dev)
goto out_free_irq_boff;
}
+ if (priv->devtype_data.quirks & FLEXCAN_QUIRK_IRQ_BERR) {
+ err = request_irq(priv->irq_err,
+ flexcan_irq_esr, 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_mb, IRQF_SHARED, dev->name, dev);
@@ -1916,7 +1940,8 @@ static int flexcan_open(struct net_device *dev)
return 0;
out_free_irq_err:
- if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3)
+ if (priv->devtype_data.quirks &
+ (FLEXCAN_QUIRK_IRQ_BERR | FLEXCAN_QUIRK_NR_IRQ_3))
free_irq(priv->irq_err, dev);
out_free_irq_boff:
if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3)
@@ -1948,10 +1973,12 @@ static int flexcan_close(struct net_device *dev)
if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ)
free_irq(priv->irq_secondary_mb, dev);
- if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
+ if (priv->devtype_data.quirks &
+ (FLEXCAN_QUIRK_IRQ_BERR | FLEXCAN_QUIRK_NR_IRQ_3))
free_irq(priv->irq_err, dev);
+
+ if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3)
free_irq(priv->irq_boff, dev);
- }
free_irq(dev->irq, dev);
can_rx_offload_disable(&priv->offload);
@@ -2338,6 +2365,14 @@ static int flexcan_probe(struct platform_device *pdev)
if (transceiver)
priv->can.bitrate_max = transceiver->attrs.max_link_rate;
+ if (priv->devtype_data.quirks & FLEXCAN_QUIRK_IRQ_BERR) {
+ priv->irq_err = platform_get_irq_byname(pdev, "berr");
+ if (priv->irq_err < 0) {
+ err = priv->irq_err;
+ goto failed_platform_get_irq;
+ }
+ }
+
if (priv->devtype_data.quirks & FLEXCAN_QUIRK_NR_IRQ_3) {
priv->irq_boff = platform_get_irq(pdev, 1);
if (priv->irq_boff < 0) {
diff --git a/drivers/net/can/flexcan/flexcan.h b/drivers/net/can/flexcan/flexcan.h
index 16692a2502eb..bbb1a8dd4777 100644
--- a/drivers/net/can/flexcan/flexcan.h
+++ b/drivers/net/can/flexcan/flexcan.h
@@ -74,6 +74,8 @@
* both need to have an interrupt handler registered.
*/
#define FLEXCAN_QUIRK_SECONDARY_MB_IRQ BIT(18)
+/* Setup dedicated bus error and state change IRQ */
+#define FLEXCAN_QUIRK_IRQ_BERR BIT(19)
struct flexcan_devtype_data {
u32 quirks; /* quirks needed for different IP cores */
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v6 5/6] can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk
2026-07-13 8:53 ` [PATCH v6 5/6] can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk Ciprian Costea
@ 2026-07-13 9:19 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-07-13 9:19 UTC (permalink / raw)
To: Ciprian Costea
Cc: Oleksij Rempel, robh, imx, Marc Kleine-Budde, Vincent Mailhol,
Oliver Hartkopp, linux-can, devicetree, conor+dt, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent execution of separate hardware interrupt handlers (`dev->irq` and `priv->irq_err`) modifies the lockless `can_rx_offload` IRQ queue (`skb_irq_queue`), leading to list corruption.
--
--- Patch [5]: [PATCH v6 5/6] can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk ---
commit 97d482f1edd202225930276cf268822cd49ea3d2
Author: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk
Introduce FLEXCAN_QUIRK_IRQ_BERR quirk to handle hardware integration
where the FlexCAN module has a dedicated interrupt line for signaling
bus errors and device state changes. This adds the flexcan_irq_esr() handler
and is required for NXP S32N79 SoC support.
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index 8b70952f7f05d..e7e3fb41f55e7 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -1300,6 +1300,22 @@ static irqreturn_t flexcan_irq_boff(int irq, void *dev_id)
> return handled;
> }
>
> +/* Combined bus error and state change IRQ handler */
> +static irqreturn_t flexcan_irq_esr(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);
> + handled |= flexcan_do_berr(dev);
> +
> + if (handled)
> + can_rx_offload_irq_finish(&priv->offload);
> +
> + return handled;
> +}
[Severity: High]
This is a pre-existing issue, but could this lead to list corruption
if these separate IRQs run concurrently on different CPUs?
Since flexcan_irq_esr() and flexcan_irq_mb() are registered as separate
interrupts, they might fire at the same time. If flexcan_irq_esr()
processes a bus error and flexcan_irq_mb() processes a mailbox interrupt,
they both end up modifying the lockless offload->skb_irq_queue.
CPU1 handling dev->irq (flexcan_irq_mb):
flexcan_irq_mb()
can_rx_offload_irq_offload_timestamp()
__skb_queue_add_sort(&offload->skb_irq_queue, ...)
CPU2 handling priv->irq_err (flexcan_irq_esr):
flexcan_irq_esr()
flexcan_do_berr()
flexcan_irq_bus_err()
can_rx_offload_queue_timestamp()
__skb_queue_add_sort(&offload->skb_irq_queue, ...)
Furthermore, both handlers unconditionally call can_rx_offload_irq_finish(),
which locklessly splices the skb_irq_queue list:
can_rx_offload_irq_finish()
skb_queue_splice_tail_init(&offload->skb_irq_queue, ...)
Are there existing synchronization mechanisms preventing these IRQ
handlers from running simultaneously and corrupting the lockless
skb_irq_queue list pointers?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713085306.2643794-1-ciprianmarian.costea@oss.nxp.com?part=5
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 6/6] can: flexcan: add NXP S32N79 SoC support
2026-07-13 8:53 [PATCH v6 0/6] can: flexcan: Add NXP S32N79 SoC support Ciprian Costea
` (4 preceding siblings ...)
2026-07-13 8:53 ` [PATCH v6 5/6] can: flexcan: add FLEXCAN_QUIRK_IRQ_BERR quirk Ciprian Costea
@ 2026-07-13 8:53 ` Ciprian Costea
5 siblings, 0 replies; 12+ messages in thread
From: Ciprian Costea @ 2026-07-13 8:53 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Angelo Dureghello
Cc: linux-can, devicetree, linux-kernel, imx, NXP S32 Linux Team,
Christophe Lizzi, Alberto Ruiz, Enric Balletbo, Eric Chanudet,
Ciprian Marian Costea, Andra-Teodora Ilie, Larisa Grigore,
Haibo Chen
From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Add device data and compatible string for NXP S32N79 SoC.
FlexCAN IP integration on S32N79 SoC uses two interrupts:
- one for mailboxes 0-127
- one for signaling bus errors and device state changes
Co-developed-by: Andra-Teodora Ilie <andra.ilie@nxp.com>
Signed-off-by: Andra-Teodora Ilie <andra.ilie@nxp.com>
Co-developed-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Reviewed-and-tested-by: Haibo Chen <haibo.chen@nxp.com>
Tested-by: Enric Balletbo i Serra <eballetb@redhat.com>
---
drivers/net/can/flexcan/flexcan-core.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index e7e3fb41f55e..54279b91cf5e 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -403,6 +403,15 @@ static const struct flexcan_devtype_data nxp_s32g2_devtype_data = {
FLEXCAN_QUIRK_SECONDARY_MB_IRQ,
};
+static const struct flexcan_devtype_data nxp_s32n_devtype_data = {
+ .quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
+ FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_RX_MAILBOX |
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SUPPORT_FD |
+ FLEXCAN_QUIRK_SUPPORT_ECC | FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
+ FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX_RTR |
+ FLEXCAN_QUIRK_IRQ_BERR,
+};
+
static const struct can_bittiming_const flexcan_bittiming_const = {
.name = DRV_NAME,
.tseg1_min = 4,
@@ -2222,6 +2231,7 @@ static const struct of_device_id flexcan_of_match[] = {
{ .compatible = "fsl,ls1021ar2-flexcan", .data = &fsl_ls1021a_r2_devtype_data, },
{ .compatible = "fsl,lx2160ar1-flexcan", .data = &fsl_lx2160a_r1_devtype_data, },
{ .compatible = "nxp,s32g2-flexcan", .data = &nxp_s32g2_devtype_data, },
+ { .compatible = "nxp,s32n79-flexcan", .data = &nxp_s32n_devtype_data, },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, flexcan_of_match);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread