ARM: ks8695: convert to generic time and clocksource

Old platforms using ancient gettimeoffset() and other arcane APIs are standing in the way of cleaning up the ARM kernel.

This is an attempt at blind-coding a generic time and clocksource driver for the platform by way of a datasheet and looking at the old code, it'd be great if someone who is actually using this machine could test it.

If noone volunteers to do this I will instead propose a patch deleting the machine altogether.

Signed-off-by Linus Walleij <[email protected]>

---
  arch/arm/Kconfig            |    3 +-
  arch/arm/mach-ks8695/time.c |  123 ++++++++++++++++++++++++++++++-------------
  2 files changed, 88 insertions(+), 38 deletions(-)

arch/arm/Kconfig +2 -1

index c5f9ae5..181b697 100644
@@ -652,8 +652,9 @@ config ARCH_KS8695
  	bool "Micrel/Kendin KS8695"
  	select CPU_ARM922T
  	select ARCH_REQUIRE_GPIOLIB
-	select ARCH_USES_GETTIMEOFFSET
  	select NEED_MACH_MEMORY_H
+	select CLKSRC_MMIO
+	select GENERIC_CLOCKEVENTS
  	help
  	  Support for Micrel/Kendin KS8695 "Centaur" (ARM922T) based
  	  System-on-Chip devices.

arch/arm/mach-ks8695/time.c +86 -37

index 6974c23..1a933f1 100644
@@ -25,6 +25,8 @@
  #include <linux/kernel.h>
  #include <linux/sched.h>
  #include <linux/io.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
 
  #include <asm/mach/time.h>
  #include <asm/system_misc.h>

@@ -53,44 +55,68 @@
  /* Timer0 Timeout Counter Register */
  #define T0TC_WATCHDOG		(0xff)		/* Enable watchdog mode */
 
-/*
- * Returns number of ms since last clock interrupt.  Note that interrupts
- * will have been disabled by do_gettimeoffset()
- */
-static unsigned long ks8695_gettimeoffset (void)
+static void ks8695_set_mode(enum clock_event_mode mode,
+			    struct clock_event_device *evt)
  {
-	unsigned long elapsed, tick2, intpending;
+	if (mode == CLOCK_EVT_FEAT_PERIODIC) {
+		u32 rate = DIV_ROUND_CLOSEST(KS8695_CLOCK_RATE, HZ);
+		u32 half = DIV_ROUND_CLOSEST(rate, 2);
+		u32 tmcon;
+
+		/* Disable timer 1 */
+		tmcon = readl_relaxed(KS8695_TMR_VA + KS8695_TMCON);
+		tmcon &= TMCON_T1EN;
+		writel_relaxed(tmcon, KS8695_TMR_VA + KS8695_TMCON);
+
+		/* Both registers need to count down */
+		writel_relaxed(half, KS8695_TMR_VA + KS8695_T1TC);
+		writel_relaxed(half, KS8695_TMR_VA + KS8695_T1PD);
+
+		/* Re-enable timer1 */
+		tmcon |= TMCON_T1EN;
+		writel_relaxed(tmcon, KS8695_TMR_VA + KS8695_TMCON);
+	}
+}
 
