[PATCH 0/4] ARM/ARM64 architected timer updates

[PATCH 0/4] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Hi,

This patch series adds support to configure the rate and enable the event
stream for architected timer. Since the timer control register is reset to
zero on warm boot, CPU PM notifier is added to re-initialse it.

Regards,
Sudeep

Sudeep KarkadaNagesha (3):
  ARM: arch_timer: add macros for bits in control register
  ARM/ARM64: arch_timer: remove __cpuinit attribute for
    arch_counter_set_user_access
  drivers: clocksource: add CPU PM notifier for ARM architected timer

Will Deacon (1):
  drivers: clocksource: configure event stream for ARM arch timer

 arch/arm/include/asm/arch_timer.h    | 10 ++++++--
 arch/arm64/include/asm/arch_timer.h  | 12 +++++++---
 drivers/clocksource/arm_arch_timer.c | 46 +++++++++++++++++++++++++++++++++++-
 include/clocksource/arm_arch_timer.h | 10 ++++++++
 4 files changed, 72 insertions(+), 6 deletions(-)

-- 
1.8.1.2

[PATCH 1/4] drivers: clocksource: configure event stream for ARM arch timer

[Sudeep KarkadaNagesha]

From: Will Deacon <will.deacon@arm.com>

The ARM architected timer can generate events (used for waking up
CPUs executing the wfe instruction) at a frequency represented as a
power-of-2 divisor of the clock rate.

This patch configures the event stream, aiming for a period of 100us
between events. This can be used to implement wfe-based timeouts for
userspace locking implementations.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
 arch/arm/include/asm/arch_timer.h    | 5 +++--
 arch/arm64/include/asm/arch_timer.h  | 6 +++---
 drivers/clocksource/arm_arch_timer.c | 9 ++++++++-
 include/clocksource/arm_arch_timer.h | 2 ++
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index accefe0..75c8ec3 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -89,14 +89,15 @@ static inline u64 arch_counter_get_cntvct(void)
 	return cval;
 }
 
-static inline void __cpuinit arch_counter_set_user_access(void)
+static inline void __cpuinit arch_counter_set_user_access(int divider)
 {
 	u32 cntkctl;
 
 	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
 
 	/* disable user access to everything */
-	cntkctl &= ~((3 << 8) | (7 << 0));
+	cntkctl &= ~((3 << 8) | (0xf << 4) | (3 << 0));
+	cntkctl |= (divider << 4) | (1 << 2);
 
 	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
 }
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index d56ed11..5fcaa6f 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -97,16 +97,16 @@ static inline u32 arch_timer_get_cntfrq(void)
 	return val;
 }
 
-static inline void __cpuinit arch_counter_set_user_access(void)
+static inline void __cpuinit arch_counter_set_user_access(int divider)
 {
 	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));
+	cntkctl &= ~((3 << 8) | (0xf << 4) | (1 << 0));
 
 	/* Enable user access to the virtual counter and frequency. */
-	cntkctl |= (1 << 1);
+	cntkctl |= (divider << 4) | (1 << 2) | (1 << 1);
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 053d846..11aaf06 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -125,6 +125,8 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
 
 static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
 {
+	int evt_stream_div, pos;
+
 	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
 	clk->name = "arch_sys_timer";
 	clk->rating = 450;
@@ -153,7 +155,12 @@ 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();
+	/* Find the closest power of two to the divisor */
+	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
+	pos = fls(evt_stream_div);
+	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
+		pos--;
+	arch_counter_set_user_access(min(pos, 15));
 
 	return 0;
 }
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index c463ce9..24dc140 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -29,6 +29,8 @@
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
 
+#define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
+
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
-- 
1.8.1.2

[PATCH 2/4] ARM: arch_timer: add macros for bits in control register

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Add macros to describe the bitfields in the ARM architected timer
control register to make code easy to understand.

Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/arch_timer.h    |  9 +++++++--
 arch/arm64/include/asm/arch_timer.h  | 10 ++++++++--
 include/clocksource/arm_arch_timer.h |  8 ++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 75c8ec3..6725017 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -96,8 +96,13 @@ static inline void __cpuinit arch_counter_set_user_access(int divider)
 	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
 
 	/* disable user access to everything */
-	cntkctl &= ~((3 << 8) | (0xf << 4) | (3 << 0));
-	cntkctl |= (divider << 4) | (1 << 2);
+	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
+			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_EVT_TRIGGER_MASK
+			| ARCH_TIMER_USR_VCT_ACCESS_EN
+			| ARCH_TIMER_USR_PCT_ACCESS_EN);
+	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+			| ARCH_TIMER_VIRT_EVT_EN;
 
 	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
 }
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 5fcaa6f..2269944 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -103,10 +103,16 @@ static inline void __cpuinit arch_counter_set_user_access(int divider)
 
 	/* Disable user access to the timers and the physical counter. */
 	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
-	cntkctl &= ~((3 << 8) | (0xf << 4) | (1 << 0));
+	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
+			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_EVT_TRIGGER_MASK
+			| ARCH_TIMER_USR_PCT_ACCESS_EN)
 
 	/* Enable user access to the virtual counter and frequency. */
-	cntkctl |= (divider << 4) | (1 << 2) | (1 << 1);
+	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+			| ARCH_TIMER_VIRT_EVT_EN
+			| ARCH_TIMER_USR_VCT_ACCESS_EN;
+
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 24dc140..c4d0fc4 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -29,6 +29,14 @@
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
 
+#define ARCH_TIMER_USR_PCT_ACCESS_EN	(1 << 0) /* physical counter */
+#define ARCH_TIMER_USR_VCT_ACCESS_EN	(1 << 1) /* virtual counter */
+#define ARCH_TIMER_VIRT_EVT_EN		(1 << 2)
+#define ARCH_TIMER_EVT_TRIGGER_SHIFT	(4)
+#define ARCH_TIMER_EVT_TRIGGER_MASK	(0xF << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+#define ARCH_TIMER_USR_VT_ACCESS_EN	(1 << 8) /* virtual timer registers */
+#define ARCH_TIMER_USR_PT_ACCESS_EN	(1 << 9) /* physical timer registers */
+
 #define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
 
 #ifdef CONFIG_ARM_ARCH_TIMER
-- 
1.8.1.2

[PATCH 3/4] ARM/ARM64: arch_timer: remove __cpuinit attribute for arch_counter_set_user_access

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

We must not declare arch_counter_set_user_access as __cpuinit as we
need to use it in warm reset paths like CPU PM notifiers.

Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/arch_timer.h   | 2 +-
 arch/arm64/include/asm/arch_timer.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 6725017..26c65ae 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -89,7 +89,7 @@ static inline u64 arch_counter_get_cntvct(void)
 	return cval;
 }
 
-static inline void __cpuinit arch_counter_set_user_access(int divider)
+static inline void arch_counter_set_user_access(int divider)
 {
 	u32 cntkctl;
 
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 2269944..3494102 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -97,7 +97,7 @@ static inline u32 arch_timer_get_cntfrq(void)
 	return val;
 }
 
-static inline void __cpuinit arch_counter_set_user_access(int divider)
+static inline void arch_counter_set_user_access(int divider)
 {
 	u32 cntkctl;
 
-- 
1.8.1.2

Re: [PATCH 3/4] ARM/ARM64: arch_timer: remove __cpuinit attribute for arch_counter_set_user_access

[Sudeep KarkadaNagesha]

On 18/06/13 18:07, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> We must not declare arch_counter_set_user_access as __cpuinit as we
> need to use it in warm reset paths like CPU PM notifiers.
> 
This patch should be dropped as __cpuinit is removed completely by
http://www.spinics.net/lists/arm-kernel/msg252410.html
Just happen to see it now.

Regards,
Sudeep

[PATCH 4/4] drivers: clocksource: add CPU PM notifier for ARM architected timer

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Few control settings done in architected timer as part of initialisation
are lost when CPU enters deeper power states. They need to be re-initialised
when the CPU is (warm)reset again.

This patch moves all such initialisation into separate function that can
be used both in cold and warm CPU reset paths. It also adds CPU PM
notifiers to do the timer initialisation on warm resets.

Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 51 +++++++++++++++++++++++++++++++-----
 1 file changed, 44 insertions(+), 7 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 11aaf06..1c691b1 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -13,6 +13,7 @@
 #include <linux/device.h>
 #include <linux/smp.h>
 #include <linux/cpu.h>
+#include <linux/cpu_pm.h>
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
@@ -123,10 +124,20 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
 	return 0;
 }
 
-static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
+static void arch_timer_initialise(void)
 {
 	int evt_stream_div, pos;
 
+	/* Find the closest power of two to the divisor */
+	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
+	pos = fls(evt_stream_div);
+	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
+		pos--;
+	arch_counter_set_user_access(min(pos, 15));
+}
+
+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;
@@ -155,12 +166,7 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
 			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
 	}
 
-	/* Find the closest power of two to the divisor */
-	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
-	pos = fls(evt_stream_div);
-	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
-		pos--;
-	arch_counter_set_user_access(min(pos, 15));
+	arch_timer_initialise();
 
 	return 0;
 }
@@ -267,6 +273,31 @@ static struct notifier_block arch_timer_cpu_nb __cpuinitdata = {
 	.notifier_call = arch_timer_cpu_notify,
 };
 
+#ifdef CONFIG_CPU_PM
+static int arch_timer_cpu_pm_notify(struct notifier_block *self,
+				    unsigned long action, void *hcpu)
+{
+	if (action == CPU_PM_EXIT)
+		arch_timer_initialise();
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block arch_timer_cpu_pm_notifier = {
+	.notifier_call = arch_timer_cpu_pm_notify,
+};
+
+static int __init arch_timer_cpu_pm_init(void)
+{
+	return cpu_pm_register_notifier(&arch_timer_cpu_pm_notifier);
+}
+#else
+static int __init arch_timer_cpu_pm_init(void)
+{
+	return 0;
+}
+#endif
+
 static int __init arch_timer_register(void)
 {
 	int err;
@@ -316,11 +347,17 @@ static int __init arch_timer_register(void)
 	if (err)
 		goto out_free_irq;
 
+	err = arch_timer_cpu_pm_init();
+	if (err)
+		goto out_unreg_notify;
+
 	/* Immediately configure the timer on the boot CPU */
 	arch_timer_setup(this_cpu_ptr(arch_timer_evt));
 
 	return 0;
 
+out_unreg_notify:
+	unregister_cpu_notifier(&arch_timer_cpu_nb);
 out_free_irq:
 	if (arch_timer_use_virtual)
 		free_percpu_irq(arch_timer_ppi[VIRT_PPI], arch_timer_evt);
-- 
1.8.1.2

Re: [PATCH 4/4] drivers: clocksource: add CPU PM notifier for ARM architected timer

[Jon Medhurst (Tixy)]

On Tue, 2013-06-18 at 18:07 +0100, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> Few control settings done in architected timer as part of initialisation
> are lost when CPU enters deeper power states. They need to be re-initialised
> when the CPU is (warm)reset again.
> 
> This patch moves all such initialisation into separate function that can
> be used both in cold and warm CPU reset paths. It also adds CPU PM
> notifiers to do the timer initialisation on warm resets.
> 
> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reviewed-by: Will Deacon <will.deacon@arm.com>
> ---
>  drivers/clocksource/arm_arch_timer.c | 51 +++++++++++++++++++++++++++++++-----
>  1 file changed, 44 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 11aaf06..1c691b1 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -13,6 +13,7 @@
>  #include <linux/device.h>
>  #include <linux/smp.h>
>  #include <linux/cpu.h>
> +#include <linux/cpu_pm.h>
>  #include <linux/clockchips.h>
>  #include <linux/interrupt.h>
>  #include <linux/of_irq.h>
> @@ -123,10 +124,20 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
>  	return 0;
>  }
>  
> -static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
> +static void arch_timer_initialise(void)
>  {
>  	int evt_stream_div, pos;
>  
> +	/* Find the closest power of two to the divisor */
> +	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
> +	pos = fls(evt_stream_div);
> +	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> +		pos--;
> +	arch_counter_set_user_access(min(pos, 15));

Would it not be good to calculate the value once in arch_timer_setup
rather than repeatedly in this function? The calculations aren't that
expensive, but when I gave these patches a spin on TC2 I noticed that
this function gets called >500 times a second, so it seems a bit
wasteful.

> +}
> +
> +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;
> @@ -155,12 +166,7 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
>  			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
>  	}
>  
> -	/* Find the closest power of two to the divisor */
> -	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
> -	pos = fls(evt_stream_div);
> -	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> -		pos--;
> -	arch_counter_set_user_access(min(pos, 15));
> +	arch_timer_initialise();
>  
>  	return 0;
>  }
> @@ -267,6 +273,31 @@ static struct notifier_block arch_timer_cpu_nb __cpuinitdata = {
>  	.notifier_call = arch_timer_cpu_notify,
>  };
>  
> +#ifdef CONFIG_CPU_PM
> +static int arch_timer_cpu_pm_notify(struct notifier_block *self,
> +				    unsigned long action, void *hcpu)
> +{
> +	if (action == CPU_PM_EXIT)
> +		arch_timer_initialise();
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block arch_timer_cpu_pm_notifier = {
> +	.notifier_call = arch_timer_cpu_pm_notify,
> +};
> +
> +static int __init arch_timer_cpu_pm_init(void)
> +{
> +	return cpu_pm_register_notifier(&arch_timer_cpu_pm_notifier);
> +}
> +#else
> +static int __init arch_timer_cpu_pm_init(void)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static int __init arch_timer_register(void)
>  {
>  	int err;
> @@ -316,11 +347,17 @@ static int __init arch_timer_register(void)
>  	if (err)
>  		goto out_free_irq;
>  
> +	err = arch_timer_cpu_pm_init();
> +	if (err)
> +		goto out_unreg_notify;
> +
>  	/* Immediately configure the timer on the boot CPU */
>  	arch_timer_setup(this_cpu_ptr(arch_timer_evt));
>  
>  	return 0;
>  
> +out_unreg_notify:
> +	unregister_cpu_notifier(&arch_timer_cpu_nb);
>  out_free_irq:
>  	if (arch_timer_use_virtual)
>  		free_percpu_irq(arch_timer_ppi[VIRT_PPI], arch_timer_evt);

-- 
Tixy

Re: [PATCH 4/4] drivers: clocksource: add CPU PM notifier for ARM architected timer

[Sudeep KarkadaNagesha]

On 02/07/13 17:09, Jon Medhurst (Tixy) wrote:
> On Tue, 2013-06-18 at 18:07 +0100, Sudeep KarkadaNagesha wrote:
>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>
>> Few control settings done in architected timer as part of initialisation
>> are lost when CPU enters deeper power states. They need to be re-initialised
>> when the CPU is (warm)reset again.
>>
>> This patch moves all such initialisation into separate function that can
>> be used both in cold and warm CPU reset paths. It also adds CPU PM
>> notifiers to do the timer initialisation on warm resets.
>>
>> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Reviewed-by: Will Deacon <will.deacon@arm.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 51 +++++++++++++++++++++++++++++++-----
>>  1 file changed, 44 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>> index 11aaf06..1c691b1 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -13,6 +13,7 @@
>>  #include <linux/device.h>
>>  #include <linux/smp.h>
>>  #include <linux/cpu.h>
>> +#include <linux/cpu_pm.h>
>>  #include <linux/clockchips.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/of_irq.h>
>> @@ -123,10 +124,20 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
>>  	return 0;
>>  }
>>  
>> -static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
>> +static void arch_timer_initialise(void)
>>  {
>>  	int evt_stream_div, pos;
>>  
>> +	/* Find the closest power of two to the divisor */
>> +	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>> +	pos = fls(evt_stream_div);
>> +	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
>> +		pos--;
>> +	arch_counter_set_user_access(min(pos, 15));
> 
> Would it not be good to calculate the value once in arch_timer_setup
> rather than repeatedly in this function? The calculations aren't that
> expensive, but when I gave these patches a spin on TC2 I noticed that
> this function gets called >500 times a second, so it seems a bit
> wasteful.
> 
Makes sense, will save the divider and re-use it.

Regards,
Sudeep

[PATCH v2 0/3] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

This patch series adds support to configure the rate and enable the event
stream for architected timer. Since the timer control register is reset to
zero on warm boot, CPU PM notifier is added to re-initialse it.

Changes v1->v2:
1. Saved event stream divider value on cold reset path and re-used it
   in CPU PM notifier instead of calculating every time.
2. Rebased on v3.11-rc2(to avoid conflicts with __cpuinit* deletion)
3. Dropped "ARM/ARM64: arch_timer: remove __cpuinit attribute for
   arch_counter_set_user_access"(already done as part of __cpuinit*
   deletion)

Regards,
Sudeep


Sudeep KarkadaNagesha (2):
  ARM: arch_timer: add macros for bits in control register
  drivers: clocksource: add CPU PM notifier for ARM architected timer

Will Deacon (1):
  drivers: clocksource: configure event stream for ARM arch timer

 arch/arm/include/asm/arch_timer.h    | 10 ++++++--
 arch/arm64/include/asm/arch_timer.h  | 12 +++++++---
 drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++++++++++++-
 include/clocksource/arm_arch_timer.h | 10 ++++++++
 4 files changed, 71 insertions(+), 6 deletions(-)

-- 
1.8.1.2

[PATCH v2 1/3] drivers: clocksource: configure event stream for ARM arch timer

[Sudeep KarkadaNagesha]

From: Will Deacon <will.deacon@arm.com>

The ARM architected timer can generate events (used for waking up
CPUs executing the wfe instruction) at a frequency represented as a
power-of-2 divisor of the clock rate.

This patch configures the event stream, aiming for a period of 100us
between events. This can be used to implement wfe-based timeouts for
userspace locking implementations.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/arch_timer.h    | 5 +++--
 arch/arm64/include/asm/arch_timer.h  | 6 +++---
 drivers/clocksource/arm_arch_timer.c | 9 ++++++++-
 include/clocksource/arm_arch_timer.h | 2 ++
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index e406d57..c3d9ef7 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -89,14 +89,15 @@ static inline u64 arch_counter_get_cntvct(void)
 	return cval;
 }
 
