RE: [PATCH v2] OMAP3: I2C: Errata ID i207: Clear wrong RDR interrupt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux