linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api
  2012-12-19 15:10 [PATCHv2 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
@ 2012-12-19 15:11 ` Mark Rutland
  0 siblings, 0 replies; 38+ messages in thread
From: Mark Rutland @ 2012-12-19 15:11 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, the arch_timer driver is tied to the arm port, as it relies
on code in arch/arm/smp.c to setup and teardown timers as cores are
hotplugged on and off. The timer is registered through an arm-specific
registration mechanism, preventing sharing the driver with the arm64
port.

This patch moves the driver to using a cpu notifier instead, making it
easier to port.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/arch_timer.c |   52 +++++++++++++++++++++++------------------
 1 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index eb08e53..834d347 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -21,7 +21,6 @@
 #include <linux/io.h>
 
 #include <asm/delay.h>
-#include <asm/localtimer.h>
 #include <asm/arch_timer.h>
 #include <asm/sched_clock.h>
 
@@ -37,7 +36,7 @@ enum ppi_nr {
 
 static int arch_timer_ppi[MAX_TIMER_PPI];
 
-static struct clock_event_device __percpu **arch_timer_evt;
+static struct clock_event_device __percpu *arch_timer_evt;
 static struct delay_timer arch_delay_timer;
 
 static bool arch_timer_use_virtual = true;
@@ -63,14 +62,14 @@ static irqreturn_t inline timer_handler(const int access,
 
 static irqreturn_t arch_timer_handler_virt(int irq, void *dev_id)
 {
-	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+	struct clock_event_device *evt = dev_id;
 
 	return timer_handler(ARCH_TIMER_VIRT_ACCESS, evt);
 }
 
 static irqreturn_t arch_timer_handler_phys(int irq, void *dev_id)
 {
-	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+	struct clock_event_device *evt = dev_id;
 
 	return timer_handler(ARCH_TIMER_PHYS_ACCESS, evt);
 }
@@ -141,13 +140,13 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
 		clk->set_next_event = arch_timer_set_next_event_phys;
 	}
 
+	clk->cpumask = cpumask_of(smp_processor_id());
+
 	clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, NULL);
 
 	clockevents_config_and_register(clk, arch_timer_rate,
 					0xf, 0x7fffffff);
 
-	*__this_cpu_ptr(arch_timer_evt) = clk;
-
 	if (arch_timer_use_virtual)
 		enable_percpu_irq(arch_timer_ppi[VIRT_PPI], 0);
 	else {
@@ -245,12 +244,26 @@ static void __cpuinit arch_timer_stop(struct clock_event_device *clk)
 	clk->set_mode(CLOCK_EVT_MODE_UNUSED, clk);
 }
 
-static struct local_timer_ops arch_timer_ops __cpuinitdata = {
-	.setup	= arch_timer_setup,
-	.stop	= arch_timer_stop,
-};
+static int __cpuinit arch_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(arch_timer_evt);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		arch_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		arch_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
 
-static struct clock_event_device arch_timer_global_evt;
+static struct notifier_block arch_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = arch_timer_cpu_notify,
+};
 
 static int __init arch_timer_register(void)
 {
@@ -261,7 +274,7 @@ static int __init arch_timer_register(void)
 	if (err)
 		goto out;
 
-	arch_timer_evt = alloc_percpu(struct clock_event_device *);
+	arch_timer_evt = alloc_percpu(struct clock_event_device);
 	if (!arch_timer_evt) {
 		err = -ENOMEM;
 		goto out;
@@ -297,20 +310,13 @@ static int __init arch_timer_register(void)
 		goto out_free;
 	}
 
-	err = local_timer_register(&arch_timer_ops);
-	if (err) {
-		/*
-		 * We couldn't register as a local timer (could be
-		 * because we're on a UP platform, or because some
-		 * other local timer is already present...). Try as a
-		 * global timer instead.
-		 */
-		arch_timer_global_evt.cpumask = cpumask_of(0);
-		err = arch_timer_setup(&arch_timer_global_evt);
-	}
+	err = register_cpu_notifier(&arch_timer_cpu_nb);
 	if (err)
 		goto out_free_irq;
 
+	/* Immediately configure the timer on the boot CPU */
+	arch_timer_setup(this_cpu_ptr(arch_timer_evt));
+
 	/* Use the architected timer for the delay loop. */
 	arch_delay_timer.read_current_timer = &arch_timer_read_current_timer;
 	arch_delay_timer.freq = arch_timer_rate;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv3 00/11] Unify arm_generic and arch_timer drivers
@ 2013-01-09 16:07 Mark Rutland
  2013-01-09 16:07 ` [PATCHv2 01/11] arm: arch_timer: balance device_node refcounting Mark Rutland
                   ` (10 more replies)
  0 siblings, 11 replies; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

This is the rebase to an -rc I promised back in December [1].

Changes since v2:
* Rebase to v3.8-rc2
Changes since v1:
* Rename arch_counter_{enable=>set}_user_access.
* * Explicitly disable access to counters from userspace on arm.
* * Add device_node refcounting fix.
* * Fix conflict with Will Deacon's AArch64 vdso changes.
* * Simplify the cpu notifier.
* * Fix a couple of style issues.

Currently we have two drivers for the ARM generic / architected timer,
arch_timer in arch/arm, and arm_generic in drivers/clocksource. This is
an unnecessary duplication of code, and will only lead to maintenance
headaches later.

This patch series splits the generic portion of the arch_timer out to
drivers/clocksource, and ports the arm64 code to use it. The now unused
arm_generic driver is removed in the process.

Separating the arch_timer driver from the arm-specific local_timer api
currently loses us broadcast timer support. I've posted another series
[2] which remedies this by decoupling the timer broadcast mechanism from
drivers.

The series applies to v3.8-rc2, and has been tested on a TC2 test chip.
It also applies to Catalin's soc-armv8-model branch [3], the result of
which has been tested on an AArch64 model.

Christoffer, this will conflict with one of your KVM/ARM architected
timers support patches ([PATCH v5 1/4] ARM: arch_timers: switch to
physical timers if HYP mode is available). I'm happy to fold that into
this series if you're ok with that?

Thanks,
Mark

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138130.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/140528.html
[3] git://git.kernel.org/pub/scm/linux/kernel/git/cmarinas/linux-aarch64.git soc-armv8-model

Mark Rutland (11):
  arm: arch_timer: balance device_node refcounting
  arm: arch_timer: remove redundant available check
  arm: arch_timer: use u64/u32 for register data
  arm: arch_timer: standardise counter reading
  arm: arch_timer: split cntfrq accessor
  arm: arch_timer: factor out register accessors
  arm: arch_timer: divorce from local_timer api
  arm: arch_timer: add arch_counter_set_user_access
  arm: arch_timer: move core to drivers/clocksource
  arm64: move from arm_generic to arm_arch_timer
  Documentation: Add ARMv8 to arch_timer devicetree

 .../devicetree/bindings/arm/arch_timer.txt         |    7 +-
 arch/arm/Kconfig                                   |    3 +-
 arch/arm/include/asm/arch_timer.h                  |  111 ++++-
 arch/arm/kernel/arch_timer.c                       |  504 +-------------------
 arch/arm/mach-omap2/Kconfig                        |    2 +-
 arch/arm64/Kconfig                                 |    1 +
 arch/arm64/include/asm/arch_timer.h                |  133 +++++
 arch/arm64/include/asm/arm_generic.h               |  100 ----
 arch/arm64/kernel/time.c                           |   29 +-
 drivers/clocksource/Kconfig                        |    6 +-
 drivers/clocksource/Makefile                       |    2 +-
 drivers/clocksource/arm_arch_timer.c               |  375 +++++++++++++++
 drivers/clocksource/arm_generic.c                  |  232 ---------
 include/clocksource/arm_arch_timer.h               |   63 +++
 include/clocksource/arm_generic.h                  |   21 -
 15 files changed, 734 insertions(+), 855 deletions(-)
 create mode 100644 arch/arm64/include/asm/arch_timer.h
 delete mode 100644 arch/arm64/include/asm/arm_generic.h
 create mode 100644 drivers/clocksource/arm_arch_timer.c
 delete mode 100644 drivers/clocksource/arm_generic.c
 create mode 100644 include/clocksource/arm_arch_timer.h
 delete mode 100644 include/clocksource/arm_generic.h

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 01/11] arm: arch_timer: balance device_node refcounting
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 12:59   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 02/11] arm: arch_timer: remove redundant available check Mark Rutland
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

When we get the device_node for the arch timer, it's refcount is
automatically incremented in of_find_matching_node, but it is
never decremented.

This patch decrements the refcount on the node after we're finished
using it.

Reported-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm/kernel/arch_timer.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index c8ef207..6dd73c6 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -488,6 +488,8 @@ int __init arch_timer_of_register(void)
 	for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)
 		arch_timer_ppi[i] = irq_of_parse_and_map(np, i);
 
+	of_node_put(np);
+
 	/*
 	 * If no interrupt provided for virtual timer, we'll have to
 	 * stick to the physical timer. It'd better be accessible...
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 02/11] arm: arch_timer: remove redundant available check
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
  2013-01-09 16:07 ` [PATCHv2 01/11] arm: arch_timer: balance device_node refcounting Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:11   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 03/11] arm: arch_timer: use u64/u32 for register data Mark Rutland
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

This check is a holdover from the pre-devicetree days. As the timer
is not probed except by platforms which register it via devicetree,
it's not strictly necessary.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/arch_timer.c |   12 ------------
 1 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 6dd73c6..1bb3b58 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -20,11 +20,9 @@
 #include <linux/of_irq.h>
 #include <linux/io.h>
 
-#include <asm/cputype.h>
 #include <asm/delay.h>
 #include <asm/localtimer.h>
 #include <asm/arch_timer.h>
-#include <asm/system_info.h>
 #include <asm/sched_clock.h>
 
 static unsigned long arch_timer_rate;
@@ -259,20 +257,10 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
 	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) {
 		freq = arch_timer_reg_read(ARCH_TIMER_PHYS_ACCESS,
 					   ARCH_TIMER_REG_FREQ);
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 03/11] arm: arch_timer: use u64/u32 for register data
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
  2013-01-09 16:07 ` [PATCHv2 01/11] arm: arch_timer: balance device_node refcounting Mark Rutland
  2013-01-09 16:07 ` [PATCHv2 02/11] arm: arch_timer: remove redundant available check Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:19   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 04/11] arm: arch_timer: standardise counter reading Mark Rutland
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

To ensure the correct size of types, use u64 for the return value of
arch_timer_get_cnt{p,v}ct, and u32 for arch_timer_rate, matching the
size of the registers these values are taken from. While we're changing
them anyway, simplify the implementation of arch_timer_get_cnt{p,v}ct.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/arch_timer.c |   30 +++++++++++-------------------
 1 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 1bb3b58..498c29f 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -25,7 +25,7 @@
 #include <asm/arch_timer.h>
 #include <asm/sched_clock.h>
 
-static unsigned long arch_timer_rate;
+static u32 arch_timer_rate;
 
 enum ppi_nr {
 	PHYS_SECURE_PPI,
@@ -121,27 +121,18 @@ static inline u32 arch_timer_reg_read(const int access, const int reg)
 	return val;
 }
 
-static inline cycle_t arch_timer_counter_read(const int access)
+static inline u64 arch_counter_get_cntpct(void)
 {
-	cycle_t cval = 0;
-
-	if (access == ARCH_TIMER_PHYS_ACCESS)
-		asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval));
-
-	if (access == ARCH_TIMER_VIRT_ACCESS)
-		asm volatile("mrrc p15, 1, %Q0, %R0, c14" : "=r" (cval));
-
+	u64 cval;
+	asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval));
 	return cval;
 }
 
-static inline cycle_t arch_counter_get_cntpct(void)
-{
-	return arch_timer_counter_read(ARCH_TIMER_PHYS_ACCESS);
-}
-
-static inline cycle_t arch_counter_get_cntvct(void)
+static inline u64 arch_counter_get_cntvct(void)
 {
-	return arch_timer_counter_read(ARCH_TIMER_VIRT_ACCESS);
+	u64 cval;
+	asm volatile("mrrc p15, 1, %Q0, %R0, c14" : "=r" (cval));
+	return cval;
 }
 
 static irqreturn_t inline timer_handler(const int access,
@@ -259,7 +250,7 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
 
 static int arch_timer_available(void)
 {
-	unsigned long freq;
+	u32 freq;
 
 	if (arch_timer_rate == 0) {
 		freq = arch_timer_reg_read(ARCH_TIMER_PHYS_ACCESS,
@@ -275,7 +266,8 @@ static int arch_timer_available(void)
 	}
 
 	pr_info_once("Architected local timer running at %lu.%02luMHz (%s).\n",
-		     arch_timer_rate / 1000000, (arch_timer_rate / 10000) % 100,
+		     (unsigned long)arch_timer_rate / 1000000,
+		     (unsigned long)(arch_timer_rate / 10000) % 100,
 		     arch_timer_use_virtual ? "virt" : "phys");
 	return 0;
 }
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 04/11] arm: arch_timer: standardise counter reading
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
                   ` (2 preceding siblings ...)
  2013-01-09 16:07 ` [PATCHv2 03/11] arm: arch_timer: use u64/u32 for register data Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:23   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 05/11] arm: arch_timer: split cntfrq accessor Mark Rutland
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

We're currently inconsistent with respect to our accesses to the
physical and virtual counters, mixing and matching the two.

This patch introduces and uses a function for accessing the correct
counter based on whether we're using physical or virtual interrupts.
All current accesses to the counter accessors are redirected through
it.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/arch_timer.c |   48 ++++++++++-------------------------------
 1 files changed, 12 insertions(+), 36 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 498c29f..0d2681c 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -272,51 +272,32 @@ static int arch_timer_available(void)
 	return 0;
 }
 
-static u32 notrace arch_counter_get_cntpct32(void)
+u64 arch_timer_read_counter(void)
 {
-	cycle_t cnt = arch_counter_get_cntpct();
-
-	/*
-	 * The sched_clock infrastructure only knows about counters
-	 * with at most 32bits. Forget about the upper 24 bits for the
-	 * time being...
-	 */
-	return (u32)cnt;
+	if (arch_timer_use_virtual)
+		return arch_counter_get_cntvct();
+	else
+		return arch_counter_get_cntpct();
 }
 