-static inline void arch_counter_set_user_access(void)
+static inline void arch_counter_set_user_access(int divider)
 {
 	u32 cntkctl;
 
 	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
 
 	/* disable user access to everything */
-	cntkctl &= ~((3 << 8) | (7 << 0));
+	cntkctl &= ~((3 << 8) | (0xf << 4) | (3 << 0));
+	cntkctl |= (divider << 4) | (1 << 2);
 
 	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
 }
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 98abd47..8c1e42f 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -97,16 +97,16 @@ static inline u32 arch_timer_get_cntfrq(void)
 	return val;
 }
 
-static inline void arch_counter_set_user_access(void)
+static inline void arch_counter_set_user_access(int divider)
 {
 	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));
+	cntkctl &= ~((3 << 8) | (0xf << 4) | (1 << 0));
 
 	/* Enable user access to the virtual counter and frequency. */
-	cntkctl |= (1 << 1);
+	cntkctl |= (divider << 4) | (1 << 2) | (1 << 1);
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index ffadd83..6301ee5 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -125,6 +125,8 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
 
 static int arch_timer_setup(struct clock_event_device *clk)
 {
+	int evt_stream_div, pos;
+
 	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
 	clk->name = "arch_sys_timer";
 	clk->rating = 450;
@@ -153,7 +155,12 @@ static int arch_timer_setup(struct clock_event_device *clk)
 			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
 	}
 
-	arch_counter_set_user_access();
+	/* Find the closest power of two to the divisor */
+	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
+	pos = fls(evt_stream_div);
+	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
+		pos--;
+	arch_counter_set_user_access(min(pos, 15));
 
 	return 0;
 }
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index c463ce9..24dc140 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -29,6 +29,8 @@
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
 
+#define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
+
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
-- 
1.8.1.2

Re: [PATCH v2 1/3] drivers: clocksource: configure event stream for ARM arch timer

[Catalin Marinas]

On Mon, Jul 22, 2013 at 12:21:20PM +0100, Sudeep KarkadaNagesha wrote:
> From: Will Deacon <will.deacon@arm.com>
> 
> The ARM architected timer can generate events (used for waking up
> CPUs executing the wfe instruction) at a frequency represented as a
> power-of-2 divisor of the clock rate.
> 
> This patch configures the event stream, aiming for a period of 100us
> between events. This can be used to implement wfe-based timeouts for
> userspace locking implementations.
...
> --- a/include/clocksource/arm_arch_timer.h
> +++ b/include/clocksource/arm_arch_timer.h
> @@ -29,6 +29,8 @@
>  #define ARCH_TIMER_PHYS_ACCESS		0
>  #define ARCH_TIMER_VIRT_ACCESS		1
>  
> +#define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */

BTW, if user-space starts using this, it will become an ABI. Is this the
right frequency?

In addition, do we want to expose this via hwcap? Something like
HWCAP_EVSTR100US?

-- 
Catalin

Re: [PATCH v2 1/3] drivers: clocksource: configure event stream for ARM arch timer

[Will Deacon]

On Tue, Jul 23, 2013 at 11:23:34AM +0100, Catalin Marinas wrote:
> On Mon, Jul 22, 2013 at 12:21:20PM +0100, Sudeep KarkadaNagesha wrote:
> > From: Will Deacon <will.deacon@arm.com>
> > 
> > The ARM architected timer can generate events (used for waking up
> > CPUs executing the wfe instruction) at a frequency represented as a
> > power-of-2 divisor of the clock rate.
> > 
> > This patch configures the event stream, aiming for a period of 100us
> > between events. This can be used to implement wfe-based timeouts for
> > userspace locking implementations.
> ...
> > --- a/include/clocksource/arm_arch_timer.h
> > +++ b/include/clocksource/arm_arch_timer.h
> > @@ -29,6 +29,8 @@
> >  #define ARCH_TIMER_PHYS_ACCESS		0
> >  #define ARCH_TIMER_VIRT_ACCESS		1
> >  
> > +#define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
> 
> BTW, if user-space starts using this, it will become an ABI. Is this the
> right frequency?

It doesn't quite become ABI; not all platforms will use the architected
timers and not all timers can support an arbitrary frequency. The best we
can do is calculate something as close to the target value as possible.

I spoke to both tools developers and some HSA driver guys about the frequency,
and this is what ended up being suggested.

> In addition, do we want to expose this via hwcap? Something like
> HWCAP_EVSTR100US?

Hmm, maybe, but we don't want people to try and use this for any accurate
time measurements, so I wouldn't include the period.

Will

Re: [PATCH v2 1/3] drivers: clocksource: configure event stream for ARM arch timer

[Catalin Marinas]

On Tue, Jul 23, 2013 at 11:33:33AM +0100, Will Deacon wrote:
> On Tue, Jul 23, 2013 at 11:23:34AM +0100, Catalin Marinas wrote:
> > On Mon, Jul 22, 2013 at 12:21:20PM +0100, Sudeep KarkadaNagesha wrote:
> > > From: Will Deacon <will.deacon@arm.com>
> > > 
> > > The ARM architected timer can generate events (used for waking up
> > > CPUs executing the wfe instruction) at a frequency represented as a
> > > power-of-2 divisor of the clock rate.
> > > 
> > > This patch configures the event stream, aiming for a period of 100us
> > > between events. This can be used to implement wfe-based timeouts for
> > > userspace locking implementations.
> > ...
> > > --- a/include/clocksource/arm_arch_timer.h
> > > +++ b/include/clocksource/arm_arch_timer.h
> > > @@ -29,6 +29,8 @@
> > >  #define ARCH_TIMER_PHYS_ACCESS		0
> > >  #define ARCH_TIMER_VIRT_ACCESS		1
> > >  
> > > +#define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
> > 
> > BTW, if user-space starts using this, it will become an ABI. Is this the
> > right frequency?
> 
> It doesn't quite become ABI; not all platforms will use the architected
> timers and not all timers can support an arbitrary frequency. The best we
> can do is calculate something as close to the target value as possible.

ABI in the sense that if it is available and advertised by the kernel as
such, people may use it.

> I spoke to both tools developers and some HSA driver guys about the frequency,
> and this is what ended up being suggested.
> 
> > In addition, do we want to expose this via hwcap? Something like
> > HWCAP_EVSTR100US?
> 
> Hmm, maybe, but we don't want people to try and use this for any accurate
> time measurements, so I wouldn't include the period.

Definitely not for accurate time but some user-space may find the delay
too small or too large. I'm fine without specifying the period, maybe
add a comment in the kernel like /* currently 100us */.

-- 
Catalin

[PATCH v3 1/3] drivers: clocksource: configure event stream for ARM arch timer

[Sudeep KarkadaNagesha]

From: Will Deacon <will.deacon@arm.com>

The ARM architected timer can generate events (used for waking up
CPUs executing the wfe instruction) at a frequency represented as a
power-of-2 divisor of the clock rate.

This patch configures the event stream, aiming for a period of 100us
between events. This can be used to implement wfe-based timeouts for
userspace locking implementations.

This patch also adds the hwcaps for the same, so that userspace can
detect this feature.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/arch_timer.h    | 6 ++++--
 arch/arm/include/uapi/asm/hwcap.h    | 1 +
 arch/arm/kernel/setup.c              | 1 +
 arch/arm64/include/asm/arch_timer.h  | 6 +++---
 drivers/clocksource/arm_arch_timer.c | 9 ++++++++-
 include/clocksource/arm_arch_timer.h | 2 ++
 6 files changed, 19 insertions(+), 6 deletions(-)

------------>8---------------------

Hi,

This version adds the hwcaps for publishing the event stream feature to the
userspace.

This patch is now rebased on rmk's fixes branch as it conflicts with ab8d46c0 
"ARM: 7788/1: elf: fix lpae hwcap feature reporting in proc/cpuinfo"

Regards,
Sudeep

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index e406d57..8963877 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -89,16 +89,18 @@ static inline u64 arch_counter_get_cntvct(void)
 	return cval;
 }
 
-static inline void arch_counter_set_user_access(void)
+static inline void arch_counter_set_user_access(int divider)
 {
 	u32 cntkctl;
 
 	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
 
 	/* disable user access to everything */
-	cntkctl &= ~((3 << 8) | (7 << 0));
+	cntkctl &= ~((3 << 8) | (0xf << 4) | (3 << 0));
+	cntkctl |= (divider << 4) | (1 << 2);
 
 	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
+	elf_hwcap |= HWCAP_EVTSTRM;
 }
 #endif
 
diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
index 6d34d08..7dcc10d 100644
--- a/arch/arm/include/uapi/asm/hwcap.h
+++ b/arch/arm/include/uapi/asm/hwcap.h
@@ -26,5 +26,6 @@
 #define HWCAP_VFPD32	(1 << 19)	/* set if VFP has 32 regs (not 16) */
 #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
 #define HWCAP_LPAE	(1 << 20)
+#define HWCAP_EVTSTRM	(1 << 21)
 
 #endif /* _UAPI__ASMARM_HWCAP_H */
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 96286cb..5191956 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -973,6 +973,7 @@ static const char *hwcap_str[] = {
 	"idivt",
 	"vfpd32",
 	"lpae",
+	"evtstrm",
 	NULL
 };
 
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 98abd47..8c1e42f 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -97,16 +97,16 @@ static inline u32 arch_timer_get_cntfrq(void)
 	return val;
 }
 
-static inline void arch_counter_set_user_access(void)
+static inline void arch_counter_set_user_access(int divider)
 {
 	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));
+	cntkctl &= ~((3 << 8) | (0xf << 4) | (1 << 0));
 
 	/* Enable user access to the virtual counter and frequency. */
