OMAP4: CPUidle: Add coupled idle support

PATCH New Santosh Shilimkar 7 participants 25 messages Mar 30, 2012 – Aug 7, 2012

Subsystems (4)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP power management support

lore

Share & tools

lore

Santosh Shilimkar

author

2012-03-30 13:27 UTC #0 lore raw

cover OMAP4: CPUidle: Add coupled idle support

The series adds the coupled cpuidle support for OMAP4 based on the v2 series posted [1]. This makes OMAP4 to support SMP cpuidle and also removes the hard dependency of off-lining CPU1 to trigger deeper C-states.

I have put together a branch which is based on 3.3 kernel with Len Browns next branch [2] which has time keeping and other cpuidle patches which will mostly get merged by 3.4-rc1 and rebased coupled idle series from [1].

git://gitorious.org/omap-sw-develoment/linux-omap-dev.git
for_3.5/omap4_coupled_cpuidle-rebase

Special thanks to Kevin Hilman and Colin Cross on their help on this series.

Kevin Hilman (1):
ARM: OMAP4: CPUidle: add synchronization for coupled idle states

Santosh Shilimkar (2):
ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus
ARM: OMAP4: cpuidle: Use coupled cpuidle states to implement SMP
cpuidle.

 arch/arm/mach-omap2/Kconfig       |    1 +
 arch/arm/mach-omap2/cpuidle44xx.c |  187 ++++++++++++++++++++++++-------------
 arch/arm/mach-omap2/timer.c       |    4 +-
 3 files changed, 124 insertions(+), 68 deletions(-)

Regards
Santosh
[1] http://lwn.net/Articles/486484/

[2] Power Management
git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git

Santosh Shilimkar

author

2012-03-30 13:27 UTC #1 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.

Signed-off-by Colin Cross <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

---
 arch/arm/mach-omap2/timer.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/timer.c +3 -1

index fc73567..e33f9c4 100644
@@ -135,6 +135,7 @@ static struct clock_event_device clockevent_gpt = {
 	.name		= "gp timer",
 	.features       = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
 	.shift		= 32,
+	.rating		= 300,
 	.set_next_event	= omap2_gp_timer_set_next_event,
 	.set_mode	= omap2_gp_timer_set_mode,
 };

@@ -225,7 +226,8 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
 		clockevent_delta2ns(3, &clockevent_gpt);
 		/* Timer internal resynch latency. */
 
