[PATCH AUTOSEL 6.19-5.10] mailbox: bcm-ferxrm-mailbox: Use default primary handler

[PATCH AUTOSEL 6.19-5.10] mailbox: bcm-ferxrm-mailbox: Use default primary handler

[Sasha Levin]

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

[ Upstream commit 03843d95a4a4e0ba22ad4fcda65ccf21822b104c ]

request_threaded_irq() is invoked with a primary and a secondary handler
and no flags are passed. The primary handler is the same as
irq_default_primary_handler() so there is no need to have an identical
copy.

The lack of the IRQF_ONESHOT flag can be dangerous because the interrupt
source is not masked while the threaded handler is active. This means,
especially on LEVEL typed interrupt lines, the interrupt can fire again
before the threaded handler had a chance to run.

Use the default primary interrupt handler by specifying NULL and set
IRQF_ONESHOT so the interrupt source is masked until the secondary handler
is done.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Link: https://patch.msgid.link/20260128095540.863589-5-bigeasy@linutronix.de
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Complete Analysis

### 1. COMMIT MESSAGE ANALYSIS

The subject says "Use default primary handler" which sounds like a
cleanup, but the commit body reveals a real bug:

> "The lack of the IRQF_ONESHOT flag can be dangerous because the
interrupt source is not masked while the threaded handler is active.
This means, especially on LEVEL typed interrupt lines, the interrupt can
fire again before the threaded handler had a chance to run."

The authors are Sebastian Andrzej Siewior (bigeasy, a core IRQ/RT
developer) and Thomas Gleixner (the IRQ subsystem maintainer and co-
author of the `__setup_irq` protection). This is coming from the very
people who understand threaded IRQ semantics best.

### 2. CODE CHANGE ANALYSIS

The change is small and surgical — two distinct modifications:

**A) Remove the redundant `flexrm_irq_event` primary handler:**

```1176:1182:drivers/mailbox/bcm-flexrm-mailbox.c
static irqreturn_t flexrm_irq_event(int irq, void *dev_id)
{
        /* We only have MSI for completions so just wakeup IRQ thread */
        /* Ring related errors will be informed via completion
descriptors */

        return IRQ_WAKE_THREAD;
}
```

This function is **identical** in behavior to
`irq_default_primary_handler()` in `kernel/irq/manage.c`:

```976:979:kernel/irq/manage.c
static irqreturn_t irq_default_primary_handler(int irq, void *dev_id)
{
        return IRQ_WAKE_THREAD;
}
```

Both simply return `IRQ_WAKE_THREAD`. There's zero functional
difference.

**B) Change the `request_threaded_irq()` call:**

Old code:
```
request_threaded_irq(ring->irq, flexrm_irq_event, flexrm_irq_thread, 0,
...)
```

New code:
```
request_threaded_irq(ring->irq, NULL, flexrm_irq_thread, IRQF_ONESHOT,
...)
```

### 3. THE BUG MECHANISM

This is a real bug with two dimensions:

**Dimension 1: Missing IRQF_ONESHOT on non-ONESHOT_SAFE interrupts**

The bcm-flexrm-mailbox driver uses **platform MSI** (via
`platform_device_msi_init_and_alloc_irqs()`), NOT PCI MSI. I verified
that while PCI MSI irqchips have `IRQCHIP_ONESHOT_SAFE` set (in
`drivers/pci/msi/irqdomain.c`), platform MSI does NOT. This means the
IRQ subsystem's safety net — automatically stripping `IRQF_ONESHOT` for
chips that don't need it — does not apply here.

Without `IRQF_ONESHOT`, the interrupt line is **not masked** while the
threaded handler (`flexrm_irq_thread`) runs. On a **level-triggered**
interrupt line, this creates an interrupt storm:
1. Interrupt fires → primary handler returns `IRQ_WAKE_THREAD`
2. Interrupt line is re-enabled immediately (no masking)
3. Device still has the line asserted → interrupt fires again
   immediately
4. Goto 1 — the thread never gets to run, the system is stuck in hard
   IRQ context

The commit message explicitly describes this: "especially on LEVEL typed
interrupt lines, the interrupt can fire again before the threaded
handler had a chance to run."

**Dimension 2: Forced threading bypass**

The old code provided an explicit primary handler (`flexrm_irq_event`),
even though it's functionally identical to
`irq_default_primary_handler`. This is problematic because
`irq_setup_forced_threading()` has a special check:

```1302:1303:kernel/irq/manage.c
        if (new->handler == irq_default_primary_handler)
                return 0;
```

When `handler != irq_default_primary_handler` (i.e., it's the driver's
custom `flexrm_irq_event`), forced threading proceeds and creates a
**secondary action** with the original thread handler, and converts the
primary handler to run in a thread too. This is wasteful and changes the
behavior on PREEMPT_RT kernels — instead of a simple wake-and-handle
flow, it creates an unnecessary secondary handler chain. But more
critically, with the old code and forced threading, the check at line
1295:

```1295:1296:kernel/irq/manage.c
        if (new->flags & (IRQF_NO_THREAD | IRQF_PERCPU | IRQF_ONESHOT))
                return 0;
```

Since flags=0 (no IRQF_ONESHOT), forced threading continues and sets
`IRQF_ONESHOT` at line 1305. But without forced threading (normal
kernels), IRQF_ONESHOT is never set, and the interrupt runs without
masking.

**Why the old code doesn't hit the __setup_irq safety check:**

The `__setup_irq` code at line 1664-1684 rejects `handler==NULL` without
`IRQF_ONESHOT` by checking `new->handler ==
irq_default_primary_handler`. But since the old code passes
`flexrm_irq_event` (a different function pointer that does the same
thing), this safety check is **bypassed**. The driver sneaks past the
protection that Thomas Gleixner himself added in commit 1c6c69525b40e
("genirq: Reject bogus threaded irq requests").

### 4. CLASSIFICATION

This is a **bug fix** — specifically fixing a potential interrupt storm
/ system hang on level-triggered interrupt configurations. It's
disguised as cleanup but addresses a real correctness issue.

### 5. SCOPE AND RISK ASSESSMENT

- **Lines changed:** ~10 lines removed, ~2 lines changed — extremely
  small
- **Files touched:** 1 file (`drivers/mailbox/bcm-flexrm-mailbox.c`)
- **Risk:** Very low. The change is:
  - Removing dead code (a function identical to
    `irq_default_primary_handler`)
  - Passing `NULL` + `IRQF_ONESHOT` instead of a custom handler +
    flags=0
  - This is the canonical correct way to request a threaded IRQ with no
    real primary handler
- **Could it break something?** No. The behavior with `IRQF_ONESHOT` is
  strictly safer — the interrupt is masked during threaded handler
  execution. The primary handler behavior is identical
  (`IRQ_WAKE_THREAD`).

### 6. USER IMPACT

- This driver is used on Broadcom iProc SoCs (embedded ARM) for FlexRM
  offload engine mailbox operations
- The bug manifests as an **interrupt storm causing system hang** on
  level-triggered interrupt configurations
- Even on edge-triggered (MSI) configurations, the missing
  `IRQF_ONESHOT` creates a window where the interrupt can re-fire before
  the thread handler runs, potentially causing lost completions or
  spurious interrupt warnings

### 7. STABILITY INDICATORS

- **Author:** Sebastian Andrzej Siewior — a core kernel developer,
  especially for PREEMPT_RT and IRQ subsystem
- **Acked by:** Thomas Gleixner — the IRQ subsystem maintainer and
  creator of the `__setup_irq` safety checks
- The fix follows a well-established pattern used across many drivers
- The patch is self-contained with no dependencies

### 8. DEPENDENCY CHECK

