[U-Boot] [PATCH RFC] ARMV7: OMAP: I2C driver: Reduce excessively long udelay calls

Steve Sakoman steve at sakoman.com
Fri Oct 15 18:07:53 CEST 2010


I've been preparing a patch series for Beagle and Overo that detects expansion
board configuration information by reading a 128 byte I2C EEPROM on each
expansion board.

Using the OMAP I2C driver in its current form takes about 5-6 seconds to read
this small number of bytes!

Examining the code I see that there are a large number of fairly long udelay calls
throughout the driver (10 - 50 milliseconds). I looked through the linux driver
and did not see equivalent delays in that code.  In fact the longest delay in the
linux code was one millisecond.

In looking at the TRM I2C section for OMAP3 and OMAP4 I don't see any requirement
for delays in the programming model description.

In a series of test builds I found that (at least on OMAP3 and OMAP4) I could
eliminate just about all of these delays.

I don't have access to earlier OMAP family hardware, so I don't know for certain
that they do or do not require these delays.

This patch takes the conservative approach of shortening all the existing delays
to 1 msec, to match the few Linux driver delays.  It also modifies the driver's
wait_for_bb routine to timeout after 1 second, again to match the Linux driver
behavior.

Tested on OMAP3 and OMAP4 -- the EEPROM read is now instantaneous, as is the i2c probe
command, whaich also used to be extremely slow.

Comments welcome, as well as test results from those who might have access to old OMAP
hardware.

Steve



diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
index 3febd1f..c3a25df 100644
--- a/drivers/i2c/omap24xx_i2c.c
+++ b/drivers/i2c/omap24xx_i2c.c
@@ -27,7 +27,7 @@
 
 #include "omap24xx_i2c.h"
 
-#define I2C_TIMEOUT	10
+#define I2C_TIMEOUT	1000
 
 static void wait_for_bb (void);
 static u16 wait_for_pin (void);
@@ -108,7 +108,7 @@ void i2c_init (int speed, int slaveadd)
 
 	if (readw (&i2c_base->con) & I2C_CON_EN) {
 		writew (0, &i2c_base->con);
-		udelay (50000);
+		udelay (1000);
 	}
 
 	writew(0x2, &i2c_base->sysc); /* for ES2 after soft reset */
@@ -164,7 +164,7 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value)
 	if (status & I2C_STAT_XRDY) {
 		/* Important: have to use byte access */
 		writeb (regoffset, &i2c_base->data);
-		udelay (20000);
+		udelay (1000);
 		if (readw (&i2c_base->stat) & I2C_STAT_NACK) {
 			i2c_error = 1;
 		}
@@ -176,7 +176,7 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value)
 		writew (I2C_CON_EN, &i2c_base->con);
 		while (readw(&i2c_base->stat) &
 			(I2C_STAT_XRDY | I2C_STAT_ARDY)) {
-			udelay (10000);
+			udelay (1000);
 			/* Have to clear pending interrupt to clear I2C_STAT */
 			writew (0xFFFF, &i2c_base->stat);
 		}
@@ -197,7 +197,7 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value)
 #else
 			*value = readw (&i2c_base->data);
 #endif
-			udelay (20000);
+			udelay (1000);
 		} else {
 			i2c_error = 1;
 		}
@@ -206,7 +206,7 @@ static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value)
 			writew (I2C_CON_EN, &i2c_base->con);
 			while (readw (&i2c_base->stat) &
 				(I2C_STAT_RRDY | I2C_STAT_ARDY)) {
-				udelay (10000);
+				udelay (1000);
 				writew (0xFFFF, &i2c_base->stat);
 			}
 		}
@@ -256,7 +256,7 @@ static int i2c_write_byte (u8 devaddr, u8 regoffset, u8 value)
 		writew ((value << 8) + regoffset, &i2c_base->data);
 #endif
 		/* must have enough delay to allow BB bit to go low */
-		udelay (50000);
+		udelay (1000);
 		if (readw (&i2c_base->stat) & I2C_STAT_NACK) {
 			i2c_error = 1;
 		}
@@ -322,7 +322,7 @@ int i2c_probe (uchar chip)
 	/* stop bit needed here */
 	writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, &i2c_base->con);
 	/* enough delay for the NACK bit set */
-	udelay (50000);
+	udelay (1000);
 
 	if (!(readw (&i2c_base->stat) & I2C_STAT_NACK)) {
 		res = 0;      /* success case */
@@ -331,7 +331,7 @@ int i2c_probe (uchar chip)
 	} else {
 		writew(0xFFFF, &i2c_base->stat);	 /* failue, clear sources*/
 		writew (readw (&i2c_base->con) | I2C_CON_STP, &i2c_base->con); /* finish up xfer */
-		udelay(20000);
+		udelay(1000);
 		wait_for_bb ();
 	}
 	flush_fifo();
@@ -392,13 +392,13 @@ int i2c_write (uchar chip, uint addr, int alen, uchar * buffer, int len)
 
 static void wait_for_bb (void)
 {
-	int timeout = 10;
+	int timeout = I2C_TIMEOUT;
 	u16 stat;
 
 	writew(0xFFFF, &i2c_base->stat);	 /* clear current interruts...*/
 	while ((stat = readw (&i2c_base->stat) & I2C_STAT_BB) && timeout--) {
 		writew (stat, &i2c_base->stat);
-		udelay (50000);
+		udelay (1000);
 	}
 
 	if (timeout <= 0) {
@@ -411,7 +411,7 @@ static void wait_for_bb (void)
 static u16 wait_for_pin (void)
 {
 	u16 status;
-	int timeout = 10;
+	int timeout = I2C_TIMEOUT;
 
 	do {
 		udelay (1000);




More information about the U-Boot mailing list