From patchwork Fri Jan 23 18:00:17 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "pang.xunlei" X-Patchwork-Id: 43682 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-lb0-f200.google.com (mail-lb0-f200.google.com [209.85.217.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id D396D240D9 for ; Fri, 23 Jan 2015 18:00:29 +0000 (UTC) Received: by mail-lb0-f200.google.com with SMTP id u10sf5036266lbd.3 for ; Fri, 23 Jan 2015 10:00:28 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mime-version:in-reply-to:references :date:message-id:subject:from:to:cc:content-type:sender:precedence :list-id:x-original-sender:x-original-authentication-results :mailing-list:list-post:list-help:list-archive:list-unsubscribe; bh=ZlPXvt3hbz76K2MBOWbt5KwG1sVHFQ+9/FDwEYS7aQA=; b=MWcKdGS+Cp0B+4UdSxOO6GTWsRJs9ZpJk0R4mTBdlRPMP2vNncpGKdkH2ktxYVUOsb dbJ5SImBP/LB72abi9GKvDskjiEwf3YlhyRrj0yc7Td4xOXGtIv1bJGvqoozh5a727Uh AQUmKql7ks2mrNk1AsFKWTzAEe+v1D2NciybPCqCTjr1TuFHL/8mtF/506ZkRZdsg8Cf YtEFeYBKZdH3Qehj2FWa8Svl8Pdeuu2MMypgsRZiHXOqHFcMuICiXwpcZdZ9M1yHm5ds zecEgJD90P4qO244BIWUhgbTBfk5C5UgDQrkUJeUdQkiSclM0fzDL2nX0BUpOeuAHjoa 64SA== X-Gm-Message-State: ALoCoQmhVobr0p3AvEQc1HSr5F92NjdGvillM93h5x9kmlQ77IveJNZiwWi/warVYeWwp5TvGxGP X-Received: by 10.112.9.199 with SMTP id c7mr1192689lbb.7.1422036028802; Fri, 23 Jan 2015 10:00:28 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.5.9 with SMTP id o9ls334202lao.83.gmail; Fri, 23 Jan 2015 10:00:28 -0800 (PST) X-Received: by 10.152.21.228 with SMTP id y4mr8859087lae.72.1422036028621; Fri, 23 Jan 2015 10:00:28 -0800 (PST) Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com. [209.85.217.181]) by mx.google.com with ESMTPS id j1si1931969lag.127.2015.01.23.10.00.28 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 23 Jan 2015 10:00:28 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.181 as permitted sender) client-ip=209.85.217.181; Received: by mail-lb0-f181.google.com with SMTP id u10so8332352lbd.12 for ; Fri, 23 Jan 2015 10:00:28 -0800 (PST) X-Received: by 10.152.3.70 with SMTP id a6mr8728166laa.71.1422036028469; Fri, 23 Jan 2015 10:00:28 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.9.200 with SMTP id c8csp338501lbb; Fri, 23 Jan 2015 10:00:27 -0800 (PST) X-Received: by 10.68.219.231 with SMTP id pr7mr13070956pbc.73.1422036026463; Fri, 23 Jan 2015 10:00:26 -0800 (PST) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id ej17si2629191pdb.218.2015.01.23.10.00.25; Fri, 23 Jan 2015 10:00:26 -0800 (PST) Received-SPF: none (google.com: linux-kernel-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756134AbbAWSAV (ORCPT + 28 others); Fri, 23 Jan 2015 13:00:21 -0500 Received: from mail-yk0-f172.google.com ([209.85.160.172]:63282 "EHLO mail-yk0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754530AbbAWSAS (ORCPT ); Fri, 23 Jan 2015 13:00:18 -0500 Received: by mail-yk0-f172.google.com with SMTP id 9so3411240ykp.3 for ; Fri, 23 Jan 2015 10:00:17 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.236.29.17 with SMTP id h17mr3759626yha.188.1422036017214; Fri, 23 Jan 2015 10:00:17 -0800 (PST) Received: by 10.170.191.77 with HTTP; Fri, 23 Jan 2015 10:00:17 -0800 (PST) In-Reply-To: References: <1421928077-4698-1-git-send-email-pang.xunlei@linaro.org> Date: Sat, 24 Jan 2015 02:00:17 +0800 Message-ID: Subject: Re: [PATCH v2 1/3] time: Don't bother to run rtc_resume() for the nonstop clocksource From: Xunlei Pang To: John Stultz Cc: lkml , "rtc-linux@googlegroups.com" , Thomas Gleixner , Alessandro Zummo , Arnd Bergmann Sender: linux-kernel-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: pang.xunlei@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.181 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , On 23 January 2015 at 02:30, John Stultz wrote: > On Thu, Jan 22, 2015 at 4:01 AM, Xunlei Pang wrote: >> 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 >> --- >> v1->v2: >> Modify according to Thomas' feedback. >> >> drivers/rtc/class.c | 2 +- >> include/linux/timekeeping.h | 7 +++++++ >> kernel/time/timekeeping.c | 16 +++++----------- >> 3 files changed, 13 insertions(+), 12 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; >> >> rtc_hctosys_ret = -ENODEV; >> diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h >> index 9b63d13..2b87c64 100644 >> --- a/include/linux/timekeeping.h >> +++ b/include/linux/timekeeping.h >> @@ -238,6 +238,13 @@ extern void getnstime_raw_and_real(struct timespec *ts_raw, >> */ >> extern bool persistent_clock_exist; >> extern int persistent_clock_is_local; >> +extern bool timekeeping_sleeptime_inject; >> + >> +/* Used by rtc_resume() */ >> +static inline bool timekeeping_sleeptime_injected(void) >> +{ >> + return timekeeping_sleeptime_inject; >> +} >> > > Again, there's no reason to make this a static inline, nor the > sleeptime_inject variable global. > Just make it a function in timekeeping and provide definition so it > can be used by the rtc resume. Hi John, Thanks for your comments, I've refined it, is this one ok? --- drivers/rtc/class.c | 2 +- include/linux/timekeeping.h | 1 + kernel/time/timekeeping.c | 32 ++++++++++++++++++++------------ 3 files changed, 22 insertions(+), 13 deletions(-) * This function should only be called by rtc_resume(), and allows * a suspend offset to be injected into the timekeeping values. @@ -1140,13 +1154,6 @@ void timekeeping_inject_sleeptime64(struct timespec64 *delta) struct timekeeper *tk = &tk_core.timekeeper; unsigned long flags; - /* - * Make sure we don't set the clock twice, as timekeeping_resume() - * already did it - */ - if (has_persistent_clock()) - return; - raw_spin_lock_irqsave(&timekeeper_lock, flags); write_seqcount_begin(&tk_core.seq); @@ -1162,6 +1169,7 @@ void timekeeping_inject_sleeptime64(struct timespec64 *delta) /* signal hrtimers about time change */ clock_was_set(); } +#endif /** * timekeeping_resume - Resumes the generic timekeeping subsystem. @@ -1178,8 +1186,8 @@ static void timekeeping_resume(void) struct timespec64 ts_new, ts_delta; struct timespec tmp; cycle_t cycle_now, cycle_delta; - bool suspendtime_found = false; + sleeptime_inject = false; read_persistent_clock(&tmp); ts_new = timespec_to_timespec64(tmp); @@ -1226,13 +1234,13 @@ static void timekeeping_resume(void) nsec += ((u64) cycle_delta * mult) >> shift; ts_delta = ns_to_timespec64(nsec); - suspendtime_found = true; + sleeptime_inject = true; } else if (timespec64_compare(&ts_new, &timekeeping_suspend_time) > 0) { ts_delta = timespec64_sub(ts_new, timekeeping_suspend_time); - suspendtime_found = true; + sleeptime_inject = true; } - if (suspendtime_found) + if (sleeptime_inject) __timekeeping_inject_sleeptime(tk, &ts_delta); /* Re-base the last cycle value */ 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; 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) +/** + * Used by rtc_resume(). + */ +bool timekeeping_sleeptime_injected(void) +{ + return sleeptime_inject; +} + /** * timekeeping_inject_sleeptime64 - Adds suspend interval to timeekeeping values * @delta: pointer to a timespec64 delta value * * This hook is for architectures that cannot support read_persistent_clock - * because their RTC/persistent clock is only accessible when irqs are enabled. + * because their RTC/persistent clock is only accessible when irqs are enabled, + * and also don't have an effective nonstop clocksource. *