[PATCH 3/4] mach-integrator: modernize clock event registration

[PATCH 3/4] mach-integrator: modernize clock event registration

[Linus Walleij]

From: Linus Walleij <linus.walleij@linaro.org>

Drop the reload value for the timer - the timekeeping code
will call the .set_next_event to set this anyway.

Drop mult, shift and delta calculations and let the
clockevent core scale this as appropriate.

Set the minimum interval to 1 rather than 15 (0xf), there
is nothing in the data sheets I have indicating that 15
should be some minimum value.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-integrator/integrator_ap.c |   18 +++++-------------
 1 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c
index 8516193..55d3c84 100644
--- a/arch/arm/mach-integrator/integrator_ap.c
+++ b/arch/arm/mach-integrator/integrator_ap.c
@@ -322,8 +322,6 @@ static void __init ap_init(void)
 #define TIMER1_VA_BASE IO_ADDRESS(INTEGRATOR_TIMER1_BASE)
 #define TIMER2_VA_BASE IO_ADDRESS(INTEGRATOR_TIMER2_BASE)
 
-static unsigned long timer_reload;
-
 static void integrator_clocksource_init(u32 khz)
 {
 	void __iomem *base = (void __iomem *)TIMER2_VA_BASE;
@@ -394,12 +392,10 @@ static int clkevt_set_next_event(unsigned long next, struct clock_event_device *
 
 static struct clock_event_device integrator_clockevent = {
 	.name		= "timer1",
-	.shift		= 34,
 	.features	= CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
 	.set_mode	= clkevt_set_mode,
 	.set_next_event	= clkevt_set_next_event,
 	.rating		= 300,
-	.cpumask	= cpu_all_mask,
 };
 
 static struct irqaction integrator_timer_irq = {
@@ -411,9 +407,9 @@ static struct irqaction integrator_timer_irq = {
 
 static void integrator_clockevent_init(u32 khz)
 {
-	struct clock_event_device *evt = &integrator_clockevent;
 	unsigned int ctrl = 0;
 
+	/* Calculate and program a divisor */
 	if (khz * 1000 > 0x100000 * HZ) {
 		khz /= 256;
 		ctrl |= TIMER_CTRL_DIV256;
@@ -421,17 +417,13 @@ static void integrator_clockevent_init(u32 khz)
 		khz /= 16;
 		ctrl |= TIMER_CTRL_DIV16;
 	}
-
-	timer_reload = khz * 1000 / HZ;
 	writel(ctrl, clkevt_base + TIMER_CTRL);
 
-	evt->irq = IRQ_TIMERINT1;
-	evt->mult = div_sc(khz, NSEC_PER_MSEC, evt->shift);
-	evt->max_delta_ns = clockevent_delta2ns(0xffff, evt);
-	evt->min_delta_ns = clockevent_delta2ns(0xf, evt);
-
 	setup_irq(IRQ_TIMERINT1, &integrator_timer_irq);
-	clockevents_register_device(evt);
+	clockevents_config_and_register(&integrator_clockevent,
+					khz * 1000,
+					1,
+					0xffffU);
 }
 
 /*
-- 
1.7.3.2

[PATCH 3/4] mach-integrator: modernize clock event registration

[Thomas Gleixner]

On Wed, 7 Sep 2011, Linus Walleij wrote:

> From: Linus Walleij <linus.walleij@linaro.org>
> 
> Drop the reload value for the timer - the timekeeping code
> will call the .set_next_event to set this anyway.
> 
> Drop mult, shift and delta calculations and let the
> clockevent core scale this as appropriate.
> 
> Set the minimum interval to 1 rather than 15 (0xf), there
> is nothing in the data sheets I have indicating that 15
> should be some minimum value.
> 
> Cc: Russell King <linux@arm.linux.org.uk>

Acked-by: Thomas Gleixner <tglx@linutronix.de>

[PATCH 3/4] mach-integrator: modernize clock event registration

[Russell King - ARM Linux]

On Wed, Sep 07, 2011 at 12:02:52PM +0200, Thomas Gleixner wrote:
> On Wed, 7 Sep 2011, Linus Walleij wrote:
> 
> > From: Linus Walleij <linus.walleij@linaro.org>
> > 
> > Drop the reload value for the timer - the timekeeping code
> > will call the .set_next_event to set this anyway.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> > 
> > Drop mult, shift and delta calculations and let the
> > clockevent core scale this as appropriate.
> > 
> > Set the minimum interval to 1 rather than 15 (0xf), there
> > is nothing in the data sheets I have indicating that 15
> > should be some minimum value.
> > 
> > Cc: Russell King <linux@arm.linux.org.uk>
> 
> Acked-by: Thomas Gleixner <tglx@linutronix.de>

I mentioned to Thomas that which I've underlined above yesterday after I
received Thomas' ack.  I was expecting Thomas to reply about this, but
that obviously hasn't happened, so now that it's appeared in the patch
system, it's become my problem to deal with.

My understanding is that periodic timers are not setup by the generic
clock event code, and so we do need to keep the 'timer_reload' stuff
around.  Note that patch 2 deletes the register write for this too,
so you actually have one logical change split across patch 2 and 3.

[PATCH 3/4] mach-integrator: modernize clock event registration

[Linus Walleij]

On Thu, Sep 8, 2011 at 10:55 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
>> Acked-by: Thomas Gleixner <tglx@linutronix.de>

> My understanding is that periodic timers are not setup by the generic
> clock event code, and so we do need to keep the 'timer_reload' stuff
> around.

Aha. Yes maybe I've not tested that mode well enough, I'll
have a closer look.

An alteranative is to retire the periodic mode and only
support oneshot, like the plat-nomadik/timer.c currently
does. My impression is that oneshot will always work fine,
and doing this leaves all details up to the timekeeping
core so we don't need to deal with it, but I may be wrong.
Ideas?

> Note that patch 2 deletes the register write for this too,
> so you actually have one logical change split across patch 2 and 3.

Yes, no good. I'll fix.

Yours,
Linus Walleij

[PATCH 3/4] mach-integrator: modernize clock event registration

[Thomas Gleixner]

On Thu, 8 Sep 2011, Russell King - ARM Linux wrote:

> On Wed, Sep 07, 2011 at 12:02:52PM +0200, Thomas Gleixner wrote:
> > On Wed, 7 Sep 2011, Linus Walleij wrote:
> > 
> > > From: Linus Walleij <linus.walleij@linaro.org>
> > > 
> > > Drop the reload value for the timer - the timekeeping code
> > > will call the .set_next_event to set this anyway.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> > > 
> > > Drop mult, shift and delta calculations and let the
> > > clockevent core scale this as appropriate.
> > > 
> > > Set the minimum interval to 1 rather than 15 (0xf), there
> > > is nothing in the data sheets I have indicating that 15
> > > should be some minimum value.
> > > 
> > > Cc: Russell King <linux@arm.linux.org.uk>
> > 
> > Acked-by: Thomas Gleixner <tglx@linutronix.de>
> 
> I mentioned to Thomas that which I've underlined above yesterday after I
> received Thomas' ack.  I was expecting Thomas to reply about this, but
> that obviously hasn't happened, so now that it's appeared in the patch
> system, it's become my problem to deal with.

Sorry, I missed that detail in the patch :(
 
> My understanding is that periodic timers are not setup by the generic
> clock event code, and so we do need to keep the 'timer_reload' stuff
> around.  Note that patch 2 deletes the register write for this too,
> so you actually have one logical change split across patch 2 and 3.

Periodic mode is set up via clkevt->set_mode() if the clock event
device provides CLOCK_EVT_FEAT_PERIODIC.

clkevt->set_next_event() is only used in oneshot mode.

Thanks,

	tglx

[PATCH 3/4] mach-integrator: modernize clock event registration

[Thomas Gleixner]

On Thu, 8 Sep 2011, Linus Walleij wrote:

> On Thu, Sep 8, 2011 at 10:55 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> >> Acked-by: Thomas Gleixner <tglx@linutronix.de>
> 
> > My understanding is that periodic timers are not setup by the generic
> > clock event code, and so we do need to keep the 'timer_reload' stuff
> > around.
> 
> Aha. Yes maybe I've not tested that mode well enough, I'll
> have a closer look.
> 
> An alteranative is to retire the periodic mode and only
> support oneshot, like the plat-nomadik/timer.c currently
> does. My impression is that oneshot will always work fine,
> and doing this leaves all details up to the timekeeping
> core so we don't need to deal with it, but I may be wrong.
> Ideas?

It does, but for a pure periodic setup (nohz=off, highres=off) you add
the overhead of reprogramming the device for each tick.

Thanks,

	tglx

[PATCH 3/4] mach-integrator: modernize clock event registration

[Linus Walleij]

2011/9/8 Russell King - ARM Linux <linux@arm.linux.org.uk>:

> My understanding is that periodic timers are not setup by the generic
> clock event code, and so we do need to keep the 'timer_reload' stuff
> around. ?Note that patch 2 deletes the register write for this too,
> so you actually have one logical change split across patch 2 and 3.

I've fixed this up in v2 (hopefully) and I'll update the patches
2 thru 4 to the new versions in the patch system.

Thanks Russell!
Linus Walleij