[PATCH] i2c: ls2x-v2: return IRQ_HANDLED after servicing an error

[PATCH] i2c: ls2x-v2: return IRQ_HANDLED after servicing an error

[David Carlier]

The event ISR reads SR1 and, when an error flag (ARLO/AF/BERR) is set,
calls loongson2_i2c_isr_error() which clears the offending flag, issues
STOP for the AF case, records msg->result, masks every CR2 interrupt
enable and completes the waiter. The handler then returns IRQ_NONE,
declaring to the IRQ core that the device did not interrupt.

That report is wrong. The device did interrupt and the handler fully
serviced it. Because the IRQ is requested with IRQF_SHARED, the genirq
spurious-IRQ tracker counts each error as unhandled. A bus that emits
sporadic NACKs, arbitration losses or bus errors will therefore march
toward the spurious-IRQ threshold and the line can end up disabled,
wedging the controller.

Return IRQ_HANDLED on this path. The other IRQ_NONE site, taken when
neither an event nor an error bit is set, remains correct.

Fixes: 6d1b0785f6d5 ("i2c: ls2x-v2: Add driver for Loongson-2K0300 I2C controller")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 drivers/i2c/busses/i2c-ls2x-v2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-ls2x-v2.c b/drivers/i2c/busses/i2c-ls2x-v2.c
index 517760d70169..9df73557ecc4 100644
--- a/drivers/i2c/busses/i2c-ls2x-v2.c
+++ b/drivers/i2c/busses/i2c-ls2x-v2.c
@@ -304,7 +304,7 @@ static irqreturn_t loongson2_i2c_isr_event(int irq, void *data)
 	regmap_read(priv->regmap, LOONGSON2_I2C_SR1, &status);
 	if (status & LOONGSON2_I2C_SR1_ITERREN_MASK) {
 		loongson2_i2c_isr_error(status, data);
-		return IRQ_NONE;
+		return IRQ_HANDLED;
 	}
 
 	regmap_read(priv->regmap, LOONGSON2_I2C_CR2, &cr2);
-- 
2.53.0

Re: [PATCH] i2c: ls2x-v2: return IRQ_HANDLED after servicing an error

[Andy Shevchenko]

On Wed, May 06, 2026 at 05:48:18AM +0100, David Carlier wrote:
> The event ISR reads SR1 and, when an error flag (ARLO/AF/BERR) is set,
> calls loongson2_i2c_isr_error() which clears the offending flag, issues
> STOP for the AF case, records msg->result, masks every CR2 interrupt
> enable and completes the waiter. The handler then returns IRQ_NONE,
> declaring to the IRQ core that the device did not interrupt.
> 
> That report is wrong. The device did interrupt and the handler fully
> serviced it. Because the IRQ is requested with IRQF_SHARED, the genirq
> spurious-IRQ tracker counts each error as unhandled. A bus that emits
> sporadic NACKs, arbitration losses or bus errors will therefore march
> toward the spurious-IRQ threshold and the line can end up disabled,
> wedging the controller.

Have you exhibited this on a real HW?

> Return IRQ_HANDLED on this path. The other IRQ_NONE site, taken when
> neither an event nor an error bit is set, remains correct.

Hmm... This sounds logical, but we need the Loongson folks to confirm as this
is sensitive code and changes like this may affect existing work flows.

>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-ls2x-v2.c b/drivers/i2c/busses/i2c-ls2x-v2.c
> index 517760d70169..9df73557ecc4 100644
> --- a/drivers/i2c/busses/i2c-ls2x-v2.c
> +++ b/drivers/i2c/busses/i2c-ls2x-v2.c
> @@ -304,7 +304,7 @@ static irqreturn_t loongson2_i2c_isr_event(int irq, void *data)
>  	regmap_read(priv->regmap, LOONGSON2_I2C_SR1, &status);
>  	if (status & LOONGSON2_I2C_SR1_ITERREN_MASK) {
>  		loongson2_i2c_isr_error(status, data);
> -		return IRQ_NONE;
> +		return IRQ_HANDLED;
>  	}
>  
>  	regmap_read(priv->regmap, LOONGSON2_I2C_CR2, &cr2);

P.S.
Is the analysis and/or commit message AI assisted?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] i2c: ls2x-v2: return IRQ_HANDLED after servicing an error

[Binbin Zhou]

Hi all:

On 2026/5/6 17:11, Andy Shevchenko wrote:
> On Wed, May 06, 2026 at 05:48:18AM +0100, David Carlier wrote:
>> The event ISR reads SR1 and, when an error flag (ARLO/AF/BERR) is set,
>> calls loongson2_i2c_isr_error() which clears the offending flag, issues
>> STOP for the AF case, records msg->result, masks every CR2 interrupt
>> enable and completes the waiter. The handler then returns IRQ_NONE,
>> declaring to the IRQ core that the device did not interrupt.
>>
>> That report is wrong. The device did interrupt and the handler fully
>> serviced it. Because the IRQ is requested with IRQF_SHARED, the genirq
>> spurious-IRQ tracker counts each error as unhandled. A bus that emits
>> sporadic NACKs, arbitration losses or bus errors will therefore march
>> toward the spurious-IRQ threshold and the line can end up disabled,
>> wedging the controller.
> Have you exhibited this on a real HW?
>
>> Return IRQ_HANDLED on this path. The other IRQ_NONE site, taken when
>> neither an event nor an error bit is set, remains correct.
> Hmm... This sounds logical, but we need the Loongson folks to confirm as this
> is sensitive code and changes like this may affect existing work flows.

