patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	Frank Li <Frank.Li@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Sasha Levin <sashal@kernel.org>,
	Shyam-sundar.S-k@amd.com, Guruvendra.Punugupati@amd.com,
	billy_tsai@aspeedtech.com
Subject: [PATCH AUTOSEL 6.14 4/8] i3c: mipi-i3c-hci: Fix handling status of i3c_hci_irq_handler()
Date: Wed,  4 Jun 2025 07:49:39 -0400	[thread overview]
Message-ID: <20250604114944.208828-4-sashal@kernel.org> (raw)
In-Reply-To: <20250604114944.208828-1-sashal@kernel.org>

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.com
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;
 }
-- 
2.39.5


  parent reply	other threads:[~2025-06-04 11:49 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-04 11:49 [PATCH AUTOSEL 6.14 1/8] fbcon: Make sure modelist not set on unregistered console Sasha Levin
2025-06-04 11:49 ` [PATCH AUTOSEL 6.14 2/8] watchdog: da9052_wdt: respect TWDMIN Sasha Levin
2025-06-04 11:49 ` [PATCH AUTOSEL 6.14 3/8] watchdog: stm32: Fix wakeup source leaks on device unbind Sasha Levin
2025-06-04 11:49 ` Sasha Levin [this message]
2025-06-04 11:49 ` [PATCH AUTOSEL 6.14 5/8] bus: fsl-mc: increase MC_CMD_COMPLETION_TIMEOUT_MS value Sasha Levin
2025-06-04 11:49 ` [PATCH AUTOSEL 6.14 6/8] ARM: OMAP2+: Fix l4ls clk domain handling in STANDBY Sasha Levin
2025-06-04 11:49 ` [PATCH AUTOSEL 6.14 7/8] tee: Prevent size calculation wraparound on 32-bit kernels Sasha Levin
2025-06-04 11:49 ` [PATCH AUTOSEL 6.14 8/8] Revert "bus: ti-sysc: Probe for l4_wkup and l4_cfg interconnect devices first" Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250604114944.208828-4-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=Frank.Li@nxp.com \
    --cc=Guruvendra.Punugupati@amd.com \
    --cc=Shyam-sundar.S-k@amd.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=billy_tsai@aspeedtech.com \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).