I2C: Fix twl4030 timeouts on omap3430

PATCH In review Tony Lindgren 2 participants 7 messages Mar 28, 2008 – Apr 1, 2008

lore

Share & tools

lore

Tony Lindgren

author

2008-03-28 08:41 UTC #0 lore raw

PATCH 1/1 I2C: Fix twl4030 timeouts on omap3430

Hi all,

This helps with the annoying I2C timeouts. Does anybody have an idea why the twl4030 chip does not like doing multiple transfers in a row?

To me the only difference seems to be that clocks are idled between writing the twl4030 register and reading the register value.

Regards,

Tony

Quoted 1 line

From 25c4c8f449819cb6f40b59ed1a9b25ebcc7cd72e Mon Sep 17 00:00:00 2001

From: Tony Lindgren <[email protected]>
Date: Thu, 27 Mar 2008 19:05:30 +0200
Subject: [PATCH] I2C: Fix twl4030 timeouts on omap3430

For some reason doing a twl4030 write-read cycle can hang the I2C bus on omap3430. And doing the write and read separately in twl4030_i2c_read() seems to fix the problem...

Not intended for applying, just a temporary workaround.

drivers/i2c/chips/twl4030-core.c +9 -1

index ded86e7..62868b0 100644
@@ -327,6 +327,7 @@ int twl4030_i2c_read(u8 mod_no, u8 * value, u8 reg, u8 num_bytes)
 		return -EPERM;
 	}
 	mutex_lock(&twl->xfer_lock);
+
 	/* [MSG1] fill the register address data */
 	msg = &twl->xfer_msg[0];
 	msg->addr = twl->address;

@@ -334,18 +335,25 @@ int twl4030_i2c_read(u8 mod_no, u8 * value, u8 reg, u8 num_bytes)
 	msg->flags = 0;	/* Read the register value */
 	val = twl4030_map[mod_no].base + reg;
 	msg->buf = &val;
+	ret = i2c_transfer(twl->client.adapter, twl->xfer_msg, 1);
+	if (ret < 0)
+		goto out;
+ 
 	/* [MSG2] fill the data rx buffer */
 	msg = &twl->xfer_msg[1];
 	msg->addr = twl->address;
 	msg->flags = I2C_M_RD;	/* Read the register value */
 	msg->len = num_bytes;	/* only n bytes */
 	msg->buf = value;
-	ret = i2c_transfer(twl->client.adapter, twl->xfer_msg, 2);
+	ret = i2c_transfer(twl->client.adapter, twl->xfer_msg, 1);
+
+out:
 	mutex_unlock(&twl->xfer_lock);
 
 	/* i2cTransfer returns num messages.translate it pls.. */
 	if (ret >= 0)
 		ret = 0;
+
 	return ret;
 }

Tony Lindgren

author

2008-03-31 10:43 UTC #1 lore raw

Quoted 7 lines · Tony Lindgren

Hi all,
This helps with the annoying I2C timeouts. Does anybody have an idea why the twl4030 chip does not like doing multiple transfers in a row?
To me the only difference seems to be that clocks are idled between writing the twl4030 register and reading the register value.

I'll push this today with a REVISIT comment added.

Tony

Quoted 50 lines

From 25c4c8f449819cb6f40b59ed1a9b25ebcc7cd72e Mon Sep 17 00:00:00 2001
From: Tony Lindgren <[email protected]>
Date: Thu, 27 Mar 2008 19:05:30 +0200
Subject: [PATCH] I2C: Fix twl4030 timeouts on omap3430

For some reason doing a twl4030 write-read cycle can hang the I2C bus on omap3430. And doing the write and read separately in twl4030_i2c_read() seems to fix the problem...

Not intended for applying, just a temporary workaround.

drivers/i2c/chips/twl4030-core.c +9 -1

index ded86e7..62868b0 100644
@@ -327,6 +327,7 @@ int twl4030_i2c_read(u8 mod_no, u8 * value, u8 reg, u8 num_bytes)
 		return -EPERM;
 	}
 	mutex_lock(&twl->xfer_lock);
+
 	/* [MSG1] fill the register address data */
 	msg = &twl->xfer_msg[0];
 	msg->addr = twl->address;

@@ -334,18 +335,25 @@ int twl4030_i2c_read(u8 mod_no, u8 * value, u8 reg, u8 num_bytes)
 	msg->flags = 0;	/* Read the register value */
 	val = twl4030_map[mod_no].base + reg;
 	msg->buf = &val;
+	ret = i2c_transfer(twl->client.adapter, twl->xfer_msg, 1);
+	if (ret < 0)
+		goto out;
+ 
 	/* [MSG2] fill the data rx buffer */
 	msg = &twl->xfer_msg[1];
 	msg->addr = twl->address;
 	msg->flags = I2C_M_RD;	/* Read the register value */
 	msg->len = num_bytes;	/* only n bytes */
 	msg->buf = value;
