[PATCH 3/4] ARM: Xilinx: Adding timer support to the platform

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Saturday, February 05, 2011 9:36 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; catalin.marinas at arm.com;

glikely at secretlab.ca; Kiran

Sutariya
Subject: Re: [PATCH 3/4] ARM: Xilinx: Adding timer support to the

platform

On Sat, Feb 05, 2011 at 09:08:43AM -0700, John Linn wrote:

quoted

diff --git a/arch/arm/mach-xilinx/timer.c

b/arch/arm/mach-xilinx/timer.c

quoted

new file mode 100644
index 0000000..c713e3c

--- /dev/null
+++ b/arch/arm/mach-xilinx/timer.c

@@ -0,0 +1,452 @@
+/* arch/arm/mach-xilinx/timer.c
+ *
+ * This file contains driver for the Xilinx PS Timer Counter IP.
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * based on arch\mips\kernel\time.c timer driver

Unix directory separators are / not \.

Yes, will fix those.

quoted

+ *
+ * This program is free software; you can redistribute it and/or

modify it under

quoted

+ * the terms of the GNU General Public License version 2 as

published by the

quoted

+ * Free Software Foundation; either version 2 of the License, or

(at your

quoted

+ * option) any later version.
+ *
+ * You should have received a copy of the GNU General Public

License along with

quoted

+ * this program; if not, write to the Free Software Foundation,

Inc., 59 Temple

quoted

+ * Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/types.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/io.h>
+#include <asm/mach/time.h>
+#include <mach/hardware.h>
+
+/*
+ * This driver configures the 2 16-bit count-up timers as follows:
+ *
+ * T1: Timer 1, clocksource for generic timekeeping
+ * T2: Timer 2, clockevent source for hrtimers
+ * T3: Timer 3, <unused>
+ *
+ * The input frequency to the timer module is 200MHz which is

common to all the

quoted

+ * timer channel (T1, T2, and T3)

You should probably mention that you clock the counters themselves at
6.25MHz due to the pre-scaler setting of 32.  200MHz and 16-bit

counters

didn't look too clever when I first read through this code.

Sure.

quoted

+ * Clocksource timer resolution is 160ns
+ * Clockevent timer resolution is 160ns
+ */
+#define XTTCPSS_CLOCKSOURCE	0	/* Timer 1 as a generic

timekeeping */

quoted

+#define XTTCPSS_CLOCKEVENT	1	/* Timer 2 as a clock event */
+
+#define XTTCPSS_TIMER_BASE	TTC0_BASE
+
+/*
+ * Timer Register Offset Definitions of Timer 1, Increment base

address by 4

quoted

+ * and use same offsets for Timer 2
+ */
+#define XTTCPSS_CLK_CNTRL_OFFSET	0x00 /* Clock Control Reg, RW */
+#define XTTCPSS_CNT_CNTRL_OFFSET	0x0C /* Counter Control Reg, RW

*/

quoted

+#define XTTCPSS_COUNT_VAL_OFFSET	0x18 /* Counter Value Reg, RO */
+#define XTTCPSS_INTR_VAL_OFFSET		0x24 /* Interval Count

Reg, RW */

quoted

+#define XTTCPSS_MATCH_1_OFFSET		0x30 /* Match 1 Value

Reg, RW */

quoted

+#define XTTCPSS_MATCH_2_OFFSET		0x3C /* Match 2 Value

Reg, RW */

quoted

+#define XTTCPSS_MATCH_3_OFFSET		0x48 /* Match 3 Value

Reg, RW */

quoted

+#define XTTCPSS_ISR_OFFSET		0x54 /* Interrupt Status Reg, RO

*/

quoted

+#define XTTCPSS_IER_OFFSET		0x60 /* Interrupt Enable Reg, RW

*/

quoted

+
+/*
+ * Bit mask to enable/disable the timer
+ */
+#define XTTCPSS_CNT_CNTRL_ENABLE_MASK	0xFFFFFFFE
+
+/*
+ * Definitions of the timer read/write macro
+ */
+#define xttcpss_read(addr)	__raw_readl((void __iomem *)addr)
+#define xttcpss_write(addr, val) __raw_writel(val, (void __iomem

*)(addr))

quoted