-	cntkctl |= (1 << 1);
+	cntkctl |= (divider << 4) | (1 << 2) | (1 << 1);
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index ffadd83..6301ee5 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -125,6 +125,8 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
 
 static int arch_timer_setup(struct clock_event_device *clk)
 {
+	int evt_stream_div, pos;
+
 	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
 	clk->name = "arch_sys_timer";
 	clk->rating = 450;
@@ -153,7 +155,12 @@ static int arch_timer_setup(struct clock_event_device *clk)
 			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
 	}
 
-	arch_counter_set_user_access();
+	/* Find the closest power of two to the divisor */
+	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
+	pos = fls(evt_stream_div);
+	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
+		pos--;
+	arch_counter_set_user_access(min(pos, 15));
 
 	return 0;
 }
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index c463ce9..24dc140 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -29,6 +29,8 @@
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
 
+#define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
+
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
-- 
1.8.1.2

Re: [PATCH v3 1/3] drivers: clocksource: configure event stream for ARM arch timer

[Sudeep KarkadaNagesha]

On 25/07/13 10:45, Sudeep KarkadaNagesha wrote:
> From: Will Deacon <will.deacon@arm.com>
> 
> The ARM architected timer can generate events (used for waking up
> CPUs executing the wfe instruction) at a frequency represented as a
> power-of-2 divisor of the clock rate.
> 
> This patch configures the event stream, aiming for a period of 100us
> between events. This can be used to implement wfe-based timeouts for
> userspace locking implementations.
> 
> This patch also adds the hwcaps for the same, so that userspace can
> detect this feature.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/include/asm/arch_timer.h    | 6 ++++--
>  arch/arm/include/uapi/asm/hwcap.h    | 1 +
>  arch/arm/kernel/setup.c              | 1 +
>  arch/arm64/include/asm/arch_timer.h  | 6 +++---
>  drivers/clocksource/arm_arch_timer.c | 9 ++++++++-
>  include/clocksource/arm_arch_timer.h | 2 ++
>  6 files changed, 19 insertions(+), 6 deletions(-)
> 
> ------------>8---------------------
> 
> Hi,
> 
> This version adds the hwcaps for publishing the event stream feature to the
> userspace.
> 
Hi Catalin,

If you are OK with this patch, can I have your ACK ?

Regards,
Sudeep

> This patch is now rebased on rmk's fixes branch as it conflicts with ab8d46c0 
> "ARM: 7788/1: elf: fix lpae hwcap feature reporting in proc/cpuinfo"
> 
> Regards,
> Sudeep
> 
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index e406d57..8963877 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -89,16 +89,18 @@ static inline u64 arch_counter_get_cntvct(void)
>  	return cval;
>  }
>  
> -static inline void arch_counter_set_user_access(void)
> +static inline void arch_counter_set_user_access(int divider)
>  {
>  	u32 cntkctl;
>  
>  	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
>  
>  	/* disable user access to everything */
> -	cntkctl &= ~((3 << 8) | (7 << 0));
> +	cntkctl &= ~((3 << 8) | (0xf << 4) | (3 << 0));
> +	cntkctl |= (divider << 4) | (1 << 2);
>  
>  	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
> +	elf_hwcap |= HWCAP_EVTSTRM;
>  }
>  #endif
>  
> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> index 6d34d08..7dcc10d 100644
> --- a/arch/arm/include/uapi/asm/hwcap.h
> +++ b/arch/arm/include/uapi/asm/hwcap.h
> @@ -26,5 +26,6 @@
>  #define HWCAP_VFPD32	(1 << 19)	/* set if VFP has 32 regs (not 16) */
>  #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
>  #define HWCAP_LPAE	(1 << 20)
> +#define HWCAP_EVTSTRM	(1 << 21)
>  
>  #endif /* _UAPI__ASMARM_HWCAP_H */
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 96286cb..5191956 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -973,6 +973,7 @@ static const char *hwcap_str[] = {
>  	"idivt",
>  	"vfpd32",
>  	"lpae",
> +	"evtstrm",
>  	NULL
>  };
>  
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 98abd47..8c1e42f 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -97,16 +97,16 @@ static inline u32 arch_timer_get_cntfrq(void)
>  	return val;
>  }
>  
> -static inline void arch_counter_set_user_access(void)
> +static inline void arch_counter_set_user_access(int divider)
>  {
>  	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));
> +	cntkctl &= ~((3 << 8) | (0xf << 4) | (1 << 0));
>  
>  	/* Enable user access to the virtual counter and frequency. */
> -	cntkctl |= (1 << 1);
> +	cntkctl |= (divider << 4) | (1 << 2) | (1 << 1);
>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>  }
>  
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index ffadd83..6301ee5 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -125,6 +125,8 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
>  
>  static int arch_timer_setup(struct clock_event_device *clk)
>  {
> +	int evt_stream_div, pos;
> +
>  	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
>  	clk->name = "arch_sys_timer";
>  	clk->rating = 450;
> @@ -153,7 +155,12 @@ static int arch_timer_setup(struct clock_event_device *clk)
>  			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
>  	}
>  
> -	arch_counter_set_user_access();
> +	/* Find the closest power of two to the divisor */
> +	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
> +	pos = fls(evt_stream_div);
> +	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> +		pos--;
> +	arch_counter_set_user_access(min(pos, 15));
>  
>  	return 0;
>  }
> diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
> index c463ce9..24dc140 100644
> --- a/include/clocksource/arm_arch_timer.h
> +++ b/include/clocksource/arm_arch_timer.h
> @@ -29,6 +29,8 @@
>  #define ARCH_TIMER_PHYS_ACCESS		0
>  #define ARCH_TIMER_VIRT_ACCESS		1
>  
> +#define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
> +
>  #ifdef CONFIG_ARM_ARCH_TIMER
>  
>  extern u32 arch_timer_get_rate(void);
> 

[PATCH v2 2/3] ARM: arch_timer: add macros for bits in control register

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Add macros to describe the bitfields in the ARM architected timer
control register to make code easy to understand.

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/arm/include/asm/arch_timer.h    |  9 +++++++--
 arch/arm64/include/asm/arch_timer.h  | 10 ++++++++--
 include/clocksource/arm_arch_timer.h |  8 ++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index c3d9ef7..26c65ae 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -96,8 +96,13 @@ static inline void arch_counter_set_user_access(int divider)
 	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
 
 	/* disable user access to everything */
-	cntkctl &= ~((3 << 8) | (0xf << 4) | (3 << 0));
-	cntkctl |= (divider << 4) | (1 << 2);
+	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
+			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_EVT_TRIGGER_MASK
+			| ARCH_TIMER_USR_VCT_ACCESS_EN
+			| ARCH_TIMER_USR_PCT_ACCESS_EN);
+	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+			| ARCH_TIMER_VIRT_EVT_EN;
 
 	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
 }
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 8c1e42f..3494102 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -103,10 +103,16 @@ static inline void arch_counter_set_user_access(int divider)
 
 	/* Disable user access to the timers and the physical counter. */
 	asm volatile("mrs	%0, cntkctl_el1" : "=r" (cntkctl));
-	cntkctl &= ~((3 << 8) | (0xf << 4) | (1 << 0));
+	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
+			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_EVT_TRIGGER_MASK
+			| ARCH_TIMER_USR_PCT_ACCESS_EN)
 
 	/* Enable user access to the virtual counter and frequency. */
-	cntkctl |= (divider << 4) | (1 << 2) | (1 << 1);
+	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+			| ARCH_TIMER_VIRT_EVT_EN
+			| ARCH_TIMER_USR_VCT_ACCESS_EN;
+
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 24dc140..c4d0fc4 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -29,6 +29,14 @@
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
 
+#define ARCH_TIMER_USR_PCT_ACCESS_EN	(1 << 0) /* physical counter */
+#define ARCH_TIMER_USR_VCT_ACCESS_EN	(1 << 1) /* virtual counter */
+#define ARCH_TIMER_VIRT_EVT_EN		(1 << 2)
+#define ARCH_TIMER_EVT_TRIGGER_SHIFT	(4)
+#define ARCH_TIMER_EVT_TRIGGER_MASK	(0xF << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+#define ARCH_TIMER_USR_VT_ACCESS_EN	(1 << 8) /* virtual timer registers */
+#define ARCH_TIMER_USR_PT_ACCESS_EN	(1 << 9) /* physical timer registers */
+
 #define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
 
 #ifdef CONFIG_ARM_ARCH_TIMER
-- 
1.8.1.2

[PATCH v2 3/3] drivers: clocksource: add CPU PM notifier for ARM architected timer

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Few control settings done in architected timer as part of initialisation
can be lost when CPU enters deeper power states. They need to be
re-initialised when the CPU is (warm)reset again.

This patch adds CPU PM notifiers to do the timer initialisation on warm
resets. It also save the event stream divider value calculated during
cold reset and uses the same in warm reset path.

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 38 +++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 6301ee5..7057552 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -13,6 +13,7 @@
 #include <linux/device.h>
 #include <linux/smp.h>
 #include <linux/cpu.h>
+#include <linux/cpu_pm.h>
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
@@ -39,6 +40,8 @@ static struct clock_event_device __percpu *arch_timer_evt;
 
 static bool arch_timer_use_virtual = true;
 
+static int arch_timer_evtstream_div;
+
 /*
  * Architected system timer support.
  */
@@ -160,7 +163,9 @@ static int arch_timer_setup(struct clock_event_device *clk)
 	pos = fls(evt_stream_div);
 	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
 		pos--;
-	arch_counter_set_user_access(min(pos, 15));
+	/* save divider value for use in CPU PM notifier */
+	arch_timer_evtstream_div = min(pos, 15);
+	arch_counter_set_user_access(arch_timer_evtstream_div);
 
 	return 0;
 }
@@ -267,6 +272,31 @@ static struct notifier_block arch_timer_cpu_nb = {
 	.notifier_call = arch_timer_cpu_notify,
 };
 
+#ifdef CONFIG_CPU_PM
+static int arch_timer_cpu_pm_notify(struct notifier_block *self,
+				    unsigned long action, void *hcpu)
+{
+	if (action == CPU_PM_EXIT)
+		arch_counter_set_user_access(arch_timer_evtstream_div);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block arch_timer_cpu_pm_notifier = {
+	.notifier_call = arch_timer_cpu_pm_notify,
+};
+
+static int __init arch_timer_cpu_pm_init(void)
+{
+	return cpu_pm_register_notifier(&arch_timer_cpu_pm_notifier);
+}
+#else
+static int __init arch_timer_cpu_pm_init(void)
+{
+	return 0;
+}
+#endif
+
 static int __init arch_timer_register(void)
 {
 	int err;
@@ -316,11 +346,17 @@ static int __init arch_timer_register(void)
 	if (err)
 		goto out_free_irq;
 
+	err = arch_timer_cpu_pm_init();
+	if (err)
+		goto out_unreg_notify;
+
 	/* Immediately configure the timer on the boot CPU */
 	arch_timer_setup(this_cpu_ptr(arch_timer_evt));
 
 	return 0;
 
+out_unreg_notify:
+	unregister_cpu_notifier(&arch_timer_cpu_nb);
 out_free_irq:
 	if (arch_timer_use_virtual)
 		free_percpu_irq(arch_timer_ppi[VIRT_PPI], arch_timer_evt);
-- 
1.8.1.2

Re: [PATCH v2 0/3] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

Hi Daniel,

On 22/07/13 12:21, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> This patch series adds support to configure the rate and enable the event
> stream for architected timer. Since the timer control register is reset to
> zero on warm boot, CPU PM notifier is added to re-initialse it.
> 
This series is required to support TC2 cpuidle.
It will be good if you can review and get this in for v3.12

Regards,
Sudeep

> Changes v1->v2:
> 1. Saved event stream divider value on cold reset path and re-used it
>    in CPU PM notifier instead of calculating every time.
> 2. Rebased on v3.11-rc2(to avoid conflicts with __cpuinit* deletion)
> 3. Dropped "ARM/ARM64: arch_timer: remove __cpuinit attribute for
>    arch_counter_set_user_access"(already done as part of __cpuinit*
>    deletion)
> 
> Regards,
> Sudeep
> 
> 
> Sudeep KarkadaNagesha (2):
>   ARM: arch_timer: add macros for bits in control register
>   drivers: clocksource: add CPU PM notifier for ARM architected timer
> 
> Will Deacon (1):
>   drivers: clocksource: configure event stream for ARM arch timer
> 
>  arch/arm/include/asm/arch_timer.h    | 10 ++++++--
>  arch/arm64/include/asm/arch_timer.h  | 12 +++++++---
>  drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++++++++++++-
>  include/clocksource/arm_arch_timer.h | 10 ++++++++
>  4 files changed, 71 insertions(+), 6 deletions(-)
> 

Re: [PATCH v2 0/3] ARM/ARM64 architected timer updates

[Daniel Lezcano]

On 08/02/2013 11:32 AM, Sudeep KarkadaNagesha wrote:
> Hi Daniel,
> 
> On 22/07/13 12:21, Sudeep KarkadaNagesha wrote:
>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>
>> This patch series adds support to configure the rate and enable the event
>> stream for architected timer. Since the timer control register is reset to
>> zero on warm boot, CPU PM notifier is added to re-initialse it.
>>
> This series is required to support TC2 cpuidle.
> It will be good if you can review and get this in for v3.12

So do you want me to take the series through my tree ?

> Regards,
> Sudeep
> 
>> Changes v1->v2:
>> 1. Saved event stream divider value on cold reset path and re-used it
>>    in CPU PM notifier instead of calculating every time.
>> 2. Rebased on v3.11-rc2(to avoid conflicts with __cpuinit* deletion)
>> 3. Dropped "ARM/ARM64: arch_timer: remove __cpuinit attribute for
>>    arch_counter_set_user_access"(already done as part of __cpuinit*
>>    deletion)
>>
>> Regards,
>> Sudeep
>>
>>
>> Sudeep KarkadaNagesha (2):
>>   ARM: arch_timer: add macros for bits in control register
>>   drivers: clocksource: add CPU PM notifier for ARM architected timer
>>
>> Will Deacon (1):
>>   drivers: clocksource: configure event stream for ARM arch timer
>>
>>  arch/arm/include/asm/arch_timer.h    | 10 ++++++--
>>  arch/arm64/include/asm/arch_timer.h  | 12 +++++++---
>>  drivers/clocksource/arm_arch_timer.c | 45 +++++++++++++++++++++++++++++++++++-
>>  include/clocksource/arm_arch_timer.h | 10 ++++++++
>>  4 files changed, 71 insertions(+), 6 deletions(-)
>>
> 
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2 0/3] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

