Re: [U-Boot] [PATCH 3/8] ARMV7: Modify i2c driver for more reliable operation on OMAP4

Steve, just minor nitpick below:

On 07/21/2010 02:25 PM, Steve Sakoman wrote:

In addition to modifying the init routine to follow the TRM recommendations, this patch adds a retry count to two loops to avoid the possibility of infinite loops. It also corrects error message typos in the i2c_write routine.

Signed-off-by: Steve Sakomansteve@sakoman.com

arch/arm/include/asm/arch-omap3/i2c.h | 4 ++- drivers/i2c/omap24xx_i2c.c | 37 ++++++++++++++++++++++---------- drivers/i2c/omap24xx_i2c.h | 4 +++ 3 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/arch/arm/include/asm/arch-omap3/i2c.h b/arch/arm/include/asm/arch-omap3/i2c.h index 7a4a73a..d2e7488 100644 --- a/arch/arm/include/asm/arch-omap3/i2c.h +++ b/arch/arm/include/asm/arch-omap3/i2c.h @@ -34,7 +34,9 @@ struct i2c { unsigned short stat; /* 0x08 */ unsigned short res3; unsigned short iv; /* 0x0C */

• unsigned short res4[3];

• unsigned short res4;
• unsigned short syss; /* 0x10 */
• unsigned short res4a; unsigned short buf; /* 0x14 */ unsigned short res5; unsigned short cnt; /* 0x18 */

diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c index 3256133..a91fe55 100644 --- a/drivers/i2c/omap24xx_i2c.c +++ b/drivers/i2c/omap24xx_i2c.c @@ -41,6 +41,7 @@ void i2c_init (int speed, int slaveadd) int psc, fsscll, fssclh; int hsscll = 0, hssclh = 0; u32 scll, sclh;

• int reset_timeout = 10;

I am guessing we can life with a u8, we timeout in 10ms.. would asking for a #define I2C_TIMEOUT_RESET is too much here?

/* Only handle standard, fast and high speeds */ if ((speed != OMAP_I2C_STANDARD)&& @@ -102,15 +103,22 @@ void i2c_init (int speed, int slaveadd) sclh = (unsigned int)fssclh; }

• writew(0x2,&i2c_base->sysc); /* for ES2 after soft reset */
• udelay(1000);
• writew(0x0,&i2c_base->sysc); /* will probably self clear but */
• if (readw (&i2c_base->con)& I2C_CON_EN) { writew (0,&i2c_base->con); udelay (50000); }

• writew(0x2,&i2c_base->sysc); /* for ES2 after soft reset */
• udelay(1000);
• writew(I2C_CON_EN,&i2c_base->con);
• while (!(readw(&i2c_base->syss)& I2C_SYSS_RDONE)&& reset_timeout--) {

• if (reset_timeout<= 0)
  

• 	printf("ERROR: Timeout in soft-reset\n");
  

• udelay(1000);
  

• }
• writew(0,&i2c_base->con); writew(psc,&i2c_base->psc); writew(scll,&i2c_base->scll); writew(sclh,&i2c_base->sclh);

@@ -134,6 +142,7 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value) { int i2c_error = 0; u16 status;

• u16 retries;

u8 sounds good here - this does not seem to take more than 10..

/* wait until bus not busy */ wait_for_bb (); @@ -159,15 +168,16 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value) }

if (!i2c_error) {

• /* free bus, otherwise we can't use a combined transction */
  

• writew (0,&i2c_base->con);
  

• while (readw (&i2c_base->stat) || (readw (&i2c_base->con)&  I2C_CON_MST)) {
  

• retries = 10;
  

#define sounds nice here..

• while ((readw(&i2c_base->stat)&  0x14) ||
  

• 	(readw(&i2c_base->con)&  I2C_CON_MST)) {
  udelay (10000);
  /* Have to clear pending interrupt to clear I2C_STAT */
  writew (0xFFFF,&i2c_base->stat);
  

• 	if (!retries--)
  

• 		break;
  

  }

do we just proceed in case of retry timeout?

• wait_for_bb ();
  

  /* set slave address */ writew (devaddr,&i2c_base->sa); /* read one byte from slave */

@@ -190,11 +200,14 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value) }

if (!i2c_error) {

• 	retries = 10;
  writew (I2C_CON_EN,&i2c_base->con);
  while (readw (&i2c_base->stat)
         || (readw (&i2c_base->con)&  I2C_CON_MST)) {
  	udelay (10000);
  	writew (0xFFFF,&i2c_base->stat);
  

• 		if (!retries--)
  

• 			break;
  }
  

same comment on retry failure here..

}

} @@ -276,7 +289,7 @@ static void flush_fifo(void) * you get a bus error */ while(1){

• stat = readw(&i2c_base->stat);
  

• stat = readw(&i2c_base->stat)&  I2C_STAT_RRDY;
  

  if(stat == I2C_STAT_RRDY){ #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \ defined(CONFIG_OMAP44XX)

@@ -357,18 +370,18 @@ int i2c_write (uchar chip, uint addr, int alen, uchar * buffer, int len) int i;

if (alen> 1) {

• printf ("I2C read: addr len %d not supported\n", alen);
  

• printf("I2C write: addr len %d not supported\n", alen);
  

err.. do we need this change?

return 1;

}

if (addr + len> 256) {

• printf ("I2C read: address out of range\n");
  

• printf("I2C write: address out of range\n");
  

err.. do we need this change?

return 1;

}

for (i = 0; i< len; i++) { if (i2c_write_byte (chip, addr + i, buffer[i])) {

• 	printf ("I2C read: I/O error\n");
  

• 	printf("I2C write: I/O error\n");
  

err.. do we need this change?

	i2c_init (CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
	return 1;
}

diff --git a/drivers/i2c/omap24xx_i2c.h b/drivers/i2c/omap24xx_i2c.h index 92a3416..650e33a 100644 --- a/drivers/i2c/omap24xx_i2c.h +++ b/drivers/i2c/omap24xx_i2c.h @@ -85,6 +85,10 @@ #define I2C_SYSTEST_SDA_I (1<< 1) /* SDA line sense input value */ #define I2C_SYSTEST_SDA_O (1<< 0) /* SDA line drive output value */

+/* I2C System Status Register (I2C_SYSS): */

+#define I2C_SYSS_RDONE (1<< 0) /* Internel reset monitoring */

• #define I2C_SCLL_SCLL 0 #define I2C_SCLL_SCLL_M 0xFF #define I2C_SCLL_HSSCLL 8

Regards, Nishanth Menon