[PATCH AUTOSEL 6.19-5.15] mailbox: sprd: mask interrupts that are not handled

Sasha Levin posted 1 patch 1 month, 2 weeks ago
drivers/mailbox/sprd-mailbox.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
[PATCH AUTOSEL 6.19-5.15] mailbox: sprd: mask interrupts that are not handled
Posted by Sasha Levin 1 month, 2 weeks ago
From: Otto Pflüger <otto.pflueger@abscue.de>

[ Upstream commit 75df94d05fc03fd9d861eaf79ce10fbb7a548bd8 ]

To reduce the amount of spurious interrupts, disable the interrupts that
are not handled in this driver.

Signed-off-by: Otto Pflüger <otto.pflueger@abscue.de>
Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Comprehensive Analysis

### 1. What the Commit Does

This commit changes the interrupt mask initialization in
`sprd_mbox_startup()` for the Spreadtrum mailbox driver. The two key
changes are:

**a) Deterministic initialization instead of read-modify-write:**
- **Before:** Read the hardware's current IRQ mask register state, then
  clear specific bits to unmask desired interrupts. This left other bits
  in whatever state the hardware/bootloader had them.
- **After:** Start from the full mask constant (all interrupts masked),
  then unmask only the ones the driver actually handles. This is fully
  deterministic.

**b) Stop enabling the inbox overflow interrupt:**
- **Before:** Both `SPRD_INBOX_FIFO_OVERFLOW_IRQ` (BIT(1)) and
  `SPRD_INBOX_FIFO_DELIVER_IRQ` (BIT(2)) were unmasked. The driver only
  handles delivery interrupts.
- **After:** Only `SPRD_INBOX_FIFO_DELIVER_IRQ` is unmasked.

### 2. Bug Being Fixed

The `sprd_mbox_inbox_isr()` only checks the delivery status bits
(`SPRD_INBOX_FIFO_DELIVER_MASK`). If no delivery is pending, it returns
`IRQ_NONE` with a "spurious inbox interrupt" warning. This means:

- If the inbox overflow interrupt fires without a concurrent delivery
  event, the ISR returns `IRQ_NONE`
- Repeated `IRQ_NONE` returns trigger the kernel's spurious interrupt
  detection in `note_interrupt()` (`kernel/irq/spurious.c`), which can
  eventually **disable the entire IRQ line** with a "nobody cared"
  message
- If the inbox delivery interrupt shares the same IRQ line, disabling
  the IRQ would **break all mailbox communication**

Similarly, the old code left outbox bits 1-4 and inbox bit 0 in an
indeterminate state dependent on hardware power-on defaults, potentially
enabling additional interrupts the driver doesn't handle.

### 3. Stable Kernel Criteria Assessment

- **Obviously correct:** Yes. The change is straightforward - mask
  everything, then unmask only what the driver handles. This matches the
  `sprd_mbox_shutdown()` function which already uses
  `SPRD_INBOX_FIFO_IRQ_MASK` and `SPRD_OUTBOX_FIFO_IRQ_MASK` to mask all
  interrupts.
- **Fixes a real bug:** Yes. Spurious interrupts can lead to the kernel
  disabling the IRQ line ("nobody cared"), which would break mailbox
  functionality entirely.
- **Small and contained:** Yes. 4 insertions, 6 deletions in a single
  file, affecting one function.
- **No new features:** Correct. This only changes interrupt masking
  behavior.
- **Self-contained:** Yes. No dependency on the revision 2 commit or the
  delivery flag commit. The startup function code is unchanged between
  v6.19 and the state before this patch.

### 4. Risk Assessment

- **Risk:** Very low. The change makes the interrupt state deterministic
  and only enables interrupts the driver actually handles. The shutdown
  function already uses the same mask constants.
- **Regression potential:** Minimal. The only functional change is that
  the inbox overflow interrupt is no longer enabled - but since the ISR
  never handled it, enabling it was always a bug.
- **Scope:** Single driver (sprd-mailbox), single function, 10 lines
  changed.

### 5. Backport Applicability

The driver exists since v5.8. The buggy code in `sprd_mbox_startup()`
has been present since the driver was added (`ca27fc26cd221`). The patch
should apply cleanly to all stable trees that include this driver
(5.10.y, 5.15.y, 6.1.y, 6.6.y, etc.) as the startup function has never
been modified between the initial driver addition and this fix.

### Verification

