The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED
@ 2026-07-10  7:14 Pei Xiao
  2026-07-10  7:21 ` [PATCH v2] " Pei Xiao
  0 siblings, 1 reply; 6+ messages in thread
From: Pei Xiao @ 2026-07-10  7:14 UTC (permalink / raw)
  To: troy.mitchell, andi.shyti, linux-i2c, linux-kernel, linux-riscv,
	spacemit
  Cc: 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED
  2026-07-10  7:14 [PATCH] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED Pei Xiao
@ 2026-07-10  7:21 ` Pei Xiao
  2026-07-10  7:46   ` Troy Mitchell
  2026-07-10  8:50   ` Mukesh Savaliya
  0 siblings, 2 replies; 6+ messages in thread
From: Pei Xiao @ 2026-07-10  7:21 UTC (permalink / raw)
  To: xiaopei01
  Cc: andi.shyti, linux-i2c, linux-kernel, linux-riscv, spacemit,
	troy.mitchell

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED
  2026-07-10  7:21 ` [PATCH v2] " Pei Xiao
@ 2026-07-10  7:46   ` Troy Mitchell
  2026-07-10  9:18     ` Pei Xiao
  2026-07-10  8:50   ` Mukesh Savaliya
  1 sibling, 1 reply; 6+ messages in thread
From: Troy Mitchell @ 2026-07-10  7:46 UTC (permalink / raw)
  To: Pei Xiao
  Cc: andi.shyti, linux-i2c, linux-kernel, linux-riscv, spacemit,
	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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED
  2026-07-10  7:21 ` [PATCH v2] " Pei Xiao
  2026-07-10  7:46   ` Troy Mitchell
@ 2026-07-10  8:50   ` Mukesh Savaliya
  2026-07-10  9:28     ` Pei Xiao
  1 sibling, 1 reply; 6+ messages in thread
From: Mukesh Savaliya @ 2026-07-10  8:50 UTC (permalink / raw)
  To: Pei Xiao
  Cc: andi.shyti, linux-i2c, linux-kernel, linux-riscv, spacemit,
	troy.mitchell



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;
>   


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED
  2026-07-10  7:46   ` Troy Mitchell
@ 2026-07-10  9:18     ` Pei Xiao
  0 siblings, 0 replies; 6+ messages in thread
From: Pei Xiao @ 2026-07-10  9:18 UTC (permalink / raw)
  To: Troy Mitchell; +Cc: andi.shyti, linux-i2c, linux-kernel, linux-riscv, spacemit



在 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
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED
  2026-07-10  8:50   ` Mukesh Savaliya
@ 2026-07-10  9:28     ` Pei Xiao
  0 siblings, 0 replies; 6+ messages in thread
From: Pei Xiao @ 2026-07-10  9:28 UTC (permalink / raw)
  To: Mukesh Savaliya
  Cc: andi.shyti, linux-i2c, linux-kernel, linux-riscv, spacemit,
	troy.mitchell



在 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;
>>   
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-10  9:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10  7:14 [PATCH] i2c: spacemit: fix spurious IRQ handling returning IRQ_HANDLED Pei Xiao
2026-07-10  7:21 ` [PATCH v2] " Pei Xiao
2026-07-10  7:46   ` Troy Mitchell
2026-07-10  9:18     ` Pei Xiao
2026-07-10  8:50   ` Mukesh Savaliya
2026-07-10  9:28     ` Pei Xiao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox