[PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Sonasath, Moiz]

This patch includes the workarround for I2C Errata 1.153: When an XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG

Signed-off-by: Jagadeesh Bhaskar Pakaravoor <j-pakaravoor@ti.com>
Signed-off by: Nishant Kamat <nskamat@ti.com>
Signed-off-by: Moiz Sonasath<m-sonasath@ti.com>
---
 drivers/i2c/busses/i2c-omap.c |   54 +++++++++++++++++++++++++++++++++++++---
 1 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index ece0125..e84836b 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -632,6 +632,37 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
 #define omap_i2c_rev1_isr		NULL
 #endif
 
+/* I2C Errata 1.153:
+ * When an XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG.
+ * Otherwise some data bytes can be lost while transferring them from the
+ * memory to the I2C interface.
+ */
+
+static int omap_i2c_wait_for_xudf(struct omap_i2c_dev *dev)
+{
+	u16 xudf;
+	int counter = 500;
+
+	/* We are in interrupt context. Wait for XUDF for max 7 msec */
+	xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+	while (!(xudf & OMAP_I2C_STAT_XUDF) && counter--) {
+		if (xudf & (OMAP_I2C_STAT_ROVR | OMAP_I2C_STAT_NACK |
+			    OMAP_I2C_STAT_AL))
+			return -EINVAL;
+		udelay(10);
+		xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+	}
+
+	if (!counter) {
+		/* Clear Tx FIFO */
+		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
+				OMAP_I2C_BUF_TXFIF_CLR);
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
 static irqreturn_t
 omap_i2c_isr(int this_irq, void *dev_id)
 {
@@ -639,6 +670,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
 	u16 bits;
 	u16 stat, w;
 	int err, count = 0;
+	int error;
 
 	if (dev->idle)
 		return IRQ_NONE;
@@ -647,7 +679,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
 	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
 		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
 		if (count++ == 100) {
-			dev_warn(dev->dev, "Too much work in one IRQ\n");
+			dev_dbg(dev->dev, "Too much work in one IRQ\n");
 			break;
 		}
 
@@ -715,11 +747,22 @@ omap_i2c_isr(int this_irq, void *dev_id)
 							OMAP_I2C_BUFSTAT_REG);
 			}
 			while (num_bytes) {
-				num_bytes--;
 				w = 0;
 				if (dev->buf_len) {
+					if (cpu_is_omap34xx()) {
+						/* OMAP3430 Errata 1.153 */
+						error = omap_i2c_wait_for_xudf(dev);
+						if (error) {
+							omap_i2c_ack_stat(dev, stat &
+								(OMAP_I2C_STAT_XRDY |
+								 OMAP_I2C_STAT_XDR));
+							dev_err(dev->dev, "Transmit error\n");
+							omap_i2c_complete_cmd(dev, OMAP_I2C_STAT_XUDF);
+
+							return IRQ_HANDLED;
+						}
+					}
 					w = *dev->buf++;
-					dev->buf_len--;
 					/* Data reg from  2430 is 8 bit wide */
 					if (!cpu_is_omap2430() &&
 							!cpu_is_omap34xx()) {
@@ -728,6 +771,10 @@ omap_i2c_isr(int this_irq, void *dev_id)
 							dev->buf_len--;
 						}
 					}
+					omap_i2c_write_reg(dev,
+						OMAP_I2C_DATA_REG, w);
+					num_bytes--;
+					dev->buf_len--;
 				} else {
 					if (stat & OMAP_I2C_STAT_XRDY)
 						dev_err(dev->dev,
@@ -739,7 +786,6 @@ omap_i2c_isr(int this_irq, void *dev_id)
 							"data to send\n");
 					break;
 				}
-				omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
 			}
 			omap_i2c_ack_stat(dev,
 				stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
-- 
1.5.6.3

Re: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Kevin Hilman]

"Sonasath, Moiz" <m-sonasath@ti.com> writes:

> This patch includes the workarround for I2C Errata 1.153: When an XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG

How was this tested/validated, on what platforms etc.

