[PATCH] arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

[PATCH] arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

[Felipe Balbi]

Make sure to tell the kernel that AM437x has
TWD and global timers.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---

Hi Tony,

now that all dependencies are in place, we can
finally enable twd and global_timer for AM437x.

cheers

 arch/arm/mach-omap2/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 5076d3f334d2..bb3daf0fa7f7 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -65,6 +65,9 @@ config SOC_AM43XX
 	select MACH_OMAP_GENERIC
 	select MIGHT_HAVE_CACHE_L2X0
 	select HAVE_ARM_SCU
+	select HAVE_ARM_TWD
+	select ARM_GLOBAL_TIMER
+	select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
 
 config SOC_DRA7XX
 	bool "TI DRA7XX"
-- 
2.6.2

Re: [PATCH] arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

[Grygorii Strashko]

On 11/12/2015 08:06 PM, Felipe Balbi wrote:
> Make sure to tell the kernel that AM437x has
> TWD and global timers.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
> 
> Hi Tony,
> 
> now that all dependencies are in place, we can
> finally enable twd and global_timer for AM437x.
> 

I'd appreciated if someone can clarify if all described below is valid.
(may be some questions are dummy - sorry).

After all last changes related to TI OMAP clock source/clock event/sched clock's
devices configuration we will have the following for am437x case:

clockevents:
 "timer1",	clockevent_gpt, .rating = 300, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, ti,timer-alwon,
 "arm,twd-timer",	twd_evt,.rating = 350, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP
 "arm_global_timer",	 gt_evt, rating = 300, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU

clocksources:
 "jiffies", clocksource_jiffies,  rating = 1
 if use_gptimer_clksrc
	"timer2", clocksource_gpt, .rating = 300, CLOCK_SOURCE_IS_CONTINUOUS,
	|-sched_clock_register(dmtimer_read_sched_clock, 32, clksrc.rate);
 else
	"ti,omap-counter32k", ti_32k_timer, .rating = 250, CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
	|-sched_clock_register(omap_32k_read_sched_clock, 32, 32768);

 "arm,global-timer", gt_clocksource, .rating = 300, CLOCK_SOURCE_IS_CONTINUOUS
 |-sched_clock_register(gt_sched_clock_read, 64, gt_clk_rate);

1) current clockevent device will be registered during registration of clockevent devices
and it will be  "arm,twd-timer" - processing sequence follows the way they are listed above.

2) current clocksource will be selected from fs_initcall(clocksource_done_booting);
and it will be "arm,global-timer" or "timer2" if use_gptimer_clksrc

3) sched clock: "arm,global-timer" will be selected as sched_clock *always*,
because it depend on Makefile order (if someone will decide to sort entries
in drivers/clocksource/Makefile then "ti,omap-counter32k" most probably will
be always selected as sched clock).

Uh.. 

As for me, "timer1" is selected as clockevent device incorrectly, because it's 
 ti,timer-alwon.

I think, "ti,omap-counter32k" will crash if use_gptimer_clksrc == true, because
corresponding HWmod will not be enabled (not tested this case). 

Not sure if "timer2" is selected correctly when both  "arm,global-timer" and GP timer
registered as clocksources. ?

Also, It looks like the way sched clock is selected is a little bit unsafe/unclear
(especially taking into account that clocksource dev can be changed through the sysfs
and TI multiSoC kernel).

