| Message ID | 1346275524-13817-2-git-send-email-will.deacon@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 8/29/2012 2:25 PM, Will Deacon wrote: > From: Jonathan Austin <Jonathan.Austin@arm.com> > > The current timer-based delay loop relies on the architected timer to > initiate the switch away from the polling-based implementation. This is > unfortunate for platforms without the architected timers but with a > suitable delay source (that is, constant frequency, always powered-up > and ticking as long as the CPUs are online). > > This patch introduces a registration mechanism for the delay timer > (which provides an unconditional read_current_timer implementation) and > updates the architected timer code to use the new interface. > > Signed-off-by: Jonathan Austin <jonathan.austin@arm.com> > Signed-off-by: Will Deacon <will.deacon@arm.com> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org> > diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h > index 62e7547..88401c2 100644 > --- a/arch/arm/include/asm/arch_timer.h > +++ b/arch/arm/include/asm/arch_timer.h > @@ -4,7 +4,6 @@ > #include <asm/errno.h> > > #ifdef CONFIG_ARM_ARCH_TIMER > -#define ARCH_HAS_READ_CURRENT_TIMER > int arch_timer_of_register(void); > int arch_timer_sched_clock_init(void); > #else > diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h > index dc61451..50928e8 100644 > --- a/arch/arm/include/asm/delay.h > +++ b/arch/arm/include/asm/delay.h > @@ -15,6 +15,11 @@ > > #ifndef __ASSEMBLY__ > > +struct delay_timer { > + unsigned long (*read_current_timer)(void); > + unsigned long freq; I wonder if we should print a warning and not actually switch to the timer based udelay if the frequency is not fast enough (< 1Mhz). Or people just wouldn't do that? > @@ -55,18 +62,24 @@ static void __timer_udelay(unsigned long usecs) > __timer_const_udelay(usecs * UDELAY_MULT); > } > > -void __init init_current_timer_delay(unsigned long freq) > +void __init register_current_timer_delay(struct delay_timer *timer) const? > { > - pr_info("Switching to timer-based delay loop\n"); > - lpj_fine = freq / HZ; > - loops_per_jiffy = lpj_fine; > - arm_delay_ops.delay = __timer_delay; > - arm_delay_ops.const_udelay = __timer_const_udelay; > - arm_delay_ops.udelay = __timer_udelay; > + if (!delay_calibrated) { > + pr_info("Switching to timer-based delay loop\n"); > + delay_timer = timer; > + lpj_fine = timer->freq / HZ; > + loops_per_jiffy = lpj_fine; > + arm_delay_ops.delay = __timer_delay; > + arm_delay_ops.const_udelay = __timer_const_udelay; > + arm_delay_ops.udelay = __timer_udelay; > + delay_calibrated = true; > + } else { > + pr_info("Ignoring duplicate/late registration of read_current_timer delay\n"); warn?
On Fri, Aug 31, 2012 at 12:51:25AM +0100, Stephen Boyd wrote: > On 8/29/2012 2:25 PM, Will Deacon wrote: > > From: Jonathan Austin <Jonathan.Austin@arm.com> > > > > The current timer-based delay loop relies on the architected timer to > > initiate the switch away from the polling-based implementation. This is > > unfortunate for platforms without the architected timers but with a > > suitable delay source (that is, constant frequency, always powered-up > > and ticking as long as the CPUs are online). > > > > This patch introduces a registration mechanism for the delay timer > > (which provides an unconditional read_current_timer implementation) and > > updates the architected timer code to use the new interface. > > > > Signed-off-by: Jonathan Austin <jonathan.austin@arm.com> > > Signed-off-by: Will Deacon <will.deacon@arm.com> > > Reviewed-by: Stephen Boyd <sboyd@codeaurora.org> Thanks Stephen. > I wonder if we should print a warning and not actually switch to the > timer based udelay if the frequency is not fast enough (< 1Mhz). Or > people just wouldn't do that? I don't think we need to worry about that -- we should let people decide whether they deem their timer to be appropriate or not. > > @@ -55,18 +62,24 @@ static void __timer_udelay(unsigned long usecs) > > __timer_const_udelay(usecs * UDELAY_MULT); > > } > > > > -void __init init_current_timer_delay(unsigned long freq) > > +void __init register_current_timer_delay(struct delay_timer *timer) > > const? Sure. I'll send an updated patch. > > + pr_info("Ignoring duplicate/late registration of read_current_timer delay\n"); > > warn? Nah, it's not the end of the world if this happens. We'll just fall back to the CPU-based calibration, which isn't a problem. Will
On Tue, Sep 04, 2012 at 11:07:50AM +0100, Will Deacon wrote: > On Fri, Aug 31, 2012 at 12:51:25AM +0100, Stephen Boyd wrote: > > > +void __init register_current_timer_delay(struct delay_timer *timer) > > > > const? > > Sure. I'll send an updated patch. Gah, spoke too soon. This isn't going to work, as the frequency may not be known until probe time (for example, it could be obtained by the device tree) so const data isn't going to cut it. Will
On Wed, Aug 29, 2012 at 10:25:24PM +0100, Will Deacon wrote: > From: Jonathan Austin <Jonathan.Austin@arm.com> > > The current timer-based delay loop relies on the architected timer to > initiate the switch away from the polling-based implementation. This is > unfortunate for platforms without the architected timers but with a > suitable delay source (that is, constant frequency, always powered-up > and ticking as long as the CPUs are online). > > This patch introduces a registration mechanism for the delay timer > (which provides an unconditional read_current_timer implementation) and > updates the architected timer code to use the new interface. [...] > +int read_current_timer(unsigned long *timer_val) > +{ > + return delay_timer ? delay_timer->read_current_timer() : -ENXIO; > +} So this is clearly wrong with the new function prototype -- I've fixed it on my delay branch (when the mirrors sync). Will
diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h index 62e7547..88401c2 100644 --- a/arch/arm/include/asm/arch_timer.h +++ b/arch/arm/include/asm/arch_timer.h @@ -4,7 +4,6 @@ #include <asm/errno.h> #ifdef CONFIG_ARM_ARCH_TIMER -#define ARCH_HAS_READ_CURRENT_TIMER int arch_timer_of_register(void); int arch_timer_sched_clock_init(void); #else diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h index dc61451..50928e8 100644 --- a/arch/arm/include/asm/delay.h +++ b/arch/arm/include/asm/delay.h @@ -15,6 +15,11 @@ #ifndef __ASSEMBLY__ +struct delay_timer { + unsigned long (*read_current_timer)(void); + unsigned long freq; +}; + extern struct arm_delay_ops { void (*delay)(unsigned long); void (*const_udelay)(unsigned long); @@ -56,6 +61,10 @@ extern void __loop_delay(unsigned long loops); extern void __loop_udelay(unsigned long usecs); extern void __loop_const_udelay(unsigned long); +/* Delay-loop timer registration. */ +#define ARCH_HAS_READ_CURRENT_TIMER +extern void register_current_timer_delay(struct delay_timer *timer); + #endif /* __ASSEMBLY__ */ #endif /* defined(_ARM_DELAY_H) */ diff --git a/arch/arm/include/asm/timex.h b/arch/arm/include/asm/timex.h index ce11944..9acc135 100644 --- a/arch/arm/include/asm/timex.h +++ b/arch/arm/include/asm/timex.h @@ -12,15 +12,9 @@ #ifndef _ASMARM_TIMEX_H #define _ASMARM_TIMEX_H -#include <asm/arch_timer.h> #include <mach/timex.h> typedef unsigned long cycles_t; - -#ifdef ARCH_HAS_READ_CURRENT_TIMER #define get_cycles() ({ cycles_t c; read_current_timer(&c) ? 0 : c; }) -#else -#define get_cycles() (0) -#endif #endif diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c index cf25880..700917a 100644 --- a/arch/arm/kernel/arch_timer.c +++ b/arch/arm/kernel/arch_timer.c @@ -21,6 +21,7 @@ #include <linux/io.h> #include <asm/cputype.h> +#include <asm/delay.h> #include <asm/localtimer.h> #include <asm/arch_timer.h> #include <asm/system_info.h> @@ -31,8 +32,7 @@ static int arch_timer_ppi; static int arch_timer_ppi2; static struct clock_event_device __percpu **arch_timer_evt; - -extern void init_current_timer_delay(unsigned long freq); +static struct delay_timer arch_delay_timer; /* * Architected system timer support. @@ -225,12 +225,9 @@ static cycle_t arch_counter_read(struct clocksource *cs) return arch_counter_get_cntpct(); } -int read_current_timer(unsigned long *timer_val) +static unsigned long arch_timer_read_current_timer(void) { - if (!arch_timer_rate) - return -ENXIO; - *timer_val = arch_counter_get_cntpct(); - return 0; + return arch_counter_get_cntpct(); } static struct clocksource clocksource_counter = { @@ -302,11 +299,13 @@ static int __init arch_timer_register(void) arch_timer_global_evt.cpumask = cpumask_of(0); err = arch_timer_setup(&arch_timer_global_evt); } - if (err) goto out_free_irq; - init_current_timer_delay(arch_timer_rate); + /* Use the architected timer for the delay loop. */ + arch_delay_timer.read_current_timer = &arch_timer_read_current_timer; + arch_delay_timer.freq = arch_timer_rate; + register_current_timer_delay(&arch_delay_timer); return 0; out_free_irq: diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c index 395d5fb..fe6f4fb 100644 --- a/arch/arm/lib/delay.c +++ b/arch/arm/lib/delay.c @@ -34,7 +34,14 @@ struct arm_delay_ops arm_delay_ops = { .udelay = __loop_udelay, }; -#ifdef ARCH_HAS_READ_CURRENT_TIMER +static struct delay_timer *delay_timer; +static bool delay_calibrated; + +int read_current_timer(unsigned long *timer_val) +{ + return delay_timer ? delay_timer->read_current_timer() : -ENXIO; +} + static void __timer_delay(unsigned long cycles) { cycles_t start = get_cycles(); @@ -55,18 +62,24 @@ static void __timer_udelay(unsigned long usecs) __timer_const_udelay(usecs * UDELAY_MULT); } -void __init init_current_timer_delay(unsigned long freq) +void __init register_current_timer_delay(struct delay_timer *timer) { - pr_info("Switching to timer-based delay loop\n"); - lpj_fine = freq / HZ; - loops_per_jiffy = lpj_fine; - arm_delay_ops.delay = __timer_delay; - arm_delay_ops.const_udelay = __timer_const_udelay; - arm_delay_ops.udelay = __timer_udelay; + if (!delay_calibrated) { + pr_info("Switching to timer-based delay loop\n"); + delay_timer = timer; + lpj_fine = timer->freq / HZ; + loops_per_jiffy = lpj_fine; + arm_delay_ops.delay = __timer_delay; + arm_delay_ops.const_udelay = __timer_const_udelay; + arm_delay_ops.udelay = __timer_udelay; + delay_calibrated = true; + } else { + pr_info("Ignoring duplicate/late registration of read_current_timer delay\n"); + } } unsigned long __cpuinit calibrate_delay_is_known(void) { + delay_calibrated = true; return lpj_fine; } -#endif