SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-16 Thread Kevin Hilman
Hi Santosh,

Using v3.3-rc3 and building an OMAP2+ kernel I noticed that CPUfreq
transitions fault because of the TWD cpufreq notifiers being called even
on OMAP3.

Of course, the TWD doesn't exist on OMAP3, but the TWD init is still
setting up notifiers.  I tried just adding a !cpu_is_omap44xx() check
in local_timer_setup(), but that didn't do it, so there's more timer
init that needs to be fixed up.  Can you have a look at this?

It's easy to reproduce by just manually triggering a frequency change
with the userspace governor.  Here's what I did on my 3430/n900:

# cd /sys/devices/system/cpu/cpu0/cpufreq
# cat scaling_available_frequencies
# echo 25 > scaling_setspeed   
[   13.785797] Unable to handle kernel paging request at virtual address 007e900
[   13.793426] pgd = cdc2   
[   13.796295] [007e9000] *pgd= 
[   13.800079] Internal error: Oops: 5 [#1] SMP 
[   13.804595] Modules linked in:   
[   13.807830] CPU: 0Not tainted  (3.3.0-rc3-pm+debug+initramfs #9) 
[   13.814544] PC is at twd_update_frequency+0x34/0x48  
[   13.819671] LR is at twd_update_frequency+0x10/0x48  
[   13.824829] pc : []lr : []psr: 6093  
[   13.824829] sp : ce311dd8  ip :   fp :   
[   13.836914] r10:   r9 : 0001  r8 : ce31  
[   13.842437] r7 : c0440458  r6 : c00137f8  r5 :   r4 : c0947a74   
[   13.849304] r3 :   r2 : 007e9000  r1 :   r0 :    
[   13.856201] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
[   13.863800] Control: 10c5387d  Table: 8dc20019  DAC: 0015
[   13.869842] Process sh (pid: 599, stack limit = 0xce3102f8)  
[   13.875732] Stack: (0xce311dd8 to 0xce312000)
[   13.880310] 1dc0:   6000c
[   13.888946] 1de0: 0001 0002     0
[   13.897583] 1e00:  c093d8f0  ce311ebc 0001 0001 ce310
[   13.906188] 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 0001 c0e95bf8 4
[   13.914825] 1e40:   c005ef74 ce31 c0435cf0 ce311ebc 0
[   13.923431] 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 0
[   13.932067] 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 8
[   13.940673] 1ea0: c08ba040 c033364c ce311ecc c0433b50 0002 ffea c0330
[   13.949310] 1ec0: 0007a120 0007a120 2201    ce357
[   13.957946] 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 0002 c032f47c 00034
[   13.966552] 1f00: c0331cac ce352b40 0007 c032f6d0 ce352bbc 0003d090 c0930
[   13.975189] 1f20: c093d8bc c03306a4 0007 ce311f80 0007 cdc2aec0 ce358
[   13.983795] 1f40: ce8d20c0 0007 b6fe5000 ce311f80 0007 ce31 c
[   13.992431] 1f60: c000de74 ce987400 ce8d20c0 b6fe5000   c
[   14.001037] 1f80:   001fbac8  0007 001fbac8 4
[   14.009674] 1fa0: c000df04 c000dd60 0007 001fbac8 0001 b6fe5000 0
[   14.018280] 1fc0: 0007 001fbac8 0007 0004 b6fe5000  00202
[   14.026916] 1fe0:  beb565f8 00101ffc 8e8c 6010 0001 0
[   14.035552] [] (twd_update_frequency+0x34/0x48) from [] )
[   14.046478] [] (smp_call_function_single+0x17c/0x1c8) from [] (twd_cpufreq_transition+0x24/0x30) from [)
[   14.068054] [] (notifier_call_chain+0x44/0x84) from [] ()
[   14.078887] [] (__srcu_notifier_call_chain+0x70/0xa4) from [] (srcu_notifier_call_chain+0x18/0x20) from [] (cpufreq_notify_transition+0xc8/0x1b0) from [] (omap_target+0x1b4/0x28c) from [] (__cpuf)
[   14.121765] [] (__cpufreq_driver_target+0x50/0x64) from [] (cpufreq_set+0x78/0x98) from [] (store_sc)
[   14.141296] [] (store_scaling_setspeed+0x5c/0x74) from [)
[   14.150482] [] (store+0x58/0x74) from [] (sysfs_write_fi)
[   14.159118] [] (sysfs_write_file+0x80/0xb4) from [] (vfs)
[   14.168212] [] (vfs_write+0xa8/0x138) from [] (sys_write)
[   14.17] [] (sys_write+0x40/0x6c) from [] (ret_fast_s)
[   14.185577] Code: e594300c e792210c e1a01000 e5840004 (e7930002) 
[   14.192169] ---[ end trace 5da3b5167c1ecdda ]--- 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-16 Thread Santosh Shilimkar
(+ linux-arm, Marc)

On Thursday 16 February 2012 11:54 PM, Kevin Hilman wrote:
> Hi Santosh,
> 
> Using v3.3-rc3 and building an OMAP2+ kernel I noticed that CPUfreq
> transitions fault because of the TWD cpufreq notifiers being called even
> on OMAP3.
> 
> Of course, the TWD doesn't exist on OMAP3, but the TWD init is still
> setting up notifiers.  I tried just adding a !cpu_is_omap44xx() check
> in local_timer_setup(), but that didn't do it, so there's more timer
> init that needs to be fixed up.  Can you have a look at this?
> 
> It's easy to reproduce by just manually triggering a frequency change
> with the userspace governor.  Here's what I did on my 3430/n900:
> 

Below patch fixes the issue.

Regards
Santosh

>From 3a16f7a6694c14e201fdf6ad195c821816b2de84 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar 
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local
timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case
the twd_timer_setup() isn't called. That seems to be wrong and
would eventually lead to kernel crash on the CPU frequency transitions
on the SOCs where the local timer doesn't exist or broken because of
hardware BUG.

Fix it by uisng an initialised variable. Though the twd_timer_setup()
is percpu, the idea here is to avoid the CPUFREQ registration. Hence
percpu initialised variable is not used.

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel
on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc2
 [007e9000] *pgd=
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : []lr : []psr: 6093
 sp : ce311dd8  ip :   fp : 
 r10:   r9 : 0001  r8 : ce31
 r7 : c0440458  r6 : c00137f8  r5 :   r4 : c0947a74
 r3 :   r2 : 007e9000  r1 :   r0 : 
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 0015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:   6000c
 1de0: 0001 0002     0
 1e00:  c093d8f0  ce311ebc 0001 0001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 0001 c0e95bf8 4
 1e40:   c005ef74 ce31 c0435cf0 ce311ebc 0
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 0
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 0002 ffea c0330
 1ec0: 0007a120 0007a120 2201    ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 0002 c032f47c 00034
 1f00: c0331cac ce352b40 0007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 0007 ce311f80 0007 cdc2aec0 ce358
 1f40: ce8d20c0 0007 b6fe5000 ce311f80 0007 ce31 c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000   c
 1f80:   001fbac8  0007 001fbac8 4
 1fa0: c000df04 c000dd60 0007 001fbac8 0001 b6fe5000 0
 1fc0: 0007 001fbac8 0007 0004 b6fe5000  00202
 1fe0:  beb565f8 00101ffc 8e8c 6010 0001 0
 [] (twd_update_frequency+0x34/0x48) from [] )
 [] (smp_call_function_single+0x17c/0x1c8) from [] (twd_cpufreq_transition+0x24/0x30) from [)
 [] (notifier_call_chain+0x44/0x84) from [] ()
 [] (__srcu_notifier_call_chain+0x70/0xa4) from [] (srcu_notifier_call_chain+0x18/0x20) from [] (cpufreq_notify_transition+0xc8/0x1b0) from [] (omap_target+0x1b4/0x28c) from [] (__cpuf)
 [] (__cpufreq_driver_target+0x50/0x64) from [] (cpufreq_set+0x78/0x98) from [] (store_sc)
 [] (store_scaling_setspeed+0x5c/0x74) from [)
 [] (store+0x58/0x74) from [] (sysfs_write_fi)
 [] (sysfs_write_file+0x80/0xb4) from [] (vfs)
 [] (vfs_write+0xa8/0x138) from [] (sys_write)
 [] (sys_write+0x40/0x6c) from [] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---