-	ret = i2c_transfer(twl->client.adapter, twl->xfer_msg, 2);
+	ret = i2c_transfer(twl->client.adapter, twl->xfer_msg, 1);
+
+out:
 	mutex_unlock(&twl->xfer_lock);
 
 	/* i2cTransfer returns num messages.translate it pls.. */
 	if (ret >= 0)
 		ret = 0;
+
 	return ret;
 }

Tony Lindgren

author

2008-03-31 14:30 UTC #2 lore raw

Quoted 10 lines · Tony Lindgren

Quoted 7 lines · Tony Lindgren
Hi all,
This helps with the annoying I2C timeouts. Does anybody have an idea why the twl4030 chip does not like doing multiple transfers in a row?
To me the only difference seems to be that clocks are idled between writing the twl4030 register and reading the register value.
I'll push this today with a REVISIT comment added.

Looks like this kills twl4030 interrupts, so I've reverted it.

Tony

Quoted 51 lines

Quoted 50 lines

For some reason doing a twl4030 write-read cycle can hang the I2C bus on omap3430. And doing the write and read separately in twl4030_i2c_read() seems to fix the problem...

Not intended for applying, just a temporary workaround.

drivers/i2c/chips/twl4030-core.c +9 -1

index ded86e7..62868b0 100644
@@ -327,6 +327,7 @@ int twl4030_i2c_read(u8 mod_no, u8 * value, u8 reg, u8 num_bytes)
 		return -EPERM;
 	}
 	mutex_lock(&twl->xfer_lock);
+
 	/* [MSG1] fill the register address data */
 	msg = &twl->xfer_msg[0];
 	msg->addr = twl->address;

@@ -334,18 +335,25 @@ int twl4030_i2c_read(u8 mod_no, u8 * value, u8 reg, u8 num_bytes)
 	msg->flags = 0;	/* Read the register value */
 	val = twl4030_map[mod_no].base + reg;
 	msg->buf = &val;
+	ret = i2c_transfer(twl->client.adapter, twl->xfer_msg, 1);
+	if (ret < 0)
+		goto out;
+ 
 	/* [MSG2] fill the data rx buffer */
 	msg = &twl->xfer_msg[1];
 	msg->addr = twl->address;
 	msg->flags = I2C_M_RD;	/* Read the register value */
 	msg->len = num_bytes;	/* only n bytes */
 	msg->buf = value;
-	ret = i2c_transfer(twl->client.adapter, twl->xfer_msg, 2);
+	ret = i2c_transfer(twl->client.adapter, twl->xfer_msg, 1);
+
+out:
 	mutex_unlock(&twl->xfer_lock);
 
 	/* i2cTransfer returns num messages.translate it pls.. */
 	if (ret >= 0)
 		ret = 0;
+
 	return ret;
 }

Tony Lindgren

author

2008-04-01 12:43 UTC #3 lore raw

Quoted 7 lines · Tony Lindgren

Quoted 6 lines · Tony Lindgren
Quoted 5 lines · Tony Lindgren
Hi all,
This helps with the annoying I2C timeouts. Does anybody have an idea why the twl4030 chip does not like doing multiple transfers in a row?

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
To me the only difference seems to be that clocks are idled between writing the twl4030 register and reading the register value.
I'll push this today with a REVISIT comment added.
Looks like this kills twl4030 interrupts, so I've reverted it.

After looking into this problem a bit more, looks like twl4030 reads to anything in "POWER ID" (modules 0x10 and higher) will hang twl4030 eventually and I2C controller gets stuck in mode where STP never clears.

Repeated reads to "USB ID", "AUD ID" or "AUX ID" will not hang twl4030.

Tony

Peter 'p2' De Schrijver

2008-04-01 13:01 UTC #4 lore raw

Quoted 13 lines · ext Tony Lindgren

Quoted 12 lines · Tony Lindgren
Quoted 10 lines · Tony Lindgren
Quoted 7 lines · Tony Lindgren
Hi all,
This helps with the annoying I2C timeouts. Does anybody have an idea why the twl4030 chip does not like doing multiple transfers in a row?
To me the only difference seems to be that clocks are idled between writing the twl4030 register and reading the register value.
I'll push this today with a REVISIT comment added.

Quoted 8 lines

Quoted 1 line
Looks like this kills twl4030 interrupts, so I've reverted it.
After looking into this problem a bit more, looks like twl4030 reads to anything in "POWER ID" (modules 0x10 and higher) will hang twl4030 eventually and I2C controller gets stuck in mode where STP never clears.
Repeated reads to "USB ID", "AUD ID" or "AUX ID" will not hang twl4030.

I remember seeing something similar when doing the powerbutton code. Klaus Pedersen found out that leaving CFG_BOOT to its reset value solved the problem. Unfortunately this breaks MADC and USB afaics, so it's not a real solution. CFG_BOOT is programmed in power_companion_init().

Cheers,

Peter.

Signature · 1 line

goa is a state of mind

Tony Lindgren

author

2008-04-01 13:38 UTC #5 lore raw

Quoted 20 lines · * Peter 'p2' De Schrijver <[email protected]> [080401 16:01]:

Quoted 19 lines · ext Tony Lindgren
Quoted 7 lines · Tony Lindgren
Quoted 6 lines · Tony Lindgren
Quoted 5 lines · Tony Lindgren
Hi all,
This helps with the annoying I2C timeouts. Does anybody have an idea why the twl4030 chip does not like doing multiple transfers in a row?
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
To me the only difference seems to be that clocks are idled between writing the twl4030 register and reading the register value.
I'll push this today with a REVISIT comment added.
Looks like this kills twl4030 interrupts, so I've reverted it.
After looking into this problem a bit more, looks like twl4030 reads to anything in "POWER ID" (modules 0x10 and higher) will hang twl4030 eventually and I2C controller gets stuck in mode where STP never clears.

Quoted 7 lines

Quoted 2 lines
Repeated reads to "USB ID", "AUD ID" or "AUX ID" will not hang twl4030.
I remember seeing something similar when doing the powerbutton code. Klaus Pedersen found out that leaving CFG_BOOT to its reset value solved the problem. Unfortunately this breaks MADC and USB afaics, so it's not a real solution. CFG_BOOT is programmed in power_companion_init().

Great, this helped a lot! Looks like power_companion_init() tries to get osc_ck without checking for clkg_get() return value, and osc_ck does not exist in clock34xx.h. So R_CFG_BOOT gets set to default 26MHz value :)

Will post a patch when available.

Tony

Tony Lindgren

author

2008-04-01 14:05 UTC #6 lore raw

Quoted 29 lines · Tony Lindgren

Quoted 20 lines · * Peter 'p2' De Schrijver <[email protected]> [080401 16:01]:
Quoted 19 lines · ext Tony Lindgren
Quoted 7 lines · Tony Lindgren
Quoted 6 lines · Tony Lindgren
Quoted 5 lines · Tony Lindgren
Hi all,
This helps with the annoying I2C timeouts. Does anybody have an idea why the twl4030 chip does not like doing multiple transfers in a row?
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
To me the only difference seems to be that clocks are idled between writing the twl4030 register and reading the register value.
I'll push this today with a REVISIT comment added.
Looks like this kills twl4030 interrupts, so I've reverted it.
After looking into this problem a bit more, looks like twl4030 reads to anything in "POWER ID" (modules 0x10 and higher) will hang twl4030 eventually and I2C controller gets stuck in mode where STP never clears.
Quoted 7 lines
Quoted 2 lines
Repeated reads to "USB ID", "AUD ID" or "AUX ID" will not hang twl4030.
I remember seeing something similar when doing the powerbutton code. Klaus Pedersen found out that leaving CFG_BOOT to its reset value solved the problem. Unfortunately this breaks MADC and USB afaics, so it's not a real solution. CFG_BOOT is programmed in power_companion_init().

Quoted 5 lines

Great, this helped a lot! Looks like power_companion_init() tries to get osc_ck without checking for clkg_get() return value, and osc_ck does not exist in clock34xx.h. So R_CFG_BOOT gets set to default 26MHz value :)
Will post a patch when available.

Thanks again, here's a patch, I've pushed it also.

Tony

From: Tony Lindgren <[email protected]>
Date: Tue, 1 Apr 2008 16:58:02 +0300
Subject: [PATCH] I2C: Fix twl4030 clock init

Without this patch twl4030 clock can get programmed to incorrect rate which can eventually hang twl4030 reads. Also minor formatting fixes.

Signed-off-by Tony Lindgren <[email protected]>

drivers/i2c/chips/twl4030-core.c +12 -3

@@ -672,11 +672,20 @@ static int power_companion_init(void)
 	u32 rate, ctrl = HFCLK_FREQ_26_MHZ;
 	int e = 0;
 
-	osc = clk_get(NULL,"osc_ck");
+	if (cpu_is_omap2430())
+		osc = clk_get(NULL, "osc_ck");
+	else
+		osc = clk_get(NULL, "osc_sys_ck");
+	if (IS_ERR(osc)) {
+		printk(KERN_ERR "Skipping twl3040 internal clock init and "
+				"using bootloader value (unknown osc rate)\n");
+		return 0;
+	}
+
 	rate = clk_get_rate(osc);
 	clk_put(osc);
 
-	switch(rate) {
+	switch (rate) {
 		case 19200000 : ctrl = HFCLK_FREQ_19p2_MHZ; break;
 		case 26000000 : ctrl = HFCLK_FREQ_26_MHZ; break;
 		case 38400000 : ctrl = HFCLK_FREQ_38p4_MHZ; break;

@@ -684,7 +693,7 @@ static int power_companion_init(void)
 
 	ctrl |= HIGH_PERF_SQ;
 	e |= unprotect_pm_master();
-			/* effect->MADC+USB ck en */
+	/* effect->MADC+USB ck en */
 	e |= twl4030_i2c_write_u8(TWL4030_MODULE_PM_MASTER, ctrl, R_CFG_BOOT);
 	e |= protect_pm_master();