+
+
+/**
+ * struct xttcpss_timer - This definition defines local timer

structure

quoted

+ *
+ * @name:	Name of Timer
+ * @base_addr:	Base address of timer
+ * @timer_irq:	irqaction structure for the timer device
+ * @mode:       only valid for an clock event, periodic or one-shot
+ **/
+struct xttcpss_timer {
+	char *name;

const?

Yep looks like it can be. I'll add it.

quoted

+	unsigned long base_addr;
+	struct irqaction timer_irq;

irqaction also has a const char *name pointer - is it necessary to

duplicate

it above?

I see your point. 

quoted

+	enum clock_event_mode mode;
+};
+
+
+static struct xttcpss_timer timers[];
+static struct clock_event_device xttcpss_clockevent;
+
+/*
+ * xttcpss_timer_irqs - Timers IRQ number
+ */
+static int xttcpss_timer_irqs[2] = {
+	IRQ_TIMERCOUNTER0,	/* Timer 1 IRQ number */
+	IRQ_TIMERCOUNTER0 + 1,	/* Timer 2 IRQ number */
+};
+
+/**
+ * xttcpss_set_interval - Set the timer interval value
+ *
+ * @timer:	Pointer to the timer instance
+ * @cycles:	Timer interval ticks
+ **/
+static void xttcpss_set_interval(struct xttcpss_timer *timer,
+					unsigned long cycles)
+{
+	u32 ctrl_reg;
+
+	/* Disable the counter, set the counter value  and re-enable

counter */

quoted

+	ctrl_reg = xttcpss_read(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET);

quoted

+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,

ctrl_reg);

quoted

+
+	pr_debug("set_interval, name = %s, period = %08X\n",
+			timer->name, (unsigned int)cycles);
+
+	xttcpss_write(timer->base_addr + XTTCPSS_INTR_VAL_OFFSET,

cycles);

quoted

+
+	/* Reset the counter (0x10) so that it starts from 0, one-shot
+	   mode makes this needed for timing to be right. */
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	ctrl_reg |= 0x10;
+	xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,

ctrl_reg);

quoted

+}
+
+/**
+ * xttcpss_clock_source_interrupt - Clock source timer interrupt

handler

quoted

+ *
+ * @irq:	IRQ number of the Timer
+ * @dev_id:	void pointer to the xttcpss_timer instance
+ *
+ * This will be called when 16-bit clock source counter wraps
+ *
+ * returns: Always IRQ_HANDLED - success
+ **/
+static irqreturn_t xttcpss_clock_source_interrupt(int irq, void

*dev_id)

quoted

+{
+	struct xttcpss_timer *timer = dev_id;
+
+	/* Acknowledge the interrupt */
+	xttcpss_write(timer->base_addr + XTTCPSS_ISR_OFFSET,
+		xttcpss_read(timer->base_addr + XTTCPSS_ISR_OFFSET));

Hmm.  Is it necessary to do this to keep the counter running?  If the
counter stops then it's not a good clock source.

I'm going to have to study this a bit more. I see your point. The
interrupt doesn't 
really do anything.  I'll look to see if we can just disable the
interrupt.

quoted

+
+	return IRQ_HANDLED;
+}
+
+/**
+ * xttcpss_clock_event_interrupt - Clock event timer interrupt

handler

quoted

+ *
+ * @irq:	IRQ number of the Timer
+ * @dev_id:	void pointer to the xttcpss_timer instance
+ *
+ * returns: Always IRQ_HANDLED - success
+ **/
+static irqreturn_t xttcpss_clock_event_interrupt(int irq, void

*dev_id)

quoted

