i2c: omap: fix NACK and Arbitration Lost irq handling

i2c: omap: fix NACK and Arbitration Lost irq handling

[Alexander Kochetkov]

commit 1d7afc95946487945cc7f5019b41255b72224b70 (i2c: omap: ack IRQ in parts)
changed the interrupt handler to complete transfers without clearing
XRDY (AL case) and ARDY (NACK case) flags. XRDY or ARDY interrupt will be
fired again (in parallel with omap_i2c_xfer_msg). Interrupt handler will
complete transfers second time. As a result, NACK and AL transfers
terminates with "transfer timeout" and sometimes client code segfault.

The patch restore original logic of handling NACK and AL interrupts and
fix race between interrupt handler and omap_i2c_xfer_msg (for AL and
NACK case only).

Tested on Beagleboard XM C.

Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-omap.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 90dcc2e..9af7095 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -926,14 +926,12 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 		if (stat & OMAP_I2C_STAT_NACK) {
 			err |= OMAP_I2C_STAT_NACK;
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK);
-			break;
 		}
 
 		if (stat & OMAP_I2C_STAT_AL) {
 			dev_err(dev->dev, "Arbitration lost\n");
 			err |= OMAP_I2C_STAT_AL;
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_AL);
-			break;
 		}
 
 		/*
-- 
1.7.9.5

i2c: omap: fix "Too much work in one IRQ" irq handling

[Alexander Kochetkov]

commit 66b9298878742f08cb6e79b7c7d5632d782fd1e1 (i2c: omap: switch over
to do {} while loop) changed the interrupt handler to abort transfers
in case interrupt serviced 100 times but commit's comment states that
"No functional changes otherwise.".

Also, original commit could report status 0 (no error) on aborted transfers.

The patch restore original logic. In case interrupt serviced 100 times just
quit interrupt handler and don't abort active transfer.

Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-omap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 9af7095..34b9011 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -920,7 +920,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
 		if (count++ == 100) {
 			dev_warn(dev->dev, "Too much work in one IRQ\n");
-			break;
+			goto out;
 		}
 
 		if (stat & OMAP_I2C_STAT_NACK) {
-- 
1.7.9.5

Re: i2c: omap: fix NACK and Arbitration Lost irq handling

[Grygorii Strashko]

On 11/15/2014 03:12 AM, Alexander Kochetkov wrote:
> commit 1d7afc95946487945cc7f5019b41255b72224b70 (i2c: omap: ack IRQ in parts)
> changed the interrupt handler to complete transfers without clearing
> XRDY (AL case) and ARDY (NACK case) flags. XRDY or ARDY interrupt will be
> fired again (in parallel with omap_i2c_xfer_msg). Interrupt handler will
> complete transfers second time. As a result, NACK and AL transfers
> terminates with "transfer timeout" and sometimes client code segfault.
>
> The patch restore original logic of handling NACK and AL interrupts and
> fix race between interrupt handler and omap_i2c_xfer_msg (for AL and
> NACK case only).
>
> Tested on Beagleboard XM C.

Seems you've got the same issue as I :) long time ago
https://lkml.org/lkml/2013/6/7/530

>
> Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   drivers/i2c/busses/i2c-omap.c |    2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 90dcc2e..9af7095 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -926,14 +926,12 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
>   		if (stat & OMAP_I2C_STAT_NACK) {
>   			err |= OMAP_I2C_STAT_NACK;
>   			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK);
> -			break;
>   		}
>
>   		if (stat & OMAP_I2C_STAT_AL) {
>   			dev_err(dev->dev, "Arbitration lost\n");
>   			err |= OMAP_I2C_STAT_AL;
>   			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_AL);
> -			break;
>   		}
>
>   		/*
>