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]>

---
ChangeLog v2->v3:
- Correct the MMIO timer read function: this timer counts DOWN not up - since clocksources shall move forward in time, the ever decreasing clocksource was likely ignored.
ChangeLog v1->v2:
- Corrected the mask when setting clockevents from & TMCON_T1EN to & ~TMCON_T1EN so it doesn't shut down the clock source.

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

arch/arm/Kconfig +2 -1

index 6d6e18f..e9ce038 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 +66 -34

index 6974c23..e603478 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,41 @@ 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.

Despite what the old code had, reading the T0TC (or the T0PD) does not return the current count. It just returns what you set it too.

As far
as I can tell there is no programmer access to the clocks internal counter on the KS8695. The data sheet doesn't seem to indicate any way to access it.

If I disable the clocksource_mmio_init section of code above then I get working time again. I suspect that is as good as we can get on the K8695.

Quoted 2 lines · Greg Ungerer

Despite what the old code had, reading the T0TC (or the T0PD) does not return the current count. It just returns what you set it too.

Aha!

Quoted 4 lines

As far
as I can tell there is no programmer access to the clocks internal counter on the KS8695. The data sheet doesn't seem to indicate any way to access it.

Hm, yeah the datasheet doesn't quite tell what is read back from that register, strange about the old code though, it's coded as if it could be read back :-/

Quoted 3 lines

If I disable the clocksource_mmio_init section of code above then I get working time again. I suspect that is as good as we can get on the K8695.

OK let me provide a patch that just does clocksource then. Let's just fall back to jiffies.

Yours,
Linus Walleij