[PATCH] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[PATCH] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Pei Xiao]

When the interrupt status register reads zero, the handler should
return IRQ_NONE instead of IRQ_HANDLED. What the return value
actually feeds into is the spurious interrupt accounting in
note_interrupt(): falsely claiming IRQ_HANDLED defeats the "irq XX:
nobody cared" detection, so a stuck interrupt source would never be
caught.

Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/i2c/busses/i2c-k1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
index c2d090f6ba80..487f23fde725 100644
--- a/drivers/i2c/busses/i2c-k1.c
+++ b/drivers/i2c/busses/i2c-k1.c
@@ -728,7 +728,7 @@ static irqreturn_t spacemit_i2c_irq_handler(int irq, void *devid)
 
 	status = readl(i2c->base + SPACEMIT_ISR);
 	if (!status)
-		return IRQ_HANDLED;
+		return IRQ_NONE;
 
 	i2c->status = status;
 
-- 
2.25.1

[PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Pei Xiao]

When the interrupt status register reads zero, the handler should
return IRQ_NONE instead of IRQ_HANDLED. What the return value
actually feeds into is the spurious interrupt accounting in
note_interrupt(): falsely claiming IRQ_HANDLED defeats the "irq XX:
nobody cared" detection, so a stuck interrupt source would never be
caught.

Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changlog in v2:
modify gitlog for Modify the incorrect Git commit message to match the reason for the modification.
---
 drivers/i2c/busses/i2c-k1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
index c2d090f6ba80..487f23fde725 100644
--- a/drivers/i2c/busses/i2c-k1.c
+++ b/drivers/i2c/busses/i2c-k1.c
@@ -728,7 +728,7 @@ static irqreturn_t spacemit_i2c_irq_handler(int irq, void *devid)
 
 	status = readl(i2c->base + SPACEMIT_ISR);
 	if (!status)
-		return IRQ_HANDLED;
+		return IRQ_NONE;
 
 	i2c->status = status;
 
-- 
2.25.1

Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Troy Mitchell]

On Fri Jul 10, 2026 at 12:21 AM PDT, Pei Xiao wrote:
> When the interrupt status register reads zero, the handler should
> return IRQ_NONE instead of IRQ_HANDLED. What the return value
> actually feeds into is the spurious interrupt accounting in
> note_interrupt(): falsely claiming IRQ_HANDLED defeats the "irq XX:
> nobody cared" detection, so a stuck interrupt source would never be
> caught.
>
> Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
The commit message now describes the actual mechanism correctly,
thanks for reworking it:

Reviewed-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>

One process note, though: please don't repost new versions this
quickly. v2 followed the review comments almost immediately, which
means other reviewers who were still looking at v1 never got a
chance to chime in before the thread moved on, and the discussion
gets fragmented across versions.

Documentation/process/submitting-patches.rst ("Don't get
discouraged - or impatient") asks submitters to wait a minimum of
one week before reposting; the same courtesy applies to new
versions while review is still ongoing. Collecting all the feedback
on one version and then sending a single respin saves everyone
time.

                                - Troy

Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Mukesh Savaliya]

On 7/10/2026 12:51 PM, Pei Xiao wrote:
> When the interrupt status register reads zero, the handler should
> return IRQ_NONE instead of IRQ_HANDLED. What the return value
> actually feeds into is the spurious interrupt accounting in
> note_interrupt(): falsely claiming IRQ_HANDLED defeats the "irq XX:
note_interrupt() ? You mean to say No interrupt OR invalid interrupt 
here ? I could not understand function name.
> nobody cared" detection, so a stuck interrupt source would never be
> caught.
> 
> Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> ---
> changlog in v2:
> modify gitlog for Modify the incorrect Git commit message to match the reason for the modification.
> ---
>   drivers/i2c/busses/i2c-k1.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
> index c2d090f6ba80..487f23fde725 100644
> --- a/drivers/i2c/busses/i2c-k1.c
> +++ b/drivers/i2c/busses/i2c-k1.c
> @@ -728,7 +728,7 @@ static irqreturn_t spacemit_i2c_irq_handler(int irq, void *devid)
>   
>   	status = readl(i2c->base + SPACEMIT_ISR);
>   	if (!status)
> -		return IRQ_HANDLED;
> +		return IRQ_NONE;
>   
>   	i2c->status = status;
>   

Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Pei Xiao]