-static u32 notrace arch_counter_get_cntvct32(void)
+static u32 arch_timer_read_counter32(void)
 {
-	cycle_t cnt = arch_counter_get_cntvct();
-
-	/*
-	 * The sched_clock infrastructure only knows about counters
-	 * with at most 32bits. Forget about the upper 24 bits for the
-	 * time being...
-	 */
-	return (u32)cnt;
+	return arch_timer_read_counter();
 }
 
 static cycle_t arch_counter_read(struct clocksource *cs)
 {
-	/*
-	 * Always use the physical counter for the clocksource.
-	 * CNTHCTL.PL1PCTEN must be set to 1.
-	 */
-	return arch_counter_get_cntpct();
+	return arch_timer_read_counter();
 }
 
 static unsigned long arch_timer_read_current_timer(void)
 {
-	return arch_counter_get_cntpct();
+	return arch_timer_read_counter();
 }
 
 static cycle_t arch_counter_read_cc(const struct cyclecounter *cc)
 {
-	/*
-	 * Always use the physical counter for the clocksource.
-	 * CNTHCTL.PL1PCTEN must be set to 1.
-	 */
-	return arch_counter_get_cntpct();
+	return arch_timer_read_counter();
 }
 
 static struct clocksource clocksource_counter = {
@@ -489,18 +470,13 @@ int __init arch_timer_of_register(void)
 
 int __init arch_timer_sched_clock_init(void)
 {
-	u32 (*cnt32)(void);
 	int err;
 
 	err = arch_timer_available();
 	if (err)
 		return err;
 
-	if (arch_timer_use_virtual)
-		cnt32 = arch_counter_get_cntvct32;
-	else
-		cnt32 = arch_counter_get_cntpct32;
-
-	setup_sched_clock(cnt32, 32, arch_timer_rate);
+	setup_sched_clock(arch_timer_read_counter32,
+			  32, arch_timer_rate);
 	return 0;
 }
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 05/11] arm: arch_timer: split cntfrq accessor
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
                   ` (3 preceding siblings ...)
  2013-01-09 16:07 ` [PATCHv2 04/11] arm: arch_timer: standardise counter reading Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:27   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 06/11] arm: arch_timer: factor out register accessors Mark Rutland
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

The CNTFRQ register is not duplicated for physical and virtual timers,
and accessing it as if it were is confusing.

Instead, use a separate accessor which doesn't take the access type
as a parameter.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/arch_timer.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 0d2681c..fc87d3d 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -51,8 +51,7 @@ static bool arch_timer_use_virtual = true;
 #define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
 
 #define ARCH_TIMER_REG_CTRL		0
-#define ARCH_TIMER_REG_FREQ		1
-#define ARCH_TIMER_REG_TVAL		2
+#define ARCH_TIMER_REG_TVAL		1
 
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
@@ -101,9 +100,6 @@ static inline u32 arch_timer_reg_read(const int access, const int reg)
 		case ARCH_TIMER_REG_TVAL:
 			asm volatile("mrc p15, 0, %0, c14, c2, 0" : "=r" (val));
 			break;
-		case ARCH_TIMER_REG_FREQ:
-			asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (val));
-			break;
 		}
 	}
 
@@ -121,6 +117,13 @@ static inline u32 arch_timer_reg_read(const int access, const int reg)
 	return val;
 }
 
+static inline u32 arch_timer_get_cntfrq(void)
+{
+	u32 val;
+	asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (val));
+	return val;
+}
+
 static inline u64 arch_counter_get_cntpct(void)
 {
 	u64 cval;
@@ -253,9 +256,7 @@ static int arch_timer_available(void)
 	u32 freq;
 
 	if (arch_timer_rate == 0) {
-		freq = arch_timer_reg_read(ARCH_TIMER_PHYS_ACCESS,
-					   ARCH_TIMER_REG_FREQ);
-
+		freq = arch_timer_get_cntfrq();
 		/* Check the timer frequency. */
 		if (freq == 0) {
 			pr_warn("Architected timer frequency not available\n");
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 06/11] arm: arch_timer: factor out register accessors
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
                   ` (4 preceding siblings ...)
  2013-01-09 16:07 ` [PATCHv2 05/11] arm: arch_timer: split cntfrq accessor Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:32   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api Mark Rutland
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

Currently the arch_timer register accessors are thrown together with
the main driver, preventing us from porting the driver to other
architectures.

This patch moves the register accessors into a header file, as with
the arm64 version. Constants required by the accessors are also moved.

Additionally isbs are added in arch_timer_get_cnt{v,p}ct to prevent
the cpu from speculating the reads and returning stale values.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/arch_timer.h |  101 +++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/arch_timer.c      |   92 ---------------------------------
 2 files changed, 101 insertions(+), 92 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index d40229d..701f2b7 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -1,13 +1,114 @@
 #ifndef __ASMARM_ARCH_TIMER_H
 #define __ASMARM_ARCH_TIMER_H
 
+#include <asm/barrier.h>
 #include <asm/errno.h>
+
 #include <linux/clocksource.h>
+#include <linux/types.h>
 
 #ifdef CONFIG_ARM_ARCH_TIMER
 int arch_timer_of_register(void);
 int arch_timer_sched_clock_init(void);
 struct timecounter *arch_timer_get_timecounter(void);
+
+#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
+#define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
+#define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
+
+#define ARCH_TIMER_REG_CTRL		0
+#define ARCH_TIMER_REG_TVAL		1
+
+#define ARCH_TIMER_PHYS_ACCESS		0
+#define ARCH_TIMER_VIRT_ACCESS		1
+
+/*
+ * These register accessors are marked inline so the compiler can
+ * nicely work out which register we want, and chuck away the rest of
+ * the code. At least it does so with a recent GCC (4.6.3).
+ */
+static inline void arch_timer_reg_write(const int access, const int reg, u32 val)
+{
+	if (access == ARCH_TIMER_PHYS_ACCESS) {
+		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;
+		}
+	}
+
+	if (access == ARCH_TIMER_VIRT_ACCESS) {
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			asm volatile("mcr p15, 0, %0, c14, c3, 1" : : "r" (val));
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			asm volatile("mcr p15, 0, %0, c14, c3, 0" : : "r" (val));
+			break;
+		}
+	}
+
+	isb();
+}
+
+static inline u32 arch_timer_reg_read(const int access, const int reg)
+{
+	u32 val = 0;
+
+	if (access == ARCH_TIMER_PHYS_ACCESS) {
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			asm volatile("mrc p15, 0, %0, c14, c2, 1" : "=r" (val));
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			asm volatile("mrc p15, 0, %0, c14, c2, 0" : "=r" (val));
+			break;
+		}
+	}
+
+	if (access == ARCH_TIMER_VIRT_ACCESS) {
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			asm volatile("mrc p15, 0, %0, c14, c3, 1" : "=r" (val));
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			asm volatile("mrc p15, 0, %0, c14, c3, 0" : "=r" (val));
+			break;
+		}
+	}
+
+	return val;
+}
+
+static inline u32 arch_timer_get_cntfrq(void)
+{
+	u32 val;
+	asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (val));
+	return val;
+}
+
+static inline u64 arch_counter_get_cntpct(void)
+{
+	u64 cval;
+
+	isb();
+	asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval));
+	return cval;
+}
+
+static inline u64 arch_counter_get_cntvct(void)
+{
+	u64 cval;
+
+	isb();
+	asm volatile("mrrc p15, 1, %Q0, %R0, c14" : "=r" (cval));
+	return cval;
+}
+
+
 #else
 static inline int arch_timer_of_register(void)
 {
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index fc87d3d..eb08e53 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -46,98 +46,6 @@ static bool arch_timer_use_virtual = true;
  * Architected system timer support.
  */
 
-#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
-#define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
-#define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
-
-#define ARCH_TIMER_REG_CTRL		0
-#define ARCH_TIMER_REG_TVAL		1
-
-#define ARCH_TIMER_PHYS_ACCESS		0
-#define ARCH_TIMER_VIRT_ACCESS		1
-
-/*
- * These register accessors are marked inline so the compiler can
- * nicely work out which register we want, and chuck away the rest of
- * the code. At least it does so with a recent GCC (4.6.3).
- */
-static inline void arch_timer_reg_write(const int access, const int reg, u32 val)
-{
-	if (access == ARCH_TIMER_PHYS_ACCESS) {
-		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;
-		}
-	}
-
-	if (access == ARCH_TIMER_VIRT_ACCESS) {
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			asm volatile("mcr p15, 0, %0, c14, c3, 1" : : "r" (val));
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			asm volatile("mcr p15, 0, %0, c14, c3, 0" : : "r" (val));
-			break;
-		}
-	}
-
-	isb();
-}
-
-static inline u32 arch_timer_reg_read(const int access, const int reg)
-{
-	u32 val = 0;
-
-	if (access == ARCH_TIMER_PHYS_ACCESS) {
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			asm volatile("mrc p15, 0, %0, c14, c2, 1" : "=r" (val));
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			asm volatile("mrc p15, 0, %0, c14, c2, 0" : "=r" (val));
-			break;
-		}
-	}
-
-	if (access == ARCH_TIMER_VIRT_ACCESS) {
-		switch (reg) {
-		case ARCH_TIMER_REG_CTRL:
-			asm volatile("mrc p15, 0, %0, c14, c3, 1" : "=r" (val));
-			break;
-		case ARCH_TIMER_REG_TVAL:
-			asm volatile("mrc p15, 0, %0, c14, c3, 0" : "=r" (val));
-			break;
-		}
-	}
-
-	return val;
-}
-
-static inline u32 arch_timer_get_cntfrq(void)
-{
-	u32 val;
-	asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (val));
-	return val;
-}
-
-static inline u64 arch_counter_get_cntpct(void)
-{
-	u64 cval;
-	asm volatile("mrrc p15, 0, %Q0, %R0, c14" : "=r" (cval));
-	return cval;
-}
-
-static inline u64 arch_counter_get_cntvct(void)
-{
-	u64 cval;
-	asm volatile("mrrc p15, 1, %Q0, %R0, c14" : "=r" (cval));
-	return cval;
-}
-
 static irqreturn_t inline timer_handler(const int access,
 					struct clock_event_device *evt)
 {
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
                   ` (5 preceding siblings ...)
  2013-01-09 16:07 ` [PATCHv2 06/11] arm: arch_timer: factor out register accessors Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:34   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access Mark Rutland
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, the arch_timer driver is tied to the arm port, as it relies
on code in arch/arm/smp.c to setup and teardown timers as cores are
hotplugged on and off. The timer is registered through an arm-specific
registration mechanism, preventing sharing the driver with the arm64
port.

This patch moves the driver to using a cpu notifier instead, making it
easier to port.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kernel/arch_timer.c |   52 +++++++++++++++++++++++------------------
 1 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index eb08e53..834d347 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -21,7 +21,6 @@
 #include <linux/io.h>
 
 #include <asm/delay.h>
-#include <asm/localtimer.h>
 #include <asm/arch_timer.h>
 #include <asm/sched_clock.h>
 
@@ -37,7 +36,7 @@ enum ppi_nr {
 
 static int arch_timer_ppi[MAX_TIMER_PPI];
 
-static struct clock_event_device __percpu **arch_timer_evt;
+static struct clock_event_device __percpu *arch_timer_evt;
 static struct delay_timer arch_delay_timer;
 
 static bool arch_timer_use_virtual = true;
@@ -63,14 +62,14 @@ static irqreturn_t inline timer_handler(const int access,
 
 static irqreturn_t arch_timer_handler_virt(int irq, void *dev_id)
 {
-	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+	struct clock_event_device *evt = dev_id;
 
 	return timer_handler(ARCH_TIMER_VIRT_ACCESS, evt);
 }
 
 static irqreturn_t arch_timer_handler_phys(int irq, void *dev_id)
 {
-	struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
+	struct clock_event_device *evt = dev_id;
 
 	return timer_handler(ARCH_TIMER_PHYS_ACCESS, evt);
 }
@@ -141,13 +140,13 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
 		clk->set_next_event = arch_timer_set_next_event_phys;
 	}
 
+	clk->cpumask = cpumask_of(smp_processor_id());
+
 	clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, NULL);
 
 	clockevents_config_and_register(clk, arch_timer_rate,
 					0xf, 0x7fffffff);
 
-	*__this_cpu_ptr(arch_timer_evt) = clk;
-
 	if (arch_timer_use_virtual)
 		enable_percpu_irq(arch_timer_ppi[VIRT_PPI], 0);
 	else {
@@ -245,12 +244,26 @@ static void __cpuinit arch_timer_stop(struct clock_event_device *clk)
 	clk->set_mode(CLOCK_EVT_MODE_UNUSED, clk);
 }
 
-static struct local_timer_ops arch_timer_ops __cpuinitdata = {
-	.setup	= arch_timer_setup,
-	.stop	= arch_timer_stop,
-};
+static int __cpuinit arch_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(arch_timer_evt);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		arch_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		arch_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
 
-static struct clock_event_device arch_timer_global_evt;
+static struct notifier_block arch_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = arch_timer_cpu_notify,
+};
 
 static int __init arch_timer_register(void)
 {
@@ -261,7 +274,7 @@ static int __init arch_timer_register(void)
 	if (err)
 		goto out;
 
-	arch_timer_evt = alloc_percpu(struct clock_event_device *);
+	arch_timer_evt = alloc_percpu(struct clock_event_device);
 	if (!arch_timer_evt) {
 		err = -ENOMEM;
 		goto out;
@@ -297,20 +310,13 @@ static int __init arch_timer_register(void)
 		goto out_free;
 	}
 
-	err = local_timer_register(&arch_timer_ops);
-	if (err) {
-		/*
-		 * We couldn't register as a local timer (could be
-		 * because we're on a UP platform, or because some
-		 * other local timer is already present...). Try as a
-		 * global timer instead.
-		 */
-		arch_timer_global_evt.cpumask = cpumask_of(0);
-		err = arch_timer_setup(&arch_timer_global_evt);
-	}
+	err = register_cpu_notifier(&arch_timer_cpu_nb);
 	if (err)
 		goto out_free_irq;
 
+	/* Immediately configure the timer on the boot CPU */
+	arch_timer_setup(this_cpu_ptr(arch_timer_evt));
+
 	/* Use the architected timer for the delay loop. */
 	arch_delay_timer.read_current_timer = &arch_timer_read_current_timer;
 	arch_delay_timer.freq = arch_timer_rate;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
                   ` (6 preceding siblings ...)
  2013-01-09 16:07 ` [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:40   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource Mark Rutland
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

Several bits in CNTKCTL reset to 0, including PL0VTEN. For platforms
using the generic timer which wish to have a fast gettimeofday vDSO
implementation, these bits must be set to 1 by the kernel. On other
platforms, the bootloader might enable userspace access when we don't
want it.

This patch adds arch_counter_set_user_access, which sets the PL0 access
permissions to that required by the platform. For arm, this currently
means disabling all userspace access.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm/include/asm/arch_timer.h |   11 +++++++++++
 arch/arm/kernel/arch_timer.c      |    2 ++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 701f2b7..05e3593 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -108,6 +108,17 @@ static inline u64 arch_counter_get_cntvct(void)
 	return cval;
 }
 
+static inline void __cpuinit arch_counter_set_user_access(void)
+{
+	u32 cntkctl;
+
+	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
+
+	/* disable user access to everything */
+	cntkctl &= ~((3 << 8) | (7 << 0));
+
+	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
+}
 
 #else
 static inline int arch_timer_of_register(void)
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 834d347..4f39e68 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -155,6 +155,8 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
 			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
 	}
 