+{
+	struct clock_event_device *evt = &xttcpss_clockevent;
+	struct xttcpss_timer *timer = dev_id;
+	u32 ctrl_reg;
+
+	/* Acknowledge the interrupt and call event handler */
+	xttcpss_write(timer->base_addr + XTTCPSS_ISR_OFFSET,
+		xttcpss_read(timer->base_addr + XTTCPSS_ISR_OFFSET));
+
+	if (timer->mode == CLOCK_EVT_MODE_ONESHOT) {
+
+		/* Disable the counter as it would keep running. */
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+		xttcpss_write(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

quoted

+				ctrl_reg);
+	}
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * struct xttcpss_timer timers - This definition defines local

timers

quoted

+ */
+static struct xttcpss_timer timers[] = {
+	[XTTCPSS_CLOCKSOURCE] = {
+		.name = "xttcpss clocksource",
+		.timer_irq = {
+			.flags = IRQF_DISABLED | IRQF_TIMER,
+			.handler = xttcpss_clock_source_interrupt,
+		}
+	},
+	[XTTCPSS_CLOCKEVENT] = {
+		.name = "xttcpss clockevent",
+		.timer_irq = {
+			.flags = IRQF_DISABLED | IRQF_TIMER,
+			.handler = xttcpss_clock_event_interrupt,
+		}
+	},
+};
+
+
+/**
+ * xttcpss_timer_hardware_init - Initialize the timer hardware
+ *
+ * Initialize the hardware, registers the timer interrupts, set the

clock source

quoted

+ * timer interval and enable the clock source timer
+ **/
+static void __init xttcpss_timer_hardware_init(void)
+{
+	int timer_id;
+	struct xttcpss_timer *timer;
+
+	for (timer_id = 0; timer_id < ARRAY_SIZE(timers); timer_id++) {
+		timer = &timers[timer_id];
+
+		if (!(timer->name))
+			continue;
+
+		timer->base_addr = XTTCPSS_TIMER_BASE + (4*timer_id);
+
+		/* Disable counter, Enable Interval mode, Count up

timer,

quoted

+		 * Disable Match mode, Internal Clock source select, set
+		 * prescalar to 32, and Enable the Interval interrupt */

Isn't it "pre-scaler" ?

Ok, easy to fix.

quoted

+		xttcpss_write(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

quoted

+				0x23);
+		xttcpss_write(timer->base_addr +

XTTCPSS_CLK_CNTRL_OFFSET, 0x9);

quoted

+		xttcpss_write(timer->base_addr + XTTCPSS_IER_OFFSET,

0x1);

quoted

+
+		/* Setup IRQ */
+		timer->timer_irq.name = timer->name;
+		timer->timer_irq.dev_id = (void *)timer;
+		if (timer->timer_irq.handler != NULL) {
+			setup_irq(xttcpss_timer_irqs[timer_id],
+				&timer->timer_irq);
+		}
+		if (timer_id == XTTCPSS_CLOCKSOURCE)
+			xttcpss_set_interval(timer, ~0);
+	}
+}
+
+/**
+ * xttcpss_read_cycles - Reads the timer counter register
+ *
+ * returns: Current timer counter register value
+ **/
+static cycle_t xttcpss_read_cycles(struct clocksource *cs)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKSOURCE];
+
+	return (cycle_t)xttcpss_read(timer->base_addr +
+				XTTCPSS_COUNT_VAL_OFFSET);
+}
+
+
+/*
+ * Instantiate and initialize the clock source structure
+ */
+static struct clocksource clocksource_xttcpss = {
+	.name		= "xttcpss_timer1",
+	.rating		= 200,			/* Reasonable clock

source */

quoted

+	.read		= xttcpss_read_cycles,
+	.mask		= CLOCKSOURCE_MASK(16),
+	.shift		= 0,			/* Initialized to zero

*/

quoted

+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+
+/**
+ * xttcpss_set_next_event - Sets the time interval for next event
+ *
+ * @cycles:	Timer interval ticks
+ * @evt:	Address of clock event instance
+ *
+ * returns: Always 0 - success
+ **/
+static int xttcpss_set_next_event(unsigned long cycles,
+					struct clock_event_device *evt)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+
+	xttcpss_set_interval(timer, cycles);
+	return 0;
+}
+
+/**
+ * xttcpss_set_mode - Sets the mode of timer
+ *
+ * @mode:	Mode to be set
+ * @evt:	Address of clock event instance
+ **/
+static void xttcpss_set_mode(enum clock_event_mode mode,
+					struct clock_event_device *evt)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	timer->mode = mode;
+
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		xttcpss_set_interval(timer, CLOCK_TICK_RATE / HZ);
+		break;
+	case CLOCK_EVT_MODE_ONESHOT:
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+		xttcpss_write(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

quoted

+				ctrl_reg);
+		break;
+	case CLOCK_EVT_MODE_RESUME:
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+		xttcpss_write(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

quoted

+				ctrl_reg);
+		break;
+	}
+}
+
+/*
+ * Instantiate and initialize the clock event structure
+ */
+static struct clock_event_device xttcpss_clockevent = {
+	.name		= "xttcpss_timer2",
+	.features	= CLOCK_EVT_FEAT_PERIODIC |

CLOCK_EVT_FEAT_ONESHOT,

quoted

+	.shift		= 0,		/* Initialized to zero */
+	.set_next_event	= xttcpss_set_next_event,
+	.set_mode	= xttcpss_set_mode,
+	.rating		= 200,
+};
+
+/**
+ * xttcpss_timer_init - Initialize the timer
+ *
+ * Initializes the timer hardware and registers the clock source

and clock event

quoted

+ * timers with Linux kernal timer framework
+ **/
+static void __init xttcpss_timer_init(void)
+{
+	u32 shift;
+	u64 temp;
+
+	xttcpss_timer_hardware_init();
+
+	/* Calculate the nanoseconds to cycles divisor value for clock

source

quoted

+	 * timer */
+	for (shift = 16; shift > 0; shift--) {
+		temp = (u64) NSEC_PER_SEC << shift;
+		do_div(temp, CLOCK_TICK_RATE);
+		if ((temp >> 32) == 0)
+			break;
+	}
+
+	/* Setup clocksource */
+	clocksource_xttcpss.shift = shift;
+	clocksource_xttcpss.mult =
+		clocksource_hz2mult(CLOCK_TICK_RATE,

clocksource_xttcpss.shift);

quoted

+
+	if (clocksource_register(&clocksource_xttcpss))
+		printk(KERN_ERR "xttcpss_timer_init: can't register

clocksource"

quoted

+				" for %s\n", clocksource_xttcpss.name);

There's code to calculate the shift and multipler for you.  All you
need do is replace the above with clocksource_register_hz() or
clocksource_register_khz().

Ok, will look at that.

quoted

+	/* Calculate the nanoseconds to cycles divisor value for clock

event

quoted

+	 * timer */
+	for (shift = 16; shift > 0; shift--) {
+		temp = (u64) CLOCK_TICK_RATE << shift;
+		do_div(temp, NSEC_PER_SEC);
+		if ((temp >> 32) == 0)
+			break;
+	}
+
+	/* Setup clockevent */
+	xttcpss_clockevent.shift = shift;
+	xttcpss_clockevent.mult = div_sc(CLOCK_TICK_RATE, NSEC_PER_SEC,
+

xttcpss_clockevent.shift);

clockevents_calc_mult_shift(&xttcpss_clockevent, freq, minsec) should
do away with all the above, here freq is the clock rate for the
actual counter itself (after the pre-scaler.)

Sounds good.

quoted

+
+	xttcpss_clockevent.max_delta_ns =
+		clockevent_delta2ns(0xfffe, &xttcpss_clockevent);
+	xttcpss_clockevent.min_delta_ns =
+		clockevent_delta2ns(1, &xttcpss_clockevent);
+
+	xttcpss_clockevent.cpumask = cpumask_of(0);
+	clockevents_register_device(&xttcpss_clockevent);
+}
+
+#ifdef CONFIG_PM
+/**
+ * xttcpss_timer_suspend - Suspend the timer
+ *
+ * Disables all (clock source and clock event) the timers
+ **/
+static void xttcpss_timer_suspend(void)
+{
+	struct xttcpss_timer *source_timer =

&timers[XTTCPSS_CLOCKSOURCE];

quoted

+	struct xttcpss_timer *event_timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	/* Disable clocksource timer */
+	ctrl_reg = xttcpss_read(source_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(source_timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

quoted

+				ctrl_reg);
+
+	/* Disable clockevent timer */
+	ctrl_reg = xttcpss_read(event_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(event_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+}
+
+/**
+ * xttcpss_timer_resume - Resume the timer
+ *
+ * Enables  all (clock source and clock event) the timers
+ **/
+static void xttcpss_timer_resume(void)
+{
+	struct xttcpss_timer *source_timer =

&timers[XTTCPSS_CLOCKSOURCE];

quoted

+	struct xttcpss_timer *event_timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	/* Enable clocksource timer */
+	ctrl_reg = xttcpss_read(source_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	xttcpss_write(source_timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

quoted

+				ctrl_reg);
+
+	/* Enable clockevent timer */
+	ctrl_reg = xttcpss_read(event_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	xttcpss_write(event_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+}

The generic time code should do PM for you.  In fact, if you're using
generic time code, these callbacks will never be used, as the code
which calls them is surrounded by:

#if defined(CONFIG_PM) && !defined(CONFIG_GENERIC_CLOCKEVENTS)

Ok, will remove.

Thanks, will update based on these.

-- John

quoted

+#else
+#define xttcpss_timer_suspend NULL
+#define xttcpss_timer_resume NULL
+#endif
+
+/*
+ * Instantiate and initialize the system timer structure
+ */
+struct sys_timer xttcpss_sys_timer = {
+	.init		= xttcpss_timer_init,
+	.suspend	= xttcpss_timer_suspend,
+	.resume		= xttcpss_timer_resume,
+};

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.