OMAP3: enabling CPU idle leads to panic

OMAP3: enabling CPU idle leads to panic

[Sergey Lapin]

Hi, all!

I use PM branch merged with linux-omap branch. A board is custom, so I
can't bisect this problem
(and I don't know latest working revision). It seems that's because
missing clock domain somewhere,


[16515.695617] Unable to handle kernel NULL pointer dereference at
virtual address 00000020
[16515.703735] pgd = c0004000
[16515.706481] [00000020] *pgd=00000000
[16515.710083] Internal error: Oops: 5 [#1] PREEMPT
[16515.714721] last sysfs file:
[16515.717712] Modules linked in:
[16515.720794] CPU: 0    Not tainted  (2.6.32-rc8-07113-g318afc6 #110)
[16515.727111] PC is at pwrdm_for_each_clkdm+0x34/0x88
[16515.731994] LR is at pwrdm_for_each_clkdm+0x24/0x88
[16515.736907] pc : [<c003b5c0>]    lr : [<c003b5b0>]    psr: 600000d3
[16515.736938] sp : c03f3f68  ip : fa004800  fp : 00000000
[16515.748474] r10: c003e81c  r9 : 411fc082  r8 : 00000000
[16515.753723] r7 : a00000d3  r6 : 00000000  r5 : 00000004  r4 : 00000001
[16515.760284] r3 : 00000002  r2 : c03f2000  r1 : 00000001  r0 : 00000000
[16515.766845] Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM
Segment kernel
[16515.774383] Control: 10c5387d  Table: 80004019  DAC: 00000017
[16515.780151] Process swapper (pid: 0, stack limit = 0xc03f22e8)
[16515.786010] Stack: (0xc03f3f68 to 0xc03f4000)
[16515.790405] 3f60:                   c042e4bc 00000003 00000003
c042e4bc 80026398 411fc082
[16515.798645] 3f80: 0000001f c003e76c 00000000 c022f284 00271d5a
00000000 00000005 0ec00c5c
[16515.806854] 3fa0: c03f8640 c03f8710 c042dadc c03f8640 c0474d0c
c022dee8 c03f2000 c042dadc
[16515.815093] 3fc0: c0027e14 c03f5bd0 80026398 c002f49c c0459fa4
c0008914 c0008478 00000000
[16515.823333] 3fe0: 00000000 c0027e18 00000000 10c53c7d c042dbf0
80008034 00000000 00000000
[16515.831573] [<c003b5c0>] (pwrdm_for_each_clkdm+0x34/0x88) from
[<c003e76c>] (omap3_enter_idle+0xc0/0x15c)
[16515.841217] [<c003e76c>] (omap3_enter_idle+0xc0/0x15c) from
[<c022dee8>] (cpuidle_idle_call+0xa4/0x180)
[16515.850677] [<c022dee8>] (cpuidle_idle_call+0xa4/0x180) from
[<c002f49c>] (cpu_idle+0x48/0x98)
[16515.859344] [<c002f49c>] (cpu_idle+0x48/0x98) from [<c0008914>]
(start_kernel+0x250/0x2a8)
[16515.867675] [<c0008914>] (start_kernel+0x250/0x2a8) from
[<80008034>] (0x80008034)
[16515.875274] Code: e3a06000 e1a05008 e1a04006 ea000002 (e595101c)
[16515.881530] ---[ end trace f0a89c74ea639f01 ]---
[16515.886230] Kernel panic - not syncing: Attempted to kill the idle task!


addr2line -e vmlinux c003b5c0
/home/slapin/linux-omap-my-2.6.git/arch/arm/mach-omap2/powerdomain.c:452
It is in function
int pwrdm_for_each_clkdm(), where pwrdm is NULL.
farther investigations found that "core_pwrdm" is defined only for 24xx,
but I use 3525, and it is being searched.

This is where variable allocated. If I add BUG_ON(!core_pd), it crashes here.
arch/arm/mach-omap2/cpuidle34xx.c:348:	core_pd = pwrdm_lookup("core_pwrdm");
Otherwise it crashes the above way. Any ideas on how to fix this?

S.

Re: OMAP3: enabling CPU idle leads to panic

[Kevin Hilman]

Sergey Lapin <slapinid@gmail.com> writes:

> I use PM branch merged with linux-omap branch. 

Not sure what this means.  PM branch is already based at linux-omap
master branch.  You'll need to be more specific, ideally with commit
IDs.

> A board is custom, so I can't bisect this problem (and I don't know
> latest working revision).
>
> It seems that's because missing clock domain somewhere,
>
>
> [16515.695617] Unable to handle kernel NULL pointer dereference at
> virtual address 00000020
> [16515.703735] pgd = c0004000
> [16515.706481] [00000020] *pgd=00000000
> [16515.710083] Internal error: Oops: 5 [#1] PREEMPT
> [16515.714721] last sysfs file:
> [16515.717712] Modules linked in:
> [16515.720794] CPU: 0    Not tainted  (2.6.32-rc8-07113-g318afc6 #110)
> [16515.727111] PC is at pwrdm_for_each_clkdm+0x34/0x88
> [16515.731994] LR is at pwrdm_for_each_clkdm+0x24/0x88
> [16515.736907] pc : [<c003b5c0>]    lr : [<c003b5b0>]    psr: 600000d3
> [16515.736938] sp : c03f3f68  ip : fa004800  fp : 00000000
> [16515.748474] r10: c003e81c  r9 : 411fc082  r8 : 00000000
> [16515.753723] r7 : a00000d3  r6 : 00000000  r5 : 00000004  r4 : 00000001
> [16515.760284] r3 : 00000002  r2 : c03f2000  r1 : 00000001  r0 : 00000000
> [16515.766845] Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM
> Segment kernel
> [16515.774383] Control: 10c5387d  Table: 80004019  DAC: 00000017
> [16515.780151] Process swapper (pid: 0, stack limit = 0xc03f22e8)
> [16515.786010] Stack: (0xc03f3f68 to 0xc03f4000)
> [16515.790405] 3f60:                   c042e4bc 00000003 00000003
> c042e4bc 80026398 411fc082
> [16515.798645] 3f80: 0000001f c003e76c 00000000 c022f284 00271d5a
> 00000000 00000005 0ec00c5c
> [16515.806854] 3fa0: c03f8640 c03f8710 c042dadc c03f8640 c0474d0c
> c022dee8 c03f2000 c042dadc
> [16515.815093] 3fc0: c0027e14 c03f5bd0 80026398 c002f49c c0459fa4
> c0008914 c0008478 00000000
> [16515.823333] 3fe0: 00000000 c0027e18 00000000 10c53c7d c042dbf0
> 80008034 00000000 00000000
> [16515.831573] [<c003b5c0>] (pwrdm_for_each_clkdm+0x34/0x88) from
> [<c003e76c>] (omap3_enter_idle+0xc0/0x15c)
> [16515.841217] [<c003e76c>] (omap3_enter_idle+0xc0/0x15c) from
> [<c022dee8>] (cpuidle_idle_call+0xa4/0x180)
> [16515.850677] [<c022dee8>] (cpuidle_idle_call+0xa4/0x180) from
> [<c002f49c>] (cpu_idle+0x48/0x98)
> [16515.859344] [<c002f49c>] (cpu_idle+0x48/0x98) from [<c0008914>]
> (start_kernel+0x250/0x2a8)
> [16515.867675] [<c0008914>] (start_kernel+0x250/0x2a8) from
> [<80008034>] (0x80008034)
> [16515.875274] Code: e3a06000 e1a05008 e1a04006 ea000002 (e595101c)
> [16515.881530] ---[ end trace f0a89c74ea639f01 ]---
> [16515.886230] Kernel panic - not syncing: Attempted to kill the idle task!
>
>
> addr2line -e vmlinux c003b5c0
> /home/slapin/linux-omap-my-2.6.git/arch/arm/mach-omap2/powerdomain.c:452
> It is in function
> int pwrdm_for_each_clkdm(), where pwrdm is NULL.
> farther investigations found that "core_pwrdm" is defined only for 24xx,
> but I use 3525, and it is being searched.
>
> This is where variable allocated. If I add BUG_ON(!core_pd), it crashes here.
> arch/arm/mach-omap2/cpuidle34xx.c:348:	core_pd = pwrdm_lookup("core_pwrdm");
> Otherwise it crashes the above way. Any ideas on how to fix this?

I think there is still ongoing work on support for the 3525.  I'm
guessing the clock/pwrdm init still needs work for that SoC.

I have no 3525 hardware, but maybe Sanjeev has more details.

Kevin

RE: OMAP3: enabling CPU idle leads to panic

[Premi, Sanjeev]

From: Kevin Hilman [khilman@deeprootsystems.com]
Sent: Tuesday, November 24, 2009 11:57 PM
To: Sergey Lapin; Premi, Sanjeev
Cc: linux-omap@vger.kernel.org
Subject: Re: OMAP3: enabling CPU idle leads to panic

Sergey Lapin <slapinid@gmail.com> writes:

> I use PM branch merged with linux-omap branch.

Not sure what this means.  PM branch is already based at linux-omap
master branch.  You'll need to be more specific, ideally with commit
IDs.

[sp] I will also need the last commit from pm branch - if not the id; the
commit header to start.

[snip]--[snip]

>
> addr2line -e vmlinux c003b5c0
> /home/slapin/linux-omap-my-2.6.git/arch/arm/mach-omap2/powerdomain.c:452
> It is in function
> int pwrdm_for_each_clkdm(), where pwrdm is NULL.
> farther investigations found that "core_pwrdm" is defined only for 24xx,
> but I use 3525, and it is being searched.

[sp] I am a little confused here. core_pwrdm is defined for 34xx as well.
Other than sgx, there isn't any difference between 3530 and 3525.
See the definition at:
http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=blob;f=arch/arm/mach-omap2/powerdomains34xx.h;hb=867d320b6c30d2478358eafeca0e1a6c60cf06c3#l203

>
> This is where variable allocated. If I add BUG_ON(!core_pd), it crashes here.
> arch/arm/mach-omap2/cpuidle34xx.c:348:        core_pd = pwrdm_lookup("core_pwrdm");
> Otherwise it crashes the above way. Any ideas on how to fix this?

[sp] Have you hand-picked the commits from pm branch and applied against
 master?

Best regards,
Sanjeev

PS: responding thru a web client. It doesn't seem to support quoting the original message.


I think there is still ongoing work on support for the 3525.  I'm
guessing the clock/pwrdm init still needs work for that SoC.

I have no 3525 hardware, but maybe Sanjeev has more details.

Kevin

Re: OMAP3: enabling CPU idle leads to panic

[Sergey Lapin]

On Tue, Nov 24, 2009 at 11:21 PM, Premi, Sanjeev <premi@ti.com> wrote:
> From: Kevin Hilman [khilman@deeprootsystems.com]
> Sent: Tuesday, November 24, 2009 11:57 PM
> To: Sergey Lapin; Premi, Sanjeev
> Cc: linux-omap@vger.kernel.org
> Subject: Re: OMAP3: enabling CPU idle leads to panic
>
> Sergey Lapin <slapinid@gmail.com> writes:
>
>> I use PM branch merged with linux-omap branch.
>
> Not sure what this means.  PM branch is already based at linux-omap
> master branch.  You'll need to be more specific, ideally with commit
> IDs.

I do git branch tmp origin/master
git checkout -f tmp
git merge origin/pm
git merge origin/mybranch

git log origin/pm^..origin/pm
commit 596f2d2c8153fe24de5c3b6ad78658ce9fff7f28
Author: Kevin Hilman <khilman@deeprootsystems.com>
Date:   Wed Jun 17 14:42:22 2009 -0700

    OMAP3: PM: default defconfig for PM kernel testing

    Supports 3430SDP, OMAP3EVM and 3630SDP by default.

    Also supports, Beagle, Overo, Zoom[23] and RX51 simply by changing

     System Type-->TI OMAP implementations-->Low-level debug console UART

    from UART1 to UART3.

    Signed-off-by: Kevin Hilman <khilman@ti.deeprootsystems.com>


>
> [sp] I will also need the last commit from pm branch - if not the id; the
> commit header to start.
See above

>
> [snip]--[snip]
>
>>
>> addr2line -e vmlinux c003b5c0
>> /home/slapin/linux-omap-my-2.6.git/arch/arm/mach-omap2/powerdomain.c:452
>> It is in function
>> int pwrdm_for_each_clkdm(), where pwrdm is NULL.
>> farther investigations found that "core_pwrdm" is defined only for 24xx,
>> but I use 3525, and it is being searched.
>
> [sp] I am a little confused here. core_pwrdm is defined for 34xx as well.
> Other than sgx, there isn't any difference between 3530 and 3525.
Yes, I see that in source, but that function returns NULL for unknown reason.
How could I debug it farther?


> See the definition at:
> http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=blob;f=arch/arm/mach-omap2/powerdomains34xx.h;hb=867d320b6c30d2478358eafeca0e1a6c60cf06c3#l203
>
Yes, there is.

>>
>> This is where variable allocated. If I add BUG_ON(!core_pd), it crashes here.
>> arch/arm/mach-omap2/cpuidle34xx.c:348:        core_pd = pwrdm_lookup("core_pwrdm");
>> Otherwise it crashes the above way. Any ideas on how to fix this?
>
> [sp] Have you hand-picked the commits from pm branch and applied against
>  master?
See above. I use git merge on fresh branch based on linux-omap master.

Thanks a lot,
S.

Re: OMAP3: enabling CPU idle leads to panic

[Kevin Hilman]

Sergey Lapin <slapinid@gmail.com> writes:

> On Tue, Nov 24, 2009 at 11:21 PM, Premi, Sanjeev <premi@ti.com> wrote:
>> From: Kevin Hilman [khilman@deeprootsystems.com]
>> Sent: Tuesday, November 24, 2009 11:57 PM
>> To: Sergey Lapin; Premi, Sanjeev
>> Cc: linux-omap@vger.kernel.org
>> Subject: Re: OMAP3: enabling CPU idle leads to panic
>>
>> Sergey Lapin <slapinid@gmail.com> writes:
>>
>>> I use PM branch merged with linux-omap branch.
>>
>> Not sure what this means.  PM branch is already based at linux-omap
>> master branch.  You'll need to be more specific, ideally with commit
>> IDs.
>
> I do git branch tmp origin/master
> git checkout -f tmp
> git merge origin/pm

The pm branch is already based on l-o master, but perhaps not the very
latest.  Instead of the above can you just do

git branch tmp origin/pm
git checkout -f tmp

> git merge origin/mybranch

It might if you base your 'mybranch' at the same merge point that the
pm branch is based at.  You can find the merge point using:

git merge-base origin/master origin/pm

Kevin

Re: OMAP3: enabling CPU idle leads to panic

[Sergey Lapin]

Sorry, I forgot to reply to list with this email.

On Wed, Nov 25, 2009 at 8:31 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Sergey Lapin <slapinid@gmail.com> writes:
>
>> On Tue, Nov 24, 2009 at 11:21 PM, Premi, Sanjeev <premi@ti.com> wrote:
>>> From: Kevin Hilman [khilman@deeprootsystems.com]
>>> Sent: Tuesday, November 24, 2009 11:57 PM
>>> To: Sergey Lapin; Premi, Sanjeev
>>> Cc: linux-omap@vger.kernel.org
>>> Subject: Re: OMAP3: enabling CPU idle leads to panic
>>>
>>> Sergey Lapin <slapinid@gmail.com> writes:
>>>
>>>> I use PM branch merged with linux-omap branch.
>>>
>>> Not sure what this means.  PM branch is already based at linux-omap
>>> master branch.  You'll need to be more specific, ideally with commit
>>> IDs.
>>
>> I do git branch tmp origin/master
>> git checkout -f tmp
>> git merge origin/pm
>
> The pm branch is already based on l-o master, but perhaps not the very
> latest.  Instead of the above can you just do
>
> git branch tmp origin/pm
> git checkout -f tmp
>
>> git merge origin/mybranch
>
> It might if you base your 'mybranch' at the same merge point that the
> pm branch is based at.  You can find the merge point using:
>
> git merge-base origin/master origin/pm

Well, thanks for advice, I changed my merge script accordingly, but
nothing changed. I still get panic.

[   21.591064] Unable to handle kernel NULL pointer dereference at
virtual address 00000020
[   21.599212] pgd = c0004000
[   21.601928] [00000020] *pgd=00000000
[   21.605529] Internal error: Oops: 5 [#1] PREEMPT
[   21.610168] last sysfs file:
[   21.613159] Modules linked in:
[   21.616241] CPU: 0    Not tainted  (2.6.32-rc8-07040-g4594b2e #117)
[   21.622558] PC is at pwrdm_for_each_clkdm+0x34/0x88
[   21.627441] LR is at pwrdm_for_each_clkdm+0x24/0x88
[   21.632354] pc : [<c003b5c0>]    lr : [<c003b5b0>]    psr: 600000d3
[   21.632385] sp : c03f3f68  ip : fa004800  fp : 00000000
[   21.643920] r10: c003e840  r9 : 411fc082  r8 : 00000000
[   21.649169] r7 : a00000d3  r6 : 00000000  r5 : 00000004  r4 : 00000001
[   21.655731] r3 : 00000002  r2 : c03f2000  r1 : 00000001  r0 : 00000000
[   21.662292] Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM
Segment kernel
[   21.669830] Control: 10c5387d  Table: 80004019  DAC: 00000017
[   21.675598] Process swapper (pid: 0, stack limit = 0xc03f22e8)
[   21.681457] Stack: (0xc03f3f68 to 0xc03f4000)
[   21.685852] 3f60:                   c042e4bc 00000003 00000003
c042e4bc 800263f4 411fc082
[   21.694091] 3f80: 0000001f c003e790 00000000 c022f3b8 000253d2
00000000 00000005 0ee4d5e4
[   21.702301] 3fa0: c03f8640 c03f8650 c042dadc c03f8640 c0474d4c
c022e01c c03f2000 c042dadc
[   21.710540] 3fc0: c0027e14 c03f5bd0 800263f4 c002f49c c0459fe4
c0008914 c0008478 00000000
[   21.718780] 3fe0: 00000000 c0027e18 00000000 10c53c7d c042dbf0
80008034 00000000 00000000
[   21.727020] [<c003b5c0>] (pwrdm_for_each_clkdm+0x34/0x88) from
[<c003e790>] (omap3_enter_idle+0xc0/0x15c)
[   21.736663] [<c003e790>] (omap3_enter_idle+0xc0/0x15c) from
[<c022e01c>] (cpuidle_idle_call+0xa4/0x180)
[   21.746124] [<c022e01c>] (cpuidle_idle_call+0xa4/0x180) from
[<c002f49c>] (cpu_idle+0x48/0x98)
[   21.754791] [<c002f49c>] (cpu_idle+0x48/0x98) from [<c0008914>]
(start_kernel+0x250/0x2a8)
[   21.763122] [<c0008914>] (start_kernel+0x250/0x2a8) from
[<80008034>] (0x80008034)
[   21.770751] Code: e3a06000 e1a05008 e1a04006 ea000002 (e595101c)
[   21.777008] ---[ end trace 2d4ee201e3556cfe ]---
[   21.781707] Kernel panic - not syncing: Attempted to kill the idle task!

If I add the following patch:

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
b/arch/arm/mach-omap2/cpuidle34xx.c
index 1cfa5a6..6a12e53 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -346,6 +346,7 @@ int __init omap3_idle_init(void)

       mpu_pd = pwrdm_lookup("mpu_pwrdm");
       core_pd = pwrdm_lookup("core_pwrdm");
+       BUG_ON(!core_pd);

       omap_init_power_states();
       cpuidle_register_driver(&omap3_idle_driver);

kernel dies right on BUG_ON.
Also, I patched pwrdm_lookup to print all perdms if returning NULL,
ant there was no core_pwrdm there :(

Is core_pwrdm used only with idle? When I disable cpu idle,
I have no panics.

Update: I continued my investigation.
I added the following patch to kernel:
diff --git a/arch/arm/mach-omap2/powerdomain.c
b/arch/arm/mach-omap2/powerdomain.c
index b6990e3..35a5bd0 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -185,7 +185,15 @@ void pwrdm_init(struct powerdomain **pwrdm_list)

        if (pwrdm_list) {
                for (p = pwrdm_list; *p; p++) {
-                       pwrdm_register(*p);
+#ifdef CONFIG_DEBUG_LL
+                       int i;
+                       extern void printascii(char *);
+                       printascii((*p)->name);
+                       printascii("\n");
+#endif
+                       i = pwrdm_register(*p);
+                       if (i < 0)
+                               printascii("Damn\n");
                        _pwrdm_setup(*p);
                }
        }


And got the following output with CONFIG_DEBUG_LL enabled:
gfx_pwrdm
Damn
wkup_pwrdm
iva2_pwrdm
mpu_pwrdm
neon_pwrdm
core_pwrdm
Damn
core_pwrdm
Damn
cam_pwrdm
dss_pwrdm
per_pwrdm
emu_pwrdm
sgx_pwrdm
Damn
usbhost_pwrdm
Damn
dpll1_pwrdm
dpll2_pwrdm
dpll3_pwrdm
dpll4_pwrdm
dpll5_pwrdm
Damn
<3>clockdomain: cm_clkdm: powerdomain core_pwrdm does not exist
<3>clockdomain: d2d_clkdm: powerdomain core_pwrdm does not exist
<3>clockdomain: core_l3_clkdm: powerdomain core_pwrdm does not exist
<3>clockdomain: core_l4_clkdm: powerdomain core_pwrdm does not exist
<6>Clocking rate (Crystal/Core/MPU): 26.0/166/500 MHz
<6>Reprogramming SDRC clock to 166000000 Hz

So, as I see here, core_pwrdm and some others are failed to register.
Any ideas?

S.


S.

Re: OMAP3: enabling CPU idle leads to panic

[Sergey Lapin]

Hi, all!

I've found the particular reason for my troubles with CPU idle,
since there's no core_pwrdm defined for OMAP3525 CPU.

How could I create soch structure or is there anybody having this handy?
3523 is basically 3530 without 3D graphics.

RE: OMAP3: enabling CPU idle leads to panic

[Premi, Sanjeev]

> -----Original Message-----
> From: Sergey Lapin [mailto:slapinid@gmail.com] 
> Sent: Friday, November 27, 2009 4:19 PM
> To: Kevin Hilman
> Cc: Premi, Sanjeev; linux-omap@vger.kernel.org
> Subject: Re: OMAP3: enabling CPU idle leads to panic
> 
> Hi, all!
> 
> I've found the particular reason for my troubles with CPU idle,
> since there's no core_pwrdm defined for OMAP3525 CPU.
> 
> How could I create soch structure or is there anybody having 
> this handy?
> 3523 is basically 3530 without 3D graphics.
> 
The core_pwdm is defined in powerdomains34xx.h based on
CONFIG_ARCH_OMAP34XX. This should be true for OMAP3525 as well.

~sanjeev

Re: OMAP3: enabling CPU idle leads to panic

[Sergey Lapin]

On Fri, Nov 27, 2009 at 2:02 PM, Premi, Sanjeev <premi@ti.com> wrote:
>> -----Original Message-----
>> From: Sergey Lapin [mailto:slapinid@gmail.com]
>> Sent: Friday, November 27, 2009 4:19 PM
>> To: Kevin Hilman
>> Cc: Premi, Sanjeev; linux-omap@vger.kernel.org
>> Subject: Re: OMAP3: enabling CPU idle leads to panic
>>
>> Hi, all!
>>
>> I've found the particular reason for my troubles with CPU idle,
>> since there's no core_pwrdm defined for OMAP3525 CPU.
>>
>> How could I create soch structure or is there anybody having
>> this handy?
>> 3523 is basically 3530 without 3D graphics.
>>
> The core_pwdm is defined in powerdomains34xx.h based on
> CONFIG_ARCH_OMAP34XX. This should be true for OMAP3525 as well.

Alas, that's not because omap_chip_is() function fails
on these structures.

Re: OMAP3: enabling CPU idle leads to panic

[Sergey Lapin]

>
> Alas, that's not because omap_chip_is() function fails
> on these structures.
>

I mean during registration of power domains.

Re: OMAP3: enabling CPU idle leads to panic

[Sergey Lapin]

On Fri, Nov 27, 2009 at 2:36 PM, Sergey Lapin <slapinid@gmail.com> wrote:
>>
>> Alas, that's not because omap_chip_is() function fails
>> on these structures.
>>
>
> I mean during registration of power domains.
>

int pwrdm_register(struct powerdomain *pwrdm)
{
        unsigned long flags;
        int ret = -EINVAL;

        if (!pwrdm)
                return -EINVAL;

        if (!omap_chip_is(pwrdm->omap_chip))
                return -EINVAL;
returns -EINVAL for pwrdm_core (both of them), so I get panic later.
Since OMAP2525 vs  3515 were messed-up, I think there could be more
problems there.

diff --git a/arch/arm/plat-omap/include/plat/cpu.h
b/arch/arm/plat-omap/include/plat/cpu.h
index 2e17890..8b15789 100644
--- a/arch/arm/plat-omap/include/plat/cpu.h
+++ b/arch/arm/plat-omap/include/plat/cpu.h
@@ -393,11 +393,11 @@ IS_OMAP_TYPE(3517, 0x3517)
                                                (!omap3_has_iva()) &&   \
                                                (!omap3_has_sgx()))
 # define cpu_is_omap3515()             (cpu_is_omap3430() &&           \
-                                               (omap3_has_iva()) &&    \
-                                               (!omap3_has_sgx()))
+                                               (!omap3_has_iva()) &&   \
+                                               (omap3_has_sgx()))
 # define cpu_is_omap3525()             (cpu_is_omap3430() &&           \
-                                               (omap3_has_sgx()) &&    \
-                                               (!omap3_has_iva()))
+                                               (!omap3_has_sgx()) &&   \
+                                               (omap3_has_iva()))
 # define cpu_is_omap3530()             (cpu_is_omap3430())
 # define cpu_is_omap3505()             is_omap3505()
 # define cpu_is_omap3517()             is_omap3517()

since OMAP3525 has iva but misses sgx, and vice versa 3515 has sgx but
has no iva.
I missed this piece before, but just accidentally found it now. I
don't know if these are
related, but this doesn't fix my problem with core_pwrdm.

S.

RE: OMAP3: enabling CPU idle leads to panic

[Premi, Sanjeev]

 

> -----Original Message-----
> From: Sergey Lapin [mailto:slapinid@gmail.com] 
> Sent: Friday, November 27, 2009 5:33 PM
> To: Premi, Sanjeev
> Cc: Kevin Hilman; linux-omap@vger.kernel.org
> Subject: Re: OMAP3: enabling CPU idle leads to panic
> 
> On Fri, Nov 27, 2009 at 2:36 PM, Sergey Lapin 
> <slapinid@gmail.com> wrote:
> >>
> >> Alas, that's not because omap_chip_is() function fails
> >> on these structures.
> >>
> >
> > I mean during registration of power domains.
> >
> 
> int pwrdm_register(struct powerdomain *pwrdm)
> {
>         unsigned long flags;
>         int ret = -EINVAL;
> 
>         if (!pwrdm)
>                 return -EINVAL;
> 
>         if (!omap_chip_is(pwrdm->omap_chip))
>                 return -EINVAL;
> returns -EINVAL for pwrdm_core (both of them), so I get panic later.
> Since OMAP2525 vs  3515 were messed-up, I think there could be more
> problems there.
> 
> diff --git a/arch/arm/plat-omap/include/plat/cpu.h
> b/arch/arm/plat-omap/include/plat/cpu.h
> index 2e17890..8b15789 100644
> --- a/arch/arm/plat-omap/include/plat/cpu.h
> +++ b/arch/arm/plat-omap/include/plat/cpu.h
> @@ -393,11 +393,11 @@ IS_OMAP_TYPE(3517, 0x3517)
>                                                 
> (!omap3_has_iva()) &&   \
>                                                 (!omap3_has_sgx()))
>  # define cpu_is_omap3515()             (cpu_is_omap3430() && 
>           \
> -                                               
> (omap3_has_iva()) &&    \
> -                                               (!omap3_has_sgx()))
> +                                               
> (!omap3_has_iva()) &&   \
> +                                               (omap3_has_sgx()))
>  # define cpu_is_omap3525()             (cpu_is_omap3430() && 
>           \
> -                                               
> (omap3_has_sgx()) &&    \
> -                                               (!omap3_has_iva()))
> +                                               
> (!omap3_has_sgx()) &&   \
> +                                               (omap3_has_iva()))
>  # define cpu_is_omap3530()             (cpu_is_omap3430())
>  # define cpu_is_omap3505()             is_omap3505()
>  # define cpu_is_omap3517()             is_omap3517()
> 
> since OMAP3525 has iva but misses sgx, and vice versa 3515 has sgx but
> has no iva.
> I missed this piece before, but just accidentally found it now. I
> don't know if these are
> related, but this doesn't fix my problem with core_pwrdm.
> 
> S.