+	arch_counter_set_user_access();
+
 	return 0;
 }
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
                   ` (7 preceding siblings ...)
  2013-01-09 16:07 ` [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:48   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 10/11] arm64: move from arm_generic to arm_arch_timer Mark Rutland
  2013-01-09 16:07 ` [PATCHv2 11/11] Documentation: Add ARMv8 to arch_timer devicetree Mark Rutland
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

The core functionality of the arch_timer driver is not directly tied to
anything under arch/arm, and can be split out.

This patch factors out the core of the arch_timer driver, so it can be
shared with other architectures. A couple of functions are added so
that architecture-specific code can interact with the driver without
needing to touch its internals.

The ARM_ARCH_TIMER config variable is moved out to
drivers/clocksource/Kconfig, existing uses in arch/arm are replaced with
USE_ARM_ARCH_TIMER, which selects it.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/Kconfig                     |    3 +-
 arch/arm/include/asm/arch_timer.h    |   19 +--
 arch/arm/kernel/arch_timer.c         |  375 ++--------------------------------
 arch/arm/mach-omap2/Kconfig          |    2 +-
 drivers/clocksource/Kconfig          |    3 +
 drivers/clocksource/Makefile         |    1 +
 drivers/clocksource/arm_arch_timer.c |  374 +++++++++++++++++++++++++++++++++
 include/clocksource/arm_arch_timer.h |   63 ++++++
 8 files changed, 465 insertions(+), 375 deletions(-)
 create mode 100644 drivers/clocksource/arm_arch_timer.c
 create mode 100644 include/clocksource/arm_arch_timer.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index f95ba14..487696a 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1567,9 +1567,10 @@ config HAVE_ARM_SCU
 	help
 	  This option enables support for the ARM system coherency unit
 
-config ARM_ARCH_TIMER
+config USE_ARM_ARCH_TIMER
 	bool "Architected timer support"
 	depends on CPU_V7
+	select ARM_ARCH_TIMER
 	help
 	  This option enables support for the ARM architected timer
 
diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 05e3593..7c8f835 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -5,22 +5,14 @@
 #include <asm/errno.h>
 
 #include <linux/clocksource.h>
+#include <linux/init.h>
 #include <linux/types.h>
 
+#include <clocksource/arm_arch_timer.h>
+
 #ifdef CONFIG_ARM_ARCH_TIMER
 int arch_timer_of_register(void);
 int arch_timer_sched_clock_init(void);
-struct timecounter *arch_timer_get_timecounter(void);
-
-#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
-#define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
-#define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
-
-#define ARCH_TIMER_REG_CTRL		0
-#define ARCH_TIMER_REG_TVAL		1
-
-#define ARCH_TIMER_PHYS_ACCESS		0
-#define ARCH_TIMER_VIRT_ACCESS		1
 
 /*
  * These register accessors are marked inline so the compiler can
@@ -130,11 +122,6 @@ static inline int arch_timer_sched_clock_init(void)
 {
 	return -ENXIO;
 }
-
-static inline struct timecounter *arch_timer_get_timecounter(void)
-{
-	return NULL;
-}
 #endif
 
 #endif
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 4f39e68..36ebcf4 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -9,391 +9,52 @@
  * 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/of_irq.h>
-#include <linux/io.h>
+#include <linux/types.h>
 
 #include <asm/delay.h>
-#include <asm/arch_timer.h>
 #include <asm/sched_clock.h>
 
-static u32 arch_timer_rate;
+#include <clocksource/arm_arch_timer.h>
 
-enum ppi_nr {
-	PHYS_SECURE_PPI,
-	PHYS_NONSECURE_PPI,
-	VIRT_PPI,
-	HYP_PPI,
-	MAX_TIMER_PPI
-};
-
-static int arch_timer_ppi[MAX_TIMER_PPI];
-
-static struct clock_event_device __percpu *arch_timer_evt;
-static struct delay_timer arch_delay_timer;
-
-static bool arch_timer_use_virtual = true;
-
-/*
- * Architected system timer support.
- */
-
-static irqreturn_t inline timer_handler(const int access,
-					struct clock_event_device *evt)
-{
-	unsigned long ctrl;
-	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL);
-	if (ctrl & ARCH_TIMER_CTRL_IT_STAT) {
-		ctrl |= ARCH_TIMER_CTRL_IT_MASK;
-		arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl);
-		evt->event_handler(evt);
-		return IRQ_HANDLED;
-	}
-
-	return IRQ_NONE;
-}
-
-static irqreturn_t arch_timer_handler_virt(int irq, void *dev_id)
-{
-	struct clock_event_device *evt = dev_id;
-
-	return timer_handler(ARCH_TIMER_VIRT_ACCESS, evt);
-}
-
-static irqreturn_t arch_timer_handler_phys(int irq, void *dev_id)
-{
-	struct clock_event_device *evt = dev_id;
-
-	return timer_handler(ARCH_TIMER_PHYS_ACCESS, evt);
-}
-
-static inline void timer_set_mode(const int access, int mode)
-{
-	unsigned long ctrl;
-	switch (mode) {
-	case CLOCK_EVT_MODE_UNUSED:
-	case CLOCK_EVT_MODE_SHUTDOWN:
-		ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL);
-		ctrl &= ~ARCH_TIMER_CTRL_ENABLE;
-		arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl);
-		break;
-	default:
-		break;
-	}
-}
-
-static void arch_timer_set_mode_virt(enum clock_event_mode mode,
-				     struct clock_event_device *clk)
-{
-	timer_set_mode(ARCH_TIMER_VIRT_ACCESS, mode);
-}
-
-static void arch_timer_set_mode_phys(enum clock_event_mode mode,
-				     struct clock_event_device *clk)
-{
-	timer_set_mode(ARCH_TIMER_PHYS_ACCESS, mode);
-}
-
-static inline void set_next_event(const int access, unsigned long evt)
-{
-	unsigned long ctrl;
-	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL);
-	ctrl |= ARCH_TIMER_CTRL_ENABLE;
-	ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
-	arch_timer_reg_write(access, ARCH_TIMER_REG_TVAL, evt);
-	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl);
-}
-
-static int arch_timer_set_next_event_virt(unsigned long evt,
-					  struct clock_event_device *unused)
-{
-	set_next_event(ARCH_TIMER_VIRT_ACCESS, evt);
-	return 0;
-}
-
-static int arch_timer_set_next_event_phys(unsigned long evt,
-					  struct clock_event_device *unused)
-{
-	set_next_event(ARCH_TIMER_PHYS_ACCESS, evt);
-	return 0;
-}
-
-static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
-{
-	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
-	clk->name = "arch_sys_timer";
-	clk->rating = 450;
-	if (arch_timer_use_virtual) {
-		clk->irq = arch_timer_ppi[VIRT_PPI];
-		clk->set_mode = arch_timer_set_mode_virt;
-		clk->set_next_event = arch_timer_set_next_event_virt;
-	} else {
-		clk->irq = arch_timer_ppi[PHYS_SECURE_PPI];
-		clk->set_mode = arch_timer_set_mode_phys;
-		clk->set_next_event = arch_timer_set_next_event_phys;
-	}
-
-	clk->cpumask = cpumask_of(smp_processor_id());
-
-	clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, NULL);
-
-	clockevents_config_and_register(clk, arch_timer_rate,
-					0xf, 0x7fffffff);
-
-	if (arch_timer_use_virtual)
-		enable_percpu_irq(arch_timer_ppi[VIRT_PPI], 0);
-	else {
-		enable_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI], 0);
-		if (arch_timer_ppi[PHYS_NONSECURE_PPI])
-			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
-	}
-
-	arch_counter_set_user_access();
-
-	return 0;
-}
-
-static int arch_timer_available(void)
-{
-	u32 freq;
-
-	if (arch_timer_rate == 0) {
-		freq = arch_timer_get_cntfrq();
-		/* Check the timer frequency. */
-		if (freq == 0) {
-			pr_warn("Architected timer frequency not available\n");
-			return -EINVAL;
-		}
-
-		arch_timer_rate = freq;
-	}
-
-	pr_info_once("Architected local timer running at %lu.%02luMHz (%s).\n",
-		     (unsigned long)arch_timer_rate / 1000000,
-		     (unsigned long)(arch_timer_rate / 10000) % 100,
-		     arch_timer_use_virtual ? "virt" : "phys");
-	return 0;
-}
-
-u64 arch_timer_read_counter(void)
-{
-	if (arch_timer_use_virtual)
-		return arch_counter_get_cntvct();
-	else
-		return arch_counter_get_cntpct();
-}
-
-static u32 arch_timer_read_counter32(void)
-{
-	return arch_timer_read_counter();
-}
-
-static cycle_t arch_counter_read(struct clocksource *cs)
-{
-	return arch_timer_read_counter();
-}
-
-static unsigned long arch_timer_read_current_timer(void)
+static unsigned long arch_timer_read_counter_long(void)
 {
 	return arch_timer_read_counter();
 }
 
-static cycle_t arch_counter_read_cc(const struct cyclecounter *cc)
+static u32 arch_timer_read_counter_u32(void)
 {
 	return arch_timer_read_counter();
 }
 
-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 struct cyclecounter cyclecounter = {
-	.read	= arch_counter_read_cc,
-	.mask	= CLOCKSOURCE_MASK(56),
-};
-
-static struct timecounter timecounter;
-
-struct timecounter *arch_timer_get_timecounter(void)
-{
-	return &timecounter;
-}
-
-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());
-
-	if (arch_timer_use_virtual)
-		disable_percpu_irq(arch_timer_ppi[VIRT_PPI]);
-	else {
-		disable_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI]);
-		if (arch_timer_ppi[PHYS_NONSECURE_PPI])
-			disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]);
-	}
-
-	clk->set_mode(CLOCK_EVT_MODE_UNUSED, clk);
-}
-
-static int __cpuinit arch_timer_cpu_notify(struct notifier_block *self,
-					   unsigned long action, void *hcpu)
-{
-	struct clock_event_device *evt = this_cpu_ptr(arch_timer_evt);
-
-	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_STARTING:
-		arch_timer_setup(evt);
-		break;
-	case CPU_DYING:
-		arch_timer_stop(evt);
-		break;
-	}
-
-	return NOTIFY_OK;
-}
-
-static struct notifier_block arch_timer_cpu_nb __cpuinitdata = {
-	.notifier_call = arch_timer_cpu_notify,
-};
+static struct delay_timer arch_delay_timer;
 
-static int __init arch_timer_register(void)
+static void __init arch_timer_delay_timer_register(void)
 {
-	int err;
-	int ppi;
-
-	err = arch_timer_available();
-	if (err)
-		goto out;
-
-	arch_timer_evt = alloc_percpu(struct clock_event_device);
-	if (!arch_timer_evt) {
-		err = -ENOMEM;
-		goto out;
-	}
-
-	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
-	cyclecounter.mult = clocksource_counter.mult;
-	cyclecounter.shift = clocksource_counter.shift;
-	timecounter_init(&timecounter, &cyclecounter,
-			 arch_counter_get_cntpct());
-
-	if (arch_timer_use_virtual) {
-		ppi = arch_timer_ppi[VIRT_PPI];
-		err = request_percpu_irq(ppi, arch_timer_handler_virt,
-					 "arch_timer", arch_timer_evt);
-	} else {
-		ppi = arch_timer_ppi[PHYS_SECURE_PPI];
-		err = request_percpu_irq(ppi, arch_timer_handler_phys,
-					 "arch_timer", arch_timer_evt);
-		if (!err && arch_timer_ppi[PHYS_NONSECURE_PPI]) {
-			ppi = arch_timer_ppi[PHYS_NONSECURE_PPI];
-			err = request_percpu_irq(ppi, arch_timer_handler_phys,
-						 "arch_timer", arch_timer_evt);
-			if (err)
-				free_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI],
-						arch_timer_evt);
-		}
-	}
-
-	if (err) {
-		pr_err("arch_timer: can't register interrupt %d (%d)\n",
-		       ppi, err);
-		goto out_free;
-	}
-
-	err = register_cpu_notifier(&arch_timer_cpu_nb);
-	if (err)
-		goto out_free_irq;
-
-	/* Immediately configure the timer on the boot CPU */
-	arch_timer_setup(this_cpu_ptr(arch_timer_evt));
-
 	/* Use the architected timer for the delay loop. */
-	arch_delay_timer.read_current_timer = &arch_timer_read_current_timer;
-	arch_delay_timer.freq = arch_timer_rate;
+	arch_delay_timer.read_current_timer = arch_timer_read_counter_long;
+	arch_delay_timer.freq = arch_timer_get_rate();
 	register_current_timer_delay(&arch_delay_timer);
-	return 0;
-
-out_free_irq:
-	if (arch_timer_use_virtual)
-		free_percpu_irq(arch_timer_ppi[VIRT_PPI], arch_timer_evt);
-	else {
-		free_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI],
-				arch_timer_evt);
-		if (arch_timer_ppi[PHYS_NONSECURE_PPI])
-			free_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI],
-					arch_timer_evt);
-	}
-
-out_free:
-	free_percpu(arch_timer_evt);
-out:
-	return err;
 }
 
-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;
-	int i;
+	int ret;
 
-	np = of_find_matching_node(NULL, arch_timer_of_match);
-	if (!np) {
-		pr_err("arch_timer: can't find DT node\n");
-		return -ENODEV;
-	}
+	ret = arch_timer_init();
+	if (ret)
+		return ret;
 
-	/* 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_delay_timer_register();
 
-	for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)
-		arch_timer_ppi[i] = irq_of_parse_and_map(np, i);
-
-	of_node_put(np);
-
-	/*
-	 * If no interrupt provided for virtual timer, we'll have to
-	 * stick to the physical timer. It'd better be accessible...
-	 */
-	if (!arch_timer_ppi[VIRT_PPI]) {
-		arch_timer_use_virtual = false;
-
-		if (!arch_timer_ppi[PHYS_SECURE_PPI] ||
-		    !arch_timer_ppi[PHYS_NONSECURE_PPI]) {
-			pr_warn("arch_timer: No interrupt available, giving up\n");
-			return -EINVAL;
-		}
-	}
-
-	return arch_timer_register();
+	return 0;
 }
 
 int __init arch_timer_sched_clock_init(void)
 {
-	int err;
-
-	err = arch_timer_available();
-	if (err)
-		return err;
+	if (arch_timer_get_rate() == 0)
+		return -ENXIO;
 
-	setup_sched_clock(arch_timer_read_counter32,
-			  32, arch_timer_rate);
+	setup_sched_clock(arch_timer_read_counter_u32,
+			  32, arch_timer_get_rate());
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 41b581f..403096c 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -76,12 +76,12 @@ config ARCH_OMAP4
 
 config SOC_OMAP5
 	bool "TI OMAP5"
-	select ARM_ARCH_TIMER
 	select ARM_CPU_SUSPEND if PM
 	select ARM_GIC
 	select CPU_V7
 	select HAVE_SMP
 	select COMMON_CLK
+	select USE_ARM_ARCH_TIMER
 
 comment "OMAP Core Type"
 	depends on ARCH_OMAP2
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 7fdcbd3..dbb085a 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -58,3 +58,6 @@ config CLKSRC_ARM_GENERIC
 	def_bool y if ARM64
 	help
 	  This option enables support for the ARM generic timer.
+
+config ARM_ARCH_TIMER
+	bool
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index f93453d..32f858c 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835_timer.o
 obj-$(CONFIG_SUNXI_TIMER)	+= sunxi_timer.o
 
 obj-$(CONFIG_CLKSRC_ARM_GENERIC)	+= arm_generic.o
+obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
new file mode 100644
index 0000000..823f2df
--- /dev/null
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -0,0 +1,374 @@
+/*
+ *  linux/drivers/clocksource/arm_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/device.h>
+#include <linux/smp.h>
+#include <linux/cpu.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/of_irq.h>
+#include <linux/io.h>
+
+#include <asm/arch_timer.h>
+
+#include <clocksource/arm_arch_timer.h>
+
+static u32 arch_timer_rate;
+
+enum ppi_nr {
+	PHYS_SECURE_PPI,
+	PHYS_NONSECURE_PPI,
+	VIRT_PPI,
+	HYP_PPI,
+	MAX_TIMER_PPI
+};
+
+static int arch_timer_ppi[MAX_TIMER_PPI];
+
+static struct clock_event_device __percpu *arch_timer_evt;
+
+static bool arch_timer_use_virtual = true;
+
+/*
+ * Architected system timer support.
+ */
+
+static inline irqreturn_t timer_handler(const int access,
+					struct clock_event_device *evt)
+{
+	unsigned long ctrl;
+	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL);
+	if (ctrl & ARCH_TIMER_CTRL_IT_STAT) {
+		ctrl |= ARCH_TIMER_CTRL_IT_MASK;
+		arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl);
+		evt->event_handler(evt);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static irqreturn_t arch_timer_handler_virt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	return timer_handler(ARCH_TIMER_VIRT_ACCESS, evt);
+}
+
+static irqreturn_t arch_timer_handler_phys(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	return timer_handler(ARCH_TIMER_PHYS_ACCESS, evt);
+}
+
+static inline void timer_set_mode(const int access, int mode)
+{
+	unsigned long ctrl;
+	switch (mode) {
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL);
+		ctrl &= ~ARCH_TIMER_CTRL_ENABLE;
+		arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl);
+		break;
+	default:
+		break;
+	}
+}
+
+static void arch_timer_set_mode_virt(enum clock_event_mode mode,
+				     struct clock_event_device *clk)
+{
+	timer_set_mode(ARCH_TIMER_VIRT_ACCESS, mode);
+}
+
+static void arch_timer_set_mode_phys(enum clock_event_mode mode,
+				     struct clock_event_device *clk)
+{
+	timer_set_mode(ARCH_TIMER_PHYS_ACCESS, mode);
+}
+
+static inline void set_next_event(const int access, unsigned long evt)
+{
+	unsigned long ctrl;
+	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL);
+	ctrl |= ARCH_TIMER_CTRL_ENABLE;
+	ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
+	arch_timer_reg_write(access, ARCH_TIMER_REG_TVAL, evt);
+	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl);
+}
+
+static int arch_timer_set_next_event_virt(unsigned long evt,
+					  struct clock_event_device *unused)
+{
+	set_next_event(ARCH_TIMER_VIRT_ACCESS, evt);
+	return 0;
+}
+
+static int arch_timer_set_next_event_phys(unsigned long evt,
+					  struct clock_event_device *unused)
+{
+	set_next_event(ARCH_TIMER_PHYS_ACCESS, evt);
+	return 0;
+}
+
+static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
+{
+	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
+	clk->name = "arch_sys_timer";
+	clk->rating = 450;
+	if (arch_timer_use_virtual) {
+		clk->irq = arch_timer_ppi[VIRT_PPI];
+		clk->set_mode = arch_timer_set_mode_virt;
+		clk->set_next_event = arch_timer_set_next_event_virt;
+	} else {
+		clk->irq = arch_timer_ppi[PHYS_SECURE_PPI];
+		clk->set_mode = arch_timer_set_mode_phys;
+		clk->set_next_event = arch_timer_set_next_event_phys;
+	}
+
+	clk->cpumask = cpumask_of(smp_processor_id());
+
+	clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, NULL);
+
+	clockevents_config_and_register(clk, arch_timer_rate,
+					0xf, 0x7fffffff);
+
+	if (arch_timer_use_virtual)
+		enable_percpu_irq(arch_timer_ppi[VIRT_PPI], 0);
+	else {
+		enable_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI], 0);
+		if (arch_timer_ppi[PHYS_NONSECURE_PPI])
+			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
+	}
+
+	arch_counter_set_user_access();
+
+	return 0;
+}
+
+static int arch_timer_available(void)
+{
+	u32 freq;
+
+	if (arch_timer_rate == 0) {
+		freq = arch_timer_get_cntfrq();
+		/* Check the timer frequency. */
+		if (freq == 0) {
+			pr_warn("Architected timer frequency not available\n");
+			return -EINVAL;
+		}
+
+		arch_timer_rate = freq;
+	}
+
+	pr_info_once("Architected local timer running at %lu.%02luMHz (%s).\n",
+		     (unsigned long)arch_timer_rate / 1000000,
+		     (unsigned long)(arch_timer_rate / 10000) % 100,
+		     arch_timer_use_virtual ? "virt" : "phys");
+	return 0;
+}
+
+u32 arch_timer_get_rate(void)
+{
+	return arch_timer_rate;
+}
+
+u64 arch_timer_read_counter(void)
+{
+	if (arch_timer_use_virtual)
+		return arch_counter_get_cntvct();
+	else
+		return arch_counter_get_cntpct();
+}
+
+static cycle_t arch_counter_read(struct clocksource *cs)
+{
+	return arch_timer_read_counter();
+}
+
+static cycle_t arch_counter_read_cc(const struct cyclecounter *cc)
+{
+	return arch_timer_read_counter();
+}
+
+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 struct cyclecounter cyclecounter = {
+	.read	= arch_counter_read_cc,
+	.mask	= CLOCKSOURCE_MASK(56),
+};
+
+static struct timecounter timecounter;
+
+struct timecounter *arch_timer_get_timecounter(void)
+{
+	return &timecounter;
+}
+
+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());
+
+	if (arch_timer_use_virtual)
+		disable_percpu_irq(arch_timer_ppi[VIRT_PPI]);
+	else {
+		disable_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI]);
+		if (arch_timer_ppi[PHYS_NONSECURE_PPI])
+			disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]);
+	}
+
+	clk->set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+}
+
+static int __cpuinit arch_timer_cpu_notify(struct notifier_block *self,
+					   unsigned long action, void *hcpu)
+{
+	struct clock_event_device *evt = this_cpu_ptr(arch_timer_evt);
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_STARTING:
+		arch_timer_setup(evt);
+		break;
+	case CPU_DYING:
+		arch_timer_stop(evt);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block arch_timer_cpu_nb __cpuinitdata = {
+	.notifier_call = arch_timer_cpu_notify,
+};
+
+static int __init arch_timer_register(void)
+{
+	int err;
+	int ppi;
+
+	err = arch_timer_available();
+	if (err)
+		goto out;
+
+	arch_timer_evt = alloc_percpu(struct clock_event_device);
+	if (!arch_timer_evt) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
+	cyclecounter.mult = clocksource_counter.mult;
+	cyclecounter.shift = clocksource_counter.shift;
+	timecounter_init(&timecounter, &cyclecounter,
+			 arch_counter_get_cntpct());
+
+	if (arch_timer_use_virtual) {
+		ppi = arch_timer_ppi[VIRT_PPI];
+		err = request_percpu_irq(ppi, arch_timer_handler_virt,
+					 "arch_timer", arch_timer_evt);
+	} else {
+		ppi = arch_timer_ppi[PHYS_SECURE_PPI];
+		err = request_percpu_irq(ppi, arch_timer_handler_phys,
+					 "arch_timer", arch_timer_evt);
+		if (!err && arch_timer_ppi[PHYS_NONSECURE_PPI]) {
+			ppi = arch_timer_ppi[PHYS_NONSECURE_PPI];
+			err = request_percpu_irq(ppi, arch_timer_handler_phys,
+						 "arch_timer", arch_timer_evt);
+			if (err)
+				free_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI],
+						arch_timer_evt);
+		}
+	}
+
+	if (err) {
+		pr_err("arch_timer: can't register interrupt %d (%d)\n",
+		       ppi, err);
+		goto out_free;
+	}
+
+	err = register_cpu_notifier(&arch_timer_cpu_nb);
+	if (err)
+		goto out_free_irq;
+
+	/* Immediately configure the timer on the boot CPU */
+	arch_timer_setup(this_cpu_ptr(arch_timer_evt));
+
+	return 0;
+
+out_free_irq:
+	if (arch_timer_use_virtual)
+		free_percpu_irq(arch_timer_ppi[VIRT_PPI], arch_timer_evt);
+	else {
+		free_percpu_irq(arch_timer_ppi[PHYS_SECURE_PPI],
+				arch_timer_evt);
+		if (arch_timer_ppi[PHYS_NONSECURE_PPI])
+			free_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI],
+					arch_timer_evt);
+	}
+
+out_free:
+	free_percpu(arch_timer_evt);
+out:
+	return err;
+}
+
+static const struct of_device_id arch_timer_of_match[] __initconst = {
+	{ .compatible	= "arm,armv7-timer",	},
+	{},
+};
+
+int __init arch_timer_init(void)
+{
+	struct device_node *np;
+	u32 freq;
+	int i;
+
+	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;
+
+	for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)
+		arch_timer_ppi[i] = irq_of_parse_and_map(np, i);
+
+	of_node_put(np);
+
+	/*
+	 * If no interrupt provided for virtual timer, we'll have to
+	 * stick to the physical timer. It'd better be accessible...
+	 */
+	if (!arch_timer_ppi[VIRT_PPI]) {
+		arch_timer_use_virtual = false;
+
+		if (!arch_timer_ppi[PHYS_SECURE_PPI] ||
+		    !arch_timer_ppi[PHYS_NONSECURE_PPI]) {
+			pr_warn("arch_timer: No interrupt available, giving up\n");
+			return -EINVAL;
+		}
+	}
+
+	return arch_timer_register();
+}
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
new file mode 100644
index 0000000..dff2f22
--- /dev/null
+++ b/include/clocksource/arm_arch_timer.h
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2012 ARM Ltd.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __CLKSOURCE_ARM_ARCH_TIMER_H
+#define __CLKSOURCE_ARM_ARCH_TIMER_H
+
+#include <linux/clocksource.h>
+#include <linux/types.h>
+
+#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
+#define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
+#define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
+
+#define ARCH_TIMER_REG_CTRL		0
+#define ARCH_TIMER_REG_TVAL		1
+
+#define ARCH_TIMER_PHYS_ACCESS		0
+#define ARCH_TIMER_VIRT_ACCESS		1
+
+#ifdef CONFIG_ARM_ARCH_TIMER
+
+extern int arch_timer_init(void);
+extern u32 arch_timer_get_rate(void);
+extern u64 arch_timer_read_counter(void);
+extern struct timecounter *arch_timer_get_timecounter(void);
+
+#else
+
+static inline int arch_timer_init(void)
+{
+	return -ENXIO;
+}
+
+static inline u32 arch_timer_get_rate(void)
+{
+	return 0;
+}
+
+extern u64 arch_timer_read_counter(void)
+{
+	return 0;
+}
+
+extern struct timecounter *arch_timer_get_timecounter(void)
+{
+	return NULL;
+}
+
+#endif
+
+#endif
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 10/11] arm64: move from arm_generic to arm_arch_timer
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
                   ` (8 preceding siblings ...)
  2013-01-09 16:07 ` [PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:50   ` Santosh Shilimkar
  2013-01-09 16:07 ` [PATCHv2 11/11] Documentation: Add ARMv8 to arch_timer devicetree Mark Rutland
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

The arch_timer driver supports a superset of the functionality of the
arm_generic driver, and is not tied to a particular arch.

This patch moves arm64 to use the arch_timer driver, gaining additional
functionality in doing so, and removes the (now unused) arm_generic
driver. Timer-related hooks specific to arm64 are moved into
arch/arm64/kernel/time.c.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/Kconfig                   |    1 +
 arch/arm64/include/asm/arch_timer.h  |  133 +++++++++++++++++++
 arch/arm64/include/asm/arm_generic.h |  100 ---------------
 arch/arm64/kernel/time.c             |   29 ++++-
 drivers/clocksource/Kconfig          |    5 -
 drivers/clocksource/Makefile         |    1 -
 drivers/clocksource/arm_arch_timer.c |    1 +
 drivers/clocksource/arm_generic.c    |  232 ----------------------------------
 include/clocksource/arm_generic.h    |   21 ---
 9 files changed, 162 insertions(+), 361 deletions(-)
 create mode 100644 arch/arm64/include/asm/arch_timer.h
 delete mode 100644 arch/arm64/include/asm/arm_generic.h
 delete mode 100644 drivers/clocksource/arm_generic.c
 delete mode 100644 include/clocksource/arm_generic.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9c829b0..6e9447c 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -2,6 +2,7 @@ config ARM64
 	def_bool y
 	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
 	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
+	select ARM_ARCH_TIMER
 	select COMMON_CLK
 	select GENERIC_CLOCKEVENTS
 	select GENERIC_HARDIRQS_NO_DEPRECATED
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
new file mode 100644
index 0000000..91e2a6a
--- /dev/null
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -0,0 +1,133 @@
+/*
+ * arch/arm64/include/asm/arch_timer.h
+ *
+ * Copyright (C) 2012 ARM Ltd.
+ * Author: Marc Zyngier <marc.zyngier@arm.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __ASM_ARCH_TIMER_H
+#define __ASM_ARCH_TIMER_H
+
+#include <asm/barrier.h>
+
+#include <linux/init.h>
+#include <linux/types.h>
+
+#include <clocksource/arm_arch_timer.h>
+
+static inline void arch_timer_reg_write(int access, int reg, u32 val)
+{
+	if (access == ARCH_TIMER_PHYS_ACCESS) {
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			asm volatile("msr cntp_ctl_el0,  %0" : : "r" (val));
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			asm volatile("msr cntp_tval_el0, %0" : : "r" (val));
+			break;
+		default:
+			BUILD_BUG();
+		}
+	} else if (access == ARCH_TIMER_VIRT_ACCESS) {
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			asm volatile("msr cntv_ctl_el0,  %0" : : "r" (val));
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			asm volatile("msr cntv_tval_el0, %0" : : "r" (val));
+			break;
+		default:
+			BUILD_BUG();
+		}
+	} else {
+		BUILD_BUG();
+	}
+
+	isb();
+}
+
+static inline u32 arch_timer_reg_read(int access, int reg)
+{
+	u32 val;
+
+	if (access == ARCH_TIMER_PHYS_ACCESS) {
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			asm volatile("mrs %0,  cntp_ctl_el0" : "=r" (val));
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			asm volatile("mrs %0, cntp_tval_el0" : "=r" (val));
+			break;
+		default:
+			BUILD_BUG();
+		}
+	} else if (access == ARCH_TIMER_VIRT_ACCESS) {
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			asm volatile("mrs %0,  cntv_ctl_el0" : "=r" (val));
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			asm volatile("mrs %0, cntv_tval_el0" : "=r" (val));
+			break;
+		default:
+			BUILD_BUG();
+		}
+	} else {
+		BUILD_BUG();
+	}
+
+	return val;
+}
+
+static inline u32 arch_timer_get_cntfrq(void)
+{
+	u32 val;
+	asm volatile("mrs %0,   cntfrq_el0" : "=r" (val));
+	return val;
+}
+
+static inline void __cpuinit arch_counter_set_user_access(void)
+{
+	u32 cntkctl;
+
+	/* Disable user access to the timers and the physical counter. */
+	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
+	cntkctl &= ~((3 << 8) | (1 << 0));
+
+	/* Enable user access to the virtual counter and frequency. */
+	cntkctl |= (1 << 1);
+	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
+}
+
+static inline u64 arch_counter_get_cntpct(void)
+{
+	u64 cval;
+
+	isb();
+	asm volatile("mrs %0, cntpct_el0" : "=r" (cval));
+
+	return cval;
+}
+
+static inline u64 arch_counter_get_cntvct(void)
+{
+	u64 cval;
+
+	isb();
+	asm volatile("mrs %0, cntvct_el0" : "=r" (cval));
+
+	return cval;
+}
+
+#endif
diff --git a/arch/arm64/include/asm/arm_generic.h b/arch/arm64/include/asm/arm_generic.h
deleted file mode 100644
index df2aeb8..0000000
--- a/arch/arm64/include/asm/arm_generic.h
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * arch/arm64/include/asm/arm_generic.h
- *
- * Copyright (C) 2012 ARM Ltd.
- * Author: Marc Zyngier <marc.zyngier@arm.com>
- *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-#ifndef __ASM_ARM_GENERIC_H
-#define __ASM_ARM_GENERIC_H
-
-#include <linux/clocksource.h>
-
-#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
-#define ARCH_TIMER_CTRL_IMASK		(1 << 1)
-#define ARCH_TIMER_CTRL_ISTATUS		(1 << 2)
-
-#define ARCH_TIMER_REG_CTRL		0
-#define ARCH_TIMER_REG_FREQ		1
-#define ARCH_TIMER_REG_TVAL		2
-
-static inline void arch_timer_reg_write(int reg, u32 val)
-{
-	switch (reg) {
-	case ARCH_TIMER_REG_CTRL:
-		asm volatile("msr cntp_ctl_el0,  %0" : : "r" (val));
-		break;
-	case ARCH_TIMER_REG_TVAL:
-		asm volatile("msr cntp_tval_el0, %0" : : "r" (val));
-		break;
-	default:
-		BUILD_BUG();
-	}
-
-	isb();
-}
-
-static inline u32 arch_timer_reg_read(int reg)
-{
-	u32 val;
-
-	switch (reg) {
-	case ARCH_TIMER_REG_CTRL:
-		asm volatile("mrs %0,  cntp_ctl_el0" : "=r" (val));
-		break;
-	case ARCH_TIMER_REG_FREQ:
-		asm volatile("mrs %0,   cntfrq_el0" : "=r" (val));
-		break;
-	case ARCH_TIMER_REG_TVAL:
-		asm volatile("mrs %0, cntp_tval_el0" : "=r" (val));
-		break;
-	default:
-		BUILD_BUG();
-	}
-
-	return val;
-}
-
-static inline void __cpuinit arch_counter_enable_user_access(void)
-{
-	u32 cntkctl;
-
-	/* Disable user access to the timers and the physical counter. */
-	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
-	cntkctl &= ~((3 << 8) | (1 << 0));
-
-	/* Enable user access to the virtual counter and frequency. */
-	cntkctl |= (1 << 1);
-	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
-}
-
-static inline cycle_t arch_counter_get_cntpct(void)
-{
-	cycle_t cval;
-
-	asm volatile("mrs %0, cntpct_el0" : "=r" (cval));
-
-	return cval;
-}
-
-static inline cycle_t arch_counter_get_cntvct(void)
-{
-	cycle_t cval;
-
-	asm volatile("mrs %0, cntvct_el0" : "=r" (cval));
-
-	return cval;
-}
-
-#endif
diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c
index 3b4b725..b0ef18d 100644
--- a/arch/arm64/kernel/time.c
+++ b/arch/arm64/kernel/time.c
@@ -31,8 +31,9 @@
 #include <linux/syscore_ops.h>
 #include <linux/timer.h>
 #include <linux/irq.h>
+#include <linux/delay.h>
 
-#include <clocksource/arm_generic.h>
+#include <clocksource/arm_arch_timer.h>
 
 #include <asm/thread_info.h>
 #include <asm/stacktrace.h>
@@ -59,7 +60,31 @@ unsigned long profile_pc(struct pt_regs *regs)
 EXPORT_SYMBOL(profile_pc);
 #endif
 
+static u64 sched_clock_mult __read_mostly;
+
+unsigned long long notrace sched_clock(void)
+{
+	return arch_timer_read_counter() * sched_clock_mult;
+}
+
+int read_current_timer(unsigned long *timer_value)
+{
+	*timer_value = arch_timer_read_counter();
+	return 0;
+}
+
 void __init time_init(void)
 {
-	arm_generic_timer_init();
+	u32 arch_timer_rate;
+
+	if (arch_timer_init())
+		panic("Unable to initialise architected timer.\n");
+
+	arch_timer_rate = arch_timer_get_rate();
+
+	/* Cache the sched_clock multiplier to save a divide in the hot path. */
+	sched_clock_mult = NSEC_PER_SEC / arch_timer_rate;
+
+	/* Calibrate the delay loop directly */
+	lpj_fine = arch_timer_rate / HZ;
 }
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index dbb085a..6479842 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -54,10 +54,5 @@ config CLKSRC_DBX500_PRCMU_SCHED_CLOCK
 	help
 	  Use the always on PRCMU Timer as sched_clock
 
-config CLKSRC_ARM_GENERIC
-	def_bool y if ARM64
-	help
-	  This option enables support for the ARM generic timer.
-
 config ARM_ARCH_TIMER
 	bool
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 32f858c..e69511c 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -17,5 +17,4 @@ obj-$(CONFIG_ARMADA_370_XP_TIMER)	+= time-armada-370-xp.o
 obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835_timer.o
 obj-$(CONFIG_SUNXI_TIMER)	+= sunxi_timer.o
 
-obj-$(CONFIG_CLKSRC_ARM_GENERIC)	+= arm_generic.o
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 823f2df..cfea474 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -332,6 +332,7 @@ out:
 
 static const struct of_device_id arch_timer_of_match[] __initconst = {
 	{ .compatible	= "arm,armv7-timer",	},
+	{ .compatible	= "arm,armv8-timer",	},
 	{},
 };
 
diff --git a/drivers/clocksource/arm_generic.c b/drivers/clocksource/arm_generic.c
deleted file mode 100644
index 8ae1a61..0000000
--- a/drivers/clocksource/arm_generic.c
+++ /dev/null
@@ -1,232 +0,0 @@
-/*
- * Generic timers support
- *
- * Copyright (C) 2012 ARM Ltd.
- * Author: Marc Zyngier <marc.zyngier@arm.com>
- *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#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/interrupt.h>
-#include <linux/clockchips.h>
-#include <linux/of_irq.h>
-#include <linux/io.h>
-
-#include <clocksource/arm_generic.h>
-
-#include <asm/arm_generic.h>
-
-static u32 arch_timer_rate;
-static u64 sched_clock_mult __read_mostly;
-static DEFINE_PER_CPU(struct clock_event_device, arch_timer_evt);
-static int arch_timer_ppi;
-
-static irqreturn_t arch_timer_handle_irq(int irq, void *dev_id)
-{
-	struct clock_event_device *evt = dev_id;
-	unsigned long ctrl;
-
-	ctrl = arch_timer_reg_read(ARCH_TIMER_REG_CTRL);
-	if (ctrl & ARCH_TIMER_CTRL_ISTATUS) {
-		ctrl |= ARCH_TIMER_CTRL_IMASK;
-		arch_timer_reg_write(ARCH_TIMER_REG_CTRL, ctrl);
-		evt->event_handler(evt);
-		return IRQ_HANDLED;
-	}
-
-	return IRQ_NONE;
-}
-
-static void arch_timer_stop(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_stop();
-		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_IMASK;
-
-	arch_timer_reg_write(ARCH_TIMER_REG_TVAL, evt);
-	arch_timer_reg_write(ARCH_TIMER_REG_CTRL, ctrl);
-
-	return 0;
-}
-
-static void __cpuinit arch_timer_setup(struct clock_event_device *clk)
-{
-	/* Let's make sure the timer is off before doing anything else */
-	arch_timer_stop();
-
-	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
-	clk->name = "arch_sys_timer";
-	clk->rating = 400;
-	clk->set_mode = arch_timer_set_mode;
-	clk->set_next_event = arch_timer_set_next_event;
-	clk->irq = arch_timer_ppi;
-	clk->cpumask = cpumask_of(smp_processor_id());
-
-	clockevents_config_and_register(clk, arch_timer_rate,
-					0xf, 0x7fffffff);
-
-	enable_percpu_irq(clk->irq, 0);
-
-	/* Ensure the virtual counter is visible to userspace for the vDSO. */
-	arch_counter_enable_user_access();
-}
-
-static void __init arch_timer_calibrate(void)
-{
-	if (arch_timer_rate == 0) {
-		arch_timer_reg_write(ARCH_TIMER_REG_CTRL, 0);
-		arch_timer_rate = arch_timer_reg_read(ARCH_TIMER_REG_FREQ);
-
-		/* Check the timer frequency. */
-		if (arch_timer_rate == 0)
-			panic("Architected timer frequency is set to zero.\n"
-			      "You must set this in your .dts file\n");
-	}
-
-	/* Cache the sched_clock multiplier to save a divide in the hot path. */
-
-	sched_clock_mult = DIV_ROUND_CLOSEST(NSEC_PER_SEC, arch_timer_rate);
-
-	pr_info("Architected local timer running at %u.%02uMHz.\n",
-		 arch_timer_rate / 1000000, (arch_timer_rate / 10000) % 100);
-}
-
-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 | CLOCK_SOURCE_VALID_FOR_HRES),
-};
-
-int read_current_timer(unsigned long *timer_value)
-{
-	*timer_value = arch_counter_get_cntpct();
-	return 0;
-}
-
-unsigned long long notrace sched_clock(void)
-{
-	return arch_counter_get_cntvct() * sched_clock_mult;
-}
-
-static int __cpuinit arch_timer_cpu_notify(struct notifier_block *self,
-					   unsigned long action, void *hcpu)
-{
-	int cpu = (long)hcpu;
-	struct clock_event_device *clk = per_cpu_ptr(&arch_timer_evt, cpu);
-
-	switch(action) {
-	case CPU_STARTING:
-	case CPU_STARTING_FROZEN:
-		arch_timer_setup(clk);
-		break;
-
-	case CPU_DYING:
-	case CPU_DYING_FROZEN:
-		pr_debug("arch_timer_teardown disable IRQ%d cpu #%d\n",
-			 clk->irq, cpu);
-		disable_percpu_irq(clk->irq);
-		arch_timer_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
-		break;
-	}
-
-	return NOTIFY_OK;
-}
-
-static struct notifier_block __cpuinitdata arch_timer_cpu_nb = {
-	.notifier_call = arch_timer_cpu_notify,
-};
-
-static const struct of_device_id arch_timer_of_match[] __initconst = {
-	{ .compatible = "arm,armv8-timer" },
-	{},
-};
-
-int __init arm_generic_timer_init(void)
-{
-	struct device_node *np;
-	int err;
-	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_calibrate();
-
-	arch_timer_ppi = irq_of_parse_and_map(np, 0);
-	pr_info("arch_timer: found %s irq %d\n", np->name, arch_timer_ppi);
-
-	err = request_percpu_irq(arch_timer_ppi, arch_timer_handle_irq,
-				 np->name, &arch_timer_evt);
-	if (err) {
-		pr_err("arch_timer: can't register interrupt %d (%d)\n",
-		       arch_timer_ppi, err);
-		return err;
-	}
-
-	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
-
-	/* Calibrate the delay loop directly */
-	lpj_fine = DIV_ROUND_CLOSEST(arch_timer_rate, HZ);
-
-	/* Immediately configure the timer on the boot CPU */
-	arch_timer_setup(this_cpu_ptr(&arch_timer_evt));
-
-	register_cpu_notifier(&arch_timer_cpu_nb);
-
-	return 0;
-}
diff --git a/include/clocksource/arm_generic.h b/include/clocksource/arm_generic.h
deleted file mode 100644
index 5b41b0d..0000000
--- a/include/clocksource/arm_generic.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * Copyright (C) 2012 ARM Ltd.
- *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-#ifndef __CLKSOURCE_ARM_GENERIC_H
-#define __CLKSOURCE_ARM_GENERIC_H
-
-extern int arm_generic_timer_init(void);
-
-#endif
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 11/11] Documentation: Add ARMv8 to arch_timer devicetree
  2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
                   ` (9 preceding siblings ...)
  2013-01-09 16:07 ` [PATCHv2 10/11] arm64: move from arm_generic to arm_arch_timer Mark Rutland
@ 2013-01-09 16:07 ` Mark Rutland
  2013-01-11 13:52   ` Santosh Shilimkar
  10 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-09 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 .../devicetree/bindings/arm/arch_timer.txt         |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index 52478c8..20746e5 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -1,13 +1,14 @@
 * ARM architected timer
 
-ARM Cortex-A7 and Cortex-A15 have a per-core architected timer, which
-provides per-cpu timers.
+ARM cores may have a per-core architected timer, which provides per-cpu timers.
 
 The timer is attached to a GIC to deliver its per-processor interrupts.
 
 ** Timer node properties:
 
-- compatible : Should at least contain "arm,armv7-timer".
+- compatible : Should at least contain one of
+	"arm,armv7-timer"
+	"arm,armv8-timer"
 
 - interrupts : Interrupt list for secure, non-secure, virtual and
   hypervisor timers, in that order.
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCHv2 01/11] arm: arch_timer: balance device_node refcounting
  2013-01-09 16:07 ` [PATCHv2 01/11] arm: arch_timer: balance device_node refcounting Mark Rutland
