Messages in this thread | | | Date | Wed, 11 Feb 2015 08:01:43 +0800 | Subject | Re: [PATCH v3 1/2] time: Don't bother to run rtc_resume() for the nonstop clocksource | From | John Stultz <> |
| |
On Thu, Jan 29, 2015 at 11:59 PM, Xunlei Pang <xlpang@126.com> wrote: > From: Xunlei Pang <pang.xunlei@linaro.org> > > If a system does not provide a persistent_clock(), the time > will be updated on resume by rtc_resume(). With the addition > of the non-stop clocksources for suspend timing, those systems > set the time on resume in timekeeping_resume(), but may not > provide a valid persistent_clock(). > > This results in the rtc_resume() logic thinking no one has set > the time and it then will over-write the suspend time again, > which is not necessary and only increases clock error. > > So, fix this for rtc_resume(). > > Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org> > --- > v2->v3: > Refine according to John's comments using internal variable. > > drivers/rtc/class.c | 2 +- > include/linux/timekeeping.h | 1 + > kernel/time/timekeeping.c | 32 ++++++++++++++++++++------------ > 3 files changed, 22 insertions(+), 13 deletions(-) > > diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c > index 472a5ad..6100af5 100644 > --- a/drivers/rtc/class.c > +++ b/drivers/rtc/class.c > @@ -102,7 +102,7 @@ static int rtc_resume(struct device *dev) > struct timespec64 sleep_time; > int err; > > - if (has_persistent_clock()) > + if (timekeeping_sleeptime_injected()) > return 0;
Took a closer look here.. So you're replacing has_persistent_clock() in the resume side, but not the suspend... Can we not cleanup has_persistent_clock and consolidate to one accessor for both sides of the suspend?
> > rtc_hctosys_ret = -ENODEV; > diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h > index 9b63d13..17a460d 100644 > --- a/include/linux/timekeeping.h > +++ b/include/linux/timekeeping.h > @@ -225,6 +225,7 @@ static inline void timekeeping_clocktai(struct timespec *ts) > /* > * RTC specific > */ > +extern bool timekeeping_sleeptime_injected(void); > extern void timekeeping_inject_sleeptime64(struct timespec64 *delta); > > /* > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > index 6a93185..b02133e 100644 > --- a/kernel/time/timekeeping.c > +++ b/kernel/time/timekeeping.c > @@ -1125,12 +1125,26 @@ static void __timekeeping_inject_sleeptime(struct timekeeper *tk, > tk_debug_account_sleep_time(delta); > } > > +static bool sleeptime_inject; > + > +#if defined(CONFIG_RTC_CLASS) && \ > + defined(CONFIG_PM_SLEEP) && \ > + defined(CONFIG_RTC_HCTOSYS_DEVICE)
This change wasn't explained in the commit message. Its fine as a small optimization, but probably should be split into its own patch.
thanks -john
| |