[PATCH v2 0/4] A7/A15 architected timer support

[PATCH v2 0/4] A7/A15 architected timer support

[Marc Zyngier]

This patch series adds support for the architected timers found in
Cortex-A7 and Cortex-A15. It adds local timer support, sched_clock,
global timer and DT support.

Tested on Cortex A15, based on Russell's for-armsoc branch plus my
local timer series.

Branch also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git arch_timers-for-rmk

* From the initial revision:
- Fixed global timer registration when an SMP kernel is running on UP
  with LOCAL_TIMERS enabled.

Marc Zyngier (4):
  ARM: local timers: Add A15 architected timer support
  ARM: architected timers: Add A15 specific sched_clock implementation
  ARM: architected timers: add DT support
  ARM: architected timers: add support for UP timer

 .../devicetree/bindings/arm/arch_timer.txt         |   23 ++
 arch/arm/Kconfig                                   |    7 +
 arch/arm/include/asm/arch_timer.h                  |   13 +
 arch/arm/kernel/Makefile                           |    1 +
 arch/arm/kernel/arch_timer.c                       |  363 ++++++++++++++++++++
 5 files changed, 407 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/arch_timer.txt
 create mode 100644 arch/arm/include/asm/arch_timer.h
 create mode 100644 arch/arm/kernel/arch_timer.c

-- 
1.7.7.1

[PATCH v2 1/4] ARM: local timers: Add A15 architected timer support

[Marc Zyngier]

Add support for the A15 generic timer and clocksource.
As the timer generates interrupts on a different PPI depending
on the execution mode (normal or secure), it is possible to
register two different PPIs.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/Kconfig                  |    7 +
 arch/arm/include/asm/arch_timer.h |   12 ++
 arch/arm/kernel/Makefile          |    1 +
 arch/arm/kernel/arch_timer.c      |  286 +++++++++++++++++++++++++++++++++++++
 4 files changed, 306 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/arch_timer.h
 create mode 100644 arch/arm/kernel/arch_timer.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 24626b0..acf2813 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1522,6 +1522,13 @@ config HAVE_ARM_SCU
 	help
 	  This option enables support for the ARM system coherency unit
 
+config ARM_ARCH_TIMER
+	bool "Architected timer support"
+	depends on CPU_V7
+	select TICK_ONESHOT
+	help
+	  This option enables support for the ARM architected timer
+
 config HAVE_ARM_TWD
 	bool
 	depends on SMP
diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
new file mode 100644
index 0000000..20de858
--- /dev/null
+++ b/arch/arm/include/asm/arch_timer.h
@@ -0,0 +1,12 @@
+#ifndef __ASMARM_ARCH_TIMER_H
+#define __ASMARM_ARCH_TIMER_H
+
+#include <linux/ioport.h>
+
+struct arch_timer {
+	struct resource	res[2];
+};
+
+int arch_timer_register(struct arch_timer *);
+
+#endif
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 43b740d..518cea4 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_HAVE_SCHED_CLOCK)	+= sched_clock.o
 obj-$(CONFIG_SMP)		+= smp.o smp_tlb.o
 obj-$(CONFIG_HAVE_ARM_SCU)	+= smp_scu.o
 obj-$(CONFIG_HAVE_ARM_TWD)	+= smp_twd.o
+obj-$(CONFIG_ARM_ARCH_TIMER)	+= arch_timer.o
 obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o
 obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
