| Message ID | 1447351566-2328-1-git-send-email-balbi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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" >
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.
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
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
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.
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
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"
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(+)