The code being modified exists in all stable trees that have this
driver. The driver `bcm-flexrm-mailbox.c` has been in the kernel since
at least v4.14. The `request_threaded_irq()` with `NULL` +
`IRQF_ONESHOT` pattern has been supported since the genirq safety check
was added in 2012 (commit 1c6c69525b40e). This patch applies cleanly to
any stable tree.

### CONCLUSION

This commit fixes a real bug: a missing `IRQF_ONESHOT` flag that can
cause an interrupt storm and system hang on level-triggered interrupt
lines. The old code also inadvertently bypassed the kernel's own safety
check for this exact scenario (by providing a custom handler identical
to the default one). The fix is small (net -10 lines), self-contained,
authored by core IRQ subsystem developers, and follows the canonical
pattern for threaded interrupts. It has zero risk of regression —
`IRQF_ONESHOT` is strictly correct and the removed handler was
functionally identical to the default.

**YES**

 drivers/mailbox/bcm-flexrm-mailbox.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/mailbox/bcm-flexrm-mailbox.c b/drivers/mailbox/bcm-flexrm-mailbox.c
index 41f79e51d9e5a..4255fefc3a5a0 100644
--- a/drivers/mailbox/bcm-flexrm-mailbox.c
+++ b/drivers/mailbox/bcm-flexrm-mailbox.c
@@ -1173,14 +1173,6 @@ static int flexrm_debugfs_stats_show(struct seq_file *file, void *offset)
 
 /* ====== FlexRM interrupt handler ===== */
 
-static irqreturn_t flexrm_irq_event(int irq, void *dev_id)
-{
-	/* We only have MSI for completions so just wakeup IRQ thread */
-	/* Ring related errors will be informed via completion descriptors */
-
-	return IRQ_WAKE_THREAD;
-}
-
 static irqreturn_t flexrm_irq_thread(int irq, void *dev_id)
 {
 	flexrm_process_completions(dev_id);
@@ -1271,10 +1263,8 @@ static int flexrm_startup(struct mbox_chan *chan)
 		ret = -ENODEV;
 		goto fail_free_cmpl_memory;
 	}