> Signed-off-by: Jagadeesh Bhaskar Pakaravoor <j-pakaravoor@ti.com>
> Signed-off by: Nishant Kamat <nskamat@ti.com>
> Signed-off-by: Moiz Sonasath<m-sonasath@ti.com>
> ---
>  drivers/i2c/busses/i2c-omap.c |   54 +++++++++++++++++++++++++++++++++++++---
>  1 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index ece0125..e84836b 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -632,6 +632,37 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
>  #define omap_i2c_rev1_isr		NULL
>  #endif
>  
> +/* I2C Errata 1.153:
> + * When an XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG.
> + * Otherwise some data bytes can be lost while transferring them from the
> + * memory to the I2C interface.
> + */
> +
> +static int omap_i2c_wait_for_xudf(struct omap_i2c_dev *dev)
> +{
> +	u16 xudf;
> +	int counter = 500;
> +
> +	/* We are in interrupt context. Wait for XUDF for max 7 msec */

What does being in interrupt context have to do with how long you
wait?  Threaded interrupts are now in mainline and will become the
default, so this ISR may run in thread context.


> +	xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> +	while (!(xudf & OMAP_I2C_STAT_XUDF) && counter--) {
> +		if (xudf & (OMAP_I2C_STAT_ROVR | OMAP_I2C_STAT_NACK |
> +			    OMAP_I2C_STAT_AL))
> +			return -EINVAL;
> +		udelay(10);
> +		xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> +	}
> +
> +	if (!counter) {
> +		/* Clear Tx FIFO */
> +		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
> +				OMAP_I2C_BUF_TXFIF_CLR);
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
>  static irqreturn_t
>  omap_i2c_isr(int this_irq, void *dev_id)
>  {
> @@ -639,6 +670,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  	u16 bits;
>  	u16 stat, w;
>  	int err, count = 0;
> +	int error;
>  
>  	if (dev->idle)
>  		return IRQ_NONE;
> @@ -647,7 +679,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
>  		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
>  		if (count++ == 100) {
> -			dev_warn(dev->dev, "Too much work in one IRQ\n");
> +			dev_dbg(dev->dev, "Too much work in one IRQ\n");

Should stay as dev_warn I think.

>  			break;
>  		}
>  
> @@ -715,11 +747,22 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  							OMAP_I2C_BUFSTAT_REG);
>  			}
>  			while (num_bytes) {
> -				num_bytes--;
>  				w = 0;
>  				if (dev->buf_len) {
> +					if (cpu_is_omap34xx()) {
> +						/* OMAP3430 Errata 1.153 */
> +						error = omap_i2c_wait_for_xudf(dev);
> +						if (error) {
> +							omap_i2c_ack_stat(dev, stat &
> +								(OMAP_I2C_STAT_XRDY |
> +								 OMAP_I2C_STAT_XDR));
> +							dev_err(dev->dev, "Transmit error\n");
> +							omap_i2c_complete_cmd(dev, OMAP_I2C_STAT_XUDF);
> +
> +							return IRQ_HANDLED;
> +						}
> +					}

Yuck, too much wrapping here.  Running through checkpatch would've warned
you about this.  Looks like you need to break this out into a subroutine.


>  					w = *dev->buf++;
> -					dev->buf_len--;
>  					/* Data reg from  2430 is 8 bit wide */
>  					if (!cpu_is_omap2430() &&
>  							!cpu_is_omap34xx()) {
> @@ -728,6 +771,10 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  							dev->buf_len--;
>  						}
>  					}
> +					omap_i2c_write_reg(dev,
> +						OMAP_I2C_DATA_REG, w);
> +					num_bytes--;
> +					dev->buf_len--;
>  				} else {
>  					if (stat & OMAP_I2C_STAT_XRDY)
>  						dev_err(dev->dev,
> @@ -739,7 +786,6 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  							"data to send\n");
>  					break;
>  				}
> -				omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
>  			}
>  			omap_i2c_ack_stat(dev,
>  				stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
> -- 
> 1.5.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Kamat, Nishant]

Kevin, 

> -----Original Message-----
> From: Kevin Hilman
>
> "Sonasath, Moiz" <m-sonasath@ti.com> writes:
> 
> > This patch includes the workarround for I2C Errata 1.153: 
[...]

