Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753542AbeAILQO (ORCPT + 1 other); Tue, 9 Jan 2018 06:16:14 -0500 Received: from mail-ot0-f195.google.com ([74.125.82.195]:40142 "EHLO mail-ot0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751119AbeAILQM (ORCPT ); Tue, 9 Jan 2018 06:16:12 -0500 X-Google-Smtp-Source: ACJfBovsASLruDT2e0YLDkk9DZT2QAN8n6ZXI5ZNj8OghSgONRERtLKHU0WwXbkvUpa7bJ7ALB+pPeYc50eQOIA3CMA= MIME-Version: 1.0 In-Reply-To: References: From: Arnd Bergmann Date: Tue, 9 Jan 2018 12:16:10 +0100 X-Google-Sender-Auth: ppMendBUcfgh-MCyzmPv3-0iq3w Message-ID: Subject: Re: [PATCH 1/2] clocksource: Add persistent timer support To: Baolin Wang Cc: Daniel Lezcano , Thomas Gleixner , Mark Brown , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Tue, Jan 9, 2018 at 10:09 AM, Baolin Wang wrote: > On some platforms (such as Spreadtrum sc9860 platform), they need one > persistent timer to calculate the suspended time and compensate it > for the OS time. > > But now there are no method to register one persistent timer on some > architectures (such as arm64), thus this patch adds one common framework > for timer drivers to register one persistent timer and implements the > read_persistent_clock64() to compensate the OS time. > > Signed-off-by: Baolin Wang Hi Baolin, I like the idea of having a more generalized way of dealing with persistent clocks, but I wonder if we can do a little better than coming up with another API that is completely separated from the clocksource API. The situation on sc9860 appears to be similar to what we have on OMAP: there is one clockcource that is persistent, and another one that is for some reason preferred on some systems as the system clock, but that is not persistent. On OMAP, we register drivers/clocksource/timer-ti-32k.c as a clocksource with CLOCK_SOURCE_SUSPEND_NONSTOP set, and then also register the same thing from arch/arm/plat-omap/counter_32k.c through the arm32 specific register_persistent_clock() interface, and then we also register another clocksource on some chips that may be preferred. In timekeeping_resume(), we fall back to read_persistent_clock64() when the currently active clocksource doesn't have CLOCK_SOURCE_SUSPEND_NONSTOP set, so that works fine with both the OMAP requirement and your new code, but we might be able to do better: If the persistent clock also gets registered as a normal clocksource, and the primary clocksource doesn't have CLOCK_SOURCE_SUSPEND_NONSTOP set, maybe we can switch clock sources in timekeeping_suspend(), and back in timekeeping_resume()? Alternatively, we might keep the read_persistent_clock64() interface for your case, but then get rid of persistent_timer_init_and_register() and make it a hook inside of __clocksource_register_scale() that gets called for any clocksource with CLOCK_SOURCE_SUSPEND_NONSTOP set? Arnd