[RFC/RFT 1/3] iop: clocksource support

[RFC/RFT 1/3] iop: clocksource support

[Mikael Pettersson]

This updates the IOP platform to expose the free-running
timer 1 as a clocksource object. This timer is now also
properly initialised, which requires a new write_tcr1()
function from the mach implementation code. Apart from the
explicit initialisation, there is no functional change in
how timer 1 is programmed.

The one concern I have is whether the clocksource .shift
value of 20 is appropriate for timer frequencies in the
200/266/333 MHz range which seem to be typical for IOP.

Tested on n2100, compile-tested for all plat-iop machines.

Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
---
 arch/arm/include/asm/hardware/iop3xx.h    |    5 +++++
 arch/arm/mach-iop13xx/include/mach/time.h |    5 +++++
 arch/arm/plat-iop/time.c                  |   27 +++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

diff -rupN linux-2.6.31-rc7/arch/arm/include/asm/hardware/iop3xx.h linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/include/asm/hardware/iop3xx.h
--- linux-2.6.31-rc7/arch/arm/include/asm/hardware/iop3xx.h	2008-10-11 10:43:49.000000000 +0200
+++ linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/include/asm/hardware/iop3xx.h	2009-08-22 12:06:28.000000000 +0200
@@ -259,6 +259,11 @@ static inline u32 read_tcr1(void)
 	return val;
 }
 
+static inline void write_tcr1(u32 val)
+{
+	asm volatile("mcr p6, 0, %0, c3, c1, 0" : : "r" (val));
+}
+
 static inline void write_trr0(u32 val)
 {
 	asm volatile("mcr p6, 0, %0, c4, c1, 0" : : "r" (val));
diff -rupN linux-2.6.31-rc7/arch/arm/mach-iop13xx/include/mach/time.h linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/mach-iop13xx/include/mach/time.h
--- linux-2.6.31-rc7/arch/arm/mach-iop13xx/include/mach/time.h	2008-12-25 15:54:13.000000000 +0100
+++ linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/mach-iop13xx/include/mach/time.h	2009-08-22 12:06:28.000000000 +0200
@@ -90,6 +90,11 @@ static inline u32 read_tcr1(void)
 	return val;
 }
 