> > +static int omap_i2c_wait_for_xudf(struct omap_i2c_dev *dev)
> > +{
> > +	u16 xudf;
> > +	int counter = 500;
> > +
> > +	/* We are in interrupt context. Wait for XUDF for max 7 msec */
> 
> What does being in interrupt context have to do with how long you
> wait?  Threaded interrupts are now in mainline and will become the
> default, so this ISR may run in thread context.

The interrupt context comment was meant to say that we can't sleep. Perhaps, with threaded interrupts that might not be true (I am not sure). We can remove the interrupt context comment.


> > @@ -647,7 +679,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  	while ((stat = (omap_i2c_read_reg(dev, 
> OMAP_I2C_STAT_REG))) & bits) {
> >  		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
> >  		if (count++ == 100) {
> > -			dev_warn(dev->dev, "Too much work in 
> one IRQ\n");
> > +			dev_dbg(dev->dev, "Too much work in one IRQ\n");
> 
> Should stay as dev_warn I think.
> 

When I2C is used to transfer a large number of bytes continuously (e.g. during some camera sensor firmware update), we hit the max count more often now (because of the delay introduced by the workaround implementation). In this case, its undesirable to see the dev_warn messages fill up the console. Changing this to dev_dbg means that this message is not printed in the expected case.


Regards,
Nishant

RE: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Menon, Nishanth]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Kamat, Nishant
> Sent: Tuesday, June 23, 2009 12:55 PM
> 
> > > @@ -647,7 +679,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
> > >  	while ((stat = (omap_i2c_read_reg(dev,
> > OMAP_I2C_STAT_REG))) & bits) {
> > >  		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
> > >  		if (count++ == 100) {
> > > -			dev_warn(dev->dev, "Too much work in
> > one IRQ\n");
> > > +			dev_dbg(dev->dev, "Too much work in one IRQ\n");
> >
> > Should stay as dev_warn I think.
> >
> 
> When I2C is used to transfer a large number of bytes continuously (e.g.
> during some camera sensor firmware update), we hit the max count more
> often now (because of the delay introduced by the workaround
> implementation). In this case, its undesirable to see the dev_warn
> messages fill up the console. Changing this to dev_dbg means that this
> message is not printed in the expected case.
Just wondering on this -> cant I do a multibyte write upto 255 bytes? Is count==100 not enough given that we used buffered writes? The real question is this:
Are devices expected to trigger retry logic to the extent where the error condition is triggered?

I think this might be an indication of something else being at fault with the sensor /configuration of sensor - hiding the error messages by moving warn->dbg is not correct IMHO.

Regards,
Nishanth Menon

Re: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Paul Walmsley]

Moiz, Nishant, Jagadeesh,

Looks like this patch has some serious problems.

On Mon, 22 Jun 2009, Sonasath, Moiz wrote:

> This patch includes the workarround for I2C Errata 1.153: When an 
> XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG
> 
> Signed-off-by: Jagadeesh Bhaskar Pakaravoor <j-pakaravoor@ti.com>
> Signed-off by: Nishant Kamat <nskamat@ti.com>
> Signed-off-by: Moiz Sonasath<m-sonasath@ti.com>
> ---
>  drivers/i2c/busses/i2c-omap.c |   54 +++++++++++++++++++++++++++++++++++++---
>  1 files changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index ece0125..e84836b 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -632,6 +632,37 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
>  #define omap_i2c_rev1_isr		NULL
>  #endif
>  
> +/* I2C Errata 1.153:
> + * When an XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG.
> + * Otherwise some data bytes can be lost while transferring them from the
> + * memory to the I2C interface.
> + */
> +
> +static int omap_i2c_wait_for_xudf(struct omap_i2c_dev *dev)
> +{
> +	u16 xudf;
> +	int counter = 500;

Shouldn't the timeout threshold depend on the I2C bus speed and transmit 
FIFO threshold?

> +
> +	/* We are in interrupt context. Wait for XUDF for max 7 msec */

There's no way an interrupt service routine should loop for up to 7 
milliseconds.  Why does it need to wait this long?  Shouldn't this patch 
zero the transmit FIFO threshold to minimize the amount of time between 
XRDY/XDR and XUDF?

> +	xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> +	while (!(xudf & OMAP_I2C_STAT_XUDF) && counter--) {

Why is a separate I2C_STAT loop used here?  Shouldn't this patch just use 
the existing I2C_STAT loop in the ISR and use a state variable to indicate 
the current state of the ISR?

> +		if (xudf & (OMAP_I2C_STAT_ROVR | OMAP_I2C_STAT_NACK |
> +			    OMAP_I2C_STAT_AL))
> +			return -EINVAL;
> +		udelay(10);

udelay() in an ISR is a big red flag.  Why is this needed?

> +		xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> +	}
> +
> +	if (!counter) {
> +		/* Clear Tx FIFO */
> +		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
> +				OMAP_I2C_BUF_TXFIF_CLR);
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
>  static irqreturn_t
>  omap_i2c_isr(int this_irq, void *dev_id)
>  {
> @@ -639,6 +670,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  	u16 bits;
>  	u16 stat, w;
>  	int err, count = 0;
> +	int error;
>  
>  	if (dev->idle)
>  		return IRQ_NONE;
> @@ -647,7 +679,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
>  		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
>  		if (count++ == 100) {
> -			dev_warn(dev->dev, "Too much work in one IRQ\n");
> +			dev_dbg(dev->dev, "Too much work in one IRQ\n");
>  			break;
>  		}
>  
> @@ -715,11 +747,22 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  							OMAP_I2C_BUFSTAT_REG);
>  			}
>  			while (num_bytes) {
> -				num_bytes--;
>  				w = 0;
>  				if (dev->buf_len) {
> +					if (cpu_is_omap34xx()) {
> +						/* OMAP3430 Errata 1.153 */

What about this I2C IP block on previous chip versions?  Is this problem 
also present on 2430, which also has FIFO transmit capability?

> +						error = omap_i2c_wait_for_xudf(dev);
> +						if (error) {
> +							omap_i2c_ack_stat(dev, stat &
> +								(OMAP_I2C_STAT_XRDY |
> +								 OMAP_I2C_STAT_XDR));
> +							dev_err(dev->dev, "Transmit error\n");
> +							omap_i2c_complete_cmd(dev, OMAP_I2C_STAT_XUDF);
> +
> +							return IRQ_HANDLED;
> +						}
> +					}
>  					w = *dev->buf++;
> -					dev->buf_len--;
>  					/* Data reg from  2430 is 8 bit wide */
>  					if (!cpu_is_omap2430() &&
>  							!cpu_is_omap34xx()) {
> @@ -728,6 +771,10 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  							dev->buf_len--;
>  						}
>  					}
> +					omap_i2c_write_reg(dev,
> +						OMAP_I2C_DATA_REG, w);
> +					num_bytes--;
> +					dev->buf_len--;
>  				} else {
>  					if (stat & OMAP_I2C_STAT_XRDY)
>  						dev_err(dev->dev,
> @@ -739,7 +786,6 @@ omap_i2c_isr(int this_irq, void *dev_id)
>  							"data to send\n");
>  					break;
>  				}
> -				omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
>  			}
>  			omap_i2c_ack_stat(dev,
>  				stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));


- Paul

Re: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Paul Walmsley]

Hello Moiz, Nishant, Jagadeesh,

Any plans to respond to these comments?  

- Paul

On Mon, 22 Jun 2009, Paul Walmsley wrote:

> Moiz, Nishant, Jagadeesh,
> 
> Looks like this patch has some serious problems.
> 
> On Mon, 22 Jun 2009, Sonasath, Moiz wrote:
> 
> > This patch includes the workarround for I2C Errata 1.153: When an 
> > XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG
> > 
> > Signed-off-by: Jagadeesh Bhaskar Pakaravoor <j-pakaravoor@ti.com>
> > Signed-off by: Nishant Kamat <nskamat@ti.com>
> > Signed-off-by: Moiz Sonasath<m-sonasath@ti.com>
> > ---
> >  drivers/i2c/busses/i2c-omap.c |   54 +++++++++++++++++++++++++++++++++++++---
> >  1 files changed, 50 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> > index ece0125..e84836b 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -632,6 +632,37 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
> >  #define omap_i2c_rev1_isr		NULL
> >  #endif
> >  
> > +/* I2C Errata 1.153:
> > + * When an XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG.
> > + * Otherwise some data bytes can be lost while transferring them from the
> > + * memory to the I2C interface.
> > + */
> > +
> > +static int omap_i2c_wait_for_xudf(struct omap_i2c_dev *dev)
> > +{
> > +	u16 xudf;
> > +	int counter = 500;
> 
> Shouldn't the timeout threshold depend on the I2C bus speed and transmit 
> FIFO threshold?
> 
> > +
> > +	/* We are in interrupt context. Wait for XUDF for max 7 msec */
> 
> There's no way an interrupt service routine should loop for up to 7 
> milliseconds.  Why does it need to wait this long?  Shouldn't this patch 
> zero the transmit FIFO threshold to minimize the amount of time between 
> XRDY/XDR and XUDF?
> 
> > +	xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> > +	while (!(xudf & OMAP_I2C_STAT_XUDF) && counter--) {
> 
> Why is a separate I2C_STAT loop used here?  Shouldn't this patch just use 
> the existing I2C_STAT loop in the ISR and use a state variable to indicate 
> the current state of the ISR?
> 
> > +		if (xudf & (OMAP_I2C_STAT_ROVR | OMAP_I2C_STAT_NACK |
> > +			    OMAP_I2C_STAT_AL))
> > +			return -EINVAL;
> > +		udelay(10);
> 
> udelay() in an ISR is a big red flag.  Why is this needed?
> 
> > +		xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> > +	}
> > +
> > +	if (!counter) {
> > +		/* Clear Tx FIFO */
> > +		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
> > +				OMAP_I2C_BUF_TXFIF_CLR);
> > +		return -ETIMEDOUT;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static irqreturn_t
> >  omap_i2c_isr(int this_irq, void *dev_id)
> >  {
> > @@ -639,6 +670,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  	u16 bits;
> >  	u16 stat, w;
> >  	int err, count = 0;
> > +	int error;
> >  
> >  	if (dev->idle)
> >  		return IRQ_NONE;
> > @@ -647,7 +679,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
> >  		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
> >  		if (count++ == 100) {
> > -			dev_warn(dev->dev, "Too much work in one IRQ\n");
> > +			dev_dbg(dev->dev, "Too much work in one IRQ\n");
> >  			break;
> >  		}
> >  
> > @@ -715,11 +747,22 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  							OMAP_I2C_BUFSTAT_REG);
> >  			}
> >  			while (num_bytes) {
> > -				num_bytes--;
> >  				w = 0;
> >  				if (dev->buf_len) {
> > +					if (cpu_is_omap34xx()) {
> > +						/* OMAP3430 Errata 1.153 */
> 
> What about this I2C IP block on previous chip versions?  Is this problem 
> also present on 2430, which also has FIFO transmit capability?
> 
> > +						error = omap_i2c_wait_for_xudf(dev);
> > +						if (error) {
> > +							omap_i2c_ack_stat(dev, stat &
> > +								(OMAP_I2C_STAT_XRDY |
> > +								 OMAP_I2C_STAT_XDR));
> > +							dev_err(dev->dev, "Transmit error\n");
> > +							omap_i2c_complete_cmd(dev, OMAP_I2C_STAT_XUDF);
> > +
> > +							return IRQ_HANDLED;
> > +						}
> > +					}
> >  					w = *dev->buf++;
> > -					dev->buf_len--;
> >  					/* Data reg from  2430 is 8 bit wide */
> >  					if (!cpu_is_omap2430() &&
> >  							!cpu_is_omap34xx()) {
> > @@ -728,6 +771,10 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  							dev->buf_len--;
> >  						}
> >  					}
> > +					omap_i2c_write_reg(dev,
> > +						OMAP_I2C_DATA_REG, w);
> > +					num_bytes--;
> > +					dev->buf_len--;
> >  				} else {
> >  					if (stat & OMAP_I2C_STAT_XRDY)
> >  						dev_err(dev->dev,
> > @@ -739,7 +786,6 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  							"data to send\n");
> >  					break;
> >  				}
> > -				omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
> >  			}
> >  			omap_i2c_ack_stat(dev,
> >  				stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
> 
> 
> - Paul
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Sonasath, Moiz]

Hello Paul!

I am working on this issue and will get back soon.

Regards
Moiz Sonasath
Linux Baseport Team, Dallas
214-567-5514


-----Original Message-----
From: Paul Walmsley [mailto:paul@pwsan.com] 
Sent: Wednesday, July 08, 2009 11:51 AM
To: Sonasath, Moiz
Cc: linux-omap@vger.kernel.org; Pakaravoor, Jagadeesh; Kamat, Nishant; Pandita, Vikram
Subject: Re: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

Hello Moiz, Nishant, Jagadeesh,

Any plans to respond to these comments?  

- Paul

On Mon, 22 Jun 2009, Paul Walmsley wrote:

> Moiz, Nishant, Jagadeesh,
> 
> Looks like this patch has some serious problems.
> 
> On Mon, 22 Jun 2009, Sonasath, Moiz wrote:
> 
> > This patch includes the workarround for I2C Errata 1.153: When an 
> > XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG
> > 
> > Signed-off-by: Jagadeesh Bhaskar Pakaravoor <j-pakaravoor@ti.com>
> > Signed-off by: Nishant Kamat <nskamat@ti.com>
> > Signed-off-by: Moiz Sonasath<m-sonasath@ti.com>
> > ---
> >  drivers/i2c/busses/i2c-omap.c |   54 +++++++++++++++++++++++++++++++++++++---
> >  1 files changed, 50 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> > index ece0125..e84836b 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -632,6 +632,37 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
> >  #define omap_i2c_rev1_isr		NULL
> >  #endif
> >  
> > +/* I2C Errata 1.153:
> > + * When an XRDY/XDR is hit, wait for XUDF before writing data to DATA_REG.
> > + * Otherwise some data bytes can be lost while transferring them from the
> > + * memory to the I2C interface.
> > + */
> > +
> > +static int omap_i2c_wait_for_xudf(struct omap_i2c_dev *dev)
> > +{
> > +	u16 xudf;
> > +	int counter = 500;
> 
> Shouldn't the timeout threshold depend on the I2C bus speed and transmit 
> FIFO threshold?
> 
> > +
> > +	/* We are in interrupt context. Wait for XUDF for max 7 msec */
> 
> There's no way an interrupt service routine should loop for up to 7 
> milliseconds.  Why does it need to wait this long?  Shouldn't this patch 
> zero the transmit FIFO threshold to minimize the amount of time between 
> XRDY/XDR and XUDF?
> 
> > +	xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> > +	while (!(xudf & OMAP_I2C_STAT_XUDF) && counter--) {
> 
> Why is a separate I2C_STAT loop used here?  Shouldn't this patch just use 
> the existing I2C_STAT loop in the ISR and use a state variable to indicate 
> the current state of the ISR?
> 
> > +		if (xudf & (OMAP_I2C_STAT_ROVR | OMAP_I2C_STAT_NACK |
> > +			    OMAP_I2C_STAT_AL))
> > +			return -EINVAL;
> > +		udelay(10);
> 
> udelay() in an ISR is a big red flag.  Why is this needed?
> 
> > +		xudf = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> > +	}
> > +
> > +	if (!counter) {
> > +		/* Clear Tx FIFO */
> > +		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG,
> > +				OMAP_I2C_BUF_TXFIF_CLR);
> > +		return -ETIMEDOUT;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static irqreturn_t
> >  omap_i2c_isr(int this_irq, void *dev_id)
> >  {
> > @@ -639,6 +670,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  	u16 bits;
> >  	u16 stat, w;
> >  	int err, count = 0;
> > +	int error;
> >  
> >  	if (dev->idle)
> >  		return IRQ_NONE;
> > @@ -647,7 +679,7 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
> >  		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
> >  		if (count++ == 100) {
> > -			dev_warn(dev->dev, "Too much work in one IRQ\n");
> > +			dev_dbg(dev->dev, "Too much work in one IRQ\n");
> >  			break;
> >  		}
> >  
> > @@ -715,11 +747,22 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  							OMAP_I2C_BUFSTAT_REG);
> >  			}
> >  			while (num_bytes) {
> > -				num_bytes--;
> >  				w = 0;
> >  				if (dev->buf_len) {
> > +					if (cpu_is_omap34xx()) {
> > +						/* OMAP3430 Errata 1.153 */
> 
> What about this I2C IP block on previous chip versions?  Is this problem 
> also present on 2430, which also has FIFO transmit capability?
> 
> > +						error = omap_i2c_wait_for_xudf(dev);
> > +						if (error) {
> > +							omap_i2c_ack_stat(dev, stat &
> > +								(OMAP_I2C_STAT_XRDY |
> > +								 OMAP_I2C_STAT_XDR));
> > +							dev_err(dev->dev, "Transmit error\n");
> > +							omap_i2c_complete_cmd(dev, OMAP_I2C_STAT_XUDF);
> > +
> > +							return IRQ_HANDLED;
> > +						}
> > +					}
> >  					w = *dev->buf++;
> > -					dev->buf_len--;
> >  					/* Data reg from  2430 is 8 bit wide */
> >  					if (!cpu_is_omap2430() &&
> >  							!cpu_is_omap34xx()) {
> > @@ -728,6 +771,10 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  							dev->buf_len--;
> >  						}
> >  					}
> > +					omap_i2c_write_reg(dev,
> > +						OMAP_I2C_DATA_REG, w);
> > +					num_bytes--;
> > +					dev->buf_len--;
> >  				} else {
> >  					if (stat & OMAP_I2C_STAT_XRDY)
> >  						dev_err(dev->dev,
> > @@ -739,7 +786,6 @@ omap_i2c_isr(int this_irq, void *dev_id)
> >  							"data to send\n");
> >  					break;
> >  				}
> > -				omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
> >  			}
> >  			omap_i2c_ack_stat(dev,
> >  				stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
> 
> 
> - Paul
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Menon, Nishanth]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Sonasath, Moiz
> Sent: Monday, June 22, 2009 7:50 PM
> To: linux-omap@vger.kernel.org
> Cc: Pandita, Vikram
> Subject: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon
> errata 1.153
> 
> This patch includes the workarround for I2C Errata 1.153: When an XRDY/XDR
> is hit, wait for XUDF before writing data to DATA_REG

