| Message ID | fe8f75e39938954be9f3f1f1e2179c4c398d6f55.1515486685.git.baolin.wang@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] clocksource: Add persistent timer support | expand |
On Tue, Jan 9, 2018 at 10:09 AM, Baolin Wang <baolin.wang@linaro.org> 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 <baolin.wang@linaro.org> 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
On 9 January 2018 at 19:16, Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Jan 9, 2018 at 10:09 AM, Baolin Wang <baolin.wang@linaro.org> 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 <baolin.wang@linaro.org> > > 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 If we register one clocksource, the clocksource can only convert maximum 10 minutes time considering a good conversion precision. But 10 minutes is not enough for suspend, we need one larger time can be suspended. That is why I did not register the persistent clock as one clocksource. We can saw in arch/arm/plat-omap/counter_32k.c, the counter implemented its mult and shift with the maximum time 120000 seconds, instead of using the clocksource to read persistent clock. > CLOCK_SOURCE_SUSPEND_NONSTOP set, maybe we can > switch clock sources in timekeeping_suspend(), and back in > timekeeping_resume()? Alternatively, we might keep the I checked there are no APIs to switch one SUSPEND_NONSTOP clocksource now, maybe we can implement them but we should solve the 10 minutes conversion limitation for clocksource firstly. > 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? I think this is simple and good to solve our issue, but like I said the conversion limitation of one clocksource is not enough for persistent clock. Thanks for your suggestion. -- Baolin.wang Best Regards
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 9a6b087..9cf3d41 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -27,6 +27,9 @@ config CLKBLD_I8253 config CLKSRC_MMIO bool +config PERSISTENT_TIMER + bool + config BCM2835_TIMER bool "BCM2835 timer driver" if COMPILE_TEST select CLKSRC_MMIO diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index d6dec44..42556f4 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -76,3 +76,4 @@ obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o obj-$(CONFIG_H8300_TPU) += h8300_tpu.o obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o obj-$(CONFIG_X86_NUMACHIP) += numachip.o +obj-$(CONFIG_PERSISTENT_TIMER) += persistent-timer.o diff --git a/drivers/clocksource/persistent-timer.c b/drivers/clocksource/persistent-timer.c new file mode 100644 index 0000000..89a631d --- /dev/null +++ b/drivers/clocksource/persistent-timer.c @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2017 Linaro, Inc. + * + * Author: Baolin Wang <baolin.wang@linaro.org> + */ + +#include <linux/clk.h> +#include <linux/clocksource.h> +#include <linux/of.h> +#include <linux/of_address.h> + +#include "persistent-timer.h" + +static struct persistent_timer *ptimer; + +void read_persistent_clock64(struct timespec64 *ts) +{ + u64 cycles, delta, nsecs; + + if (!ptimer) { + ts->tv_sec = 0; + ts->tv_nsec = 0; + return; + } + + cycles = ptimer->read(ptimer); + delta = (cycles - ptimer->last_cycles) & ptimer->mask; + + nsecs = clocksource_cyc2ns(delta, ptimer->mult, ptimer->shift); + timespec64_add_ns(&ptimer->ts, nsecs); + *ts = ptimer->ts; + + ptimer->last_cycles = cycles; +} + +static __init void persistent_timer_freq_init(struct persistent_timer *p) +{ + u64 max_sec = p->mask; + + do_div(max_sec, p->freq); + + /* + * Some counter's mask can be larger than 32bit, so we need to limit + * the max suspend time to have a good conversion precision. 12 hours + * can be enough usually. + */ + if (max_sec > 43200) + max_sec = 43200; + + clocks_calc_mult_shift(&p->mult, &p->shift, p->freq, + NSEC_PER_SEC, (u32)max_sec); +} + +static __init int persistent_timer_clk_init(struct device_node *np, + struct persistent_timer *p) +{ + int ret; + + p->clk = of_clk_get(np, 0); + if (IS_ERR(p->clk)) { + pr_err("Can't get timer clock\n"); + return PTR_ERR(p->clk); + } + + ret = clk_prepare_enable(p->clk); + if (ret) { + pr_err("Failed to enable clock for %pOF\n", np); + clk_put(p->clk); + return ret; + } + + p->freq = clk_get_rate(p->clk); + if (!p->freq) { + pr_err("Failed to get clock rate for %pOF\n", np); + clk_disable_unprepare(p->clk); + clk_put(p->clk); + return -EINVAL; + } + + return 0; +} + +static __init int persistent_timer_base_init(struct device_node *np, + struct persistent_timer *p) +{ + p->base = of_io_request_and_map(np, 0, of_node_full_name(np)); + if (IS_ERR(p->base)) { + pr_err("Can't map timer registers\n"); + return PTR_ERR(p->base); + } + + return 0; +} + +int __init persistent_timer_init_and_register(struct device_node *np, + struct persistent_timer *p) +{ + int ret; + + if (!p->read || !p->mask) + return -EINVAL; + + if (p->flags & PERSISTENT_TIMER_BASE) { + ret = persistent_timer_base_init(np, p); + if (ret) + return ret; + } + + if (p->flags & PERSISTENT_TIMER_CLOCK) { + ret = persistent_timer_clk_init(np, p); + if (ret) + goto err_clk; + } + + if (p->flags & PERSISTENT_TIMER_MULT_SHIFT) + persistent_timer_freq_init(p); + + p->last_cycles = 0; + ptimer = p; + return 0; + +err_clk: + if (p->flags & PERSISTENT_TIMER_BASE) + iounmap(p->base); + + return ret; +} + +void __init persistent_timer_cleanup(struct persistent_timer *p) +{ + if (p->flags & PERSISTENT_TIMER_CLOCK) { + p->freq = 0; + clk_disable_unprepare(p->clk); + clk_put(p->clk); + } + + if (p->flags & PERSISTENT_TIMER_BASE) + iounmap(p->base); + + ptimer = NULL; +} diff --git a/drivers/clocksource/persistent-timer.h b/drivers/clocksource/persistent-timer.h new file mode 100644 index 0000000..7705027 --- /dev/null +++ b/drivers/clocksource/persistent-timer.h @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 +#ifndef __PERSISTENT_TIMER_H__ +#define __PERSISTENT_TIMER_H__ + +#define PERSISTENT_TIMER_BASE BIT(0) +#define PERSISTENT_TIMER_CLOCK BIT(1) +#define PERSISTENT_TIMER_MULT_SHIFT BIT(2) + +struct persistent_timer { + u64 (*read)(struct persistent_timer *p); + void __iomem *base; + u64 last_cycles; + u64 mask; + u32 mult; + u32 shift; + struct clk *clk; + unsigned long freq; + unsigned int flags; + struct timespec64 ts; +}; + +extern int __init persistent_timer_init_and_register(struct device_node *np, + struct persistent_timer *p); +extern void __init persistent_timer_cleanup(struct persistent_timer *p); + +#endif
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 <baolin.wang@linaro.org> --- drivers/clocksource/Kconfig | 3 + drivers/clocksource/Makefile | 1 + drivers/clocksource/persistent-timer.c | 142 ++++++++++++++++++++++++++++++++ drivers/clocksource/persistent-timer.h | 26 ++++++ 4 files changed, 172 insertions(+) create mode 100644 drivers/clocksource/persistent-timer.c create mode 100644 drivers/clocksource/persistent-timer.h -- 1.7.9.5