| Message ID | 1460457775-23850-1-git-send-email-grygorii.strashko@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/12/2016 05:42 AM, Grygorii Strashko wrote: > The commit 55ee7017ee31 ("arm: omap2: board-generic: use > omap4_local_timer_init for AM437x") unintentionally changes the > clocksource devices for AM437x from OMAP GP Timer to SyncTimer32K. > > Unfortunately, the SyncTimer32K is starving from frequency deviation > as mentioned in commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer > as clocksource") and, as reported by Franklin [1], even its monotonic > nature is under question (most probably there is a HW issue, but it's > still under investigation). > > Taking into account above facts It's reasonable to rollback to the use > of omap3_gptimer_timer_init(). I tested this patch and was not able to reproduce this issue after rebooting the AM437 GP evm 30 times. I then tried without this patch and the issue showed up after rebooting the board for the 5th time. Tested-by: Franklin S Cooper Jr. <fcooper@ti.com> > > [1] http://www.spinics.net/lists/linux-omap/msg127425.html > Cc: Felipe Balbi <balbi@kernel.org> > Cc: Lokesh Vutla <lokeshvutla@ti.com> > Reported-by: Cooper Jr., Franklin <fcooper@ti.com> > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > --- > arch/arm/mach-omap2/board-generic.c | 2 +- > arch/arm/mach-omap2/timer.c | 7 +++++-- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c > index bab814d..60f5e83 100644 > --- a/arch/arm/mach-omap2/board-generic.c > +++ b/arch/arm/mach-omap2/board-generic.c > @@ -306,7 +306,7 @@ DT_MACHINE_START(AM43_DT, "Generic AM43 (Flattened Device Tree)") > .init_late = am43xx_init_late, > .init_irq = omap_gic_of_init, > .init_machine = omap_generic_init, > - .init_time = omap4_local_timer_init, > + .init_time = omap3_gptimer_timer_init, > .dt_compat = am43_boards_compat, > .restart = omap44xx_restart, > MACHINE_END > diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c > index 6261e93..7cc055c 100644 > --- a/arch/arm/mach-omap2/timer.c > +++ b/arch/arm/mach-omap2/timer.c > @@ -500,16 +500,19 @@ void __init omap3_secure_sync32k_timer_init(void) > } > #endif /* CONFIG_ARCH_OMAP3 */ > > -#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX) > +#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX) || \ > + defined(CONFIG_SOC_AM43XX) > void __init omap3_gptimer_timer_init(void) > { > __omap_sync32k_timer_init(2, "timer_sys_ck", NULL, > 1, "timer_sys_ck", "ti,timer-alwon", true); > + if (of_have_populated_dt()) > + clocksource_probe(); > } > #endif > > #if defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5) || \ > - defined(CONFIG_SOC_DRA7XX) || defined(CONFIG_SOC_AM43XX) > + defined(CONFIG_SOC_DRA7XX) > static void __init omap4_sync32k_timer_init(void) > { > __omap_sync32k_timer_init(1, "timer_32k_ck", "ti,timer-alwon", >
* Grygorii Strashko <grygorii.strashko@ti.com> [160412 03:44]: > The commit 55ee7017ee31 ("arm: omap2: board-generic: use > omap4_local_timer_init for AM437x") unintentionally changes the > clocksource devices for AM437x from OMAP GP Timer to SyncTimer32K. > > Unfortunately, the SyncTimer32K is starving from frequency deviation > as mentioned in commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer > as clocksource") and, as reported by Franklin [1], even its monotonic > nature is under question (most probably there is a HW issue, but it's > still under investigation). > > Taking into account above facts It's reasonable to rollback to the use > of omap3_gptimer_timer_init(). I thought only the ePOS EVM does not have the 32k clock available? Maybe this is the the old sync timer autocorrection drift issue? Regards, Tony
* Franklin S Cooper Jr. <fcooper@ti.com> [160412 09:02]: > > > On 04/12/2016 05:42 AM, Grygorii Strashko wrote: > > The commit 55ee7017ee31 ("arm: omap2: board-generic: use > > omap4_local_timer_init for AM437x") unintentionally changes the > > clocksource devices for AM437x from OMAP GP Timer to SyncTimer32K. > > > > Unfortunately, the SyncTimer32K is starving from frequency deviation > > as mentioned in commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer > > as clocksource") and, as reported by Franklin [1], even its monotonic > > nature is under question (most probably there is a HW issue, but it's > > still under investigation). > > > > Taking into account above facts It's reasonable to rollback to the use > > of omap3_gptimer_timer_init(). > > I tested this patch and was not able to reproduce this issue after > rebooting the AM437 GP evm 30 times. I then tried without this patch and > the issue showed up after rebooting the board for the 5th time. What's the problem that happens? Would be good to have it in the description so people know. Regards, Tony
On 04/12/2016 07:04 PM, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.strashko@ti.com> [160412 03:44]: >> The commit 55ee7017ee31 ("arm: omap2: board-generic: use >> omap4_local_timer_init for AM437x") unintentionally changes the >> clocksource devices for AM437x from OMAP GP Timer to SyncTimer32K. >> >> Unfortunately, the SyncTimer32K is starving from frequency deviation >> as mentioned in commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer >> as clocksource") and, as reported by Franklin [1], even its monotonic >> nature is under question (most probably there is a HW issue, but it's >> still under investigation). >> >> Taking into account above facts It's reasonable to rollback to the use >> of omap3_gptimer_timer_init(). > > I thought only the ePOS EVM does not have the 32k clock available? > Maybe this is the the old sync timer autocorrection drift issue? > May be, as i mentioned in [1] it could be errata same as for Watchdog Advisory 22 (or OMAP_TIMER_ERRATA_I103_I767). But as per commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer as clocksource") there is no reason to use SyncTimer32K as clocksource any way (not only on epos): commit 5b5c01359152f3ddaa1aa0e5d1141bc2b29ba2c5 Author: Rajendra Nayak <rnayak@ti.com> Date: Fri Feb 7 15:51:26 2014 +0530 ARM: OMAP2+: AM43x: Use gptimer as clocksource The SyncTimer in AM43x is clocked using the following two sources: 1) An inaccuarte 32k clock (CLK_32KHZ) derived from PER DPLL, causing system time to go slowly (~10% deviation). ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 2) external 32KHz RTC clock, which may not always be available on board like in the case of ePOS EVM Use gptimer as clocksource instead, as is done in the case of AM335x (which does not have a SyncTimer). With this, system time keeping works accurately. Signed-off-by: Rajendra Nayak <rnayak@ti.com> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> Signed-off-by: Tony Lindgren <tony@atomide.com> [1] http://www.spinics.net/lists/linux-omap/msg127425.html
* Grygorii Strashko <grygorii.strashko@ti.com> [160412 11:31]: > On 04/12/2016 07:04 PM, Tony Lindgren wrote: > > * Grygorii Strashko <grygorii.strashko@ti.com> [160412 03:44]: > >> The commit 55ee7017ee31 ("arm: omap2: board-generic: use > >> omap4_local_timer_init for AM437x") unintentionally changes the > >> clocksource devices for AM437x from OMAP GP Timer to SyncTimer32K. > >> > >> Unfortunately, the SyncTimer32K is starving from frequency deviation > >> as mentioned in commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer > >> as clocksource") and, as reported by Franklin [1], even its monotonic > >> nature is under question (most probably there is a HW issue, but it's > >> still under investigation). > >> > >> Taking into account above facts It's reasonable to rollback to the use > >> of omap3_gptimer_timer_init(). > > > > I thought only the ePOS EVM does not have the 32k clock available? > > Maybe this is the the old sync timer autocorrection drift issue? > > > > May be, as i mentioned in [1] it could be errata same as for Watchdog > Advisory 22 (or OMAP_TIMER_ERRATA_I103_I767). > > But as per commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer > as clocksource") there is no reason to use SyncTimer32K as > clocksource any way (not only on epos): > > commit 5b5c01359152f3ddaa1aa0e5d1141bc2b29ba2c5 > Author: Rajendra Nayak <rnayak@ti.com> > Date: Fri Feb 7 15:51:26 2014 +0530 > > ARM: OMAP2+: AM43x: Use gptimer as clocksource > > The SyncTimer in AM43x is clocked using the following two sources: > 1) An inaccuarte 32k clock (CLK_32KHZ) derived from PER DPLL, causing system > time to go slowly (~10% deviation). > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 2) external 32KHz RTC clock, which may not always be available on board like > in the case of ePOS EVM This sounds similar to recent "ARM: dts: dra7: Correct clock tree for sys_32k_ck", can you see if similar changes help for am43x? Regards, Tony
On Wednesday 13 April 2016 12:12 AM, Tony Lindgren wrote: > * Grygorii Strashko <grygorii.strashko@ti.com> [160412 11:31]: >> On 04/12/2016 07:04 PM, Tony Lindgren wrote: >>> * Grygorii Strashko <grygorii.strashko@ti.com> [160412 03:44]: >>>> The commit 55ee7017ee31 ("arm: omap2: board-generic: use >>>> omap4_local_timer_init for AM437x") unintentionally changes the >>>> clocksource devices for AM437x from OMAP GP Timer to SyncTimer32K. >>>> >>>> Unfortunately, the SyncTimer32K is starving from frequency deviation >>>> as mentioned in commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer >>>> as clocksource") and, as reported by Franklin [1], even its monotonic >>>> nature is under question (most probably there is a HW issue, but it's >>>> still under investigation). >>>> >>>> Taking into account above facts It's reasonable to rollback to the use >>>> of omap3_gptimer_timer_init(). >>> >>> I thought only the ePOS EVM does not have the 32k clock available? >>> Maybe this is the the old sync timer autocorrection drift issue? >>> >> >> May be, as i mentioned in [1] it could be errata same as for Watchdog >> Advisory 22 (or OMAP_TIMER_ERRATA_I103_I767). >> >> But as per commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer >> as clocksource") there is no reason to use SyncTimer32K as >> clocksource any way (not only on epos): >> >> commit 5b5c01359152f3ddaa1aa0e5d1141bc2b29ba2c5 >> Author: Rajendra Nayak <rnayak@ti.com> >> Date: Fri Feb 7 15:51:26 2014 +0530 >> >> ARM: OMAP2+: AM43x: Use gptimer as clocksource >> >> The SyncTimer in AM43x is clocked using the following two sources: >> 1) An inaccuarte 32k clock (CLK_32KHZ) derived from PER DPLL, causing system >> time to go slowly (~10% deviation). >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ I don't think this statement is right. If clock from PER DPLL is inaccurate then all the IPs which uses PER DPLL are in danger. It is the On-Chip 32K RC Osc clock that is not an accurate clock-source as per the design/spec. >> 2) external 32KHz RTC clock, which may not always be available on board like >> in the case of ePOS EVM Also this 32KHz RTC clock is not enabled by default and is handled by RTC. IMO, it is better to change the clock source of sync timer to CLK_32KHz(like it is done in case of am43x-epos). Or do you have any other reason to shift to gptimer? Thanks and regards, Lokesh
On 05/25/2016 12:53 PM, Lokesh Vutla wrote: > > > On Wednesday 13 April 2016 12:12 AM, Tony Lindgren wrote: >> * Grygorii Strashko <grygorii.strashko@ti.com> [160412 11:31]: >>> On 04/12/2016 07:04 PM, Tony Lindgren wrote: >>>> * Grygorii Strashko <grygorii.strashko@ti.com> [160412 03:44]: >>>>> The commit 55ee7017ee31 ("arm: omap2: board-generic: use >>>>> omap4_local_timer_init for AM437x") unintentionally changes the >>>>> clocksource devices for AM437x from OMAP GP Timer to SyncTimer32K. >>>>> >>>>> Unfortunately, the SyncTimer32K is starving from frequency deviation >>>>> as mentioned in commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer >>>>> as clocksource") and, as reported by Franklin [1], even its monotonic >>>>> nature is under question (most probably there is a HW issue, but it's >>>>> still under investigation). >>>>> >>>>> Taking into account above facts It's reasonable to rollback to the use >>>>> of omap3_gptimer_timer_init(). >>>> >>>> I thought only the ePOS EVM does not have the 32k clock available? >>>> Maybe this is the the old sync timer autocorrection drift issue? >>>> >>> >>> May be, as i mentioned in [1] it could be errata same as for Watchdog >>> Advisory 22 (or OMAP_TIMER_ERRATA_I103_I767). >>> >>> But as per commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer >>> as clocksource") there is no reason to use SyncTimer32K as >>> clocksource any way (not only on epos): >>> >>> commit 5b5c01359152f3ddaa1aa0e5d1141bc2b29ba2c5 >>> Author: Rajendra Nayak <rnayak@ti.com> >>> Date: Fri Feb 7 15:51:26 2014 +0530 >>> >>> ARM: OMAP2+: AM43x: Use gptimer as clocksource >>> >>> The SyncTimer in AM43x is clocked using the following two sources: >>> 1) An inaccuarte 32k clock (CLK_32KHZ) derived from PER DPLL, causing system >>> time to go slowly (~10% deviation). >>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > I don't think this statement is right. If clock from PER DPLL is > inaccurate then all the IPs which uses PER DPLL are in danger. It is the > On-Chip 32K RC Osc clock that is not an accurate clock-source as per the > design/spec. > >>> 2) external 32KHz RTC clock, which may not always be available on board like >>> in the case of ePOS EVM > > Also this 32KHz RTC clock is not enabled by default and is handled by RTC. > > IMO, it is better to change the clock source of sync timer to > CLK_32KHz(like it is done in case of am43x-epos). Or do you have any > other reason to shift to gptimer? > In my opinion this is because of HW issue - OMAP4 errata i103, which, for some reasons, is not mentioned for am437x (and I do not have confirmation). Originally issue was described and fix implemented for OMAP4 by commit commit 73152a98a25392a451bb0d588a1fbe2796b3c909 Author: Santosh Shilimkar <santosh.shilimkar@ti.com> Date: Mon Mar 12 20:34:45 2012 +0530 OMAP4: Workaround the OCP synchronisation issue with 32K synctimer. I saw exactly similar behavior by my self: - for some reason ti_32k_read_cycles() return value lower the previously read value - this causes timekeeping_get_delta() to return negative value - which, in turn, causes big jiffies shift: 4294939981 -> 13079884 - and this blow up Timekeeping core brain and causes RCU stall error messages, which is reasonable because all jiffies based timeout will misbehave. From registers I can see that 32K synctimer is in no-idle mode 32K synctimer.SYSC = 0x00000008 PRCM_CM_WKUP_SYNCTIMER_CLKCTRL = 0x00000102 and, seems, it's impossible to use static dep on am437x. So, right now, I don't know how this issues can be W/A, so rallback.
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c index bab814d..60f5e83 100644 --- a/arch/arm/mach-omap2/board-generic.c +++ b/arch/arm/mach-omap2/board-generic.c @@ -306,7 +306,7 @@ DT_MACHINE_START(AM43_DT, "Generic AM43 (Flattened Device Tree)") .init_late = am43xx_init_late, .init_irq = omap_gic_of_init, .init_machine = omap_generic_init, - .init_time = omap4_local_timer_init, + .init_time = omap3_gptimer_timer_init, .dt_compat = am43_boards_compat, .restart = omap44xx_restart, MACHINE_END diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 6261e93..7cc055c 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -500,16 +500,19 @@ void __init omap3_secure_sync32k_timer_init(void) } #endif /* CONFIG_ARCH_OMAP3 */ -#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX) +#if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_SOC_AM33XX) || \ + defined(CONFIG_SOC_AM43XX) void __init omap3_gptimer_timer_init(void) { __omap_sync32k_timer_init(2, "timer_sys_ck", NULL, 1, "timer_sys_ck", "ti,timer-alwon", true); + if (of_have_populated_dt()) + clocksource_probe(); } #endif #if defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5) || \ - defined(CONFIG_SOC_DRA7XX) || defined(CONFIG_SOC_AM43XX) + defined(CONFIG_SOC_DRA7XX) static void __init omap4_sync32k_timer_init(void) { __omap_sync32k_timer_init(1, "timer_32k_ck", "ti,timer-alwon",
The commit 55ee7017ee31 ("arm: omap2: board-generic: use omap4_local_timer_init for AM437x") unintentionally changes the clocksource devices for AM437x from OMAP GP Timer to SyncTimer32K. Unfortunately, the SyncTimer32K is starving from frequency deviation as mentioned in commit 5b5c01359152 ("ARM: OMAP2+: AM43x: Use gptimer as clocksource") and, as reported by Franklin [1], even its monotonic nature is under question (most probably there is a HW issue, but it's still under investigation). Taking into account above facts It's reasonable to rollback to the use of omap3_gptimer_timer_init(). [1] http://www.spinics.net/lists/linux-omap/msg127425.html Cc: Felipe Balbi <balbi@kernel.org> Cc: Lokesh Vutla <lokeshvutla@ti.com> Reported-by: Cooper Jr., Franklin <fcooper@ti.com> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> --- arch/arm/mach-omap2/board-generic.c | 2 +- arch/arm/mach-omap2/timer.c | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-)