Is this workaround valid for omap2430 also?

Regards,
Nishanth Menon

Re: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Igor Mazanov]

Hello,

Menon, Nishanth wrote:
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Sonasath, Moiz
>> Sent: Monday, June 22, 2009 7:50 PM
>> To: linux-omap@vger.kernel.org
>> Cc: Pandita, Vikram
>> Subject: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon
>> errata 1.153
>>
>> This patch includes the workarround for I2C Errata 1.153: When an XRDY/XDR
>> is hit, wait for XUDF before writing data to DATA_REG
> 
> Is this workaround valid for omap2430 also?

Some kind of such workaround needs to be applied and for OMAP1 ISR too. 
I had
the same problem on our OMAP5910 based custom made board. While writing 
a large contiguous amount of data constantly occurs a situation when 
dev->buf_len = 0, but I2C_CNT register != 0, and, as result, I2C 
controller doesn't generate ARDY IRQ, no complete_cmd occurs in ISR, and 
we get "controller timed out" issues then...

So, here is a part of modified OMAP1 ISR. It works for me at least.

/*
  * Is there a bug in the OMAP5910 I2C controller? It
  * generates a bunch of fake XRDY interrupts under high load.
  * As result, there is a very high chance to have a situation
  * when dev->buf_len = 0 already, but I2C_CNT != 0. So, there
  * is no ARDY irq then, no complete_cmd, controller timed out
  * issues...
  *
  * Workaround:
  *
  * When we get XRDY interrupt without transmit underflow flag
  * (XUDF bit in the I2C_STAT register), delay for 100 microsecs
  * and ignore it.
  *
  * We write data into I2C_DATA register in case of transmit
  * underflow condition ONLY.
  */