new file mode 100644
index 0000000..680cf1e
--- /dev/null
+++ b/arch/arm/kernel/arch_timer.c
@@ -0,0 +1,286 @@
+/*
+ *  linux/arch/arm/kernel/arch_timer.c
+ *
+ *  Copyright (C) 2011 ARM Ltd.
+ *  All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/smp.h>
+#include <linux/cpu.h>
+#include <linux/jiffies.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+
+#include <asm/cputype.h>
+#include <asm/localtimer.h>
+#include <asm/arch_timer.h>
+
+static unsigned long arch_timer_rate;
+static int arch_timer_ppi;
+static int arch_timer_ppi2;
+
+static struct clock_event_device __percpu **arch_timer_evt;
+
+/*
+ * Architected system timer support.
+ */
+
+#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
+#define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
+
+#define ARCH_TIMER_REG_CTRL		0
+#define ARCH_TIMER_REG_FREQ		1
+#define ARCH_TIMER_REG_TVAL		2
+
+static void arch_timer_reg_write(int reg, u32 val)
+{
+	switch (reg) {
+	case ARCH_TIMER_REG_CTRL:
+		asm volatile("mcr p15, 0, %0, c14, c2, 1" : : "r" (val));
+		break;
+	case ARCH_TIMER_REG_TVAL:
+		asm volatile("mcr p15, 0, %0, c14, c2, 0" : : "r" (val));
+		break;
+	}
+
+	isb();
+}
+
+static u32 arch_timer_reg_read(int reg)
+{
+	u32 val;
+
+	switch (reg) {
+	case ARCH_TIMER_REG_CTRL:
+		asm volatile("mrc p15, 0, %0, c14, c2, 1" : "=r" (val));
+		break;
+	case ARCH_TIMER_REG_FREQ:
+		asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (val));
+		break;
+	case ARCH_TIMER_REG_TVAL:
+		asm volatile("mrc p15, 0, %0, c14, c2, 0" : "=r" (val));
+		break;
+	default:
+		BUG();
+	}
+
+	return val;
+}
+
+static irqreturn_t arch_timer_handler(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+	unsigned long ctrl;
+
+	ctrl = arch_timer_reg_read(ARCH_TIMER_REG_CTRL);
+	if (ctrl & 0x4) {
+		ctrl |= ARCH_TIMER_CTRL_IT_MASK;
+		arch_timer_reg_write(ARCH_TIMER_REG_CTRL, ctrl);
+		evt->event_handler(evt);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static void arch_timer_disable(void)
+{
+	unsigned long ctrl;
+
+	ctrl = arch_timer_reg_read(ARCH_TIMER_REG_CTRL);
+	ctrl &= ~ARCH_TIMER_CTRL_ENABLE;
+	arch_timer_reg_write(ARCH_TIMER_REG_CTRL, ctrl);
+}
+
+static void arch_timer_set_mode(enum clock_event_mode mode,
+				struct clock_event_device *clk)
+{
+	switch (mode) {
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		arch_timer_disable();
+		break;
+	default:
+		break;
+	}
+}
+
+static int arch_timer_set_next_event(unsigned long evt,
+				     struct clock_event_device *unused)
+{
+	unsigned long ctrl;
+
+	ctrl = arch_timer_reg_read(ARCH_TIMER_REG_CTRL);
+	ctrl |= ARCH_TIMER_CTRL_ENABLE;
+	ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
+
+	arch_timer_reg_write(ARCH_TIMER_REG_TVAL, evt);
+	arch_timer_reg_write(ARCH_TIMER_REG_CTRL, ctrl);
+
+	return 0;
+}
+
+static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
+{
+	/* Be safe... */
+	arch_timer_disable();
+
+	clk->features = CLOCK_EVT_FEAT_ONESHOT;
+	clk->name = "arch_sys_timer";
+	clk->rating = 450;
+	clk->set_mode = arch_timer_set_mode;
+	clk->set_next_event = arch_timer_set_next_event;
+	clk->irq = arch_timer_ppi;
+
+	clockevents_config_and_register(clk, arch_timer_rate,
+					0xf, 0x7fffffff);
+
+	*__this_cpu_ptr(arch_timer_evt) = clk;
+
+	enable_percpu_irq(clk->irq, 0);
+	if (arch_timer_ppi2)
+		enable_percpu_irq(arch_timer_ppi2, 0);
+
+	return 0;
+}
+
+/* Is the optional system timer available? */
+static int local_timer_is_architected(void)
+{
+	return (cpu_architecture() >= CPU_ARCH_ARMv7) &&
+	       ((read_cpuid_ext(CPUID_EXT_PFR1) >> 16) & 0xf) == 1;
+}
+
+static int arch_timer_available(void)
+{
+	unsigned long freq;
+
+	if (!local_timer_is_architected())
+		return -ENXIO;
+
+	if (arch_timer_rate == 0) {
+		arch_timer_reg_write(ARCH_TIMER_REG_CTRL, 0);
+		freq = arch_timer_reg_read(ARCH_TIMER_REG_FREQ);
+
+		/* Check the timer frequency. */
+		if (freq == 0) {
+			pr_warn("Architected timer frequency not available\n");
+			return -EINVAL;
+		}
+
+		arch_timer_rate = freq;
+		pr_info("Architected local timer running at %lu.%02luMHz.\n",
+			arch_timer_rate / 1000000, (arch_timer_rate / 10000) % 100);
+	}
+
+	return 0;
+}
+
+static inline cycle_t arch_counter_get_cntpct(void)
+{
+	u32 cvall, cvalh;
+
+	asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
+
+	return ((u64) cvalh << 32) | cvall;
+}
+
+static inline cycle_t arch_counter_get_cntvct(void)
+{
+	u32 cvall, cvalh;
+
+	asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
+
+	return ((u64) cvalh << 32) | cvall;
+}
+
+static cycle_t arch_counter_read(struct clocksource *cs)
+{
+	return arch_counter_get_cntpct();
+}
+
+static struct clocksource clocksource_counter = {
+	.name	= "arch_sys_counter",
+	.rating	= 400,
+	.read	= arch_counter_read,
+	.mask	= CLOCKSOURCE_MASK(56),
+	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+static void __cpuinit arch_timer_stop(struct clock_event_device *clk)
+{
+	pr_debug("arch_timer_teardown disable IRQ%d cpu #%d\n",
+		 clk->irq, smp_processor_id());
+	disable_percpu_irq(clk->irq);
+	if (arch_timer_ppi2)
+		disable_percpu_irq(arch_timer_ppi2);
+	arch_timer_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+}
+
+static struct local_timer_ops arch_timer_ops __cpuinitdata = {
+	.setup	= arch_timer_setup,
+	.stop	= arch_timer_stop,
+};
+
+int __init arch_timer_register(struct arch_timer *at)
+{
+	int err;
+
+	if (at->res[0].start <= 0 || !(at->res[0].flags & IORESOURCE_IRQ))
+		return -EINVAL;
+
+	err = arch_timer_available();
+	if (err)
+		return err;
+
+	arch_timer_evt = alloc_percpu(struct clock_event_device *);
+	if (!arch_timer_evt)
+		return -ENOMEM;
+
+	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
+
+	arch_timer_ppi = at->res[0].start;
+	err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
+				 "arch_timer", arch_timer_evt);
+	if (err) {
+		pr_err("arch_timer: can't register interrupt %d (%d)\n",
+		       arch_timer_ppi, err);
+		goto out_free;
+	}
+
+	if (at->res[1].start > 0 || (at->res[1].flags & IORESOURCE_IRQ)) {
+		arch_timer_ppi2 = at->res[1].start;
+		err = request_percpu_irq(arch_timer_ppi2, arch_timer_handler,
+					 "arch_timer", arch_timer_evt);
+		if (err) {
+			pr_err("arch_timer: can't register interrupt %d (%d)\n",
+			       arch_timer_ppi2, err);
+			arch_timer_ppi2 = 0;
+			goto out_free_irq;
+		}
+	}
+
+	err = local_timer_register(&arch_timer_ops);
+	if (err)
+		goto out_free_irq;
+
+	return 0;
+
+out_free_irq:
+	free_percpu_irq(arch_timer_ppi, arch_timer_evt);
+	if (arch_timer_ppi2)
+		free_percpu_irq(arch_timer_ppi2, arch_timer_evt);
+	
+out_free:
+	free_percpu(arch_timer_evt);
+
+	return err;
+}
-- 
1.7.7.1

[PATCH v2 2/4] ARM: architected timers: Add A15 specific sched_clock implementation

[Marc Zyngier]

Provide an A15 sched_clock implementation using the virtual counter,
which is thought to be more useful than the physical one in a
virtualised environment, as it can offset the time spent in another
VM or the hypervisor.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/arch_timer.h |    1 +
 arch/arm/kernel/arch_timer.c      |   25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 20de858..ca0f783 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -8,5 +8,6 @@ struct arch_timer {
 };
 
 int arch_timer_register(struct arch_timer *);
+int arch_timer_sched_clock_init(void);
 
 #endif
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 680cf1e..1a203f0 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -22,6 +22,7 @@
 #include <asm/cputype.h>
 #include <asm/localtimer.h>
 #include <asm/arch_timer.h>
+#include <asm/sched_clock.h>
 
 static unsigned long arch_timer_rate;
 static int arch_timer_ppi;
@@ -202,6 +203,18 @@ static inline cycle_t arch_counter_get_cntvct(void)
 	return ((u64) cvalh << 32) | cvall;
 }
 
+static u32 notrace arch_counter_get_cntvct32(void)
+{
+	cycle_t cntvct = arch_counter_get_cntvct();
+
+	/*
+	 * The sched_clock infrastructure only knows about counters
+	 * with@most 32bits. Forget about the upper 24 bits for the
+	 * time being...
+	 */
+	return (u32)(cntvct & (u32)~0);
+}
+
 static cycle_t arch_counter_read(struct clocksource *cs)
 {
 	return arch_counter_get_cntpct();
@@ -284,3 +297,15 @@ out_free:
 
 	return err;
 }
+
+int arch_timer_sched_clock_init(void)
+{
+	int err;
+
+	err = arch_timer_available();
+	if (err)
+		return err;
+
+	setup_sched_clock(arch_counter_get_cntvct32, 32, arch_timer_rate);
+	return 0;
+}
-- 
1.7.7.1

[PATCH v2 3/4] ARM: architected timers: add DT support

[Marc Zyngier]

Add runtime DT support and documentation for the Cortex A7/A15
architected timers.

Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 .../devicetree/bindings/arm/arch_timer.txt         |   23 +++++++++
 arch/arm/kernel/arch_timer.c                       |   53 +++++++++++++++++---
 2 files changed, 69 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/arch_timer.txt

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
new file mode 100644
index 0000000..84e93f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -0,0 +1,23 @@
+* ARM architected timer
+
+ARM Cortex-A7 and Cortex-A15 have a per-core architected timer, which
+provides a per-cpu local timer.
+
+The timer is attached to a GIC to deliver its two per-processor
+interrupts (one for the secure mode, one for the non-secure mode).
+
+** Timer node properties:
+
+- compatible : Should be "arm,armv7-timer"
+
+- interrupts : One or two interrupts for secure and non-secure mode
+
+- clock-frequency : The frequency of the main counter, in Hz. Optionnal.
+
+Example:
+
+	timer {
+		compatible = "arm,armv7-timer"";
+		interrupts = <1 13 0xf08 1 14 0xf08>;
+		clock-frequency = <100000000>;
+	};
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 1a203f0..6cb7d39 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -17,6 +17,7 @@
 #include <linux/jiffies.h>
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
+#include <linux/of_irq.h>
 #include <linux/io.h>
 
 #include <asm/cputype.h>
@@ -243,13 +244,10 @@ static struct local_timer_ops arch_timer_ops __cpuinitdata = {
 	.stop	= arch_timer_stop,
 };
 
-int __init arch_timer_register(struct arch_timer *at)
+static int __init arch_timer_common_register(void)
 {
 	int err;
 
-	if (at->res[0].start <= 0 || !(at->res[0].flags & IORESOURCE_IRQ))
-		return -EINVAL;
-
 	err = arch_timer_available();
 	if (err)
 		return err;
@@ -260,7 +258,6 @@ int __init arch_timer_register(struct arch_timer *at)
 
 	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
 
-	arch_timer_ppi = at->res[0].start;
 	err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
 				 "arch_timer", arch_timer_evt);
 	if (err) {
@@ -269,8 +266,7 @@ int __init arch_timer_register(struct arch_timer *at)
 		goto out_free;
 	}
 
-	if (at->res[1].start > 0 || (at->res[1].flags & IORESOURCE_IRQ)) {
-		arch_timer_ppi2 = at->res[1].start;
+	if (arch_timer_ppi2) {
 		err = request_percpu_irq(arch_timer_ppi2, arch_timer_handler,
 					 "arch_timer", arch_timer_evt);
 		if (err) {
@@ -298,6 +294,49 @@ out_free:
 	return err;
 }
 
+int __init arch_timer_register(struct arch_timer *at)
+{
+	if (at->res[0].start <= 0 || !(at->res[0].flags & IORESOURCE_IRQ))
+		return -EINVAL;
+
+	arch_timer_ppi = at->res[0].start;
+
+	if (at->res[1].start > 0 || (at->res[1].flags & IORESOURCE_IRQ))
+		arch_timer_ppi2 = at->res[1].start;
+
+	return arch_timer_common_register();
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id arch_timer_of_match[] __initconst = {
+	{ .compatible	= "arm,armv7-timer",	},
+	{},
+};
+
+int __init arch_timer_of_register(void)
+{
+	struct device_node *np;
+	u32 freq;
+
+	np = of_find_matching_node(NULL, arch_timer_of_match);
+	if (!np) {
+		pr_err("arch_timer: can't find DT node\n");
+		return -ENODEV;
+	}
+
+	/* Try to determine the frequency from the device tree or CNTFRQ */
+	if (!of_property_read_u32(np, "clock-frequency", &freq))
+		arch_timer_rate = freq;
+
+	arch_timer_ppi = irq_of_parse_and_map(np, 0);
+	arch_timer_ppi2 = irq_of_parse_and_map(np, 1);
+	pr_info("arch_timer: found %s irqs %d %d\n",
+		np->name, arch_timer_ppi, arch_timer_ppi2);
+
+	return arch_timer_common_register();
+}
+#endif
+
 int arch_timer_sched_clock_init(void)
 {
 	int err;
-- 
1.7.7.1

[PATCH v2 4/4] ARM: architected timers: add support for UP timer

[Marc Zyngier]

If CONFIG_LOCAL_TIMERS is not defined, let the architected timer
driver register a single clock_event_device that is used as a
global timer.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/arch_timer.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 6cb7d39..a6a3aee 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -229,6 +229,7 @@ static struct clocksource clocksource_counter = {
 	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
+#ifdef CONFIG_LOCAL_TIMERS
 static void __cpuinit arch_timer_stop(struct clock_event_device *clk)
 {
 	pr_debug("arch_timer_teardown disable IRQ%d cpu #%d\n",
@@ -243,6 +244,9 @@ static struct local_timer_ops arch_timer_ops __cpuinitdata = {
 	.setup	= arch_timer_setup,
 	.stop	= arch_timer_stop,
 };
+#else
+static struct clock_event_device arch_timer_global_evt;
+#endif
 
 static int __init arch_timer_common_register(void)
 {
@@ -277,7 +281,16 @@ static int __init arch_timer_common_register(void)
 		}
 	}
 
-	err = local_timer_register(&arch_timer_ops);
+#ifdef CONFIG_LOCAL_TIMERS
+	if (is_smp())
+		err = local_timer_register(&arch_timer_ops);
+	else
+#endif
+	{
+		arch_timer_global_evt.cpumask = cpumask_of(0);
+		err = arch_timer_setup(&arch_timer_global_evt);
+	}
+
 	if (err)
 		goto out_free_irq;
 
-- 
1.7.7.1

[PATCH v2 1/4] ARM: local timers: Add A15 architected timer support

[Christopher Covington]

Hi Marc,

On 02/01/2012 11:59 AM, Marc Zyngier wrote:
> Add support for the A15 generic timer and clocksource.
> As the timer generates interrupts on a different PPI depending
> on the execution mode (normal or secure), it is possible to
> register two different PPIs.

[snip]

> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> new file mode 100644
> index 0000000..680cf1e
> --- /dev/null
> +++ b/arch/arm/kernel/arch_timer.c
> @@ -0,0 +1,286 @@
> +/*
> + *  linux/arch/arm/kernel/arch_timer.c
> + *
> + *  Copyright (C) 2011 ARM Ltd.
> + *  All Rights Reserved
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/smp.h>
> +#include <linux/cpu.h>
> +#include <linux/jiffies.h>
> +#include <linux/clockchips.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +
> +#include <asm/cputype.h>
> +#include <asm/localtimer.h>
> +#include <asm/arch_timer.h>
> +
> +static unsigned long arch_timer_rate;
> +static int arch_timer_ppi;
> +static int arch_timer_ppi2;
> +
> +static struct clock_event_device __percpu **arch_timer_evt;
> +
> +/*
> + * Architected system timer support.
> + */
> +
> +#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
> +#define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
> +
> +#define ARCH_TIMER_REG_CTRL		0
> +#define ARCH_TIMER_REG_FREQ		1
> +#define ARCH_TIMER_REG_TVAL		2

[snip]

> +static irqreturn_t arch_timer_handler(int irq, void *dev_id)
> +{
> +	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
> +	unsigned long ctrl;
> +
> +	ctrl = arch_timer_reg_read(ARCH_TIMER_REG_CTRL);
> +	if (ctrl & 0x4) {

Perhaps this bit could be given a name like the others.

[snip]

> +static inline cycle_t arch_counter_get_cntpct(void)
> +{
> +	u32 cvall, cvalh;
> +
> +	asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> +
> +	return ((u64) cvalh << 32) | cvall;

Would it not be better to cast to cycle_t?

> +}
> +
> +static inline cycle_t arch_counter_get_cntvct(void)
> +{
> +	u32 cvall, cvalh;
> +
> +	asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> +
> +	return ((u64) cvalh << 32) | cvall;

Ditto.

> +}

[snip]

> +out_free_irq:
> +	free_percpu_irq(arch_timer_ppi, arch_timer_evt);
> +	if (arch_timer_ppi2)
> +		free_percpu_irq(arch_timer_ppi2, arch_timer_evt);
> +	

Trailing whitespace.

> +out_free:
> +	free_percpu(arch_timer_evt);
> +
> +	return err;
> +}

Regards,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[PATCH v2 3/4] ARM: architected timers: add DT support

[Christopher Covington]

Hi Marc,

On 02/01/2012 11:59 AM, Marc Zyngier wrote:
> Add runtime DT support and documentation for the Cortex A7/A15
> architected timers.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  .../devicetree/bindings/arm/arch_timer.txt         |   23 +++++++++
>  arch/arm/kernel/arch_timer.c                       |   53 +++++++++++++++++---
>  2 files changed, 69 insertions(+), 7 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/arch_timer.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> new file mode 100644
> index 0000000..84e93f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -0,0 +1,23 @@
> +* ARM architected timer
> +
> +ARM Cortex-A7 and Cortex-A15 have a per-core architected timer, which
> +provides a per-cpu local timer.
> +
> +The timer is attached to a GIC to deliver its two per-processor
> +interrupts (one for the secure mode, one for the non-secure mode).
> +
> +** Timer node properties:
> +
> +- compatible : Should be "arm,armv7-timer"
> +
> +- interrupts : One or two interrupts for secure and non-secure mode
> +
> +- clock-frequency : The frequency of the main counter, in Hz. Optionnal.

Spelling.

[snip]

Regards,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[PATCH v2 4/4] ARM: architected timers: add support for UP timer

[Christopher Covington]

Hi Marc,

On 02/01/2012 11:59 AM, Marc Zyngier wrote:
> If CONFIG_LOCAL_TIMERS is not defined, let the architected timer
> driver register a single clock_event_device that is used as a
> global timer.

[snip]

>  static int __init arch_timer_common_register(void)
>  {
> @@ -277,7 +281,16 @@ static int __init arch_timer_common_register(void)
>  		}
>  	}
>  
> -	err = local_timer_register(&arch_timer_ops);
> +#ifdef CONFIG_LOCAL_TIMERS
> +	if (is_smp())
> +		err = local_timer_register(&arch_timer_ops);
> +	else
> +#endif
> +	{
> +		arch_timer_global_evt.cpumask = cpumask_of(0);
> +		err = arch_timer_setup(&arch_timer_global_evt);
> +	}
> +
>  	if (err)
>  		goto out_free_irq;
>  

The logic here seems unnecessarily split between the preprocessor and
compiler. Certainly this can be revised to a more elegant form that
better keeps with the guidelines on ifdef usage [1].

1. http://www.linuxjournal.com/article/5780?page=0,3

Regards,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

[PATCH v2 4/4] ARM: architected timers: add support for UP timer

[Marc Zyngier]

Hi Christopher,

On 13/02/12 23:07, Christopher Covington wrote:
> On 02/01/2012 11:59 AM, Marc Zyngier wrote:
>> If CONFIG_LOCAL_TIMERS is not defined, let the architected timer
>> driver register a single clock_event_device that is used as a
>> global timer.
> 
> [snip]
> 
>>  static int __init arch_timer_common_register(void)
>>  {
>> @@ -277,7 +281,16 @@ static int __init arch_timer_common_register(void)
>>  		}
>>  	}
>>  
>> -	err = local_timer_register(&arch_timer_ops);
>> +#ifdef CONFIG_LOCAL_TIMERS
>> +	if (is_smp())
>> +		err = local_timer_register(&arch_timer_ops);
>> +	else
>> +#endif
>> +	{
>> +		arch_timer_global_evt.cpumask = cpumask_of(0);
>> +		err = arch_timer_setup(&arch_timer_global_evt);
>> +	}
>> +
>>  	if (err)
>>  		goto out_free_irq;
>>  
> 
> The logic here seems unnecessarily split between the preprocessor and
> compiler. Certainly this can be revised to a more elegant form that
> better keeps with the guidelines on ifdef usage [1].
> 
> 1. http://www.linuxjournal.com/article/5780?page=0,3

This would require a small change in the local timer code (basically
refusing to register a local timer if not on an SMP platform). But this
is just moving code from one #ifdef section to another...

Thanks for reviewing.

	M.
-- 
Jazz is not dead. It just smells funny...