Tony, > -----Original Message----- > From: Tony Lindgren [mailto:tony@xxxxxxxxxxx] > Sent: Thursday, April 22, 2010 3:13 AM > To: G, Manjunath Kondaiah > Cc: linux-i2c@xxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; > ben-linux@xxxxxxxxx; Kalliguddi, Hema; Menon, Nishanth > Subject: Re: [PATCH v2] OMAP3: I2C: Errata ID i207: Clear > wrong RDR interrupt > > * Manjunatha GK <manjugk@xxxxxx> [100413 06:32]: > > 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 <manjugk@xxxxxx> > > Cc: linux-omap@xxxxxxxxxxxxxxx > > Cc: linux-i2c@xxxxxxxxxxxxxxx > > Cc: ben-linux@xxxxxxxxx > > Cc: Kalliguddi, Hema <hemahk@xxxxxx> > > Cc: Nishanth Menon <nm@xxxxxx> > > --- > > Review comments for earlier post can be found at: > > https://patchwork.kernel.org/patch/90122/ > > > > 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; > > + } > > + > > + } > > + else > > + return IRQ_HANDLED; > > + } > > Please move these hacks into a separate inline function that > you initialize > during the init. Then in the function you do it if > OMAP_I2C_QUIRK_1234 flag > has been set. > > In general, it seems that a lot of TI omap4 patches just do: > > if (!cpu_is_omap4430()) { > indent_old_code_even_more > ... > } else { > add_hacks_for_omap4 > ... > } > > And sprinkle that all over the place. > > This is not a maintainable way of doing things. Please do something > like this instead: > > #ifdef CONFIG_ARCH_OMAP4 > static inline void omap_i2c_quirk_1234(struct device *dev) > { > /* Do hacks here */ > } > #else > static inline void omap_i2c_quirk_1234(struct device *dev) > { > } > #endif This might create issue for multi omap build? -Manjunath-- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html