I will try to test this boundary case in the next couple of days.

>
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-ls2x-v2.c b/drivers/i2c/busses/i2c-ls2x-v2.c
>> index 517760d70169..9df73557ecc4 100644
>> --- a/drivers/i2c/busses/i2c-ls2x-v2.c
>> +++ b/drivers/i2c/busses/i2c-ls2x-v2.c
>> @@ -304,7 +304,7 @@ static irqreturn_t loongson2_i2c_isr_event(int irq, void *data)
>>   	regmap_read(priv->regmap, LOONGSON2_I2C_SR1, &status);
>>   	if (status & LOONGSON2_I2C_SR1_ITERREN_MASK) {
>>   		loongson2_i2c_isr_error(status, data);
>> -		return IRQ_NONE;
>> +		return IRQ_HANDLED;
>>   	}
>>   
>>   	regmap_read(priv->regmap, LOONGSON2_I2C_CR2, &cr2);
> P.S.
> Is the analysis and/or commit message AI assisted?
Thanks.
Binbin
>

Re: [PATCH] i2c: ls2x-v2: return IRQ_HANDLED after servicing an error

[Andy Shevchenko]

On Wed, May 06, 2026 at 11:26:14AM +0100, David CARLIER wrote:
> On Wed, 6 May 2026, 10:12 Andy Shevchenko, <andriy.shevchenko@intel.com>
> wrote:
> > On Wed, May 06, 2026 at 05:48:18AM +0100, David Carlier wrote:

...

> > Is the analysis and/or commit message AI assisted?
> >
> No h w but I Vs seen the pattern in another driver but I used codex to
> write commit message so I ll send a V2 later then. Cheers

Don't forget to mention any AI assistance. There is a special tag
Assisted-by: IIRC, but dunno if it's for reports and/or code analysis.


-- 
With Best Regards,
Andy Shevchenko

[PATCH v2] i2c: ls2x-v2: return IRQ_HANDLED after servicing an error

[David Carlier]

The event ISR reads SR1 and, when an error flag (ARLO/AF/BERR) is set,
calls loongson2_i2c_isr_error() which clears the offending flag, issues
STOP for the AF case, records msg->result, masks every CR2 interrupt
enable and completes the waiter. The handler then returns IRQ_NONE,
declaring to the IRQ core that the device did not interrupt.

That report is wrong. The device did interrupt and the handler fully
serviced it. Because the IRQ is requested with IRQF_SHARED, the genirq
spurious-IRQ tracker counts each error as unhandled. A bus that emits
sporadic NACKs, arbitration losses or bus errors will therefore march
toward the spurious-IRQ threshold and the line can end up disabled,
wedging the controller.

Return IRQ_HANDLED on this path. The other IRQ_NONE site, taken when
neither an event nor an error bit is set, remains correct.

Fixes: 6d1b0785f6d5 ("i2c: ls2x-v2: Add driver for Loongson-2K0300 I2C controller")
Assisted-by: Codex:GPT-5.5
Signed-off-by: David Carlier <devnexen@gmail.com>
---
v2:
 - Add Assisted-by: Codex:GPT-5.5 trailer (no code change).

 drivers/i2c/busses/i2c-ls2x-v2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-ls2x-v2.c b/drivers/i2c/busses/i2c-ls2x-v2.c
index 517760d70169..9df73557ecc4 100644
--- a/drivers/i2c/busses/i2c-ls2x-v2.c
+++ b/drivers/i2c/busses/i2c-ls2x-v2.c
@@ -304,7 +304,7 @@ static irqreturn_t loongson2_i2c_isr_event(int irq, void *data)
 	regmap_read(priv->regmap, LOONGSON2_I2C_SR1, &status);
 	if (status & LOONGSON2_I2C_SR1_ITERREN_MASK) {
 		loongson2_i2c_isr_error(status, data);
-		return IRQ_NONE;
+		return IRQ_HANDLED;
 	}
 
 	regmap_read(priv->regmap, LOONGSON2_I2C_CR2, &cr2);
-- 
2.53.0

Re: [PATCH v2] i2c: ls2x-v2: return IRQ_HANDLED after servicing an error

[Andy Shevchenko]

On Wed, May 06, 2026 at 04:40:15PM +0100, David Carlier wrote:
> The event ISR reads SR1 and, when an error flag (ARLO/AF/BERR) is set,
> calls loongson2_i2c_isr_error() which clears the offending flag, issues
> STOP for the AF case, records msg->result, masks every CR2 interrupt
> enable and completes the waiter. The handler then returns IRQ_NONE,
> declaring to the IRQ core that the device did not interrupt.
> 
> That report is wrong. The device did interrupt and the handler fully
> serviced it. Because the IRQ is requested with IRQF_SHARED, the genirq
> spurious-IRQ tracker counts each error as unhandled. A bus that emits
> sporadic NACKs, arbitration losses or bus errors will therefore march
> toward the spurious-IRQ threshold and the line can end up disabled,
> wedging the controller.
> 
> Return IRQ_HANDLED on this path. The other IRQ_NONE site, taken when
> neither an event nor an error bit is set, remains correct.

From the analysis and code sounds legit,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
but my tag applies only after Loongson people confirm this is correct change.

-- 
With Best Regards,
Andy Shevchenko