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

PATCH New Felipe Balbi 4 participants 7 messages Nov 12, 2015 – Nov 18, 2015

Subsystems (3)

Arm (32-bit) Arm sub-architectures OMAP2+ support

lore

Share & tools

lore

Felipe Balbi

author

2015-11-12 18:06 UTC #0 lore raw

PATCH 1/1 arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

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

Signed-off-by Felipe Balbi <[email protected]>

---

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(+)

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

index 5076d3f334d2..bb3daf0fa7f7 100644
@@ -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"

Signature · 1 line

2.6.2

Grygorii Strashko

2015-11-13 12:48 UTC #1 lore raw

PATCH 1/1 arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

Quoted 11 lines · Felipe Balbi

Make sure to tell the kernel that AM437x has
TWD and global timers.
Signed-off-by Felipe Balbi <[email protected]>
---
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.

Quoted 18 lines

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

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

index 5076d3f334d2..bb3daf0fa7f7 100644
@@ -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"

Signature · 2 lines

regards,
-grygorii

Mason

2015-11-13 13:07 UTC #2 lore raw

PATCH 1/1 arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

Quoted 16 lines · Grygorii Strashko

Quoted 11 lines · Felipe Balbi
Make sure to tell the kernel that AM437x has
TWD and global timers.
Signed-off-by Felipe Balbi <[email protected]>
---
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).

Quoted 7 lines

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.

Quoted 10 lines

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.

santosh shilimkar

2015-11-13 16:39 UTC #3 lore raw

PATCH 1/1 arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

Quoted 5 lines · Mason

Quoted 4 lines · Grygorii Strashko
Quoted 3 lines · Felipe Balbi
Make sure to tell the kernel that AM437x has
TWD and global timers.

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
Signed-off-by Felipe Balbi <[email protected]>
---
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

Grygorii Strashko

2015-11-18 14:33 UTC #4 lore raw

PATCH 1/1 arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

Quoted 9 lines · santosh shilimkar

Quoted 8 lines · Mason
Quoted 7 lines · Grygorii Strashko
Quoted 6 lines · Felipe Balbi
Make sure to tell the kernel that AM437x has
TWD and global timers.
Signed-off-by Felipe Balbi <[email protected]>
---

Quoted 6 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
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

Quoted 1 line

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.

Quoted 8 lines

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.

Signature · 2 lines

regards,
-grygorii

santosh shilimkar

2015-11-18 18:01 UTC #5 lore raw

PATCH 1/1 arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

Quoted 29 lines · Grygorii Strashko

Quoted 9 lines · santosh shilimkar
Quoted 8 lines · Mason
Quoted 7 lines · Grygorii Strashko
Quoted 6 lines · Felipe Balbi
Make sure to tell the kernel that AM437x has
TWD and global timers.
Signed-off-by Felipe Balbi <[email protected]>
---
Quoted 6 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
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
Quoted 1 line
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.
Quoted 5 lines
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.

Quoted 8 lines

Quoted 3 lines
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.

Quoted 16 lines

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

Review

Quoted 4 lines

(quoted diff) +2 -2

-       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

Grygorii Strashko

2015-11-13 17:15 UTC #6 lore raw

PATCH 1/1 arm: omap2: Kconfig: select TWD and global timer on AM43xx devices

In reply to Grygorii Strashko

Quoted 39 lines · Grygorii Strashko

Quoted 11 lines · Felipe Balbi
Make sure to tell the kernel that AM437x has
TWD and global timers.
Signed-off-by Felipe Balbi <[email protected]>
---
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.

Quoted 7 lines

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.

Quoted 8 lines

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

Signature · 2 lines

regards,
-grygorii