Reported-by: Kevin Hilman 
Signed-off-by: Santosh Shilimkar 
---
 arch/arm/kernel/smp_twd.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 4285daa..753ae37 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -30,6 +30,7 @@ void __iomem *twd_base;

 static struct clk *twd_clk;
 static unsigned long twd_timer_rate;
+static bool twd_initialised;

 static struct clock_event_device __percpu **twd_evt;

@@ -129,7 +130,7 @@ static struct notifier_block twd_cpufreq_nb = {

 static int twd_cpufreq_init(void)
 {
-   if (!IS_ERR(twd_clk))
+   if ((!IS_ERR(twd_

Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-17 Thread Marc Zyngier

Hi Santosh,

On Fri, 17 Feb 2012 13:18:37 +0530, Santosh Shilimkar
 wrote:
> 
> From 3a16f7a6694c14e201fdf6ad195c821816b2de84 Mon Sep 17 00:00:00 2001
> From: Santosh Shilimkar 
> Date: Fri, 17 Feb 2012 11:11:28 +0530
> Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local
> timers are not initialised.
> 
> Current ARM local timer code registers CPUFREQ notifiers even in case
> the twd_timer_setup() isn't called. That seems to be wrong and
> would eventually lead to kernel crash on the CPU frequency transitions
> on the SOCs where the local timer doesn't exist or broken because of
> hardware BUG.
> 
> Fix it by uisng an initialised variable. Though the twd_timer_setup()
> is percpu, the idea here is to avoid the CPUFREQ registration. Hence
> percpu initialised variable is not used.
> 
> The issue was observed with v3.3-rc3 and building an OMAP2+ kernel
> on OMAP3 SOC which doesn't have TWD.
> 
> Below is the dump for reference :
> 
>  Unable to handle kernel paging request at virtual address 007e900
>  pgd = cdc2
>  [007e9000] *pgd=
>  Internal error: Oops: 5 [#1] SMP
>  Modules linked in:
>  CPU: 0Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
>  PC is at twd_update_frequency+0x34/0x48
>  LR is at twd_update_frequency+0x10/0x48
>  pc : []lr : []psr: 6093
>  sp : ce311dd8  ip :   fp : 
>  r10:   r9 : 0001  r8 : ce31
>  r7 : c0440458  r6 : c00137f8  r5 :   r4 : c0947a74
>  r3 :   r2 : 007e9000  r1 :   r0 : 
>  Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
>  Control: 10c5387d  Table: 8dc20019  DAC: 0015
>  Process sh (pid: 599, stack limit = 0xce3102f8)
>  Stack: (0xce311dd8 to 0xce312000)
>  1dc0:   6000c
>  1de0: 0001 0002     0
>  1e00:  c093d8f0  ce311ebc 0001 0001 ce310
>  1e20: c001386c c0437c4c c0e95b60 c0e95ba8 0001 c0e95bf8 4
>  1e40:   c005ef74 ce31 c0435cf0 ce311ebc 0
>  1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 0
>  1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 8
>  1ea0: c08ba040 c033364c ce311ecc c0433b50 0002 ffea c0330
>  1ec0: 0007a120 0007a120 2201    ce357
>  1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 0002 c032f47c 00034
>  1f00: c0331cac ce352b40 0007 c032f6d0 ce352bbc 0003d090 c0930
>  1f20: c093d8bc c03306a4 0007 ce311f80 0007 cdc2aec0 ce358
>  1f40: ce8d20c0 0007 b6fe5000 ce311f80 0007 ce31 c
>  1f60: c000de74 ce987400 ce8d20c0 b6fe5000   c
>  1f80:   001fbac8  0007 001fbac8 4
>  1fa0: c000df04 c000dd60 0007 001fbac8 0001 b6fe5000 0
>  1fc0: 0007 001fbac8 0007 0004 b6fe5000  00202
>  1fe0:  beb565f8 00101ffc 8e8c 6010 0001 0
>  [] (twd_update_frequency+0x34/0x48) from [] )
>  [] (smp_call_function_single+0x17c/0x1c8) from [  [] (twd_cpufreq_transition+0x24/0x30) from [)
>  [] (notifier_call_chain+0x44/0x84) from [] ()
>  [] (__srcu_notifier_call_chain+0x70/0xa4) from [  [] (srcu_notifier_call_chain+0x18/0x20) from [  [] (cpufreq_notify_transition+0xc8/0x1b0) from [  [] (omap_target+0x1b4/0x28c) from [] (__cpuf)
>  [] (__cpufreq_driver_target+0x50/0x64) from [  [] (cpufreq_set+0x78/0x98) from [] (store_sc)
>  [] (store_scaling_setspeed+0x5c/0x74) from [)
>  [] (store+0x58/0x74) from [] (sysfs_write_fi)
>  [] (sysfs_write_file+0x80/0xb4) from [] (vfs)
>  [] (vfs_write+0xa8/0x138) from [] (sys_write)
>  [] (sys_write+0x40/0x6c) from [] (ret_fast_s)
>  Code: e594300c e792210c e1a01000 e5840004 (e7930002)
>  ---[ end trace 5da3b5167c1ecdda ]---
> 
> Reported-by: Kevin Hilman 
> Signed-off-by: Santosh Shilimkar 
> ---
>  arch/arm/kernel/smp_twd.c |4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> index 4285daa..753ae37 100644
> --- a/arch/arm/kernel/smp_twd.c
> +++ b/arch/arm/kernel/smp_twd.c
> @@ -30,6 +30,7 @@ void __iomem *twd_base;
> 
>  static struct clk *twd_clk;
>  static unsigned long twd_timer_rate;
> +static bool twd_initialised;
> 
>  static struct clock_event_device __percpu **twd_evt;
> 
> @@ -129,7 +130,7 @@ static struct notifier_block twd_cpufreq_nb = {
> 
>  static int twd_cpufreq_init(void)
>  {
> - if (!IS_ERR(twd_clk))
> + if ((!IS_ERR(twd_clk)) && twd_initialised)
>   return cpufreq_register_notifier(&twd_cpufreq_nb,
>   CPUFREQ_TRANSITION_NOTIFIER);

Testing twd_evt and *__this_cpu_ptr(twd_evt) should have a similar effect,
and save the additional variable.

M.
-- 
Fast, cheap, reliable. Pick two.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info a

Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-17 Thread Shilimkar, Santosh
On Fri, Feb 17, 2012 at 3:17 PM, Marc Zyngier  wrote:
>
> Hi Santosh,
>
> On Fri, 17 Feb 2012 13:18:37 +0530, Santosh Shilimkar
>  wrote:
>>

[...]

>>  arch/arm/kernel/smp_twd.c |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>> index 4285daa..753ae37 100644
>> --- a/arch/arm/kernel/smp_twd.c
>> +++ b/arch/arm/kernel/smp_twd.c
>> @@ -30,6 +30,7 @@ void __iomem *twd_base;
>>
>>  static struct clk *twd_clk;
>>  static unsigned long twd_timer_rate;
>> +static bool twd_initialised;
>>
>>  static struct clock_event_device __percpu **twd_evt;
>>
>> @@ -129,7 +130,7 @@ static struct notifier_block twd_cpufreq_nb = {
>>
>>  static int twd_cpufreq_init(void)
>>  {
>> -     if (!IS_ERR(twd_clk))
>> +     if ((!IS_ERR(twd_clk)) && twd_initialised)
>>               return cpufreq_register_notifier(&twd_cpufreq_nb,
>>                       CPUFREQ_TRANSITION_NOTIFIER);
>
> Testing twd_evt and *__this_cpu_ptr(twd_evt) should have a similar effect,
> and save the additional variable.
>
Indeed.
Updated patch below.

>From 4d351f59e5dabe065d51304455ee01b191cb56d6 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar 
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if
local timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case
the twd_timer_setup() isn't called. That seems to be wrong and
would eventually lead to kernel crash on the CPU frequency transitions
on the SOCs where the local timer doesn't exist or broken because of
hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel
on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc2
 [007e9000] *pgd=
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : []lr : []psr: 6093
 sp : ce311dd8  ip :   fp : 
 r10:   r9 : 0001  r8 : ce31
 r7 : c0440458  r6 : c00137f8  r5 :   r4 : c0947a74
 r3 :   r2 : 007e9000  r1 :   r0 : 
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 0015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:   6000c
 1de0: 0001 0002     0
 1e00:  c093d8f0  ce311ebc 0001 0001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 0001 c0e95bf8 4
 1e40:   c005ef74 ce31 c0435cf0 ce311ebc 0
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 0
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 0002 ffea c0330
 1ec0: 0007a120 0007a120 2201    ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 0002 c032f47c 00034
 1f00: c0331cac ce352b40 0007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 0007 ce311f80 0007 cdc2aec0 ce358
 1f40: ce8d20c0 0007 b6fe5000 ce311f80 0007 ce31 c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000   c
 1f80:   001fbac8  0007 001fbac8 4
 1fa0: c000df04 c000dd60 0007 001fbac8 0001 b6fe5000 0
 1fc0: 0007 001fbac8 0007 0004 b6fe5000  00202
 1fe0:  beb565f8 00101ffc 8e8c 6010 0001 0
 [] (twd_update_frequency+0x34/0x48) from [] )
 [] (smp_call_function_single+0x17c/0x1c8) from [] (twd_cpufreq_transition+0x24/0x30) from [)
 [] (notifier_call_chain+0x44/0x84) from [] ()
 [] (__srcu_notifier_call_chain+0x70/0xa4) from [] (srcu_notifier_call_chain+0x18/0x20) from [] (cpufreq_notify_transition+0xc8/0x1b0) from [] (omap_target+0x1b4/0x28c) from [] (__cpuf)
 [] (__cpufreq_driver_target+0x50/0x64) from [] (cpufreq_set+0x78/0x98) from [] (store_sc)
 [] (store_scaling_setspeed+0x5c/0x74) from [)
 [] (store+0x58/0x74) from [] (sysfs_write_fi)
 [] (sysfs_write_file+0x80/0xb4) from [] (vfs)
 [] (vfs_write+0xa8/0x138) from [] (sys_write)
 [] (sys_write+0x40/0x6c) from [] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---

Reported-by: Kevin Hilman 
Signed-off-by: Santosh Shilimkar 
---
 arch/arm/kernel/smp_twd.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 4285daa..265a18a 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {

 static int twd_cpufreq_init(void)
 {

Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-17 Thread Russell King - ARM Linux
On Fri, Feb 17, 2012 at 04:10:57PM +0530, Shilimkar, Santosh wrote:
> - if (!IS_ERR(twd_clk))
> + if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))

Too many parens.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-17 Thread Marc Zyngier
On 17/02/12 10:40, Shilimkar, Santosh wrote:
> On Fri, Feb 17, 2012 at 3:17 PM, Marc Zyngier  wrote:
>>
>> Hi Santosh,
>>
>> On Fri, 17 Feb 2012 13:18:37 +0530, Santosh Shilimkar
>>  wrote:
>>>
> 
> [...]
> 
>>>  arch/arm/kernel/smp_twd.c |4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>>> index 4285daa..753ae37 100644
>>> --- a/arch/arm/kernel/smp_twd.c
>>> +++ b/arch/arm/kernel/smp_twd.c
>>> @@ -30,6 +30,7 @@ void __iomem *twd_base;
>>>
>>>  static struct clk *twd_clk;
>>>  static unsigned long twd_timer_rate;
>>> +static bool twd_initialised;
>>>
>>>  static struct clock_event_device __percpu **twd_evt;
>>>
>>> @@ -129,7 +130,7 @@ static struct notifier_block twd_cpufreq_nb = {
>>>
>>>  static int twd_cpufreq_init(void)
>>>  {
>>> - if (!IS_ERR(twd_clk))
>>> + if ((!IS_ERR(twd_clk)) && twd_initialised)
>>>   return cpufreq_register_notifier(&twd_cpufreq_nb,
>>>   CPUFREQ_TRANSITION_NOTIFIER);
>>
>> Testing twd_evt and *__this_cpu_ptr(twd_evt) should have a similar effect,
>> and save the additional variable.
>>
> Indeed.
> Updated patch below.
> 
> From 4d351f59e5dabe065d51304455ee01b191cb56d6 Mon Sep 17 00:00:00 2001
> From: Santosh Shilimkar 
> Date: Fri, 17 Feb 2012 11:11:28 +0530
> Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if
> local timers are not initialised.
> 
> Current ARM local timer code registers CPUFREQ notifiers even in case
> the twd_timer_setup() isn't called. That seems to be wrong and
> would eventually lead to kernel crash on the CPU frequency transitions
> on the SOCs where the local timer doesn't exist or broken because of
> hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).
> 
> The issue was observed with v3.3-rc3 and building an OMAP2+ kernel
> on OMAP3 SOC which doesn't have TWD.
> 
> Below is the dump for reference :
> 
>  Unable to handle kernel paging request at virtual address 007e900
>  pgd = cdc2
>  [007e9000] *pgd=
>  Internal error: Oops: 5 [#1] SMP
>  Modules linked in:
>  CPU: 0Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
>  PC is at twd_update_frequency+0x34/0x48
>  LR is at twd_update_frequency+0x10/0x48
>  pc : []lr : []psr: 6093
>  sp : ce311dd8  ip :   fp : 
>  r10:   r9 : 0001  r8 : ce31
>  r7 : c0440458  r6 : c00137f8  r5 :   r4 : c0947a74
>  r3 :   r2 : 007e9000  r1 :   r0 : 
>  Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
>  Control: 10c5387d  Table: 8dc20019  DAC: 0015
>  Process sh (pid: 599, stack limit = 0xce3102f8)
>  Stack: (0xce311dd8 to 0xce312000)
>  1dc0:   6000c
>  1de0: 0001 0002     0
>  1e00:  c093d8f0  ce311ebc 0001 0001 ce310
>  1e20: c001386c c0437c4c c0e95b60 c0e95ba8 0001 c0e95bf8 4
>  1e40:   c005ef74 ce31 c0435cf0 ce311ebc 0
>  1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 0
>  1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 8
>  1ea0: c08ba040 c033364c ce311ecc c0433b50 0002 ffea c0330
>  1ec0: 0007a120 0007a120 2201    ce357
>  1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 0002 c032f47c 00034
>  1f00: c0331cac ce352b40 0007 c032f6d0 ce352bbc 0003d090 c0930
>  1f20: c093d8bc c03306a4 0007 ce311f80 0007 cdc2aec0 ce358
>  1f40: ce8d20c0 0007 b6fe5000 ce311f80 0007 ce31 c
>  1f60: c000de74 ce987400 ce8d20c0 b6fe5000   c
>  1f80:   001fbac8  0007 001fbac8 4
>  1fa0: c000df04 c000dd60 0007 001fbac8 0001 b6fe5000 0
>  1fc0: 0007 001fbac8 0007 0004 b6fe5000  00202
>  1fe0:  beb565f8 00101ffc 8e8c 6010 0001 0
>  [] (twd_update_frequency+0x34/0x48) from [] )
>  [] (smp_call_function_single+0x17c/0x1c8) from [  [] (twd_cpufreq_transition+0x24/0x30) from [)
>  [] (notifier_call_chain+0x44/0x84) from [] ()
>  [] (__srcu_notifier_call_chain+0x70/0xa4) from [  [] (srcu_notifier_call_chain+0x18/0x20) from [  [] (cpufreq_notify_transition+0xc8/0x1b0) from [  [] (omap_target+0x1b4/0x28c) from [] (__cpuf)
>  [] (__cpufreq_driver_target+0x50/0x64) from [  [] (cpufreq_set+0x78/0x98) from [] (store_sc)
>  [] (store_scaling_setspeed+0x5c/0x74) from [)
>  [] (store+0x58/0x74) from [] (sysfs_write_fi)
>  [] (sysfs_write_file+0x80/0xb4) from [] (vfs)
>  [] (vfs_write+0xa8/0x138) from [] (sys_write)
>  [] (sys_write+0x40/0x6c) from [] (ret_fast_s)
>  Code: e594300c e792210c e1a01000 e5840004 (e7930002)
>  ---[ end trace 5da3b5167c1ecdda ]---
> 
> Reported-by: Kevin Hilman 
> Signed-off-by: Santosh Shilimkar 
> ---
>  arch/arm/kernel/smp_twd.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch

Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-17 Thread Santosh Shilimkar
On Friday 17 February 2012 04:36 PM, Marc Zyngier wrote:
> On 17/02/12 10:40, Shilimkar, Santosh wrote:
>> On Fri, Feb 17, 2012 at 3:17 PM, Marc Zyngier  wrote:
>>>
>>> Hi Santosh,
>>>
>>> On Fri, 17 Feb 2012 13:18:37 +0530, Santosh Shilimkar
>>>  wrote:

>>

[..]

>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>> index 4285daa..265a18a 100644
>> --- a/arch/arm/kernel/smp_twd.c
>> +++ b/arch/arm/kernel/smp_twd.c
>> @@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
>>
>>  static int twd_cpufreq_init(void)
>>  {
>> -if (!IS_ERR(twd_clk))
>> +if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))
> 
> Erm... Shouldn't that rather be:
>   if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
> 
My bad. You are right. Updated patch below.
Tested on OMAP3430 SDP with CPUFREQ enabled.

Regards
Santosh

>From f58c515a9d1a2c729c05a726d5176843381952ae Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar 
Date: Fri, 17 Feb 2012 11:11:28 +0530
Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local
timers are not initialised.

Current ARM local timer code registers CPUFREQ notifiers even in case
the twd_timer_setup() isn't called. That seems to be wrong and
would eventually lead to kernel crash on the CPU frequency transitions
on the SOCs where the local timer doesn't exist or broken because of
hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).

The issue was observed with v3.3-rc3 and building an OMAP2+ kernel
on OMAP3 SOC which doesn't have TWD.

Below is the dump for reference :

 Unable to handle kernel paging request at virtual address 007e900
 pgd = cdc2
 [007e9000] *pgd=
 Internal error: Oops: 5 [#1] SMP
 Modules linked in:
 CPU: 0Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
 PC is at twd_update_frequency+0x34/0x48
 LR is at twd_update_frequency+0x10/0x48
 pc : []lr : []psr: 6093
 sp : ce311dd8  ip :   fp : 
 r10:   r9 : 0001  r8 : ce31
 r7 : c0440458  r6 : c00137f8  r5 :   r4 : c0947a74
 r3 :   r2 : 007e9000  r1 :   r0 : 
 Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
 Control: 10c5387d  Table: 8dc20019  DAC: 0015
 Process sh (pid: 599, stack limit = 0xce3102f8)
 Stack: (0xce311dd8 to 0xce312000)
 1dc0:   6000c
 1de0: 0001 0002     0
 1e00:  c093d8f0  ce311ebc 0001 0001 ce310
 1e20: c001386c c0437c4c c0e95b60 c0e95ba8 0001 c0e95bf8 4
 1e40:   c005ef74 ce31 c0435cf0 ce311ebc 0
 1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 0
 1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 8
 1ea0: c08ba040 c033364c ce311ecc c0433b50 0002 ffea c0330
 1ec0: 0007a120 0007a120 2201    ce357
 1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 0002 c032f47c 00034
 1f00: c0331cac ce352b40 0007 c032f6d0 ce352bbc 0003d090 c0930
 1f20: c093d8bc c03306a4 0007 ce311f80 0007 cdc2aec0 ce358
 1f40: ce8d20c0 0007 b6fe5000 ce311f80 0007 ce31 c
 1f60: c000de74 ce987400 ce8d20c0 b6fe5000   c
 1f80:   001fbac8  0007 001fbac8 4
 1fa0: c000df04 c000dd60 0007 001fbac8 0001 b6fe5000 0
 1fc0: 0007 001fbac8 0007 0004 b6fe5000  00202
 1fe0:  beb565f8 00101ffc 8e8c 6010 0001 0
 [] (twd_update_frequency+0x34/0x48) from [] )
 [] (smp_call_function_single+0x17c/0x1c8) from [] (twd_cpufreq_transition+0x24/0x30) from [)
 [] (notifier_call_chain+0x44/0x84) from [] ()
 [] (__srcu_notifier_call_chain+0x70/0xa4) from [] (srcu_notifier_call_chain+0x18/0x20) from [] (cpufreq_notify_transition+0xc8/0x1b0) from [] (omap_target+0x1b4/0x28c) from [] (__cpuf)
 [] (__cpufreq_driver_target+0x50/0x64) from [] (cpufreq_set+0x78/0x98) from [] (store_sc)
 [] (store_scaling_setspeed+0x5c/0x74) from [)
 [] (store+0x58/0x74) from [] (sysfs_write_fi)
 [] (sysfs_write_file+0x80/0xb4) from [] (vfs)
 [] (vfs_write+0xa8/0x138) from [] (sys_write)
 [] (sys_write+0x40/0x6c) from [] (ret_fast_s)
 Code: e594300c e792210c e1a01000 e5840004 (e7930002)
 ---[ end trace 5da3b5167c1ecdda ]---

Reported-by: Kevin Hilman 
Signed-off-by: Santosh Shilimkar 
---
 arch/arm/kernel/smp_twd.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 4285daa..9834851 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {

 static int twd_cpufreq_init(void)
 {
-   if (!IS_ERR(twd_clk))
+if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
return cpufreq_register_notifier(&twd_cpufreq_nb,
CPUFREQ_TRANSITION_NOTIFIER);

-

Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-17 Thread Marc Zyngier
On 17/02/12 12:28, Santosh Shilimkar wrote:
> On Friday 17 February 2012 04:36 PM, Marc Zyngier wrote:
>> On 17/02/12 10:40, Shilimkar, Santosh wrote:
>>> On Fri, Feb 17, 2012 at 3:17 PM, Marc Zyngier  wrote:

 Hi Santosh,

 On Fri, 17 Feb 2012 13:18:37 +0530, Santosh Shilimkar
  wrote:
>
>>>
> 
> [..]
> 
>>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>>> index 4285daa..265a18a 100644
>>> --- a/arch/arm/kernel/smp_twd.c
>>> +++ b/arch/arm/kernel/smp_twd.c
>>> @@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
>>>
>>>  static int twd_cpufreq_init(void)
>>>  {
>>> -   if (!IS_ERR(twd_clk))
>>> +   if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))
>>
>> Erm... Shouldn't that rather be:
>>  if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
>>
> My bad. You are right. Updated patch below.
> Tested on OMAP3430 SDP with CPUFREQ enabled.
> 
> Regards
> Santosh
> 
> From f58c515a9d1a2c729c05a726d5176843381952ae Mon Sep 17 00:00:00 2001
> From: Santosh Shilimkar 
> Date: Fri, 17 Feb 2012 11:11:28 +0530
> Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local
> timers are not initialised.
> 
> Current ARM local timer code registers CPUFREQ notifiers even in case
> the twd_timer_setup() isn't called. That seems to be wrong and
> would eventually lead to kernel crash on the CPU frequency transitions
> on the SOCs where the local timer doesn't exist or broken because of
> hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).
> 
> The issue was observed with v3.3-rc3 and building an OMAP2+ kernel
> on OMAP3 SOC which doesn't have TWD.
> 
> Below is the dump for reference :
> 
>  Unable to handle kernel paging request at virtual address 007e900
>  pgd = cdc2
>  [007e9000] *pgd=
>  Internal error: Oops: 5 [#1] SMP
>  Modules linked in:
>  CPU: 0Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
>  PC is at twd_update_frequency+0x34/0x48
>  LR is at twd_update_frequency+0x10/0x48
>  pc : []lr : []psr: 6093
>  sp : ce311dd8  ip :   fp : 
>  r10:   r9 : 0001  r8 : ce31
>  r7 : c0440458  r6 : c00137f8  r5 :   r4 : c0947a74
>  r3 :   r2 : 007e9000  r1 :   r0 : 
>  Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
>  Control: 10c5387d  Table: 8dc20019  DAC: 0015
>  Process sh (pid: 599, stack limit = 0xce3102f8)
>  Stack: (0xce311dd8 to 0xce312000)
>  1dc0:   6000c
>  1de0: 0001 0002     0
>  1e00:  c093d8f0  ce311ebc 0001 0001 ce310
>  1e20: c001386c c0437c4c c0e95b60 c0e95ba8 0001 c0e95bf8 4
>  1e40:   c005ef74 ce31 c0435cf0 ce311ebc 0
>  1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 0
>  1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 8
>  1ea0: c08ba040 c033364c ce311ecc c0433b50 0002 ffea c0330
>  1ec0: 0007a120 0007a120 2201    ce357
>  1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 0002 c032f47c 00034
>  1f00: c0331cac ce352b40 0007 c032f6d0 ce352bbc 0003d090 c0930
>  1f20: c093d8bc c03306a4 0007 ce311f80 0007 cdc2aec0 ce358
>  1f40: ce8d20c0 0007 b6fe5000 ce311f80 0007 ce31 c
>  1f60: c000de74 ce987400 ce8d20c0 b6fe5000   c
>  1f80:   001fbac8  0007 001fbac8 4
>  1fa0: c000df04 c000dd60 0007 001fbac8 0001 b6fe5000 0
>  1fc0: 0007 001fbac8 0007 0004 b6fe5000  00202
>  1fe0:  beb565f8 00101ffc 8e8c 6010 0001 0
>  [] (twd_update_frequency+0x34/0x48) from [] )
>  [] (smp_call_function_single+0x17c/0x1c8) from [  [] (twd_cpufreq_transition+0x24/0x30) from [)
>  [] (notifier_call_chain+0x44/0x84) from [] ()
>  [] (__srcu_notifier_call_chain+0x70/0xa4) from [  [] (srcu_notifier_call_chain+0x18/0x20) from [  [] (cpufreq_notify_transition+0xc8/0x1b0) from [  [] (omap_target+0x1b4/0x28c) from [] (__cpuf)
>  [] (__cpufreq_driver_target+0x50/0x64) from [  [] (cpufreq_set+0x78/0x98) from [] (store_sc)
>  [] (store_scaling_setspeed+0x5c/0x74) from [)
>  [] (store+0x58/0x74) from [] (sysfs_write_fi)
>  [] (sysfs_write_file+0x80/0xb4) from [] (vfs)
>  [] (vfs_write+0xa8/0x138) from [] (sys_write)
>  [] (sys_write+0x40/0x6c) from [] (ret_fast_s)
>  Code: e594300c e792210c e1a01000 e5840004 (e7930002)
>  ---[ end trace 5da3b5167c1ecdda ]---
> 
> Reported-by: Kevin Hilman 
> Signed-off-by: Santosh Shilimkar 

Acked-by: Marc Zyngier 

> ---
>  arch/arm/kernel/smp_twd.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> index 4285daa..9834851 100644
> --- a/arch/arm/kernel/smp_twd.c
> +++ b/arch/arm/kernel/smp_twd.c
> @@ -129,7 +129,7 @@ static struct notifier_block twd_cp

Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-17 Thread Kevin Hilman
Santosh Shilimkar  writes:

> On Friday 17 February 2012 04:36 PM, Marc Zyngier wrote:
>> On 17/02/12 10:40, Shilimkar, Santosh wrote:
>>> On Fri, Feb 17, 2012 at 3:17 PM, Marc Zyngier  wrote:

 Hi Santosh,

 On Fri, 17 Feb 2012 13:18:37 +0530, Santosh Shilimkar
  wrote:
>
>>>
>
> [..]
>
>>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>>> index 4285daa..265a18a 100644
>>> --- a/arch/arm/kernel/smp_twd.c
>>> +++ b/arch/arm/kernel/smp_twd.c
>>> @@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
>>>
>>>  static int twd_cpufreq_init(void)
>>>  {
>>> -   if (!IS_ERR(twd_clk))
>>> +   if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))
>> 
>> Erm... Shouldn't that rather be:
>>  if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
>> 
> My bad. You are right. Updated patch below.
> Tested on OMAP3430 SDP with CPUFREQ enabled.
>
> Regards
> Santosh
>
> From f58c515a9d1a2c729c05a726d5176843381952ae Mon Sep 17 00:00:00 2001
> From: Santosh Shilimkar 
> Date: Fri, 17 Feb 2012 11:11:28 +0530
> Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local
> timers are not initialised.
>
> Current ARM local timer code registers CPUFREQ notifiers even in case
> the twd_timer_setup() isn't called. That seems to be wrong and
> would eventually lead to kernel crash on the CPU frequency transitions
> on the SOCs where the local timer doesn't exist or broken because of
> hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).
>
> The issue was observed with v3.3-rc3 and building an OMAP2+ kernel
> on OMAP3 SOC which doesn't have TWD.
>
> Below is the dump for reference :
>
>  Unable to handle kernel paging request at virtual address 007e900
>  pgd = cdc2
>  [007e9000] *pgd=
>  Internal error: Oops: 5 [#1] SMP
>  Modules linked in:
>  CPU: 0Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
>  PC is at twd_update_frequency+0x34/0x48
>  LR is at twd_update_frequency+0x10/0x48
>  pc : []lr : []psr: 6093
>  sp : ce311dd8  ip :   fp : 
>  r10:   r9 : 0001  r8 : ce31
>  r7 : c0440458  r6 : c00137f8  r5 :   r4 : c0947a74
>  r3 :   r2 : 007e9000  r1 :   r0 : 
>  Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
>  Control: 10c5387d  Table: 8dc20019  DAC: 0015
>  Process sh (pid: 599, stack limit = 0xce3102f8)
>  Stack: (0xce311dd8 to 0xce312000)
>  1dc0:   6000c
>  1de0: 0001 0002     0
>  1e00:  c093d8f0  ce311ebc 0001 0001 ce310
>  1e20: c001386c c0437c4c c0e95b60 c0e95ba8 0001 c0e95bf8 4
>  1e40:   c005ef74 ce31 c0435cf0 ce311ebc 0
>  1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 0
>  1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 8
>  1ea0: c08ba040 c033364c ce311ecc c0433b50 0002 ffea c0330
>  1ec0: 0007a120 0007a120 2201    ce357
>  1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 0002 c032f47c 00034
>  1f00: c0331cac ce352b40 0007 c032f6d0 ce352bbc 0003d090 c0930
>  1f20: c093d8bc c03306a4 0007 ce311f80 0007 cdc2aec0 ce358
>  1f40: ce8d20c0 0007 b6fe5000 ce311f80 0007 ce31 c
>  1f60: c000de74 ce987400 ce8d20c0 b6fe5000   c
>  1f80:   001fbac8  0007 001fbac8 4
>  1fa0: c000df04 c000dd60 0007 001fbac8 0001 b6fe5000 0
>  1fc0: 0007 001fbac8 0007 0004 b6fe5000  00202
>  1fe0:  beb565f8 00101ffc 8e8c 6010 0001 0
>  [] (twd_update_frequency+0x34/0x48) from [] )
>  [] (smp_call_function_single+0x17c/0x1c8) from [  [] (twd_cpufreq_transition+0x24/0x30) from [)
>  [] (notifier_call_chain+0x44/0x84) from [] ()
>  [] (__srcu_notifier_call_chain+0x70/0xa4) from [  [] (srcu_notifier_call_chain+0x18/0x20) from [  [] (cpufreq_notify_transition+0xc8/0x1b0) from [  [] (omap_target+0x1b4/0x28c) from [] (__cpuf)
>  [] (__cpufreq_driver_target+0x50/0x64) from [  [] (cpufreq_set+0x78/0x98) from [] (store_sc)
>  [] (store_scaling_setspeed+0x5c/0x74) from [)
>  [] (store+0x58/0x74) from [] (sysfs_write_fi)
>  [] (sysfs_write_file+0x80/0xb4) from [] (vfs)
>  [] (vfs_write+0xa8/0x138) from [] (sys_write)
>  [] (sys_write+0x40/0x6c) from [] (ret_fast_s)
>  Code: e594300c e792210c e1a01000 e5840004 (e7930002)
>  ---[ end trace 5da3b5167c1ecdda ]---
>
> Reported-by: Kevin Hilman 
> Signed-off-by: Santosh Shilimkar 

Tested-by: Kevin Hilman 

CAn you please add to Russell's patch system.  This needs to be fixed
for v3.3-rc.

Thanks,

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


Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-17 Thread Shilimkar, Santosh
On Sat, Feb 18, 2012 at 12:19 AM, Kevin Hilman  wrote:
> Santosh Shilimkar  writes:
>
>> On Friday 17 February 2012 04:36 PM, Marc Zyngier wrote:
>>> On 17/02/12 10:40, Shilimkar, Santosh wrote:
 On Fri, Feb 17, 2012 at 3:17 PM, Marc Zyngier  wrote:
>
> Hi Santosh,
>
> On Fri, 17 Feb 2012 13:18:37 +0530, Santosh Shilimkar
>  wrote:
>>

>>
>> [..]
>>
 diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
 index 4285daa..265a18a 100644
 --- a/arch/arm/kernel/smp_twd.c
 +++ b/arch/arm/kernel/smp_twd.c
 @@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {

  static int twd_cpufreq_init(void)
  {
 -   if (!IS_ERR(twd_clk))
 +   if ((!twd_evt) && (!__this_cpu_ptr(twd_evt)) && (!IS_ERR(twd_clk)))
>>>
>>> Erm... Shouldn't that rather be:
>>>      if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
>>>
>> My bad. You are right. Updated patch below.
>> Tested on OMAP3430 SDP with CPUFREQ enabled.
>>
>> Regards
>> Santosh
>>
>> From f58c515a9d1a2c729c05a726d5176843381952ae Mon Sep 17 00:00:00 2001
>> From: Santosh Shilimkar 
>> Date: Fri, 17 Feb 2012 11:11:28 +0530
>> Subject: [PATCH] ARM: smp_twd: Don't register CPUFREQ notifiers if local
>> timers are not initialised.
>>
>> Current ARM local timer code registers CPUFREQ notifiers even in case
>> the twd_timer_setup() isn't called. That seems to be wrong and
>> would eventually lead to kernel crash on the CPU frequency transitions
>> on the SOCs where the local timer doesn't exist or broken because of
>> hardware BUG. Fix it by testing twd_evt and *__this_cpu_ptr(twd_evt).
>>
>> The issue was observed with v3.3-rc3 and building an OMAP2+ kernel
>> on OMAP3 SOC which doesn't have TWD.
>>
>> Below is the dump for reference :
>>
>>  Unable to handle kernel paging request at virtual address 007e900
>>  pgd = cdc2
>>  [007e9000] *pgd=
>>  Internal error: Oops: 5 [#1] SMP
>>  Modules linked in:
>>  CPU: 0    Not tainted  (3.3.0-rc3-pm+debug+initramfs #9)
>>  PC is at twd_update_frequency+0x34/0x48
>>  LR is at twd_update_frequency+0x10/0x48
>>  pc : []    lr : []    psr: 6093
>>  sp : ce311dd8  ip :   fp : 
>>  r10:   r9 : 0001  r8 : ce31
>>  r7 : c0440458  r6 : c00137f8  r5 :   r4 : c0947a74
>>  r3 :   r2 : 007e9000  r1 :   r0 : 
>>  Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment usr
>>  Control: 10c5387d  Table: 8dc20019  DAC: 0015
>>  Process sh (pid: 599, stack limit = 0xce3102f8)
>>  Stack: (0xce311dd8 to 0xce312000)
>>  1dc0:                                                       6000c
>>  1de0: 0001 0002     0
>>  1e00:  c093d8f0  ce311ebc 0001 0001 ce310
>>  1e20: c001386c c0437c4c c0e95b60 c0e95ba8 0001 c0e95bf8 4
>>  1e40:   c005ef74 ce31 c0435cf0 ce311ebc 0
>>  1e60: ce352b40 0007a120 c08d5108 c08ba040 c08ba040 c005f030 0
>>  1e80: c08bc554 c032fe2c 0007a120 c08d4b64 ce352b40 c08d8618 8
>>  1ea0: c08ba040 c033364c ce311ecc c0433b50 0002 ffea c0330
>>  1ec0: 0007a120 0007a120 2201    ce357
>>  1ee0: ce3d6000 cdc2aed8 ce352ba0 c0470164 0002 c032f47c 00034
>>  1f00: c0331cac ce352b40 0007 c032f6d0 ce352bbc 0003d090 c0930
>>  1f20: c093d8bc c03306a4 0007 ce311f80 0007 cdc2aec0 ce358
>>  1f40: ce8d20c0 0007 b6fe5000 ce311f80 0007 ce31 c
>>  1f60: c000de74 ce987400 ce8d20c0 b6fe5000   c
>>  1f80:   001fbac8  0007 001fbac8 4
>>  1fa0: c000df04 c000dd60 0007 001fbac8 0001 b6fe5000 0
>>  1fc0: 0007 001fbac8 0007 0004 b6fe5000  00202
>>  1fe0:  beb565f8 00101ffc 8e8c 6010 0001 0
>>  [] (twd_update_frequency+0x34/0x48) from [] )
>>  [] (smp_call_function_single+0x17c/0x1c8) from [>  [] (twd_cpufreq_transition+0x24/0x30) from [)
>>  [] (notifier_call_chain+0x44/0x84) from [] ()
>>  [] (__srcu_notifier_call_chain+0x70/0xa4) from [>  [] (srcu_notifier_call_chain+0x18/0x20) from [>  [] (cpufreq_notify_transition+0xc8/0x1b0) from [>  [] (omap_target+0x1b4/0x28c) from [] (__cpuf)
>>  [] (__cpufreq_driver_target+0x50/0x64) from [>  [] (cpufreq_set+0x78/0x98) from [] (store_sc)
>>  [] (store_scaling_setspeed+0x5c/0x74) from [)
>>  [] (store+0x58/0x74) from [] (sysfs_write_fi)
>>  [] (sysfs_write_file+0x80/0xb4) from [] (vfs)
>>  [] (vfs_write+0xa8/0x138) from [] (sys_write)
>>  [] (sys_write+0x40/0x6c) from [] (ret_fast_s)
>>  Code: e594300c e792210c e1a01000 e5840004 (e7930002)
>>  ---[ end trace 5da3b5167c1ecdda ]---
>>
>> Reported-by: Kevin Hilman 
>> Signed-off-by: Santosh Shilimkar 
>
> Tested-by: Kevin Hilman 
>
> CAn you please add to Russell's patch system.  This needs to be fixed
> for v3.3-rc.
>
Done .
7328/1 in patch system.

Regards
Santosh
--
To unsubscribe from this list: send the lin

Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-21 Thread Russell King - ARM Linux
On Fri, Feb 17, 2012 at 05:58:34PM +0530, Santosh Shilimkar wrote:
> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
> index 4285daa..9834851 100644
> --- a/arch/arm/kernel/smp_twd.c
> +++ b/arch/arm/kernel/smp_twd.c
> @@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
> 
>  static int twd_cpufreq_init(void)
>  {
> - if (!IS_ERR(twd_clk))
> +  if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))

You seem to have an additional space before 'if'.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: SMP local timers and their CPUfreq notifiers setup on OMAP3

2012-02-21 Thread Shilimkar, Santosh
On Tue, Feb 21, 2012 at 2:41 PM, Russell King - ARM Linux
 wrote:
> On Fri, Feb 17, 2012 at 05:58:34PM +0530, Santosh Shilimkar wrote:
>> diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
>> index 4285daa..9834851 100644
>> --- a/arch/arm/kernel/smp_twd.c
>> +++ b/arch/arm/kernel/smp_twd.c
>> @@ -129,7 +129,7 @@ static struct notifier_block twd_cpufreq_nb = {
>>
>>  static int twd_cpufreq_init(void)
>>  {
>> -     if (!IS_ERR(twd_clk))
>> +      if (twd_evt && *__this_cpu_ptr(twd_evt) && !IS_ERR(twd_clk))
>
> You seem to have an additional space before 'if'.
Yep.
Fixed in the updated version in patch system.
patch 7336/1

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html