I feel, the problem could be due to this snippet below.
omap_rev() does a compare against omap_revision not just
the revision bits.

Had submitted a patch earlier; will need some time to
find it.

	} else if (cpu_is_omap343x()) {
		omap_chip.oc = CHIP_IS_OMAP3430;
		if (omap_rev() == OMAP3430_REV_ES1_0)
			omap_chip.oc |= CHIP_IS_OMAP3430ES1;
		else if (omap_rev() >= OMAP3430_REV_ES2_0 &&
			 omap_rev() <= OMAP3430_REV_ES2_1)
			omap_chip.oc |= CHIP_IS_OMAP3430ES2;
		else if (omap_rev() == OMAP3430_REV_ES3_0)
			omap_chip.oc |= CHIP_IS_OMAP3430ES3_0;
		else if (omap_rev() == OMAP3430_REV_ES3_1)
			omap_chip.oc |= CHIP_IS_OMAP3430ES3_1;
	} else {

In meantime, can you try setting the following (explicitly)
to the code above:
		else if (omap_rev() == OMAP3430_REV_ES3_1)
			omap_chip.oc |= CHIP_IS_OMAP3430ES3_1;
+		else
+			omap_chip.oc |= CHIP_IS_OMAP3430ES3_1

~sanjeev
> 

Re: OMAP3: enabling CPU idle leads to panic

[Sergey Lapin]

>> -----Original Message-----
>> From: Sergey Lapin [mailto:slapinid@gmail.com]
>> Sent: Friday, November 27, 2009 5:33 PM
>> To: Premi, Sanjeev
>> Cc: Kevin Hilman; linux-omap@vger.kernel.org
>> Subject: Re: OMAP3: enabling CPU idle leads to panic
>>
>> On Fri, Nov 27, 2009 at 2:36 PM, Sergey Lapin
>> <slapinid@gmail.com> wrote:
>> >>
>> >> Alas, that's not because omap_chip_is() function fails
>> >> on these structures.
>> >>
>> >
>> > I mean during registration of power domains.
>> >
>>
>> int pwrdm_register(struct powerdomain *pwrdm)
>> {
>>         unsigned long flags;
>>         int ret = -EINVAL;
>>
>>         if (!pwrdm)
>>                 return -EINVAL;
>>
>>         if (!omap_chip_is(pwrdm->omap_chip))
>>                 return -EINVAL;
>> returns -EINVAL for pwrdm_core (both of them), so I get panic later.
>> Since OMAP2525 vs  3515 were messed-up, I think there could be more
>> problems there.
>>
>> diff --git a/arch/arm/plat-omap/include/plat/cpu.h
>> b/arch/arm/plat-omap/include/plat/cpu.h
>> index 2e17890..8b15789 100644
>> --- a/arch/arm/plat-omap/include/plat/cpu.h
>> +++ b/arch/arm/plat-omap/include/plat/cpu.h
>> @@ -393,11 +393,11 @@ IS_OMAP_TYPE(3517, 0x3517)
>>
>> (!omap3_has_iva()) &&   \
>>                                                 (!omap3_has_sgx()))
>>  # define cpu_is_omap3515()             (cpu_is_omap3430() &&
>>           \
>> -
>> (omap3_has_iva()) &&    \
>> -                                               (!omap3_has_sgx()))
>> +
>> (!omap3_has_iva()) &&   \
>> +                                               (omap3_has_sgx()))
>>  # define cpu_is_omap3525()             (cpu_is_omap3430() &&
>>           \
>> -
>> (omap3_has_sgx()) &&    \
>> -                                               (!omap3_has_iva()))
>> +
>> (!omap3_has_sgx()) &&   \
>> +                                               (omap3_has_iva()))
>>  # define cpu_is_omap3530()             (cpu_is_omap3430())
>>  # define cpu_is_omap3505()             is_omap3505()
>>  # define cpu_is_omap3517()             is_omap3517()
>>
>> since OMAP3525 has iva but misses sgx, and vice versa 3515 has sgx but
>> has no iva.
>> I missed this piece before, but just accidentally found it now. I
>> don't know if these are
>> related, but this doesn't fix my problem with core_pwrdm.
>>
>> S.
>
> I feel, the problem could be due to this snippet below.
> omap_rev() does a compare against omap_revision not just
> the revision bits.
>
> Had submitted a patch earlier; will need some time to
> find it.
>
>        } else if (cpu_is_omap343x()) {
>                omap_chip.oc = CHIP_IS_OMAP3430;
>                if (omap_rev() == OMAP3430_REV_ES1_0)
>                        omap_chip.oc |= CHIP_IS_OMAP3430ES1;
>                else if (omap_rev() >= OMAP3430_REV_ES2_0 &&
>                         omap_rev() <= OMAP3430_REV_ES2_1)
>                        omap_chip.oc |= CHIP_IS_OMAP3430ES2;
>                else if (omap_rev() == OMAP3430_REV_ES3_0)
>                        omap_chip.oc |= CHIP_IS_OMAP3430ES3_0;
>                else if (omap_rev() == OMAP3430_REV_ES3_1)
>                        omap_chip.oc |= CHIP_IS_OMAP3430ES3_1;
>        } else {
>
> In meantime, can you try setting the following (explicitly)
> to the code above:
>                else if (omap_rev() == OMAP3430_REV_ES3_1)
>                        omap_chip.oc |= CHIP_IS_OMAP3430ES3_1;
> +               else
> +                       omap_chip.oc |= CHIP_IS_OMAP3430ES3_1
>
> ~sanjeev

It is fixed, no more panics, thanks a lot!

RE: OMAP3: enabling CPU idle leads to panic

[Premi, Sanjeev]

> -----Original Message-----
> From: Sergey Lapin [mailto:slapinid@gmail.com] 
> Sent: Friday, November 27, 2009 6:08 PM
> To: Premi, Sanjeev
> Cc: Kevin Hilman; linux-omap@vger.kernel.org
> Subject: Re: OMAP3: enabling CPU idle leads to panic
> 
> >> -----Original Message-----
> >> From: Sergey Lapin [mailto:slapinid@gmail.com]
> >> Sent: Friday, November 27, 2009 5:33 PM
> >> To: Premi, Sanjeev
> >> Cc: Kevin Hilman; linux-omap@vger.kernel.org
> >> Subject: Re: OMAP3: enabling CPU idle leads to panic
> >>
> >> On Fri, Nov 27, 2009 at 2:36 PM, Sergey Lapin
> >> <slapinid@gmail.com> wrote:
> >> >>
> >> >> Alas, that's not because omap_chip_is() function fails
> >> >> on these structures.
> >> >>
> >> >
> >> > I mean during registration of power domains.
> >> >
> >>

[snip]--[snip]

> > Had submitted a patch earlier; will need some time to
> > find it.
> >
> >        } else if (cpu_is_omap343x()) {
> >                omap_chip.oc = CHIP_IS_OMAP3430;
> >                if (omap_rev() == OMAP3430_REV_ES1_0)
> >                        omap_chip.oc |= CHIP_IS_OMAP3430ES1;
> >                else if (omap_rev() >= OMAP3430_REV_ES2_0 &&
> >                         omap_rev() <= OMAP3430_REV_ES2_1)
> >                        omap_chip.oc |= CHIP_IS_OMAP3430ES2;
> >                else if (omap_rev() == OMAP3430_REV_ES3_0)
> >                        omap_chip.oc |= CHIP_IS_OMAP3430ES3_0;
> >                else if (omap_rev() == OMAP3430_REV_ES3_1)
> >                        omap_chip.oc |= CHIP_IS_OMAP3430ES3_1;
> >        } else {
> >
> > In meantime, can you try setting the following (explicitly)
> > to the code above:
> >                else if (omap_rev() == OMAP3430_REV_ES3_1)
> >                        omap_chip.oc |= CHIP_IS_OMAP3430ES3_1;
> > +               else
> > +                       omap_chip.oc |= CHIP_IS_OMAP3430ES3_1
> >
> > ~sanjeev
> 
> It is fixed, no more panics, thanks a lot!
> 
Great! I will take ongoing touchscreen issues to closure and
submit a formal patch again.

~sanjeev