From: Jarkko Nikula jarkko.nikula@linux.intel.com
[ Upstream commit 279c24021b838e76ca8441e9446e0ab45271153a ]
Return IRQ_HANDLED from the i3c_hci_irq_handler() only if some INTR_STATUS bit was set or if DMA/PIO handler handled it.
Currently it returns IRQ_HANDLED in case INTR_STATUS is zero and IO handler returns false. Which could be the case if interrupt comes from other device or is spurious.
Reviewed-by: Frank Li Frank.Li@nxp.com Signed-off-by: Jarkko Nikula jarkko.nikula@linux.intel.com Link: https://lore.kernel.org/r/20250409140401.299251-2-jarkko.nikula@linux.intel.... Signed-off-by: Alexandre Belloni alexandre.belloni@bootlin.com Signed-off-by: Sasha Levin sashal@kernel.org ---
Based on my comprehensive analysis of the commit, here is my assessment: **YES** This commit should be backported to stable kernel trees. Here's my extensive analysis: ## Code Change Analysis The commit fixes a critical logic bug in the `i3c_hci_irq_handler()` function in `drivers/i3c/master/mipi-i3c-hci/core.c`. The specific changes are: 1. **Line 597**: Added `result = IRQ_HANDLED;` when `val` (INTR_STATUS) is non-zero 2. **Line 608**: Changed from unconditional call to conditional: `if (hci->io->irq_handler(hci)) result = IRQ_HANDLED;` 3. **Lines 611-612**: Removed the `else result = IRQ_HANDLED;` logic that was incorrectly tied to having no unexpected status bits ## Why This Should Be Backported ### 1. **Fixes Important Functional Bug** The current code has incorrect interrupt handling logic that returns `IRQ_NONE` even when interrupts are successfully processed by the DMA/PIO handlers. This can lead to: - **Spurious interrupt detection issues**: The kernel may incorrectly think interrupts are unhandled - **Interrupt storm problems**: In shared IRQ scenarios, returning `IRQ_NONE` when interrupts were actually handled can cause the kernel to disable the IRQ line - **System stability issues**: Incorrect IRQ return values can affect overall system interrupt handling ### 2. **Small, Contained Fix** - The change is minimal (only 3 lines modified) - No architectural changes or new features - Only affects the interrupt return value logic - Low risk of introducing regressions ### 3. **Critical Subsystem Impact** The I3C subsystem is used for critical hardware communication, particularly in: - Embedded systems and IoT devices - Industrial applications - Hardware that uses MIPI I3C HCI controllers ### 4. **Follows Stable Backport Criteria** This matches the pattern of **Similar Commit #4** (IB/hfi1 interrupt handler fix) which was marked "Backport Status: YES" for fixing incorrect IRQ return values. ### 5. **Reviewed and Tested** The commit includes "Reviewed-by: Frank Li Frank.Li@nxp.com" indicating it has been properly reviewed by subsystem maintainers. ### 6. **Historical Context Shows This Is a Real Problem** From the git history, I can see this driver has had multiple interrupt-related fixes: - Commit 45357c9b37bb changed the interrupt handling logic per MIPI spec compliance - That change inadvertently introduced this return value bug - This commit fixes the logical error without changing the spec-compliant behavior ### 7. **Clear Problem and Solution** The commit message clearly explains: - **Problem**: "Return IRQ_HANDLED...only if some INTR_STATUS bit was set or if DMA/PIO handler handled it" - **Current bug**: "Currently it returns IRQ_HANDLED in case INTR_STATUS is zero and IO handler returns false" - **Impact**: "Which could be the case if interrupt comes from other device or is spurious" ## Risk Assessment **Minimal Risk**: The change only affects when `IRQ_HANDLED` vs `IRQ_NONE` is returned, without changing any of the actual interrupt processing logic. The worst case would be reverting to the previous (also incorrect but differently incorrect) behavior, but the new logic is clearly more correct. This is exactly the type of important bug fix that stable trees should include: it fixes a real functional problem, has minimal risk, and affects critical system functionality (interrupt handling).
drivers/i3c/master/mipi-i3c-hci/core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c index 648c501407ced..96f077945efae 100644 --- a/drivers/i3c/master/mipi-i3c-hci/core.c +++ b/drivers/i3c/master/mipi-i3c-hci/core.c @@ -596,6 +596,7 @@ static irqreturn_t i3c_hci_irq_handler(int irq, void *dev_id)
if (val) { reg_write(INTR_STATUS, val); + result = IRQ_HANDLED; }
if (val & INTR_HC_RESET_CANCEL) { @@ -607,12 +608,11 @@ static irqreturn_t i3c_hci_irq_handler(int irq, void *dev_id) val &= ~INTR_HC_INTERNAL_ERR; }
- hci->io->irq_handler(hci); + if (hci->io->irq_handler(hci)) + result = IRQ_HANDLED;
if (val) dev_err(&hci->master.dev, "unexpected INTR_STATUS %#x\n", val); - else - result = IRQ_HANDLED;
return result; }