Hi Feng, Looking at the OMAP implementation, persistent_clock is updated on every read of the 32K counter. If the read doesn't happen often enough to accurately update persistent_clock, then the 32K counter would fail the definition of a persistent clock and some other timekeeping source should be used. Regards, Zoran On 12 October 2013 00:48, Feng Tang <feng.tang@xxxxxxxxx> wrote: > Hi Zoran, > > Thanks for the patch! (This reply may be toooo late :)) > > One question just for curiosity: for the counter_32K timer, it's running > at 32K Hz and has one 32b counter. I understand it is only for suspend > time calculation use, but the wrap time for it is about > 4G/32K ~= 128K seconds ~= 35 hours > What if one suspend time is longer than that? > > - Feng > > On Fri, May 17, 2013 at 11:24:05AM -0700, Zoran Markovic wrote: >> Since commit <31ade30692dc9680bfc95700d794818fa3f754ac>, timekeeping_init() >> checks for presence of persistent clock by attempting to read a non-zero >> time value from real-time clock. This is an issue on platforms where >> persistent_clock (instead of a RTC) is implemented as a free-running counter >> starting from zero on each boot and running during suspend. Examples are some >> ARM platforms (e.g. PandaBoard). An attempt to read such a clock during >> timekeeping_init() may return zero value and falsely declare persistent clock >> as missing. Additionally, in the above case suspend times may be accounted >> twice (once from timekeeping_resume() and once from rtc_resume()), resulting >> in a gradual drift of system time. >> >> This patch does a run-time correction of the issue by doing the same check >> during timekeeping_suspend(). >> >> A better long-term solution would have to return error when trying to read >> non-existing clock and zero when trying to read an uninitialized clock, but >> that would require changing all persistent_clock implementations. >> >> This patch addresses the immediate breakage, for now. >> >> Cc: John Stultz <john.stultz@xxxxxxxxxx> >> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> >> Cc: Feng Tang <feng.tang@xxxxxxxxx> >> Cc: stable@xxxxxxxxxxxxxxx >> Signed-off-by: Zoran Markovic <zoran.markovic@xxxxxxxxxx> >> --- >> kernel/time/timekeeping.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c >> index 98cd470..baeeb5c 100644 >> --- a/kernel/time/timekeeping.c >> +++ b/kernel/time/timekeeping.c >> @@ -975,6 +975,14 @@ static int timekeeping_suspend(void) >> >> read_persistent_clock(&timekeeping_suspend_time); >> >> + /* >> + * On some systems the persistent_clock can not be detected at >> + * timekeeping_init by its return value, so if we see a valid >> + * value returned, update the persistent_clock_exists flag. >> + */ >> + if (timekeeping_suspend_time.tv_sec || timekeeping_suspend_time.tv_nsec) >> + persistent_clock_exist = true; >> + >> raw_spin_lock_irqsave(&timekeeper_lock, flags); >> write_seqcount_begin(&timekeeper_seq); >> timekeeping_forward_now(tk); >> -- >> 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html