-	clockevent_gpt.cpumask = cpumask_of(0);
+	clockevent_gpt.cpumask = cpu_all_mask;
+	clockevent_gpt.irq = omap_dm_timer_get_irq(&clkev);
 	clockevents_register_device(&clockevent_gpt);
 
 	pr_info("OMAP clockevent source: GPTIMER%d at %lu Hz\n",

Signature · 1 line

1.7.5.4

Daniel Mack

2012-08-03 07:16 UTC #2 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Quoted 6 lines · Santosh Shilimkar

For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.

These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.

Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?

The upstream commit ids are

11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."

Quoted 52 lines

[    2.483556] net eth0: phy found : id is : 0x4dd072
[    2.489176] Unable to handle kernel NULL pointer dereference at virtual address 00000004
[    2.497666] pgd = c0004000
[    2.500507] [00000004] *pgd=00000000
[    2.504282] Internal error: Oops: 805 [#1] SMP ARM
[    2.509309] Modules linked in:
[    2.512530] CPU: 0    Not tainted  (3.6.0-rc1-00036-g6c4c4ee-dirty #152)
[    2.519579] PC is at cache_alloc_refill+0x1b0/0x620
[    2.524695] LR is at 0xc0
[    2.527449] pc : [<c00fc0f0>]    lr : [<000000c0>]    psr: 60000093
[    2.527449] sp : cf83dd70  ip : 00000014  fp : 00200200
[    2.539478] r10: 00000028  r9 : c0df68c0  r8 : 00000028
[    2.544957] r7 : cf928000  r6 : cf87f3c0  r5 : cf88d000  r4 : cf810740
[    2.551800] r3 : 00100100  r2 : cf87f3c8  r1 : 00000000  r0 : 00000000
[    2.558647] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[    2.566400] Control: 10c5387d  Table: 80004019  DAC: 00000017
[    2.572425] Process swapper/0 (pid: 1, stack limit = 0xcf83c2f8)
[    2.578723] Stack: (0xcf83dd70 to 0xcf83e000)
[    2.583297] dd60:                                     00000000 00000000 cf83b3c0 cf87f3d0
[    2.591875] dd80: cf87f3e4 00000020 00000000 00000000 00000077 cf810740 cfb2a000 00000020
[    2.600453] dda0: 00000020 60000013 00000001 c03fdd40 cfb8d800 c00fd0f4 00000001 00000020
[    2.609031] ddc0: cfb8d800 cfb8d800 cfb2a000 cfb2a000 00000700 00000700 cfb46500 c03fdd40
[    2.617609] dde0: 00000000 cfb8d800 00000700 cfb2a000 cfb46500 00000001 cfb46500 c0400588
[    2.626187] de00: cfb8de00 00000010 000000d0 c0316ae4 000000d0 c0964698 cfb8dea8 cfb8df28
[    2.634766] de20: 76616c73 00302d65 00000000 cfb8d800 0000000d 0000016b 00001202 c04ce7c4
[    2.643343] de40: cfb8d800 cfb8d800 c0502030 00000000 cfb8d82c c089aa80 0000016b 00001202
[    2.651922] de60: c07ec0c0 c040bb6c c040bad0 cfb8d800 00001203 00001202 00000001 c040bd98
[    2.660500] de80: 00000000 cfb8d800 00001202 00000003 c07e4154 c040bee4 cfb8d800 c07e416c
[    2.669076] dea0: 00000003 c07cc3cc 00000001 00000000 00000002 00000000 00000000 00000000
[    2.677653] dec0: 00000000 00000000 cf83c000 00000000 00000002 00000000 00000000 00000000
[    2.686230] dee0: 00000000 00000000 c089d23c cf83c000 00000000 00000000 00000002 00000000
[    2.694807] df00: 00000000 c008c454 00000002 00000000 00000000 c04526a8 00000000 c02aa9ac
[    2.703385] df20: 60000013 c0851010 cf806440 c089d22c 00000002 cf83c000 00000000 0000009e
[    2.711963] df40: c07e5dd0 c07ca394 c089d22c c08a35c0 cf83c000 c07d0ee8 c08a35c0 cf83c000
[    2.720541] df60: 00000000 0000009e c07e5dd8 c07cc230 00000000 c00088ec 80000013 ffffffff
[    2.729119] df80: c0776b54 c07cc230 0000009e c005c48c c06c904c c0776174 00000007 00000007
[    2.737697] dfa0: 60000013 c07d0ee8 00000007 c07d0ec8 c08a35c0 0000009e c07e5dd8 c07a21c4
[    2.746274] dfc0: 00000000 c07a28fc 00000007 00000007 c07a21c4 00000000 00000000 c07a2800
[    2.754852] dfe0: c00149c0 00000013 00000000 00000000 00000000 c00149c0 55755555 75715555
[    2.763444] [<c00fc0f0>] (cache_alloc_refill+0x1b0/0x620) from [<c00fd0f4>] (kmem_cache_alloc+0xfc/0x184)
[    2.773496] [<c00fd0f4>] (kmem_cache_alloc+0xfc/0x184) from [<c03fdd40>] (build_skb+0x24/0xa0)
[    2.782540] [<c03fdd40>] (build_skb+0x24/0xa0) from [<c0400588>] (__netdev_alloc_skb+0x94/0xdc)
[    2.791681] [<c0400588>] (__netdev_alloc_skb+0x94/0xdc) from [<c0316ae4>] (cpsw_ndo_open+0x364/0x490)
[    2.801366] [<c0316ae4>] (cpsw_ndo_open+0x364/0x490) from [<c040bb6c>] (__dev_open+0x9c/0xf8)
[    2.810314] [<c040bb6c>] (__dev_open+0x9c/0xf8) from [<c040bd98>] (__dev_change_flags+0x78/0x13c)
[    2.819628] [<c040bd98>] (__dev_change_flags+0x78/0x13c) from [<c040bee4>] (dev_change_flags+0x10/0x48)
[    2.829493] [<c040bee4>] (dev_change_flags+0x10/0x48) from [<c07cc3cc>] (ip_auto_config+0x19c/0xfec)
[    2.839079] [<c07cc3cc>] (ip_auto_config+0x19c/0xfec) from [<c00088ec>] (do_one_initcall+0x34/0x184)
[    2.848679] [<c00088ec>] (do_one_initcall+0x34/0x184) from [<c07a28fc>] (kernel_init+0xfc/0x1c8)
[    2.857914] [<c07a28fc>] (kernel_init+0xfc/0x1c8) from [<c00149c0>] (kernel_thread_exit+0x0/0x8)
[    2.867131] Code: e1500001 3affffe9 e8970003 e1a0a008 (e5801004) 
[    2.873538] ---[ end trace 1020ef2bbda0ff23 ]---

Koen Kooi

2012-08-03 07:21 UTC #3 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:

Quoted 15 lines · Santosh Shilimkar

Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?

Quoted 6 lines

The upstream commit ids are
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."

I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case? I'd really want cpufreq and cpuidle to work properly, but right now I'll settle for "it boots".

regards,

Koen

Koen Kooi

2012-08-03 08:30 UTC #4 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:

Quoted 18 lines

Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 15 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?

Quoted 8 lines

Quoted 6 lines
The upstream commit ids are
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?

To answer my own question: No, the reverts aren't needed if you disable cpuidle.

Shilimkar, Santosh

author

2012-08-03 09:27 UTC #5 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Quoted 23 lines · Koen Kooi

Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.

This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o

May be omap2plus build some how the code gets executed on AMXX

Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX

arch/arm/mach-omap2/pm44xx.c +3 -0

index ea24174..195e756 100644
@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
        struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
        struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
        if (omap_rev() == OMAP4430_REV_ES1_0) {
                WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
                return -ENODEV;

Regards
Ssantosh

Koen Kooi

2012-08-03 09:33 UTC #6 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Op 3 aug. 2012, om 11:27 heeft "Shilimkar, Santosh" <[email protected]> het volgende geschreven:

Quoted 46 lines · Koen Kooi

Quoted 23 lines
Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.
This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o
May be omap2plus build some how the code gets executed on AMXX
Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX
arch/arm/mach-omap2/pm44xx.c +0 -0
index ea24174..195e756 100644

arch/arm/mach-omap2/pm44xx.c int __init omap4_pm_init(void) · line 155

Quoted 9 lines

arch/arm/mach-omap2/pm44xx.c +3 -0

@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
       struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
       struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
       if (omap_rev() == OMAP4430_REV_ES1_0) {
               WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
               return -ENODEV;

That does seem to fix it:

root at beaglebone:~# zcat /proc/config.gz | grep -i cpu_idle CONFIG_CPU_IDLE=y
CONFIG_CPU_IDLE_GOV_LADDER=y CONFIG_CPU_IDLE_GOV_MENU=y CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED=y
root at beaglebone:~# zcat /proc/config.gz | grep -i omap4 CONFIG_ARCH_OMAP4=y
CONFIG_MACH_OMAP4_PANDA=y
# CONFIG_OMAP4_ERRATA_I688 is not set
# CONFIG_KEYBOARD_OMAP4 is not set CONFIG_OMAP4_DSS_HDMI=y

regards,

Koen

Hiremath, Vaibhav

2012-08-03 09:42 UTC #7 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Quoted 61 lines · Koen Kooi

Op 3 aug. 2012, om 11:27 heeft "Shilimkar, Santosh" <[email protected]> het volgende geschreven:
Quoted 46 lines · Koen Kooi
Quoted 23 lines
Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.
This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o
May be omap2plus build some how the code gets executed on AMXX
Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX
arch/arm/mach-omap2/pm44xx.c +0 -0
index ea24174..195e756 100644
arch/arm/mach-omap2/pm44xx.c int __init omap4_pm_init(void) · line 155
Quoted 9 lines
arch/arm/mach-omap2/pm44xx.c +3 -0
@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
       struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
       struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
       if (omap_rev() == OMAP4430_REV_ES1_0) {
               WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
               return -ENODEV;
That does seem to fix it:

Quoted 12 lines

root at beaglebone:~# zcat /proc/config.gz | grep -i cpu_idle CONFIG_CPU_IDLE=y
CONFIG_CPU_IDLE_GOV_LADDER=y CONFIG_CPU_IDLE_GOV_MENU=y CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED=y
root at beaglebone:~# zcat /proc/config.gz | grep -i omap4 CONFIG_ARCH_OMAP4=y
CONFIG_MACH_OMAP4_PANDA=y
# CONFIG_OMAP4_ERRATA_I688 is not set
# CONFIG_KEYBOARD_OMAP4 is not set CONFIG_OMAP4_DSS_HDMI=y

This patch is not required, Without this patch is works for me,

[root at arago /]# cat /proc/version
Linux version 3.6.0-rc1-next-20120803-00010-g5f6bf0f (a0393758 at psplinux064) (gcc version 4.5.3 20110311 (prerelease) (GCC) ) #1 SMP Fri Aug 3 15:06:57 IST 2012 [root at arago /]#

Do you have below patch??
ARM: OMAP: cpu: Make cpu_class_is_omap2 true for all non-omap1 platforms

Also I have pushed branch (based on linux-next/master) to
https://github.com/hvaibhav/am335x-linux/tree/am335x-linux-next-master

Thanks,
Vaibhav

Shilimkar, Santosh

author

2012-08-03 09:48 UTC #8 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Quoted 75 lines · Vaibhav

Quoted 61 lines · Koen Kooi
Op 3 aug. 2012, om 11:27 heeft "Shilimkar, Santosh" <[email protected]> het volgende geschreven:
Quoted 46 lines · Koen Kooi
Quoted 23 lines
Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.
This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o
May be omap2plus build some how the code gets executed on AMXX
Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX
arch/arm/mach-omap2/pm44xx.c +0 -0
index ea24174..195e756 100644
arch/arm/mach-omap2/pm44xx.c int __init omap4_pm_init(void) · line 155
Quoted 9 lines
arch/arm/mach-omap2/pm44xx.c +3 -0
@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
       struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
       struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
       if (omap_rev() == OMAP4430_REV_ES1_0) {
               WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
               return -ENODEV;
That does seem to fix it:
Quoted 12 lines
root at beaglebone:~# zcat /proc/config.gz | grep -i cpu_idle CONFIG_CPU_IDLE=y
CONFIG_CPU_IDLE_GOV_LADDER=y CONFIG_CPU_IDLE_GOV_MENU=y CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED=y
root at beaglebone:~# zcat /proc/config.gz | grep -i omap4 CONFIG_ARCH_OMAP4=y
CONFIG_MACH_OMAP4_PANDA=y
# CONFIG_OMAP4_ERRATA_I688 is not set
# CONFIG_KEYBOARD_OMAP4 is not set CONFIG_OMAP4_DSS_HDMI=y

Quoted 2 lines

This patch is not required, Without this patch is works for me,

Which is good news.Do you have CPUIDLE enabled in your build when you tested it. Note that CPUIDLE is not enabled by-default in omap2plus build and as I see from Koen's email, he did enable it in his testing.

Please confirm that 3.6-rc1 boots fine even with CPUIDLE enabled and we don't need
the above mentioned patch. If it is not the case, I can post the fix.

Thanks for testing.

Regards
Santosh

Hiremath, Vaibhav

2012-08-03 10:32 UTC #9 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Quoted 78 lines · Santosh

Quoted 75 lines · Vaibhav
Quoted 61 lines · Koen Kooi
Op 3 aug. 2012, om 11:27 heeft "Shilimkar, Santosh" <[email protected]> het volgende geschreven:
Quoted 46 lines · Koen Kooi
Quoted 23 lines
Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.
This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o
May be omap2plus build some how the code gets executed on AMXX
Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX
arch/arm/mach-omap2/pm44xx.c +0 -0
index ea24174..195e756 100644
arch/arm/mach-omap2/pm44xx.c int __init omap4_pm_init(void) · line 155
Quoted 9 lines
arch/arm/mach-omap2/pm44xx.c +3 -0
@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
       struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
       struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
       if (omap_rev() == OMAP4430_REV_ES1_0) {
               WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
               return -ENODEV;
That does seem to fix it:
Quoted 12 lines
root at beaglebone:~# zcat /proc/config.gz | grep -i cpu_idle CONFIG_CPU_IDLE=y
CONFIG_CPU_IDLE_GOV_LADDER=y CONFIG_CPU_IDLE_GOV_MENU=y CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED=y
root at beaglebone:~# zcat /proc/config.gz | grep -i omap4 CONFIG_ARCH_OMAP4=y
CONFIG_MACH_OMAP4_PANDA=y
# CONFIG_OMAP4_ERRATA_I688 is not set
# CONFIG_KEYBOARD_OMAP4 is not set CONFIG_OMAP4_DSS_HDMI=y
Quoted 2 lines
This patch is not required, Without this patch is works for me,

Quoted 8 lines

Which is good news.Do you have CPUIDLE enabled in your build when you tested it. Note that CPUIDLE is not enabled by-default in omap2plus build and as I see from Koen's email, he did enable it in his testing.
Please confirm that 3.6-rc1 boots fine even with CPUIDLE enabled and we don't need
the above mentioned patch. If it is not the case, I can post the fix.

Just FYI, I have also tested with CPUIDLE enabled and it worked for me.

Thanks,
Vaibhav

Shilimkar, Santosh

author

2012-08-03 10:33 UTC #10 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Quoted 83 lines · Vaibhav

Quoted 82 lines · Santosh
Quoted 75 lines · Vaibhav
Quoted 61 lines · Koen Kooi
Op 3 aug. 2012, om 11:27 heeft "Shilimkar, Santosh" <[email protected]> het volgende geschreven:
Quoted 46 lines · Koen Kooi
Quoted 23 lines
Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.
This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o
May be omap2plus build some how the code gets executed on AMXX
Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX
arch/arm/mach-omap2/pm44xx.c +0 -0
index ea24174..195e756 100644
arch/arm/mach-omap2/pm44xx.c int __init omap4_pm_init(void) · line 155
Quoted 9 lines
arch/arm/mach-omap2/pm44xx.c +3 -0
@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
       struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
       struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
       if (omap_rev() == OMAP4430_REV_ES1_0) {
               WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
               return -ENODEV;
That does seem to fix it:
Quoted 12 lines
root at beaglebone:~# zcat /proc/config.gz | grep -i cpu_idle CONFIG_CPU_IDLE=y
CONFIG_CPU_IDLE_GOV_LADDER=y CONFIG_CPU_IDLE_GOV_MENU=y CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED=y
root at beaglebone:~# zcat /proc/config.gz | grep -i omap4 CONFIG_ARCH_OMAP4=y
CONFIG_MACH_OMAP4_PANDA=y
# CONFIG_OMAP4_ERRATA_I688 is not set
# CONFIG_KEYBOARD_OMAP4 is not set CONFIG_OMAP4_DSS_HDMI=y
Quoted 2 lines
This patch is not required, Without this patch is works for me,
Which is good news.Do you have CPUIDLE enabled in your build when you tested it. Note that CPUIDLE is not enabled by-default in omap2plus build and as I see from Koen's email, he did enable it in his testing.

Quoted 7 lines

Quoted 4 lines
Please confirm that 3.6-rc1 boots fine even with CPUIDLE enabled and we don't need
the above mentioned patch. If it is not the case, I can post the fix.
Just FYI, I have also tested with CPUIDLE enabled and it worked for me.

Thanks Vaibhav for update.

Regards
Santosh

Koen Kooi

2012-08-03 10:04 UTC #11 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

In reply to Hiremath, Vaibhav

Op 3 aug. 2012, om 11:42 heeft "Hiremath, Vaibhav" <[email protected]> het volgende geschreven:

Quoted 75 lines · Koen Kooi

Quoted 61 lines
Op 3 aug. 2012, om 11:27 heeft "Shilimkar, Santosh" <[email protected]> het volgende geschreven:
Quoted 46 lines · Koen Kooi
Quoted 23 lines
Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.
This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o
May be omap2plus build some how the code gets executed on AMXX
Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX
arch/arm/mach-omap2/pm44xx.c +0 -0
index ea24174..195e756 100644
arch/arm/mach-omap2/pm44xx.c int __init omap4_pm_init(void) · line 155
Quoted 9 lines
arch/arm/mach-omap2/pm44xx.c +3 -0
@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
      struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
      struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
      if (omap_rev() == OMAP4430_REV_ES1_0) {
              WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
              return -ENODEV;
That does seem to fix it:
Quoted 12 lines
root@beaglebone:~# zcat /proc/config.gz | grep -i cpu_idle
CONFIG_CPU_IDLE=y
CONFIG_CPU_IDLE_GOV_LADDER=y
CONFIG_CPU_IDLE_GOV_MENU=y
CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED=y
root@beaglebone:~# zcat /proc/config.gz | grep -i omap4   
CONFIG_ARCH_OMAP4=y
CONFIG_MACH_OMAP4_PANDA=y
# CONFIG_OMAP4_ERRATA_I688 is not set
# CONFIG_KEYBOARD_OMAP4 is not set
CONFIG_OMAP4_DSS_HDMI=y

Quoted 1 line

This patch is not required, Without this patch is works for me,

I just retested and I don't need Santosh' patch, booting with cpuidle enable works now, after refreshing your patchset (and dropping the rtc commit, it conflicts with the other rtc patches out there).

Quoted 7 lines

[root@arago /]# cat /proc/version
Linux version 3.6.0-rc1-next-20120803-00010-g5f6bf0f (a0393758@psplinux064) (gcc version 4.5.3 20110311 (prerelease) (GCC) ) #1 SMP Fri Aug 3 15:06:57 IST 2012 [root@arago /]#

Quoted 6 lines

Do you have below patch??
ARM: OMAP: cpu: Make cpu_class_is_omap2 true for all non-omap1 platforms
Also I have pushed branch (based on linux-next/master) to
https://github.com/hvaibhav/am335x-linux/tree/am335x-linux-next-master

Thanks! Ajays USB work and AnilKumar's pinctrl work are there as branches, any plans to add da8xx-fb, rtc and networking branches? As you can see from the cpsw patches, they were impossible to test because the davinci_mdio ones were missing.

regards,

Koen

Shilimkar, Santosh

author

2012-08-03 10:14 UTC #12 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Quoted 78 lines · Koen Kooi

Op 3 aug. 2012, om 11:42 heeft "Hiremath, Vaibhav" <[email protected]> het volgende geschreven:
Quoted 75 lines · Koen Kooi
Quoted 73 lines
Op 3 aug. 2012, om 11:27 heeft "Shilimkar, Santosh" <[email protected]> het volgende geschreven:
Quoted 46 lines · Koen Kooi
Quoted 23 lines
Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.
This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o
May be omap2plus build some how the code gets executed on AMXX
Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX
arch/arm/mach-omap2/pm44xx.c +0 -0
index ea24174..195e756 100644
arch/arm/mach-omap2/pm44xx.c int __init omap4_pm_init(void) · line 155
Quoted 9 lines
arch/arm/mach-omap2/pm44xx.c +3 -0
@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
      struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
      struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
      if (omap_rev() == OMAP4430_REV_ES1_0) {
              WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
              return -ENODEV;
That does seem to fix it:
root at beaglebone:~# zcat /proc/config.gz | grep -i cpu_idle CONFIG_CPU_IDLE=y
CONFIG_CPU_IDLE_GOV_LADDER=y CONFIG_CPU_IDLE_GOV_MENU=y CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED=y
root at beaglebone:~# zcat /proc/config.gz | grep -i omap4 CONFIG_ARCH_OMAP4=y
CONFIG_MACH_OMAP4_PANDA=y
# CONFIG_OMAP4_ERRATA_I688 is not set
# CONFIG_KEYBOARD_OMAP4 is not set CONFIG_OMAP4_DSS_HDMI=y

Quoted 4 lines

Quoted 1 line
This patch is not required, Without this patch is works for me,
I just retested and I don't need Santosh' patch, booting with cpuidle enable works now, after refreshing your patchset (and dropping the rtc commit, it conflicts with the other rtc patches out there).

Thanks Koen for confirming. That means the issues was coming from additional patching on top of 3.6-rc1
where some patches were not refreshed for AMXX.

Regards
Santosh

Hiremath, Vaibhav

2012-08-03 10:34 UTC #13 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Quoted 79 lines · Santosh

Quoted 78 lines · Koen Kooi
Op 3 aug. 2012, om 11:42 heeft "Hiremath, Vaibhav" <[email protected]> het volgende geschreven:
Quoted 75 lines · Koen Kooi
Quoted 73 lines
Op 3 aug. 2012, om 11:27 heeft "Shilimkar, Santosh" <[email protected]> het volgende geschreven:
Quoted 46 lines · Koen Kooi
Quoted 23 lines
Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.
This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o
May be omap2plus build some how the code gets executed on AMXX
Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX
arch/arm/mach-omap2/pm44xx.c +0 -0
index ea24174..195e756 100644
arch/arm/mach-omap2/pm44xx.c int __init omap4_pm_init(void) · line 155
Quoted 9 lines
arch/arm/mach-omap2/pm44xx.c +3 -0
@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
      struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
      struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
      if (omap_rev() == OMAP4430_REV_ES1_0) {
              WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
              return -ENODEV;
That does seem to fix it:
root at beaglebone:~# zcat /proc/config.gz | grep -i cpu_idle CONFIG_CPU_IDLE=y
CONFIG_CPU_IDLE_GOV_LADDER=y CONFIG_CPU_IDLE_GOV_MENU=y CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED=y
root at beaglebone:~# zcat /proc/config.gz | grep -i omap4 CONFIG_ARCH_OMAP4=y
CONFIG_MACH_OMAP4_PANDA=y
# CONFIG_OMAP4_ERRATA_I688 is not set
# CONFIG_KEYBOARD_OMAP4 is not set CONFIG_OMAP4_DSS_HDMI=y

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
This patch is not required, Without this patch is works for me,
I just retested and I don't need Santosh' patch, booting with cpuidle enable works now, after refreshing your patchset (and dropping the rtc commit, it conflicts with the other rtc patches out there).
Thanks Koen for confirming. That means the issues was coming from additional patching on top of 3.6-rc1
where some patches were not refreshed for AMXX.

Actually its not that.
AM335x needs to be added as part of cpu_class_is_omap2(), without this it gets into issues.
The patch has been already submitted to the list, but still not merged by Tony.

Thanks,
Vaibhav

Tony Lindgren

maintainer

2012-08-07 06:50 UTC #14 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

Quoted 6 lines · Hiremath, Vaibhav

Actually its not that.
AM335x needs to be added as part of cpu_class_is_omap2(), without this it gets into issues.
The patch has been already submitted to the list, but still not merged by Tony.

Let's just add it to the list of cpu_class_is_omap2() with a proper patch description for the fix. Then we still need to sort it out properly from common zImage point of view without having to maintain that list.

Vaibhav, can you please post your original patch with the description updated for v3.6-rc1?

Regards,

Tony

Hiremath, Vaibhav

2012-08-03 10:23 UTC #15 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

In reply to Koen Kooi

Quoted 78 lines · Koen Kooi

Op 3 aug. 2012, om 11:42 heeft "Hiremath, Vaibhav" <[email protected]> het volgende geschreven:
Quoted 75 lines · Koen Kooi
Quoted 73 lines
Op 3 aug. 2012, om 11:27 heeft "Shilimkar, Santosh" <[email protected]> het volgende geschreven:
Quoted 46 lines · Koen Kooi
Quoted 23 lines
Op 3 aug. 2012, om 09:21 heeft Koen Kooi <[email protected]> het volgende geschreven:
Quoted 20 lines
Op 3 aug. 2012, om 09:16 heeft Daniel Mack <[email protected]> het volgende geschreven:
Quoted 17 lines · Santosh Shilimkar
Quoted 6 lines
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.
Once I revert 1/3, the board boots again but crashes with the Ooops below. With the entire series reverted, everything works again as expected. Any idea?
The upstream commit ids are
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
11d6ec2e "ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus"
5b4d5bcc "ARM: OMAP4: CPUidle: add synchronization for coupled idle states" b93d70ae "ARM: OMAP4: CPUidle: Open broadcast clock-event device."
I've had boot problems with cpuidle enabled as well, what happens if you disable it? Is the revert still needed in that case?
To answer my own question: No, the reverts aren't needed if you disable cpuidle.
This is really strange since CPUIDLE code is really OMAP4 specific.
obj-$(CONFIG_ARCH_OMAP4) += cpuidle44xx.o
May be omap2plus build some how the code gets executed on AMXX
Can you try below and see if the boot with CPUIDLE enabled goes away on AMXX
arch/arm/mach-omap2/pm44xx.c +0 -0
index ea24174..195e756 100644
arch/arm/mach-omap2/pm44xx.c int __init omap4_pm_init(void) · line 155
Quoted 9 lines
arch/arm/mach-omap2/pm44xx.c +3 -0
@@ -147,6 +147,9 @@ int __init omap4_pm_init(void)
      struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm, *l4wkup;
      struct clockdomain *ducati_clkdm, *l3_2_clkdm, *l4_per_clkdm;
 
+       if (!cpu_is_omap44xx())
+               return -ENODEV;
+
      if (omap_rev() == OMAP4430_REV_ES1_0) {
              WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
              return -ENODEV;
That does seem to fix it:
root at beaglebone:~# zcat /proc/config.gz | grep -i cpu_idle CONFIG_CPU_IDLE=y
CONFIG_CPU_IDLE_GOV_LADDER=y CONFIG_CPU_IDLE_GOV_MENU=y CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED=y
root at beaglebone:~# zcat /proc/config.gz | grep -i omap4 CONFIG_ARCH_OMAP4=y
CONFIG_MACH_OMAP4_PANDA=y
# CONFIG_OMAP4_ERRATA_I688 is not set
# CONFIG_KEYBOARD_OMAP4 is not set CONFIG_OMAP4_DSS_HDMI=y

Quoted 4 lines

Quoted 1 line
This patch is not required, Without this patch is works for me,
I just retested and I don't need Santosh' patch, booting with cpuidle enable works now, after refreshing your patchset (and dropping the rtc commit, it conflicts with the other rtc patches out there).

Thanks for conforming Koen.

Quoted 7 lines

Quoted 7 lines
[root at arago /]# cat /proc/version
Linux version 3.6.0-rc1-next-20120803-00010-g5f6bf0f (a0393758 at psplinux064) (gcc version 4.5.3 20110311 (prerelease) (GCC) ) #1 SMP Fri Aug 3 15:06:57 IST 2012 [root at arago /]#

Quoted 9 lines

Quoted 6 lines
Do you have below patch??
ARM: OMAP: cpu: Make cpu_class_is_omap2 true for all non-omap1 platforms
Also I have pushed branch (based on linux-next/master) to
https://github.com/hvaibhav/am335x-linux/tree/am335x-linux-next-master
Thanks! Ajays USB work and AnilKumar's pinctrl work are there as branches, any plans to add da8xx-fb, rtc and networking branches? As you can see from the cpsw patches, they were impossible to test because the davinci_mdio ones were missing.

Yes that's the plan, I am in the process of updating it. Hopefully I will have branch for each major module.

Thanks,
Vaibhav

Hiremath, Vaibhav

2012-08-03 08:22 UTC #16 lore raw

PATCH 1/3 ARM: OMAP: timer: allow gp timer clock-event to be used on both cpus

In reply to Daniel Mack

Quoted 11 lines · Daniel Mack

Quoted 6 lines · Santosh Shilimkar
For coupled cpuidle to work when both cpus are active, it needs a global timer that can handle events for both cpus. This timer is used as the broadcast clock-event when the per-cpu timer hardware stop in low power states. Set the cpumask of clockevent_gpt to all cpus, set the rating correctly, and set the irq to allow the clockevent core to determine the affinity of the timer.
These patches made it to mainline now, shortly befor 3.6-rc1, and it breaks boot on my AM33xx board.

Thanks Daniel for testing and reporting. I haven't tried 3.6-rc1 yet, as omap-linux/master has not yet migrated to it.
I am testing it now, and will let you know on the status.

Thanks,
Vaibhav

Santosh Shilimkar

author

2012-03-30 13:27 UTC #17 lore raw

PATCH 2/3 ARM: OMAP4: cpuidle: Use coupled cpuidle states to implement SMP cpuidle.

In reply to Santosh Shilimkar

OMAP4 CPUDILE driver is converted mainly based on notes from the coupled cpuidle patch series.

The changes include :
- Register both CPUs and C-states to cpuidle driver.
- Set struct cpuidle_device.coupled_cpus
- Set struct cpuidle_device.safe_state to non coupled state.
- Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each state that affects multiple cpus.
- Separate ->enter hooks for coupled & simple idle.
- CPU0 wait loop for CPU1 power transition.
- CPU1 wakeup mechanism for the idle exit.
- Enabling ARCH_NEEDS_CPU_IDLE_COUPLED for OMAP4.

Thanks to Kevin Hilman and Colin Cross on the suggestions/fixes on the intermediate version of this patch.

Signed-off-by Santosh Shilimkar <[email protected]>

CC Kevin Hilman <[email protected]>

Cc Colin Cross <[email protected]>

---
 arch/arm/mach-omap2/Kconfig       |    1 +
 arch/arm/mach-omap2/cpuidle44xx.c |  167 ++++++++++++++++++++++---------------
 2 files changed, 101 insertions(+), 67 deletions(-)

arch/arm/mach-omap2/Kconfig +1 -0

index e20c8ab..250786e 100644
@@ -54,6 +54,7 @@ config ARCH_OMAP4
 	select PM_OPP if PM
 	select USB_ARCH_HAS_EHCI
 	select ARM_CPU_SUSPEND if PM
+	select ARCH_NEEDS_CPU_IDLE_COUPLED if CPU_IDLE
 
 comment "OMAP Core Type"
 	depends on ARCH_OMAP2

arch/arm/mach-omap2/cpuidle44xx.c +100 ~~-67~~

244 lines not on this page · raw message

Signature · 1 line

1.7.5.4

Colin Cross

2012-03-30 19:43 UTC #18 lore raw

PATCH 2/3 ARM: OMAP4: cpuidle: Use coupled cpuidle states to implement SMP cpuidle.

Quoted 30 lines · Santosh Shilimkar

OMAP4 CPUDILE driver is converted mainly based on notes from the coupled cpuidle patch series.
The changes include :
- Register both CPUs and C-states to cpuidle driver.
- Set struct cpuidle_device.coupled_cpus
- Set struct cpuidle_device.safe_state to non coupled state.
- Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each ?state that affects multiple cpus.
- Separate ->enter hooks for coupled & simple idle.
- CPU0 wait loop for CPU1 power transition.
- CPU1 wakeup mechanism for the idle exit.
- Enabling ARCH_NEEDS_CPU_IDLE_COUPLED for OMAP4.
Thanks to Kevin Hilman and Colin Cross on the suggestions/fixes on the intermediate version of this patch.
Signed-off-by Santosh Shilimkar <[email protected]>
CC Kevin Hilman <[email protected]>
Cc Colin Cross <[email protected]>
---
?arch/arm/mach-omap2/Kconfig ? ? ? | ? ?1 + ?arch/arm/mach-omap2/cpuidle44xx.c | ?167 ++++++++++++++++++++++--------------- ?2 files changed, 101 insertions(+), 67 deletions(-)
arch/arm/mach-omap2/Kconfig +0 -0
index e20c8ab..250786e 100644
@@ -54,6 +54,7 @@ config ARCH_OMAP4

Quoted 4 lines

? ? ? ?select PM_OPP if PM
? ? ? ?select USB_ARCH_HAS_EHCI
? ? ? ?select ARM_CPU_SUSPEND if PM
+ ? ? ? select ARCH_NEEDS_CPU_IDLE_COUPLED if CPU_IDLE

The "if CPU_IDLE" is not necessary, ARCH_NEEDS_CPU_IDLE_COUPLED is designed to have no effect if CPU_IDLE is not set.

Quoted 66 lines

?comment "OMAP Core Type"
? ? ? ?depends on ARCH_OMAP2
arch/arm/mach-omap2/cpuidle44xx.c +0 -0
index f386cbe..5724393 100644
@@ -21,6 +21,7 @@
?#include "common.h"
?#include "pm.h"
?#include "prm.h"
+#include "clockdomain.h"
?#ifdef CONFIG_CPU_IDLE
arch/arm/mach-omap2/cpuidle44xx.c +2 -1
@@ -44,10 +45,11 @@ static struct cpuidle_params cpuidle_params_table[] = {
 ?#define OMAP4_NUM_STATES ARRAY_SIZE(cpuidle_params_table)
 
 ?struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES];
-static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;
+static struct powerdomain *mpu_pd, *cpu_pd[NR_CPUS];
+static struct clockdomain *cpu_clkdm[NR_CPUS];
 
?/**
- * omap4_enter_idle - Programs OMAP4 to enter the specified state
+ * omap4_enter_idle_coupled_[simple/coupled] - OMAP4 cpuidle entry functions ?* @dev: cpuidle device
?* @drv: cpuidle driver
?* @index: the index of state to be entered
arch/arm/mach-omap2/cpuidle44xx.c +0 -0
@@ -56,34 +58,40 @@ static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;
?* specified low power state selected by the governor. ?* Returns the amount of time spent in the low power state. ?*/
arch/arm/mach-omap2/cpuidle44xx.c +12 -1
-static int omap4_enter_idle(struct cpuidle_device *dev,
+static int omap4_enter_idle_simple(struct cpuidle_device *dev,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_driver *drv,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int index)
+{
+ ? ? ? local_fiq_disable();
+ ? ? ? omap_do_wfi();
+ ? ? ? local_fiq_enable();
+
+ ? ? ? return index;
+}
+
+static int omap4_enter_idle_coupled(struct cpuidle_device *dev,
? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_driver *drv,
? ? ? ? ? ? ? ? ? ? ? ?int index)
?{
? ? ? ?struct omap4_idle_statedata *cx =
? ? ? ? ? ? ? ? ? ? ? ?cpuidle_get_statedata(&dev->states_usage[index]);
- ? ? ? u32 cpu1_state;
? ? ? ?int cpu_id = smp_processor_id();
? ? ? ?local_fiq_disable();
+ ? ? ? clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id); +
? ? ? ?/*
- ? ? ? ?* CPU0 has to stay ON (i.e in C1) until CPU1 is OFF state.
+ ? ? ? ?* CPU0 has to wait and stay ON until CPU1 is OFF state. ? ? ? ? * This is necessary to honour hardware recommondation ? ? ? ? * of triggeing all the possible low power modes once CPU1 is ? ? ? ? * out of coherency and in OFF mode.
- ? ? ? ?* Update dev->last_state so that governor stats reflects right
- ? ? ? ?* data.

Quoted 8 lines

? ? ? ? */
- ? ? ? cpu1_state = pwrdm_read_pwrst(cpu1_pd);
- ? ? ? if (cpu1_state != PWRDM_POWER_OFF) {
- ? ? ? ? ? ? ? index = drv->safe_state_index;
- ? ? ? ? ? ? ? cx = cpuidle_get_statedata(&dev->states_usage[index]);
+ ? ? ? if (dev->cpu == 0) {
+ ? ? ? ? ? ? ? while (pwrdm_read_pwrst(cpu_pd[1]) != PWRDM_POWER_OFF)
+ ? ? ? ? ? ? ? ? ? ? ? cpu_relax();

If something goes wrong in the core coupled code or in the cpu 1 power state transition, this will hang forever and be hard to debug. It might be worth adding a timeout with a BUG_ON.

arch/arm/mach-omap2/cpuidle44xx.c static int omap4_enter_idle(struct cpuidle_device *dev, · line 100

Quoted 11 lines

? ? ? ?}
- ? ? ? if (index > 0)
- ? ? ? ? ? ? ? clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id); -
? ? ? ?/*
? ? ? ? * Call idle CPU PM enter notifier chain so that
? ? ? ? * VFP and per CPU interrupt context is saved.
arch/arm/mach-omap2/cpuidle44xx.c +0 -0
@@ -91,25 +99,35 @@ static int omap4_enter_idle(struct cpuidle_device *dev,
 ? ? ? ?if (cx->cpu_state == PWRDM_POWER_OFF)
 ? ? ? ? ? ? ? ?cpu_pm_enter();

This should never get called without cpu_state == PWRDM_POWER_OFF, and even if it did, calling cpu_pm_enter shouldn't hurt anything. It would be clearer to unconditionally call cpu_pm_enter().

Quoted 34 lines

arch/arm/mach-omap2/cpuidle44xx.c +20 ~~-10~~

- ? ? ? pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
- ? ? ? omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
-
- ? ? ? /*
- ? ? ? ?* Call idle CPU cluster PM enter notifier chain
- ? ? ? ?* to save GIC and wakeupgen context.
- ? ? ? ?*/
- ? ? ? if ((cx->mpu_state == PWRDM_POWER_RET) &&
- ? ? ? ? ? ? ? (cx->mpu_logic_state == PWRDM_POWER_OFF))
- ? ? ? ? ? ? ? ? ? ? ? cpu_cluster_pm_enter();
+ ? ? ? if (dev->cpu == 0) {
+ ? ? ? ? ? ? ? pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
+ ? ? ? ? ? ? ? omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
+
+ ? ? ? ? ? ? ? /*
+ ? ? ? ? ? ? ? ?* Call idle CPU cluster PM enter notifier chain
+ ? ? ? ? ? ? ? ?* to save GIC and wakeupgen context.
+ ? ? ? ? ? ? ? ?*/
+ ? ? ? ? ? ? ? if ((cx->mpu_state == PWRDM_POWER_RET) &&
+ ? ? ? ? ? ? ? ? ? ? ? (cx->mpu_logic_state == PWRDM_POWER_OFF))
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cpu_cluster_pm_enter();
+ ? ? ? }
 
 ? ? ? ?omap4_enter_lowpower(dev->cpu, cx->cpu_state);
 
+ ? ? ? if (dev->cpu == 0) {
+ ? ? ? ? ? ? ? /* Wakeup CPU1 only if it is not offlined */
+ ? ? ? ? ? ? ? if (cpumask_test_cpu(1, cpu_online_mask)) {
+ ? ? ? ? ? ? ? ? ? ? ? clkdm_wakeup(cpu_clkdm[1]);
+ ? ? ? ? ? ? ? ? ? ? ? clkdm_allow_idle(cpu_clkdm[1]);
+ ? ? ? ? ? ? ? }
+ ? ? ? }
+

Quoted 5 lines

? ? ? ?/*
? ? ? ? * Call idle CPU PM exit notifier chain to restore ? ? ? ? * VFP and per CPU IRQ context. Only CPU0 state is ? ? ? ? * considered since CPU1 is managed by CPU hotplug. ? ? ? ? */

This comment is no longer accurate? cpu_pm_enter is called on cpu 1 above.

Quoted 3 lines

- ? ? ? if (pwrdm_read_prev_pwrst(cpu0_pd) == PWRDM_POWER_OFF)
+ ? ? ? if (pwrdm_read_prev_pwrst(cpu_pd[dev->cpu]) == PWRDM_POWER_OFF) ? ? ? ? ? ? ? ?cpu_pm_exit();

This should get called unconditionally. It's not explicitly stated, but the cpu_pm_* api expects cpu_pm_exit() to be called after cpu_pm_enter(), even if the low power state was not entered. Otherwise, a cpu_pm_enter notifier that disables the hardware will not get a chance to re-enable it.

Shilimkar, Santosh

author

2012-03-31 06:37 UTC #19 lore raw

PATCH 2/3 ARM: OMAP4: cpuidle: Use coupled cpuidle states to implement SMP cpuidle.

Quoted 27 lines · Colin Cross

Quoted 25 lines · Santosh Shilimkar
OMAP4 CPUDILE driver is converted mainly based on notes from the coupled cpuidle patch series.
The changes include :
- Register both CPUs and C-states to cpuidle driver.
- Set struct cpuidle_device.coupled_cpus
- Set struct cpuidle_device.safe_state to non coupled state.
- Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each ?state that affects multiple cpus.
- Separate ->enter hooks for coupled & simple idle.
- CPU0 wait loop for CPU1 power transition.
- CPU1 wakeup mechanism for the idle exit.
- Enabling ARCH_NEEDS_CPU_IDLE_COUPLED for OMAP4.
Thanks to Kevin Hilman and Colin Cross on the suggestions/fixes on the intermediate version of this patch.
Signed-off-by Santosh Shilimkar <[email protected]>
CC Kevin Hilman <[email protected]>
Cc Colin Cross <[email protected]>
---
?arch/arm/mach-omap2/Kconfig ? ? ? | ? ?1 + ?arch/arm/mach-omap2/cpuidle44xx.c | ?167 ++++++++++++++++++++++--------------- ?2 files changed, 101 insertions(+), 67 deletions(-)

Quoted 12 lines

Quoted 9 lines
arch/arm/mach-omap2/Kconfig +0 -0
index e20c8ab..250786e 100644
@@ -54,6 +54,7 @@ config ARCH_OMAP4
? ? ? ?select PM_OPP if PM
? ? ? ?select USB_ARCH_HAS_EHCI
? ? ? ?select ARM_CPU_SUSPEND if PM
+ ? ? ? select ARCH_NEEDS_CPU_IDLE_COUPLED if CPU_IDLE
The "if CPU_IDLE" is not necessary, ARCH_NEEDS_CPU_IDLE_COUPLED is designed to have no effect if CPU_IDLE is not set.

OK. Will drop that if then.

Quoted 69 lines

Quoted 69 lines
?comment "OMAP Core Type"
? ? ? ?depends on ARCH_OMAP2
arch/arm/mach-omap2/cpuidle44xx.c +0 -0
index f386cbe..5724393 100644
@@ -21,6 +21,7 @@
?#include "common.h"
?#include "pm.h"
?#include "prm.h"
+#include "clockdomain.h"
?#ifdef CONFIG_CPU_IDLE
arch/arm/mach-omap2/cpuidle44xx.c +2 -1
@@ -44,10 +45,11 @@ static struct cpuidle_params cpuidle_params_table[] = {
 ?#define OMAP4_NUM_STATES ARRAY_SIZE(cpuidle_params_table)
 
 ?struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES];
-static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;
+static struct powerdomain *mpu_pd, *cpu_pd[NR_CPUS];
+static struct clockdomain *cpu_clkdm[NR_CPUS];
 
?/**
- * omap4_enter_idle - Programs OMAP4 to enter the specified state
+ * omap4_enter_idle_coupled_[simple/coupled] - OMAP4 cpuidle entry functions ?* @dev: cpuidle device
?* @drv: cpuidle driver
?* @index: the index of state to be entered
arch/arm/mach-omap2/cpuidle44xx.c +0 -0
@@ -56,34 +58,40 @@ static struct powerdomain *mpu_pd, *cpu0_pd, *cpu1_pd;
?* specified low power state selected by the governor. ?* Returns the amount of time spent in the low power state. ?*/
arch/arm/mach-omap2/cpuidle44xx.c +12 -1
-static int omap4_enter_idle(struct cpuidle_device *dev,
+static int omap4_enter_idle_simple(struct cpuidle_device *dev,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_driver *drv,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int index)
+{
+ ? ? ? local_fiq_disable();
+ ? ? ? omap_do_wfi();
+ ? ? ? local_fiq_enable();
+
+ ? ? ? return index;
+}
+
+static int omap4_enter_idle_coupled(struct cpuidle_device *dev,
? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_driver *drv,
? ? ? ? ? ? ? ? ? ? ? ?int index)
?{
? ? ? ?struct omap4_idle_statedata *cx =
? ? ? ? ? ? ? ? ? ? ? ?cpuidle_get_statedata(&dev->states_usage[index]);
- ? ? ? u32 cpu1_state;
? ? ? ?int cpu_id = smp_processor_id();
? ? ? ?local_fiq_disable();
+ ? ? ? clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id); +
? ? ? ?/*
- ? ? ? ?* CPU0 has to stay ON (i.e in C1) until CPU1 is OFF state.
+ ? ? ? ?* CPU0 has to wait and stay ON until CPU1 is OFF state. ? ? ? ? * This is necessary to honour hardware recommondation ? ? ? ? * of triggeing all the possible low power modes once CPU1 is ? ? ? ? * out of coherency and in OFF mode.
- ? ? ? ?* Update dev->last_state so that governor stats reflects right
- ? ? ? ?* data.
? ? ? ? */
- ? ? ? cpu1_state = pwrdm_read_pwrst(cpu1_pd);
- ? ? ? if (cpu1_state != PWRDM_POWER_OFF) {

Quoted 9 lines

Quoted 5 lines
- ? ? ? ? ? ? ? index = drv->safe_state_index;
- ? ? ? ? ? ? ? cx = cpuidle_get_statedata(&dev->states_usage[index]);
+ ? ? ? if (dev->cpu == 0) {
+ ? ? ? ? ? ? ? while (pwrdm_read_pwrst(cpu_pd[1]) != PWRDM_POWER_OFF)
+ ? ? ? ? ? ? ? ? ? ? ? cpu_relax();
If something goes wrong in the core coupled code or in the cpu 1 power state transition, this will hang forever and be hard to debug. ?It might be worth adding a timeout with a BUG_ON.

This condition is handled in patch 3/3

Quoted 15 lines

arch/arm/mach-omap2/cpuidle44xx.c static int omap4_enter_idle(struct cpuidle_device *dev, · line 100
Quoted 11 lines
? ? ? ?}
- ? ? ? if (index > 0)
- ? ? ? ? ? ? ? clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id); -
? ? ? ?/*
? ? ? ? * Call idle CPU PM enter notifier chain so that
? ? ? ? * VFP and per CPU interrupt context is saved.
arch/arm/mach-omap2/cpuidle44xx.c +0 -0
@@ -91,25 +99,35 @@ static int omap4_enter_idle(struct cpuidle_device *dev,
 ? ? ? ?if (cx->cpu_state == PWRDM_POWER_OFF)
 ? ? ? ? ? ? ? ?cpu_pm_enter();
This should never get called without cpu_state == PWRDM_POWER_OFF, and even if it did, calling cpu_pm_enter shouldn't hurt anything. ?It would be clearer to unconditionally call cpu_pm_enter().

Actually coupled state is called only for CPU mode, so the check can be removed.

Quoted 26 lines

arch/arm/mach-omap2/cpuidle44xx.c +12 ~~-10~~

- ? ? ? pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
- ? ? ? omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
-
- ? ? ? /*
- ? ? ? ?* Call idle CPU cluster PM enter notifier chain
- ? ? ? ?* to save GIC and wakeupgen context.
- ? ? ? ?*/
- ? ? ? if ((cx->mpu_state == PWRDM_POWER_RET) &&
- ? ? ? ? ? ? ? (cx->mpu_logic_state == PWRDM_POWER_OFF))
- ? ? ? ? ? ? ? ? ? ? ? cpu_cluster_pm_enter();
+ ? ? ? if (dev->cpu == 0) {
+ ? ? ? ? ? ? ? pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state);
+ ? ? ? ? ? ? ? omap_set_pwrdm_state(mpu_pd, cx->mpu_state);
+
+ ? ? ? ? ? ? ? /*
+ ? ? ? ? ? ? ? ?* Call idle CPU cluster PM enter notifier chain
+ ? ? ? ? ? ? ? ?* to save GIC and wakeupgen context.
+ ? ? ? ? ? ? ? ?*/
+ ? ? ? ? ? ? ? if ((cx->mpu_state == PWRDM_POWER_RET) &&
+ ? ? ? ? ? ? ? ? ? ? ? (cx->mpu_logic_state == PWRDM_POWER_OFF))
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cpu_cluster_pm_enter();
+ ? ? ? }
 
 ? ? ? ?omap4_enter_lowpower(dev->cpu, cx->cpu_state);

Quoted 15 lines

Quoted 8 lines
arch/arm/mach-omap2/cpuidle44xx.c +8 -0
+ ? ? ? if (dev->cpu == 0) {
+ ? ? ? ? ? ? ? /* Wakeup CPU1 only if it is not offlined */
+ ? ? ? ? ? ? ? if (cpumask_test_cpu(1, cpu_online_mask)) {
+ ? ? ? ? ? ? ? ? ? ? ? clkdm_wakeup(cpu_clkdm[1]);
+ ? ? ? ? ? ? ? ? ? ? ? clkdm_allow_idle(cpu_clkdm[1]);
+ ? ? ? ? ? ? ? }
+ ? ? ? }
+
Quoted 5 lines
? ? ? ?/*
? ? ? ? * Call idle CPU PM exit notifier chain to restore ? ? ? ? * VFP and per CPU IRQ context. Only CPU0 state is ? ? ? ? * considered since CPU1 is managed by CPU hotplug. ? ? ? ? */
This comment is no longer accurate? ?cpu_pm_enter is called on cpu 1 above.

Yep. Didn't pay much attention on the comments. Will fix it.

Quoted 8 lines

Quoted 3 lines
- ? ? ? if (pwrdm_read_prev_pwrst(cpu0_pd) == PWRDM_POWER_OFF)
+ ? ? ? if (pwrdm_read_prev_pwrst(cpu_pd[dev->cpu]) == PWRDM_POWER_OFF) ? ? ? ? ? ? ? ?cpu_pm_exit();
This should get called unconditionally. ?It's not explicitly stated, but the cpu_pm_* api expects cpu_pm_exit() to be called after cpu_pm_enter(), even if the low power state was not entered. Otherwise, a cpu_pm_enter notifier that disables the hardware will not get a chance to re-enable it.

I know what you mean now. the hardware like CPU interface can be disabled/enabled in the notifiers so would make sense to call them unconditionally. Will remove the check in both cases.

Regards
Santosh

Santosh Shilimkar

author

2012-03-30 13:27 UTC #20 lore raw

PATCH 3/3 ARM: OMAP4: CPUidle: add synchronization for coupled idle states

In reply to Santosh Shilimkar

From: Kevin Hilman <[email protected]>

With coupled idle states, a failure for any CPU to hit a low power state must be coordinated such that all CPUs abort.

On OMAP4, when entering a coupled state, CPU0 has to wait for CPU1 to enter its low power state before it can enter its low power state.

This is implemented by letting CPU0 wait for the CPU1 powerdomain to hit off. However, there are conditions where CPU1 might abort/fail and not hit off while CPU0 is waiting for it. For example, a CPU1 wakeup or a failed attempt to hit off due to hardware conditions.

To avoid the deadlock where CPU0 would continually wait for CPU1 to hit off-mode, this patch adds a flag to signal when each CPU has come out of its low-power state. CPU0 then checks whether CPU1 has hit off *or* has already completed its attempt to hit off. If the latter, CPU0 must abort its attempt to hit a low-power state so the coupled state enter method can return.

In addition, cpuidle_coupled_parallel_barrier() is used to ensure the clearing of the 'done' flag is synchronized on all CPUs.

Cc Colin Cross <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

---
 arch/arm/mach-omap2/cpuidle44xx.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

arch/arm/mach-omap2/cpuidle44xx.c +21 -1

index 5724393..fcf6653 100644
@@ -48,6 +48,9 @@ struct omap4_idle_statedata omap4_idle_data[OMAP4_NUM_STATES];
 static struct powerdomain *mpu_pd, *cpu_pd[NR_CPUS];
 static struct clockdomain *cpu_clkdm[NR_CPUS];
 
+static atomic_t abort_barrier;
+static bool cpu_done[NR_CPUS];
+
 /**
  * omap4_enter_idle_coupled_[simple/coupled] - OMAP4 cpuidle entry functions
  * @dev: cpuidle device

@@ -88,8 +91,20 @@ static int omap4_enter_idle_coupled(struct cpuidle_device *dev,
 	 * out of coherency and in OFF mode.
 	 */
 	if (dev->cpu == 0) {
-		while (pwrdm_read_pwrst(cpu_pd[1]) != PWRDM_POWER_OFF)
+		while (pwrdm_read_pwrst(cpu_pd[1]) != PWRDM_POWER_OFF) {
 			cpu_relax();
+
+			/*
+			 * CPU1 could have already entered & exited idle
+			 * without hitting off because of a wakeup
+			 * or a failed attempt to hit off mode.  Check for
+			 * that here, otherwise we could spin forever
+			 * waiting for CPU1 off.
+			 */
+			if (cpu_done[1])
+			    goto fail;
+
+		}
 	}
 
 	/*

@@ -113,6 +128,7 @@ static int omap4_enter_idle_coupled(struct cpuidle_device *dev,
 	}
 
 	omap4_enter_lowpower(dev->cpu, cx->cpu_state);
+	cpu_done[dev->cpu] = true;
 
 	if (dev->cpu == 0) {
 		/* Wakeup CPU1 only if it is not offlined */

@@ -137,6 +153,10 @@ static int omap4_enter_idle_coupled(struct cpuidle_device *dev,
 	if (omap4_mpuss_read_prev_context_state())
 		cpu_cluster_pm_exit();
 
+fail:
+	cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
+	cpu_done[dev->cpu] = false;
+
 	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id);
 
 	local_fiq_enable();

Signature · 1 line

1.7.5.4

Kevin Hilman

2012-04-03 05:04 UTC #21 lore raw

cover OMAP4: CPUidle: Add coupled idle support

In reply to Santosh Shilimkar

Hi Santosh,

Quoted 9 lines · Santosh Shilimkar

The series adds the coupled cpuidle support for OMAP4 based on the v2 series posted [1]. This makes OMAP4 to support SMP cpuidle and also removes the hard dependency of off-lining CPU1 to trigger deeper C-states.
I have put together a branch which is based on 3.3 kernel with Len Browns next branch [2] which has time keeping and other cpuidle patches which will mostly get merged by 3.4-rc1 and rebased coupled idle series from [1].

Thanks for rebasing this.

Quoted 2 lines

git://gitorious.org/omap-sw-develoment/linux-omap-dev.git
for_3.5/omap4_coupled_cpuidle-rebase

This branch by itself seems to work fine. However, when combining with other stuff that has merged for v3.4, it hangs during boot. I haven't yet isolated the problem, but it's easy to reproduce by combining your branch with v3.4-rc1:

git checkout -b test/coupled-v3.4 v3.4-rc1
git merge -s recursive -X ours santosh/for_3.5/omap4_coupled_cpuidle-rebase [1]

This hangs on boot, and it seems like a coupled state deadlock because commenting out the coupled states in the C-state creation of cpuidle44xx.c makes it boot just fine.

Kevin

[1] There were a couple conflicts in your branch with ACPI stuff from v3.4-rc1 (maybe Len updated his branch?), so I just threw them away with a recursive/ours merge.

Santosh Shilimkar

author

2012-04-03 15:06 UTC #22 lore raw

cover OMAP4: CPUidle: Add coupled idle support

Quoted 24 lines · Kevin Hilman

Hi Santosh,
Quoted 9 lines · Santosh Shilimkar
The series adds the coupled cpuidle support for OMAP4 based on the v2 series posted [1]. This makes OMAP4 to support SMP cpuidle and also removes the hard dependency of off-lining CPU1 to trigger deeper C-states.
I have put together a branch which is based on 3.3 kernel with Len Browns next branch [2] which has time keeping and other cpuidle patches which will mostly get merged by 3.4-rc1 and rebased coupled idle series from [1].
Thanks for rebasing this.
Quoted 2 lines
git://gitorious.org/omap-sw-develoment/linux-omap-dev.git
for_3.5/omap4_coupled_cpuidle-rebase
This branch by itself seems to work fine. However, when combining with other stuff that has merged for v3.4, it hangs during boot. I haven't yet isolated the problem, but it's easy to reproduce by combining your branch with v3.4-rc1:

Quoted 7 lines

git checkout -b test/coupled-v3.4 v3.4-rc1
git merge -s recursive -X ours santosh/for_3.5/omap4_coupled_cpuidle-rebase [1]
This hangs on boot, and it seems like a coupled state deadlock because commenting out the coupled states in the C-state creation of cpuidle44xx.c makes it boot just fine.

I managed to reproduce the issue. Just to ensure that any OMAP changes have not introduced the regression I merged all Tony's pull request on my branch and tried it out. OMAP changes are fine and coupled idle continue to work.

Started bisecting the commits. For bisect I have to create a series which is not dependent on Len's cpuidle updates. First round of bisect was not successful so tried one more time. Was very close to narrowing down on commit but then encountered set of commits where either CPUIDLE itself is broken(deeper C-states are not getting attempted) or I get softIRQ 08 pending error.

The last bad commit in bisect was ... [8682df25ca9afd3aac30f2c72d00bd98de2118e8] Merge branch 'fortglx/3.4/rtc' of git://git.linaro.org/people/jstultz/linux into timers/core

So far looks like, one of below series has introduced a race which is getting highlighted with coupled cpuidle patchset.

9b612fa Merge branch 'fortglx/3.4/clocksource' of
git://git.linaro.org/people/jstultz/linux into timers/core 8682df2 Merge branch 'fortglx/3.4/rtc' of
git://git.linaro.org/people/jstultz/linux into timers/core 97ac984 Merge branch 'fortglx/3.4/time' of
git://git.linaro.org/people/jstultz/linux into timers/core

Will try to continue the debug tomorrow. let me know if you find some thing interesting in your day time.

Regards
santosh

Santosh Shilimkar

author

2012-04-09 06:54 UTC #23 lore raw

cover OMAP4: CPUidle: Add coupled idle support

Quoted 52 lines · Santosh Shilimkar

Quoted 24 lines · Kevin Hilman
Hi Santosh,
Quoted 9 lines · Santosh Shilimkar
The series adds the coupled cpuidle support for OMAP4 based on the v2 series posted [1]. This makes OMAP4 to support SMP cpuidle and also removes the hard dependency of off-lining CPU1 to trigger deeper C-states.
I have put together a branch which is based on 3.3 kernel with Len Browns next branch [2] which has time keeping and other cpuidle patches which will mostly get merged by 3.4-rc1 and rebased coupled idle series from [1].
Thanks for rebasing this.
Quoted 2 lines
git://gitorious.org/omap-sw-develoment/linux-omap-dev.git
for_3.5/omap4_coupled_cpuidle-rebase
This branch by itself seems to work fine. However, when combining with other stuff that has merged for v3.4, it hangs during boot. I haven't yet isolated the problem, but it's easy to reproduce by combining your branch with v3.4-rc1:
Quoted 7 lines
git checkout -b test/coupled-v3.4 v3.4-rc1
git merge -s recursive -X ours santosh/for_3.5/omap4_coupled_cpuidle-rebase [1]
This hangs on boot, and it seems like a coupled state deadlock because commenting out the coupled states in the C-state creation of cpuidle44xx.c makes it boot just fine.
I managed to reproduce the issue. Just to ensure that any OMAP changes have not introduced the regression I merged all Tony's pull request on my branch and tried it out. OMAP changes are fine and coupled idle continue to work.
Started bisecting the commits. For bisect I have to create a series which is not dependent on Len's cpuidle updates. First round of bisect was not successful so tried one more time. Was very close to narrowing down on commit but then encountered set of commits where either CPUIDLE itself is broken(deeper C-states are not getting attempted) or I get softIRQ 08 pending error.
The last bad commit in bisect was ... [8682df25ca9afd3aac30f2c72d00bd98de2118e8] Merge branch 'fortglx/3.4/rtc' of git://git.linaro.org/people/jstultz/linux into timers/core
So far looks like, one of below series has introduced a race which is getting highlighted with coupled cpuidle patchset.

Quoted 7 lines

9b612fa Merge branch 'fortglx/3.4/clocksource' of
git://git.linaro.org/people/jstultz/linux into timers/core 8682df2 Merge branch 'fortglx/3.4/rtc' of
git://git.linaro.org/people/jstultz/linux into timers/core 97ac984 Merge branch 'fortglx/3.4/time' of
git://git.linaro.org/people/jstultz/linux into timers/core

Finally managed to crack down the issue.
The broad-cast clock-event was remaining in shut-down mode and hence we were loosing the OS tick after entering to low power states. The cases where it use to work was mainly because of external interrupts like NFS etc.

Have posted a patch [1] on LKML which fixes the issue. Waiting for feedback from Thomas on it.

Have updated the git tree with patch [1] + 3.4-rc2 and same is available here[2]

Regards
Santosh

[1] https://lkml.org/lkml/2012/4/9/13

[2] git://gitorious.org/omap-sw-develoment/linux-omap-dev.git
for_3.5/coupled_cpuidle-rebase

Shilimkar, Santosh

author

2012-04-17 10:23 UTC #24 lore raw

cover OMAP4: CPUidle: Add coupled idle support

Quoted 60 lines · Santosh Shilimkar

Quoted 52 lines · Santosh Shilimkar
Quoted 24 lines · Kevin Hilman
Hi Santosh,
Quoted 9 lines · Santosh Shilimkar
The series adds the coupled cpuidle support for OMAP4 based on the v2 series posted [1]. This makes OMAP4 to support SMP cpuidle and also removes the hard dependency of off-lining CPU1 to trigger deeper C-states.
I have put together a branch which is based on 3.3 kernel with Len Browns next branch [2] which has time keeping and other cpuidle patches which will mostly get merged by 3.4-rc1 and rebased coupled idle series from [1].
Thanks for rebasing this.
Quoted 2 lines
git://gitorious.org/omap-sw-develoment/linux-omap-dev.git
for_3.5/omap4_coupled_cpuidle-rebase
This branch by itself seems to work fine. ?However, when combining with other stuff that has merged for v3.4, it hangs during boot. ?I haven't yet isolated the problem, but it's easy to reproduce by combining your branch with v3.4-rc1:
Quoted 7 lines
? git checkout -b test/coupled-v3.4 v3.4-rc1
? git merge -s recursive -X ours santosh/for_3.5/omap4_coupled_cpuidle-rebase [1]
This hangs on boot, and it seems like a coupled state deadlock because commenting out the coupled states in the C-state creation of cpuidle44xx.c makes it boot just fine.
I managed to reproduce the issue. Just to ensure that any OMAP changes have not introduced the regression I merged all Tony's pull request on my branch and tried it out. OMAP changes are fine and coupled idle continue to work.
Started bisecting the commits. For bisect I have to create a series which is not dependent on Len's cpuidle updates. First round of bisect was not successful so tried one more time. Was very close to narrowing down on commit but then encountered set of commits where either CPUIDLE itself is broken(deeper C-states are not getting attempted) or I get softIRQ 08 pending error.
The last bad commit in bisect was ... [8682df25ca9afd3aac30f2c72d00bd98de2118e8] Merge branch 'fortglx/3.4/rtc' of git://git.linaro.org/people/jstultz/linux into timers/core
So far looks like, one of below series has introduced a race which is getting highlighted with coupled cpuidle patchset.
Quoted 7 lines
9b612fa Merge branch 'fortglx/3.4/clocksource' of
git://git.linaro.org/people/jstultz/linux into timers/core 8682df2 Merge branch 'fortglx/3.4/rtc' of
git://git.linaro.org/people/jstultz/linux into timers/core 97ac984 Merge branch 'fortglx/3.4/time' of
git://git.linaro.org/people/jstultz/linux into timers/core

Quoted 9 lines

Finally managed to crack down the issue.
The broad-cast clock-event was remaining in shut-down mode and hence we were loosing the OS tick after entering to low power states. The cases where it use to work was mainly because of external interrupts like NFS etc.
Have posted a patch [1] on LKML which fixes the issue. Waiting for feedback from Thomas on it.

Based on the discussion on patch [1], I ended up creating another patch(end of the email) for OMAP4 idle driver. Git tree is updated accordingly and available here [2]

Regards,
Santosh

[1] https://lkml.org/lkml/2012/4/9/13

[2] git://gitorious.org/omap-sw-develoment/linux-omap-dev.git
for_3.5/coupled_cpuidle-rebase

Quoted 1 line

From d36a31b21e3d839ff0ae440186874bfeb6e7edc1 Mon Sep 17 00:00:00 2001

From: Santosh Shilimkar <[email protected]>
Date: Tue, 17 Apr 2012 15:09:20 +0530
Subject: [PATCH] ARM: OMAP4: CPUidle: Open broadcast clock-event device.

OMAP4 idle driver uses CLOCK_EVT_NOTIFY_BROADCAST_[ENTER/EXIT] for broadcast clock events. But _ENTER/_EXIT doesn't really open broadcast clock events and to explicitly setup the broadcast device, CLOCK_EVT_NOTIFY_BROADCAST_ON should be used.

So far the broadcast device was opened up by generic code and hence OMAP4 idle was working without explicit CLOCK_EVT_NOTIFY_BROADCAST_ON.

Quoted 1 line

From commit 77b0d60 {clockevents: Leave the broadcast device in shutdown

mode}, the default setup is skipped so driver needs to open up the broadcast device.

Without this patch, boot might hang since CPU enters deeper idle state in which local timer stalls and broad cast timer is not armed.

Discussion thread :

	https://lkml.org/lkml/2012/4/9/13

Signed-off-by Santosh Shilimkar <[email protected]>

---
 arch/arm/mach-omap2/cpuidle44xx.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-omap2/cpuidle44xx.c
index ccc8e82..cf804e9 100644

arch/arm/mach-omap2/cpuidle44xx.c +1 -0

@@ -259,6 +259,7 @@ int __init omap4_idle_init(void)
 		dev->state_count++;
 		drv->state_count++;
 
+		clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu_id);
 		cpuidle_register_driver(&omap4_idle_driver);
 
 		if (cpuidle_register_device(dev)) {

Signature · 1 line

1.7.5.4