-	/*
-	 * Get the current number of ticks.  Note that there is a race
-	 * condition between us reading the timer and checking for an
-	 * interrupt.  We solve this by ensuring that the counter has not
-	 * reloaded between our two reads.
-	 */
-	elapsed = readl_relaxed(KS8695_TMR_VA + KS8695_T1TC) + readl_relaxed(KS8695_TMR_VA + KS8695_T1PD);
-	do {
-		tick2 = elapsed;
-		intpending = readl_relaxed(KS8695_IRQ_VA + KS8695_INTST) & (1 << KS8695_IRQ_TIMER1);
-		elapsed = readl_relaxed(KS8695_TMR_VA + KS8695_T1TC) + readl_relaxed(KS8695_TMR_VA + KS8695_T1PD);
-	} while (elapsed > tick2);
-
-	/* Convert to number of ticks expired (not remaining) */
-	elapsed = (CLOCK_TICK_RATE / HZ) - elapsed;
-
-	/* Is interrupt pending?  If so, then timer has been reloaded already. */
-	if (intpending)
-		elapsed += (CLOCK_TICK_RATE / HZ);
-
-	/* Convert ticks to usecs */
-	return (unsigned long)(elapsed * (tick_nsec / 1000)) / LATCH;
+static int ks8695_set_next_event(unsigned long cycles,
+				 struct clock_event_device *evt)
+
+{
+	u32 half = DIV_ROUND_CLOSEST(cycles, 2);
+	u32 tmcon;
+
+	/* Disable timer 1 */
+	tmcon = readl_relaxed(KS8695_TMR_VA + KS8695_TMCON);
+	tmcon &= TMCON_T1EN;
+	writel_relaxed(tmcon, KS8695_TMR_VA + KS8695_TMCON);
+
+	/* Both registers need to count down */
+	writel_relaxed(half, KS8695_TMR_VA + KS8695_T1TC);
+	writel_relaxed(half, KS8695_TMR_VA + KS8695_T1PD);
+
+	/* Re-enable timer1 */
+	tmcon |= TMCON_T1EN;
+	writel_relaxed(tmcon, KS8695_TMR_VA + KS8695_TMCON);
+
+	return 0;
  }
 
+static struct clock_event_device clockevent_ks8695 = {
+	.name		= "ks8695_t1tc",
+	.rating		= 300, /* Reasonably fast and accurate clock event */
+	.features	= CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
+	.set_next_event	= ks8695_set_next_event,
+	.set_mode	= ks8695_set_mode,
+};
+
  /*
   * IRQ handler for the timer.
   */
  static irqreturn_t ks8695_timer_interrupt(int irq, void *dev_id)
  {
-	timer_tick();
+	struct clock_event_device *evt = &clockevent_ks8695;
+
+	evt->event_handler(evt);
  	return IRQ_HANDLED;
  }
 

@@ -102,18 +128,42 @@ static struct irqaction ks8695_timer_irq = {
 
  static void ks8695_timer_setup(void)
  {
-	unsigned long tmout = CLOCK_TICK_RATE / HZ;
  	unsigned long tmcon;
 
-	/* disable timer1 */
+	/* Disable timer 0 and 1 */
  	tmcon = readl_relaxed(KS8695_TMR_VA + KS8695_TMCON);
-	writel_relaxed(tmcon & ~TMCON_T1EN, KS8695_TMR_VA + KS8695_TMCON);
+	tmcon &= TMCON_T0EN;
+	tmcon &= TMCON_T1EN;
+	writel_relaxed(tmcon, KS8695_TMR_VA + KS8695_TMCON);
 
-	writel_relaxed(tmout / 2, KS8695_TMR_VA + KS8695_T1TC);
-	writel_relaxed(tmout / 2, KS8695_TMR_VA + KS8695_T1PD);
+	/*
+	 * Set up timer 0 to loop indefinately, count 32 bits with TOUT0
+	 * set to zero, then 1 bit with TOUT0 set to 1. The reason we are doing
+	 * this is that it's not allowed to set either register to 0, then the
+	 * behaviour is "unpredictable". This injects a faulty pulse every
+	 * 2^32-1 cycles but we can surely live with that rather than
+	 * complicating the code.
+	 */
+	writel_relaxed(0xFFFFFFFFU, KS8695_TMR_VA + KS8695_T0TC);
+	writel_relaxed(1, KS8695_TMR_VA + KS8695_T0PD);
+	/* Start timer 0 */
+	tmcon |= TMCON_T0EN;
+	writel_relaxed(tmcon, KS8695_TMR_VA + KS8695_TMCON);
+	/* Use timer 0 as clocksource */
+	if (clocksource_mmio_init((void *) KS8695_TMR_VA + KS8695_T0TC,
+			"ks8695_t0tc", KS8695_CLOCK_RATE, 300, 32,
+			clocksource_mmio_readl_up))
+		pr_err("timer: failed to initialize KS8695 clock source\n");
 
-	/* re-enable timer1 */
-	writel_relaxed(tmcon | TMCON_T1EN, KS8695_TMR_VA + KS8695_TMCON);
+	/*
+	 * Use timer 1 to fire IRQs on the timeline, only support one-shot
+	 * mode and the system will take care of the rest. Minimum 2 cycles
+	 * (one on each counter) maximum 2*2^32, but the API will only
+	 * accept up to a 32bit full word (0xFFFFFFFFU).
+	 */
+	clockevents_config_and_register(&clockevent_ks8695,
+					KS8695_CLOCK_RATE, 2,
+					0xFFFFFFFFU);
  }
 
  static void __init ks8695_timer_init (void)

@@ -126,7 +176,6 @@ static void __init ks8695_timer_init (void)
 
  struct sys_timer ks8695_timer = {
  	.init		= ks8695_timer_init,
-	.offset		= ks8695_gettimeoffset,
  };
 
  void ks8695_restart(char mode, const char *cmd)

Ok, tested on a KS8695 based machine. Boots and runs, obviously clock is basically working. But 'date' never shows the time increasing:

  # date
  Thu Jan  1 00:00:00 UTC 1970
  # date
  Thu Jan  1 00:00:00 UTC 1970
  # date
  Thu Jan  1 00:00:00 UTC 1970
  #

And yes, that did work before applying this patch :-)

