Skip to contentSkip to conversation

kernel oops for 3430sdp

PATCH In review Aguirre Rodriguez, Sergio Alberto 6 participants 15 messages Oct 7, 2008 – Oct 10, 2008

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

Hi all,

I attempted to run a kernel image in my 3430sdp compiled with the latest commit. I performed these steps:

1.- Loaded omap_3430sdp_defconfig
2.- Entered menuconfig and I added the option:

Device Drivers --->

	GPIO support --->
		<*>   TWL4030/TPS659x0 GPIO Driver

3.- compiled it and loaded in my 3430SDP with u-boot

4.- Booted and got a kernel Oops:

<6>musb_hdrc: version 6.0, musb-dma, otg (peripheral+host), debug=0 <1>Unable to handle kernel NULL pointer dereference at virtual address 00000000 <1>pgd = c0004000
<1>[00000000] *pgd=00000000

Internal error: Oops: 5 [#1]
Modules linked in:
CPU: 0    Not tainted  (2.6.27-rc8-omap1-05072-gf9ceca7 #6)
PC is at musb_platform_init+0x24/0x120
LR is at 0x0
pc : [<c001df5c>]    lr : [<00000000>]    psr: a0000013

sp : c681dd50 ip : c68281b4 fp : c681dd64
r10: c001d288 r9 : 00000000 r8 : c0374b08
r7 : c037950c r6 : c68280d8 r5 : 00000000 r4 : c68280d8 r3 : c035fba4 r2 : 00000000 r1 : c681dcf8 r0 : 00000000
Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 00c5387f Table: 80004018 DAC: 00000017 Process swapper (pid: 1, stack limit = 0xc681c2e0)

Stack: (0xc681dd50 to 0xc681e000)

dd40: c0362d34 00000000 c681de44 c681dd68
dd60: c001d634 c001df44 00000000 c036d050 c681dd9c c681dd80 c029c10c c0362b90
dd80: c0362b98 0000005c d80ab000 c0362cb8 c681ddac c681dda0 c029c16c c029c07c
dda0: c681ddec c681ddb0 c00dd55c c029c168 c00dd294 c00dd170 c68621b8 c6862158
ddc0: c681ddf0 c6832548 c681ddec 00000000 c6862158 c681ddf0 c6832548 00000001
dde0: c681de24 c681ddf0 c00de1d0 c00dd54c c6832548 00000000 00000000 00000001
de00: c681de44 c0362b98 00000000 c0362c00 c037950c c0374b08 c681de34 c0362b98
de20: c0362c44 c037950c c037950c c0374b08 00000000 c001d288 c681de54 c681de48
de40: c01a7298 c001d438 c681de74 c681de58 c01a6530 c01a7284 c0362b98 c0362c44
de60: c037950c c037950c c681de94 c681de78 c01a6628 c01a646c 00000000 c681de9c
de80: c01a65dc c037950c c681dec4 c681de98 c01a5ba8 c01a65e8 00000000 c6803d58
dea0: c6803d58 c0362be0 00000000 c037950c 00000000 c684c660 c681ded4 c681dec8
dec0: c01a6378 c01a5b68 c681df04 c681ded8 c01a6034 c01a6364 c02afc98 c037950c
dee0: 00000000 c037f380 c037950c 00000000 00000000 00000000 c681df2c c681df08
df00: c01a681c c01a5f98 c037f380 c03794ec 00000000 00000000 00000000 c001d288
df20: c681df3c c681df30 c01a7630 c01a6790 c681df54 c681df40 c01a7664 c01a75c4
df40: c037f380 c0025454 c681df64 c681df58 c001d2c4 c01a7658 c681dfdc c681df68
df60: c002d298 c001d294 c681df94 c681df78 c00d7800 c00d75bc c681df94 c683e260
df80: c00d7944 c681df9e c681dfc4 c681df98 c0077dfc c00d77d4 c681dfb4 3533cd28
dfa0: 00000031 00000000 00000192 c0025454 00000000 c0025524 c0025454 00000000
dfc0: 00000000 00000000 00000000 00000000 c681dff4 c681dfe0 c00088d4 c002d254
dfe0: 00000000 00000000 00000000 c681dff8 c005468c c0008870 124a023d 8e0b1e5e

Backtrace:
[<c001df38>] (musb_platform_init+0x0/0x120) from [<c001d634>] (musb_probe+0x208/0xb0c)
 r5:00000000 r4:c0362d34
[<c001d42c>] (musb_probe+0x0/0xb0c) from [<c01a7298>] (platform_drv_probe+0x20/0x24)
[<c01a7278>] (platform_drv_probe+0x0/0x24) from [<c01a6530>] (driver_probe_device+0xd0/0x17c)
[<c01a6460>] (driver_probe_device+0x0/0x17c) from [<c01a6628>] (__driver_attach+0x4c/0x70)
 r7:c037950c r6:c037950c r5:c0362c44 r4:c0362b98
[<c01a65dc>] (__driver_attach+0x0/0x70) from [<c01a5ba8>] (bus_for_each_dev+0x4c/0x84)
 r7:c037950c r6:c01a65dc r5:c681de9c r4:00000000
[<c01a5b5c>] (bus_for_each_dev+0x0/0x84) from [<c01a6378>] (driver_attach+0x20/0x28)
 r7:c684c660 r6:00000000 r5:c037950c r4:00000000
[<c01a6358>] (driver_attach+0x0/0x28) from [<c01a6034>] (bus_add_driver+0xa8/0x214)
[<c01a5f8c>] (bus_add_driver+0x0/0x214) from [<c01a681c>] (driver_register+0x98/0x120)
 r8:00000000 r7:00000000 r6:00000000 r5:c037950c r4:c037f380
[<c01a6784>] (driver_register+0x0/0x120) from [<c01a7630>] (platform_driver_register+0x78/0x94)
[<c01a75b8>] (platform_driver_register+0x0/0x94) from [<c01a7664>] (platform_driver_probe+0x18/0x68)
[<c01a764c>] (platform_driver_probe+0x0/0x68) from [<c001d2c4>] (musb_init+0x3c/0x54)
 r5:c0025454 r4:c037f380
[<c001d288>] (musb_init+0x0/0x54) from [<c002d298>] (__exception_text_end+0x50/0x17c)
[<c002d248>] (__exception_text_end+0x0/0x17c) from [<c00088d4>] (kernel_init+0x70/0xdc)
[<c0008864>] (kernel_init+0x0/0xdc) from [<c005468c>] (do_exit+0x0/0x6b8)
 r5:00000000 r4:00000000
Code: e5943000 e284c0dc e3530000 e1a0e000 (e8be000f)

<4>---[ end trace 1b75b31a2719ed1c ]--- <0>Kernel panic - not syncing: Attempted to kill init!

Is someone else seeing this?

Regards,
Sergio

#1 lore raw
Quoted 1 line · Sergio Alberto

<1>Unable to handle kernel NULL pointer dereference at virtual address 00000000

Did you try with the hack I've sent, to work around one problematic (and surprisingly reproducible!) i2c-omap timeout? Appended.

Given I2C faults, many things blow up rudely ... hardly any I2C drivers are written to tolerate transfer errors. Not entirely unreasonably, since such errors are otherwise quite rare.

I currently suspect there's an issue where the i2c-omap code can handle a bunch of requests that group closely together ... or are separated by a lot of time ... but there's some middle ground where if you wait the "right" amount of time before making the next request, it times out instead of working correctly.

I have no proof behind that theory, but it does seem to match up to a few of the observed facts. Notably that the i2c timeouts appear when the only device on that bus is the TWL4030, and the drivers make known-to-be-valid requets ... the timeouts started to appear only after some trivial changes in init timings, which were caused by moving code out of drivers/i2c into directories that are more appropriate.

- Dave

---
 drivers/i2c/chips/twl4030-pwrirq.c |   10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/i2c/chips/twl4030-pwrirq.c +10 -0
@@ -161,6 +161,8 @@ static int twl4030_pwrirq_thread(void *d
 	return 0;
 }
 
+#include <linux/delay.h>
+
 static int __init twl4030_pwrirq_init(void)
 {
 	int i, err;
@@ -168,8 +170,16 @@ static int __init twl4030_pwrirq_init(vo
 	twl4030_pwrirq_mask = 0xff;
 	twl4030_pwrirq_pending_unmask = 0;
 
+/* HEY:  core already did this.
+ * But that's surely not why we
+ * sometimes see timeouts here ...
+ */
+for (i = 0; i < 5; i++) {
 	err = twl4030_i2c_write_u8(TWL4030_MODULE_INT, twl4030_pwrirq_mask,
 					TWL4030_INT_PWR_IMR1);
+if (!err) break;
+msleep(10);
+}
 	if (err)
 		return err;
 
#2 lore raw

Thanks for that, it worked fine now.

Quoted 69 lines

-----Original Message-----
From: David Brownell [mailto:[email protected]]
Sent: Tuesday, October 07, 2008 1:29 PM
To: Aguirre Rodriguez, Sergio Alberto
Cc: [email protected]
Subject: Re: kernel oops for 3430sdp

Quoted 1 line · Sergio Alberto

<1>Unable to handle kernel NULL pointer dereference at virtual address

00000000

Did you try with the hack I've sent, to work around one problematic (and surprisingly reproducible!) i2c-omap timeout? Appended.

Given I2C faults, many things blow up rudely ... hardly any I2C drivers are written to tolerate transfer errors. Not entirely unreasonably, since such errors are otherwise quite rare.

I currently suspect there's an issue where the i2c-omap code can handle a bunch of requests that group closely together ... or are separated by a lot of time ... but there's some middle ground where if you wait the "right" amount of time before making the next request, it times out instead of working correctly.

I have no proof behind that theory, but it does seem to match up to a few of the observed facts. Notably that the i2c timeouts appear when the only device on that bus is the TWL4030, and the drivers make known-to-be-valid requets ... the timeouts started to appear only after some trivial changes in init timings, which were caused by moving code out of drivers/i2c into directories that are more appropriate.

- Dave

---
 drivers/i2c/chips/twl4030-pwrirq.c |   10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/i2c/chips/twl4030-pwrirq.c +10 -0
@@ -161,6 +161,8 @@ static int twl4030_pwrirq_thread(void *d
 	return 0;
 }
 
+#include <linux/delay.h>
+
 static int __init twl4030_pwrirq_init(void)
 {
 	int i, err;
@@ -168,8 +170,16 @@ static int __init twl4030_pwrirq_init(vo
 	twl4030_pwrirq_mask = 0xff;
 	twl4030_pwrirq_pending_unmask = 0;
 
+/* HEY:  core already did this.
+ * But that's surely not why we
+ * sometimes see timeouts here ...
+ */
+for (i = 0; i < 5; i++) {
 	err = twl4030_i2c_write_u8(TWL4030_MODULE_INT, twl4030_pwrirq_mask,
 					TWL4030_INT_PWR_IMR1);
+if (!err) break;
+msleep(10);
+}
 	if (err)
 		return err;
 
#3 lore raw
In reply to David Brownell
Quoted 17 lines · David Brownell
Quoted 1 line · Sergio Alberto

<1>Unable to handle kernel NULL pointer dereference at virtual address 00000000

Did you try with the hack I've sent, to work around one problematic (and surprisingly reproducible!) i2c-omap timeout? Appended.

Given I2C faults, many things blow up rudely ... hardly any I2C drivers are written to tolerate transfer errors. Not entirely unreasonably, since such errors are otherwise quite rare.

I currently suspect there's an issue where the i2c-omap code can handle a bunch of requests that group closely together ... or are separated by a lot of time ... but there's some middle ground where if you wait the "right" amount of time before making the next request, it times out instead of working correctly.

Quoted 7 lines

I have no proof behind that theory, but it does seem to match up to a few of the observed facts. Notably that the i2c timeouts appear when the only device on that bus is the TWL4030, and the drivers make known-to-be-valid requets ... the timeouts started to appear only after some trivial changes in init timings, which were caused by moving code out of drivers/i2c into directories that are more appropriate.

I was debugging the i2c-omap.c today but so far no clue why that i2c timeout is coming. It's really weird that it only fails on twl4030-usb (well, actually pwrirq and twl4030-usb fails since it can't request_irq).

Signature · 1 line
balbi
maintainer
#4 lore raw

PATCH 1/1 twl4030: Fix pwrirq by making sure the module is responding (Re: kernel oops for 3430sdp)

Quoted 26 lines · Felipe Balbi
Quoted 17 lines · David Brownell
Quoted 1 line · Sergio Alberto

<1>Unable to handle kernel NULL pointer dereference at virtual address 00000000

Did you try with the hack I've sent, to work around one problematic (and surprisingly reproducible!) i2c-omap timeout? Appended.

Given I2C faults, many things blow up rudely ... hardly any I2C drivers are written to tolerate transfer errors. Not entirely unreasonably, since such errors are otherwise quite rare.

I currently suspect there's an issue where the i2c-omap code can handle a bunch of requests that group closely together ... or are separated by a lot of time ... but there's some middle ground where if you wait the "right" amount of time before making the next request, it times out instead of working correctly.

Quoted 7 lines

I have no proof behind that theory, but it does seem to match up to a few of the observed facts. Notably that the i2c timeouts appear when the only device on that bus is the TWL4030, and the drivers make known-to-be-valid requets ... the timeouts started to appear only after some trivial changes in init timings, which were caused by moving code out of drivers/i2c into directories that are more appropriate.

Quoted 4 lines

I was debugging the i2c-omap.c today but so far no clue why that i2c timeout is coming. It's really weird that it only fails on twl4030-usb (well, actually pwrirq and twl4030-usb fails since it can't request_irq).

I suspect that twl is not yet initialized for pwrirq handling at this point. At least this patch seems to help, no idea on what registers we should check though.. Anybody got ideas on what needs to be checked for pwrirq?

Tony

Quoted 1 line

From f5766a9dbd845911f7bf87bbc71437964b2cc91a Mon Sep 17 00:00:00 2001

From: Tony Lindgren <[email protected]>
Date: Wed, 8 Oct 2008 17:22:30 +0300
Subject: [PATCH] twl4030: Fix pwrirq by making sure the module is responding

Something needs to be checked with twl pwriq module before trying to use it. Not yet sure what should be checked though.. At least PWR_IMR1 seems to change from 0x0 after few reads.

drivers/i2c/chips/twl4030-pwrirq.c +10 -1
index ce66dd4..fd17c3e 100644
@@ -130,10 +130,19 @@ static int twl4030_pwrirq_thread(void *data)
 
 static int __init twl4030_pwrirq_init(void)
 {
-	int i, err;
+	int i = 10, err;
+	u8 tmp;
 
 	twl4030_pwrirq_mask = 0xff;
 
+	/* Wait for other the module to start responding */
+	while (i--) {
+		err = twl4030_i2c_read_u8(TWL4030_MODULE_INT, &tmp,
+						TWL4030_INT_PWR_IMR1);
+		if ((tmp & 0xf) != 0)
+			break;
+	}
+
 	err = twl4030_i2c_write_u8(TWL4030_MODULE_INT, twl4030_pwrirq_mask,
 					TWL4030_INT_PWR_IMR1);
 	if (err)
#5 lore raw

Re: [PATCH] twl4030: Fix pwrirq by making sure the module is responding (Re: kernel oops for 3430sdp)

Quoted 2 lines · Tony Lindgren

I suspect that twl is not yet initialized for pwrirq handling at this point. At least this patch seems to help,

Did you verify that the value you read isn't 0xff? Or does the issue seem to be the mere attempt to read a register?

I have a hard time seeing the root cause be anything other than problems in i2c-omap, since the timeouts appear at various other places too.

Quoted 3 lines

no idea on what registers
we should check though.. Anybody got ideas on what needs to be checked for pwrirq?

Well, there does seem to be an issue where the "pending IRQ" mechanism can leave a PWRIRQ.PWRON interrupt pending sometimes. If you see a bunch of alarming-but-harmless boot messages like

TWL4030 module irq 373 is disabled but can't be masked!

before pwrirq registers, that seems to be the issue. A debug hack I applied (appended) reported that the mask (IMR) was 0xff but the IRQ was still being raised ... when pwrirq registered itself, the stream of those messages stopped. (Or, when this hack dumped the status and thus kicked in clear-on-read ...)

- Dave

=============================

---
 drivers/mfd/twl4030-core.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
drivers/mfd/twl4030-core.c +30 -0
@@ -574,6 +574,36 @@ static int twl4030_irq_thread(void *data
 			continue;
 		}
 
+		/* power irq before handler registered? */
+		if ((pih_isr & BIT(5)) && irq_desc[twl4030_irq_base + 5].depth) {
+			u8 buf[8];
+			int status;
+			int retries = 3;
+
+			while (retries--) {
+				status = twl4030_i2c_read(TWL4030_MODULE_INT,
+						buf, 0, sizeof buf);
+				if (status == 0) {
+					pr_crit("... PWR: "
+						"%02x.%02x/%02x.%02x "
+						"%02x; "
+						"%02x.%02x, "
+						"%02x "
+						"\n",
+						/* {ISR,IMR}{1,2} */
+						buf[0], buf[1], buf[2], buf[3],
+						/* SIH */
+						buf[4],
+						/* EDR */
+						buf[5], buf[6],
+						/* CTRL */
+						buf[7]);
+				} else
+					pr_crit("... read PWR --> %d\n",
+						status);
+			}
+		}
+
 		/* these handlers deal with the relevant SIH irq status */
 		local_irq_disable();
 		for (module_irq = twl4030_irq_base;
maintainer
#6 lore raw

Re: [PATCH] twl4030: Fix pwrirq by making sure the module is responding (Re: kernel oops for 3430sdp)

Quoted 6 lines · David Brownell
Quoted 2 lines · Tony Lindgren

I suspect that twl is not yet initialized for pwrirq handling at this point. At least this patch seems to help,

Did you verify that the value you read isn't 0xff? Or does the issue seem to be the mere attempt to read a register?

Yeah, it seems to be 0 first, then 0xf0, then 0xf7, then 0xff. It seems that at 0xf0 it still does not work. Values from memory, but something like that anyways.

Quoted 3 lines

I have a hard time seeing the root cause be anything other than problems in i2c-omap, since the timeouts appear at various other places too.

Well at least one twl bug was happening when the source clock was initialized incorrectly.. And that only happened with one of the twl modules.

Quoted 8 lines
Quoted 3 lines

no idea on what registers
we should check though.. Anybody got ideas on what needs to be checked for pwrirq?

Well, there does seem to be an issue where the "pending IRQ" mechanism can leave a PWRIRQ.PWRON interrupt pending sometimes. If you see a bunch of alarming-but-harmless boot messages like

Quoted 7 lines

TWL4030 module irq 373 is disabled but can't be masked!

before pwrirq registers, that seems to be the issue. A debug hack I applied (appended) reported that the mask (IMR) was 0xff but the IRQ was still being raised ... when pwrirq registered itself, the stream of those messages stopped. (Or, when this hack dumped the status and thus kicked in clear-on-read ...)

Hmm, maybe there's some status register for each modules that should be checked?

Tony

Quoted 53 lines

- Dave

=============================

---
 drivers/mfd/twl4030-core.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
drivers/mfd/twl4030-core.c +30 -1
@@ -574,6 +574,36 @@ static int twl4030_irq_thread(void *data
 			continue;
 		}
 
+		/* power irq before handler registered? */
+		if ((pih_isr & BIT(5)) && irq_desc[twl4030_irq_base + 5].depth) {
+			u8 buf[8];
+			int status;
+			int retries = 3;
+
+			while (retries--) {
+				status = twl4030_i2c_read(TWL4030_MODULE_INT,
+						buf, 0, sizeof buf);
+				if (status == 0) {
+					pr_crit("... PWR: "
+						"%02x.%02x/%02x.%02x "
+						"%02x; "
+						"%02x.%02x, "
+						"%02x "
+						"\n",
+						/* {ISR,IMR}{1,2} */
+						buf[0], buf[1], buf[2], buf[3],
+						/* SIH */
+						buf[4],
+						/* EDR */
+						buf[5], buf[6],
+						/* CTRL */
+						buf[7]);
+				} else
+					pr_crit("... read PWR --> %d\n",
+						status);
+			}
+		}
+
 		/* these handlers deal with the relevant SIH irq status */
 		local_irq_disable();
 		for (module_irq = twl4030_irq_base;
 
--

To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw

Re: [PATCH] twl4030: Fix pwrirq by making sure the module is responding (Re: kernel oops for 3430sdp)

Quoted 7 lines · Tony Lindgren
Quoted 7 lines

Well, there does seem to be an issue where the "pending IRQ" mechanism can leave a PWRIRQ.PWRON interrupt pending sometimes.  If you see a bunch of alarming-but-harmless boot messages like

  TWL4030 module irq 373 is disabled but can't be masked!

Quoted 8 lines
Quoted 5 lines

before pwrirq registers, that seems to be the issue.  A debug hack I applied (appended) reported that the mask (IMR) was 0xff but the IRQ was still being raised ... when pwrirq registered itself, the stream of those messages stopped.  (Or, when this hack dumped the status and thus kicked in clear-on-read ...)

Hmm, maybe there's some status register for each modules that should be checked?

I think this was just a natural consequence of PENDDIS=0 in the SIH_CTRL register. A minor annoyance in the init sequence since it causes extra IRQs, but nothing significant.

I haven't come across a TWL register that could explain I2C timeouts. At least, not yet.

- Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw

Re: [PATCH] twl4030: Fix pwrirq by making sure the module is responding (Re: kernel oops for 3430sdp)

In reply to Tony Lindgren
Quoted 9 lines · Tony Lindgren
Quoted 5 lines
Quoted 2 lines

I suspect that twl is not yet initialized for pwrirq handling at this point. At least this patch seems to help,

Did you verify that the value you read isn't 0xff?  Or does the issue seem to be the mere attempt to read a register?

Yeah, it seems to be 0 first, then 0xf0, then 0xf7, then 0xff. It seems that at 0xf0 it still does not work. Values from memory, but something like that anyways.

Huh. Most bizarre. Oh wait ... that might be explained by taking a ginormous amount of time to shift out of the 1.5 MHz clock mode. See 3.4.12 of the manual:

When power is first applied to the device, it does not know the external HF clock frequency (HFCLK_FREQ has not been set by the host processor). To ensure that the power subchip registers can be accessed, the default register access frequency is 1.5 MHz (1⁄2 of 3 MHz).

Doesn't say how to tell that's happening, or how long it'd take to change to 3 MHz. (Which only happens if HFCLK isn't 19.2 MHz.) Maybe wait for BACKUP_MISC_CFG.PWR_CLK_FREQ to clear... it's just controlling a simple divide-by-two, not a PLL, so it should be pretty much immediate.

Quoted 7 lines
Quoted 3 lines

I have a hard time seeing the root cause be anything other than problems in i2c-omap, since the timeouts appear at various other places too.

Well at least one twl bug was happening when the source clock was initialized incorrectly.. And that only happened with one of the twl modules.

The issue above? But we know that HFCLK_FREQ is getting set, so that wouldn't explain i2c-omap timeouts that show up later.

Or maybe there's another clocking issue...

I hate to look at the i2c-omap, but it really seems like that's got to be the root cause here. After all, the timeout message appears way too quickly for it to be a *legitimate* timeout for any I2C protocol action. (SMBus has timeouts. I2C doesn't...)

- Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#9 lore raw

PATCH 1/1 i2c-omap: timeouts begone

I can tell someone's going to want a fix that's a
lot more power-aware ... ;)

This seems to remove the need for the "hack" patch.

drivers/i2c/busses/i2c-omap.c +6 -0
@@ -202,6 +202,9 @@ static void omap_i2c_put_clocks(struct omap_i2c_dev *dev)
 
 static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 {
+	if (1)
+		return;
+
 	if (dev->iclk != NULL)
 		clk_enable(dev->iclk);
 	clk_enable(dev->fclk);
@@ -214,6 +217,9 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
 	u16 iv;
 
+	if (1)
+		return;
+
 	dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
 	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
 	if (dev->rev1)
#10 lore raw

Re: [PATCH 2.6.27-rc9-omap] i2c-omap: timeouts begone

Quoted 19 lines · David Brownell

I can tell someone's going to want a fix that's a
lot more power-aware ... ;)

This seems to remove the need for the "hack" patch.

drivers/i2c/busses/i2c-omap.c +3 -0
@@ -202,6 +202,9 @@ static void omap_i2c_put_clocks(struct omap_i2c_dev *dev)
 
 static void omap_i2c_unidle(struct omap_i2c_dev *dev)
 {
+	if (1)
+		return;
+
 	if (dev->iclk != NULL)
 		clk_enable(dev->iclk);
 	clk_enable(dev->fclk);

drivers/i2c/busses/i2c-omap.c static void omap_i2c_idle(struct omap_i2c_dev *dev) · line 225

Quoted 9 lines
drivers/i2c/busses/i2c-omap.c +3 -0
@@ -214,6 +217,9 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev)
 {
 	u16 iv;
 
+	if (1)
+		return;
+
 	dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG);
 	omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);
 	if (dev->rev1)

At least we can say the problem seems to be related to pm. Maybe after context restore we should wait until the clock stabilizes or some register changes ??

Signature · 1 line
balbi
#11 lore raw

RE: [PATCH 2.6.27-rc9-omap] i2c-omap: timeouts begone

Quoted 3 lines

At least we can say the problem seems to be related to pm. Maybe after context restore we should wait until the clock stabilizes or some register changes ??

Maybe. An experiment would be to not shut the clock off in the idle routine.

Actually, scanning i2c_init shows what looks to be a problem.

235 static int omap_i2c_init(struct omap_i2c_dev *dev) 236 {

 237         u16 psc = 0, scll = 0, sclh = 0;
 238         u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
 239         unsigned long fclk_rate = 12000000;
 240         unsigned long timeout;
 241         unsigned long internal_clk = 0;

242
243 if (!dev->rev1) {
244 omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, OMAP_I2C_SYSC_SRST);

None of the module local OCP registers are being setup properly. A write of a 0x2 kills other settings. If after this operation the SSYC reg will say:
-Forced Idle
-Locally gate I+F clock on ocp-segment idle request
-wake up disabled

If it is left this way what can happen is when L4/L3 clock stop partial idle is broadcast, this module will ack and gate its clocks. This will result in you dropping data. Or if the data was sent ok, but clock stop happens, you won't be able to wake the system properly as you've cleared the local wakeup generation. The result will be a timeout.

On OMAP2 I2C wasn't hooked properly into PRCM for handshake so it needed a workaround. The same isn't true for OMAP3. Treating the two as if they are the same with respect to power is false.

Regards,
Richard W.

#12 lore raw

RE: [PATCH 2.6.27-rc9-omap] i2c-omap: timeouts begone

Hi Richard,

trying to fix this problem and have a few questions.

Quoted 7 lines · Richard

None of the module local OCP registers are being setup properly. A write of a 0x2 kills other settings. If after this operation the SSYC reg will say:
-Forced Idle
-Locally gate I+F clock on ocp-segment idle request -wake up disabled

Quoted 5 lines

If it is left this way what can happen is when L4/L3 clock stop partial idle is broadcast, this module will ack and gate its clocks. This will result in you dropping data. Or if the data was sent ok, but clock stop happens, you won't be able to wake the system properly as you've cleared the local wakeup generation. The result will be a timeout.

Okay, so for OMAP3, the workarounds in the omapzoom tree to change the SYSC.CLOCKACTIVITY bits when the clocks are enabled and disabled are not necessary?

It sounds like the only thing that is needed on OMAP3 is to make sure that the SYSCONFIG register is set properly after a software reset?

Quoted 3 lines

On OMAP2 I2C wasn't hooked properly into PRCM for handshake so it needed a workaround. The same isn't true for OMAP3. Treating the two as if they are the same with respect to power is false.

Could you explain further about which workaround this is? OMAP2 I2C does not appear to have any SYSC bits exposed other than SOFTRESET?

- Paul

#13 lore raw

RE: [PATCH 2.6.27-rc9-omap] i2c-omap: timeouts begone

In reply to Felipe Balbi
Quoted 14 lines

Actually, scanning i2c_init shows what looks to be a problem.

235 static int omap_i2c_init(struct omap_i2c_dev *dev) 236 {

 237         u16 psc = 0, scll = 0, sclh = 0;
 238         u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
 239         unsigned long fclk_rate = 12000000;
 240         unsigned long timeout;
 241         unsigned long internal_clk = 0;

242
243 if (!dev->rev1) {
244 omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG,
OMAP_I2C_SYSC_SRST);

Quoted 6 lines

None of the module local OCP registers are being setup properly. A write of a 0x2 kills other settings. If after this operation the SSYC reg will say:
-Forced Idle
-Locally gate I+F clock on ocp-segment idle request -wake up disabled

BTW, I see these settings are correct in Zoom tree in clock functions. http://git.omapzoom.org/?p=omapkernel;a=blob;f=drivers/i2c/busses/i2c-omap.c;hb=d0b4bad4926f9a147a73498f7a62a0ae1cf6f83a

Regards,
Richard W.

#14 lore raw

Re: [PATCH 2.6.27-rc9-omap] i2c-omap: timeouts begone

Quoted 1 line · Richard

BTW, I see these settings are correct in Zoom tree in clock functions.

Someone working with that Zoom tree might consider putting together a patch. ;)

Meanwhile, I'll stick with my simple one.

- Dave