@ 2013-01-11 12:59   ` Santosh Shilimkar
  0 siblings, 0 replies; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 12:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> When we get the device_node for the arch timer, it's refcount is
> automatically incremented in of_find_matching_node, but it is
> never decremented.
>
> This patch decrements the refcount on the node after we're finished
> using it.
>
> Reported-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
Looks right.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 02/11] arm: arch_timer: remove redundant available check
  2013-01-09 16:07 ` [PATCHv2 02/11] arm: arch_timer: remove redundant available check Mark Rutland
@ 2013-01-11 13:11   ` Santosh Shilimkar
  2013-01-11 14:07     ` Mark Rutland
  0 siblings, 1 reply; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> This check is a holdover from the pre-devicetree days. As the timer
> is not probed except by platforms which register it via devicetree,
> it's not strictly necessary.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
Multi-platform build without DT could still benefit from the
check but I guess the most of the A15 based platform are DT
only, so its should be good to get rid of the check.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 03/11] arm: arch_timer: use u64/u32 for register data
  2013-01-09 16:07 ` [PATCHv2 03/11] arm: arch_timer: use u64/u32 for register data Mark Rutland
@ 2013-01-11 13:19   ` Santosh Shilimkar
  0 siblings, 0 replies; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> To ensure the correct size of types, use u64 for the return value of