-	ret = request_threaded_irq(ring->irq,
-				   flexrm_irq_event,
-				   flexrm_irq_thread,
-				   0, dev_name(ring->mbox->dev), ring);
+	ret = request_threaded_irq(ring->irq, NULL, flexrm_irq_thread,
+				   IRQF_ONESHOT, dev_name(ring->mbox->dev), ring);
 	if (ret) {
 		dev_err(ring->mbox->dev,
 			"failed to request ring%d IRQ\n", ring->num);
-- 
2.51.0

[PATCH AUTOSEL 6.19-5.10] mailbox: bcm-ferxrm-mailbox: Use default primary handler

[Sasha Levin]

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

[ Upstream commit fa84883d44422208b45869a67c0265234fdce1f0 ]

request_threaded_irq() is invoked with a primary and a secondary handler
and no flags are passed. The primary handler is the same as
irq_default_primary_handler() so there is no need to have an identical
copy.
The lack of the IRQF_ONESHOT can be dangerous because the interrupt
source is not masked while the threaded handler is active. This means,
especially on LEVEL typed interrupt lines, the interrupt can fire again
before the threaded handler had a chance to run.

Use the default primary interrupt handler by specifying NULL and set
IRQF_ONESHOT so the interrupt source is masked until the secondary
handler is done.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Both commits are the same patch. The one being evaluated (fa84883d44422,
signed by Jassi Brar) is the one from the mailbox tree.

## Summary of Analysis

### What the commit fixes

The commit fixes a missing `IRQF_ONESHOT` flag in
`request_threaded_irq()` for the BCM FlexRM mailbox driver. The existing
code had a custom primary handler (`flexrm_irq_event`) that was
functionally identical to the kernel's `irq_default_primary_handler()` —
it simply returned `IRQ_WAKE_THREAD`. However, because the code didn't
set `IRQF_ONESHOT`, the interrupt line is not masked while the threaded
handler runs. On level-triggered interrupt lines, this can cause an
interrupt storm (the interrupt fires repeatedly before the threaded
handler gets to run).

### Why it matters

1. **Correctness**: Without `IRQF_ONESHOT`, if the interrupt is level-
   triggered (or chained through a level-triggered parent), the
   interrupt will continuously fire between the primary handler
   returning and the threaded handler completing. This is a potential
   interrupt storm / soft lockup.

2. **Author credibility**: Sebastian Siewior is the PREEMPT_RT
   maintainer and a leading expert on interrupt handling in Linux.
   Thomas Gleixner, the IRQ subsystem maintainer, also signed off on
   this patch via the tip tree.

3. **Pattern**: This is part of a systematic series fixing the same
   pattern across multiple drivers (efct, btintel_pcie, fsl-mc, etc.),
   all from the same author.

### Stable kernel criteria

- **Obviously correct**: Yes - removes an exact duplicate of
  `irq_default_primary_handler()` and adds the missing `IRQF_ONESHOT`
  flag. This is the canonical pattern for threaded IRQs.
- **Fixes a real bug**: Yes - missing `IRQF_ONESHOT` can cause interrupt
  storms on level-triggered lines.
- **Small and contained**: Yes - 1 file changed, 2 insertions, 12
  deletions. The change is strictly self-contained.
- **No new features**: Correct - this is purely a bug fix.
- **Risk**: Very low. The behavior is identical except that the
  interrupt line is now properly masked during threaded handler
  execution, which prevents the storm.

### Nuance: MSI vs Level-Triggered

The driver uses platform MSI interrupts for completions (the comment
says "We only have MSI for completions"). MSI interrupts are typically
edge-triggered and may not suffer from the re-assertion problem.
However:
1. Platform MSIs may be chained through parent controllers that ARE
   level-triggered.
2. The underlying IRQ chip may not have `IRQCHIP_ONESHOT_SAFE` set (only
   PCI MSI explicitly sets this).
3. Even if the risk is lower for MSI, adding `IRQF_ONESHOT` is the
   correct defensive practice and has zero downside.
4. The kernel enforcement check (`__setup_irq`) would actually reject
   the new code (handler=NULL without IRQF_ONESHOT) if the chip lacks
   `IRQCHIP_ONESHOT_SAFE`, so `IRQF_ONESHOT` is needed for the code to
   even work.

### Verification

- **Verified** `flexrm_irq_event()` returns only `IRQ_WAKE_THREAD` at
  line 1176-1182 of bcm-flexrm-mailbox.c, identical to
  `irq_default_primary_handler()` at kernel/irq/manage.c:976-979.
- **Verified** `request_threaded_irq()` with handler=NULL uses
  `irq_default_primary_handler` (kernel/irq/manage.c:2128-2132).
- **Verified** `__setup_irq()` requires IRQF_ONESHOT when
  handler==irq_default_primary_handler and chip lacks
  IRQCHIP_ONESHOT_SAFE (kernel/irq/manage.c:1664-1685).
- **Verified** platform MSI irq chips do NOT automatically set
  `IRQCHIP_ONESHOT_SAFE` - only PCI MSI does
  (drivers/pci/msi/irqdomain.c:133,187). No irqchip drivers set it
  either.
- **Verified** the driver has existed since v4.12 (commit
  dbc049eee7300), present in all active stable trees.
- **Verified** the fix is self-contained: only touches 1 file, no
  dependencies on other commits.
- **Verified** author Sebastian Siewior is the PREEMPT_RT maintainer
  with deep IRQ expertise, and this is part of a systematic series.
- **Verified** the commit also went through Thomas Gleixner's tip tree
  (commit 03843d95a4a4e) with his Signed-off-by, confirming IRQ
  subsystem maintainer review.

This is a small, safe, well-understood bug fix from a domain expert that
prevents a potential interrupt storm. It meets all stable kernel
criteria.

**YES**

 drivers/mailbox/bcm-flexrm-mailbox.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/mailbox/bcm-flexrm-mailbox.c b/drivers/mailbox/bcm-flexrm-mailbox.c
index 41f79e51d9e5a..4255fefc3a5a0 100644
--- a/drivers/mailbox/bcm-flexrm-mailbox.c
+++ b/drivers/mailbox/bcm-flexrm-mailbox.c
@@ -1173,14 +1173,6 @@ static int flexrm_debugfs_stats_show(struct seq_file *file, void *offset)
 
 /* ====== FlexRM interrupt handler ===== */
 
-static irqreturn_t flexrm_irq_event(int irq, void *dev_id)
-{
-	/* We only have MSI for completions so just wakeup IRQ thread */
-	/* Ring related errors will be informed via completion descriptors */
-
-	return IRQ_WAKE_THREAD;
-}
-
 static irqreturn_t flexrm_irq_thread(int irq, void *dev_id)
 {
 	flexrm_process_completions(dev_id);
@@ -1271,10 +1263,8 @@ static int flexrm_startup(struct mbox_chan *chan)
 		ret = -ENODEV;
 		goto fail_free_cmpl_memory;
 	}
-	ret = request_threaded_irq(ring->irq,
-				   flexrm_irq_event,
-				   flexrm_irq_thread,
-				   0, dev_name(ring->mbox->dev), ring);
+	ret = request_threaded_irq(ring->irq, NULL, flexrm_irq_thread,
+				   IRQF_ONESHOT, dev_name(ring->mbox->dev), ring);
 	if (ret) {
 		dev_err(ring->mbox->dev,
 			"failed to request ring%d IRQ\n", ring->num);
-- 
2.51.0

[PATCH AUTOSEL 6.19-6.1] mailbox: pcc: Remove spurious IRQF_ONESHOT usage

[Sasha Levin]

From: Mark Brown <broonie@kernel.org>

[ Upstream commit 673327028cd61db68a1e0c708be2e302c082adf9 ]

The PCC code currently specifies IRQF_ONESHOT if the interrupt could
potentially be shared but doesn't actually use request_threaded_irq() and
the interrupt handler does not use IRQ_WAKE_THREAD so IRQF_ONESHOT is
never relevant. Since commit aef30c8d569c ("genirq: Warn about using
IRQF_ONESHOT without a threaded handler") specifying it has resulted in a
WARN_ON(), fix this by removing IRQF_ONESHOT.

Reported-by: Aishwarya TCV <Aishwarya.TCV@arm.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Summary of Analysis

### What the commit fixes

The PCC mailbox driver (`drivers/mailbox/pcc.c`) incorrectly specifies
`IRQF_ONESHOT` when requesting a shared interrupt. `IRQF_ONESHOT` is
only meaningful for threaded interrupt handlers (registered via
`request_threaded_irq()`), but this driver uses `devm_request_irq()`
with a hardcoded primary handler (`pcc_mbox_irq`) and no thread
function.

Since commit `aef30c8d569c` (merged 2026-01-13 into v6.19 cycle), the
generic IRQ code now emits a `WARN_ON_ONCE()` when `IRQF_ONESHOT` is
specified without a threaded handler. This means on any system with a
PCC shared interrupt, the kernel produces a warning splat at
boot/runtime — a visible regression for users.

### Stable kernel criteria assessment

1. **Obviously correct and tested**: Yes — the change removes a flag
   that has no effect (since the driver doesn't use threaded IRQs).
   Reviewed by the SCMI/PCC co-maintainer Sudeep Holla.

2. **Fixes a real bug**: Yes — it fixes a `WARN_ON()` that fires at
   runtime. WARN_ONs are treated as bugs by many distributions and can
   be flagged by automated testing.

3. **Important issue**: Moderate — a WARN_ON at boot is visible to users
   and breaks CI/testing systems. On PREEMPT_RT systems, the underlying
   issue (IRQF_ONESHOT without threading) could also be problematic
   since the handler is exempt from forced-threading.

4. **Small and contained**: Yes — single line change, single file, no
   side effects.

5. **No new features**: Correct — this purely removes a spurious flag.

### Dependencies

This fix only makes sense if commit `aef30c8d569c` ("genirq: Warn about
using IRQF_ONESHOT without a threaded handler") is also present in the
stable tree. That commit was merged in the v6.19 cycle (January 2026).
If `aef30c8d569c` is NOT backported to stable, then the `IRQF_ONESHOT`
flag is harmless (no WARN_ON fires, and the flag is simply ignored).
However, the `IRQF_ONESHOT` was always incorrect — removing it is safe
regardless.

The PCC shared interrupt support was added by commit `3db174e478cb0b` in
September 2023 (v6.7 cycle), so this code exists in stable trees 6.6.y
and later.

### Risk assessment

**Risk: Extremely low.** Removing an unused flag from an IRQ
registration call cannot break anything. The flag was never functional
since the driver doesn't use threaded IRQs.

**Benefit: Eliminates WARN_ON** splat on systems with shared PCC
interrupts, which affects ACPI-based ARM64 and x86 platforms using PCC
for CPPC/SCMI.

### Verification

- **Verified**: Commit `aef30c8d569c` exists in tree and adds
  `WARN_ON_ONCE(new->flags & IRQF_ONESHOT && !new->thread_fn)` in
  `kernel/irq/manage.c` (dated 2026-01-13).
- **Verified**: `drivers/mailbox/pcc.c` uses `devm_request_irq()` (line
  556), NOT `request_threaded_irq()`.
- **Verified**: No `IRQ_WAKE_THREAD` or `request_threaded_irq` usage in
  `pcc.c` (grep returned no matches).
- **Verified**: The `IRQF_ONESHOT` was introduced by commit
  `3db174e478cb0b` ("mailbox: pcc: Support shared interrupt for multiple
  subspaces", 2023-09-11), which is in the v6.7 cycle, making it present
  in 6.6.y stable and later.
- **Verified**: The change is a single flag removal (`IRQF_SHARED |
  IRQF_ONESHOT` → `IRQF_SHARED`), one line modified.
- **Verified**: Reviewed by Sudeep Holla (PCC/SCMI co-maintainer),
  reported by an ARM engineer.

### Conclusion

This is a straightforward fix for a WARN_ON triggered at runtime. The
change is minimal (removing one unused flag), obviously correct
(verified by code inspection that no threaded handler exists), reviewed
by a domain expert, and has zero risk of regression. It meets all stable
kernel criteria.

The only nuance is that the WARN_ON itself comes from a very recent
commit (`aef30c8d569c`), which may or may not be present in all stable
trees. However, even without that commit, removing the spurious
`IRQF_ONESHOT` is correct — the flag was never needed and was always a
no-op in this context. Removing incorrect flags proactively prevents
future issues and is good practice.

**YES**

 drivers/mailbox/pcc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index ff292b9e0be9e..060489e5ae6de 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -552,7 +552,7 @@ static int pcc_startup(struct mbox_chan *chan)
 
 	if (pchan->plat_irq > 0) {
 		irqflags = pcc_chan_plat_irq_can_be_shared(pchan) ?
-						IRQF_SHARED | IRQF_ONESHOT : 0;
+						IRQF_SHARED : 0;
 		rc = devm_request_irq(chan->mbox->dev, pchan->plat_irq, pcc_mbox_irq,
 				      irqflags, MBOX_IRQ_NAME, chan);
 		if (unlikely(rc)) {
-- 
2.51.0