在 2026/7/10 15:46, Troy Mitchell 写道:
> On Fri Jul 10, 2026 at 12:21 AM PDT, Pei Xiao wrote:
>> When the interrupt status register reads zero, the handler should
>> return IRQ_NONE instead of IRQ_HANDLED. What the return value
>> actually feeds into is the spurious interrupt accounting in
>> note_interrupt(): falsely claiming IRQ_HANDLED defeats the "irq XX:
>> nobody cared" detection, so a stuck interrupt source would never be
>> caught.
>>
>> Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> The commit message now describes the actual mechanism correctly,
> thanks for reworking it:
>
> Reviewed-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
>
> One process note, though: please don't repost new versions this
> quickly. v2 followed the review comments almost immediately, which
> means other reviewers who were still looking at v1 never got a
> chance to chime in before the thread moved on, and the discussion
> gets fragmented across versions.
Because I noticed a simple mistake right after sending v1
—I forgot to include the version number and changelog for v2—
so I thought that by fixing it without waiting for reviewers to point it
out, 
I could save reviewers' time and get the patch merged more quickly.
 I may have been wrong about that. Sorry.

Pei.
> Documentation/process/submitting-patches.rst ("Don't get
> discouraged - or impatient") asks submitters to wait a minimum of
> one week before reposting; the same courtesy applies to new
> versions while review is still ongoing. Collecting all the feedback
> on one version and then sending a single respin saves everyone
> time.
>
>                                 - Troy
>

Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Pei Xiao]

在 2026/7/10 16:50, Mukesh Savaliya 写道:
>
>
> On 7/10/2026 12:51 PM, Pei Xiao wrote:
>> When the interrupt status register reads zero, the handler should
>> return IRQ_NONE instead of IRQ_HANDLED. What the return value
>> actually feeds into is the spurious interrupt accounting in
>> note_interrupt(): falsely claiming IRQ_HANDLED defeats the "irq XX:
> note_interrupt() ? You mean to say No interrupt OR invalid interrupt
> here ? I could not understand function name. 
https://elixir.bootlin.com/linux/v7.2-rc1/source/kernel/irq/spurious.c#L222

if(unlikely
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/unlikely>(action_ret==IRQ_NONE
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/IRQ_NONE>)){ /* * If
we are seeing only the odd spurious IRQ caused by * bus asynchronicity
then don't eventually trigger an error, * otherwise the counter becomes
a doomsday timer for otherwise * working systems */ if(time_after
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/time_after>(jiffies
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/jiffies>,desc->last_unhandled
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/last_unhandled>+HZ
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/HZ>/10))
desc->irqs_unhandled
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/irqs_unhandled>=1;
else desc->irqs_unhandled
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/irqs_unhandled>++;
desc->last_unhandled
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/last_unhandled>=jiffies
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/jiffies>; } ...

