[U-Boot] Patches to add some more omap24xx_i2c/twl6035 error handling

Hi,
I am encountering the following i2c error on OMAP5 with mainline u-boot:
OMAP5430 EVM # mmc rescan timed out in wait_for_bb: I2C_STAT=1410
It seems the first call to i2c_write for bus I2C1 will fail and will leave the bus with SCL stuck low, preventing further i2c operations.
While still debugging this issue, I would like to propose the following patches, which add some more error handling in this error case already:
[PATCH 1/2] omap24xx_i2c: Handle wait_for_bb error [PATCH 2/2] power: twl6035: complain on LDO9 error
Best regards,
V.

We add a return code to wait_for_bb() to be able to report errors to the callers properly. We in turn handle this new error code in i2c_read, i2c_write and i2c_probe.
Signed-off-by: Vincent Stehlé v-stehle@ti.com --- drivers/i2c/omap24xx_i2c.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c index 094305f..1bbb2ca 100644 --- a/drivers/i2c/omap24xx_i2c.c +++ b/drivers/i2c/omap24xx_i2c.c @@ -31,7 +31,7 @@ DECLARE_GLOBAL_DATA_PTR;
#define I2C_TIMEOUT 1000
-static void wait_for_bb(void); +static int wait_for_bb(void); static u16 wait_for_pin(void); static void flush_fifo(void);
@@ -159,7 +159,8 @@ static int i2c_read_byte(u8 devaddr, u16 regoffset, u8 alen, u8 *value) u16 w;
/* wait until bus not busy */ - wait_for_bb(); + if (wait_for_bb()) + return 1;
/* one byte only */ writew(alen, &i2c_base->cnt); @@ -260,7 +261,8 @@ int i2c_probe(uchar chip) return res;
/* wait until bus not busy */ - wait_for_bb(); + if (wait_for_bb()) + return res;
/* try to read one byte */ writew(1, &i2c_base->cnt); @@ -279,7 +281,10 @@ int i2c_probe(uchar chip) res = 1; writew(0xff, &i2c_base->stat); writew (readw (&i2c_base->con) | I2C_CON_STP, &i2c_base->con); - wait_for_bb (); + + if (wait_for_bb()) + res = 1; + break; } if (status & I2C_STAT_ARDY) { @@ -351,7 +356,8 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len) }
/* wait until bus not busy */ - wait_for_bb(); + if (wait_for_bb()) + return 1;
/* start address phase - will write regoffset + len bytes data */ /* TODO consider case when !CONFIG_OMAP243X/34XX/44XX */ @@ -394,7 +400,7 @@ write_exit: return i2c_error; }
-static void wait_for_bb(void) +static int wait_for_bb(void) { int timeout = I2C_TIMEOUT; u16 stat; @@ -408,8 +414,10 @@ static void wait_for_bb(void) if (timeout <= 0) { printf("timed out in wait_for_bb: I2C_STAT=%x\n", readw(&i2c_base->stat)); + return 1; } writew(0xFFFF, &i2c_base->stat); /* clear delayed stuff*/ + return 0; }
static u16 wait_for_pin(void)

On Mon, Dec 03, 2012 at 05:23:16AM -0000, Vincent Stehlé wrote:
We add a return code to wait_for_bb() to be able to report errors to the callers properly. We in turn handle this new error code in i2c_read, i2c_write and i2c_probe.
Signed-off-by: Vincent Stehlé v-stehle@ti.com
Applied to u-boot-ti/master, thanks!

