2012-11-12 20:51:15

by Stephen Warren

[permalink] [raw]
Subject: [PATCH V2 2/11] time: convert arch_gettimeoffset to a pointer

From: Stephen Warren <[email protected]>

Currently, whenever CONFIG_ARCH_USES_GETTIMEOFFSET is enabled, each
arch core provides a single implementation of arch_gettimeoffset(). In
many cases, different sub-architectures, different machines, or
different timer providers exist, and so the arch ends up implementing
arch_gettimeoffset() as a call-through-pointer anyway. Examples are
ARM, Cris, M68K, and it's arguable that the remaining architectures,
M32R and Blackfin, should be doing this anyway.

Modify arch_gettimeoffset so that it itself is a function pointer, which
the arch initializes. This will allow later changes to move the
initialization of this function into individual machine support or timer
drivers. This is particularly useful for code in drivers/clocksource
which should rely on an arch-independant mechanism to register their
implementation of arch_gettimeoffset().

This patch also converts the Cris architecture to set arch_gettimeoffset
directly to the final implementation in time_init(), because Cris already
had separate time_init() functions per sub-architecture. M68K and ARM
are converted to set arch_gettimeoffset the final implementation in later
patches, because they already have function pointers in place for this
purpose.

Cc: Russell King <[email protected]>
Cc: Mike Frysinger <[email protected]>
Cc: Mikael Starvik <[email protected]>
Cc: Hirokazu Takata <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Acked-by: Geert Uytterhoeven <[email protected]>
Acked-by: Jesper Nilsson <[email protected]>
Signed-off-by: Stephen Warren <[email protected]>
---
v2:
* s/gettimeoffset/get_arch_timeoffset/ to make the name less generic
and more arch-specific.

(Note: I've only reposted patch 2/11 this time around, and I still hope
to take it through the arm-soc tree due to dependencies, so I'm not
requesting this be applied by the timekeeping maintainers)
---
arch/arm/kernel/time.c | 6 +++++-
arch/blackfin/kernel/time.c | 6 +++++-
arch/cris/arch-v10/kernel/time.c | 6 ++++--
arch/cris/kernel/time.c | 11 -----------
arch/m32r/kernel/time.c | 4 +++-
arch/m68k/kernel/time.c | 16 ++++++++++------
include/linux/time.h | 4 +---
kernel/time/timekeeping.c | 26 ++++++++++++++++++++------
8 files changed, 48 insertions(+), 31 deletions(-)

diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index 09be0c3..b0190b4 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -70,7 +70,7 @@ EXPORT_SYMBOL(profile_pc);
#endif

#ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
-u32 arch_gettimeoffset(void)
+static u32 arm_gettimeoffset(void)
{
if (system_timer->offset != NULL)
return system_timer->offset() * 1000;
@@ -164,6 +164,10 @@ device_initcall(timer_init_syscore_ops);

void __init time_init(void)
{
+#ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
+ arch_gettimeoffset = arm_gettimeoffset;
+#endif
+
system_timer = machine_desc->timer;
system_timer->init();
sched_clock_postinit();
diff --git a/arch/blackfin/kernel/time.c b/arch/blackfin/kernel/time.c
index 2310b24..3126b92 100644
--- a/arch/blackfin/kernel/time.c
+++ b/arch/blackfin/kernel/time.c
@@ -85,7 +85,7 @@ time_sched_init(irqreturn_t(*timer_routine) (int, void *))
/*
* Should return useconds since last timer tick
*/
-u32 arch_gettimeoffset(void)
+static u32 blackfin_gettimeoffset(void)
{
unsigned long offset;
unsigned long clocks_per_jiffy;
@@ -141,6 +141,10 @@ void read_persistent_clock(struct timespec *ts)

void __init time_init(void)
{
+#ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
+ arch_gettimeoffset = blackfin_gettimeoffset;
+#endif
+
#ifdef CONFIG_RTC_DRV_BFIN
/* [#2663] hack to filter junk RTC values that would cause
* userspace to have to deal with time values greater than
diff --git a/arch/cris/arch-v10/kernel/time.c b/arch/cris/arch-v10/kernel/time.c
index 162892f..fce7c54 100644
--- a/arch/cris/arch-v10/kernel/time.c
+++ b/arch/cris/arch-v10/kernel/time.c
@@ -55,9 +55,9 @@ unsigned long get_ns_in_jiffie(void)
return ns;
}

-unsigned long do_slow_gettimeoffset(void)
+static u32 cris_v10_gettimeoffset(void)
{
- unsigned long count;
+ u32 count;

/* The timer interrupt comes from Etrax timer 0. In order to get
* better precision, we check the current value. It might have
@@ -191,6 +191,8 @@ static struct irqaction irq2 = {
void __init
time_init(void)
{
+ arch_gettimeoffset = cris_v10_gettimeoffset;
+
/* probe for the RTC and read it if it exists
* Before the RTC can be probed the loops_per_usec variable needs
* to be initialized to make usleep work. A better value for
diff --git a/arch/cris/kernel/time.c b/arch/cris/kernel/time.c
index b063c92..fe6acda 100644
--- a/arch/cris/kernel/time.c
+++ b/arch/cris/kernel/time.c
@@ -39,17 +39,6 @@
extern unsigned long loops_per_jiffy; /* init/main.c */
unsigned long loops_per_usec;

-
-#ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
-extern unsigned long do_slow_gettimeoffset(void);
-static unsigned long (*do_gettimeoffset)(void) = do_slow_gettimeoffset;
-
-u32 arch_gettimeoffset(void)
-{
- return do_gettimeoffset();
-}
-#endif
-
int set_rtc_mmss(unsigned long nowtime)
{
D(printk(KERN_DEBUG "set_rtc_mmss(%lu)\n", nowtime));
diff --git a/arch/m32r/kernel/time.c b/arch/m32r/kernel/time.c
index 84dd040..1a15f81 100644
--- a/arch/m32r/kernel/time.c
+++ b/arch/m32r/kernel/time.c
@@ -57,7 +57,7 @@ extern void smp_local_timer_interrupt(void);

static unsigned long latch;

-u32 arch_gettimeoffset(void)
+static u32 m32r_gettimeoffset(void)
{
unsigned long elapsed_time = 0; /* [us] */

@@ -165,6 +165,8 @@ void read_persistent_clock(struct timespec *ts)

void __init time_init(void)
{
+ arch_gettimeoffset = m32r_gettimeoffset;
+
#if defined(CONFIG_CHIP_M32102) || defined(CONFIG_CHIP_XNUX2) \
|| defined(CONFIG_CHIP_VDEC2) || defined(CONFIG_CHIP_M32700) \
|| defined(CONFIG_CHIP_OPSP) || defined(CONFIG_CHIP_M32104)
diff --git a/arch/m68k/kernel/time.c b/arch/m68k/kernel/time.c
index 5d0bcaa..c2994c8 100644
--- a/arch/m68k/kernel/time.c
+++ b/arch/m68k/kernel/time.c
@@ -80,14 +80,9 @@ void read_persistent_clock(struct timespec *ts)
}
}

-void __init time_init(void)
-{
- mach_sched_init(timer_interrupt);
-}
-
#ifdef CONFIG_ARCH_USES_GETTIMEOFFSET

-u32 arch_gettimeoffset(void)
+static u32 m68k_gettimeoffset(void)
{
return mach_gettimeoffset() * 1000;
}
@@ -106,3 +101,12 @@ static int __init rtc_init(void)
module_init(rtc_init);

#endif /* CONFIG_ARCH_USES_GETTIMEOFFSET */
+
+void __init time_init(void)
+{
+#ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
+ arch_gettimeoffset = m68k_gettimeoffset;
+#endif
+
+ mach_sched_init(timer_interrupt);
+}
diff --git a/include/linux/time.h b/include/linux/time.h
index 4d358e9..05e32a7 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -142,9 +142,7 @@ void timekeeping_inject_sleeptime(struct timespec *delta);
* finer then tick granular time.
*/
#ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
-extern u32 arch_gettimeoffset(void);
-#else
-static inline u32 arch_gettimeoffset(void) { return 0; }
+extern u32 (*arch_gettimeoffset)(void);
#endif

extern void do_gettimeofday(struct timeval *tv);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index e424970..00b58c7 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -140,6 +140,20 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
}

/* Timekeeper helper functions. */
+
+#ifdef CONFIG_ARCH_USES_GETTIMEOFFSET
+u32 (*arch_gettimeoffset)(void);
+
+u32 get_arch_timeoffset(void)
+{
+ if (likely(arch_gettimeoffset))
+ return arch_gettimeoffset();
+ return 0;
+}
+#else
+static inline u32 get_arch_timeoffset(void) { return 0; }
+#endif
+
static inline s64 timekeeping_get_ns(struct timekeeper *tk)
{
cycle_t cycle_now, cycle_delta;
@@ -156,8 +170,8 @@ static inline s64 timekeeping_get_ns(struct timekeeper *tk)
nsec = cycle_delta * tk->mult + tk->xtime_nsec;
nsec >>= tk->shift;

- /* If arch requires, add in gettimeoffset() */
- return nsec + arch_gettimeoffset();
+ /* If arch requires, add in get_arch_timeoffset() */
+ return nsec + get_arch_timeoffset();
}

static inline s64 timekeeping_get_ns_raw(struct timekeeper *tk)
@@ -176,8 +190,8 @@ static inline s64 timekeeping_get_ns_raw(struct timekeeper *tk)
/* convert delta to nanoseconds. */
nsec = clocksource_cyc2ns(cycle_delta, clock->mult, clock->shift);

- /* If arch requires, add in gettimeoffset() */
- return nsec + arch_gettimeoffset();
+ /* If arch requires, add in get_arch_timeoffset() */
+ return nsec + get_arch_timeoffset();
}

/* must hold write on timekeeper.lock */
@@ -210,8 +224,8 @@ static void timekeeping_forward_now(struct timekeeper *tk)

tk->xtime_nsec += cycle_delta * tk->mult;

- /* If arch requires, add in gettimeoffset() */
- tk->xtime_nsec += (u64)arch_gettimeoffset() << tk->shift;
+ /* If arch requires, add in get_arch_timeoffset() */
+ tk->xtime_nsec += (u64)get_arch_timeoffset() << tk->shift;

tk_normalize_xtime(tk);

--
1.7.0.4


2012-11-12 21:36:39

by john stultz

[permalink] [raw]
Subject: Re: [PATCH V2 2/11] time: convert arch_gettimeoffset to a pointer

On 11/12/2012 12:51 PM, Stephen Warren wrote:
> From: Stephen Warren <[email protected]>
>
> Currently, whenever CONFIG_ARCH_USES_GETTIMEOFFSET is enabled, each
> arch core provides a single implementation of arch_gettimeoffset(). In
> many cases, different sub-architectures, different machines, or
> different timer providers exist, and so the arch ends up implementing
> arch_gettimeoffset() as a call-through-pointer anyway. Examples are
> ARM, Cris, M68K, and it's arguable that the remaining architectures,
> M32R and Blackfin, should be doing this anyway.
>
> Modify arch_gettimeoffset so that it itself is a function pointer, which
> the arch initializes. This will allow later changes to move the
> initialization of this function into individual machine support or timer
> drivers. This is particularly useful for code in drivers/clocksource
> which should rely on an arch-independant mechanism to register their
> implementation of arch_gettimeoffset().
>
> This patch also converts the Cris architecture to set arch_gettimeoffset
> directly to the final implementation in time_init(), because Cris already
> had separate time_init() functions per sub-architecture. M68K and ARM
> are converted to set arch_gettimeoffset the final implementation in later
> patches, because they already have function pointers in place for this
> purpose.
>
> Cc: Russell King <[email protected]>
> Cc: Mike Frysinger <[email protected]>
> Cc: Mikael Starvik <[email protected]>
> Cc: Hirokazu Takata <[email protected]>
> Cc: John Stultz <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Acked-by: Geert Uytterhoeven <[email protected]>
> Acked-by: Jesper Nilsson <[email protected]>
> Signed-off-by: Stephen Warren <[email protected]>
> ---
> v2:
> * s/gettimeoffset/get_arch_timeoffset/ to make the name less generic
> and more arch-specific.
>
> (Note: I've only reposted patch 2/11 this time around, and I still hope
> to take it through the arm-soc tree due to dependencies, so I'm not
> requesting this be applied by the timekeeping maintainers)

One last thing to watch out for: If you're trying to build a kernel that
mixes clocksource support with get_arch_timeoffset, you'll need to
rework the #ifdef in update_wall_time(), since we currently assume with
get_arch_timeoffset() that you're using tick + interpolation, so every
call to update_wall_time() only moves time forward by one jiffy.

Otherwise, thanks for the name tweak. Going through the arm-soc tree is
fine with me.

Acked-by: John Stultz <[email protected]>

thanks
-john

2012-11-13 17:14:40

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH V2 2/11] time: convert arch_gettimeoffset to a pointer

On 11/12/2012 02:36 PM, John Stultz wrote:
> On 11/12/2012 12:51 PM, Stephen Warren wrote:
>> Currently, whenever CONFIG_ARCH_USES_GETTIMEOFFSET is enabled, each
>> arch core provides a single implementation of arch_gettimeoffset(). In
>> many cases, different sub-architectures, different machines, or
>> different timer providers exist, and so the arch ends up implementing
>> arch_gettimeoffset() as a call-through-pointer anyway. Examples are
>> ARM, Cris, M68K, and it's arguable that the remaining architectures,
>> M32R and Blackfin, should be doing this anyway.
>>
>> Modify arch_gettimeoffset so that it itself is a function pointer, which
>> the arch initializes. This will allow later changes to move the
>> initialization of this function into individual machine support or timer
>> drivers. This is particularly useful for code in drivers/clocksource
>> which should rely on an arch-independant mechanism to register their
>> implementation of arch_gettimeoffset().
...
> One last thing to watch out for: If you're trying to build a kernel that
> mixes clocksource support with get_arch_timeoffset, you'll need to
> rework the #ifdef in update_wall_time(), since we currently assume with
> get_arch_timeoffset() that you're using tick + interpolation, so every
> call to update_wall_time() only moves time forward by one jiffy.

OK. I don't have any immediate plans to do that, although I wouldn't be
surprised if we (the ARM community in general) end up wanting to do that
at some point. It all depends on which ARM sub-architectures end up
getting converted to the multi-platform zImage support I guess.

> Otherwise, thanks for the name tweak. Going through the arm-soc tree is
> fine with me.
>
> Acked-by: John Stultz <[email protected]>

Thanks.

2012-11-13 21:52:54

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V2 2/11] time: convert arch_gettimeoffset to a pointer

On Tuesday 13 November 2012, Stephen Warren wrote:
> > One last thing to watch out for: If you're trying to build a kernel that
> > mixes clocksource support with get_arch_timeoffset, you'll need to
> > rework the #ifdef in update_wall_time(), since we currently assume with
> > get_arch_timeoffset() that you're using tick + interpolation, so every
> > call to update_wall_time() only moves time forward by one jiffy.
>
> OK. I don't have any immediate plans to do that, although I wouldn't be
> surprised if we (the ARM community in general) end up wanting to do that
> at some point. It all depends on which ARM sub-architectures end up
> getting converted to the multi-platform zImage support I guess.

IIRC the plan is to get rid of ARCH_USES_GETTIMEOFFSET in the long run,
at least on ARM. Certainly for the platforms that are part of
the multiplatform support. The only platforms remaining that currently
use it are:

* gemini
* ebsa110
* ep93xx
* h720x
* rpc
* s3c24xx
* s3c64xx
* s5pc100
* shark

Most of these are actually ARMv4 based and not maintained very actively.
I'm pretty sure we can take care of ep93xx and the samsung platforms,
which are the more active of the above.

Arnd