if (stat & OMAP_I2C_STAT_XRDY) {
	if (!(stat & OMAP_I2C_STAT_XUDF)) {
		udelay(100);
		continue;
	} else {
		w = 0;
		if (dev->buf_len) {
			w = *dev->buf++;
			dev->buf_len--;
			if (dev->buf_len) {
				w |= *dev->buf++ << 8;
				dev->buf_len--;
			}
			omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
		} else {
			dev_err(dev->dev, "XRDY IRQ while no "
						"data to send\n");
			break;
		}
		continue;
	}
}	

Regards,
Igor.

RE: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Menon, Nishanth]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Igor Mazanov
> Sent: Tuesday, June 23, 2009 4:43 PM
> To: linux-omap@vger.kernel.org
> Subject: Re: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon
> errata 1.153
> 
> /*
>   * Is there a bug in the OMAP5910 I2C controller? It
>   * generates a bunch of fake XRDY interrupts under high load.
>   * As result, there is a very high chance to have a situation
>   * when dev->buf_len = 0 already, but I2C_CNT != 0. So, there
>   * is no ARDY irq then, no complete_cmd, controller timed out
>   * issues...
>   *
>   * Workaround:
>   *
>   * When we get XRDY interrupt without transmit underflow flag
>   * (XUDF bit in the I2C_STAT register), delay for 100 microsecs
>   * and ignore it.
>   *
>   * We write data into I2C_DATA register in case of transmit
>   * underflow condition ONLY.
>   */
> if (stat & OMAP_I2C_STAT_XRDY) {
> 	if (!(stat & OMAP_I2C_STAT_XUDF)) {
> 		udelay(100);
> 		continue;
> 	} else {
> 		w = 0;
> 		if (dev->buf_len) {
> 			w = *dev->buf++;
> 			dev->buf_len--;
> 			if (dev->buf_len) {
> 				w |= *dev->buf++ << 8;
> 				dev->buf_len--;
> 			}
> 			omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
> 		} else {
> 			dev_err(dev->dev, "XRDY IRQ while no "
> 						"data to send\n");
> 			break;
> 		}
> 		continue;
> 	}
> }
> 
Could you submit a patch please?