On 02/08/13 10:48, Daniel Lezcano wrote:
> On 08/02/2013 11:32 AM, Sudeep KarkadaNagesha wrote:
>> Hi Daniel,
>>
>> On 22/07/13 12:21, Sudeep KarkadaNagesha wrote:
>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>
>>> This patch series adds support to configure the rate and enable the event
>>> stream for architected timer. Since the timer control register is reset to
>>> zero on warm boot, CPU PM notifier is added to re-initialse it.
>>>
>> This series is required to support TC2 cpuidle.
>> It will be good if you can review and get this in for v3.12
> 
> So do you want me to take the series through my tree ?
> 
Yes I believe it has to go through your(ARM specific clocksource?) tree,
but currently there's a conflict with another bug fix(that should come
from rmk as -rc fix)
(Commit ab8d46c0 "ARM: 7788/1: elf: fix lpae hwcap feature reporting in
proc/cpuinfo" in rmk -fixes)

Regards,
Sudeep

Re: [PATCH v2 0/3] ARM/ARM64 architected timer updates

[Daniel Lezcano]

On 08/02/2013 11:55 AM, Sudeep KarkadaNagesha wrote:
> On 02/08/13 10:48, Daniel Lezcano wrote:
>> On 08/02/2013 11:32 AM, Sudeep KarkadaNagesha wrote:
>>> Hi Daniel,
>>>
>>> On 22/07/13 12:21, Sudeep KarkadaNagesha wrote:
>>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>
>>>> This patch series adds support to configure the rate and enable the event
>>>> stream for architected timer. Since the timer control register is reset to
>>>> zero on warm boot, CPU PM notifier is added to re-initialse it.
>>>>
>>> This series is required to support TC2 cpuidle.
>>> It will be good if you can review and get this in for v3.12
>>
>> So do you want me to take the series through my tree ?
>>
> Yes I believe it has to go through your(ARM specific clocksource?) tree,
> but currently there's a conflict with another bug fix(that should come
> from rmk as -rc fix)
> (Commit ab8d46c0 "ARM: 7788/1: elf: fix lpae hwcap feature reporting in
> proc/cpuinfo" in rmk -fixes)

IMO, you should rebase this patch on top of tips/core, I will apply the
series and when the rmk fix hits this branch (through tips/urgent), that
will be up to the maintainer to resolve this conflict.


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2 0/3] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

Hi Daniel,

On 02/08/13 13:12, Daniel Lezcano wrote:
> On 08/02/2013 11:55 AM, Sudeep KarkadaNagesha wrote:
>> On 02/08/13 10:48, Daniel Lezcano wrote:
>>> On 08/02/2013 11:32 AM, Sudeep KarkadaNagesha wrote:
>>>> Hi Daniel,
>>>>
>>>> On 22/07/13 12:21, Sudeep KarkadaNagesha wrote:
>>>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>>
>>>>> This patch series adds support to configure the rate and enable the event
>>>>> stream for architected timer. Since the timer control register is reset to
>>>>> zero on warm boot, CPU PM notifier is added to re-initialse it.
>>>>>
>>>> This series is required to support TC2 cpuidle.
>>>> It will be good if you can review and get this in for v3.12
>>>
>>> So do you want me to take the series through my tree ?
>>>
>> Yes I believe it has to go through your(ARM specific clocksource?) tree,
>> but currently there's a conflict with another bug fix(that should come
>> from rmk as -rc fix)
>> (Commit ab8d46c0 "ARM: 7788/1: elf: fix lpae hwcap feature reporting in
>> proc/cpuinfo" in rmk -fixes)
> 
> IMO, you should rebase this patch on top of tips/core, I will apply the
> series and when the rmk fix hits this branch (through tips/urgent), that
> will be up to the maintainer to resolve this conflict.
> 

The fix with which this series was conflicting is already in v3.11-rc4
So can I send pull request based on v3.11-rc4 or if you prefer any
particular tree, can you provide details please ?

Regards,
Sudeep

Re: [PATCH v2 0/3] ARM/ARM64 architected timer updates

[Daniel Lezcano]

On 08/05/2013 06:25 PM, Sudeep KarkadaNagesha wrote:
> Hi Daniel,
> 
> On 02/08/13 13:12, Daniel Lezcano wrote:
>> On 08/02/2013 11:55 AM, Sudeep KarkadaNagesha wrote:
>>> On 02/08/13 10:48, Daniel Lezcano wrote:
>>>> On 08/02/2013 11:32 AM, Sudeep KarkadaNagesha wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> On 22/07/13 12:21, Sudeep KarkadaNagesha wrote:
>>>>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>>>
>>>>>> This patch series adds support to configure the rate and enable the event
>>>>>> stream for architected timer. Since the timer control register is reset to
>>>>>> zero on warm boot, CPU PM notifier is added to re-initialse it.
>>>>>>
>>>>> This series is required to support TC2 cpuidle.
>>>>> It will be good if you can review and get this in for v3.12
>>>>
>>>> So do you want me to take the series through my tree ?
>>>>
>>> Yes I believe it has to go through your(ARM specific clocksource?) tree,
>>> but currently there's a conflict with another bug fix(that should come
>>> from rmk as -rc fix)
>>> (Commit ab8d46c0 "ARM: 7788/1: elf: fix lpae hwcap feature reporting in
>>> proc/cpuinfo" in rmk -fixes)
>>
>> IMO, you should rebase this patch on top of tips/core, I will apply the
>> series and when the rmk fix hits this branch (through tips/urgent), that
>> will be up to the maintainer to resolve this conflict.
>>
> 
> The fix with which this series was conflicting is already in v3.11-rc4
> So can I send pull request based on v3.11-rc4 or if you prefer any
> particular tree, can you provide details please ?

Ok, I will ping Thomas to update the tips/core branch to 3.11-rc4.



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2 0/3] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

On 06/08/13 09:09, Daniel Lezcano wrote:
> On 08/05/2013 06:25 PM, Sudeep KarkadaNagesha wrote:
>> Hi Daniel,
>>
>> On 02/08/13 13:12, Daniel Lezcano wrote:
>>> On 08/02/2013 11:55 AM, Sudeep KarkadaNagesha wrote:
>>>> On 02/08/13 10:48, Daniel Lezcano wrote:
>>>>> On 08/02/2013 11:32 AM, Sudeep KarkadaNagesha wrote:
>>>>>> Hi Daniel,
>>>>>>
>>>>>> On 22/07/13 12:21, Sudeep KarkadaNagesha wrote:
>>>>>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>>>>
>>>>>>> This patch series adds support to configure the rate and enable the event
>>>>>>> stream for architected timer. Since the timer control register is reset to
>>>>>>> zero on warm boot, CPU PM notifier is added to re-initialse it.
>>>>>>>
>>>>>> This series is required to support TC2 cpuidle.
>>>>>> It will be good if you can review and get this in for v3.12
>>>>>
>>>>> So do you want me to take the series through my tree ?
>>>>>
>>>> Yes I believe it has to go through your(ARM specific clocksource?) tree,
>>>> but currently there's a conflict with another bug fix(that should come
>>>> from rmk as -rc fix)
>>>> (Commit ab8d46c0 "ARM: 7788/1: elf: fix lpae hwcap feature reporting in
>>>> proc/cpuinfo" in rmk -fixes)
>>>
>>> IMO, you should rebase this patch on top of tips/core, I will apply the
>>> series and when the rmk fix hits this branch (through tips/urgent), that
>>> will be up to the maintainer to resolve this conflict.
>>>
>>
>> The fix with which this series was conflicting is already in v3.11-rc4
>> So can I send pull request based on v3.11-rc4 or if you prefer any
>> particular tree, can you provide details please ?
> 
> Ok, I will ping Thomas to update the tips/core branch to 3.11-rc4.
> 
Thanks Daniel, will send pull request once it's updated.

Regards
Sudeep

Re: [PATCH v2 0/3] ARM/ARM64 architected timer updates

[Daniel Lezcano]

On 08/06/2013 11:01 AM, Sudeep KarkadaNagesha wrote:
> On 06/08/13 09:09, Daniel Lezcano wrote:
>> On 08/05/2013 06:25 PM, Sudeep KarkadaNagesha wrote:
>>> Hi Daniel,
>>>
>>> On 02/08/13 13:12, Daniel Lezcano wrote:
>>>> On 08/02/2013 11:55 AM, Sudeep KarkadaNagesha wrote:
>>>>> On 02/08/13 10:48, Daniel Lezcano wrote:
>>>>>> On 08/02/2013 11:32 AM, Sudeep KarkadaNagesha wrote:
>>>>>>> Hi Daniel,
>>>>>>>
>>>>>>> On 22/07/13 12:21, Sudeep KarkadaNagesha wrote:
>>>>>>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>>>>>
>>>>>>>> This patch series adds support to configure the rate and enable the event
>>>>>>>> stream for architected timer. Since the timer control register is reset to
>>>>>>>> zero on warm boot, CPU PM notifier is added to re-initialse it.
>>>>>>>>
>>>>>>> This series is required to support TC2 cpuidle.
>>>>>>> It will be good if you can review and get this in for v3.12
>>>>>>
>>>>>> So do you want me to take the series through my tree ?
>>>>>>
>>>>> Yes I believe it has to go through your(ARM specific clocksource?) tree,
>>>>> but currently there's a conflict with another bug fix(that should come
>>>>> from rmk as -rc fix)
>>>>> (Commit ab8d46c0 "ARM: 7788/1: elf: fix lpae hwcap feature reporting in
>>>>> proc/cpuinfo" in rmk -fixes)
>>>>
>>>> IMO, you should rebase this patch on top of tips/core, I will apply the
>>>> series and when the rmk fix hits this branch (through tips/urgent), that
>>>> will be up to the maintainer to resolve this conflict.
>>>>
>>>
>>> The fix with which this series was conflicting is already in v3.11-rc4
>>> So can I send pull request based on v3.11-rc4 or if you prefer any
>>> particular tree, can you provide details please ?
>>
>> Ok, I will ping Thomas to update the tips/core branch to 3.11-rc4.
>>
> Thanks Daniel, will send pull request once it's updated.

Sudeep,

please send the PR, I will deal with it and send Thomas the PR later.

Thanks
  -- Daniel


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v3 0/6] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

This patch series adds support to configure the rate and enable the
event stream for architected timer. The event streams can be used to
impose a timeout on a WFE, to safeguard against any programming error
in case an expected event is not generated or even to implement
wfe-based timeouts for userspace locking implementations.

Since the timer control register is reset to zero on warm boot, CPU
PM notifier is added to re-initialize it.

Changes v2->v3:
1. Moved ARM and ARM64 changes into separate patches
2. Added native hwcaps definations(ARM/ARM64) and compat-specific
   definitions(ARM64) to the users for the event stream feature. 

Changes v1->v2:
1. Saved event stream divider value on cold reset path and re-used it
   in CPU PM notifier instead of calculating every time.
2. Rebased on v3.11-rc2(to avoid conflicts with __cpuinit* deletion)
3. Dropped "ARM/ARM64: arch_timer: remove __cpuinit attribute for
   arch_counter_set_user_access"(already done as part of __cpuinit*
   deletion)

Regards,
Sudeep


Sudeep KarkadaNagesha (5):
  ARM/ARM64: arch_timer: add macros for bits in control register
  ARM: arch_timer: configure and enable event stream
  ARM64: arch_timer: configure and enable event stream
  drivers: clocksource: enable hwcaps for event stream on ARM arch timer
  drivers: clocksource: add CPU PM notifier for ARM architected timer

Will Deacon (1):
  drivers: clocksource: configure event stream for ARM arch timer

 arch/arm/include/asm/arch_timer.h    | 18 +++++++++++---
 arch/arm/include/uapi/asm/hwcap.h    |  1 +
 arch/arm/kernel/setup.c              |  1 +
 arch/arm64/include/asm/arch_timer.h  | 22 +++++++++++++----
 arch/arm64/include/asm/hwcap.h       |  4 ++-
 arch/arm64/include/uapi/asm/hwcap.h  |  1 +
 arch/arm64/kernel/setup.c            |  1 +
 drivers/clocksource/arm_arch_timer.c | 48 +++++++++++++++++++++++++++++++++++-
 include/clocksource/arm_arch_timer.h | 10 ++++++++
 9 files changed, 96 insertions(+), 10 deletions(-)

-- 
1.8.1.2

[PATCH v3 1/6] ARM/ARM64: arch_timer: add macros for bits in control register

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Add macros to describe the bitfields in the ARM architected timer
control register to make code easy to understand.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/arm/include/asm/arch_timer.h    |  9 +++++++--
 arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
 include/clocksource/arm_arch_timer.h |  8 ++++++++
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index e406d57..9ef74da 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -95,8 +95,13 @@ static inline void arch_counter_set_user_access(void)
 
 	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
 