+static inline void write_tcr1(u32 val)
+{
+	asm volatile("mcr p6, 0, %0, c3, c9, 0" : : "r" (val));
+}
+
 static inline void write_trr0(u32 val)
 {
 	asm volatile("mcr p6, 0, %0, c4, c9, 0" : : "r" (val));
diff -rupN linux-2.6.31-rc7/arch/arm/plat-iop/time.c linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/plat-iop/time.c
--- linux-2.6.31-rc7/arch/arm/plat-iop/time.c	2008-12-25 15:54:14.000000000 +0100
+++ linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/plat-iop/time.c	2009-08-22 12:06:28.000000000 +0200
@@ -19,6 +19,7 @@
 #include <linux/init.h>
 #include <linux/timex.h>
 #include <linux/io.h>
+#include <linux/clocksource.h>
 #include <mach/hardware.h>
 #include <asm/irq.h>
 #include <asm/uaccess.h>
@@ -26,6 +27,23 @@
 #include <asm/mach/time.h>
 #include <mach/time.h>
 
+/*
+ * IOP clocksource (free-running timer 1).
+ */
+static cycle_t iop_clocksource_read(struct clocksource *unused)
+{
+	return 0xffffffffu - read_tcr1();
+}
+
+static struct clocksource iop_clocksource = {
+	.name 		= "iop_timer1",
+	.rating		= 300,
+	.read		= iop_clocksource_read,
+	.mask		= CLOCKSOURCE_MASK(32),
+	.shift 		= 20,	/* ??? crude guesstimate */
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
 static unsigned long ticks_per_jiffy;
 static unsigned long ticks_per_usec;
 static unsigned long next_jiffy_time;
@@ -99,8 +117,17 @@ void __init iop_init_time(unsigned long 
 	 */
 	write_trr0(ticks_per_jiffy - 1);
 	write_tmr0(timer_ctl);
+
+	/*
+	 * Set up free-running clocksource timer 1.
+	 */
 	write_trr1(0xffffffff);
+	write_tcr1(0xffffffff);
 	write_tmr1(timer_ctl);
+	iop_clocksource.mult =
+		clocksource_hz2mult(tick_rate,
+				    iop_clocksource.shift);
+	clocksource_register(&iop_clocksource);
 
 	setup_irq(IRQ_IOP_TIMER0, &iop_timer_irq);
 }

[RFC/RFT 1/3] iop: clocksource support

[Linus Walleij]

Tjenare Mikael,

2009/8/22 Mikael Pettersson <mikpe@it.uu.se>:

> This updates the IOP platform to expose the free-running
> timer 1 as a clocksource object. This timer is now also
> properly initialised, which requires a new write_tcr1()
> function from the mach implementation code. Apart from the
> explicit initialisation, there is no functional change in
> how timer 1 is programmed.
>
> The one concern I have is whether the clocksource .shift
> value of 20 is appropriate for timer frequencies in the
> 200/266/333 MHz range which seem to be typical for IOP.
>
> Tested on n2100, compile-tested for all plat-iop machines.
>
> Signed-off-by: Mikael Pettersson <mikpe@it.uu.se>
> ---
> ?arch/arm/include/asm/hardware/iop3xx.h ? ?| ? ?5 +++++
> ?arch/arm/mach-iop13xx/include/mach/time.h | ? ?5 +++++
> ?arch/arm/plat-iop/time.c ? ? ? ? ? ? ? ? ?| ? 27 +++++++++++++++++++++++++++
> ?3 files changed, 37 insertions(+)
>
> diff -rupN linux-2.6.31-rc7/arch/arm/include/asm/hardware/iop3xx.h linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/include/asm/hardware/iop3xx.h
> --- linux-2.6.31-rc7/arch/arm/include/asm/hardware/iop3xx.h ? ? 2008-10-11 10:43:49.000000000 +0200
> +++ linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/include/asm/hardware/iop3xx.h ? ? ? 2009-08-22 12:06:28.000000000 +0200
> @@ -259,6 +259,11 @@ static inline u32 read_tcr1(void)
> ? ? ? ?return val;
> ?}
>
> +static inline void write_tcr1(u32 val)
> +{
> + ? ? ? asm volatile("mcr p6, 0, %0, c3, c1, 0" : : "r" (val));
> +}
> +
> ?static inline void write_trr0(u32 val)
> ?{
> ? ? ? ?asm volatile("mcr p6, 0, %0, c4, c1, 0" : : "r" (val));
> diff -rupN linux-2.6.31-rc7/arch/arm/mach-iop13xx/include/mach/time.h linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/mach-iop13xx/include/mach/time.h
> --- linux-2.6.31-rc7/arch/arm/mach-iop13xx/include/mach/time.h ?2008-12-25 15:54:13.000000000 +0100
> +++ linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/mach-iop13xx/include/mach/time.h ? ?2009-08-22 12:06:28.000000000 +0200
> @@ -90,6 +90,11 @@ static inline u32 read_tcr1(void)
> ? ? ? ?return val;
> ?}
>
> +static inline void write_tcr1(u32 val)
> +{
> + ? ? ? asm volatile("mcr p6, 0, %0, c3, c9, 0" : : "r" (val));
> +}
> +
> ?static inline void write_trr0(u32 val)
> ?{
> ? ? ? ?asm volatile("mcr p6, 0, %0, c4, c9, 0" : : "r" (val));
> diff -rupN linux-2.6.31-rc7/arch/arm/plat-iop/time.c linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/plat-iop/time.c
> --- linux-2.6.31-rc7/arch/arm/plat-iop/time.c ? 2008-12-25 15:54:14.000000000 +0100
> +++ linux-2.6.31-rc7.arm-iop-1-clocksource/arch/arm/plat-iop/time.c ? ? 2009-08-22 12:06:28.000000000 +0200
> @@ -19,6 +19,7 @@
> ?#include <linux/init.h>
> ?#include <linux/timex.h>
> ?#include <linux/io.h>
> +#include <linux/clocksource.h>
> ?#include <mach/hardware.h>
> ?#include <asm/irq.h>
> ?#include <asm/uaccess.h>
> @@ -26,6 +27,23 @@
> ?#include <asm/mach/time.h>
> ?#include <mach/time.h>
>
> +/*
> + * IOP clocksource (free-running timer 1).
> + */
> +static cycle_t iop_clocksource_read(struct clocksource *unused)
> +{
> + ? ? ? return 0xffffffffu - read_tcr1();

So tcr1 counts downwards and wraps around?

> +}
> +
> +static struct clocksource iop_clocksource = {
> + ? ? ? .name ? ? ? ? ? = "iop_timer1",
> + ? ? ? .rating ? ? ? ? = 300,
> + ? ? ? .read ? ? ? ? ? = iop_clocksource_read,
> + ? ? ? .mask ? ? ? ? ? = CLOCKSOURCE_MASK(32),
> + ? ? ? .shift ? ? ? ? ?= 20, ? /* ??? crude guesstimate */

Calculate this using the algorithm in arch/mips/kernel/time.c
they have a dynamically changing clocksource...

> + ? ? ? .flags ? ? ? ? ?= CLOCK_SOURCE_IS_CONTINUOUS,
> +};
> +
> ?static unsigned long ticks_per_jiffy;
> ?static unsigned long ticks_per_usec;
> ?static unsigned long next_jiffy_time;
> @@ -99,8 +117,17 @@ void __init iop_init_time(unsigned long
> ? ? ? ? */
> ? ? ? ?write_trr0(ticks_per_jiffy - 1);
> ? ? ? ?write_tmr0(timer_ctl);
> +
> + ? ? ? /*
> + ? ? ? ?* Set up free-running clocksource timer 1.
> + ? ? ? ?*/
> ? ? ? ?write_trr1(0xffffffff);
> + ? ? ? write_tcr1(0xffffffff);
> ? ? ? ?write_tmr1(timer_ctl);
> + ? ? ? iop_clocksource.mult =
> + ? ? ? ? ? ? ? clocksource_hz2mult(tick_rate,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? iop_clocksource.shift);
> + ? ? ? clocksource_register(&iop_clocksource);
>
> ? ? ? ?setup_irq(IRQ_IOP_TIMER0, &iop_timer_irq);
> ?}
>

Linus Walleij

[RFC/RFT 1/3] iop: clocksource support

[Mikael Pettersson]

Linus Walleij writes:
 > > +/*
 > > + * IOP clocksource (free-running timer 1).
 > > + */
 > > +static cycle_t iop_clocksource_read(struct clocksource *unused)
 > > +{
 > > + ? ? ? return 0xffffffffu - read_tcr1();
 > 
 > So tcr1 counts downwards and wraps around?

Yes. The timers are decrementers that when reaching zero either
halt or reload from another register and continue decrementing,
depending on settings in a mode register.

 > > +static struct clocksource iop_clocksource = {
 > > + ? ? ? .name ? ? ? ? ? = "iop_timer1",
 > > + ? ? ? .rating ? ? ? ? = 300,
 > > + ? ? ? .read ? ? ? ? ? = iop_clocksource_read,
 > > + ? ? ? .mask ? ? ? ? ? = CLOCKSOURCE_MASK(32),
 > > + ? ? ? .shift ? ? ? ? ?= 20, ? /* ??? crude guesstimate */
 > 
 > Calculate this using the algorithm in arch/mips/kernel/time.c
 > they have a dynamically changing clocksource...

Thanks, I've implemented that now. On my n2100 with its 198MHz
timer it computes shift = 29 and mult = 0xa19dbcc4.

The algorithm always computes the largest shift/mult pair that
solves the equation:

	// 0 <= shift && shift <= 32
	u64 mult = ((u64)1E9 << shift) / hz;
	(mult >> 32) == 0

Can I assume that this is to minimize precision loss?

/Mikael

[RFC/RFT 1/3] iop: clocksource support

[Linus Walleij]

2009/9/1 Mikael Pettersson <mikpe@it.uu.se>:

> ?> Calculate this using the algorithm in arch/mips/kernel/time.c
> ?> they have a dynamically changing clocksource...
>
> The algorithm always computes the largest shift/mult pair that
> solves the equation:
>
> ? ? ? ?// 0 <= shift && shift <= 32
> ? ? ? ?u64 mult = ((u64)1E9 << shift) / hz;
> ? ? ? ?(mult >> 32) == 0
>
> Can I assume that this is to minimize precision loss?

So I think, I asked the question of how to calculate div but noone
answered IIRC, then I found the MIPS code and it contained this
clue. Ralf Baechle from the MIPS camp wrote this code so lets
ask him.

Linus Walleij

[RFC/RFT 1/3] iop: clocksource support

[Ralf Baechle]

On Tue, Sep 01, 2009 at 11:48:57PM +0200, Linus Walleij wrote:

> > ?> Calculate this using the algorithm in arch/mips/kernel/time.c
> > ?> they have a dynamically changing clocksource...
> >
> > The algorithm always computes the largest shift/mult pair that
> > solves the equation:
> >
> > ? ? ? ?// 0 <= shift && shift <= 32
> > ? ? ? ?u64 mult = ((u64)1E9 << shift) / hz;
> > ? ? ? ?(mult >> 32) == 0
> >
> > Can I assume that this is to minimize precision loss?
> 
> So I think, I asked the question of how to calculate div but noone
> answered IIRC, then I found the MIPS code and it contained this
> clue. Ralf Baechle from the MIPS camp wrote this code so lets
> ask him.

The code you're talking about is in arch/mips/kernel/time.c:

[...]
void __init clocksource_set_clock(struct clocksource *cs, unsigned int clock)
{
	u64 temp;
	u32 shift;

	/* Find a shift value */
	for (shift = 32; shift > 0; shift--) {
		temp = (u64) NSEC_PER_SEC << shift;
		do_div(temp, clock);
		if ((temp >> 32) == 0)
			break;
	}
	cs->shift = shift;
	cs->mult = (u32) temp;
}

void __cpuinit clockevent_set_clock(struct clock_event_device *cd,
	unsigned int clock)
{
	u64 temp;
	u32 shift;

	/* Find a shift value */
	for (shift = 32; shift > 0; shift--) {
		temp = (u64) clock << shift;
		do_div(temp, NSEC_PER_SEC);
		if ((temp >> 32) == 0)
			break;
	}
	cd->shift = shift;
	cd->mult = (u32) temp;
}
[...]

The algorithm tries to minimize the loss of precision.  Like two years ago
I had already agreed with Thomas Gleixner to move this function into
generic code but somehow that never happened.

  Ralf