What is the policy/right way to configure all this clocks?
- should only one clock source be selected for each of them in config file?
- how to dial with sched clock if it depends only on Makefile order ? :(
- should we convert all of them to modules and load depending on current hw?

Could it be reasonable to configure sched clock together with clocksource dev
registration/selection?
- add read_sched field 
struct clocksource {
	u64 (*read_sched_clock)(void);

- configure sched clock from clocksource core if read_sched != null

Thanks.

>   arch/arm/mach-omap2/Kconfig | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index 5076d3f334d2..bb3daf0fa7f7 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -65,6 +65,9 @@ config SOC_AM43XX
>   	select MACH_OMAP_GENERIC
>   	select MIGHT_HAVE_CACHE_L2X0
>   	select HAVE_ARM_SCU
> +	select HAVE_ARM_TWD
> +	select ARM_GLOBAL_TIMER
> +	select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
>   
>   config SOC_DRA7XX
>   	bool "TI DRA7XX"
> 


-- 
regards,
-grygorii

Re: [PATCH] arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

[Mason]

On 13/11/2015 13:48, Grygorii Strashko wrote:
> On 11/12/2015 08:06 PM, Felipe Balbi wrote:
>> Make sure to tell the kernel that AM437x has
>> TWD and global timers.
>>
>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>> ---
>>
>> Hi Tony,
>>
>> now that all dependencies are in place, we can
>> finally enable twd and global_timer for AM437x.
>>
> 
> I'd appreciated if someone can clarify if all described below is valid.
> (may be some questions are dummy - sorry).
> 
> After all last changes related to TI OMAP clock source/clock event/sched clock's
> devices configuration we will have the following for am437x case:
> 
> clockevents:
>  "timer1",	clockevent_gpt, .rating = 300, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, ti,timer-alwon,
>  "arm,twd-timer",	twd_evt,.rating = 350, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP
>  "arm_global_timer",	 gt_evt, rating = 300, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU

NB: AFAIU/IIUC the global timer ticks at cpuclk/N but the driver
does not deal with frequency updates (unlike smp_twd.c) I'm not
sure global timer can be used reliably together with cpufreq.

> clocksources:
>  "jiffies", clocksource_jiffies,  rating = 1
>  if use_gptimer_clksrc
> 	"timer2", clocksource_gpt, .rating = 300, CLOCK_SOURCE_IS_CONTINUOUS,
> 	|-sched_clock_register(dmtimer_read_sched_clock, 32, clksrc.rate);
>  else
> 	"ti,omap-counter32k", ti_32k_timer, .rating = 250, CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
> 	|-sched_clock_register(omap_32k_read_sched_clock, 32, 32768);
> 
>  "arm,global-timer", gt_clocksource, .rating = 300, CLOCK_SOURCE_IS_CONTINUOUS

If using cpufreq, I don't think the global timer can be used
as a clock source.

Again AFAIU and IIUC and I may be wrong.

Regards.

Re: [PATCH] arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

[santosh shilimkar]

On 11/13/2015 5:07 AM, Mason wrote:
> On 13/11/2015 13:48, Grygorii Strashko wrote:
>> On 11/12/2015 08:06 PM, Felipe Balbi wrote:
>>> Make sure to tell the kernel that AM437x has
>>> TWD and global timers.
>>>
>>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>>> ---
>>>
>>> Hi Tony,
>>>
>>> now that all dependencies are in place, we can
>>> finally enable twd and global_timer for AM437x.
>>>
Is AM437x SMP SOC ? Is TWD on AM437x works in low
power states ?
Probably I haven't followed the recent updates, but does
the TWD supports C3STOP on UP systems ? The broadcast
code was SMP only in past, and TWD use to die in
low power state on past OMAP SOCs.

If either of these are still the issue then
TWD shouldn't be used.

Regards,
Santosh

Re: [PATCH] arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

[Grygorii Strashko]

On 11/13/2015 06:39 PM, santosh shilimkar wrote:
> On 11/13/2015 5:07 AM, Mason wrote:
>> On 13/11/2015 13:48, Grygorii Strashko wrote:
>>> On 11/12/2015 08:06 PM, Felipe Balbi wrote:
>>>> Make sure to tell the kernel that AM437x has
>>>> TWD and global timers.
>>>>
>>>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>>>> ---
>>>>
>>>> Hi Tony,
>>>>
>>>> now that all dependencies are in place, we can
>>>> finally enable twd and global_timer for AM437x.
>>>>
> Is AM437x SMP SOC ? 

No. But it has ARM TWD and GT

> Is TWD on AM437x works in low power states ?

No. But TWD, seems, is not a problem here if omap gp timer
can be used as broadcast device.

> Probably I haven't followed the recent updates, but does
> the TWD supports C3STOP on UP systems ? The broadcast
> code was SMP only in past, and TWD use to die in
> low power state on past OMAP SOCs.
> 
> If either of these are still the issue then
> TWD shouldn't be used.
> 

Yep. I see the problem with ARM Global timer here if
CPUIdle is enabled and ARM Global timer is selected as
clocksource device.

I think, it will be right thing to disable "global_timer"
and "local_timer" nodes in am437x.dtsi by default - if
someone would like to use them then those nodes have
to be enabled explicitly in board file.
(TWD timer will be enabled on OMAP multi SoC build
 irrespectively of HAVE_ARM_TWD is selected for am437x or not,
 because it will be selected for omap4). 
I've just sent corresponding patch: 
 http://www.spinics.net/lists/arm-kernel/msg461141.html

Also, probably, it could be reasonable to:
- make ARM_GLOBAL_TIMER fully selectable option and allow select it from defconfig.

- or - update this patch as below
-       select ARM_GLOBAL_TIMER
-       select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
+       select ARM_GLOBAL_TIMER if !CPU_IDLE
+       select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK if ARM_GLOBAL_TIMER



Thanks.
-- 
regards,
-grygorii

Re: [PATCH] arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

[santosh shilimkar]

On 11/18/2015 6:33 AM, Grygorii Strashko wrote:
> On 11/13/2015 06:39 PM, santosh shilimkar wrote:
>> On 11/13/2015 5:07 AM, Mason wrote:
>>> On 13/11/2015 13:48, Grygorii Strashko wrote:
>>>> On 11/12/2015 08:06 PM, Felipe Balbi wrote:
>>>>> Make sure to tell the kernel that AM437x has
>>>>> TWD and global timers.
>>>>>
>>>>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>>>>> ---
>>>>>
>>>>> Hi Tony,
>>>>>
>>>>> now that all dependencies are in place, we can
>>>>> finally enable twd and global_timer for AM437x.
>>>>>
>> Is AM437x SMP SOC ?
>
> No. But it has ARM TWD and GT
>
>> Is TWD on AM437x works in low power states ?
>
> No. But TWD, seems, is not a problem here if omap gp timer
> can be used as broadcast device.
>
>> Probably I haven't followed the recent updates, but does
>> the TWD supports C3STOP on UP systems ? The broadcast
>> code was SMP only in past, and TWD use to die in
>> low power state on past OMAP SOCs.
>>
>> If either of these are still the issue then
>> TWD shouldn't be used.
>>
>
> Yep. I see the problem with ARM Global timer here if
> CPUIdle is enabled and ARM Global timer is selected as
> clocksource device.
>
Its expected and well known limitation.

> I think, it will be right thing to disable "global_timer"
> and "local_timer" nodes in am437x.dtsi by default - if
> someone would like to use them then those nodes have
> to be enabled explicitly in board file.
> (TWD timer will be enabled on OMAP multi SoC build
>   irrespectively of HAVE_ARM_TWD is selected for am437x or not,
>   because it will be selected for omap4).
> I've just sent corresponding patch:
>   http://www.spinics.net/lists/arm-kernel/msg461141.html
>
> Also, probably, it could be reasonable to:
> - make ARM_GLOBAL_TIMER fully selectable option and allow select it from defconfig.
>
> - or - update this patch as below
> -       select ARM_GLOBAL_TIMER
> -       select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
> +       select ARM_GLOBAL_TIMER if !CPU_IDLE
> +       select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK if ARM_GLOBAL_TIMER
>
>
+1

Re: [PATCH] arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

[Grygorii Strashko]

On 11/13/2015 02:48 PM, Grygorii Strashko wrote:
> On 11/12/2015 08:06 PM, Felipe Balbi wrote:
>> Make sure to tell the kernel that AM437x has
>> TWD and global timers.
>>
>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>> ---
>>
>> Hi Tony,
>>
>> now that all dependencies are in place, we can
>> finally enable twd and global_timer for AM437x.
>>
> 
> I'd appreciated if someone can clarify if all described below is valid.
> (may be some questions are dummy - sorry).
> 
> After all last changes related to TI OMAP clock source/clock event/sched clock's
> devices configuration we will have the following for am437x case:
> 
> clockevents:
>   "timer1",	clockevent_gpt, .rating = 300, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT, ti,timer-alwon,
>   "arm,twd-timer",	twd_evt,.rating = 350, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP
>   "arm_global_timer",	 gt_evt, rating = 300, CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERCPU
> 
> clocksources:
>   "jiffies", clocksource_jiffies,  rating = 1
>   if use_gptimer_clksrc
> 	"timer2", clocksource_gpt, .rating = 300, CLOCK_SOURCE_IS_CONTINUOUS,
> 	|-sched_clock_register(dmtimer_read_sched_clock, 32, clksrc.rate);
>   else
> 	"ti,omap-counter32k", ti_32k_timer, .rating = 250, CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
> 	|-sched_clock_register(omap_32k_read_sched_clock, 32, 32768);
> 
>   "arm,global-timer", gt_clocksource, .rating = 300, CLOCK_SOURCE_IS_CONTINUOUS
>   |-sched_clock_register(gt_sched_clock_read, 64, gt_clk_rate);
> 
> 1) current clockevent device will be registered during registration of clockevent devices
> and it will be  "arm,twd-timer" - processing sequence follows the way they are listed above.
> 
> 2) current clocksource will be selected from fs_initcall(clocksource_done_booting);
> and it will be "arm,global-timer" or "timer2" if use_gptimer_clksrc
> 
> 3) sched clock: "arm,global-timer" will be selected as sched_clock *always*,
> because it depend on Makefile order (if someone will decide to sort entries
> in drivers/clocksource/Makefile then "ti,omap-counter32k" most probably will
> be always selected as sched clock).

Ok. Sry. Above is not completely true. There are no dependency on makefile
and sched clock with Max freq will be selected - and never changed even if
corresponding clocksource can be changed through sysfs.

> 
> Uh..
> 
> As for me, "timer1" is selected as clockevent device incorrectly, because it's
>   ti,timer-alwon.
> 
> I think, "ti,omap-counter32k" will crash if use_gptimer_clksrc == true, because
> corresponding HWmod will not be enabled (not tested this case).

Ok. Below is confirmation for the case if use_gptimer_clksrc == true, so
https://www.spinics.net/lists/linux-omap/msg122576.html
"[PATCH 00/11] arm: omap: counter32k rework" introduces regression !?

cat /sys/devices/system/clocksource/clocksource0/available_clocksource 
timer2 arm_global_timer 32k_counter 
/ # echo 32k_counter > /sys/devices/system/clocksource/clocksource0/current_cloc
ksource 
[  102.194313] Unhandled fault: imprecise external abort (0x1406) at 0x0020ab5c
[  102.197862] pgd = ee22c000





-- 
regards,
-grygorii