> arch_timer_get_cnt{p,v}ct, and u32 for arch_timer_rate, matching the
> size of the registers these values are taken from. While we're changing
> them anyway, simplify the implementation of arch_timer_get_cnt{p,v}ct.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 04/11] arm: arch_timer: standardise counter reading
  2013-01-09 16:07 ` [PATCHv2 04/11] arm: arch_timer: standardise counter reading Mark Rutland
@ 2013-01-11 13:23   ` Santosh Shilimkar
  2013-01-15 10:25     ` Mark Rutland
  0 siblings, 1 reply; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> We're currently inconsistent with respect to our accesses to the
> physical and virtual counters, mixing and matching the two.
>
> This patch introduces and uses a function for accessing the correct
> counter based on whether we're using physical or virtual interrupts.
> All current accesses to the counter accessors are redirected through
> it.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/arm/kernel/arch_timer.c |   48 ++++++++++-------------------------------
>   1 files changed, 12 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> index 498c29f..0d2681c 100644
> --- a/arch/arm/kernel/arch_timer.c
> +++ b/arch/arm/kernel/arch_timer.c
> @@ -272,51 +272,32 @@ static int arch_timer_available(void)
>   	return 0;
>   }
>
> -static u32 notrace arch_counter_get_cntpct32(void)
> +u64 arch_timer_read_counter(void)
>   {
> -	cycle_t cnt = arch_counter_get_cntpct();
> -
> -	/*
> -	 * The sched_clock infrastructure only knows about counters
> -	 * with at most 32bits. Forget about the upper 24 bits for the
> -	 * time being...
> -	 */
> -	return (u32)cnt;
> +	if (arch_timer_use_virtual)
> +		return arch_counter_get_cntvct();
> +	else
> +		return arch_counter_get_cntpct();
>   }
>

[...]

> @@ -489,18 +470,13 @@ int __init arch_timer_of_register(void)
>
>   int __init arch_timer_sched_clock_init(void)
>   {
> -	u32 (*cnt32)(void);
>   	int err;
>
>   	err = arch_timer_available();
>   	if (err)
>   		return err;
>
> -	if (arch_timer_use_virtual)
> -		cnt32 = arch_counter_get_cntvct32;
> -	else
> -		cnt32 = arch_counter_get_cntpct32;
> -
> -	setup_sched_clock(cnt32, 32, arch_timer_rate);
> +	setup_sched_clock(arch_timer_read_counter32,
> +			  32, arch_timer_rate);
>   	return 0;
>   }
>
I think the original idea had merit since the check was needed
in init code instead of proposed one which has if check for
every counter read function. No ?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 05/11] arm: arch_timer: split cntfrq accessor
  2013-01-09 16:07 ` [PATCHv2 05/11] arm: arch_timer: split cntfrq accessor Mark Rutland
