All of lore.kernel.org
 help / color / mirror / Atom feed
From: mattias.wallin@stericsson.com (Mattias Wallin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 0/3] clocksource: add db8500 PRCMU timer
Date: Thu, 2 Jun 2011 14:10:02 +0200	[thread overview]
Message-ID: <4DE77D9A.6090301@stericsson.com> (raw)
In-Reply-To: <20110602110137.GU3660@n2100.arm.linux.org.uk>

On 06/02/2011 01:01 PM, Russell King - ARM Linux wrote:
> On Thu, Jun 02, 2011 at 12:18:35PM +0200, Mattias Wallin wrote:
>> On 06/02/2011 11:46 AM, Russell King - ARM Linux wrote:
>>> Why don't we just find a way of fixing sched_clock so that the value
>>> doesn't reset over a suspend/resume cycle?
>> Even if the value isn't reset during suspend/resume we want the
>> clocksource to keep counting. Or is it ok to have the clocksource stop
>> or freeze during suspend?
>
> kernel/time/timekeeping.c:timekeeping_suspend():
>
>          timekeeping_forward_now();
>
> which does:
>          cycle_now = clock->read(clock);
>          cycle_delta = (cycle_now - clock->cycle_last)&  clock->mask;
>          clock->cycle_last = cycle_now;
>
> So that updates the time with the current offset between cycle_last and
> the current value.
>
> kernel/time/timekeeping.c:timekeeping_resume():
>          /* re-base the last cycle value */
>          timekeeper.clock->cycle_last = timekeeper.clock->read(timekeeper.clock);
>
> So this re-sets cycle_last to the current value of the clocksource.  This
> means that on resume, the clocksource can start counting from any value it
> likes.
>
> So, without any additional external inputs, time resumes incrementing at
> the point where the suspend occurred without any jump backwards or forwards.
>
> The code accounts for the sleep time by using read_persistent_clock() read
> a timer which continues running during sleep to calculate the delta between
> suspend and resume, and injects the delta between them to wind the time
> forward.
>
>> Then we have cpuidle. Is it ok to stop/freeze the timer during cpuidle
>> sleep states?
>
> During _idle_ (iow, no task running) sched_clock and the clocksource
> should both continue to run - the scheduler needs to know how long the
> system has been idle for, and the clocksource can't stop because we'll
> lose track of time.
>
> Remember that the clockevent stuff is used as a trigger to the timekeeping
> code to read the clocksource, and update the current time.  Time is moved
> forward by the delta between a previous clocksource read and the current
> clocksource read.  So stopping or resetting the clocksource in unexpected
> ways (other than over suspend/resume as mentioned above) will result in
> time going weird.

Hmm, I have missed the existence of the read_persistent_clock(). It 
sounds like I should keep the MTU as my clocksource / sched_clock and 
have the PRCMU Timer as a persistent_clock instead.

Then one problem remains. The MTU will be powered during cstates: 
running, wfi, ApIdle (arm retenetion). The MTU will loose power during 
cstates ApSleep and ApDeepSleep. So I need to do a similar sync as 
suspend does against the persistent_clock but when leaving and enter the 
deeper cstates.

Should I solve it in the clocksource framework with a flag telling which 
cstates the timer will stop/freeze and then inject time from the 
persistent_clock for those cstates? (I am thinking something like the 
CLOCK_EVT_FEAT_C3STOP flag)

Am I on the wrong track here or how should I solve it?

WARNING: multiple messages have this Message-ID (diff)
From: Mattias Wallin <mattias.wallin@stericsson.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCHv2 0/3] clocksource: add db8500 PRCMU timer
Date: Thu, 2 Jun 2011 14:10:02 +0200	[thread overview]
Message-ID: <4DE77D9A.6090301@stericsson.com> (raw)
In-Reply-To: <20110602110137.GU3660@n2100.arm.linux.org.uk>

