On 11/20/2015 09:32 PM, John Stultz wrote: > On Fri, Nov 20, 2015 at 11:28 AM, Grygorii Strashko > <grygorii.strashko@xxxxxx> wrote: >> On 11/20/2015 09:09 PM, John Stultz wrote: >>> On Fri, Nov 20, 2015 at 10:35 AM, Grygorii Strashko >>> <grygorii.strashko@xxxxxx> wrote: >>>> Hi Santosh, >>>> >>>> On 11/20/2015 07:23 PM, santosh shilimkar wrote: >>>>> + Thomas, Marc >>>>> >>>>> On 11/20/2015 5:57 AM, Grygorii Strashko wrote: >>>>>> Now the System stall is observed on TI AM437x based board >>>>>> (am437x-gp-evm) during resuming from System suspend when ARM Global >>>>>> timer is selected as clocksource device - SysRq are working, but >>>>>> nothing else. The reason of stall is that ARM Global timer loses its >>>>>> contexts. >>>>>> >>>>>> The reason of stall is that ARM Global timer loses its contexts during >>>>>> System suspend: >>>>>> GT_CONTROL.TIMER_ENABLE = 0 (unbanked) >>>>>> GT_COUNTERx = 0 >>>>>> >>>>>> Hence, update ARM Global timer driver to reflect above behaviour >>>>>> - re-enable ARM Global timer on resume GT_CONTROL.TIMER_ENABLE = 1 >>>>>> - ensure clocksource and clockevent devices have coresponding flags >>>>>> (CLOCK_SOURCE_SUSPEND_NONSTOP and CLOCK_EVT_FEAT_C3STOP) set >>>>>> depending on presence of "always-on" DT property. >>>>>> >>>>> Something which loses context in low power states can't be >>>>> called "always-on" >>>> >>>> Sry, it's kinda new area for me and I could make mistakes. >>>> >>>> While working on this patch I've: >>>> - re-used implementation from ARM arch timer >>>> commit 82a5619410d4c4df65c04272db198eca5a867c18 >>>> Author: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> >>>> Date: Tue Apr 8 10:04:32 2014 +0100 >>>> >>>> clocksource: arch_arm_timer: Fix age-old arch timer C3STOP detection issue >>>> >>>> >>>> - and followed timekeeping.txt: >>>> "Typically the clock source is a monotonic, atomic counter which will provide >>>> n bits which count from 0 to 2^(n-1) and then wraps around to 0 and start over. >>>> It will ideally NEVER stop ticking as long as the system is running. It >>>> may stop during system suspend." >>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >>>> >>>> And that exactly my use-case: I'd like to use ARM GT as clocksource >>>> and with CPUIdle = n. But System suspend has to be allowed. >>>> >>>> >>>>> >>>>> Also if the clock-soucre can't tick in the low power states >>>>> then that device shouldn't be used as a clock-source. >>>> >>>> Agree. clocksource can't (except with suspend). Have I missed something? >>> >>> I think the point Stantosh is making is that if the clocksource stops >>> in suspend (which is allowed) you should not be setting >>> CLOCK_SOURCE_SUSPEND_NONSTOP (which promises the clocksource doesn't >>> stop in suspend, so it can be used for suspend timing as well). >>> >> >> Ok. Thanks. I got it now. Adding CLOCK_SOURCE_SUSPEND_NONSTOP is mistake. >> >>> The contradictory part in your patch is that you're also setting the >>> clockevent logic as CLOCK_EVT_FEAT_C3STOP, which flags that the >>> clockevent hardware might stop in low-power idle states (ie: not >>> suspend, but while the system is running). Usually hardware that >>> halts in low-power mode (like the apic on some x86 machines) is not >>> also used for timekeeping (instead we use the hpet/acpi there). >> >> Sry, I've set CLOCK_EVT_FEAT_C3STOP if "always-on" = *false* > > You might also consider renaming that value from always_on to > something more descriptive, given the subtlety of the different states > here. Maybe instead use a flag called halts_in_idle or something? > I'd better just drop it for now hence I'm still not sure what is more reasonable continue with DT property or just add this flag always. -- regards, -grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html