@ 2013-01-11 13:27   ` Santosh Shilimkar
  2013-01-11 14:16     ` Mark Rutland
  0 siblings, 1 reply; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> The CNTFRQ register is not duplicated for physical and virtual timers,
> and accessing it as if it were is confusing.
>
> Instead, use a separate accessor which doesn't take the access type
> as a parameter.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/arm/kernel/arch_timer.c |   17 +++++++++--------
>   1 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> index 0d2681c..fc87d3d 100644
> --- a/arch/arm/kernel/arch_timer.c
> +++ b/arch/arm/kernel/arch_timer.c
> @@ -51,8 +51,7 @@ static bool arch_timer_use_virtual = true;
>   #define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
>
>   #define ARCH_TIMER_REG_CTRL		0
> -#define ARCH_TIMER_REG_FREQ		1
> -#define ARCH_TIMER_REG_TVAL		2
> +#define ARCH_TIMER_REG_TVAL		1
>
>   #define ARCH_TIMER_PHYS_ACCESS		0
>   #define ARCH_TIMER_VIRT_ACCESS		1
> @@ -101,9 +100,6 @@ static inline u32 arch_timer_reg_read(const int access, const int reg)
>   		case ARCH_TIMER_REG_TVAL:
>   			asm volatile("mrc p15, 0, %0, c14, c2, 0" : "=r" (val));
>   			break;
> -		case ARCH_TIMER_REG_FREQ:
> -			asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (val));
> -			break;
>   		}
>   	}
>
> @@ -121,6 +117,13 @@ static inline u32 arch_timer_reg_read(const int access, const int reg)
>   	return val;
>   }
>
> +static inline u32 arch_timer_get_cntfrq(void)
> +{
> +	u32 val;
> +	asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (val));
> +	return val;
> +}
> +
>   static inline u64 arch_counter_get_cntpct(void)
>   {
>   	u64 cval;
> @@ -253,9 +256,7 @@ static int arch_timer_available(void)
>   	u32 freq;
>
>   	if (arch_timer_rate == 0) {
> -		freq = arch_timer_reg_read(ARCH_TIMER_PHYS_ACCESS,
> -					   ARCH_TIMER_REG_FREQ);
> -
> +		freq = arch_timer_get_cntfrq();
Not related to this patch a new line here will be good.
>   		/* Check the timer frequency. */
>   		if (freq == 0) {
>   			pr_warn("Architected timer frequency not available\n");
>
Otherwise patch looks fine to me.
Acked-by: Santosh Shilimkar<santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 06/11] arm: arch_timer: factor out register accessors
  2013-01-09 16:07 ` [PATCHv2 06/11] arm: arch_timer: factor out register accessors Mark Rutland
@ 2013-01-11 13:32   ` Santosh Shilimkar
  2013-01-11 14:31     ` Mark Rutland
  0 siblings, 1 reply; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> Currently the arch_timer register accessors are thrown together with
> the main driver, preventing us from porting the driver to other
> architectures.
>
> This patch moves the register accessors into a header file, as with
> the arm64 version. Constants required by the accessors are also moved.
>
> Additionally isbs are added in arch_timer_get_cnt{v,p}ct to prevent
> the cpu from speculating the reads and returning stale values.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/arm/include/asm/arch_timer.h |  101 +++++++++++++++++++++++++++++++++++++
>   arch/arm/kernel/arch_timer.c      |   92 ---------------------------------
>   2 files changed, 101 insertions(+), 92 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index d40229d..701f2b7 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -1,13 +1,114 @@
>   #ifndef __ASMARM_ARCH_TIMER_H
>   #define __ASMARM_ARCH_TIMER_H
>
> +#include <asm/barrier.h>
>   #include <asm/errno.h>
> +
>   #include <linux/clocksource.h>
> +#include <linux/types.h>
>
>   #ifdef CONFIG_ARM_ARCH_TIMER
>   int arch_timer_of_register(void);
>   int arch_timer_sched_clock_init(void);
>   struct timecounter *arch_timer_get_timecounter(void);
> +
> +#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
> +#define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
> +#define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
> +
> +#define ARCH_TIMER_REG_CTRL		0
> +#define ARCH_TIMER_REG_TVAL		1
> +
> +#define ARCH_TIMER_PHYS_ACCESS		0
> +#define ARCH_TIMER_VIRT_ACCESS		1
> +
> +/*
> + * These register accessors are marked inline so the compiler can
> + * nicely work out which register we want, and chuck away the rest of
> + * the code. At least it does so with a recent GCC (4.6.3).
> + */
> +static inline void arch_timer_reg_write(const int access, const int reg, u32 val)
> +{
> +	if (access == ARCH_TIMER_PHYS_ACCESS) {
> +		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;
> +		}
> +	}
> +
> +	if (access == ARCH_TIMER_VIRT_ACCESS) {
> +		switch (reg) {
> +		case ARCH_TIMER_REG_CTRL:
> +			asm volatile("mcr p15, 0, %0, c14, c3, 1" : : "r" (val));
> +			break;
> +		case ARCH_TIMER_REG_TVAL:
> +			asm volatile("mcr p15, 0, %0, c14, c3, 0" : : "r" (val));
> +			break;
> +		}
> +	}
> +
> +	isb();
> +}
The isb() additions is actually a sepoerate fix. I suggest you to split
the subject patch into 1) Movement of header data 2) isb() additions.

Feel free to add my ack on updated patches if you agree.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api
  2013-01-09 16:07 ` [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api Mark Rutland
@ 2013-01-11 13:34   ` Santosh Shilimkar
  2013-01-11 16:46     ` Catalin Marinas
  0 siblings, 1 reply; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> Currently, the arch_timer driver is tied to the arm port, as it relies
> on code in arch/arm/smp.c to setup and teardown timers as cores are
> hotplugged on and off. The timer is registered through an arm-specific
> registration mechanism, preventing sharing the driver with the arm64
> port.
>
> This patch moves the driver to using a cpu notifier instead, making it
> easier to port.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
This is really a nit idea. I think we should do the same
for ARM gic code.

For the patch,
Acked-by : Santosh Shilimkar<santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
  2013-01-09 16:07 ` [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access Mark Rutland
@ 2013-01-11 13:40   ` Santosh Shilimkar
  2013-01-11 14:54     ` Mark Rutland
  2013-01-11 16:50     ` Catalin Marinas
  0 siblings, 2 replies; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> Several bits in CNTKCTL reset to 0, including PL0VTEN. For platforms
> using the generic timer which wish to have a fast gettimeofday vDSO
> implementation, these bits must be set to 1 by the kernel. On other
> platforms, the bootloader might enable userspace access when we don't
> want it.
>
> This patch adds arch_counter_set_user_access, which sets the PL0 access
> permissions to that required by the platform. For arm, this currently
minor nit.
s/arm/ARM

> means disabling all userspace access.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>   arch/arm/include/asm/arch_timer.h |   11 +++++++++++
>   arch/arm/kernel/arch_timer.c      |    2 ++
>   2 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index 701f2b7..05e3593 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -108,6 +108,17 @@ static inline u64 arch_counter_get_cntvct(void)
>   	return cval;
>   }
>
> +static inline void __cpuinit arch_counter_set_user_access(void)
> +{
> +	u32 cntkctl;
> +
> +	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
> +
> +	/* disable user access to everything */
> +	cntkctl &= ~((3 << 8) | (7 << 0));
> +
> +	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
> +}
>
>   #else
>   static inline int arch_timer_of_register(void)
> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> index 834d347..4f39e68 100644
> --- a/arch/arm/kernel/arch_timer.c
> +++ b/arch/arm/kernel/arch_timer.c
> @@ -155,6 +155,8 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
>   			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
>   	}
>
> +	arch_counter_set_user_access();
So how do you expect platform to enabled the user-space access in case
they want to access it for some cases.

Regards
Santosh

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource
  2013-01-09 16:07 ` [PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource Mark Rutland
@ 2013-01-11 13:48   ` Santosh Shilimkar
  2013-01-11 15:04     ` Mark Rutland
  0 siblings, 1 reply; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> The core functionality of the arch_timer driver is not directly tied to
> anything under arch/arm, and can be split out.
>
> This patch factors out the core of the arch_timer driver, so it can be
> shared with other architectures. A couple of functions are added so
> that architecture-specific code can interact with the driver without
> needing to touch its internals.
>
> The ARM_ARCH_TIMER config variable is moved out to
> drivers/clocksource/Kconfig, existing uses in arch/arm are replaced with
> USE_ARM_ARCH_TIMER, which selects it.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/arm/Kconfig                     |    3 +-
>   arch/arm/include/asm/arch_timer.h    |   19 +--
>   arch/arm/kernel/arch_timer.c         |  375 ++--------------------------------
>   arch/arm/mach-omap2/Kconfig          |    2 +-
>   drivers/clocksource/Kconfig          |    3 +
>   drivers/clocksource/Makefile         |    1 +
>   drivers/clocksource/arm_arch_timer.c |  374 +++++++++++++++++++++++++++++++++
>   include/clocksource/arm_arch_timer.h |   63 ++++++
>   8 files changed, 465 insertions(+), 375 deletions(-)
>   create mode 100644 drivers/clocksource/arm_arch_timer.c
>   create mode 100644 include/clocksource/arm_arch_timer.h
>
It would have been easy if you have formated the patch with -C option.
That will just leave the delta changes only and hiding the file
movement related diff.

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index f95ba14..487696a 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1567,9 +1567,10 @@ config HAVE_ARM_SCU
>   	help
>   	  This option enables support for the ARM system coherency unit
>
> -config ARM_ARCH_TIMER
> +config USE_ARM_ARCH_TIMER
>   	bool "Architected timer support"
>   	depends on CPU_V7
> +	select ARM_ARCH_TIMER
>   	help
>   	  This option enables support for the ARM architected timer
>
How about HAVE_ARM_ARCH_TIMER in-line with HAVE_ARM_TWD. No strong
opinion though.

Regards
Santosh

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 10/11] arm64: move from arm_generic to arm_arch_timer
  2013-01-09 16:07 ` [PATCHv2 10/11] arm64: move from arm_generic to arm_arch_timer Mark Rutland
@ 2013-01-11 13:50   ` Santosh Shilimkar
  0 siblings, 0 replies; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> The arch_timer driver supports a superset of the functionality of the
> arm_generic driver, and is not tied to a particular arch.
>
> This patch moves arm64 to use the arch_timer driver, gaining additional
> functionality in doing so, and removes the (now unused) arm_generic
> driver. Timer-related hooks specific to arm64 are moved into
> arch/arm64/kernel/time.c.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
Nice.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 11/11] Documentation: Add ARMv8 to arch_timer devicetree
  2013-01-09 16:07 ` [PATCHv2 11/11] Documentation: Add ARMv8 to arch_timer devicetree Mark Rutland
@ 2013-01-11 13:52   ` Santosh Shilimkar
  0 siblings, 0 replies; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 13:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
At least a line of change log will do :-)

> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
Acked-by: Santosh Shilimkar<santosh.shilimkar@ti.com>

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 02/11] arm: arch_timer: remove redundant available check
  2013-01-11 13:11   ` Santosh Shilimkar
@ 2013-01-11 14:07     ` Mark Rutland
  0 siblings, 0 replies; 38+ messages in thread
From: Mark Rutland @ 2013-01-11 14:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 01:11:41PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > This check is a holdover from the pre-devicetree days. As the timer
> > is not probed except by platforms which register it via devicetree,
> > it's not strictly necessary.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> Multi-platform build without DT could still benefit from the
> check but I guess the most of the A15 based platform are DT
> only, so its should be good to get rid of the check.

Indeed. I'm under the impression that all platforms which might have an
architected timer are devicetree only.

> 
> Regards,
> Santosh
> 

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 05/11] arm: arch_timer: split cntfrq accessor
  2013-01-11 13:27   ` Santosh Shilimkar
@ 2013-01-11 14:16     ` Mark Rutland
  0 siblings, 0 replies; 38+ messages in thread
From: Mark Rutland @ 2013-01-11 14:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 01:27:44PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > The CNTFRQ register is not duplicated for physical and virtual timers,
> > and accessing it as if it were is confusing.
> >
> > Instead, use a separate accessor which doesn't take the access type
> > as a parameter.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >   arch/arm/kernel/arch_timer.c |   17 +++++++++--------
> >   1 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> > index 0d2681c..fc87d3d 100644
> > --- a/arch/arm/kernel/arch_timer.c
> > +++ b/arch/arm/kernel/arch_timer.c
> > @@ -51,8 +51,7 @@ static bool arch_timer_use_virtual = true;
> >   #define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
> >
> >   #define ARCH_TIMER_REG_CTRL		0
> > -#define ARCH_TIMER_REG_FREQ		1
> > -#define ARCH_TIMER_REG_TVAL		2
> > +#define ARCH_TIMER_REG_TVAL		1
> >
> >   #define ARCH_TIMER_PHYS_ACCESS		0
> >   #define ARCH_TIMER_VIRT_ACCESS		1
> > @@ -101,9 +100,6 @@ static inline u32 arch_timer_reg_read(const int access, const int reg)
> >   		case ARCH_TIMER_REG_TVAL:
> >   			asm volatile("mrc p15, 0, %0, c14, c2, 0" : "=r" (val));
> >   			break;
> > -		case ARCH_TIMER_REG_FREQ:
> > -			asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (val));
> > -			break;
> >   		}
> >   	}
> >
> > @@ -121,6 +117,13 @@ static inline u32 arch_timer_reg_read(const int access, const int reg)
> >   	return val;
> >   }
> >
> > +static inline u32 arch_timer_get_cntfrq(void)
> > +{
> > +	u32 val;
> > +	asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r" (val));
> > +	return val;
> > +}
> > +
> >   static inline u64 arch_counter_get_cntpct(void)
> >   {
> >   	u64 cval;
> > @@ -253,9 +256,7 @@ static int arch_timer_available(void)
> >   	u32 freq;
> >
> >   	if (arch_timer_rate == 0) {
> > -		freq = arch_timer_reg_read(ARCH_TIMER_PHYS_ACCESS,
> > -					   ARCH_TIMER_REG_FREQ);
> > -
> > +		freq = arch_timer_get_cntfrq();
> Not related to this patch a new line here will be good.

Whoops. I hadn't intended to alter the line spacing here. I'll fix that up.

> >   		/* Check the timer frequency. */
> >   		if (freq == 0) {
> >   			pr_warn("Architected timer frequency not available\n");
> >
> Otherwise patch looks fine to me.
> Acked-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
> 

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 06/11] arm: arch_timer: factor out register accessors
  2013-01-11 13:32   ` Santosh Shilimkar
@ 2013-01-11 14:31     ` Mark Rutland
  0 siblings, 0 replies; 38+ messages in thread
From: Mark Rutland @ 2013-01-11 14:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 01:32:06PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > Currently the arch_timer register accessors are thrown together with
> > the main driver, preventing us from porting the driver to other
> > architectures.
> >
> > This patch moves the register accessors into a header file, as with
> > the arm64 version. Constants required by the accessors are also moved.
> >
> > Additionally isbs are added in arch_timer_get_cnt{v,p}ct to prevent
> > the cpu from speculating the reads and returning stale values.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >   arch/arm/include/asm/arch_timer.h |  101 +++++++++++++++++++++++++++++++++++++
> >   arch/arm/kernel/arch_timer.c      |   92 ---------------------------------
> >   2 files changed, 101 insertions(+), 92 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> > index d40229d..701f2b7 100644
> > --- a/arch/arm/include/asm/arch_timer.h
> > +++ b/arch/arm/include/asm/arch_timer.h
> > @@ -1,13 +1,114 @@
> >   #ifndef __ASMARM_ARCH_TIMER_H
> >   #define __ASMARM_ARCH_TIMER_H
> >
> > +#include <asm/barrier.h>
> >   #include <asm/errno.h>
> > +
> >   #include <linux/clocksource.h>
> > +#include <linux/types.h>
> >
> >   #ifdef CONFIG_ARM_ARCH_TIMER
> >   int arch_timer_of_register(void);
> >   int arch_timer_sched_clock_init(void);
> >   struct timecounter *arch_timer_get_timecounter(void);
> > +
> > +#define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
> > +#define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
> > +#define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
> > +
> > +#define ARCH_TIMER_REG_CTRL		0
> > +#define ARCH_TIMER_REG_TVAL		1
> > +
> > +#define ARCH_TIMER_PHYS_ACCESS		0
> > +#define ARCH_TIMER_VIRT_ACCESS		1
> > +
> > +/*
> > + * These register accessors are marked inline so the compiler can
> > + * nicely work out which register we want, and chuck away the rest of
> > + * the code. At least it does so with a recent GCC (4.6.3).
> > + */
> > +static inline void arch_timer_reg_write(const int access, const int reg, u32 val)
> > +{
> > +	if (access == ARCH_TIMER_PHYS_ACCESS) {
> > +		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;
> > +		}
> > +	}
> > +
> > +	if (access == ARCH_TIMER_VIRT_ACCESS) {
> > +		switch (reg) {
> > +		case ARCH_TIMER_REG_CTRL:
> > +			asm volatile("mcr p15, 0, %0, c14, c3, 1" : : "r" (val));
> > +			break;
> > +		case ARCH_TIMER_REG_TVAL:
> > +			asm volatile("mcr p15, 0, %0, c14, c3, 0" : : "r" (val));
> > +			break;
> > +		}
> > +	}
> > +
> > +	isb();
> > +}
> The isb() additions is actually a sepoerate fix. I suggest you to split
> the subject patch into 1) Movement of header data 2) isb() additions.
> 
> Feel free to add my ack on updated patches if you agree.

Thanks, will do.

> Regards,
> Santosh
> 
> 

Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
  2013-01-11 13:40   ` Santosh Shilimkar
@ 2013-01-11 14:54     ` Mark Rutland
  2013-01-11 15:07       ` Will Deacon
  2013-01-11 16:50     ` Catalin Marinas
  1 sibling, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-11 14:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > Several bits in CNTKCTL reset to 0, including PL0VTEN. For platforms
> > using the generic timer which wish to have a fast gettimeofday vDSO
> > implementation, these bits must be set to 1 by the kernel. On other
> > platforms, the bootloader might enable userspace access when we don't
> > want it.
> >
> > This patch adds arch_counter_set_user_access, which sets the PL0 access
> > permissions to that required by the platform. For arm, this currently
> minor nit.
> s/arm/ARM
> 
> > means disabling all userspace access.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >   arch/arm/include/asm/arch_timer.h |   11 +++++++++++
> >   arch/arm/kernel/arch_timer.c      |    2 ++
> >   2 files changed, 13 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> > index 701f2b7..05e3593 100644
> > --- a/arch/arm/include/asm/arch_timer.h
> > +++ b/arch/arm/include/asm/arch_timer.h
> > @@ -108,6 +108,17 @@ static inline u64 arch_counter_get_cntvct(void)
> >   	return cval;
> >   }
> >
> > +static inline void __cpuinit arch_counter_set_user_access(void)
> > +{
> > +	u32 cntkctl;
> > +
> > +	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
> > +
> > +	/* disable user access to everything */
> > +	cntkctl &= ~((3 << 8) | (7 << 0));
> > +
> > +	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
> > +}
> >
> >   #else
> >   static inline int arch_timer_of_register(void)
> > diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> > index 834d347..4f39e68 100644
> > --- a/arch/arm/kernel/arch_timer.c
> > +++ b/arch/arm/kernel/arch_timer.c
> > @@ -155,6 +155,8 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
> >   			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
> >   	}
> >
> > +	arch_counter_set_user_access();
> So how do you expect platform to enabled the user-space access in case
> they want to access it for some cases.

Unlike AArch64, at the moment we don't have the infrastructure to map this for
userspace accesses, so it isn't much of a problem.

If in future we wish to map it on 32bit platforms, the arm implementation of
arch_counter_set_user_access can be modified to allow userspace access to
specific registers, and additional code would be required to actually map it
into the user address space, etc.

> 
> Regards
> Santosh
> 

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource
  2013-01-11 13:48   ` Santosh Shilimkar
@ 2013-01-11 15:04     ` Mark Rutland
  0 siblings, 0 replies; 38+ messages in thread
From: Mark Rutland @ 2013-01-11 15:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 01:48:32PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > The core functionality of the arch_timer driver is not directly tied to
> > anything under arch/arm, and can be split out.
> >
> > This patch factors out the core of the arch_timer driver, so it can be
> > shared with other architectures. A couple of functions are added so
> > that architecture-specific code can interact with the driver without
> > needing to touch its internals.
> >
> > The ARM_ARCH_TIMER config variable is moved out to
> > drivers/clocksource/Kconfig, existing uses in arch/arm are replaced with
> > USE_ARM_ARCH_TIMER, which selects it.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >   arch/arm/Kconfig                     |    3 +-
> >   arch/arm/include/asm/arch_timer.h    |   19 +--
> >   arch/arm/kernel/arch_timer.c         |  375 ++--------------------------------
> >   arch/arm/mach-omap2/Kconfig          |    2 +-
> >   drivers/clocksource/Kconfig          |    3 +
> >   drivers/clocksource/Makefile         |    1 +
> >   drivers/clocksource/arm_arch_timer.c |  374 +++++++++++++++++++++++++++++++++
> >   include/clocksource/arm_arch_timer.h |   63 ++++++
> >   8 files changed, 465 insertions(+), 375 deletions(-)
> >   create mode 100644 drivers/clocksource/arm_arch_timer.c
> >   create mode 100644 include/clocksource/arm_arch_timer.h
> >
> It would have been easy if you have formated the patch with -C option.
> That will just leave the delta changes only and hiding the file
> movement related diff.

Sorry, I'll make sure I do that next time.

> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index f95ba14..487696a 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1567,9 +1567,10 @@ config HAVE_ARM_SCU
> >   	help
> >   	  This option enables support for the ARM system coherency unit
> >
> > -config ARM_ARCH_TIMER
> > +config USE_ARM_ARCH_TIMER
> >   	bool "Architected timer support"
> >   	depends on CPU_V7
> > +	select ARM_ARCH_TIMER
> >   	help
> >   	  This option enables support for the ARM architected timer
> >
> How about HAVE_ARM_ARCH_TIMER in-line with HAVE_ARM_TWD. No strong
> opinion though.

Sure. It'll also make it more consistent with HAVE_ARM_SCU.

> 
> Regards
> Santosh
> 

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
  2013-01-11 14:54     ` Mark Rutland
@ 2013-01-11 15:07       ` Will Deacon
  2013-01-11 17:00         ` Santosh Shilimkar
  0 siblings, 1 reply; 38+ messages in thread
From: Will Deacon @ 2013-01-11 15:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 02:54:52PM +0000, Mark Rutland wrote:
> On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
> > So how do you expect platform to enabled the user-space access in case
> > they want to access it for some cases.
> 
> Unlike AArch64, at the moment we don't have the infrastructure to map this for
> userspace accesses, so it isn't much of a problem.
> 
> If in future we wish to map it on 32bit platforms, the arm implementation of
> arch_counter_set_user_access can be modified to allow userspace access to
> specific registers, and additional code would be required to actually map it
> into the user address space, etc.

I'd also add that it's not up to a platform to decide whether to expose
this to userspace: it needs to be an architecture-wide decision. Otherwise,
userspace becomes SoC-specific, which is a complete disaster.

So, if userspace people want these available, they need to convince us to
flip the switch. In the meantime, it should default to off so that if/when
we do enable it we can do it in a sane manner for ARM (perhaps via the
vectors page).

Will

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api
  2013-01-11 13:34   ` Santosh Shilimkar
@ 2013-01-11 16:46     ` Catalin Marinas
  2013-01-11 16:56       ` Santosh Shilimkar
  0 siblings, 1 reply; 38+ messages in thread
From: Catalin Marinas @ 2013-01-11 16:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 01:34:23PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > Currently, the arch_timer driver is tied to the arm port, as it relies
> > on code in arch/arm/smp.c to setup and teardown timers as cores are
> > hotplugged on and off. The timer is registered through an arm-specific
> > registration mechanism, preventing sharing the driver with the arm64
> > port.
> >
> > This patch moves the driver to using a cpu notifier instead, making it
> > easier to port.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> This is really a nit idea. I think we should do the same
> for ARM gic code.

I plan to do the same once Rob's GIC patches get merged. In my
soc-armv8-model branch I have a copy of gic.c into drivers/irqchip and
the CPU interface is done automatically via a notifier. The only trick
is to set the priority of the GIC notifier higher than the timer one.

-- 
Catalin

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
  2013-01-11 13:40   ` Santosh Shilimkar
  2013-01-11 14:54     ` Mark Rutland
@ 2013-01-11 16:50     ` Catalin Marinas
  2013-01-11 16:57       ` Santosh Shilimkar
  2013-01-11 17:54       ` Mark Rutland
  1 sibling, 2 replies; 38+ messages in thread
From: Catalin Marinas @ 2013-01-11 16:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > Several bits in CNTKCTL reset to 0, including PL0VTEN. For platforms
> > using the generic timer which wish to have a fast gettimeofday vDSO
> > implementation, these bits must be set to 1 by the kernel. On other
> > platforms, the bootloader might enable userspace access when we don't
> > want it.
> >
> > This patch adds arch_counter_set_user_access, which sets the PL0 access
> > permissions to that required by the platform. For arm, this currently
> minor nit.
> s/arm/ARM

I think Mark meant arch/arm. Or we could call it AArch32 where we don't
use this feature.

-- 
Catalin

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api
  2013-01-11 16:46     ` Catalin Marinas
@ 2013-01-11 16:56       ` Santosh Shilimkar
  0 siblings, 0 replies; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 16:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 11 January 2013 10:16 PM, Catalin Marinas wrote:
> On Fri, Jan 11, 2013 at 01:34:23PM +0000, Santosh Shilimkar wrote:
>> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
>>> Currently, the arch_timer driver is tied to the arm port, as it relies
>>> on code in arch/arm/smp.c to setup and teardown timers as cores are
>>> hotplugged on and off. The timer is registered through an arm-specific
>>> registration mechanism, preventing sharing the driver with the arm64
>>> port.
>>>
>>> This patch moves the driver to using a cpu notifier instead, making it
>>> easier to port.
>>>
>>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>>> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>> This is really a nit idea. I think we should do the same
>> for ARM gic code.
>
> I plan to do the same once Rob's GIC patches get merged. In my
> soc-armv8-model branch I have a copy of gic.c into drivers/irqchip and
> the CPU interface is done automatically via a notifier. The only trick
> is to set the priority of the GIC notifier higher than the timer one.
>
Great. Thanks for the information.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
  2013-01-11 16:50     ` Catalin Marinas
@ 2013-01-11 16:57       ` Santosh Shilimkar
  2013-01-11 17:54       ` Mark Rutland
  1 sibling, 0 replies; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 16:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 11 January 2013 10:20 PM, Catalin Marinas wrote:
> On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
>> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
>>> Several bits in CNTKCTL reset to 0, including PL0VTEN. For platforms
>>> using the generic timer which wish to have a fast gettimeofday vDSO
>>> implementation, these bits must be set to 1 by the kernel. On other
>>> platforms, the bootloader might enable userspace access when we don't
>>> want it.
>>>
>>> This patch adds arch_counter_set_user_access, which sets the PL0 access
>>> permissions to that required by the platform. For arm, this currently
>> minor nit.
>> s/arm/ARM
>
> I think Mark meant arch/arm. Or we could call it AArch32 where we don't
> use this feature.
>
OK. AArch32 or even arch/arm is just fine then.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
  2013-01-11 15:07       ` Will Deacon
@ 2013-01-11 17:00         ` Santosh Shilimkar
  0 siblings, 0 replies; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-11 17:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 11 January 2013 08:37 PM, Will Deacon wrote:
> On Fri, Jan 11, 2013 at 02:54:52PM +0000, Mark Rutland wrote:
>> On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
>>> So how do you expect platform to enabled the user-space access in case
>>> they want to access it for some cases.
>>
>> Unlike AArch64, at the moment we don't have the infrastructure to map this for
>> userspace accesses, so it isn't much of a problem.
>>
>> If in future we wish to map it on 32bit platforms, the arm implementation of
>> arch_counter_set_user_access can be modified to allow userspace access to
>> specific registers, and additional code would be required to actually map it
>> into the user address space, etc.
>
> I'd also add that it's not up to a platform to decide whether to expose
> this to userspace: it needs to be an architecture-wide decision. Otherwise,
> userspace becomes SoC-specific, which is a complete disaster.
>
> So, if userspace people want these available, they need to convince us to
> flip the switch. In the meantime, it should default to off so that if/when
> we do enable it we can do it in a sane manner for ARM (perhaps via the
> vectors page).
>
Thanks Will for rationale behind the change. Good to capture the 
reasoning in changelog for future reference.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access
  2013-01-11 16:50     ` Catalin Marinas
  2013-01-11 16:57       ` Santosh Shilimkar
@ 2013-01-11 17:54       ` Mark Rutland
  1 sibling, 0 replies; 38+ messages in thread
From: Mark Rutland @ 2013-01-11 17:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 04:50:53PM +0000, Catalin Marinas wrote:
> On Fri, Jan 11, 2013 at 01:40:22PM +0000, Santosh Shilimkar wrote:
> > On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > > Several bits in CNTKCTL reset to 0, including PL0VTEN. For platforms
> > > using the generic timer which wish to have a fast gettimeofday vDSO
> > > implementation, these bits must be set to 1 by the kernel. On other
> > > platforms, the bootloader might enable userspace access when we don't
> > > want it.
> > >
> > > This patch adds arch_counter_set_user_access, which sets the PL0 access
> > > permissions to that required by the platform. For arm, this currently
> > minor nit.
> > s/arm/ARM
> 
> I think Mark meant arch/arm. Or we could call it AArch32 where we don't
> use this feature.

Indeed, I meant arch/arm. I'll try to be more consistent with that in future.

Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 04/11] arm: arch_timer: standardise counter reading
  2013-01-11 13:23   ` Santosh Shilimkar
@ 2013-01-15 10:25     ` Mark Rutland
  2013-01-15 10:38       ` Santosh Shilimkar
  0 siblings, 1 reply; 38+ messages in thread
From: Mark Rutland @ 2013-01-15 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 11, 2013 at 01:23:33PM +0000, Santosh Shilimkar wrote:
> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
> > We're currently inconsistent with respect to our accesses to the
> > physical and virtual counters, mixing and matching the two.
> >
> > This patch introduces and uses a function for accessing the correct
> > counter based on whether we're using physical or virtual interrupts.
> > All current accesses to the counter accessors are redirected through
> > it.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >   arch/arm/kernel/arch_timer.c |   48 ++++++++++-------------------------------
> >   1 files changed, 12 insertions(+), 36 deletions(-)
> >
> > diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> > index 498c29f..0d2681c 100644
> > --- a/arch/arm/kernel/arch_timer.c
> > +++ b/arch/arm/kernel/arch_timer.c
> > @@ -272,51 +272,32 @@ static int arch_timer_available(void)
> >   	return 0;
> >   }
> >
> > -static u32 notrace arch_counter_get_cntpct32(void)
> > +u64 arch_timer_read_counter(void)
> >   {
> > -	cycle_t cnt = arch_counter_get_cntpct();
> > -
> > -	/*
> > -	 * The sched_clock infrastructure only knows about counters
> > -	 * with at most 32bits. Forget about the upper 24 bits for the
> > -	 * time being...
> > -	 */
> > -	return (u32)cnt;
> > +	if (arch_timer_use_virtual)
> > +		return arch_counter_get_cntvct();
> > +	else
> > +		return arch_counter_get_cntpct();
> >   }
> >
> 
> [...]
> 
> > @@ -489,18 +470,13 @@ int __init arch_timer_of_register(void)
> >
> >   int __init arch_timer_sched_clock_init(void)
> >   {
> > -	u32 (*cnt32)(void);
> >   	int err;
> >
> >   	err = arch_timer_available();
> >   	if (err)
> >   		return err;
> >
> > -	if (arch_timer_use_virtual)
> > -		cnt32 = arch_counter_get_cntvct32;
> > -	else
> > -		cnt32 = arch_counter_get_cntpct32;
> > -
> > -	setup_sched_clock(cnt32, 32, arch_timer_rate);
> > +	setup_sched_clock(arch_timer_read_counter32,
> > +			  32, arch_timer_rate);
> >   	return 0;
> >   }
> >
> I think the original idea had merit since the check was needed
> in init code instead of proposed one which has if check for
> every counter read function. No ?
> 

The original idea was good in that it avoided the check on each read path, but
in several places the logic got duplicated (e.g. for choosing which
width-altering wrapper in the above block). I'd like ensure this logic is
consolidated.

I'll change arch_timer_read_counter to a function pointer, and set this in
arch_timer_of_register before registering anything. Everything would still be
indirected through it, but it won't have to do a check on every read.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCHv2 04/11] arm: arch_timer: standardise counter reading
  2013-01-15 10:25     ` Mark Rutland
