> -----Original Message-----
> From: Menon, Nishanth
> Sent: Tuesday, April 13, 2010 10:18 PM
> To: G, Manjunath Kondaiah
> Cc: [email protected]; [email protected];
> [email protected]; Kalliguddi, Hema
> Subject: Re: [PATCH v2] OMAP3: I2C: Errata ID i207: Clear
> wrong RDR interrupt
>
> On 04/13/2010 07:01 AM, G, Manjunath Kondaiah wrote:
> > Under certain rare conditions, I2C_STAT[13].RDR bit may be set
> > and the corresponding interrupt fire, even there is no data in
> > the receive FIFO, or the I2C data transfer is still ongoing.
> > These spurious RDR events must be ignored by the software.
> >
> > This patch handles and ignores RDR spurious interrupts.
> >
> > Patch tested on OMAP zoom3 board.
> >
> > Signed-off-by: Manjunatha GK<[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: Kalliguddi, Hema<[email protected]>
> > Cc: Nishanth Menon<[email protected]>
> > ---
> > Review comments for earlier post can be found at:
> > https://patchwork.kernel.org/patch/90122/
> Overall, the comments have not been implemented, so my NAK continues
> unfortunately. please review the comments again.
This errata is applicable to OMAP2 and OMAP3 and not applicable to
OMAP4. I will update the same in patch description.
Rest of review comments will be taken care as explained below.
>
> >
> > drivers/i2c/busses/i2c-omap.c | 32
> +++++++++++++++++++++++++++++++-
> > 1 files changed, 31 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-omap.c
> b/drivers/i2c/busses/i2c-omap.c
> > index ae6f5c1..d4ec886 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -733,10 +733,40 @@ complete:
> > }
> > if (stat& (OMAP_I2C_STAT_RRDY | OMAP_I2C_STAT_RDR)) {
> > u8 num_bytes = 1;
> > +
> > + /*
> > + * I2C Errata(Errata Nos. OMAP2: 1.67,
> OMAP3: 1.8)
> > + * Not applicable for OMAP4.
> > + * Under certain rare conditions, RDR
> could be set again
> > + * when the bus is busy, then ignore
> the interrupt and
> > + * clear the interrupt.
> > + */
> > + if ((stat& OMAP_I2C_STAT_RDR)&&
> !cpu_is_omap44xx()) {
> > + /* Step 1: If RDR is set, clear it */
> > + omap_i2c_ack_stat(dev, stat&
> OMAP_I2C_STAT_RDR);
> > +
> > + /* Step 2: */
> > + if(!(omap_i2c_read_reg(dev,
> OMAP_I2C_STAT_REG)
> > + &
> OMAP_I2C_STAT_BB)) {
> > + /* Step 3: */
> > + while(omap_i2c_read_reg(dev,
> > + OMAP_I2C_STAT_REG)
> > + &
> OMAP_I2C_STAT_RDR) {
> > +
> omap_i2c_ack_stat(dev, stat
> > + &
> OMAP_I2C_STAT_RDR);
> > + dev_err(dev->dev,
> > + "I2C : RDR when
> the bus is busy.\n");
> > + continue;
> continue in the inner while loop? NAK. should have been an if
> condition
> here.
It's my bad. I will fix it.
>
> > + }
> > +
> > + }
> > + else
> > + return IRQ_HANDLED;
> using a continue is better as commented for patch v1.
Agreed.
>
> > + }
> > if (dev->fifo_size) {
> > if (stat& OMAP_I2C_STAT_RRDY)
> > num_bytes = dev->fifo_size;
> > - else /* read RXSTAT on RDR
> interrupt */
> > + else /* Step4: read RXSTAT on
> RDR interrupt */
> dont really need this..
I have changed only comment. I will revert if it is not required.
-Manjunath--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
