[PATCHv3 00/11] Unify arm_generic and arch_timer drivers

[PATCHv3 00/11] Unify arm_generic and arch_timer drivers

[Mark Rutland]

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

[PATCHv2 01/11] arm: arch_timer: balance device_node refcounting

[Mark Rutland]

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

[PATCHv2 01/11] arm: arch_timer: balance device_node refcounting

[Santosh Shilimkar]

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>

[PATCHv2 02/11] arm: arch_timer: remove redundant available check

[Mark Rutland]

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

[PATCHv2 02/11] arm: arch_timer: remove redundant available check

[Santosh Shilimkar]

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

[PATCHv2 02/11] arm: arch_timer: remove redundant available check

[Mark Rutland]

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.

[PATCHv2 03/11] arm: arch_timer: use u64/u32 for register data

[Mark Rutland]

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

[PATCHv2 03/11] arm: arch_timer: use u64/u32 for register data

[Santosh Shilimkar]

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>

[PATCHv2 04/11] arm: arch_timer: standardise counter reading

[Mark Rutland]

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

[PATCHv2 04/11] arm: arch_timer: standardise counter reading

[Santosh Shilimkar]

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 ?

[PATCHv2 04/11] arm: arch_timer: standardise counter reading

[Mark Rutland]

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.

[PATCHv2 04/11] arm: arch_timer: standardise counter reading

[Santosh Shilimkar]

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

[PATCHv2 05/11] arm: arch_timer: split cntfrq accessor

[Mark Rutland]

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

[PATCHv2 05/11] arm: arch_timer: split cntfrq accessor

[Santosh Shilimkar]

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>

[PATCHv2 05/11] arm: arch_timer: split cntfrq accessor

[Mark Rutland]

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.

[PATCHv2 06/11] arm: arch_timer: factor out register accessors

[Mark Rutland]

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

[PATCHv2 06/11] arm: arch_timer: factor out register accessors

[Santosh Shilimkar]

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

[PATCHv2 06/11] arm: arch_timer: factor out register accessors

[Mark Rutland]

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.

[PATCHv2 07/11] arm: arch_timer: divorce from local_timer api

[Mark Rutland]

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

[PATCHv2 07/11] arm: arch_timer: divorce from local_timer api

[Santosh Shilimkar]

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>

[PATCHv2 07/11] arm: arch_timer: divorce from local_timer api

[Catalin Marinas]

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

[PATCHv2 07/11] arm: arch_timer: divorce from local_timer api

[Santosh Shilimkar]

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

[PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access

[Mark Rutland]

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

[PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access

[Santosh Shilimkar]

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

[PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access

[Mark Rutland]

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.

[PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access

[Will Deacon]

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

[PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access

[Santosh Shilimkar]

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

[PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access

[Catalin Marinas]

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

[PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access

[Santosh Shilimkar]

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

[PATCHv2 08/11] arm: arch_timer: add arch_counter_set_user_access

[Mark Rutland]

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.

[PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource

[Mark Rutland]

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

[PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource

[Santosh Shilimkar]

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

[PATCHv2 09/11] arm: arch_timer: move core to drivers/clocksource

[Mark Rutland]

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.

[PATCHv2 10/11] arm64: move from arm_generic to arm_arch_timer

[Mark Rutland]

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

[PATCHv2 10/11] arm64: move from arm_generic to arm_arch_timer

[Santosh Shilimkar]

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>

[PATCHv2 11/11] Documentation: Add ARMv8 to arch_timer devicetree

[Mark Rutland]

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

[PATCHv2 11/11] Documentation: Add ARMv8 to arch_timer devicetree

[Santosh Shilimkar]

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>

[PATCHv2 00/11] Unify arm_generic and arch_timer drivers

[Mark Rutland]

This is an updated version the series I posted a few weeks ago [1].

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 attempts to remedy this by decoupling the timer broadcast
mechanism from drivers.

Due to the unfortunate timing of the merge window and the dependence on
Will's vdso changes in mainline since v3.7, I've based this on today's
upstream HEAD (752451f0). I'll repost the series in January rebased onto
an -rc.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/135651.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/137929.html

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

[PATCHv2 06/11] arm: arch_timer: factor out register accessors

[Mark Rutland]

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