-	/* disable user access to everything */
-	cntkctl &= ~((3 << 8) | (7 << 0));
+	/* Disable user access to both physical/virtual counters/timers. */
+	/* Also disable virtual event stream */
+	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
+			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_VIRT_EVT_EN
+			| ARCH_TIMER_USR_VCT_ACCESS_EN
+			| ARCH_TIMER_USR_PCT_ACCESS_EN);
 
 	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
 }
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 98abd47..00b09d0 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -101,12 +101,16 @@ static inline void 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);
+	/* Disable user access to the timers and the physical counter. */
+	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
+			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_USR_PCT_ACCESS_EN);
+
+	/* Enable user access to the virtual counter. */
+	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
+
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index c463ce9..551f7e9 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -29,6 +29,14 @@
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
 
+#define ARCH_TIMER_USR_PCT_ACCESS_EN	(1 << 0) /* physical counter */
+#define ARCH_TIMER_USR_VCT_ACCESS_EN	(1 << 1) /* virtual counter */
+#define ARCH_TIMER_VIRT_EVT_EN		(1 << 2)
+#define ARCH_TIMER_EVT_TRIGGER_SHIFT	(4)
+#define ARCH_TIMER_EVT_TRIGGER_MASK	(0xF << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+#define ARCH_TIMER_USR_VT_ACCESS_EN	(1 << 8) /* virtual timer registers */
+#define ARCH_TIMER_USR_PT_ACCESS_EN	(1 << 9) /* physical timer registers */
+
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
-- 
1.8.1.2

Re: [PATCH v3 1/6] ARM/ARM64: arch_timer: add macros for bits in control register

[Daniel Lezcano]

On 08/13/2013 07:29 PM, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> Add macros to describe the bitfields in the ARM architected timer
> control register to make code easy to understand.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reviewed-by: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> ---
>  arch/arm/include/asm/arch_timer.h    |  9 +++++++--
>  arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
>  include/clocksource/arm_arch_timer.h |  8 ++++++++
>  3 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index e406d57..9ef74da 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -95,8 +95,13 @@ static inline void arch_counter_set_user_access(void)
>  
>  	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
>  
> -	/* disable user access to everything */
> -	cntkctl &= ~((3 << 8) | (7 << 0));
> +	/* Disable user access to both physical/virtual counters/timers. */
> +	/* Also disable virtual event stream */
> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
> +			| ARCH_TIMER_USR_VT_ACCESS_EN
> +			| ARCH_TIMER_VIRT_EVT_EN
> +			| ARCH_TIMER_USR_VCT_ACCESS_EN
> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>  
>  	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
>  }
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 98abd47..00b09d0 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -101,12 +101,16 @@ static inline void 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);
> +	/* Disable user access to the timers and the physical counter. */
> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
> +			| ARCH_TIMER_USR_VT_ACCESS_EN
> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
> +
> +	/* Enable user access to the virtual counter. */
> +	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> +
>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>  }

Before doing those changes, I suggest you kill include/asm/arch_timer.h
header file and move the functions directly in the arm_arch_timer.c as
they are only used by it.

> diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
> index c463ce9..551f7e9 100644
> --- a/include/clocksource/arm_arch_timer.h
> +++ b/include/clocksource/arm_arch_timer.h
> @@ -29,6 +29,14 @@
>  #define ARCH_TIMER_PHYS_ACCESS		0
>  #define ARCH_TIMER_VIRT_ACCESS		1
>  
> +#define ARCH_TIMER_USR_PCT_ACCESS_EN	(1 << 0) /* physical counter */
> +#define ARCH_TIMER_USR_VCT_ACCESS_EN	(1 << 1) /* virtual counter */
> +#define ARCH_TIMER_VIRT_EVT_EN		(1 << 2)
> +#define ARCH_TIMER_EVT_TRIGGER_SHIFT	(4)
> +#define ARCH_TIMER_EVT_TRIGGER_MASK	(0xF << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> +#define ARCH_TIMER_USR_VT_ACCESS_EN	(1 << 8) /* virtual timer registers */
> +#define ARCH_TIMER_USR_PT_ACCESS_EN	(1 << 9) /* physical timer registers */
> +
>  #ifdef CONFIG_ARM_ARCH_TIMER
>  
>  extern u32 arch_timer_get_rate(void);
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 1/6] ARM/ARM64: arch_timer: add macros for bits in control register

[Marc Zyngier]

Hi Daniel,

On 2013-08-14 09:50, Daniel Lezcano wrote:
> On 08/13/2013 07:29 PM, Sudeep KarkadaNagesha wrote:
>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>
>> Add macros to describe the bitfields in the ARM architected timer
>> control register to make code easy to understand.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Reviewed-by: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>> ---
>>  arch/arm/include/asm/arch_timer.h    |  9 +++++++--
>>  arch/arm64/include/asm/arch_timer.h  | 12 ++++++++----
>>  include/clocksource/arm_arch_timer.h |  8 ++++++++
>>  3 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch_timer.h 
>> b/arch/arm/include/asm/arch_timer.h
>> index e406d57..9ef74da 100644
>> --- a/arch/arm/include/asm/arch_timer.h
>> +++ b/arch/arm/include/asm/arch_timer.h
>> @@ -95,8 +95,13 @@ static inline void 
>> arch_counter_set_user_access(void)
>>
>>  	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
>>
>> -	/* disable user access to everything */
>> -	cntkctl &= ~((3 << 8) | (7 << 0));
>> +	/* Disable user access to both physical/virtual counters/timers. 
>> */
>> +	/* Also disable virtual event stream */
>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +			| ARCH_TIMER_VIRT_EVT_EN
>> +			| ARCH_TIMER_USR_VCT_ACCESS_EN
>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>
>>  	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
>>  }
>> diff --git a/arch/arm64/include/asm/arch_timer.h 
>> b/arch/arm64/include/asm/arch_timer.h
>> index 98abd47..00b09d0 100644
>> --- a/arch/arm64/include/asm/arch_timer.h
>> +++ b/arch/arm64/include/asm/arch_timer.h
>> @@ -101,12 +101,16 @@ static inline void 
>> 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);
>> +	/* Disable user access to the timers and the physical counter. */
>> +	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>> +			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>> +
>> +	/* Enable user access to the virtual counter. */
>> +	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>> +
>>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>>  }
>
> Before doing those changes, I suggest you kill 
> include/asm/arch_timer.h
> header file and move the functions directly in the arm_arch_timer.c 
> as
> they are only used by it.

How do you suggest to proceed? Having #ifdefs in arm_arch_timer.c to 
cope with the inline assembly functions present in both arm and arm64 
include files? Doesn't seem much of a progress to me...

>> diff --git a/include/clocksource/arm_arch_timer.h 
>> b/include/clocksource/arm_arch_timer.h
>> index c463ce9..551f7e9 100644
>> --- a/include/clocksource/arm_arch_timer.h
>> +++ b/include/clocksource/arm_arch_timer.h
>> @@ -29,6 +29,14 @@
>>  #define ARCH_TIMER_PHYS_ACCESS		0
>>  #define ARCH_TIMER_VIRT_ACCESS		1
>>
>> +#define ARCH_TIMER_USR_PCT_ACCESS_EN	(1 << 0) /* physical counter 
>> */
>> +#define ARCH_TIMER_USR_VCT_ACCESS_EN	(1 << 1) /* virtual counter */
>> +#define ARCH_TIMER_VIRT_EVT_EN		(1 << 2)
>> +#define ARCH_TIMER_EVT_TRIGGER_SHIFT	(4)
>> +#define ARCH_TIMER_EVT_TRIGGER_MASK	(0xF << 
>> ARCH_TIMER_EVT_TRIGGER_SHIFT)
>> +#define ARCH_TIMER_USR_VT_ACCESS_EN	(1 << 8) /* virtual timer 
>> registers */
>> +#define ARCH_TIMER_USR_PT_ACCESS_EN	(1 << 9) /* physical timer 
>> registers */
>> +
>>  #ifdef CONFIG_ARM_ARCH_TIMER
>>
>>  extern u32 arch_timer_get_rate(void);
>>

         M.
-- 
Fast, cheap, reliable. Pick two.

Re: [PATCH v3 1/6] ARM/ARM64: arch_timer: add macros for bits in control register

[Daniel Lezcano]

On 08/14/2013 11:03 AM, Marc Zyngier wrote:
> Hi Daniel,

[ ... ]

>>> --- a/arch/arm64/include/asm/arch_timer.h
>>> +++ b/arch/arm64/include/asm/arch_timer.h
>>> @@ -101,12 +101,16 @@ static inline void
>>> 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);
>>> +    /* Disable user access to the timers and the physical counter. */
>>> +    cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>> +            | ARCH_TIMER_USR_VT_ACCESS_EN
>>> +            | ARCH_TIMER_USR_PCT_ACCESS_EN);
>>> +
>>> +    /* Enable user access to the virtual counter. */
>>> +    cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>>> +
>>>      asm volatile("msr    cntkctl_el1, %0" : : "r" (cntkctl));
>>>  }
>>
>> Before doing those changes, I suggest you kill include/asm/arch_timer.h
>> header file and move the functions directly in the arm_arch_timer.c as
>> they are only used by it.
> 
> How do you suggest to proceed? Having #ifdefs in arm_arch_timer.c to
> cope with the inline assembly functions present in both arm and arm64
> include files? Doesn't seem much of a progress to me...

I understand, this is arguable.

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v3 2/6] drivers: clocksource: configure event stream for ARM arch timer

[Sudeep KarkadaNagesha]

From: Will Deacon <will.deacon@arm.com>

The ARM architected timer can generate events (used for waking up
CPUs executing the wfe instruction) at a frequency represented as a
power-of-2 divisor of the clock rate.

An event stream might be used:
- To impose a time-out on a WFE polling loop.
- To safeguard against any programming error in case an expected event
  is not generated.
- To implement wfe-based timeouts for userspace locking implementations.

This patch only computes the event stream frequency, aiming for a period
of 100us between events. ARM/ARM64 specific backends needs to configure
and enable the event stream using this frequency.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
[sudeep: moving ARM/ARM64 changes into separate patches,
         cosmetic changes to commit message]
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/arm/include/asm/arch_timer.h    | 2 +-
 arch/arm64/include/asm/arch_timer.h  | 2 +-
 drivers/clocksource/arm_arch_timer.c | 9 ++++++++-
 include/clocksource/arm_arch_timer.h | 2 ++
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 9ef74da..12f3fac 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -89,7 +89,7 @@ static inline u64 arch_counter_get_cntvct(void)
 	return cval;
 }
 
-static inline void arch_counter_set_user_access(void)
+static inline void arch_counter_set_user_access(int divider)
 {
 	u32 cntkctl;
 
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 00b09d0..69f5c8e 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -97,7 +97,7 @@ static inline u32 arch_timer_get_cntfrq(void)
 	return val;
 }
 
-static inline void arch_counter_set_user_access(void)
+static inline void arch_counter_set_user_access(int divider)
 {
 	u32 cntkctl;
 
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index ffadd83..6301ee5 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -125,6 +125,8 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
 
 static int arch_timer_setup(struct clock_event_device *clk)
 {
+	int evt_stream_div, pos;
+
 	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
 	clk->name = "arch_sys_timer";
 	clk->rating = 450;
@@ -153,7 +155,12 @@ static int arch_timer_setup(struct clock_event_device *clk)
 			enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
 	}
 
-	arch_counter_set_user_access();
+	/* Find the closest power of two to the divisor */
+	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
+	pos = fls(evt_stream_div);
+	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
+		pos--;
+	arch_counter_set_user_access(min(pos, 15));
 
 	return 0;
 }
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 551f7e9..c4d0fc4 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -37,6 +37,8 @@
 #define ARCH_TIMER_USR_VT_ACCESS_EN	(1 << 8) /* virtual timer registers */
 #define ARCH_TIMER_USR_PT_ACCESS_EN	(1 << 9) /* physical timer registers */
 
+#define ARCH_TIMER_EVT_STREAM_FREQ	10000	/* 100us */
+
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern u32 arch_timer_get_rate(void);
-- 
1.8.1.2

[PATCH v3 3/6] ARM: arch_timer: configure and enable event stream

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

This patch configures the event stream frequency and enables it.
It also adds the hwcaps definitions to the user to detect this event
stream feature.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
[sudeep: moving ARM/ARM64 changes into separate patches]
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/arm/include/asm/arch_timer.h | 11 +++++++++--
 arch/arm/include/uapi/asm/hwcap.h |  1 +
 arch/arm/kernel/setup.c           |  1 +
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 12f3fac..25abeb5 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -96,15 +96,22 @@ static inline void arch_counter_set_user_access(int divider)
 	asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
 
 	/* Disable user access to both physical/virtual counters/timers. */
-	/* Also disable virtual event stream */
 	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
 			| ARCH_TIMER_USR_VT_ACCESS_EN
-			| ARCH_TIMER_VIRT_EVT_EN
+			| ARCH_TIMER_EVT_TRIGGER_MASK
 			| ARCH_TIMER_USR_VCT_ACCESS_EN
 			| ARCH_TIMER_USR_PCT_ACCESS_EN);
+	/* Set the divider and enable virtual event stream */
+	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+			| ARCH_TIMER_VIRT_EVT_EN;
 
 	asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
 }
+
+static inline void arch_timer_set_hwcap_evtstrm(void)
+{
+	elf_hwcap |= HWCAP_EVTSTRM;
+}
 #endif
 
 #endif
diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
index 6d34d08..7dcc10d 100644
--- a/arch/arm/include/uapi/asm/hwcap.h
+++ b/arch/arm/include/uapi/asm/hwcap.h
@@ -26,5 +26,6 @@
 #define HWCAP_VFPD32	(1 << 19)	/* set if VFP has 32 regs (not 16) */
 #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
 #define HWCAP_LPAE	(1 << 20)
+#define HWCAP_EVTSTRM	(1 << 21)
 
 #endif /* _UAPI__ASMARM_HWCAP_H */
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index afc2489..305ffc9 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -975,6 +975,7 @@ static const char *hwcap_str[] = {
 	"idivt",
 	"vfpd32",
 	"lpae",
+	"evtstrm",
 	NULL
 };
 
-- 
1.8.1.2

