diff mbox

[v9,1/9] clocksource: time-armada-370-xp: Marvell Armada 370/XP SoC timer driver

Message ID 1341588221-3822-2-git-send-email-thomas.petazzoni@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Petazzoni July 6, 2012, 3:23 p.m. UTC
From: Gregory Clement <gregory.clement@free-electrons.com>

Timer 0 is used as free-running clocksource, while timer 1 is used as
clock_event_device.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Lior Amsalem <alior@marvell.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Yehuda Yitschak <yehuday@marvell.com>
Tested-by: Lior Amsalem <alior@marvell.com>
Acked-by: Andrew Lunn <andrew@lunn.ch>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: John Stultz <johnstul@us.ibm.com>
---
 drivers/clocksource/Kconfig              |    3 +
 drivers/clocksource/Makefile             |    3 +-
 drivers/clocksource/time-armada-370-xp.c |  226 ++++++++++++++++++++++++++++++
 include/linux/time-armada-370-xp.h       |   18 +++
 4 files changed, 249 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clocksource/time-armada-370-xp.c
 create mode 100644 include/linux/time-armada-370-xp.h

Comments

John Stultz July 18, 2012, 11:48 p.m. UTC | #1
On 07/06/2012 08:23 AM, Thomas Petazzoni wrote:
> From: Gregory Clement <gregory.clement@free-electrons.com>
>
> Timer 0 is used as free-running clocksource, while timer 1 is used as
> clock_event_device.

I don't see the any clocksource in this patch.  Why is this tagged 
clocksource?

I'm also not a huge fan of adding clockevent drivers to 
drivers/clocksource/.

Further, LinusW and I have different opinions on this (and its not that 
huge of an issue), but I'd really prefer to see additions to 
drivers/clocksource be only for clocksources that are likely to be 
shared *between architectures*.

Similarly, I'd prefer architecture specific clocksources (like TSC on 
x86,  timebase on ppc, etc) stay in the arch subdir, just so theirs a 
clear ownership of the driver (ie: if in 10 years specific hw support is 
dropped from the arch directory, we don't end up with zombie drivers 
that live on because generic driver maintainers don't know what hardware 
they're actually connected to).

(But I'm somewhat flexible on this last point, as long as there really 
is a chance it might be shared at some point)

thanks
-john
John Stultz July 18, 2012, 11:51 p.m. UTC | #2
On 07/18/2012 04:48 PM, John Stultz wrote:
> On 07/06/2012 08:23 AM, Thomas Petazzoni wrote:
>> From: Gregory Clement <gregory.clement@free-electrons.com>
>>
>> Timer 0 is used as free-running clocksource, while timer 1 is used as
>> clock_event_device.
>
> I don't see the any clocksource in this patch.  Why is this tagged 
> clocksource?

Whoops. Now of course I see the clocksource_mmio_init().  Hrmm..

> I'm also not a huge fan of adding clockevent drivers to 
> drivers/clocksource/.

Still have mixed feelings on this point.

thanks
-john
Nicolas Pitre July 19, 2012, 1:41 a.m. UTC | #3
On Wed, 18 Jul 2012, John Stultz wrote:

> I'm also not a huge fan of adding clockevent drivers to drivers/clocksource/.
> 
> Further, LinusW and I have different opinions on this (and its not that huge
> of an issue), but I'd really prefer to see additions to drivers/clocksource be
> only for clocksources that are likely to be shared *between architectures*.

This is contrary to the push we've made for about 2 years now. We 
decided it is more beneficial to gather drivers according to their 
purpose and not according to the architecture they belong to.  The 
reason is that it is far easier to abstract common functionality if 
those drivers are close by.

> Similarly, I'd prefer architecture specific clocksources (like TSC on x86,
> timebase on ppc, etc) stay in the arch subdir, just so theirs a clear
> ownership of the driver (ie: if in 10 years specific hw support is dropped
> from the arch directory, we don't end up with zombie drivers that live on
> because generic driver maintainers don't know what hardware they're actually
> connected to).

The same argument was said about cpufreq drivers, and cpuidle drivers, 
and IRQ controller drivers, etc.  They ended up in a common directory 
anyway.  In the end this helps generic driver maintainers because you 
don't end up with the same infrastructure copied over and over, each 
time with slight alteration, making any common API maintenance a 
nightmare.

