[PATCH v2] clocksource: arm_global_timer: fix suspend resume

Grygorii Strashko grygorii.strashko at ti.com
Fri Nov 20 12:42:14 PST 2015

On 11/20/2015 09:32 PM, John Stultz wrote:
> On Fri, Nov 20, 2015 at 11:28 AM, Grygorii Strashko
> <grygorii.strashko at ti.com> wrote:
>> On 11/20/2015 09:09 PM, John Stultz wrote:
>>> On Fri, Nov 20, 2015 at 10:35 AM, Grygorii Strashko
>>> <grygorii.strashko at ti.com> 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
>>>>>>      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 at arm.com>
>>>> 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.


More information about the linux-arm-kernel mailing list