Regards,
Nishanth Menon

Re: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon errata 1.153

[Igor Mazanov]

Menon, Nishanth wrote:
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Igor Mazanov
>> Sent: Tuesday, June 23, 2009 4:43 PM
>> To: linux-omap@vger.kernel.org
>> Subject: Re: [PATCH] [RFC][OMAP3:I2C]Workaround for OMAP3430 I2C silicon
>> errata 1.153
>>
>> /*
>>   * Is there a bug in the OMAP5910 I2C controller? It
>>   * generates a bunch of fake XRDY interrupts under high load.
>>   * As result, there is a very high chance to have a situation
>>   * when dev->buf_len = 0 already, but I2C_CNT != 0. So, there
>>   * is no ARDY irq then, no complete_cmd, controller timed out
>>   * issues...
>>   *
>>   * Workaround:
>>   *
>>   * When we get XRDY interrupt without transmit underflow flag
>>   * (XUDF bit in the I2C_STAT register), delay for 100 microsecs
>>   * and ignore it.
>>   *
>>   * We write data into I2C_DATA register in case of transmit
>>   * underflow condition ONLY.
>>   */
>> if (stat & OMAP_I2C_STAT_XRDY) {
>> 	if (!(stat & OMAP_I2C_STAT_XUDF)) {
>> 		udelay(100);
>> 		continue;
>> 	} else {
>> 		w = 0;
>> 		if (dev->buf_len) {
>> 			w = *dev->buf++;
>> 			dev->buf_len--;
>> 			if (dev->buf_len) {
>> 				w |= *dev->buf++ << 8;
>> 				dev->buf_len--;
>> 			}
>> 			omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
>> 		} else {
>> 			dev_err(dev->dev, "XRDY IRQ while no "
>> 						"data to send\n");
>> 			break;
>> 		}
>> 		continue;
>> 	}
>> }
>>
> Could you submit a patch please?
>