Quoted 12 lines · Greg Ungerer

Ok, tested on a KS8695 based machine. Boots and runs, obviously clock is basically working. But 'date' never shows the time increasing:

   # date
   Thu Jan  1 00:00:00 UTC 1970
   # date
   Thu Jan  1 00:00:00 UTC 1970
   # date
   Thu Jan  1 00:00:00 UTC 1970
   #

Quoted 1 line

And yes, that did work before applying this patch :-)

That mean clockevent is running but not clocksource.

And I think that is because I forgot a ~ in the mask for disabling the T1 timer, so setting up the clockevent disables the clocksource ...

I'll send out a fixed version, plese test!

(BTW I had not clue the 68K maintainer had a platform like this, fun!)

Quoted 12 lines · Greg Ungerer

Ok, tested on a KS8695 based machine. Boots and runs, obviously clock is basically working. But 'date' never shows the time increasing:

  # date
  Thu Jan  1 00:00:00 UTC 1970
  # date
  Thu Jan  1 00:00:00 UTC 1970
  # date
  Thu Jan  1 00:00:00 UTC 1970
  #

Quoted 1 line

And yes, that did work before applying this patch :-)

That mean clockevent is running but not clocksource.

And I think that is because I forgot a ~ in the mask for disabling the T1 timer, so setting up the clockevent disables the clocksource ...

I'll send out a fixed version, plese test!

(BTW I had not clue the 68K maintainer had a platform like this, fun!)

Quoted 23 lines · Linus Walleij

Quoted 12 lines · Greg Ungerer

Ok, tested on a KS8695 based machine. Boots and runs, obviously clock is basically working. But 'date' never shows the time increasing:

   # date
   Thu Jan  1 00:00:00 UTC 1970
   # date
   Thu Jan  1 00:00:00 UTC 1970
   # date
   Thu Jan  1 00:00:00 UTC 1970
   #

Quoted 1 line

And yes, that did work before applying this patch :-)

That mean clockevent is running but not clocksource.

And I think that is because I forgot a ~ in the mask for disabling the T1 timer, so setting up the clockevent disables the clocksource ...

I'll send out a fixed version, plese test!

Quoted 2 lines

(BTW I had not clue the 68K maintainer had a platform like this, fun!)

Note that this platform is one of those we talked about removing from the kernel for potentially being completely unused. Do we have reason to believe that people are still using ks8695 for anything with new kernels?

If any of you are doing that, we'll definitely keep the port around, the idea is not to break anything that people are actually using. In that case, I would like to see a MAINTAINERS entry for the platform so we have a contact for someone who is able to ack and maybe test patches.

Quoted 5 lines

Note that this platform is one of those we talked about removing from the kernel for potentially being completely unused. Do we have reason to believe that people are still using ks8695 for anything with new kernels?

I'll put my hand up as a user of it. I run every release kernel on KS8695 platforms, though I don't tend to do much testing of rc kernels on it.