On 06/02/2011 01:01 PM, Russell King - ARM Linux wrote:
> On Thu, Jun 02, 2011 at 12:18:35PM +0200, Mattias Wallin wrote:
>> On 06/02/2011 11:46 AM, Russell King - ARM Linux wrote:
>>> Why don't we just find a way of fixing sched_clock so that the value
>>> doesn't reset over a suspend/resume cycle?
>> Even if the value isn't reset during suspend/resume we want the
>> clocksource to keep counting. Or is it ok to have the clocksource stop
>> or freeze during suspend?
>
> kernel/time/timekeeping.c:timekeeping_suspend():
>
>          timekeeping_forward_now();
>
> which does:
>          cycle_now = clock->read(clock);
>          cycle_delta = (cycle_now - clock->cycle_last)&  clock->mask;
>          clock->cycle_last = cycle_now;
>
> So that updates the time with the current offset between cycle_last and
> the current value.
>
> kernel/time/timekeeping.c:timekeeping_resume():
>          /* re-base the last cycle value */
>          timekeeper.clock->cycle_last = timekeeper.clock->read(timekeeper.clock);
>
> So this re-sets cycle_last to the current value of the clocksource.  This
> means that on resume, the clocksource can start counting from any value it
> likes.
>
> So, without any additional external inputs, time resumes incrementing at
> the point where the suspend occurred without any jump backwards or forwards.
>
> The code accounts for the sleep time by using read_persistent_clock() read
> a timer which continues running during sleep to calculate the delta between
> suspend and resume, and injects the delta between them to wind the time
> forward.
>
>> Then we have cpuidle. Is it ok to stop/freeze the timer during cpuidle
>> sleep states?
>
> During _idle_ (iow, no task running) sched_clock and the clocksource
> should both continue to run - the scheduler needs to know how long the
> system has been idle for, and the clocksource can't stop because we'll
> lose track of time.
>
> Remember that the clockevent stuff is used as a trigger to the timekeeping
> code to read the clocksource, and update the current time.  Time is moved
> forward by the delta between a previous clocksource read and the current
> clocksource read.  So stopping or resetting the clocksource in unexpected
> ways (other than over suspend/resume as mentioned above) will result in
> time going weird.

Hmm, I have missed the existence of the read_persistent_clock(). It 
sounds like I should keep the MTU as my clocksource / sched_clock and 
have the PRCMU Timer as a persistent_clock instead.

Then one problem remains. The MTU will be powered during cstates: 
running, wfi, ApIdle (arm retenetion). The MTU will loose power during 
cstates ApSleep and ApDeepSleep. So I need to do a similar sync as 
suspend does against the persistent_clock but when leaving and enter the 
deeper cstates.

Should I solve it in the clocksource framework with a flag telling which 
cstates the timer will stop/freeze and then inject time from the 
persistent_clock for those cstates? (I am thinking something like the 
CLOCK_EVT_FEAT_C3STOP flag)

Am I on the wrong track here or how should I solve it?

  reply	other threads:[~2011-06-02 12:10 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-02  9:34 [PATCHv2 0/3] clocksource: add db8500 PRCMU timer Mattias Wallin
2011-06-02  9:34 ` Mattias Wallin
2011-06-02  9:46 ` Russell King - ARM Linux
2011-06-02  9:46   ` Russell King - ARM Linux
2011-06-02 10:18   ` Mattias Wallin
2011-06-02 10:18     ` Mattias Wallin
2011-06-02 11:01     ` Russell King - ARM Linux
2011-06-02 11:01       ` Russell King - ARM Linux
2011-06-02 12:10       ` Mattias Wallin [this message]
2011-06-02 12:10         ` Mattias Wallin
2011-06-02 12:57         ` Santosh Shilimkar
2011-06-02 12:57           ` Santosh Shilimkar
2011-06-02 13:04           ` Russell King - ARM Linux
2011-06-02 13:04             ` Russell King - ARM Linux
2011-06-02 13:16             ` Santosh Shilimkar
2011-06-02 13:16               ` Santosh Shilimkar
2011-06-02 18:47           ` john stultz
2011-06-02 18:47             ` john stultz
2011-06-08 13:44             ` Mattias Wallin
2011-06-08 13:44               ` Mattias Wallin
2011-06-09 21:59               ` Russell King - ARM Linux
2011-06-09 21:59                 ` Russell King - ARM Linux
2011-06-10  8:54                 ` Mattias Wallin
2011-06-10  8:54                   ` Mattias Wallin
2011-06-10 16:00                   ` Mattias Wallin
2011-06-10 16:00                     ` Mattias Wallin
2011-07-10 14:19   ` Russell King - ARM Linux
2011-07-10 14:19     ` Russell King - ARM Linux
2012-02-04 12:30   ` Russell King - ARM Linux
2012-02-04 12:30     ` Russell King - ARM Linux
2012-02-05 15:11     ` Linus Walleij
2012-02-05 15:11       ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DE77D9A.6090301@stericsson.com \
    --to=mattias.wallin@stericsson.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.