OMAP1 I2C controller generates a huge amount of fake XRDY interrupts when large 
continuous blocks of data are transmitted via I2C. As result data have no time 
to be transmitted physically over I2C data line if we just look on XRDY bit 
before writing to I2C_DATA register. Taking into account a transmit underrun 
condition (XUDF bit in the I2C_STAT register) help us to transmit in more 
predictable way.

So,

Signed-off-by: Igor Mazanov <i.mazanov@gmail.com>
---
  drivers/i2c/busses/i2c-omap.c |  140 ++++++++++++++++++++++++++++------------
  1 files changed, 98 insertions(+), 42 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index ece0125..7464848 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -436,10 +436,15 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,

  	omap_i2c_write_reg(dev, OMAP_I2C_CNT_REG, dev->buf_len);

-	/* Clear the FIFO Buffers */
-	w = omap_i2c_read_reg(dev, OMAP_I2C_BUF_REG);
-	w |= OMAP_I2C_BUF_RXFIF_CLR | OMAP_I2C_BUF_TXFIF_CLR;
-	omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, w);
+	/* Clear the FIFO Buffers
+	 * Useless for OMAP1 family - according TRMs (spru681b, spru760c),
+	 * FIFO are not controllable in this way
+	 */
+	if (!cpu_class_is_omap1()) {
+		w = omap_i2c_read_reg(dev, OMAP_I2C_BUF_REG);
+		w |= OMAP_I2C_BUF_RXFIF_CLR | OMAP_I2C_BUF_TXFIF_CLR;
+		omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, w);
+	}

  	init_completion(&dev->cmd_complete);
  	dev->cmd_err = 0;