We handle i2c_write return code and complain in case of error. We propagate the error, too, to allow better handling at the upper level in the future.
Signed-off-by: Vincent Stehlé v-stehle@ti.com --- drivers/power/twl6035.c | 17 +++++++++++++---- include/twl6035.h | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/power/twl6035.c b/drivers/power/twl6035.c index 624c09e..d3de698 100644 --- a/drivers/power/twl6035.c +++ b/drivers/power/twl6035.c @@ -50,16 +50,25 @@ void twl6035_init_settings(void) return; }
-void twl6035_mmc1_poweron_ldo(void) +int twl6035_mmc1_poweron_ldo(void) { u8 val = 0;
/* set LDO9 TWL6035 to 3V */ val = 0x2b; /* (3 -.9)*28 +1 */ - palmas_write_u8(0x48, LDO9_VOLTAGE, val); + + if (palmas_write_u8(0x48, LDO9_VOLTAGE, val)) { + printf("twl6035: could not set LDO9 voltage.\n"); + return 1; + }
/* TURN ON LDO9 */ val = LDO_ON | LDO_MODE_SLEEP | LDO_MODE_ACTIVE; - palmas_write_u8(0x48, LDO9_CTRL, val); - return; + + if (palmas_write_u8(0x48, LDO9_CTRL, val)) { + printf("twl6035: could not turn on LDO9.\n"); + return 1; + } + + return 0; } diff --git a/include/twl6035.h b/include/twl6035.h index e21ddba..ce74348 100644 --- a/include/twl6035.h +++ b/include/twl6035.h @@ -39,4 +39,4 @@ int twl6035_i2c_write_u8(u8 chip_no, u8 val, u8 reg); int twl6035_i2c_read_u8(u8 chip_no, u8 *val, u8 reg); void twl6035_init_settings(void); -void twl6035_mmc1_poweron_ldo(void); +int twl6035_mmc1_poweron_ldo(void);

On Mon, Dec 03, 2012 at 05:23:17AM -0000, Vincent Stehlé wrote:
We handle i2c_write return code and complain in case of error. We propagate the error, too, to allow better handling at the upper level in the future.
Signed-off-by: Vincent Stehlé v-stehle@ti.com
Applied to u-boot-ti/master, thanks!

OMAP5 has 8b i2c data register field, like OMAP2, 3 and 4. Handle in the same way. This fixes the following error on OMAP5:
OMAP5430 EVM # mmc rescan timed out in wait_for_bb: I2C_STAT=1410 twl6035: could not turn on LDO9.
Signed-off-by: Vincent Stehlé v-stehle@ti.com --- drivers/i2c/omap24xx_i2c.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c index 1bbb2ca..54e9b15 100644 --- a/drivers/i2c/omap24xx_i2c.c +++ b/drivers/i2c/omap24xx_i2c.c @@ -180,7 +180,8 @@ static int i2c_read_byte(u8 devaddr, u16 regoffset, u8 alen, u8 *value) if (status & I2C_STAT_XRDY) { w = tmpbuf[i++]; #if !(defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \ - defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX)) + defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \ + defined(CONFIG_OMAP54XX)) w |= tmpbuf[i++] << 8; #endif writew(w, &i2c_base->data); @@ -210,7 +211,8 @@ static int i2c_read_byte(u8 devaddr, u16 regoffset, u8 alen, u8 *value) } if (status & I2C_STAT_RRDY) { #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \ - defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) + defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \ + defined(CONFIG_OMAP54XX) *value = readb(&i2c_base->data); #else *value = readw(&i2c_base->data); @@ -240,7 +242,8 @@ static void flush_fifo(void) stat = readw(&i2c_base->stat); if (stat == I2C_STAT_RRDY) { #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \ - defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) + defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \ + defined(CONFIG_OMAP54XX) readb(&i2c_base->data); #else readw(&i2c_base->data); @@ -294,7 +297,8 @@ int i2c_probe(uchar chip) if (status & I2C_STAT_RRDY) { res = 0; #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \ - defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) + defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \ + defined(CONFIG_OMAP54XX) readb(&i2c_base->data); #else readw(&i2c_base->data); @@ -382,7 +386,8 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len) if (status & I2C_STAT_XRDY) { w = (i < 0) ? tmpbuf[2+i] : buffer[i]; #if !(defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \ - defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX)) + defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX) || \ + defined(CONFIG_OMAP54XX)) w |= ((++i < 0) ? tmpbuf[2+i] : buffer[i]) << 8; #endif writew(w, &i2c_base->data);
participants (2)
-
Tom Rini
-
Vincent Stehlé