if(unlikely
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/unlikely>(desc->irqs_unhandled
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/irqs_unhandled>>99900)){
/* * The interrupt is stuck */ __report_bad_irq
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/__report_bad_irq>(desc,action_ret);
/* * Now kill the IRQ */ pr_emerg
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/pr_emerg>("Disabling
IRQ #%d\n",irq); desc->istate
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/istate>|=IRQS_SPURIOUS_DISABLED
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/IRQS_SPURIOUS_DISABLED>;
desc->depth <https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/depth>++;
irq_disable
<https://elixir.bootlin.com/linux/v7.2-rc1/C/ident/irq_disable>(desc) a
stuck interrupt source would catch here.


>> nobody cared" detection, so a stuck interrupt source would never be
>> caught.
>>
>> Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
>> ---
>> changlog in v2:
>> modify gitlog for Modify the incorrect Git commit message to match
>> the reason for the modification.
>> ---
>>   drivers/i2c/busses/i2c-k1.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
>> index c2d090f6ba80..487f23fde725 100644
>> --- a/drivers/i2c/busses/i2c-k1.c
>> +++ b/drivers/i2c/busses/i2c-k1.c
>> @@ -728,7 +728,7 @@ static irqreturn_t spacemit_i2c_irq_handler(int
>> irq, void *devid)
>>         status = readl(i2c->base + SPACEMIT_ISR);
>>       if (!status)
>> -        return IRQ_HANDLED;
>> +        return IRQ_NONE;
>>         i2c->status = status;
>>   
>
>

Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Mukesh Savaliya]

On 7/10/2026 2:58 PM, Pei Xiao wrote:
> 
> 
> 在 2026/7/10 16:50, Mukesh Savaliya 写道:
>>
>>
>> On 7/10/2026 12:51 PM, Pei Xiao wrote:
>>> When the interrupt status register reads zero, the handler should
>>> return IRQ_NONE instead of IRQ_HANDLED. What the return value
>>> actually feeds into is the spurious interrupt accounting in
>>> note_interrupt(): falsely claiming IRQ_HANDLED defeats the "irq XX:
>> note_interrupt() ? You mean to say No interrupt OR invalid interrupt
>> here ? I could not understand function name.
> https://elixir.bootlin.com/linux/v7.2-rc1/source/kernel/irq/spurious.c#L222
Thanks a lot ! I understood now.
>
  [...]

>>>
>>> Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
>>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
>>> ---
>>> changlog in v2:
>>> modify gitlog for Modify the incorrect Git commit message to match
>>> the reason for the modification.
>>> ---

Reviewed-by: Mukesh Savaliya <mukesh.savaliya@oss.qualcomm.com>

Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Troy Mitchell]

On Fri Jul 10, 2026 at 2:18 AM PDT, Pei Xiao wrote:
>
>
> 在 2026/7/10 15:46, Troy Mitchell 写道:
>> On Fri Jul 10, 2026 at 12:21 AM PDT, Pei Xiao wrote:
>>> When the interrupt status register reads zero, the handler should
>>> return IRQ_NONE instead of IRQ_HANDLED. What the return value
>>> actually feeds into is the spurious interrupt accounting in
>>> note_interrupt(): falsely claiming IRQ_HANDLED defeats the "irq XX:
>>> nobody cared" detection, so a stuck interrupt source would never be
>>> caught.
>>>
>>> Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
>>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
>> The commit message now describes the actual mechanism correctly,
>> thanks for reworking it:
>>
>> Reviewed-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
>>
>> One process note, though: please don't repost new versions this
>> quickly. v2 followed the review comments almost immediately, which
>> means other reviewers who were still looking at v1 never got a
>> chance to chime in before the thread moved on, and the discussion
>> gets fragmented across versions.
> Because I noticed a simple mistake right after sending v1
> —I forgot to include the version number and changelog for v2—
> so I thought that by fixing it without waiting for reviewers to point it
> out, 
> I could save reviewers' time and get the patch merged more quickly.
>  I may have been wrong about that. Sorry.
>
I'm not referring to this thread. You posted an earlier version [1],
and then sent the patches in this thread just one day later (I know that
one was sent by mistake, without a changelog).

Link: https://lore.kernel.org/all/aaba8ba228a99f580050004c5b7e262d855de44c.1783590752.git.xiaopei01@kylinos.cn/ [1]

                                          - Troy

Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Andi Shyti]

Hi Pei,

> > One process note, though: please don't repost new versions this
> > quickly. v2 followed the review comments almost immediately, which
> > means other reviewers who were still looking at v1 never got a
> > chance to chime in before the thread moved on, and the discussion
> > gets fragmented across versions.

> Because I noticed a simple mistake right after sending v1
> —I forgot to include the version number and changelog for v2—
> so I thought that by fixing it without waiting for reviewers to point it
> out, 
> I could save reviewers' time and get the patch merged more quickly.
>  I may have been wrong about that. Sorry.

You could have just replied to your own patch. It happens to
everyone to make such mistakes.

Andi

Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Andi Shyti]

Hi Pei,

On Fri, Jul 10, 2026 at 03:21:13PM +0800, Pei Xiao wrote:
> When the interrupt status register reads zero, the handler should
> return IRQ_NONE instead of IRQ_HANDLED. What the return value
> actually feeds into is the spurious interrupt accounting in
> note_interrupt(): falsely claiming IRQ_HANDLED defeats the "irq XX:
> nobody cared" detection, so a stuck interrupt source would never be
> caught.
> 
> Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>