Zombie drivers shouldn't be a huge issue when no one selects their 
kconfig symbol.  On the other hand, drivers burried under various 
architecture directories are hard to maintain and abstract (ask tglx 
about his IRQ driver changes).

> (But I'm somewhat flexible on this last point, as long as there really is a
> chance it might be shared at some point)

I think that common purpose is more important a criteria than 
sharability.  It's hard to say in advance if a piece of code might be 
shared or not.  On the other hand, its purpose should be rather clear.


Nicolas
John Stultz July 19, 2012, 3:57 a.m. UTC | #4
On 07/18/2012 06:41 PM, Nicolas Pitre wrote:
> On Wed, 18 Jul 2012, John Stultz wrote:
>
>> I'm also not a huge fan of adding clockevent drivers to drivers/clocksource/.
>>
>> Further, LinusW and I have different opinions on this (and its not that huge
>> of an issue), but I'd really prefer to see additions to drivers/clocksource be
>> only for clocksources that are likely to be shared *between architectures*.
> This is contrary to the push we've made for about 2 years now. We
> decided it is more beneficial to gather drivers according to their
> purpose and not according to the architecture they belong to.  The
> reason is that it is far easier to abstract common functionality if
> those drivers are close by.
>
>> Similarly, I'd prefer architecture specific clocksources (like TSC on x86,
>> timebase on ppc, etc) stay in the arch subdir, just so theirs a clear
>> ownership of the driver (ie: if in 10 years specific hw support is dropped
>> from the arch directory, we don't end up with zombie drivers that live on
>> because generic driver maintainers don't know what hardware they're actually
>> connected to).
> The same argument was said about cpufreq drivers, and cpuidle drivers,
> and IRQ controller drivers, etc.  They ended up in a common directory
> anyway.  In the end this helps generic driver maintainers because you
> don't end up with the same infrastructure copied over and over, each
> time with slight alteration, making any common API maintenance a
> nightmare.
>
> Zombie drivers shouldn't be a huge issue when no one selects their
> kconfig symbol.  On the other hand, drivers burried under various
> architecture directories are hard to maintain and abstract (ask tglx
> about his IRQ driver changes).
>
>> (But I'm somewhat flexible on this last point, as long as there really is a
>> chance it might be shared at some point)
> I think that common purpose is more important a criteria than
> sharability.  It's hard to say in advance if a piece of code might be
> shared or not.  On the other hand, its purpose should be rather clear.

Alright, alright,  I'll just grumble along with it then. :)
-john
diff mbox

Patch

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 99c6b20..b323631 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -16,6 +16,9 @@  config CLKSRC_MMIO
 config DW_APB_TIMER
 	bool
 
+config ARMADA_370_XP_TIMER
+	bool
+
 config CLKSRC_DBX500_PRCMU
 	bool "Clocksource PRCMU Timer"
 	depends on UX500_SOC_DB8500
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index dd3e661..022015c 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -10,4 +10,5 @@  obj-$(CONFIG_EM_TIMER_STI)	+= em_sti.o
 obj-$(CONFIG_CLKBLD_I8253)	+= i8253.o
 obj-$(CONFIG_CLKSRC_MMIO)	+= mmio.o
 obj-$(CONFIG_DW_APB_TIMER)	+= dw_apb_timer.o
