| Message ID | 1415388855-35074-1-git-send-email-anatol.pomozov@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hi This patch opens possibility for further timekeeping cleanup: read_persistent_clock() is defined as a weak and expected that architecture implement it. The users of this function need to check return value. If it is equal zero then persistent clock is not provided. It looks hacky. It makes code nicer if architecture called register_persistent_clock(&my_persistent_clock); and then users check if persistent_clock variable is non-NULL. On Fri, Nov 7, 2014 at 11:34 AM, Anatol Pomozov <anatol.pomozov@gmail.com> wrote: > ARM timekeeping functionality allows to register persistent/boot clock dynamically. > This code is arch-independent and can be useful on other plaforms as well. > > As a byproduct of this change, tegra20_timer becomes ARM64 compatible. > > Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit > board, made sure high-resolution clock works. > > Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com> > --- > arch/arm/include/asm/mach/time.h | 5 ----- > arch/arm/kernel/time.c | 36 ------------------------------- > arch/arm/plat-omap/counter_32k.c | 9 +++++--- > drivers/clocksource/tegra20_timer.c | 10 +++++---- > include/linux/timekeeping.h | 11 ++++++++++ > kernel/time/timekeeping.c | 43 +++++++++++++++++++++++++++++++++---- > 6 files changed, 62 insertions(+), 52 deletions(-) > > diff --git a/arch/arm/include/asm/mach/time.h b/arch/arm/include/asm/mach/time.h > index 90c12e1..3cbcafc 100644 > --- a/arch/arm/include/asm/mach/time.h > +++ b/arch/arm/include/asm/mach/time.h > @@ -12,9 +12,4 @@ > > extern void timer_tick(void); > > -struct timespec; > -typedef void (*clock_access_fn)(struct timespec *); > -extern int register_persistent_clock(clock_access_fn read_boot, > - clock_access_fn read_persistent); > - > #endif > diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c > index 0cc7e58..0aa1dcd 100644 > --- a/arch/arm/kernel/time.c > +++ b/arch/arm/kernel/time.c > @@ -76,42 +76,6 @@ void timer_tick(void) > } > #endif > > -static void dummy_clock_access(struct timespec *ts) > -{ > - ts->tv_sec = 0; > - ts->tv_nsec = 0; > -} > - > -static clock_access_fn __read_persistent_clock = dummy_clock_access; > -static clock_access_fn __read_boot_clock = dummy_clock_access;; > - > -void read_persistent_clock(struct timespec *ts) > -{ > - __read_persistent_clock(ts); > -} > - > -void read_boot_clock(struct timespec *ts) > -{ > - __read_boot_clock(ts); > -} > - > -int __init register_persistent_clock(clock_access_fn read_boot, > - clock_access_fn read_persistent) > -{ > - /* Only allow the clockaccess functions to be registered once */ > - if (__read_persistent_clock == dummy_clock_access && > - __read_boot_clock == dummy_clock_access) { > - if (read_boot) > - __read_boot_clock = read_boot; > - if (read_persistent) > - __read_persistent_clock = read_persistent; > - > - return 0; > - } > - > - return -EINVAL; > -} > - > void __init time_init(void) > { > if (machine_desc->init_time) { > diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c > index 61b4d70..0dbfffd 100644 > --- a/arch/arm/plat-omap/counter_32k.c > +++ b/arch/arm/plat-omap/counter_32k.c > @@ -18,10 +18,9 @@ > #include <linux/err.h> > #include <linux/io.h> > #include <linux/clocksource.h> > +#include <linux/time.h> > #include <linux/sched_clock.h> > > -#include <asm/mach/time.h> > - > #include <plat/counter-32k.h> > > /* OMAP2_32KSYNCNT_CR_OFF: offset of 32ksync counter register */ > @@ -76,6 +75,10 @@ static void omap_read_persistent_clock(struct timespec *ts) > spin_unlock_irqrestore(&read_persistent_clock_lock, flags); > } > > +static const struct persistent_clock_ops omap_persistent_clock = { > + .read = omap_read_persistent_clock > +}; > + > /** > * omap_init_clocksource_32k - setup and register counter 32k as a > * kernel clocksource > @@ -116,7 +119,7 @@ int __init omap_init_clocksource_32k(void __iomem *vbase) > } > > sched_clock_register(omap_32k_read_sched_clock, 32, 32768); > - register_persistent_clock(NULL, omap_read_persistent_clock); > + register_persistent_clock(&omap_persistent_clock); > pr_info("OMAP clocksource: 32k_counter at 32768 Hz\n"); > > return 0; > diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c > index d2616ef..210130e 100644 > --- a/drivers/clocksource/tegra20_timer.c > +++ b/drivers/clocksource/tegra20_timer.c > @@ -28,9 +28,7 @@ > #include <linux/of_irq.h> > #include <linux/sched_clock.h> > #include <linux/delay.h> > - > -#include <asm/mach/time.h> > -#include <asm/smp_twd.h> > +#include <linux/timekeeping.h> > > #define RTC_SECONDS 0x08 > #define RTC_SHADOW_SECONDS 0x0c > @@ -142,6 +140,10 @@ static void tegra_read_persistent_clock(struct timespec *ts) > *ts = *tsp; > } > > +static const struct persistent_clock_ops tegra_persistent_clock = { > + .read = tegra_read_persistent_clock > +}; > + > static unsigned long tegra_delay_timer_read_counter_long(void) > { > return readl(timer_reg_base + TIMERUS_CNTR_1US); > @@ -252,7 +254,7 @@ static void __init tegra20_init_rtc(struct device_node *np) > else > clk_prepare_enable(clk); > > - register_persistent_clock(NULL, tegra_read_persistent_clock); > + register_persistent_clock(&tegra_persistent_clock); > } > CLOCKSOURCE_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); > > diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h > index 1caa6b0..02023f7 100644 > --- a/include/linux/timekeeping.h > +++ b/include/linux/timekeeping.h > @@ -201,6 +201,17 @@ static inline bool has_persistent_clock(void) > return persistent_clock_exist; > } > > +struct persistent_clock_ops { > + void (*read)(struct timespec *ts); > + int (*update)(const struct timespec ts); > +}; > + > +struct boot_clock_ops { > + void (*read)(struct timespec *ts); > +}; > + > +extern void register_persistent_clock(const struct persistent_clock_ops *clock); > +extern void register_boot_clock(const struct boot_clock_ops *clock); > extern void read_persistent_clock(struct timespec *ts); > extern void read_boot_clock(struct timespec *ts); > extern int update_persistent_clock(struct timespec now); > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > index fb4a9c2..414c172 100644 > --- a/kernel/time/timekeeping.c > +++ b/kernel/time/timekeeping.c > @@ -66,6 +66,9 @@ int __read_mostly timekeeping_suspended; > /* Flag for if there is a persistent clock on this platform */ > bool __read_mostly persistent_clock_exist = false; > > +const struct persistent_clock_ops __read_mostly *persistent_clock = NULL; > +const struct boot_clock_ops __read_mostly *boot_clock = NULL; > + > static inline void tk_normalize_xtime(struct timekeeper *tk) > { > while (tk->tkr.xtime_nsec >= ((u64)NSEC_PER_SEC << tk->tkr.shift)) { > @@ -956,6 +959,30 @@ u64 timekeeping_max_deferment(void) > return ret; > } > > +extern void register_persistent_clock(const struct persistent_clock_ops *clock) > +{ > + BUG_ON(!clock); > + BUG_ON(!clock->read); > + > + if (persistent_clock) { > + pr_warn("Ignore extra persistent clock registration"); > + return; > + } > + persistent_clock = clock; > +} > + > +extern void register_boot_clock(const struct boot_clock_ops *clock) > +{ > + BUG_ON(!clock); > + BUG_ON(!clock->read); > + > + if (boot_clock) { > + pr_warn("Ignore extra boot clock registration"); > + return; > + } > + boot_clock = clock; > +} > + > /** > * read_persistent_clock - Return time from the persistent clock. > * > @@ -967,8 +994,12 @@ u64 timekeeping_max_deferment(void) > */ > void __weak read_persistent_clock(struct timespec *ts) > { > - ts->tv_sec = 0; > - ts->tv_nsec = 0; > + if (persistent_clock) { > + persistent_clock->read(ts); > + } else { > + ts->tv_sec = 0; > + ts->tv_nsec = 0; > + } > } > > /** > @@ -982,8 +1013,12 @@ void __weak read_persistent_clock(struct timespec *ts) > */ > void __weak read_boot_clock(struct timespec *ts) > { > - ts->tv_sec = 0; > - ts->tv_nsec = 0; > + if (boot_clock) { > + boot_clock->read(ts); > + } else { > + ts->tv_sec = 0; > + ts->tv_nsec = 0; > + } > } > > /* > -- > 2.1.0.rc2.206.gedb03e5 > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: > ARM timekeeping functionality allows to register persistent/boot clock dynamically. > This code is arch-independent and can be useful on other plaforms as well. > > As a byproduct of this change, tegra20_timer becomes ARM64 compatible. > > Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit > board, made sure high-resolution clock works. Using this on an upstream kernel doesn't work, though, because 64-bit ARM doesn't implement struct delay_timer which the driver needs since v3.17. But I suppose the delay timer infrastructure could be moved into the core similar to the persistent and boot clock as this patch does. Thierry
Hi On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: >> ARM timekeeping functionality allows to register persistent/boot clock dynamically. >> This code is arch-independent and can be useful on other plaforms as well. >> >> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. >> >> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit >> board, made sure high-resolution clock works. > > Using this on an upstream kernel doesn't work, though, because 64-bit > ARM doesn't implement struct delay_timer which the driver needs since > v3.17. > > But I suppose the delay timer infrastructure could be moved into the > core similar to the persistent and boot clock as this patch does. Thanks. It makes sense, I will send it in a separate patch, once this one will be reviewed. On our kernel I haven't seen this issue as we still use 3.14. In fact none of arch/arm/lib/delay.c code seems ARM32 related. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 10 Nov 2014, Anatol Pomozov wrote: > On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding > <thierry.reding@gmail.com> wrote: > > On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: > >> ARM timekeeping functionality allows to register persistent/boot clock dynamically. > >> This code is arch-independent and can be useful on other plaforms as well. > >> > >> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. > >> > >> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit > >> board, made sure high-resolution clock works. > > > > Using this on an upstream kernel doesn't work, though, because 64-bit > > ARM doesn't implement struct delay_timer which the driver needs since > > v3.17. > > > > But I suppose the delay timer infrastructure could be moved into the > > core similar to the persistent and boot clock as this patch does. > > Thanks. It makes sense, I will send it in a separate patch, once this > one will be reviewed. On our kernel I haven't seen this issue as we > still use 3.14. That's why you should test/compile your stuff on latest greatest and not on a year old conglomorate of unknown provenance. :) Aside of that I really wonder why we need that persistent_clock stuff at all. We already have mechanisms to register persistent clocks AKA RTCs after the early boot process and update the wall clock time before we actually need it. Nothing in early boot depends on correct wall clock at all. So instead of adding more extra persistent clock nonsense, can we just move all of that to the place where it belongs, i.e. RTC? Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 13, 2014 at 2:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Mon, 10 Nov 2014, Anatol Pomozov wrote: >> On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding >> <thierry.reding@gmail.com> wrote: >> > On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: >> >> ARM timekeeping functionality allows to register persistent/boot clock dynamically. >> >> This code is arch-independent and can be useful on other plaforms as well. >> >> >> >> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. >> >> >> >> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit >> >> board, made sure high-resolution clock works. >> > >> > Using this on an upstream kernel doesn't work, though, because 64-bit >> > ARM doesn't implement struct delay_timer which the driver needs since >> > v3.17. >> > >> > But I suppose the delay timer infrastructure could be moved into the >> > core similar to the persistent and boot clock as this patch does. >> >> Thanks. It makes sense, I will send it in a separate patch, once this >> one will be reviewed. On our kernel I haven't seen this issue as we >> still use 3.14. > > That's why you should test/compile your stuff on latest greatest and > not on a year old conglomorate of unknown provenance. :) > > Aside of that I really wonder why we need that persistent_clock stuff > at all. We already have mechanisms to register persistent clocks AKA > RTCs after the early boot process and update the wall clock time > before we actually need it. Nothing in early boot depends on correct > wall clock at all. > > So instead of adding more extra persistent clock nonsense, can we just > move all of that to the place where it belongs, i.e. RTC? Sigh.. I've got this on an eventual todo list.. The big problem though is that the RTC infrastructure can't be called with irqs off, so its not as optimal for measuring suspend time. Some of the suspend-time measurement with clocksources that don't halt is interesting here. So we need to add to the RTC infrastructure special accessors that are safe when irqs are off, and we can then deprecate the persistent clock bits. There's still evaluation quirks with setting the time earlier in boot or not (possibly some rng effects as well there), but that could be worked out if we had the suspend timing via safe RTC interfaces sorted. thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 13 Nov 2014, John Stultz wrote: > On Thu, Nov 13, 2014 at 2:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > Aside of that I really wonder why we need that persistent_clock stuff > > at all. We already have mechanisms to register persistent clocks AKA > > RTCs after the early boot process and update the wall clock time > > before we actually need it. Nothing in early boot depends on correct > > wall clock at all. > > > > So instead of adding more extra persistent clock nonsense, can we just > > move all of that to the place where it belongs, i.e. RTC? > > Sigh.. I've got this on an eventual todo list.. The big problem though > is that the RTC infrastructure can't be called with irqs off, so its > not as optimal for measuring suspend time. Some of the suspend-time > measurement with clocksources that don't halt is interesting here. > > So we need to add to the RTC infrastructure special accessors that are > safe when irqs are off, and we can then deprecate the persistent clock > bits. There's still evaluation quirks with setting the time earlier in > boot or not (possibly some rng effects as well there), but that could > be worked out if we had the suspend timing via safe RTC interfaces > sorted. So we better work on this instead of creating more legacy enforcement APIs.... Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi On Thu, Nov 13, 2014 at 2:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Mon, 10 Nov 2014, Anatol Pomozov wrote: >> On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding >> <thierry.reding@gmail.com> wrote: >> > On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: >> >> ARM timekeeping functionality allows to register persistent/boot clock dynamically. >> >> This code is arch-independent and can be useful on other plaforms as well. >> >> >> >> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. >> >> >> >> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit >> >> board, made sure high-resolution clock works. >> > >> > Using this on an upstream kernel doesn't work, though, because 64-bit >> > ARM doesn't implement struct delay_timer which the driver needs since >> > v3.17. >> > >> > But I suppose the delay timer infrastructure could be moved into the >> > core similar to the persistent and boot clock as this patch does. >> >> Thanks. It makes sense, I will send it in a separate patch, once this >> one will be reviewed. On our kernel I haven't seen this issue as we >> still use 3.14. > > That's why you should test/compile your stuff on latest greatest and > not on a year old conglomorate of unknown provenance. :) Unfortunately it is not possible to test this patch with upstream. There is no ARM64 bit support for Tegra yet. I am trying to cleanup/upstream my ChromeOS patches and this clock patch in particular makes one small step towards this goal. Also Thierry mentioned that he works on full ARM64 Tegra support and it is really exciting! So what I suppose to do with my patch? If it does not work could anyone provide patch that removes ARM arch dependency from tegra20_timer.c? > Aside of that I really wonder why we need that persistent_clock stuff > at all. We already have mechanisms to register persistent clocks AKA > RTCs after the early boot process and update the wall clock time > before we actually need it. Nothing in early boot depends on correct > wall clock at all. > > So instead of adding more extra persistent clock nonsense, can we just > move all of that to the place where it belongs, i.e. RTC? > > Thanks, > > tglx > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 14 Nov 2014, Anatol Pomozov wrote: > On Thu, Nov 13, 2014 at 2:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, 10 Nov 2014, Anatol Pomozov wrote: > >> On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding > >> <thierry.reding@gmail.com> wrote: > >> > On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: > >> >> ARM timekeeping functionality allows to register persistent/boot clock dynamically. > >> >> This code is arch-independent and can be useful on other plaforms as well. > >> >> > >> >> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. > >> >> > >> >> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit > >> >> board, made sure high-resolution clock works. > >> > > >> > Using this on an upstream kernel doesn't work, though, because 64-bit > >> > ARM doesn't implement struct delay_timer which the driver needs since > >> > v3.17. > >> > > >> > But I suppose the delay timer infrastructure could be moved into the > >> > core similar to the persistent and boot clock as this patch does. > >> > >> Thanks. It makes sense, I will send it in a separate patch, once this > >> one will be reviewed. On our kernel I haven't seen this issue as we > >> still use 3.14. > > > > That's why you should test/compile your stuff on latest greatest and > > not on a year old conglomorate of unknown provenance. :) > > Unfortunately it is not possible to test this patch with upstream. > There is no ARM64 bit support for Tegra yet. I am trying to > cleanup/upstream my ChromeOS patches and this clock patch in > particular makes one small step towards this goal. Also Thierry > mentioned that he works on full ARM64 Tegra support and it is really > exciting! Everything is exciting, but it does not change the fact, that this patch cannot work on current upstream. > So what I suppose to do with my patch? If it does not work could > anyone provide patch that removes ARM arch dependency from > tegra20_timer.c? Huch? You want other people to solve your problems? Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi On Fri, Nov 14, 2014 at 4:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > On Fri, 14 Nov 2014, Anatol Pomozov wrote: >> On Thu, Nov 13, 2014 at 2:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >> > On Mon, 10 Nov 2014, Anatol Pomozov wrote: >> >> On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding >> >> <thierry.reding@gmail.com> wrote: >> >> > On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: >> >> >> ARM timekeeping functionality allows to register persistent/boot clock dynamically. >> >> >> This code is arch-independent and can be useful on other plaforms as well. >> >> >> >> >> >> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. >> >> >> >> >> >> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit >> >> >> board, made sure high-resolution clock works. >> >> > >> >> > Using this on an upstream kernel doesn't work, though, because 64-bit >> >> > ARM doesn't implement struct delay_timer which the driver needs since >> >> > v3.17. >> >> > >> >> > But I suppose the delay timer infrastructure could be moved into the >> >> > core similar to the persistent and boot clock as this patch does. >> >> >> >> Thanks. It makes sense, I will send it in a separate patch, once this >> >> one will be reviewed. On our kernel I haven't seen this issue as we >> >> still use 3.14. >> > >> > That's why you should test/compile your stuff on latest greatest and >> > not on a year old conglomorate of unknown provenance. :) >> >> Unfortunately it is not possible to test this patch with upstream. >> There is no ARM64 bit support for Tegra yet. I am trying to >> cleanup/upstream my ChromeOS patches and this clock patch in >> particular makes one small step towards this goal. Also Thierry >> mentioned that he works on full ARM64 Tegra support and it is really >> exciting! > > Everything is exciting, but it does not change the fact, that this > patch cannot work on current upstream. Could you please be more specific what exactly does not work? Are you talking about delay timer? But my patch does not touch any delay timer code. I can compile tegra_timer for ARM. And this code is not usable on arm64 anyway because whole Tegra is not ported yet. Somebody should make additional changes to upstream tegra20_timer.c code. I might try to do it later when Tegra will be ported. >> So what I suppose to do with my patch? If it does not work could >> anyone provide patch that removes ARM arch dependency from >> tegra20_timer.c? > > Huch? You want other people to solve your problems? This is not the point. I provided patch that fixes the issue. Other people said that they have ideas how to do it different (and better) way. So I am asking to share these ideas represented as a patch. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/14/2014 03:03 PM, Anatol Pomozov wrote: > Hi > > On Thu, Nov 13, 2014 at 2:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >> On Mon, 10 Nov 2014, Anatol Pomozov wrote: >>> On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding >>> <thierry.reding@gmail.com> wrote: >>>> On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: >>>>> ARM timekeeping functionality allows to register persistent/boot clock dynamically. >>>>> This code is arch-independent and can be useful on other plaforms as well. >>>>> >>>>> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. >>>>> >>>>> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit >>>>> board, made sure high-resolution clock works. >>>> >>>> Using this on an upstream kernel doesn't work, though, because 64-bit >>>> ARM doesn't implement struct delay_timer which the driver needs since >>>> v3.17. >>>> >>>> But I suppose the delay timer infrastructure could be moved into the >>>> core similar to the persistent and boot clock as this patch does. >>> >>> Thanks. It makes sense, I will send it in a separate patch, once this >>> one will be reviewed. On our kernel I haven't seen this issue as we >>> still use 3.14. >> >> That's why you should test/compile your stuff on latest greatest and >> not on a year old conglomorate of unknown provenance. :) > > Unfortunately it is not possible to test this patch with upstream. > There is no ARM64 bit support for Tegra yet. I am trying to > cleanup/upstream my ChromeOS patches and this clock patch in > particular makes one small step towards this goal. Also Thierry > mentioned that he works on full ARM64 Tegra support and it is really > exciting! What we usually do is send patches in the order the kernel boot process needs them. First modify the kernel to know about 64-bit Tegra, add earlyprintk support, make sure the early boot process spits out something on the UART, then add whatever next item is missing (e.g. clock driver, timers, ...). That way, every patch we apply can actually be tested in the mainline kernel, since the code actually reaches that point in execution. If we were for example to send in a ton of driver patches for ARM64 right now, we couldn't test them. Quite possibly those patches wouldn't fully work, and we'd just have churn fixing them up later once the base CPU/SoC support was added. It's better to only upstream patches that can actually be exercised in order to avoid that churn. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 14 Nov 2014, Anatol Pomozov wrote: > On Fri, Nov 14, 2014 at 4:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > >> So what I suppose to do with my patch? If it does not work could > >> anyone provide patch that removes ARM arch dependency from > >> tegra20_timer.c? > > > > Huch? You want other people to solve your problems? > > This is not the point. I provided patch that fixes the issue. Other > people said that they have ideas how to do it different (and better) > way. So I am asking to share these ideas represented as a patch. That's not the way it works. You sent a patch to solve an problem which you are facing. Now the people who review the patch think that there is a better approach than moving code from arm/ to the timekeeping core code. So it's up to you to come up with a patch which solves the problem in the right way. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 15 Nov 2014, Thomas Gleixner wrote: > On Fri, 14 Nov 2014, Anatol Pomozov wrote: > > On Fri, Nov 14, 2014 at 4:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > >> So what I suppose to do with my patch? If it does not work could > > >> anyone provide patch that removes ARM arch dependency from > > >> tegra20_timer.c? > > > > > > Huch? You want other people to solve your problems? > > > > This is not the point. I provided patch that fixes the issue. Other > > people said that they have ideas how to do it different (and better) > > way. So I am asking to share these ideas represented as a patch. > > That's not the way it works. > > You sent a patch to solve an problem which you are facing. > > Now the people who review the patch think that there is a better > approach than moving code from arm/ to the timekeeping core code. > > So it's up to you to come up with a patch which solves the problem in > the right way. And just for the record this whole thing is just hilarious. ARM64 selects ARM_ARCH_TIMER which registers the architected timer as the primary clocksource. Now that timer has the following flag set: CLOCK_SOURCE_SUSPEND_NONSTOP And that flag causes the core timekeeping code to use the clocksource to figure out the time which the machine spent in suspend. So registering that tegra RTC as persistent clock does not have any value at all. Simply because it is completely irrelevant at boot time whether the RTC can be accessed right at timekeeping init or not. Nothing in early boot needs wall clock time. It's good enough to set wall clock time late in the boot process as the first use case is when the root file system gets mounted. And the RTC driver which obviously deals with the same hardware is initialized before that. So you are trying to move something which is outright pointless to the core code just because you carry that patch in your ChromeOS support hackery. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 13, 2014 at 03:21:22PM -0800, John Stultz wrote: > On Thu, Nov 13, 2014 at 2:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, 10 Nov 2014, Anatol Pomozov wrote: > >> On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding > >> <thierry.reding@gmail.com> wrote: > >> > On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: > >> >> ARM timekeeping functionality allows to register persistent/boot clock dynamically. > >> >> This code is arch-independent and can be useful on other plaforms as well. > >> >> > >> >> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. > >> >> > >> >> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit > >> >> board, made sure high-resolution clock works. > >> > > >> > Using this on an upstream kernel doesn't work, though, because 64-bit > >> > ARM doesn't implement struct delay_timer which the driver needs since > >> > v3.17. > >> > > >> > But I suppose the delay timer infrastructure could be moved into the > >> > core similar to the persistent and boot clock as this patch does. > >> > >> Thanks. It makes sense, I will send it in a separate patch, once this > >> one will be reviewed. On our kernel I haven't seen this issue as we > >> still use 3.14. > > > > That's why you should test/compile your stuff on latest greatest and > > not on a year old conglomorate of unknown provenance. :) > > > > Aside of that I really wonder why we need that persistent_clock stuff > > at all. We already have mechanisms to register persistent clocks AKA > > RTCs after the early boot process and update the wall clock time > > before we actually need it. Nothing in early boot depends on correct > > wall clock at all. > > > > So instead of adding more extra persistent clock nonsense, can we just > > move all of that to the place where it belongs, i.e. RTC? > > Sigh.. I've got this on an eventual todo list.. The big problem though > is that the RTC infrastructure can't be called with irqs off, so its > not as optimal for measuring suspend time. Is that because many RTC devices are accessed over something like I2C or SPI where interrupts are needed? Or are there additional reasons? > Some of the suspend-time measurement with clocksources that don't halt > is interesting here. > > So we need to add to the RTC infrastructure special accessors that are > safe when irqs are off, and we can then deprecate the persistent clock > bits. There's still evaluation quirks with setting the time earlier in > boot or not (possibly some rng effects as well there), but that could > be worked out if we had the suspend timing via safe RTC interfaces > sorted. If it's only about slow busses, perhaps we could copy what other subsystems have been doing and add a ->can_sleep flag to RTC devices to mark those that can't be accessed with IRQs off. Having extra accessors seems to me like it won't work well. As I understand it we have two types of RTC devices: those that use slow busses and hence can't be accessed with interrupts off, and those that don't use a slow bus and therefore can be used with interrupts disabled. For the former I don't think it's possible to implement accessors that are safe when IRQs are disabled and for the latter the accessors don't need to be special. So I think a simple flag should be enough. I've been thinking a little about how the implementation could look in practice. Would we simply add code to the weak implementation of the read_persistent_clock() function (kernel/time/timekeeping.c) which looks for an RTC device usable as persistent clock? So something like this: void __weak read_persistent_clock(struct timespec *ts) { struct rtc_device *rtc; rtc = rtc_class_open_persistent(); if (rtc) { struct rtc_time tm; int err; err = rtc_read_time(rtc, &tm); rtc_class_close(rtc); if (!err) { rtc_tm_to_timespec(&tm, ts); return; } } ts->tv_sec = 0; tv->tv_nsec = 0; } Where rtc_class_open_persistent() could be like rtc_class_open(), except that it uses a match function like this: static int __rtc_match_persistent(struct device *dev, const void *data) { struct rtc_device *rtc = to_rtc_device(dev); return !rtc->can_sleep; } If you still prefer to do this with accessors I suspect something very similar could be done. Adding Paul, who's been looking into this as well, to Cc. Thierry
On Sat, Nov 15, 2014 at 02:38:00AM +0100, Thomas Gleixner wrote: > On Sat, 15 Nov 2014, Thomas Gleixner wrote: > > On Fri, 14 Nov 2014, Anatol Pomozov wrote: > > > On Fri, Nov 14, 2014 at 4:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > > >> So what I suppose to do with my patch? If it does not work could > > > >> anyone provide patch that removes ARM arch dependency from > > > >> tegra20_timer.c? > > > > > > > > Huch? You want other people to solve your problems? > > > > > > This is not the point. I provided patch that fixes the issue. Other > > > people said that they have ideas how to do it different (and better) > > > way. So I am asking to share these ideas represented as a patch. > > > > That's not the way it works. > > > > You sent a patch to solve an problem which you are facing. > > > > Now the people who review the patch think that there is a better > > approach than moving code from arm/ to the timekeeping core code. > > > > So it's up to you to come up with a patch which solves the problem in > > the right way. > > And just for the record this whole thing is just hilarious. > > ARM64 selects ARM_ARCH_TIMER which registers the architected timer as > the primary clocksource. > > Now that timer has the following flag set: > > CLOCK_SOURCE_SUSPEND_NONSTOP > > And that flag causes the core timekeeping code to use the clocksource > to figure out the time which the machine spent in suspend. As I understand it the architected timer will be turned off along with the rest of the CPU complex on Tegra. I'm not sure if that's specific to Tegra or something that other SoCs may do as well. Cc'ing Paul who's more familiar with the details. Thierry
On 11/15/2014 01:51 AM, Anatol Pomozov wrote: > Hi > > On Fri, Nov 14, 2014 at 4:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >> On Fri, 14 Nov 2014, Anatol Pomozov wrote: >>> On Thu, Nov 13, 2014 at 2:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >>>> On Mon, 10 Nov 2014, Anatol Pomozov wrote: >>>>> On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding >>>>> <thierry.reding@gmail.com> wrote: >>>>>> On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: >>>>>>> ARM timekeeping functionality allows to register persistent/boot clock dynamically. >>>>>>> This code is arch-independent and can be useful on other plaforms as well. >>>>>>> >>>>>>> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. >>>>>>> >>>>>>> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit >>>>>>> board, made sure high-resolution clock works. >>>>>> >>>>>> Using this on an upstream kernel doesn't work, though, because 64-bit >>>>>> ARM doesn't implement struct delay_timer which the driver needs since >>>>>> v3.17. >>>>>> >>>>>> But I suppose the delay timer infrastructure could be moved into the >>>>>> core similar to the persistent and boot clock as this patch does. >>>>> >>>>> Thanks. It makes sense, I will send it in a separate patch, once this >>>>> one will be reviewed. On our kernel I haven't seen this issue as we >>>>> still use 3.14. >>>> >>>> That's why you should test/compile your stuff on latest greatest and >>>> not on a year old conglomorate of unknown provenance. :) >>> >>> Unfortunately it is not possible to test this patch with upstream. >>> There is no ARM64 bit support for Tegra yet. I am trying to >>> cleanup/upstream my ChromeOS patches and this clock patch in >>> particular makes one small step towards this goal. Also Thierry >>> mentioned that he works on full ARM64 Tegra support and it is really >>> exciting! >> >> Everything is exciting, but it does not change the fact, that this >> patch cannot work on current upstream. > > Could you please be more specific what exactly does not work? Are you > talking about delay timer? But my patch does not touch any delay timer > code. I can compile tegra_timer for ARM. And this code is not usable > on arm64 anyway because whole Tegra is not ported yet. Somebody should > make additional changes to upstream tegra20_timer.c code. I might try > to do it later when Tegra will be ported. > >>> So what I suppose to do with my patch? If it does not work could >>> anyone provide patch that removes ARM arch dependency from >>> tegra20_timer.c? >> >> Huch? You want other people to solve your problems? > > This is not the point. I provided patch that fixes the issue. Other > people said that they have ideas how to do it different (and better) > way. So I am asking to share these ideas represented as a patch. I think your issue is solved [1] ... for the moment. Thanks -- Daniel [1] https://lkml.org/lkml/2015/1/9/264
On Fri, Jan 09, 2015 at 09:49:14AM +0000, Thierry Reding wrote: > On Sat, Nov 15, 2014 at 02:38:00AM +0100, Thomas Gleixner wrote: > > On Sat, 15 Nov 2014, Thomas Gleixner wrote: > > > On Fri, 14 Nov 2014, Anatol Pomozov wrote: > > > > On Fri, Nov 14, 2014 at 4:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > > > >> So what I suppose to do with my patch? If it does not work could > > > > >> anyone provide patch that removes ARM arch dependency from > > > > >> tegra20_timer.c? > > > > > > > > > > Huch? You want other people to solve your problems? > > > > > > > > This is not the point. I provided patch that fixes the issue. Other > > > > people said that they have ideas how to do it different (and better) > > > > way. So I am asking to share these ideas represented as a patch. > > > > > > That's not the way it works. > > > > > > You sent a patch to solve an problem which you are facing. > > > > > > Now the people who review the patch think that there is a better > > > approach than moving code from arm/ to the timekeeping core code. > > > > > > So it's up to you to come up with a patch which solves the problem in > > > the right way. > > > > And just for the record this whole thing is just hilarious. > > > > ARM64 selects ARM_ARCH_TIMER which registers the architected timer as > > the primary clocksource. > > > > Now that timer has the following flag set: > > > > CLOCK_SOURCE_SUSPEND_NONSTOP > > > > And that flag causes the core timekeeping code to use the clocksource > > to figure out the time which the machine spent in suspend. > > As I understand it the architected timer will be turned off along with > the rest of the CPU complex on Tegra. I'm not sure if that's specific to > Tegra or something that other SoCs may do as well. That doesn't sound right to me: the architecture specifies that the system counter must be implemented in an always-on power domain. Note that that only applies to the counter, not the timers (as the comparators can be turned off with the CPUs). Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 09, 2015 at 01:59:07PM +0000, Mark Rutland wrote: > On Fri, Jan 09, 2015 at 09:49:14AM +0000, Thierry Reding wrote: > > On Sat, Nov 15, 2014 at 02:38:00AM +0100, Thomas Gleixner wrote: > > > On Sat, 15 Nov 2014, Thomas Gleixner wrote: > > > > On Fri, 14 Nov 2014, Anatol Pomozov wrote: > > > > > On Fri, Nov 14, 2014 at 4:18 PM, Thomas Gleixner <tglx@linutronix.de> wrote: > > > > > >> So what I suppose to do with my patch? If it does not work could > > > > > >> anyone provide patch that removes ARM arch dependency from > > > > > >> tegra20_timer.c? > > > > > > > > > > > > Huch? You want other people to solve your problems? > > > > > > > > > > This is not the point. I provided patch that fixes the issue. Other > > > > > people said that they have ideas how to do it different (and better) > > > > > way. So I am asking to share these ideas represented as a patch. > > > > > > > > That's not the way it works. > > > > > > > > You sent a patch to solve an problem which you are facing. > > > > > > > > Now the people who review the patch think that there is a better > > > > approach than moving code from arm/ to the timekeeping core code. > > > > > > > > So it's up to you to come up with a patch which solves the problem in > > > > the right way. > > > > > > And just for the record this whole thing is just hilarious. > > > > > > ARM64 selects ARM_ARCH_TIMER which registers the architected timer as > > > the primary clocksource. > > > > > > Now that timer has the following flag set: > > > > > > CLOCK_SOURCE_SUSPEND_NONSTOP > > > > > > And that flag causes the core timekeeping code to use the clocksource > > > to figure out the time which the machine spent in suspend. > > > > As I understand it the architected timer will be turned off along with > > the rest of the CPU complex on Tegra. I'm not sure if that's specific to > > Tegra or something that other SoCs may do as well. > > That doesn't sound right to me: the architecture specifies that the > system counter must be implemented in an always-on power domain. > > Note that that only applies to the counter, not the timers (as the > comparators can be turned off with the CPUs). I'll let Paul comment on this, since I don't know the intimate details. Of course if the system counter is indeed active across suspend/resume then it doesn't seem like we'd need the Tegra timer at all on 64-bit. Unless of course if we use it for something else. Thierry
On Fri, Jan 9, 2015 at 1:43 AM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Thu, Nov 13, 2014 at 03:21:22PM -0800, John Stultz wrote: >> On Thu, Nov 13, 2014 at 2:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote: >> > On Mon, 10 Nov 2014, Anatol Pomozov wrote: >> >> On Mon, Nov 10, 2014 at 1:53 AM, Thierry Reding >> >> <thierry.reding@gmail.com> wrote: >> >> > On Fri, Nov 07, 2014 at 11:34:15AM -0800, Anatol Pomozov wrote: >> >> >> ARM timekeeping functionality allows to register persistent/boot clock dynamically. >> >> >> This code is arch-independent and can be useful on other plaforms as well. >> >> >> >> >> >> As a byproduct of this change, tegra20_timer becomes ARM64 compatible. >> >> >> >> >> >> Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit >> >> >> board, made sure high-resolution clock works. >> >> > >> >> > Using this on an upstream kernel doesn't work, though, because 64-bit >> >> > ARM doesn't implement struct delay_timer which the driver needs since >> >> > v3.17. >> >> > >> >> > But I suppose the delay timer infrastructure could be moved into the >> >> > core similar to the persistent and boot clock as this patch does. >> >> >> >> Thanks. It makes sense, I will send it in a separate patch, once this >> >> one will be reviewed. On our kernel I haven't seen this issue as we >> >> still use 3.14. >> > >> > That's why you should test/compile your stuff on latest greatest and >> > not on a year old conglomorate of unknown provenance. :) >> > >> > Aside of that I really wonder why we need that persistent_clock stuff >> > at all. We already have mechanisms to register persistent clocks AKA >> > RTCs after the early boot process and update the wall clock time >> > before we actually need it. Nothing in early boot depends on correct >> > wall clock at all. >> > >> > So instead of adding more extra persistent clock nonsense, can we just >> > move all of that to the place where it belongs, i.e. RTC? >> >> Sigh.. I've got this on an eventual todo list.. The big problem though >> is that the RTC infrastructure can't be called with irqs off, so its >> not as optimal for measuring suspend time. > > Is that because many RTC devices are accessed over something like I2C or > SPI where interrupts are needed? Or are there additional reasons? Right. The persistent_clock logic is called when irqs may be off, and many RTC devices require interrupts. >> Some of the suspend-time measurement with clocksources that don't halt >> is interesting here. >> >> So we need to add to the RTC infrastructure special accessors that are >> safe when irqs are off, and we can then deprecate the persistent clock >> bits. There's still evaluation quirks with setting the time earlier in >> boot or not (possibly some rng effects as well there), but that could >> be worked out if we had the suspend timing via safe RTC interfaces >> sorted. > > If it's only about slow busses, perhaps we could copy what other > subsystems have been doing and add a ->can_sleep flag to RTC devices to > mark those that can't be accessed with IRQs off. > > Having extra accessors seems to me like it won't work well. As I > understand it we have two types of RTC devices: those that use slow > busses and hence can't be accessed with interrupts off, and those that > don't use a slow bus and therefore can be used with interrupts disabled. > For the former I don't think it's possible to implement accessors that > are safe when IRQs are disabled and for the latter the accessors don't > need to be special. So I think a simple flag should be enough. Yea, it doesn't necessarily need to be done via a new accessor, but I assumed there would be some corner case hardware and the function pointer would allow some extra flexibility. > I've been thinking a little about how the implementation could look in > practice. Would we simply add code to the weak implementation of the > read_persistent_clock() function (kernel/time/timekeeping.c) which looks > for an RTC device usable as persistent clock? > > So something like this: > > void __weak read_persistent_clock(struct timespec *ts) > { > struct rtc_device *rtc; > > rtc = rtc_class_open_persistent(); > if (rtc) { > struct rtc_time tm; > int err; > > err = rtc_read_time(rtc, &tm); > rtc_class_close(rtc); > > if (!err) { > rtc_tm_to_timespec(&tm, ts); > return; > } > } > > ts->tv_sec = 0; > tv->tv_nsec = 0; > } > > Where rtc_class_open_persistent() could be like rtc_class_open(), except > that it uses a match function like this: > > static int __rtc_match_persistent(struct device *dev, const void *data) > { > struct rtc_device *rtc = to_rtc_device(dev); > > return !rtc->can_sleep; > } > > If you still prefer to do this with accessors I suspect something very > similar could be done. Something like this could work, but I'd prefer we move it into the rtc framework and use that directly, rather then stuffing the logic into the persistent_clock code (so we can eventually get rid of the persistent clock logic). thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi On Fri, 9 Jan 2015, Thierry Reding wrote: > On Fri, Jan 09, 2015 at 01:59:07PM +0000, Mark Rutland wrote: > > On Fri, Jan 09, 2015 at 09:49:14AM +0000, Thierry Reding wrote: > > > > > As I understand it the architected timer will be turned off along with > > > the rest of the CPU complex on Tegra. I'm not sure if that's specific to > > > Tegra or something that other SoCs may do as well. > > > > That doesn't sound right to me: the architecture specifies that the > > system counter must be implemented in an always-on power domain. > > > > Note that that only applies to the counter, not the timers (as the > > comparators can be turned off with the CPUs). > > I'll let Paul comment on this, since I don't know the intimate details. I believe Mark's comments are accurate. In Linux terms, when deep CPU power management states are in use, the ARM architected "timer" is useful as a clocksource, but useless as a wakeup source (a clockevent device). Regarding clocksources, my personal view is that the SoC timer IP blocks should be exposed as a clocksource option if the timer IP blocks are present in the hardware. That written, I would expect that most production implementations will use the ARM architected "timer" as the clocksource. - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/include/asm/mach/time.h b/arch/arm/include/asm/mach/time.h index 90c12e1..3cbcafc 100644 --- a/arch/arm/include/asm/mach/time.h +++ b/arch/arm/include/asm/mach/time.h @@ -12,9 +12,4 @@ extern void timer_tick(void); -struct timespec; -typedef void (*clock_access_fn)(struct timespec *); -extern int register_persistent_clock(clock_access_fn read_boot, - clock_access_fn read_persistent); - #endif diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c index 0cc7e58..0aa1dcd 100644 --- a/arch/arm/kernel/time.c +++ b/arch/arm/kernel/time.c @@ -76,42 +76,6 @@ void timer_tick(void) } #endif -static void dummy_clock_access(struct timespec *ts) -{ - ts->tv_sec = 0; - ts->tv_nsec = 0; -} - -static clock_access_fn __read_persistent_clock = dummy_clock_access; -static clock_access_fn __read_boot_clock = dummy_clock_access;; - -void read_persistent_clock(struct timespec *ts) -{ - __read_persistent_clock(ts); -} - -void read_boot_clock(struct timespec *ts) -{ - __read_boot_clock(ts); -} - -int __init register_persistent_clock(clock_access_fn read_boot, - clock_access_fn read_persistent) -{ - /* Only allow the clockaccess functions to be registered once */ - if (__read_persistent_clock == dummy_clock_access && - __read_boot_clock == dummy_clock_access) { - if (read_boot) - __read_boot_clock = read_boot; - if (read_persistent) - __read_persistent_clock = read_persistent; - - return 0; - } - - return -EINVAL; -} - void __init time_init(void) { if (machine_desc->init_time) { diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c index 61b4d70..0dbfffd 100644 --- a/arch/arm/plat-omap/counter_32k.c +++ b/arch/arm/plat-omap/counter_32k.c @@ -18,10 +18,9 @@ #include <linux/err.h> #include <linux/io.h> #include <linux/clocksource.h> +#include <linux/time.h> #include <linux/sched_clock.h> -#include <asm/mach/time.h> - #include <plat/counter-32k.h> /* OMAP2_32KSYNCNT_CR_OFF: offset of 32ksync counter register */ @@ -76,6 +75,10 @@ static void omap_read_persistent_clock(struct timespec *ts) spin_unlock_irqrestore(&read_persistent_clock_lock, flags); } +static const struct persistent_clock_ops omap_persistent_clock = { + .read = omap_read_persistent_clock +}; + /** * omap_init_clocksource_32k - setup and register counter 32k as a * kernel clocksource @@ -116,7 +119,7 @@ int __init omap_init_clocksource_32k(void __iomem *vbase) } sched_clock_register(omap_32k_read_sched_clock, 32, 32768); - register_persistent_clock(NULL, omap_read_persistent_clock); + register_persistent_clock(&omap_persistent_clock); pr_info("OMAP clocksource: 32k_counter at 32768 Hz\n"); return 0; diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c index d2616ef..210130e 100644 --- a/drivers/clocksource/tegra20_timer.c +++ b/drivers/clocksource/tegra20_timer.c @@ -28,9 +28,7 @@ #include <linux/of_irq.h> #include <linux/sched_clock.h> #include <linux/delay.h> - -#include <asm/mach/time.h> -#include <asm/smp_twd.h> +#include <linux/timekeeping.h> #define RTC_SECONDS 0x08 #define RTC_SHADOW_SECONDS 0x0c @@ -142,6 +140,10 @@ static void tegra_read_persistent_clock(struct timespec *ts) *ts = *tsp; } +static const struct persistent_clock_ops tegra_persistent_clock = { + .read = tegra_read_persistent_clock +}; + static unsigned long tegra_delay_timer_read_counter_long(void) { return readl(timer_reg_base + TIMERUS_CNTR_1US); @@ -252,7 +254,7 @@ static void __init tegra20_init_rtc(struct device_node *np) else clk_prepare_enable(clk); - register_persistent_clock(NULL, tegra_read_persistent_clock); + register_persistent_clock(&tegra_persistent_clock); } CLOCKSOURCE_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc); diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h index 1caa6b0..02023f7 100644 --- a/include/linux/timekeeping.h +++ b/include/linux/timekeeping.h @@ -201,6 +201,17 @@ static inline bool has_persistent_clock(void) return persistent_clock_exist; } +struct persistent_clock_ops { + void (*read)(struct timespec *ts); + int (*update)(const struct timespec ts); +}; + +struct boot_clock_ops { + void (*read)(struct timespec *ts); +}; + +extern void register_persistent_clock(const struct persistent_clock_ops *clock); +extern void register_boot_clock(const struct boot_clock_ops *clock); extern void read_persistent_clock(struct timespec *ts); extern void read_boot_clock(struct timespec *ts); extern int update_persistent_clock(struct timespec now); diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index fb4a9c2..414c172 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -66,6 +66,9 @@ int __read_mostly timekeeping_suspended; /* Flag for if there is a persistent clock on this platform */ bool __read_mostly persistent_clock_exist = false; +const struct persistent_clock_ops __read_mostly *persistent_clock = NULL; +const struct boot_clock_ops __read_mostly *boot_clock = NULL; + static inline void tk_normalize_xtime(struct timekeeper *tk) { while (tk->tkr.xtime_nsec >= ((u64)NSEC_PER_SEC << tk->tkr.shift)) { @@ -956,6 +959,30 @@ u64 timekeeping_max_deferment(void) return ret; } +extern void register_persistent_clock(const struct persistent_clock_ops *clock) +{ + BUG_ON(!clock); + BUG_ON(!clock->read); + + if (persistent_clock) { + pr_warn("Ignore extra persistent clock registration"); + return; + } + persistent_clock = clock; +} + +extern void register_boot_clock(const struct boot_clock_ops *clock) +{ + BUG_ON(!clock); + BUG_ON(!clock->read); + + if (boot_clock) { + pr_warn("Ignore extra boot clock registration"); + return; + } + boot_clock = clock; +} + /** * read_persistent_clock - Return time from the persistent clock. * @@ -967,8 +994,12 @@ u64 timekeeping_max_deferment(void) */ void __weak read_persistent_clock(struct timespec *ts) { - ts->tv_sec = 0; - ts->tv_nsec = 0; + if (persistent_clock) { + persistent_clock->read(ts); + } else { + ts->tv_sec = 0; + ts->tv_nsec = 0; + } } /** @@ -982,8 +1013,12 @@ void __weak read_persistent_clock(struct timespec *ts) */ void __weak read_boot_clock(struct timespec *ts) { - ts->tv_sec = 0; - ts->tv_nsec = 0; + if (boot_clock) { + boot_clock->read(ts); + } else { + ts->tv_sec = 0; + ts->tv_nsec = 0; + } } /*
ARM timekeeping functionality allows to register persistent/boot clock dynamically. This code is arch-independent and can be useful on other plaforms as well. As a byproduct of this change, tegra20_timer becomes ARM64 compatible. Tested: backported the change to chromeos-3.14 kernel ran on tegra 64bit board, made sure high-resolution clock works. Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com> --- arch/arm/include/asm/mach/time.h | 5 ----- arch/arm/kernel/time.c | 36 ------------------------------- arch/arm/plat-omap/counter_32k.c | 9 +++++--- drivers/clocksource/tegra20_timer.c | 10 +++++---- include/linux/timekeeping.h | 11 ++++++++++ kernel/time/timekeeping.c | 43 +++++++++++++++++++++++++++++++++---- 6 files changed, 62 insertions(+), 52 deletions(-)