[PATCH] omap i2c: add timeout to a busy loop in isr

[PATCH] omap i2c: add timeout to a busy loop in isr

[Alexander Shishkin]

Signed-off-by: Alexander Shishkin <virtuoso@slind.org>
---
 drivers/i2c/busses/i2c-omap.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 75bf3ad..7617f18 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -803,15 +803,20 @@ complete:
 				 */
 
 				if (dev->rev <= OMAP_I2C_REV_ON_3430) {
-						while (!(stat & OMAP_I2C_STAT_XUDF)) {
-							if (stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
-								omap_i2c_ack_stat(dev, stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
-								err |= OMAP_I2C_STAT_XUDF;
-								goto complete;
-							}
-							cpu_relax();
-							stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+					unsigned long timeout = 10000;
+
+					while (!(stat & OMAP_I2C_STAT_XUDF) && --timeout) {
+						if (stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
+							omap_i2c_ack_stat(dev, stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
+							err |= OMAP_I2C_STAT_XUDF;
+							goto complete;
 						}
+						cpu_relax();
+						stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+					}
+
+					if (!timeout)
+						dev_err(dev->dev, "timeout waiting on XUDF bit\n");
 				}
 
 				omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
-- 
1.6.3.3

Re: [PATCH] omap i2c: add timeout to a busy loop in isr

[Aaro Koskinen]

Hi,

Alexander Shishkin wrote:
> Signed-off-by: Alexander Shishkin <virtuoso@slind.org>

Could you add a better patch description? I.e. the patch is needed
because I2C failure can cause a kernel hang. The problem can be
reproduced by running the bus with wrong (too high) speed.

Also, you should send the patch to ben-linux@fluff.org with
linux-omap and linux-i2c@vger.kernel.org in CC.

Tested-by: Aaro Koskinen <aaro.koskinen@nokia.com>

>  drivers/i2c/busses/i2c-omap.c |   21 +++++++++++++--------
>  1 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 75bf3ad..7617f18 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -803,15 +803,20 @@ complete:
>  				 */
>  
>  				if (dev->rev <= OMAP_I2C_REV_ON_3430) {
> -						while (!(stat & OMAP_I2C_STAT_XUDF)) {
> -							if (stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
> -								omap_i2c_ack_stat(dev, stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
> -								err |= OMAP_I2C_STAT_XUDF;
> -								goto complete;
> -							}
> -							cpu_relax();
> -							stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> +					unsigned long timeout = 10000;

I think 10000 reads (with L4 core interconnect latency) is at least 0.8 ms. It should be more than
enough for normal cases.

> +					while (!(stat & OMAP_I2C_STAT_XUDF) && --timeout) {
> +						if (stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
> +							omap_i2c_ack_stat(dev, stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
> +							err |= OMAP_I2C_STAT_XUDF;
> +							goto complete;
>  						}
> +						cpu_relax();
> +						stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> +					}
> +
> +					if (!timeout)
> +						dev_err(dev->dev, "timeout waiting on XUDF bit\n");
>  				}
>  
>  				omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);

A.