@ 2013-01-15 10:38       ` Santosh Shilimkar
  0 siblings, 0 replies; 38+ messages in thread
From: Santosh Shilimkar @ 2013-01-15 10:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 15 January 2013 03:55 PM, Mark Rutland wrote:
> On Fri, Jan 11, 2013 at 01:23:33PM +0000, Santosh Shilimkar wrote:
>> On Wednesday 09 January 2013 09:37 PM, Mark Rutland wrote:
>>> We're currently inconsistent with respect to our accesses to the
>>> physical and virtual counters, mixing and matching the two.
>>>
>>> This patch introduces and uses a function for accessing the correct
>>> counter based on whether we're using physical or virtual interrupts.
>>> All current accesses to the counter accessors are redirected through
>>> it.
>>>
>>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>>> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>>    arch/arm/kernel/arch_timer.c |   48 ++++++++++-------------------------------
>>>    1 files changed, 12 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
>>> index 498c29f..0d2681c 100644
>>> --- a/arch/arm/kernel/arch_timer.c
>>> +++ b/arch/arm/kernel/arch_timer.c
>>> @@ -272,51 +272,32 @@ static int arch_timer_available(void)
>>>    	return 0;
>>>    }
>>>
>>> -static u32 notrace arch_counter_get_cntpct32(void)
>>> +u64 arch_timer_read_counter(void)
>>>    {
>>> -	cycle_t cnt = arch_counter_get_cntpct();
>>> -
>>> -	/*
>>> -	 * The sched_clock infrastructure only knows about counters
>>> -	 * with at most 32bits. Forget about the upper 24 bits for the
>>> -	 * time being...
>>> -	 */
>>> -	return (u32)cnt;
>>> +	if (arch_timer_use_virtual)
>>> +		return arch_counter_get_cntvct();
>>> +	else
>>> +		return arch_counter_get_cntpct();
>>>    }
>>>
>>
>> [...]
>>
>>> @@ -489,18 +470,13 @@ int __init arch_timer_of_register(void)
>>>
>>>    int __init arch_timer_sched_clock_init(void)
>>>    {
>>> -	u32 (*cnt32)(void);
>>>    	int err;
>>>
>>>    	err = arch_timer_available();
>>>    	if (err)
>>>    		return err;
>>>
>>> -	if (arch_timer_use_virtual)
>>> -		cnt32 = arch_counter_get_cntvct32;
>>> -	else
>>> -		cnt32 = arch_counter_get_cntpct32;
>>> -
>>> -	setup_sched_clock(cnt32, 32, arch_timer_rate);
>>> +	setup_sched_clock(arch_timer_read_counter32,
>>> +			  32, arch_timer_rate);
>>>    	return 0;
>>>    }
>>>
>> I think the original idea had merit since the check was needed
>> in init code instead of proposed one which has if check for
>> every counter read function. No ?
>>
>
> The original idea was good in that it avoided the check on each read path, but
> in several places the logic got duplicated (e.g. for choosing which
> width-altering wrapper in the above block). I'd like ensure this logic is
> consolidated.
>
> I'll change arch_timer_read_counter to a function pointer, and set this in
> arch_timer_of_register before registering anything. Everything would still be
> indirected through it, but it won't have to do a check on every read.
>
Sounds good

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2013-01-15 10:38 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-09 16:07 [PATCHv3 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
2013-01-09 16:07 ` [PATCHv2 01/11] arm: arch_timer: balance device_node refcounting Mark Rutland
2013-01-11 12:59   ` Santosh Shilimkar
2013-01-09 16:07 ` [PATCHv2 02/11] arm: arch_timer: remove redundant available check Mark Rutland
2013-01-11 13:11   ` Santosh Shilimkar
2013-01-11 14:07     ` Mark Rutland
2013-01-09 16:07 ` [PATCHv2 03/11] arm: arch_timer: use u64/u32 for register data Mark Rutland
2013-01-11 13:19   ` Santosh Shilimkar
2013-01-09 16:07 ` [PATCHv2 04/11] arm: arch_timer: standardise counter reading Mark Rutland
2013-01-11 13:23   ` Santosh Shilimkar
2013-01-15 10:25     ` Mark Rutland
2013-01-15 10:38       ` Santosh Shilimkar
2013-01-09 16:07 ` [PATCHv2 05/11] arm: arch_timer: split cntfrq accessor Mark Rutland
2013-01-11 13:27   ` Santosh Shilimkar
2013-01-11 14:16     ` Mark Rutland
2013-01-09 16:07 ` [PATCHv2 06/11] arm: arch_timer: factor out register accessors Mark Rutland
2013-01-11 13:32   ` Santosh Shilimkar
2013-01-11 14:31     ` Mark Rutland
2013-01-09 16:07 ` [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api Mark Rutland
2013-01-11 13:34   ` Santosh Shilimkar
2013-01-11 16:46     ` Catalin Marinas
2013-01-11 16:56       ` Santosh Shilimkar
2013-01-09 16:07 ` [PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access Mark Rutland
2013-01-11 13:40   ` Santosh Shilimkar
2013-01-11 14:54     ` Mark Rutland
2013-01-11 15:07       ` Will Deacon
2013-01-11 17:00         ` Santosh Shilimkar
2013-01-11 16:50     ` Catalin Marinas
2013-01-11 16:57       ` Santosh Shilimkar
2013-01-11 17:54       ` Mark Rutland
2013-01-09 16:07 ` [PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource Mark Rutland
2013-01-11 13:48   ` Santosh Shilimkar
2013-01-11 15:04     ` Mark Rutland
2013-01-09 16:07 ` [PATCHv2 10/11] arm64: move from arm_generic to arm_arch_timer Mark Rutland
2013-01-11 13:50   ` Santosh Shilimkar
2013-01-09 16:07 ` [PATCHv2 11/11] Documentation: Add ARMv8 to arch_timer devicetree Mark Rutland
2013-01-11 13:52   ` Santosh Shilimkar
  -- strict thread matches above, loose matches on Subject: below --
2012-12-19 15:10 [PATCHv2 00/11] Unify arm_generic and arch_timer drivers Mark Rutland
2012-12-19 15:11 ` [PATCHv2 07/11] arm: arch_timer: divorce from local_timer api Mark Rutland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).