-obj-$(CONFIG_CLKSRC_DBX500_PRCMU)	+= clksrc-dbx500-prcmu.o
\ No newline at end of file
+obj-$(CONFIG_CLKSRC_DBX500_PRCMU)	+= clksrc-dbx500-prcmu.o
+obj-$(CONFIG_ARMADA_370_XP_TIMER)	+= time-armada-370-xp.o
diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
new file mode 100644
index 0000000..4674f94
--- /dev/null
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -0,0 +1,226 @@ 
+/*
+ * Marvell Armada 370/XP SoC timer handling.
+ *
+ * Copyright (C) 2012 Marvell
+ *
+ * Lior Amsalem <alior@marvell.com>
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ *
+ * Timer 0 is used as free-running clocksource, while timer 1 is
+ * used as clock_event_device.
+ */
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/timer.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <asm/sched_clock.h>
+
+/*
+ * Timer block registers.
+ */
+#define TIMER_CTRL_OFF		0x0000
+#define  TIMER0_EN		 0x0001
+#define  TIMER0_RELOAD_EN	 0x0002
+#define  TIMER0_25MHZ            0x0800
+#define  TIMER0_DIV(div)         ((div) << 19)
+#define  TIMER1_EN		 0x0004
+#define  TIMER1_RELOAD_EN	 0x0008
+#define  TIMER1_25MHZ            0x1000
+#define  TIMER1_DIV(div)         ((div) << 22)
+#define TIMER_EVENTS_STATUS	0x0004
+#define  TIMER0_CLR_MASK         (~0x1)
+#define  TIMER1_CLR_MASK         (~0x100)
+#define TIMER0_RELOAD_OFF	0x0010
+#define TIMER0_VAL_OFF		0x0014
+#define TIMER1_RELOAD_OFF	0x0018
+#define TIMER1_VAL_OFF		0x001c
+
+/* Global timers are connected to the coherency fabric clock, and the
+   below divider reduces their incrementing frequency. */
+#define TIMER_DIVIDER_SHIFT     5
+#define TIMER_DIVIDER           (1 << TIMER_DIVIDER_SHIFT)
+
+/*
+ * SoC-specific data.
+ */
+static void __iomem *timer_base;
+static int timer_irq;
+
+/*
+ * Number of timer ticks per jiffy.
+ */
+static u32 ticks_per_jiffy;
+
+static u32 notrace armada_370_xp_read_sched_clock(void)
+{
+	return ~readl(timer_base + TIMER0_VAL_OFF);
+}
+
+/*
+ * Clockevent handling.
+ */
+static int
+armada_370_xp_clkevt_next_event(unsigned long delta,
+				struct clock_event_device *dev)
+{
+	u32 u;
+
+	/*
+	 * Clear clockevent timer interrupt.
+	 */
+	writel(TIMER1_CLR_MASK, timer_base + TIMER_EVENTS_STATUS);
+
+	/*
+	 * Setup new clockevent timer value.
+	 */
+	writel(delta, timer_base + TIMER1_VAL_OFF);
+
+	/*
+	 * Enable the timer.
+	 */
+	u = readl(timer_base + TIMER_CTRL_OFF);
+	u = ((u & ~TIMER1_RELOAD_EN) | TIMER1_EN |
+	     TIMER1_DIV(TIMER_DIVIDER_SHIFT));
+	writel(u, timer_base + TIMER_CTRL_OFF);
+
+	return 0;
+}
+
+static void
+armada_370_xp_clkevt_mode(enum clock_event_mode mode,
+			  struct clock_event_device *dev)
+{
+	u32 u;
+
+	if (mode == CLOCK_EVT_MODE_PERIODIC) {
+		/*
+		 * Setup timer to fire at 1/HZ intervals.
+		 */
+		writel(ticks_per_jiffy - 1, timer_base + TIMER1_RELOAD_OFF);
+		writel(ticks_per_jiffy - 1, timer_base + TIMER1_VAL_OFF);
+
+		/*
+		 * Enable timer.
+		 */
+		u = readl(timer_base + TIMER_CTRL_OFF);
+
+		writel((u | TIMER1_EN | TIMER1_RELOAD_EN |
+			TIMER1_DIV(TIMER_DIVIDER_SHIFT)),
+		       timer_base + TIMER_CTRL_OFF);
+	} else {
+		/*
+		 * Disable timer.
+		 */
+		u = readl(timer_base + TIMER_CTRL_OFF);
+		writel(u & ~TIMER1_EN, timer_base + TIMER_CTRL_OFF);
+
+		/*
+		 * ACK pending timer interrupt.
+		 */
+		writel(TIMER1_CLR_MASK, timer_base + TIMER_EVENTS_STATUS);
+
+	}
+}
+
+static struct clock_event_device armada_370_xp_clkevt = {
+	.name		= "armada_370_xp_tick",
+	.features	= CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
+	.shift		= 32,
+	.rating		= 300,
+	.set_next_event	= armada_370_xp_clkevt_next_event,
+	.set_mode	= armada_370_xp_clkevt_mode,
+};
+
+static irqreturn_t armada_370_xp_timer_interrupt(int irq, void *dev_id)
+{
+	/*
+	 * ACK timer interrupt and call event handler.
+	 */
+
+	writel(TIMER1_CLR_MASK, timer_base + TIMER_EVENTS_STATUS);
+	armada_370_xp_clkevt.event_handler(&armada_370_xp_clkevt);
+
+	return IRQ_HANDLED;
+}
+
+static struct irqaction armada_370_xp_timer_irq = {
+	.name		= "armada_370_xp_tick",
+	.flags		= IRQF_DISABLED | IRQF_TIMER,
+	.handler	= armada_370_xp_timer_interrupt
+};
+
+void __init armada_370_xp_timer_init(void)
+{
+	u32 u;
+	struct device_node *np;
+	unsigned int timer_clk;
+	int ret;
+	np = of_find_compatible_node(NULL, NULL, "marvell,armada-370-xp-timer");
+	timer_base = of_iomap(np, 0);
+	WARN_ON(!timer_base);
+
+	if (of_find_property(np, "marvell,timer-25Mhz", NULL)) {
+		/* The fixed 25MHz timer is available so let's use it */
+		u = readl(timer_base + TIMER_CTRL_OFF);
+		writel(u | TIMER0_25MHZ | TIMER1_25MHZ,
+		       timer_base + TIMER_CTRL_OFF);
+		timer_clk = 25000000;
+	} else {
+		u32 clk = 0;
+		ret = of_property_read_u32(np, "clock-frequency", &clk);
+		WARN_ON(!clk || ret < 0);
+		u = readl(timer_base + TIMER_CTRL_OFF);
+		writel(u & ~(TIMER0_25MHZ | TIMER1_25MHZ),
+		       timer_base + TIMER_CTRL_OFF);
+		timer_clk = clk / TIMER_DIVIDER;
+	}
+
+	/* We use timer 0 as clocksource, and timer 1 for
+	   clockevents */
+	timer_irq = irq_of_parse_and_map(np, 1);
+
+	ticks_per_jiffy = (timer_clk + HZ / 2) / HZ;
+
+	/*
+	 * Set scale and timer for sched_clock.
+	 */
+	setup_sched_clock(armada_370_xp_read_sched_clock, 32, timer_clk);
+
+	/*
+	 * Setup free-running clocksource timer (interrupts
+	 * disabled).
+	 */
+	writel(0xffffffff, timer_base + TIMER0_VAL_OFF);
+	writel(0xffffffff, timer_base + TIMER0_RELOAD_OFF);
+
+	u = readl(timer_base + TIMER_CTRL_OFF);
+
+	writel((u | TIMER0_EN | TIMER0_RELOAD_EN |
+		TIMER0_DIV(TIMER_DIVIDER_SHIFT)), timer_base + TIMER_CTRL_OFF);
+
+	clocksource_mmio_init(timer_base + TIMER0_VAL_OFF,
+			      "armada_370_xp_clocksource",
+			      timer_clk, 300, 32, clocksource_mmio_readl_down);
+
+	/*
+	 * Setup clockevent timer (interrupt-driven).
+	 */
+	setup_irq(timer_irq, &armada_370_xp_timer_irq);
+	armada_370_xp_clkevt.cpumask = cpumask_of(0);
+	clockevents_config_and_register(&armada_370_xp_clkevt,
+					timer_clk, 1, 0xfffffffe);
+}
+
diff --git a/include/linux/time-armada-370-xp.h b/include/linux/time-armada-370-xp.h
new file mode 100644
index 0000000..dfdfdc0
--- /dev/null
+++ b/include/linux/time-armada-370-xp.h
@@ -0,0 +1,18 @@ 
+/*
+ * Marvell Armada 370/XP SoC timer handling.
+ *
+ * Copyright (C) 2012 Marvell
+ *
+ * Lior Amsalem <alior@marvell.com>
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+ *
+ */
+#ifndef __TIME_ARMADA_370_XPPRCMU_H
+#define __TIME_ARMADA_370_XPPRCMU_H
+
+#include <linux/init.h>
+
+void __init armada_370_xp_timer_init(void);
+
+#endif