- **Verified** that `sprd_mbox_inbox_isr()` does NOT handle overflow
  interrupts - it only checks `SPRD_INBOX_FIFO_DELIVER_MASK` and returns
  `IRQ_NONE` if no delivery is pending (lines 153-193 of the v6.19
  source).
- **Verified** that `sprd_mbox_shutdown()` already uses
  `SPRD_INBOX_FIFO_IRQ_MASK` and `SPRD_OUTBOX_FIFO_IRQ_MASK` to disable
  all interrupts (lines 269-284), confirming the mask constants are the
  correct "all masked" values.
- **Verified** the startup function is unchanged between v6.19 (HEAD)
  and the state just before this patch on master (identical code at
  lines 236-268).
- **Verified** the revision 2 commit (`c6ff944003cf1`) does NOT modify
  `sprd_mbox_startup()`, confirming this patch has no dependency on it.
- **Verified** the delivery flag commit (`c77661d60d422`) modifies
  `sprd_mbox_inbox_isr()` only, not `sprd_mbox_startup()`, confirming
  independence.
- **Verified** the driver was added in v5.8-rc1 (`ca27fc26cd221`), so it
  exists in all currently maintained stable trees (5.10+).
- **Verified** via lore.kernel.org that no reviewer concerns or NAKs
  were raised about this patch.
- **Verified** that `SPRD_INBOX_FIFO_IRQ_MASK = GENMASK(2,0) = 0x7` and
  `SPRD_OUTBOX_FIFO_IRQ_MASK = GENMASK(4,0) = 0x1F` from the source
  definitions (lines 44-52).
- **Could NOT verify** specific user reports of the "nobody cared" IRQ
  being triggered in practice (no Reported-by tag, no bug report link).
  However, the code analysis clearly shows this is possible if the
  overflow interrupt fires without a concurrent delivery.

### Summary

This is a small, self-contained bug fix that prevents spurious
interrupts from potentially disabling the entire IRQ line for the
Spreadtrum mailbox driver. The fix is obviously correct (mask what you
don't handle), has minimal risk, applies cleanly to all stable trees,
and prevents a real failure mode (kernel disabling the shared IRQ). It
meets all stable kernel criteria.

**YES**

 drivers/mailbox/sprd-mailbox.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mailbox/sprd-mailbox.c b/drivers/mailbox/sprd-mailbox.c
index ee8539dfcef54..c1a5fe6cc8771 100644
--- a/drivers/mailbox/sprd-mailbox.c
+++ b/drivers/mailbox/sprd-mailbox.c
@@ -243,21 +243,19 @@ static int sprd_mbox_startup(struct mbox_chan *chan)
 		/* Select outbox FIFO mode and reset the outbox FIFO status */
 		writel(0x0, priv->outbox_base + SPRD_MBOX_FIFO_RST);
 
-		/* Enable inbox FIFO overflow and delivery interrupt */
-		val = readl(priv->inbox_base + SPRD_MBOX_IRQ_MSK);
-		val &= ~(SPRD_INBOX_FIFO_OVERFLOW_IRQ | SPRD_INBOX_FIFO_DELIVER_IRQ);
+		/* Enable inbox FIFO delivery interrupt */
+		val = SPRD_INBOX_FIFO_IRQ_MASK;
+		val &= ~SPRD_INBOX_FIFO_DELIVER_IRQ;
 		writel(val, priv->inbox_base + SPRD_MBOX_IRQ_MSK);
 
 		/* Enable outbox FIFO not empty interrupt */
-		val = readl(priv->outbox_base + SPRD_MBOX_IRQ_MSK);
+		val = SPRD_OUTBOX_FIFO_IRQ_MASK;
 		val &= ~SPRD_OUTBOX_FIFO_NOT_EMPTY_IRQ;
 		writel(val, priv->outbox_base + SPRD_MBOX_IRQ_MSK);
 
 		/* Enable supplementary outbox as the fundamental one */
 		if (priv->supp_base) {
 			writel(0x0, priv->supp_base + SPRD_MBOX_FIFO_RST);
-			val = readl(priv->supp_base + SPRD_MBOX_IRQ_MSK);
-			val &= ~SPRD_OUTBOX_FIFO_NOT_EMPTY_IRQ;
 			writel(val, priv->supp_base + SPRD_MBOX_IRQ_MSK);
 		}
 	}
-- 
2.51.0