@@ -578,55 +583,106 @@ static irqreturn_t
  omap_i2c_rev1_isr(int this_irq, void *dev_id)
  {
  	struct omap_i2c_dev *dev = dev_id;
-	u16 iv, w;
+	u16 bits;
+	u16 stat, w;
+	int err, count = 0;

  	if (dev->idle)
  		return IRQ_NONE;

-	iv = omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
-	switch (iv) {
-	case 0x00:	/* None */
-		break;
-	case 0x01:	/* Arbitration lost */
-		dev_err(dev->dev, "Arbitration lost\n");
-		omap_i2c_complete_cmd(dev, OMAP_I2C_STAT_AL);
-		break;
-	case 0x02:	/* No acknowledgement */
-		omap_i2c_complete_cmd(dev, OMAP_I2C_STAT_NACK);
-		omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_STP);
-		break;
-	case 0x03:	/* Register access ready */
-		omap_i2c_complete_cmd(dev, 0);
-		break;
-	case 0x04:	/* Receive data ready */
-		if (dev->buf_len) {
+	bits = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
+	while ((stat = (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG))) & bits) {
+		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
+		if (count++ == 200) {
+			dev_warn(dev->dev, "Too much work in one IRQ\n");
+			break;
+		}
+
+		omap_i2c_read_reg(dev, OMAP_I2C_IV_REG);
+
+		err = 0;
+		if (stat & OMAP_I2C_STAT_NACK) {
+			err |= OMAP_I2C_STAT_NACK;
+			omap_i2c_write_reg(dev, OMAP_I2C_CON_REG,
+					   OMAP_I2C_CON_STP);
+		}
+		if (stat & OMAP_I2C_STAT_AL) {
+			dev_err(dev->dev, "Arbitration lost\n");
+			err |= OMAP_I2C_STAT_AL;
+		}
+		if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_NACK |
+							OMAP_I2C_STAT_AL)) {
+				omap_i2c_complete_cmd(dev, err);
+				break;
+		}
+		if (stat & OMAP_I2C_STAT_RRDY) {
  			w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG);
-			*dev->buf++ = w;
-			dev->buf_len--;
  			if (dev->buf_len) {
-				*dev->buf++ = w >> 8;
+				*dev->buf++ = w;
  				dev->buf_len--;
+				if (dev->buf_len) {
+					*dev->buf++ = w >> 8;
+					dev->buf_len--;
+				}
+			} else {
+				dev_err(dev->dev, "RRDY IRQ while no data"
+							" requested\n");
+				break;
  			}
-		} else
-			dev_err(dev->dev, "RRDY IRQ while no data requested\n");
-		break;
-	case 0x05:	/* Transmit data ready */
-		if (dev->buf_len) {
-			w = *dev->buf++;
-			dev->buf_len--;
-			if (dev->buf_len) {
-				w |= *dev->buf++ << 8;
-				dev->buf_len--;
+			continue;
+		}
+		/*
+		 * Is there a bug in the OMAP5910 I2C controller? It
+		 * generates a bunch of fake XRDY interrupts under high load.
+		 * As result, there is a very high chance to have a situation
+		 * when dev->buf_len = 0 already, but I2C_CNT != 0. So, there
+		 * is no ARDY irq then, no complete_cmd, controller timed out
+		 * issues...
+		 *
+		 * Workaround:
+		 *
+		 * When we get XRDY interrupt without transmit underflow flag
+		 * (XUDF bit in the I2C_STAT register), delay for 100 microsecs
+		 * and ignore it.
+		 *
+		 * We write data into I2C_DATA register in case of transmit
+		 * underflow condition ONLY.
+		 */
+		if (stat & OMAP_I2C_STAT_XRDY) {
+			if (!(stat & OMAP_I2C_STAT_XUDF)) {
+				udelay(100);
+				continue;
+			} else {
+				w = 0;
+				if (dev->buf_len) {
+					w = *dev->buf++;
+					dev->buf_len--;
+					if (dev->buf_len) {
+						w |= *dev->buf++ << 8;
+						dev->buf_len--;
+					}
+					omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
+				} else {
+					dev_err(dev->dev, "XRDY IRQ while no "
+							"data to send\n");
+					break;
+				}
+				continue;
  			}
-			omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
-		} else
-			dev_err(dev->dev, "XRDY IRQ while no data to send\n");
-		break;
-	default:
-		return IRQ_NONE;
+		}
+		/* REVISIT: Are this checks useful?
+		 * It looks like we will never hit in it... */
+		if (stat & OMAP_I2C_STAT_ROVR) {
+			dev_err(dev->dev, "Receive overrun\n");
+			dev->cmd_err |= OMAP_I2C_STAT_ROVR;
+		}
+		if (stat & OMAP_I2C_STAT_XUDF) {
+			dev_err(dev->dev, "Transmit underflow\n");
+			dev->cmd_err |= OMAP_I2C_STAT_XUDF;
+		}
  	}

-	return IRQ_HANDLED;
+	return count ? IRQ_HANDLED : IRQ_NONE;
  }
  #else
  #define omap_i2c_rev1_isr		NULL
-- 
1.6.0.1