On Thu, Jan 29, 2015 at 11:59 PM, Xunlei Pang <xlp...@126.com> wrote: > From: Xunlei Pang <pang.xun...@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.xun...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