Quoted 5 lines

If any of you are doing that, we'll definitely keep the port around, the idea is not to break anything that people are actually using. In that case, I would like to see a MAINTAINERS entry for the platform so we have a contact for someone who is able to ack and maybe test patches.

I am happy enough to ack and test changes, if no one else wants to do it. I don't expect to do any real active development on it though. I do have some board files that would have been nice to push into mainline, but I have been holding off (for quite a while), hoping that someone might support DTs on this sub-arch.

Quoted 9 lines · Greg Ungerer

Quoted 5 lines

Note that this platform is one of those we talked about removing from the kernel for potentially being completely unused. Do we have reason to believe that people are still using ks8695 for anything with new kernels?

I'll put my hand up as a user of it. I run every release kernel on KS8695 platforms, though I don't tend to do much testing of rc kernels on it.

Ok, very good. Booting every new kernel on a platform is more testing than a lot of the older platforms get as far as I can tell.

Quoted 11 lines

Quoted 5 lines

If any of you are doing that, we'll definitely keep the port around, the idea is not to break anything that people are actually using. In that case, I would like to see a MAINTAINERS entry for the platform so we have a contact for someone who is able to ack and maybe test patches.

I am happy enough to ack and test changes, if no one else wants to do it. I don't expect to do any real active development on it though. I do have some board files that would have been nice to push into mainline, but I have been holding off (for quite a while), hoping that someone might support DTs on this sub-arch.

If nobody is actually working on new stuff, we might be better off with just merging the board files you have. If you don't mind, just post what you have and then we can decide what to do with them.

Quoted 2 lines · Linus Walleij

(BTW I had not clue the 68K maintainer had a platform like this, fun!)

Note that this platform is one of those we talked about removing from the kernel for potentially being completely unused.

Quoted 5 lines · Arnd Bergmann

Quoted 2 lines · Linus Walleij

(BTW I had not clue the 68K maintainer had a platform like this, fun!)

Quoted 2 lines

Note that this platform is one of those we talked about removing from the kernel for potentially being completely unused.

This clock stuff was a lackmus test from my side. The idea was that if noone tested the patches I would instead propose a patch deleting the platform.

Guess what: Greg jumps out like jack-in-the-box.

I suspect
some of these platforms may have more users than we believe.

Quoted 3 lines

I suspect
some of these platforms may have more users than we believe.

I know the guys at www.opengear.com are still building hardware around the KS8695. And they update to modern kernels from time to time. So its not completely obsolete yet. I have some SnapGear boards based around it too, though they went out of production about 2 years ago.

Quoted 4 lines · Greg Ungerer

Quoted 3 lines

I suspect
some of these platforms may have more users than we believe.

Quoted 4 lines

I know the guys at www.opengear.com are still building hardware around the KS8695. And they update to modern kernels from time to time. So its not completely obsolete yet. I have some SnapGear boards based around it too, though they went out of production about 2 years ago.

Ok, good to know. Maybe we can catch their interest with this mail thread here and get them to contribute upstream as well.

hi,

Quoted 10 lines · Arnd Bergmann

Quoted 8 lines · Greg Ungerer

Quoted 3 lines

I suspect
some of these platforms may have more users than we believe.

I know the guys at www.opengear.com are still building hardware around the KS8695. And they update to modern kernels from time to time. So its not completely obsolete yet. I have some SnapGear boards based around it too, though they went out of production about 2 years ago.

Quoted 2 lines

Ok, good to know. Maybe we can catch their interest with this mail thread here and get them to contribute upstream as well.

Vision System GmbH are (or were) also using the KS8695.

Quoted 4 lines · Greg Ungerer

Quoted 3 lines

I suspect
some of these platforms may have more users than we believe.

Quoted 4 lines

I know the guys at www.opengear.com are still building hardware around the KS8695. And they update to modern kernels from time to time. So its not completely obsolete yet. I have some SnapGear boards based around it too, though they went out of production about 2 years ago.

Ok, good to know. Maybe we can catch their interest with this mail thread here and get them to contribute upstream as well.