[PATCH v3 4/6] ARM64: arch_timer: configure and enable event stream

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

This patch configures the event stream frequency and enables it.
It also adds the hwcaps as well as compat-specific definitions to
the user to detect this event stream feature.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
[sudeep: moving ARM/ARM64 changes into separate patches]
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/arm64/include/asm/arch_timer.h | 12 ++++++++++--
 arch/arm64/include/asm/hwcap.h      |  4 +++-
 arch/arm64/include/uapi/asm/hwcap.h |  1 +
 arch/arm64/kernel/setup.c           |  1 +
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 69f5c8e..43f322a 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -106,14 +106,22 @@ static inline void arch_counter_set_user_access(int divider)
 	/* Disable user access to the timers and the physical counter. */
 	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
 			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_EVT_TRIGGER_MASK
 			| ARCH_TIMER_USR_PCT_ACCESS_EN);
 
-	/* Enable user access to the virtual counter. */
-	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
+	/* Enable event stream and user access to the virtual counter */
+	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+			| ARCH_TIMER_VIRT_EVT_EN
+			| ARCH_TIMER_USR_VCT_ACCESS_EN;
 
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
+static inline void arch_timer_set_hwcap_evtstrm(void)
+{
+	elf_hwcap |= HWCAP_EVTSTRM;
+}
+
 static inline u64 arch_counter_get_cntvct(void)
 {
 	u64 cval;
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index 6d4482f..effd8f9 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -30,6 +30,7 @@
 #define COMPAT_HWCAP_IDIVA	(1 << 17)
 #define COMPAT_HWCAP_IDIVT	(1 << 18)
 #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
+#define COMPAT_HWCAP_EVTSTRM	(1 << 21)
 
 #ifndef __ASSEMBLY__
 /*
@@ -41,7 +42,8 @@
 				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
 				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
 				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
-				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
+				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
+				 COMPAT_HWCAP_EVTSTRM)
 
 extern unsigned int elf_hwcap;
 #endif
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index eea4975..9b12476 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -21,6 +21,7 @@
  */
 #define HWCAP_FP		(1 << 0)
 #define HWCAP_ASIMD		(1 << 1)
+#define HWCAP_EVTSTRM		(1 << 2)
 
 
 #endif /* _UAPI__ASM_HWCAP_H */
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index add6ea6..1111833 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -309,6 +309,7 @@ subsys_initcall(topology_init);
 static const char *hwcap_str[] = {
 	"fp",
 	"asimd",
+	"evtstrm",
 	NULL
 };
 
-- 
1.8.1.2

Re: [PATCH v3 4/6] ARM64: arch_timer: configure and enable event stream

[Will Deacon]

Hi Sudeep,

Couple of comments inline...

On Tue, Aug 13, 2013 at 06:29:42PM +0100, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> This patch configures the event stream frequency and enables it.
> It also adds the hwcaps as well as compat-specific definitions to
> the user to detect this event stream feature.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> [sudeep: moving ARM/ARM64 changes into separate patches]
> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> ---
>  arch/arm64/include/asm/arch_timer.h | 12 ++++++++++--
>  arch/arm64/include/asm/hwcap.h      |  4 +++-
>  arch/arm64/include/uapi/asm/hwcap.h |  1 +
>  arch/arm64/kernel/setup.c           |  1 +
>  4 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 69f5c8e..43f322a 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -106,14 +106,22 @@ static inline void arch_counter_set_user_access(int divider)
>  	/* Disable user access to the timers and the physical counter. */
>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>  			| ARCH_TIMER_USR_VT_ACCESS_EN
> +			| ARCH_TIMER_EVT_TRIGGER_MASK
>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>  
> -	/* Enable user access to the virtual counter. */
> -	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
> +	/* Enable event stream and user access to the virtual counter */
> +	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> +			| ARCH_TIMER_VIRT_EVT_EN
> +			| ARCH_TIMER_USR_VCT_ACCESS_EN;
>  
>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>  }
>  
> +static inline void arch_timer_set_hwcap_evtstrm(void)
> +{
> +	elf_hwcap |= HWCAP_EVTSTRM;
> +}
> +
>  static inline u64 arch_counter_get_cntvct(void)
>  {
>  	u64 cval;
> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> index 6d4482f..effd8f9 100644
> --- a/arch/arm64/include/asm/hwcap.h
> +++ b/arch/arm64/include/asm/hwcap.h
> @@ -30,6 +30,7 @@
>  #define COMPAT_HWCAP_IDIVA	(1 << 17)
>  #define COMPAT_HWCAP_IDIVT	(1 << 18)
>  #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
> +#define COMPAT_HWCAP_EVTSTRM	(1 << 21)

Looking at the context, we should also add COMPAT_HWCAP_LPAE as a separate
patch (not part of this series).

>  #ifndef __ASSEMBLY__
>  /*
> @@ -41,7 +42,8 @@
>  				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
>  				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
>  				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
> -				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
> +				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
> +				 COMPAT_HWCAP_EVTSTRM)

So here you're hardcoding COMPAT_HWCAP_EVTSTRM in the COMPAT_ELF_HWCAP, yet
we only enable the native one from the arch-timer driver. The question then is
"Can we rely on the event-stream working for AArch64?". If so, we don't need
the native hwcap at all. If not, then we need to make the compat hwcap
dynamic.

Will

Re: [PATCH v3 4/6] ARM64: arch_timer: configure and enable event stream

[Sudeep KarkadaNagesha]

On 20/08/13 14:27, Will Deacon wrote:
> Hi Sudeep,
> 
> Couple of comments inline...
> 
> On Tue, Aug 13, 2013 at 06:29:42PM +0100, Sudeep KarkadaNagesha wrote:
>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>
>> This patch configures the event stream frequency and enables it.
>> It also adds the hwcaps as well as compat-specific definitions to
>> the user to detect this event stream feature.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Will Deacon <will.deacon@arm.com>
>> [sudeep: moving ARM/ARM64 changes into separate patches]
>> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>> ---
>>  arch/arm64/include/asm/arch_timer.h | 12 ++++++++++--
>>  arch/arm64/include/asm/hwcap.h      |  4 +++-
>>  arch/arm64/include/uapi/asm/hwcap.h |  1 +
>>  arch/arm64/kernel/setup.c           |  1 +
>>  4 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
>> index 69f5c8e..43f322a 100644
>> --- a/arch/arm64/include/asm/arch_timer.h
>> +++ b/arch/arm64/include/asm/arch_timer.h
>> @@ -106,14 +106,22 @@ static inline void arch_counter_set_user_access(int divider)
>>  	/* Disable user access to the timers and the physical counter. */
>>  	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
>>  			| ARCH_TIMER_USR_VT_ACCESS_EN
>> +			| ARCH_TIMER_EVT_TRIGGER_MASK
>>  			| ARCH_TIMER_USR_PCT_ACCESS_EN);
>>  
>> -	/* Enable user access to the virtual counter. */
>> -	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
>> +	/* Enable event stream and user access to the virtual counter */
>> +	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>> +			| ARCH_TIMER_VIRT_EVT_EN
>> +			| ARCH_TIMER_USR_VCT_ACCESS_EN;
>>  
>>  	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
>>  }
>>  
>> +static inline void arch_timer_set_hwcap_evtstrm(void)
>> +{
>> +	elf_hwcap |= HWCAP_EVTSTRM;
>> +}
>> +
>>  static inline u64 arch_counter_get_cntvct(void)
>>  {
>>  	u64 cval;
>> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
>> index 6d4482f..effd8f9 100644
>> --- a/arch/arm64/include/asm/hwcap.h
>> +++ b/arch/arm64/include/asm/hwcap.h
>> @@ -30,6 +30,7 @@
>>  #define COMPAT_HWCAP_IDIVA	(1 << 17)
>>  #define COMPAT_HWCAP_IDIVT	(1 << 18)
>>  #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
>> +#define COMPAT_HWCAP_EVTSTRM	(1 << 21)
> 
> Looking at the context, we should also add COMPAT_HWCAP_LPAE as a separate
> patch (not part of this series).
> 
Ok make sense, I can do that. IIUC even this needs at least under some
config option, assuming can't be dynamic ?

>>  #ifndef __ASSEMBLY__
>>  /*
>> @@ -41,7 +42,8 @@
>>  				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
>>  				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
>>  				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
>> -				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
>> +				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
>> +				 COMPAT_HWCAP_EVTSTRM)
> 
> So here you're hardcoding COMPAT_HWCAP_EVTSTRM in the COMPAT_ELF_HWCAP, yet
> we only enable the native one from the arch-timer driver. The question then is
> "Can we rely on the event-stream working for AArch64?". If so, we don't need
> the native hwcap at all. If not, then we need to make the compat hwcap
> dynamic.

How about something like this ? I am not sure if
arch/arm64/kernel/setup.c is apt place for compat_elf_hwcap though.

------>8--------

diff --git a/arch/arm64/include/asm/arch_timer.h
b/arch/arm64/include/asm/arch_timer.h
index 43f322a..a7d33a8 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -120,6 +120,9 @@ static inline void arch_counter_set_user_access(int
divider)
 static inline void arch_timer_set_hwcap_evtstrm(void)
 {
 	elf_hwcap |= HWCAP_EVTSTRM;
+#ifdef CONFIG_COMPAT
+	compat_elf_hwcap |= COMPAT_HWCAP_EVTSTRM;
+#endif
 }

 static inline u64 arch_counter_get_cntvct(void)
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index effd8f9..9633db0 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -32,19 +32,24 @@
 #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
 #define COMPAT_HWCAP_EVTSTRM	(1 << 21)

+#define COMPAT_ELF_HWCAP_DEFAULT	\
+				(COMPAT_HWCAP_HALF|COMPAT_HWCAP_THUMB|\
+				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
+				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
+				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
+				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
 #ifndef __ASSEMBLY__
 /*
  * This yields a mask that user programs can use to figure out what
  * instruction set this cpu supports.
  */
 #define ELF_HWCAP		(elf_hwcap)
-#define COMPAT_ELF_HWCAP	(COMPAT_HWCAP_HALF|COMPAT_HWCAP_THUMB|\
-				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
-				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
-				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
-				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
-				 COMPAT_HWCAP_EVTSTRM)
-
 extern unsigned int elf_hwcap;
+
+#ifdef CONFIG_COMPAT
+#define COMPAT_ELF_HWCAP	(compat_elf_hwcap)
+extern unsigned int compat_elf_hwcap;
+#endif
+
 #endif
 #endif
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 1111833..990fc98 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -60,6 +60,11 @@ EXPORT_SYMBOL(processor_id);
 unsigned int elf_hwcap __read_mostly;
 EXPORT_SYMBOL_GPL(elf_hwcap);

+#ifdef CONFIG_COMPAT
+unsigned int compat_elf_hwcap __read_mostly = COMPAT_ELF_HWCAP_DEFAULT;
+EXPORT_SYMBOL_GPL(compat_elf_hwcap);
+#endif
+
 static const char *cpu_name;
 static const char *machine_name;
 phys_addr_t __fdt_pointer __initdata;

Re: [PATCH v3 4/6] ARM64: arch_timer: configure and enable event stream

[Will Deacon]

On Tue, Aug 20, 2013 at 03:27:33PM +0100, Sudeep KarkadaNagesha wrote:
> On 20/08/13 14:27, Will Deacon wrote:
> > On Tue, Aug 13, 2013 at 06:29:42PM +0100, Sudeep KarkadaNagesha wrote:
> >> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> >> index 6d4482f..effd8f9 100644
> >> --- a/arch/arm64/include/asm/hwcap.h
> >> +++ b/arch/arm64/include/asm/hwcap.h
> >> @@ -30,6 +30,7 @@
> >>  #define COMPAT_HWCAP_IDIVA	(1 << 17)
> >>  #define COMPAT_HWCAP_IDIVT	(1 << 18)
> >>  #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
> >> +#define COMPAT_HWCAP_EVTSTRM	(1 << 21)
> > 
> > Looking at the context, we should also add COMPAT_HWCAP_LPAE as a separate
> > patch (not part of this series).
> > 
> Ok make sense, I can do that. IIUC even this needs at least under some
> config option, assuming can't be dynamic ?

I'm not sure I follow you... if you're asking whether the LPAE HWCAP should
be dynamic for ARMv8, then no, since the capabilities offered by LPAE are
included in ARMv8 as far as userspace is concerned.