merged to i2c/i2c-fixes.

Thanks,
Andi

Re: [PATCH] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Pei Xiao]

在 2026/7/9 18:16, Troy Mitchell 写道:
> On Thu Jul 9, 2026 at 2:54 AM PDT, Pei Xiao wrote:
>> When the interrupt status register reads zero (no interrupt pending
>> from this device), the handler should return IRQ_NONE. Returning
>> IRQ_HANDLED incorrectly claims the interrupt was serviced, which can
>> prevent other devices sharing the same IRQ line from receiving their
>> interrupts.
>>
>> Fix this by returning IRQ_NONE instead of IRQ_HANDLED when the
>> status register is zero.
> The change goes in the right direction, but the rationale in the
> commit message doesn't hold, and I think the check itself can be
> improved.
>
> This driver requests its IRQ without IRQF_SHARED (see
> spacemit_i2c_probe(), which passes only IRQF_NO_SUSPEND), so there
> are no "other devices sharing the same IRQ line" to begin with.
> Even on a genuinely shared line, the kernel invokes every handler
> registered on the line regardless of what previous handlers return,
> so IRQ_HANDLED cannot prevent other handlers from running.
Thanks for pointing out！
>
> What the return value actually feeds into is the spurious interrupt
> accounting in note_interrupt(): falsely claiming IRQ_HANDLED defeats
> the "irq XX: nobody cared" detection, so a stuck interrupt source
> would never be caught. That applies to dedicated lines as well, and
> is the real justification for this change.
>
> Could you rework the check and the commit message and send a v2?
yes, I will check again and send a v2.

Thanks! 
Pei.
>                                   - Troy

Re: [PATCH] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Troy Mitchell]

On Thu Jul 9, 2026 at 2:54 AM PDT, Pei Xiao wrote:
> When the interrupt status register reads zero (no interrupt pending
> from this device), the handler should return IRQ_NONE. Returning
> IRQ_HANDLED incorrectly claims the interrupt was serviced, which can
> prevent other devices sharing the same IRQ line from receiving their
> interrupts.
>
> Fix this by returning IRQ_NONE instead of IRQ_HANDLED when the
> status register is zero.
The change goes in the right direction, but the rationale in the
commit message doesn't hold, and I think the check itself can be
improved.

This driver requests its IRQ without IRQF_SHARED (see
spacemit_i2c_probe(), which passes only IRQF_NO_SUSPEND), so there
are no "other devices sharing the same IRQ line" to begin with.
Even on a genuinely shared line, the kernel invokes every handler
registered on the line regardless of what previous handlers return,
so IRQ_HANDLED cannot prevent other handlers from running.

What the return value actually feeds into is the spurious interrupt
accounting in note_interrupt(): falsely claiming IRQ_HANDLED defeats
the "irq XX: nobody cared" detection, so a stuck interrupt source
would never be caught. That applies to dedicated lines as well, and
is the real justification for this change.

Could you rework the check and the commit message and send a v2?

                                  - Troy

[PATCH] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED

[Pei Xiao]

When the interrupt status register reads zero (no interrupt pending
from this device), the handler should return IRQ_NONE. Returning
IRQ_HANDLED incorrectly claims the interrupt was serviced, which can
prevent other devices sharing the same IRQ line from receiving their
interrupts.

Fix this by returning IRQ_NONE instead of IRQ_HANDLED when the
status register is zero.

Fixes: 5ea558473fa3 ("i2c: spacemit: add support for SpacemiT K1 SoC")
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/i2c/busses/i2c-k1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
index c2d090f6ba80..487f23fde725 100644
--- a/drivers/i2c/busses/i2c-k1.c
+++ b/drivers/i2c/busses/i2c-k1.c
@@ -728,7 +728,7 @@ static irqreturn_t spacemit_i2c_irq_handler(int irq, void *devid)
 
 	status = readl(i2c->base + SPACEMIT_ISR);
 	if (!status)
-		return IRQ_HANDLED;
+		return IRQ_NONE;
 
 	i2c->status = status;
 
-- 
2.25.1