> >>  #ifndef __ASSEMBLY__
> >>  /*
> >> @@ -41,7 +42,8 @@
> >>  				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
> >>  				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
> >>  				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
> >> -				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
> >> +				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
> >> +				 COMPAT_HWCAP_EVTSTRM)
> > 
> > So here you're hardcoding COMPAT_HWCAP_EVTSTRM in the COMPAT_ELF_HWCAP, yet
> > we only enable the native one from the arch-timer driver. The question then is
> > "Can we rely on the event-stream working for AArch64?". If so, we don't need
> > the native hwcap at all. If not, then we need to make the compat hwcap
> > dynamic.
> 
> How about something like this ? I am not sure if
> arch/arm64/kernel/setup.c is apt place for compat_elf_hwcap though.

Looks like the right sort of idea, yes. I can't think of a better place to
put it. You could adjust it slightly so that COMPAT_ELF_HWCAP stays like it
is, with an extra '| compat_dyn_elf_hwcap' on the end, but I have no real
preference.

Will

Re: [PATCH v3 4/6] ARM64: arch_timer: configure and enable event stream

[Sudeep KarkadaNagesha]

On 20/08/13 17:16, Will Deacon wrote:
> On Tue, Aug 20, 2013 at 03:27:33PM +0100, Sudeep KarkadaNagesha wrote:
>> On 20/08/13 14:27, Will Deacon wrote:
>>> On Tue, Aug 13, 2013 at 06:29:42PM +0100, Sudeep KarkadaNagesha wrote:
>>>> diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
>>>> index 6d4482f..effd8f9 100644
>>>> --- a/arch/arm64/include/asm/hwcap.h
>>>> +++ b/arch/arm64/include/asm/hwcap.h
>>>> @@ -30,6 +30,7 @@
>>>>  #define COMPAT_HWCAP_IDIVA	(1 << 17)
>>>>  #define COMPAT_HWCAP_IDIVT	(1 << 18)
>>>>  #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
>>>> +#define COMPAT_HWCAP_EVTSTRM	(1 << 21)
>>>
>>> Looking at the context, we should also add COMPAT_HWCAP_LPAE as a separate
>>> patch (not part of this series).
>>>
>> Ok make sense, I can do that. IIUC even this needs at least under some
>> config option, assuming can't be dynamic ?
> 
> I'm not sure I follow you... if you're asking whether the LPAE HWCAP should
> be dynamic for ARMv8, then no, since the capabilities offered by LPAE are
> included in ARMv8 as far as userspace is concerned.
>
Ok, then it can be in the default list of compat hwcap.

>>>>  #ifndef __ASSEMBLY__
>>>>  /*
>>>> @@ -41,7 +42,8 @@
>>>>  				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
>>>>  				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
>>>>  				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
>>>> -				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
>>>> +				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
>>>> +				 COMPAT_HWCAP_EVTSTRM)
>>>
>>> So here you're hardcoding COMPAT_HWCAP_EVTSTRM in the COMPAT_ELF_HWCAP, yet
>>> we only enable the native one from the arch-timer driver. The question then is
>>> "Can we rely on the event-stream working for AArch64?". If so, we don't need
>>> the native hwcap at all. If not, then we need to make the compat hwcap
>>> dynamic.
>>
>> How about something like this ? I am not sure if
>> arch/arm64/kernel/setup.c is apt place for compat_elf_hwcap though.
> 
> Looks like the right sort of idea, yes. I can't think of a better place to
> put it. You could adjust it slightly so that COMPAT_ELF_HWCAP stays like it
> is, with an extra '| compat_dyn_elf_hwcap' on the end, but I have no real
> preference.
> 
Yes that's better, will do this way.

Regards,
Sudeep

[PATCH v3 4/6] ARM64: arch_timer: configure and enable event stream

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

This patch configures the event stream frequency and enables it.
It also adds the hwcaps as well as compat-specific definitions to
the user to detect this event stream feature.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
[sudeep: moving ARM/ARM64 changes into separate patches]
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 arch/arm64/include/asm/arch_timer.h | 15 +++++++++++++--
 arch/arm64/include/asm/hwcap.h      |  7 ++++++-
 arch/arm64/include/uapi/asm/hwcap.h |  1 +
 arch/arm64/kernel/setup.c           |  6 ++++++
 4 files changed, 26 insertions(+), 3 deletions(-)

Hi Will,

This is modified version enabling timer event stream hwcap compat dynamically.
Let me know if this looks fine.

Regards,
Sudeep

---->8---

diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 69f5c8e..90f9d1a 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -106,14 +106,25 @@ static inline void arch_counter_set_user_access(int divider)
 	/* Disable user access to the timers and the physical counter. */
 	cntkctl &= ~(ARCH_TIMER_USR_PT_ACCESS_EN
 			| ARCH_TIMER_USR_VT_ACCESS_EN
+			| ARCH_TIMER_EVT_TRIGGER_MASK
 			| ARCH_TIMER_USR_PCT_ACCESS_EN);
 
-	/* Enable user access to the virtual counter. */
-	cntkctl |= ARCH_TIMER_USR_VCT_ACCESS_EN;
+	/* Enable event stream and user access to the virtual counter */
+	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+			| ARCH_TIMER_VIRT_EVT_EN
+			| ARCH_TIMER_USR_VCT_ACCESS_EN;
 
 	asm volatile("msr	cntkctl_el1, %0" : : "r" (cntkctl));
 }
 
+static inline void arch_timer_set_hwcap_evtstrm(void)
+{
+	elf_hwcap |= HWCAP_EVTSTRM;
+#ifdef CONFIG_COMPAT
+	compat_dyn_elf_hwcap |= COMPAT_HWCAP_EVTSTRM;
+#endif
+}
+
 static inline u64 arch_counter_get_cntvct(void)
 {
 	u64 cval;
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index 6d4482f..022d771 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -30,6 +30,7 @@
 #define COMPAT_HWCAP_IDIVA	(1 << 17)
 #define COMPAT_HWCAP_IDIVT	(1 << 18)
 #define COMPAT_HWCAP_IDIV	(COMPAT_HWCAP_IDIVA|COMPAT_HWCAP_IDIVT)
+#define COMPAT_HWCAP_EVTSTRM	(1 << 21)
 
 #ifndef __ASSEMBLY__
 /*
@@ -37,11 +38,15 @@
  * instruction set this cpu supports.
  */
 #define ELF_HWCAP		(elf_hwcap)
+#ifdef CONFIG_COMPAT
 #define COMPAT_ELF_HWCAP	(COMPAT_HWCAP_HALF|COMPAT_HWCAP_THUMB|\
 				 COMPAT_HWCAP_FAST_MULT|COMPAT_HWCAP_EDSP|\
 				 COMPAT_HWCAP_TLS|COMPAT_HWCAP_VFP|\
 				 COMPAT_HWCAP_VFPv3|COMPAT_HWCAP_VFPv4|\
-				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV)
+				 COMPAT_HWCAP_NEON|COMPAT_HWCAP_IDIV|\
+				 compat_dyn_elf_hwcap)
+extern unsigned int compat_dyn_elf_hwcap;
+#endif
 
 extern unsigned int elf_hwcap;
 #endif
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index eea4975..9b12476 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -21,6 +21,7 @@
  */
 #define HWCAP_FP		(1 << 0)
 #define HWCAP_ASIMD		(1 << 1)
+#define HWCAP_EVTSTRM		(1 << 2)
 
 
 #endif /* _UAPI__ASM_HWCAP_H */
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index add6ea6..91af65a 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -60,6 +60,11 @@ EXPORT_SYMBOL(processor_id);
 unsigned int elf_hwcap __read_mostly;
 EXPORT_SYMBOL_GPL(elf_hwcap);
 
+#ifdef CONFIG_COMPAT
+unsigned int compat_dyn_elf_hwcap __read_mostly = 0;
+EXPORT_SYMBOL_GPL(compat_dyn_elf_hwcap);
+#endif
+
 static const char *cpu_name;
 static const char *machine_name;
 phys_addr_t __fdt_pointer __initdata;
@@ -309,6 +314,7 @@ subsys_initcall(topology_init);
 static const char *hwcap_str[] = {
 	"fp",
 	"asimd",
+	"evtstrm",
 	NULL
 };
 
-- 
1.8.1.2

Re: [PATCH v3 4/6] ARM64: arch_timer: configure and enable event stream

[Will Deacon]

Hi Sudeep,

On Tue, Aug 20, 2013 at 06:08:46PM +0100, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> This patch configures the event stream frequency and enables it.
> It also adds the hwcaps as well as compat-specific definitions to
> the user to detect this event stream feature.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> [sudeep: moving ARM/ARM64 changes into separate patches]
> Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> ---
>  arch/arm64/include/asm/arch_timer.h | 15 +++++++++++++--
>  arch/arm64/include/asm/hwcap.h      |  7 ++++++-
>  arch/arm64/include/uapi/asm/hwcap.h |  1 +
>  arch/arm64/kernel/setup.c           |  6 ++++++
>  4 files changed, 26 insertions(+), 3 deletions(-)
> 
> Hi Will,
> 
> This is modified version enabling timer event stream hwcap compat dynamically.
> Let me know if this looks fine.

Looks pretty good to me, one minor comment:

> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index add6ea6..91af65a 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -60,6 +60,11 @@ EXPORT_SYMBOL(processor_id);
>  unsigned int elf_hwcap __read_mostly;
>  EXPORT_SYMBOL_GPL(elf_hwcap);
>  
> +#ifdef CONFIG_COMPAT
> +unsigned int compat_dyn_elf_hwcap __read_mostly = 0;
> +EXPORT_SYMBOL_GPL(compat_dyn_elf_hwcap);
> +#endif

Given that the arch timer is the only user of this at the moment, and cannot
be built as a module, I don't think we should bother exporting the symbol
until it's actually required.

Will

[PATCH v3 5/6] drivers: clocksource: enable hwcaps for event stream on ARM arch timer

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

This patch enables hwcap definition to the users for event stream
feature on ARM architected timers.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 6301ee5..05d1e13 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -162,6 +162,9 @@ static int arch_timer_setup(struct clock_event_device *clk)
 		pos--;
 	arch_counter_set_user_access(min(pos, 15));
 
+	/* enable hwcap definition to the users for event stream feature */
+	arch_timer_set_hwcap_evtstrm();
+
 	return 0;
 }
 
-- 
1.8.1.2

[PATCH v3 6/6] drivers: clocksource: add CPU PM notifier for ARM architected timer

[Sudeep KarkadaNagesha]

From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>

Few control settings done in architected timer as part of initialisation
can be lost when CPU enters deeper power states. They need to be
re-initialised when the CPU is (warm)reset again.

This patch adds CPU PM notifiers to do the timer initialisation on warm
resets. It also save the event stream divider value calculated during
cold reset and uses the same in warm reset path.

Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 38 +++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 05d1e13..2ce9638 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -13,6 +13,7 @@
 #include <linux/device.h>
 #include <linux/smp.h>
 #include <linux/cpu.h>
+#include <linux/cpu_pm.h>
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
@@ -39,6 +40,8 @@ static struct clock_event_device __percpu *arch_timer_evt;
 
 static bool arch_timer_use_virtual = true;
 
+static int arch_timer_evtstream_div;
+
 /*
  * Architected system timer support.
  */
@@ -160,7 +163,9 @@ static int arch_timer_setup(struct clock_event_device *clk)
 	pos = fls(evt_stream_div);
 	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
 		pos--;
-	arch_counter_set_user_access(min(pos, 15));
+	/* save divider value for use in CPU PM notifier */
+	arch_timer_evtstream_div = min(pos, 15);
+	arch_counter_set_user_access(arch_timer_evtstream_div);
 
 	/* enable hwcap definition to the users for event stream feature */
 	arch_timer_set_hwcap_evtstrm();
@@ -270,6 +275,31 @@ static struct notifier_block arch_timer_cpu_nb = {
 	.notifier_call = arch_timer_cpu_notify,
 };
 
+#ifdef CONFIG_CPU_PM
+static int arch_timer_cpu_pm_notify(struct notifier_block *self,
+				    unsigned long action, void *hcpu)
+{
+	if (action == CPU_PM_EXIT)
+		arch_counter_set_user_access(arch_timer_evtstream_div);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block arch_timer_cpu_pm_notifier = {
+	.notifier_call = arch_timer_cpu_pm_notify,
+};
+
+static int __init arch_timer_cpu_pm_init(void)
+{
+	return cpu_pm_register_notifier(&arch_timer_cpu_pm_notifier);
+}
+#else
+static int __init arch_timer_cpu_pm_init(void)
+{
+	return 0;
+}
+#endif
+
 static int __init arch_timer_register(void)
 {
 	int err;
@@ -319,11 +349,17 @@ static int __init arch_timer_register(void)
 	if (err)
 		goto out_free_irq;
 
+	err = arch_timer_cpu_pm_init();
+	if (err)
+		goto out_unreg_notify;
+
 	/* Immediately configure the timer on the boot CPU */
 	arch_timer_setup(this_cpu_ptr(arch_timer_evt));
 
 	return 0;
 
+out_unreg_notify:
+	unregister_cpu_notifier(&arch_timer_cpu_nb);
 out_free_irq:
 	if (arch_timer_use_virtual)
 		free_percpu_irq(arch_timer_ppi[VIRT_PPI], arch_timer_evt);
-- 
1.8.1.2

Re: [PATCH v3 0/6] ARM/ARM64 architected timer updates

[Daniel Lezcano]

On 08/13/2013 07:29 PM, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> This patch series adds support to configure the rate and enable the
> event stream for architected timer. The event streams can be used to
> impose a timeout on a WFE, to safeguard against any programming error
> in case an expected event is not generated or even to implement
> wfe-based timeouts for userspace locking implementations.
> 
> Since the timer control register is reset to zero on warm boot, CPU
> PM notifier is added to re-initialize it.

It does not apply to my tree.

Against which tree is this patchset ? Who is supposed to take it ?

> Changes v2->v3:
> 1. Moved ARM and ARM64 changes into separate patches
> 2. Added native hwcaps definations(ARM/ARM64) and compat-specific
>    definitions(ARM64) to the users for the event stream feature. 

Ok, we have the choice:
 1. split the patchset into arch/arm changes and drivers/clocksource
 2. I ack the patchset and Olof/Kevin take it
 3. Olof/Kevin ack the patchset and I take it in my tree.

This is really becoming fuzzy.

If you want a maintainer to take a patchset you have to send an email
--to him and --cc mailing list and others concerned by the patchset.


> Changes v1->v2:
> 1. Saved event stream divider value on cold reset path and re-used it
>    in CPU PM notifier instead of calculating every time.
> 2. Rebased on v3.11-rc2(to avoid conflicts with __cpuinit* deletion)
> 3. Dropped "ARM/ARM64: arch_timer: remove __cpuinit attribute for
>    arch_counter_set_user_access"(already done as part of __cpuinit*
>    deletion)
> 
> Regards,
> Sudeep
> 
> 
> Sudeep KarkadaNagesha (5):
>   ARM/ARM64: arch_timer: add macros for bits in control register
>   ARM: arch_timer: configure and enable event stream
>   ARM64: arch_timer: configure and enable event stream
>   drivers: clocksource: enable hwcaps for event stream on ARM arch timer
>   drivers: clocksource: add CPU PM notifier for ARM architected timer
> 
> Will Deacon (1):
>   drivers: clocksource: configure event stream for ARM arch timer
> 
>  arch/arm/include/asm/arch_timer.h    | 18 +++++++++++---
>  arch/arm/include/uapi/asm/hwcap.h    |  1 +
>  arch/arm/kernel/setup.c              |  1 +
>  arch/arm64/include/asm/arch_timer.h  | 22 +++++++++++++----
>  arch/arm64/include/asm/hwcap.h       |  4 ++-
>  arch/arm64/include/uapi/asm/hwcap.h  |  1 +
>  arch/arm64/kernel/setup.c            |  1 +
>  drivers/clocksource/arm_arch_timer.c | 48 +++++++++++++++++++++++++++++++++++-
>  include/clocksource/arm_arch_timer.h | 10 ++++++++
>  9 files changed, 96 insertions(+), 10 deletions(-)
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 0/6] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

On 14/08/13 10:08, Daniel Lezcano wrote:
> On 08/13/2013 07:29 PM, Sudeep KarkadaNagesha wrote:
>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>
>> This patch series adds support to configure the rate and enable the
>> event stream for architected timer. The event streams can be used to
>> impose a timeout on a WFE, to safeguard against any programming error
>> in case an expected event is not generated or even to implement
>> wfe-based timeouts for userspace locking implementations.
>>
>> Since the timer control register is reset to zero on warm boot, CPU
>> PM notifier is added to re-initialize it.
> 
> It does not apply to my tree.
> 
> Against which tree is this patchset ? Who is supposed to take it ?
> 
As I had mentioned in previous version, because of the conflict, I had
to rebase it on 3.11-rc4

>> Changes v2->v3:
>> 1. Moved ARM and ARM64 changes into separate patches
>> 2. Added native hwcaps definations(ARM/ARM64) and compat-specific
>>    definitions(ARM64) to the users for the event stream feature. 
> 
> Ok, we have the choice:
>  1. split the patchset into arch/arm changes and drivers/clocksource
>  2. I ack the patchset and Olof/Kevin take it
>  3. Olof/Kevin ack the patchset and I take it in my tree.
> 
> This is really becoming fuzzy.
>
I am not sure if we need to split it. It would get too messy because of
dependencies. Once we have ACKs from all the corresponding maintainers,
it can go through one of the tree.

> If you want a maintainer to take a patchset you have to send an email
> --to him and --cc mailing list and others concerned by the patchset.
> 
Ok understood.

Regards,
Sudeep

Re: [PATCH v3 0/6] ARM/ARM64 architected timer updates

[Daniel Lezcano]

On 08/14/2013 11:17 AM, Sudeep KarkadaNagesha wrote:
> On 14/08/13 10:08, Daniel Lezcano wrote:
>> On 08/13/2013 07:29 PM, Sudeep KarkadaNagesha wrote:
>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>
>>> This patch series adds support to configure the rate and enable the
>>> event stream for architected timer. The event streams can be used to
>>> impose a timeout on a WFE, to safeguard against any programming error
>>> in case an expected event is not generated or even to implement
>>> wfe-based timeouts for userspace locking implementations.
>>>
>>> Since the timer control register is reset to zero on warm boot, CPU
>>> PM notifier is added to re-initialize it.
>>
>> It does not apply to my tree.
>>
>> Against which tree is this patchset ? Who is supposed to take it ?
>>
> As I had mentioned in previous version, because of the conflict, I had
> to rebase it on 3.11-rc4


dlezcano@mai:~/Work/src/clockevents$ git show
commit c095ba7224d8edc71dcef0d655911399a8bd4a3f
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sun Aug 4 13:46:46 2013 -0700

    Linux 3.11-rc4

diff --git a/Makefile b/Makefile
index 95a8e55..f93d4f7 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 VERSION = 3
 PATCHLEVEL = 11
 SUBLEVEL = 0
-EXTRAVERSION = -rc3
+EXTRAVERSION = -rc4
 NAME = Linux for Workgroups

 # *DOCUMENTATION*


dlezcano@mai:~/Work/src/clockevents$ git am -s arch_timer

Applying: ARM/ARM64: arch_timer: add macros for bits in control register
Applying: drivers: clocksource: configure event stream for ARM arch timer
Applying: ARM: arch_timer: configure and enable event stream
Applying: ARM64: arch_timer: configure and enable event stream
Applying: drivers: clocksource: add CPU PM notifier for ARM architected
timer
error: patch failed: drivers/clocksource/arm_arch_timer.c:160
error: drivers/clocksource/arm_arch_timer.c: patch does not apply
Patch failed at 0005 drivers: clocksource: add CPU PM notifier for ARM
architected timer
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".


>>> Changes v2->v3:
>>> 1. Moved ARM and ARM64 changes into separate patches
>>> 2. Added native hwcaps definations(ARM/ARM64) and compat-specific
>>>    definitions(ARM64) to the users for the event stream feature. 
>>
>> Ok, we have the choice:
>>  1. split the patchset into arch/arm changes and drivers/clocksource
>>  2. I ack the patchset and Olof/Kevin take it
>>  3. Olof/Kevin ack the patchset and I take it in my tree.
>>
>> This is really becoming fuzzy.
>>
> I am not sure if we need to split it. It would get too messy because of
> dependencies. Once we have ACKs from all the corresponding maintainers,
> it can go through one of the tree.
> 
>> If you want a maintainer to take a patchset you have to send an email
>> --to him and --cc mailing list and others concerned by the patchset.
>>
> Ok understood.
> 
> Regards,
> Sudeep
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 0/6] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

On 14/08/13 11:51, Daniel Lezcano wrote:
> On 08/14/2013 11:17 AM, Sudeep KarkadaNagesha wrote:
>> On 14/08/13 10:08, Daniel Lezcano wrote:
>>> On 08/13/2013 07:29 PM, Sudeep KarkadaNagesha wrote:
>>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>
>>>> This patch series adds support to configure the rate and enable the
>>>> event stream for architected timer. The event streams can be used to
>>>> impose a timeout on a WFE, to safeguard against any programming error
>>>> in case an expected event is not generated or even to implement
>>>> wfe-based timeouts for userspace locking implementations.
>>>>
>>>> Since the timer control register is reset to zero on warm boot, CPU
>>>> PM notifier is added to re-initialize it.
>>>
>>> It does not apply to my tree.
>>>
>>> Against which tree is this patchset ? Who is supposed to take it ?
>>>
>> As I had mentioned in previous version, because of the conflict, I had
>> to rebase it on 3.11-rc4
> 
> 
> dlezcano@mai:~/Work/src/clockevents$ git show
> commit c095ba7224d8edc71dcef0d655911399a8bd4a3f
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date:   Sun Aug 4 13:46:46 2013 -0700
> 
>     Linux 3.11-rc4
> 
> diff --git a/Makefile b/Makefile
> index 95a8e55..f93d4f7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,7 +1,7 @@
>  VERSION = 3
>  PATCHLEVEL = 11
>  SUBLEVEL = 0
> -EXTRAVERSION = -rc3
> +EXTRAVERSION = -rc4
>  NAME = Linux for Workgroups
> 
>  # *DOCUMENTATION*
> 
> 
> dlezcano@mai:~/Work/src/clockevents$ git am -s arch_timer
> 
> Applying: ARM/ARM64: arch_timer: add macros for bits in control register
> Applying: drivers: clocksource: configure event stream for ARM arch timer
> Applying: ARM: arch_timer: configure and enable event stream
> Applying: ARM64: arch_timer: configure and enable event stream
Looks like you are missing a patch here:
[PATCH v3 5/6] drivers: clocksource: enable hwcaps for event stream on
ARM arch timer

> Applying: drivers: clocksource: add CPU PM notifier for ARM architected
> timer

Regards,
Sudeep

Re: [PATCH v3 0/6] ARM/ARM64 architected timer updates

[Daniel Lezcano]

On 08/14/2013 01:03 PM, Sudeep KarkadaNagesha wrote:
> On 14/08/13 11:51, Daniel Lezcano wrote:
>> On 08/14/2013 11:17 AM, Sudeep KarkadaNagesha wrote:
>>> On 14/08/13 10:08, Daniel Lezcano wrote:
>>>> On 08/13/2013 07:29 PM, Sudeep KarkadaNagesha wrote:
>>>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>>>
>>>>> This patch series adds support to configure the rate and enable the
>>>>> event stream for architected timer. The event streams can be used to
>>>>> impose a timeout on a WFE, to safeguard against any programming error
>>>>> in case an expected event is not generated or even to implement
>>>>> wfe-based timeouts for userspace locking implementations.
>>>>>
>>>>> Since the timer control register is reset to zero on warm boot, CPU
>>>>> PM notifier is added to re-initialize it.
>>>>
>>>> It does not apply to my tree.
>>>>
>>>> Against which tree is this patchset ? Who is supposed to take it ?
>>>>
>>> As I had mentioned in previous version, because of the conflict, I had
>>> to rebase it on 3.11-rc4
>>
>>
>> dlezcano@mai:~/Work/src/clockevents$ git show
>> commit c095ba7224d8edc71dcef0d655911399a8bd4a3f
>> Author: Linus Torvalds <torvalds@linux-foundation.org>
>> Date:   Sun Aug 4 13:46:46 2013 -0700
>>
>>     Linux 3.11-rc4
>>
>> diff --git a/Makefile b/Makefile
>> index 95a8e55..f93d4f7 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1,7 +1,7 @@
>>  VERSION = 3
>>  PATCHLEVEL = 11
>>  SUBLEVEL = 0
>> -EXTRAVERSION = -rc3
>> +EXTRAVERSION = -rc4
>>  NAME = Linux for Workgroups
>>
>>  # *DOCUMENTATION*
>>
>>
>> dlezcano@mai:~/Work/src/clockevents$ git am -s arch_timer
>>
>> Applying: ARM/ARM64: arch_timer: add macros for bits in control register
>> Applying: drivers: clocksource: configure event stream for ARM arch timer
>> Applying: ARM: arch_timer: configure and enable event stream
>> Applying: ARM64: arch_timer: configure and enable event stream
> Looks like you are missing a patch here:
> [PATCH v3 5/6] drivers: clocksource: enable hwcaps for event stream on
> ARM arch timer

ah, right. Fair enough.

>> Applying: drivers: clocksource: add CPU PM notifier for ARM architected
>> timer


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v3 0/6] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

On 13/08/13 18:29, Sudeep KarkadaNagesha wrote:
> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> 
> This patch series adds support to configure the rate and enable the
> event stream for architected timer. The event streams can be used to
> impose a timeout on a WFE, to safeguard against any programming error
> in case an expected event is not generated or even to implement
> wfe-based timeouts for userspace locking implementations.
> 
> Since the timer control register is reset to zero on warm boot, CPU
> PM notifier is added to re-initialize it.
> 
> Changes v2->v3:
> 1. Moved ARM and ARM64 changes into separate patches
> 2. Added native hwcaps definations(ARM/ARM64) and compat-specific
>    definitions(ARM64) to the users for the event stream feature. 
> 
Hi Catalin,

Can you review this version of the series ?

Regards,
Sudeep

Re: [PATCH v3 0/6] ARM/ARM64 architected timer updates

[Catalin Marinas]

On Tue, Aug 20, 2013 at 06:14:52PM +0100, Sudeep KarkadaNagesha wrote:
> On 13/08/13 18:29, Sudeep KarkadaNagesha wrote:
> > From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
> > 
> > This patch series adds support to configure the rate and enable the
> > event stream for architected timer. The event streams can be used to
> > impose a timeout on a WFE, to safeguard against any programming error
> > in case an expected event is not generated or even to implement
> > wfe-based timeouts for userspace locking implementations.
> > 
> > Since the timer control register is reset to zero on warm boot, CPU
> > PM notifier is added to re-initialize it.
> > 
> > Changes v2->v3:
> > 1. Moved ARM and ARM64 changes into separate patches
> > 2. Added native hwcaps definations(ARM/ARM64) and compat-specific
> >    definitions(ARM64) to the users for the event stream feature. 
> 
> Can you review this version of the series ?

I think the series is OK with the follow-up comments addressed. Could
you please post a v4 to make sure I haven't missed anything?

Another comment I have is whether we should make this feature
configurable. The reason is mainly hardware validation: if some CPU
implementation messes the event generation (and on ARMv8 it's a bit more
complex as this is tied to the exclusive monitor) we risk not detecting
it because of the event stream.

Thanks.

-- 
Catalin

Re: [PATCH v3 0/6] ARM/ARM64 architected timer updates

[Sudeep KarkadaNagesha]

On 23/08/13 10:26, Catalin Marinas wrote:
> On Tue, Aug 20, 2013 at 06:14:52PM +0100, Sudeep KarkadaNagesha wrote:
>> On 13/08/13 18:29, Sudeep KarkadaNagesha wrote:
>>> From: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
>>>
>>> This patch series adds support to configure the rate and enable the
>>> event stream for architected timer. The event streams can be used to
>>> impose a timeout on a WFE, to safeguard against any programming error
>>> in case an expected event is not generated or even to implement
>>> wfe-based timeouts for userspace locking implementations.
>>>
>>> Since the timer control register is reset to zero on warm boot, CPU
>>> PM notifier is added to re-initialize it.
>>>
>>> Changes v2->v3:
>>> 1. Moved ARM and ARM64 changes into separate patches
>>> 2. Added native hwcaps definations(ARM/ARM64) and compat-specific
>>>    definitions(ARM64) to the users for the event stream feature. 
>>
>> Can you review this version of the series ?
> 
> I think the series is OK with the follow-up comments addressed. Could
> you please post a v4 to make sure I haven't missed anything?
> 
> Another comment I have is whether we should make this feature
> configurable. The reason is mainly hardware validation: if some CPU
> implementation messes the event generation (and on ARMv8 it's a bit more
> complex as this is tied to the exclusive monitor) we risk not detecting
> it because of the event stream.

Yes that seems reasonable. I assume we can enable it by default and we
need to disable it for hardware validation. Let me know if think otherwise.

Regards,
Sudeep