The current arch_timer only support accessing through CP15 interface.
Add support for ARM processors that only support IO mapped register
interface
Signed-off-by: Rohit Vaswani <[email protected]>
---
.../devicetree/bindings/arm/arch_timer.txt | 7 +
arch/arm/kernel/arch_timer.c | 259 ++++++++++++++++----
2 files changed, 223 insertions(+), 43 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index 52478c8..1c71799 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -14,6 +14,13 @@ The timer is attached to a GIC to deliver its per-processor interrupts.
- clock-frequency : The frequency of the main counter, in Hz. Optional.
+- irq-is-not-percpu: Specify is the timer irq is *NOT* a percpu (PPI) interrupt
+ In the default case i.e without this property, the timer irq is treated as a
+ PPI interrupt. Optional.
+
+- If the node address and reg is specified, the arch_timer will try to use the memory
+ mapped timer. Optional.
+
Example:
timer {
diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
index 1d0d9df..09604b7 100644
--- a/arch/arm/kernel/arch_timer.c
+++ b/arch/arm/kernel/arch_timer.c
@@ -18,6 +18,7 @@
#include <linux/clockchips.h>
#include <linux/interrupt.h>
#include <linux/of_irq.h>
+#include <linux/of_address.h>
#include <linux/io.h>
#include <asm/cputype.h>
@@ -29,8 +30,17 @@
static unsigned long arch_timer_rate;
static int arch_timer_ppi;
static int arch_timer_ppi2;
+static int is_irq_percpu;
static struct clock_event_device __percpu **arch_timer_evt;
+static void __iomem *timer_base;
+
+struct arch_timer_operations {
+ void (*reg_write)(int, u32);
+ u32 (*reg_read)(int);
+ cycle_t (*get_cntpct)(void);
+ cycle_t (*get_cntvct)(void);
+};
/*
* Architected system timer support.
@@ -44,7 +54,29 @@ static struct clock_event_device __percpu **arch_timer_evt;
#define ARCH_TIMER_REG_FREQ 1
#define ARCH_TIMER_REG_TVAL 2
-static void arch_timer_reg_write(int reg, u32 val)
+/* Iomapped Register Offsets */
+#define ARCH_TIMER_CNTP_LOW_REG 0x000
+#define ARCH_TIMER_CNTP_HIGH_REG 0x004
+#define ARCH_TIMER_CNTV_LOW_REG 0x008
+#define ARCH_TIMER_CNTV_HIGH_REG 0x00C
+#define ARCH_TIMER_CTRL_REG 0x02C
+#define ARCH_TIMER_FREQ_REG 0x010
+#define ARCH_TIMER_CNTP_TVAL_REG 0x028
+#define ARCH_TIMER_CNTV_TVAL_REG 0x038
+
+static void timer_reg_write_mem(int reg, u32 val)
+{
+ switch (reg) {
+ case ARCH_TIMER_REG_CTRL:
+ __raw_writel(val, timer_base + ARCH_TIMER_CTRL_REG);
+ break;
+ case ARCH_TIMER_REG_TVAL:
+ __raw_writel(val, timer_base + ARCH_TIMER_CNTP_TVAL_REG);
+ break;
+ }
+}
+
+static void timer_reg_write_cp15(int reg, u32 val)
{
switch (reg) {
case ARCH_TIMER_REG_CTRL:
@@ -58,7 +90,28 @@ static void arch_timer_reg_write(int reg, u32 val)
isb();
}
-static u32 arch_timer_reg_read(int reg)
+static u32 timer_reg_read_mem(int reg)
+{
+ u32 val;
+
+ switch (reg) {
+ case ARCH_TIMER_REG_CTRL:
+ val = __raw_readl(timer_base + ARCH_TIMER_CTRL_REG);
+ break;
+ case ARCH_TIMER_REG_FREQ:
+ val = __raw_readl(timer_base + ARCH_TIMER_FREQ_REG);
+ break;
+ case ARCH_TIMER_REG_TVAL:
+ val = __raw_readl(timer_base + ARCH_TIMER_CNTP_TVAL_REG);
+ break;
+ default:
+ BUG();
+ }
+
+ return val;
+}
+
+static u32 timer_reg_read_cp15(int reg)
{
u32 val;
@@ -79,6 +132,103 @@ static u32 arch_timer_reg_read(int reg)
return val;
}
+static cycle_t arch_counter_get_cntpct_mem(void)
+{
+ u32 cvall, cvalh, thigh;
+
+ do {
+ cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
+ cvall = __raw_readl(timer_base + ARCH_TIMER_CNTP_LOW_REG);
+ thigh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
+ } while (cvalh != thigh);
+
+ return ((cycle_t) cvalh << 32) | cvall;
+}
+
+static cycle_t arch_counter_get_cntpct_cp15(void)
+{
+ u32 cvall, cvalh;
+
+ asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
+
+ return ((cycle_t) cvalh << 32) | cvall;
+}
+
+static cycle_t arch_counter_get_cntvct_mem(void)
+{
+ u32 cvall, cvalh, thigh;
+
+ do {
+ cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
+ cvall = __raw_readl(timer_base + ARCH_TIMER_CNTV_LOW_REG);
+ thigh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
+ } while (cvalh != thigh);
+
+ return ((cycle_t) cvalh << 32) | cvall;
+}
+
+static cycle_t arch_counter_get_cntvct_cp15(void)
+{
+ u32 cvall, cvalh;
+
+ asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
+
+ return ((cycle_t) cvalh << 32) | cvall;
+}
+
+static struct arch_timer_operations arch_timer_ops_cp15 = {
+ .reg_read = &timer_reg_read_cp15,
+ .reg_write = &timer_reg_write_cp15,
+ .get_cntpct = &arch_counter_get_cntpct_cp15,
+ .get_cntvct = &arch_counter_get_cntvct_cp15,
+};
+
+static struct arch_timer_operations arch_timer_ops_mem = {
+ .reg_read = &timer_reg_read_mem,
+ .reg_write = &timer_reg_write_mem,
+ .get_cntpct = &arch_counter_get_cntpct_mem,
+ .get_cntvct = &arch_counter_get_cntvct_mem,
+};
+
+static struct arch_timer_operations *arch_specific_timer = &arch_timer_ops_cp15;
+
+static inline void arch_timer_reg_write(int reg, u32 val)
+{
+ arch_specific_timer->reg_write(reg, val);
+}
+
+static inline u32 arch_timer_reg_read(int reg)
+{
+ return arch_specific_timer->reg_read(reg);
+}
+
+static inline cycle_t arch_counter_get_cntpct(void)
+{
+ return arch_specific_timer->get_cntpct();
+}
+
+static inline cycle_t arch_counter_get_cntvct(void)
+{
+ return arch_specific_timer->get_cntvct();
+}
+
+static u32 notrace arch_counter_get_cntvct32(void)
+{
+ cycle_t cntvct = arch_counter_get_cntpct();
+
+ /*
+ * The sched_clock infrastructure only knows about counters
+ * with at most 32bits. Forget about the upper 24 bits for the
+ * time being...
+ */
+ return (u32)(cntvct & (u32)~0);
+}
+
+static cycle_t arch_counter_read(struct clocksource *cs)
+{
+ return arch_counter_get_cntpct();
+}
+
static irqreturn_t arch_timer_handler(int irq, void *dev_id)
{
struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
@@ -167,7 +317,9 @@ static int arch_timer_available(void)
{
unsigned long freq;
- if (!local_timer_is_architected())
+ if (timer_base)
+ arch_specific_timer = &arch_timer_ops_mem;
+ else if (!local_timer_is_architected())
return -ENXIO;
if (arch_timer_rate == 0) {
@@ -188,41 +340,6 @@ static int arch_timer_available(void)
return 0;
}
-static inline cycle_t arch_counter_get_cntpct(void)
-{
- u32 cvall, cvalh;
-
- asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
-
- return ((cycle_t) cvalh << 32) | cvall;
-}
-
-static inline cycle_t arch_counter_get_cntvct(void)
-{
- u32 cvall, cvalh;
-
- asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
-
- return ((cycle_t) cvalh << 32) | cvall;
-}
-
-static u32 notrace arch_counter_get_cntvct32(void)
-{
- cycle_t cntvct = arch_counter_get_cntvct();
-
- /*
- * The sched_clock infrastructure only knows about counters
- * with at most 32bits. Forget about the upper 24 bits for the
- * time being...
- */
- return (u32)(cntvct & (u32)~0);
-}
-
-static cycle_t arch_counter_read(struct clocksource *cs)
-{
- return arch_counter_get_cntpct();
-}
-
static struct clocksource clocksource_counter = {
.name = "arch_sys_counter",
.rating = 400,
@@ -262,8 +379,12 @@ static int __init arch_timer_register(void)
clocksource_register_hz(&clocksource_counter, arch_timer_rate);
- err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
+ if (is_irq_percpu)
+ err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
"arch_timer", arch_timer_evt);
+ else
+ err = request_irq(arch_timer_ppi, arch_timer_handler, 0,
+ "arch_timer", arch_timer_evt);
if (err) {
pr_err("arch_timer: can't register interrupt %d (%d)\n",
arch_timer_ppi, err);
@@ -271,8 +392,13 @@ static int __init arch_timer_register(void)
}
if (arch_timer_ppi2) {
- err = request_percpu_irq(arch_timer_ppi2, arch_timer_handler,
- "arch_timer", arch_timer_evt);
+ if (is_irq_percpu)
+ err = request_percpu_irq(arch_timer_ppi2,
+ arch_timer_handler, "arch_timer",
+ arch_timer_evt);
+ else
+ err = request_irq(arch_timer_ppi2, arch_timer_handler,
+ 0, "arch_timer", arch_timer_evt);
if (err) {
pr_err("arch_timer: can't register interrupt %d (%d)\n",
arch_timer_ppi2, err);
@@ -314,10 +440,40 @@ static const struct of_device_id arch_timer_of_match[] __initconst = {
{},
};
+static int __init arch_timer_base_init(void)
+{
+ struct device_node *np;
+
+ if (!timer_base) {
+ np = of_find_matching_node(NULL, arch_timer_of_match);
+ if (!np) {
+ pr_err("arch_timer: can't find DT node\n");
+ return -ENODEV;
+ }
+
+ if (of_get_address(np, 0, NULL, NULL)) {
+ timer_base = of_iomap(np, 0);
+ if (!timer_base) {
+ pr_err("arch_timer: cant map timer base\n");
+ return -ENOMEM;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static inline void __init arch_timer_base_free(void)
+{
+ if (timer_base)
+ iounmap(timer_base);
+}
+
int __init arch_timer_of_register(void)
{
struct device_node *np;
u32 freq;
+ int ret;
np = of_find_matching_node(NULL, arch_timer_of_match);
if (!np) {
@@ -331,20 +487,37 @@ int __init arch_timer_of_register(void)
arch_timer_ppi = irq_of_parse_and_map(np, 0);
arch_timer_ppi2 = irq_of_parse_and_map(np, 1);
+
+ ret = arch_timer_base_init();
+ if (ret)
+ return ret;
+
+ is_irq_percpu = !(of_property_read_bool(np, "irq-is-not-percpu"));
+
pr_info("arch_timer: found %s irqs %d %d\n",
np->name, arch_timer_ppi, arch_timer_ppi2);
- return arch_timer_register();
+ ret = arch_timer_register();
+ if (ret)
+ arch_timer_base_free();
+
+ return ret;
}
int __init arch_timer_sched_clock_init(void)
{
int err;
- err = arch_timer_available();
+ err = arch_timer_base_init();
if (err)
return err;
+ err = arch_timer_available();
+ if (err) {
+ arch_timer_base_free();
+ return err;
+ }
+
setup_sched_clock(arch_counter_get_cntvct32, 32, arch_timer_rate);
return 0;
}
--
Sent by an employee of the Qualcomm Innovation Center,Inc
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On 08/10/2012 04:58 PM, Rohit Vaswani wrote:
> The current arch_timer only support accessing through CP15 interface.
> Add support for ARM processors that only support IO mapped register
> interface
>
> Signed-off-by: Rohit Vaswani <[email protected]>
> ---
> .../devicetree/bindings/arm/arch_timer.txt | 7 +
> arch/arm/kernel/arch_timer.c | 259 ++++++++++++++++----
> 2 files changed, 223 insertions(+), 43 deletions(-)
The original file is 360 lines. It doesn't really seem like there's a
lot of overlap and I wonder if it is worth the extra overhead.
>
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 52478c8..1c71799 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -14,6 +14,13 @@ The timer is attached to a GIC to deliver its per-processor interrupts.
>
> - clock-frequency : The frequency of the main counter, in Hz. Optional.
>
> +- irq-is-not-percpu: Specify is the timer irq is *NOT* a percpu (PPI) interrupt
> + In the default case i.e without this property, the timer irq is treated as a
> + PPI interrupt. Optional.
The first field in the gic interrupts binding already defines this.
> +
> +- If the node address and reg is specified, the arch_timer will try to use the memory
> + mapped timer. Optional.
This timer is fundamentally different h/w. You need a new compatible string.
> +
> Example:
>
> timer {
> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> index 1d0d9df..09604b7 100644
> --- a/arch/arm/kernel/arch_timer.c
> +++ b/arch/arm/kernel/arch_timer.c
> @@ -18,6 +18,7 @@
> #include <linux/clockchips.h>
> #include <linux/interrupt.h>
> #include <linux/of_irq.h>
> +#include <linux/of_address.h>
> #include <linux/io.h>
>
> #include <asm/cputype.h>
> @@ -29,8 +30,17 @@
> static unsigned long arch_timer_rate;
> static int arch_timer_ppi;
> static int arch_timer_ppi2;
> +static int is_irq_percpu;
>
> static struct clock_event_device __percpu **arch_timer_evt;
> +static void __iomem *timer_base;
> +
> +struct arch_timer_operations {
> + void (*reg_write)(int, u32);
> + u32 (*reg_read)(int);
> + cycle_t (*get_cntpct)(void);
> + cycle_t (*get_cntvct)(void);
> +};
>
> /*
> * Architected system timer support.
> @@ -44,7 +54,29 @@ static struct clock_event_device __percpu **arch_timer_evt;
> #define ARCH_TIMER_REG_FREQ 1
> #define ARCH_TIMER_REG_TVAL 2
>
> -static void arch_timer_reg_write(int reg, u32 val)
> +/* Iomapped Register Offsets */
> +#define ARCH_TIMER_CNTP_LOW_REG 0x000
> +#define ARCH_TIMER_CNTP_HIGH_REG 0x004
> +#define ARCH_TIMER_CNTV_LOW_REG 0x008
> +#define ARCH_TIMER_CNTV_HIGH_REG 0x00C
> +#define ARCH_TIMER_CTRL_REG 0x02C
> +#define ARCH_TIMER_FREQ_REG 0x010
> +#define ARCH_TIMER_CNTP_TVAL_REG 0x028
> +#define ARCH_TIMER_CNTV_TVAL_REG 0x038
> +
> +static void timer_reg_write_mem(int reg, u32 val)
> +{
> + switch (reg) {
> + case ARCH_TIMER_REG_CTRL:
> + __raw_writel(val, timer_base + ARCH_TIMER_CTRL_REG);
> + break;
> + case ARCH_TIMER_REG_TVAL:
> + __raw_writel(val, timer_base + ARCH_TIMER_CNTP_TVAL_REG);
> + break;
This whole function seems a bit pointless as it only adds timer_base.
Rob
> + }
> +}
> +
> +static void timer_reg_write_cp15(int reg, u32 val)
> {
> switch (reg) {
> case ARCH_TIMER_REG_CTRL:
> @@ -58,7 +90,28 @@ static void arch_timer_reg_write(int reg, u32 val)
> isb();
> }
>
> -static u32 arch_timer_reg_read(int reg)
> +static u32 timer_reg_read_mem(int reg)
> +{
> + u32 val;
> +
> + switch (reg) {
> + case ARCH_TIMER_REG_CTRL:
> + val = __raw_readl(timer_base + ARCH_TIMER_CTRL_REG);
> + break;
> + case ARCH_TIMER_REG_FREQ:
> + val = __raw_readl(timer_base + ARCH_TIMER_FREQ_REG);
> + break;
> + case ARCH_TIMER_REG_TVAL:
> + val = __raw_readl(timer_base + ARCH_TIMER_CNTP_TVAL_REG);
> + break;
> + default:
> + BUG();
> + }
> +
> + return val;
> +}
> +
> +static u32 timer_reg_read_cp15(int reg)
> {
> u32 val;
>
> @@ -79,6 +132,103 @@ static u32 arch_timer_reg_read(int reg)
> return val;
> }
>
> +static cycle_t arch_counter_get_cntpct_mem(void)
> +{
> + u32 cvall, cvalh, thigh;
> +
> + do {
> + cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
> + cvall = __raw_readl(timer_base + ARCH_TIMER_CNTP_LOW_REG);
> + thigh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
> + } while (cvalh != thigh);
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static cycle_t arch_counter_get_cntpct_cp15(void)
> +{
> + u32 cvall, cvalh;
> +
> + asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static cycle_t arch_counter_get_cntvct_mem(void)
> +{
> + u32 cvall, cvalh, thigh;
> +
> + do {
> + cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
> + cvall = __raw_readl(timer_base + ARCH_TIMER_CNTV_LOW_REG);
> + thigh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
> + } while (cvalh != thigh);
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static cycle_t arch_counter_get_cntvct_cp15(void)
> +{
> + u32 cvall, cvalh;
> +
> + asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static struct arch_timer_operations arch_timer_ops_cp15 = {
> + .reg_read = &timer_reg_read_cp15,
> + .reg_write = &timer_reg_write_cp15,
> + .get_cntpct = &arch_counter_get_cntpct_cp15,
> + .get_cntvct = &arch_counter_get_cntvct_cp15,
> +};
> +
> +static struct arch_timer_operations arch_timer_ops_mem = {
> + .reg_read = &timer_reg_read_mem,
> + .reg_write = &timer_reg_write_mem,
> + .get_cntpct = &arch_counter_get_cntpct_mem,
> + .get_cntvct = &arch_counter_get_cntvct_mem,
> +};
> +
> +static struct arch_timer_operations *arch_specific_timer = &arch_timer_ops_cp15;
> +
> +static inline void arch_timer_reg_write(int reg, u32 val)
> +{
> + arch_specific_timer->reg_write(reg, val);
> +}
> +
> +static inline u32 arch_timer_reg_read(int reg)
> +{
> + return arch_specific_timer->reg_read(reg);
> +}
> +
> +static inline cycle_t arch_counter_get_cntpct(void)
> +{
> + return arch_specific_timer->get_cntpct();
> +}
> +
> +static inline cycle_t arch_counter_get_cntvct(void)
> +{
> + return arch_specific_timer->get_cntvct();
> +}
> +
> +static u32 notrace arch_counter_get_cntvct32(void)
> +{
> + cycle_t cntvct = arch_counter_get_cntpct();
> +
> + /*
> + * The sched_clock infrastructure only knows about counters
> + * with at most 32bits. Forget about the upper 24 bits for the
> + * time being...
> + */
> + return (u32)(cntvct & (u32)~0);
> +}
> +
> +static cycle_t arch_counter_read(struct clocksource *cs)
> +{
> + return arch_counter_get_cntpct();
> +}
> +
> static irqreturn_t arch_timer_handler(int irq, void *dev_id)
> {
> struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
> @@ -167,7 +317,9 @@ static int arch_timer_available(void)
> {
> unsigned long freq;
>
> - if (!local_timer_is_architected())
> + if (timer_base)
> + arch_specific_timer = &arch_timer_ops_mem;
> + else if (!local_timer_is_architected())
> return -ENXIO;
>
> if (arch_timer_rate == 0) {
> @@ -188,41 +340,6 @@ static int arch_timer_available(void)
> return 0;
> }
>
> -static inline cycle_t arch_counter_get_cntpct(void)
> -{
> - u32 cvall, cvalh;
> -
> - asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> -
> - return ((cycle_t) cvalh << 32) | cvall;
> -}
> -
> -static inline cycle_t arch_counter_get_cntvct(void)
> -{
> - u32 cvall, cvalh;
> -
> - asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> -
> - return ((cycle_t) cvalh << 32) | cvall;
> -}
> -
> -static u32 notrace arch_counter_get_cntvct32(void)
> -{
> - cycle_t cntvct = arch_counter_get_cntvct();
> -
> - /*
> - * The sched_clock infrastructure only knows about counters
> - * with at most 32bits. Forget about the upper 24 bits for the
> - * time being...
> - */
> - return (u32)(cntvct & (u32)~0);
> -}
> -
> -static cycle_t arch_counter_read(struct clocksource *cs)
> -{
> - return arch_counter_get_cntpct();
> -}
> -
> static struct clocksource clocksource_counter = {
> .name = "arch_sys_counter",
> .rating = 400,
> @@ -262,8 +379,12 @@ static int __init arch_timer_register(void)
>
> clocksource_register_hz(&clocksource_counter, arch_timer_rate);
>
> - err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
> + if (is_irq_percpu)
> + err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
> "arch_timer", arch_timer_evt);
> + else
> + err = request_irq(arch_timer_ppi, arch_timer_handler, 0,
> + "arch_timer", arch_timer_evt);
> if (err) {
> pr_err("arch_timer: can't register interrupt %d (%d)\n",
> arch_timer_ppi, err);
> @@ -271,8 +392,13 @@ static int __init arch_timer_register(void)
> }
>
> if (arch_timer_ppi2) {
> - err = request_percpu_irq(arch_timer_ppi2, arch_timer_handler,
> - "arch_timer", arch_timer_evt);
> + if (is_irq_percpu)
> + err = request_percpu_irq(arch_timer_ppi2,
> + arch_timer_handler, "arch_timer",
> + arch_timer_evt);
> + else
> + err = request_irq(arch_timer_ppi2, arch_timer_handler,
> + 0, "arch_timer", arch_timer_evt);
> if (err) {
> pr_err("arch_timer: can't register interrupt %d (%d)\n",
> arch_timer_ppi2, err);
> @@ -314,10 +440,40 @@ static const struct of_device_id arch_timer_of_match[] __initconst = {
> {},
> };
>
> +static int __init arch_timer_base_init(void)
> +{
> + struct device_node *np;
> +
> + if (!timer_base) {
> + np = of_find_matching_node(NULL, arch_timer_of_match);
> + if (!np) {
> + pr_err("arch_timer: can't find DT node\n");
> + return -ENODEV;
> + }
> +
> + if (of_get_address(np, 0, NULL, NULL)) {
> + timer_base = of_iomap(np, 0);
> + if (!timer_base) {
> + pr_err("arch_timer: cant map timer base\n");
> + return -ENOMEM;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +static inline void __init arch_timer_base_free(void)
> +{
> + if (timer_base)
> + iounmap(timer_base);
> +}
> +
> int __init arch_timer_of_register(void)
> {
> struct device_node *np;
> u32 freq;
> + int ret;
>
> np = of_find_matching_node(NULL, arch_timer_of_match);
> if (!np) {
> @@ -331,20 +487,37 @@ int __init arch_timer_of_register(void)
>
> arch_timer_ppi = irq_of_parse_and_map(np, 0);
> arch_timer_ppi2 = irq_of_parse_and_map(np, 1);
> +
> + ret = arch_timer_base_init();
> + if (ret)
> + return ret;
> +
> + is_irq_percpu = !(of_property_read_bool(np, "irq-is-not-percpu"));
> +
> pr_info("arch_timer: found %s irqs %d %d\n",
> np->name, arch_timer_ppi, arch_timer_ppi2);
>
> - return arch_timer_register();
> + ret = arch_timer_register();
> + if (ret)
> + arch_timer_base_free();
> +
> + return ret;
> }
>
> int __init arch_timer_sched_clock_init(void)
> {
> int err;
>
> - err = arch_timer_available();
> + err = arch_timer_base_init();
> if (err)
> return err;
>
> + err = arch_timer_available();
> + if (err) {
> + arch_timer_base_free();
> + return err;
> + }
> +
> setup_sched_clock(arch_counter_get_cntvct32, 32, arch_timer_rate);
> return 0;
> }
>
Thanks for your feedback Rob.
On 8/10/2012 3:10 PM, Rob Herring wrote:
> On 08/10/2012 04:58 PM, Rohit Vaswani wrote:
>> The current arch_timer only support accessing through CP15 interface.
>> Add support for ARM processors that only support IO mapped register
>> interface
>>
>> Signed-off-by: Rohit Vaswani <[email protected]>
>> ---
>> .../devicetree/bindings/arm/arch_timer.txt | 7 +
>> arch/arm/kernel/arch_timer.c | 259 ++++++++++++++++----
>> 2 files changed, 223 insertions(+), 43 deletions(-)
> The original file is 360 lines. It doesn't really seem like there's a
> lot of overlap and I wonder if it is worth the extra overhead.
>
>> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> index 52478c8..1c71799 100644
>> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
>> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> @@ -14,6 +14,13 @@ The timer is attached to a GIC to deliver its per-processor interrupts.
>>
>> - clock-frequency : The frequency of the main counter, in Hz. Optional.
>>
>> +- irq-is-not-percpu: Specify is the timer irq is *NOT* a percpu (PPI) interrupt
>> + In the default case i.e without this property, the timer irq is treated as a
>> + PPI interrupt. Optional.
> The first field in the gic interrupts binding already defines this.
Is there a generic way to extract that information from the interrupts
binding. I saw Chris Smith's patch that adds irq_is_per_cpu function.
Perhaps we can use that once it is merged ?
>
>> +
>> +- If the node address and reg is specified, the arch_timer will try to use the memory
>> + mapped timer. Optional.
> This timer is fundamentally different h/w. You need a new compatible string.
I think that the timer is the same, but it just has a different
interface. Do you still think we need a new compatible string ?
>
>> +
>> Example:
>>
>> timer {
>> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
>> index 1d0d9df..09604b7 100644
>> --- a/arch/arm/kernel/arch_timer.c
>> +++ b/arch/arm/kernel/arch_timer.c
>> @@ -18,6 +18,7 @@
>> #include <linux/clockchips.h>
>> #include <linux/interrupt.h>
>> #include <linux/of_irq.h>
>> +#include <linux/of_address.h>
>> #include <linux/io.h>
>>
>> #include <asm/cputype.h>
>> @@ -29,8 +30,17 @@
>> static unsigned long arch_timer_rate;
>> static int arch_timer_ppi;
>> static int arch_timer_ppi2;
>> +static int is_irq_percpu;
>>
>> static struct clock_event_device __percpu **arch_timer_evt;
>> +static void __iomem *timer_base;
>> +
>> +struct arch_timer_operations {
>> + void (*reg_write)(int, u32);
>> + u32 (*reg_read)(int);
>> + cycle_t (*get_cntpct)(void);
>> + cycle_t (*get_cntvct)(void);
>> +};
>>
>> /*
>> * Architected system timer support.
>> @@ -44,7 +54,29 @@ static struct clock_event_device __percpu **arch_timer_evt;
>> #define ARCH_TIMER_REG_FREQ 1
>> #define ARCH_TIMER_REG_TVAL 2
>>
>> -static void arch_timer_reg_write(int reg, u32 val)
>> +/* Iomapped Register Offsets */
>> +#define ARCH_TIMER_CNTP_LOW_REG 0x000
>> +#define ARCH_TIMER_CNTP_HIGH_REG 0x004
>> +#define ARCH_TIMER_CNTV_LOW_REG 0x008
>> +#define ARCH_TIMER_CNTV_HIGH_REG 0x00C
>> +#define ARCH_TIMER_CTRL_REG 0x02C
>> +#define ARCH_TIMER_FREQ_REG 0x010
>> +#define ARCH_TIMER_CNTP_TVAL_REG 0x028
>> +#define ARCH_TIMER_CNTV_TVAL_REG 0x038
>> +
>> +static void timer_reg_write_mem(int reg, u32 val)
>> +{
>> + switch (reg) {
>> + case ARCH_TIMER_REG_CTRL:
>> + __raw_writel(val, timer_base + ARCH_TIMER_CTRL_REG);
>> + break;
>> + case ARCH_TIMER_REG_TVAL:
>> + __raw_writel(val, timer_base + ARCH_TIMER_CNTP_TVAL_REG);
>> + break;
> This whole function seems a bit pointless as it only adds timer_base.
>
> Rob
I tried to the keep the functions similar to the cp15 interface ones. Is
there something else you suggest doing ?
Thanks,
Rohit Vaswani
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On 8/10/2012 5:58 PM, Rohit Vaswani wrote:
> The current arch_timer only support accessing through CP15 interface.
> Add support for ARM processors that only support IO mapped register
> interface
>
It looks like this patch attempts to address both (a) non-percpu arch
timers, and (b) memory mapped arch timers in one go. These should
probably be broken out into two distinct logical changes.
More below...
> Signed-off-by: Rohit Vaswani <[email protected]>
> ---
> .../devicetree/bindings/arm/arch_timer.txt | 7 +
> arch/arm/kernel/arch_timer.c | 259 ++++++++++++++++----
> 2 files changed, 223 insertions(+), 43 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 52478c8..1c71799 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -14,6 +14,13 @@ The timer is attached to a GIC to deliver its per-processor interrupts.
>
> - clock-frequency : The frequency of the main counter, in Hz. Optional.
>
> +- irq-is-not-percpu: Specify is the timer irq is *NOT* a percpu (PPI) interrupt
> + In the default case i.e without this property, the timer irq is treated as a
> + PPI interrupt. Optional.
> +
The handling of non-percpu IRQs looks broken. The code does
(enable/disable)_percpu_irq() on IRQs that may no longer be percpu.
> +- If the node address and reg is specified, the arch_timer will try to use the memory
> + mapped timer. Optional.
> +
> Example:
>
> timer {
> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> index 1d0d9df..09604b7 100644
> --- a/arch/arm/kernel/arch_timer.c
> +++ b/arch/arm/kernel/arch_timer.c
> @@ -18,6 +18,7 @@
> #include <linux/clockchips.h>
> #include <linux/interrupt.h>
> #include <linux/of_irq.h>
> +#include <linux/of_address.h>
> #include <linux/io.h>
>
> #include <asm/cputype.h>
> @@ -29,8 +30,17 @@
> static unsigned long arch_timer_rate;
> static int arch_timer_ppi;
> static int arch_timer_ppi2;
> +static int is_irq_percpu;
>
> static struct clock_event_device __percpu **arch_timer_evt;
> +static void __iomem *timer_base;
> +
Are percpu memory mapped arch timers an impossibility?
> +struct arch_timer_operations {
> + void (*reg_write)(int, u32);
> + u32 (*reg_read)(int);
> + cycle_t (*get_cntpct)(void);
> + cycle_t (*get_cntvct)(void);
> +};
>
> /*
> * Architected system timer support.
> @@ -44,7 +54,29 @@ static struct clock_event_device __percpu **arch_timer_evt;
> #define ARCH_TIMER_REG_FREQ 1
> #define ARCH_TIMER_REG_TVAL 2
>
> -static void arch_timer_reg_write(int reg, u32 val)
> +/* Iomapped Register Offsets */
> +#define ARCH_TIMER_CNTP_LOW_REG 0x000
> +#define ARCH_TIMER_CNTP_HIGH_REG 0x004
> +#define ARCH_TIMER_CNTV_LOW_REG 0x008
> +#define ARCH_TIMER_CNTV_HIGH_REG 0x00C
> +#define ARCH_TIMER_CTRL_REG 0x02C
> +#define ARCH_TIMER_FREQ_REG 0x010
> +#define ARCH_TIMER_CNTP_TVAL_REG 0x028
> +#define ARCH_TIMER_CNTV_TVAL_REG 0x038
> +
ARCH_TIMER_CNTV_TVAL_REG appears to be unused here.
> +static void timer_reg_write_mem(int reg, u32 val)
> +{
> + switch (reg) {
> + case ARCH_TIMER_REG_CTRL:
> + __raw_writel(val, timer_base + ARCH_TIMER_CTRL_REG);
> + break;
> + case ARCH_TIMER_REG_TVAL:
> + __raw_writel(val, timer_base + ARCH_TIMER_CNTP_TVAL_REG);
> + break;
> + }
> +}
> +
Wouldn't an array of offsets to map from ARCH_TIMER_REG_* to these
memory mapped registers eliminate the need to switch-case your way
through each register?
> +static void timer_reg_write_cp15(int reg, u32 val)
> {
> switch (reg) {
> case ARCH_TIMER_REG_CTRL:
> @@ -58,7 +90,28 @@ static void arch_timer_reg_write(int reg, u32 val)
> isb();
> }
>
> -static u32 arch_timer_reg_read(int reg)
> +static u32 timer_reg_read_mem(int reg)
> +{
> + u32 val;
> +
> + switch (reg) {
> + case ARCH_TIMER_REG_CTRL:
> + val = __raw_readl(timer_base + ARCH_TIMER_CTRL_REG);
> + break;
> + case ARCH_TIMER_REG_FREQ:
> + val = __raw_readl(timer_base + ARCH_TIMER_FREQ_REG);
> + break;
> + case ARCH_TIMER_REG_TVAL:
> + val = __raw_readl(timer_base + ARCH_TIMER_CNTP_TVAL_REG);
> + break;
> + default:
> + BUG();
> + }
> +
> + return val;
> +}
> +
Same as above.
> +static u32 timer_reg_read_cp15(int reg)
> {
> u32 val;
>
> @@ -79,6 +132,103 @@ static u32 arch_timer_reg_read(int reg)
> return val;
> }
>
> +static cycle_t arch_counter_get_cntpct_mem(void)
> +{
> + u32 cvall, cvalh, thigh;
> +
> + do {
> + cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
> + cvall = __raw_readl(timer_base + ARCH_TIMER_CNTP_LOW_REG);
> + thigh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
> + } while (cvalh != thigh);
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static cycle_t arch_counter_get_cntpct_cp15(void)
> +{
> + u32 cvall, cvalh;
> +
> + asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static cycle_t arch_counter_get_cntvct_mem(void)
> +{
> + u32 cvall, cvalh, thigh;
> +
> + do {
> + cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
> + cvall = __raw_readl(timer_base + ARCH_TIMER_CNTV_LOW_REG);
> + thigh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
> + } while (cvalh != thigh);
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
Repetitive - the logic is identical to arch_counter_get_cntpct_mem() above.
> +static cycle_t arch_counter_get_cntvct_cp15(void)
> +{
> + u32 cvall, cvalh;
> +
> + asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static struct arch_timer_operations arch_timer_ops_cp15 = {
> + .reg_read = &timer_reg_read_cp15,
> + .reg_write = &timer_reg_write_cp15,
> + .get_cntpct = &arch_counter_get_cntpct_cp15,
> + .get_cntvct = &arch_counter_get_cntvct_cp15,
> +};
> +
> +static struct arch_timer_operations arch_timer_ops_mem = {
> + .reg_read = &timer_reg_read_mem,
> + .reg_write = &timer_reg_write_mem,
> + .get_cntpct = &arch_counter_get_cntpct_mem,
> + .get_cntvct = &arch_counter_get_cntvct_mem,
> +};
> +
> +static struct arch_timer_operations *arch_specific_timer = &arch_timer_ops_cp15;
> +
> +static inline void arch_timer_reg_write(int reg, u32 val)
> +{
> + arch_specific_timer->reg_write(reg, val);
> +}
> +
> +static inline u32 arch_timer_reg_read(int reg)
> +{
> + return arch_specific_timer->reg_read(reg);
> +}
> +
> +static inline cycle_t arch_counter_get_cntpct(void)
> +{
> + return arch_specific_timer->get_cntpct();
> +}
> +
> +static inline cycle_t arch_counter_get_cntvct(void)
> +{
> + return arch_specific_timer->get_cntvct();
> +}
> +
The 4 pointer chasers above could lose some verbosity by being #defines.
> +static u32 notrace arch_counter_get_cntvct32(void)
> +{
> + cycle_t cntvct = arch_counter_get_cntpct();
Shouldn't that be arch_counter_get_cntvct() instead?
> +
> + /*
> + * The sched_clock infrastructure only knows about counters
> + * with at most 32bits. Forget about the upper 24 bits for the
> + * time being...
> + */
> + return (u32)(cntvct & (u32)~0);
> +}
> +
> +static cycle_t arch_counter_read(struct clocksource *cs)
> +{
> + return arch_counter_get_cntpct();
> +}
> +
Did the code really need to be moved around? Some of the functions in
this file appear to be entirely unchanged, but show up as both added and
removed lines.
> static irqreturn_t arch_timer_handler(int irq, void *dev_id)
> {
> struct clock_event_device *evt = *(struct clock_event_device **)dev_id;
> @@ -167,7 +317,9 @@ static int arch_timer_available(void)
> {
> unsigned long freq;
>
> - if (!local_timer_is_architected())
> + if (timer_base)
> + arch_specific_timer = &arch_timer_ops_mem;
> + else if (!local_timer_is_architected())
> return -ENXIO;
>
> if (arch_timer_rate == 0) {
> @@ -188,41 +340,6 @@ static int arch_timer_available(void)
> return 0;
> }
>
> -static inline cycle_t arch_counter_get_cntpct(void)
> -{
> - u32 cvall, cvalh;
> -
> - asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> -
> - return ((cycle_t) cvalh << 32) | cvall;
> -}
> -
> -static inline cycle_t arch_counter_get_cntvct(void)
> -{
> - u32 cvall, cvalh;
> -
> - asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> -
> - return ((cycle_t) cvalh << 32) | cvall;
> -}
> -
> -static u32 notrace arch_counter_get_cntvct32(void)
> -{
> - cycle_t cntvct = arch_counter_get_cntvct();
> -
> - /*
> - * The sched_clock infrastructure only knows about counters
> - * with at most 32bits. Forget about the upper 24 bits for the
> - * time being...
> - */
> - return (u32)(cntvct & (u32)~0);
> -}
> -
> -static cycle_t arch_counter_read(struct clocksource *cs)
> -{
> - return arch_counter_get_cntpct();
> -}
> -
> static struct clocksource clocksource_counter = {
> .name = "arch_sys_counter",
> .rating = 400,
> @@ -262,8 +379,12 @@ static int __init arch_timer_register(void)
>
> clocksource_register_hz(&clocksource_counter, arch_timer_rate);
>
> - err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
> + if (is_irq_percpu)
> + err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
> "arch_timer", arch_timer_evt);
> + else
> + err = request_irq(arch_timer_ppi, arch_timer_handler, 0,
> + "arch_timer", arch_timer_evt);
> if (err) {
> pr_err("arch_timer: can't register interrupt %d (%d)\n",
> arch_timer_ppi, err);
> @@ -271,8 +392,13 @@ static int __init arch_timer_register(void)
> }
>
> if (arch_timer_ppi2) {
> - err = request_percpu_irq(arch_timer_ppi2, arch_timer_handler,
> - "arch_timer", arch_timer_evt);
> + if (is_irq_percpu)
> + err = request_percpu_irq(arch_timer_ppi2,
> + arch_timer_handler, "arch_timer",
> + arch_timer_evt);
> + else
> + err = request_irq(arch_timer_ppi2, arch_timer_handler,
> + 0, "arch_timer", arch_timer_evt);
> if (err) {
> pr_err("arch_timer: can't register interrupt %d (%d)\n",
> arch_timer_ppi2, err);
> @@ -314,10 +440,40 @@ static const struct of_device_id arch_timer_of_match[] __initconst = {
> {},
> };
>
> +static int __init arch_timer_base_init(void)
> +{
> + struct device_node *np;
> +
> + if (!timer_base) {
> + np = of_find_matching_node(NULL, arch_timer_of_match);
> + if (!np) {
> + pr_err("arch_timer: can't find DT node\n");
> + return -ENODEV;
> + }
> +
> + if (of_get_address(np, 0, NULL, NULL)) {
> + timer_base = of_iomap(np, 0);
> + if (!timer_base) {
> + pr_err("arch_timer: cant map timer base\n");
> + return -ENOMEM;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +static inline void __init arch_timer_base_free(void)
> +{
> + if (timer_base)
> + iounmap(timer_base);
> +}
> +
> int __init arch_timer_of_register(void)
> {
> struct device_node *np;
> u32 freq;
> + int ret;
>
> np = of_find_matching_node(NULL, arch_timer_of_match);
> if (!np) {
> @@ -331,20 +487,37 @@ int __init arch_timer_of_register(void)
>
> arch_timer_ppi = irq_of_parse_and_map(np, 0);
> arch_timer_ppi2 = irq_of_parse_and_map(np, 1);
> +
> + ret = arch_timer_base_init();
> + if (ret)
> + return ret;
> +
> + is_irq_percpu = !(of_property_read_bool(np, "irq-is-not-percpu"));
> +
> pr_info("arch_timer: found %s irqs %d %d\n",
> np->name, arch_timer_ppi, arch_timer_ppi2);
>
> - return arch_timer_register();
> + ret = arch_timer_register();
> + if (ret)
> + arch_timer_base_free();
> +
> + return ret;
> }
>
> int __init arch_timer_sched_clock_init(void)
> {
> int err;
>
> - err = arch_timer_available();
> + err = arch_timer_base_init();
> if (err)
> return err;
>
> + err = arch_timer_available();
> + if (err) {
> + arch_timer_base_free();
> + return err;
> + }
> +
> setup_sched_clock(arch_counter_get_cntvct32, 32, arch_timer_rate);
> return 0;
> }
>
--
Cheers
- Cyril
Hi Rohit,
On Fri, 10 Aug 2012 14:58:41 -0700, Rohit Vaswani
<[email protected]>
wrote:
> The current arch_timer only support accessing through CP15 interface.
> Add support for ARM processors that only support IO mapped register
> interface
This is quite a departure from the current implementation, which raises a
couple of questions:
- What does CP15 ID_PFR1[19:16] report? Can we easily detect that we do
not have the CP15 interface?
- What about HYP mode? Is there any way to control the access of the
physical timer at PL1?
> Signed-off-by: Rohit Vaswani <[email protected]>
> ---
> .../devicetree/bindings/arm/arch_timer.txt | 7 +
> arch/arm/kernel/arch_timer.c | 259
> ++++++++++++++++----
> 2 files changed, 223 insertions(+), 43 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt
> b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 52478c8..1c71799 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -14,6 +14,13 @@ The timer is attached to a GIC to deliver its
> per-processor interrupts.
>
> - clock-frequency : The frequency of the main counter, in Hz. Optional.
>
> +- irq-is-not-percpu: Specify is the timer irq is *NOT* a percpu (PPI)
> interrupt
> + In the default case i.e without this property, the timer irq is
treated
> as a
> + PPI interrupt. Optional.
Ouch! How do you specify the various interrupts for all CPUs? Do you end
up with 4 SPIs per CPU?
> +- If the node address and reg is specified, the arch_timer will try to
> use the memory
> + mapped timer. Optional.
> +
> Example:
>
> timer {
> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
> index 1d0d9df..09604b7 100644
> --- a/arch/arm/kernel/arch_timer.c
> +++ b/arch/arm/kernel/arch_timer.c
> @@ -18,6 +18,7 @@
> #include <linux/clockchips.h>
> #include <linux/interrupt.h>
> #include <linux/of_irq.h>
> +#include <linux/of_address.h>
> #include <linux/io.h>
>
> #include <asm/cputype.h>
> @@ -29,8 +30,17 @@
> static unsigned long arch_timer_rate;
> static int arch_timer_ppi;
> static int arch_timer_ppi2;
> +static int is_irq_percpu;
>
> static struct clock_event_device __percpu **arch_timer_evt;
> +static void __iomem *timer_base;
> +
> +struct arch_timer_operations {
> + void (*reg_write)(int, u32);
> + u32 (*reg_read)(int);
> + cycle_t (*get_cntpct)(void);
> + cycle_t (*get_cntvct)(void);
> +};
I already have something similar in a patch series that I'd like to get
merged in 3.7, implementing support for the virtual timers (needed for
virtualisation). Have a look at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git
timers-next
> /*
> * Architected system timer support.
> @@ -44,7 +54,29 @@ static struct clock_event_device __percpu
> **arch_timer_evt;
> #define ARCH_TIMER_REG_FREQ 1
> #define ARCH_TIMER_REG_TVAL 2
>
> -static void arch_timer_reg_write(int reg, u32 val)
> +/* Iomapped Register Offsets */
> +#define ARCH_TIMER_CNTP_LOW_REG 0x000
> +#define ARCH_TIMER_CNTP_HIGH_REG 0x004
> +#define ARCH_TIMER_CNTV_LOW_REG 0x008
> +#define ARCH_TIMER_CNTV_HIGH_REG 0x00C
> +#define ARCH_TIMER_CTRL_REG 0x02C
> +#define ARCH_TIMER_FREQ_REG 0x010
> +#define ARCH_TIMER_CNTP_TVAL_REG 0x028
> +#define ARCH_TIMER_CNTV_TVAL_REG 0x038
> +
> +static void timer_reg_write_mem(int reg, u32 val)
> +{
> + switch (reg) {
> + case ARCH_TIMER_REG_CTRL:
> + __raw_writel(val, timer_base + ARCH_TIMER_CTRL_REG);
> + break;
> + case ARCH_TIMER_REG_TVAL:
> + __raw_writel(val, timer_base + ARCH_TIMER_CNTP_TVAL_REG);
> + break;
> + }
> +}
> +
> +static void timer_reg_write_cp15(int reg, u32 val)
> {
> switch (reg) {
> case ARCH_TIMER_REG_CTRL:
> @@ -58,7 +90,28 @@ static void arch_timer_reg_write(int reg, u32 val)
> isb();
> }
>
> -static u32 arch_timer_reg_read(int reg)
> +static u32 timer_reg_read_mem(int reg)
> +{
> + u32 val;
> +
> + switch (reg) {
> + case ARCH_TIMER_REG_CTRL:
> + val = __raw_readl(timer_base + ARCH_TIMER_CTRL_REG);
> + break;
> + case ARCH_TIMER_REG_FREQ:
> + val = __raw_readl(timer_base + ARCH_TIMER_FREQ_REG);
> + break;
> + case ARCH_TIMER_REG_TVAL:
> + val = __raw_readl(timer_base + ARCH_TIMER_CNTP_TVAL_REG);
> + break;
> + default:
> + BUG();
> + }
> +
> + return val;
> +}
> +
> +static u32 timer_reg_read_cp15(int reg)
> {
> u32 val;
>
> @@ -79,6 +132,103 @@ static u32 arch_timer_reg_read(int reg)
> return val;
> }
>
> +static cycle_t arch_counter_get_cntpct_mem(void)
> +{
> + u32 cvall, cvalh, thigh;
> +
> + do {
> + cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
> + cvall = __raw_readl(timer_base + ARCH_TIMER_CNTP_LOW_REG);
> + thigh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
> + } while (cvalh != thigh);
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static cycle_t arch_counter_get_cntpct_cp15(void)
> +{
> + u32 cvall, cvalh;
> +
> + asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static cycle_t arch_counter_get_cntvct_mem(void)
> +{
> + u32 cvall, cvalh, thigh;
> +
> + do {
> + cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
> + cvall = __raw_readl(timer_base + ARCH_TIMER_CNTV_LOW_REG);
> + thigh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
> + } while (cvalh != thigh);
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static cycle_t arch_counter_get_cntvct_cp15(void)
> +{
> + u32 cvall, cvalh;
> +
> + asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> +
> + return ((cycle_t) cvalh << 32) | cvall;
> +}
> +
> +static struct arch_timer_operations arch_timer_ops_cp15 = {
> + .reg_read = &timer_reg_read_cp15,
> + .reg_write = &timer_reg_write_cp15,
> + .get_cntpct = &arch_counter_get_cntpct_cp15,
> + .get_cntvct = &arch_counter_get_cntvct_cp15,
> +};
> +
> +static struct arch_timer_operations arch_timer_ops_mem = {
> + .reg_read = &timer_reg_read_mem,
> + .reg_write = &timer_reg_write_mem,
> + .get_cntpct = &arch_counter_get_cntpct_mem,
> + .get_cntvct = &arch_counter_get_cntvct_mem,
> +};
> +
> +static struct arch_timer_operations *arch_specific_timer =
> &arch_timer_ops_cp15;
> +
> +static inline void arch_timer_reg_write(int reg, u32 val)
> +{
> + arch_specific_timer->reg_write(reg, val);
> +}
> +
> +static inline u32 arch_timer_reg_read(int reg)
> +{
> + return arch_specific_timer->reg_read(reg);
> +}
> +
> +static inline cycle_t arch_counter_get_cntpct(void)
> +{
> + return arch_specific_timer->get_cntpct();
> +}
> +
> +static inline cycle_t arch_counter_get_cntvct(void)
> +{
> + return arch_specific_timer->get_cntvct();
> +}
> +
> +static u32 notrace arch_counter_get_cntvct32(void)
> +{
> + cycle_t cntvct = arch_counter_get_cntpct();
> +
> + /*
> + * The sched_clock infrastructure only knows about counters
> + * with at most 32bits. Forget about the upper 24 bits for the
> + * time being...
> + */
> + return (u32)(cntvct & (u32)~0);
> +}
> +
> +static cycle_t arch_counter_read(struct clocksource *cs)
> +{
> + return arch_counter_get_cntpct();
> +}
> +
> static irqreturn_t arch_timer_handler(int irq, void *dev_id)
> {
> struct clock_event_device *evt = *(struct clock_event_device
**)dev_id;
> @@ -167,7 +317,9 @@ static int arch_timer_available(void)
> {
> unsigned long freq;
>
> - if (!local_timer_is_architected())
> + if (timer_base)
> + arch_specific_timer = &arch_timer_ops_mem;
> + else if (!local_timer_is_architected())
> return -ENXIO;
>
> if (arch_timer_rate == 0) {
> @@ -188,41 +340,6 @@ static int arch_timer_available(void)
> return 0;
> }
>
> -static inline cycle_t arch_counter_get_cntpct(void)
> -{
> - u32 cvall, cvalh;
> -
> - asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> -
> - return ((cycle_t) cvalh << 32) | cvall;
> -}
> -
> -static inline cycle_t arch_counter_get_cntvct(void)
> -{
> - u32 cvall, cvalh;
> -
> - asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r" (cvalh));
> -
> - return ((cycle_t) cvalh << 32) | cvall;
> -}
> -
> -static u32 notrace arch_counter_get_cntvct32(void)
> -{
> - cycle_t cntvct = arch_counter_get_cntvct();
> -
> - /*
> - * The sched_clock infrastructure only knows about counters
> - * with at most 32bits. Forget about the upper 24 bits for the
> - * time being...
> - */
> - return (u32)(cntvct & (u32)~0);
> -}
> -
> -static cycle_t arch_counter_read(struct clocksource *cs)
> -{
> - return arch_counter_get_cntpct();
> -}
> -
> static struct clocksource clocksource_counter = {
> .name = "arch_sys_counter",
> .rating = 400,
> @@ -262,8 +379,12 @@ static int __init arch_timer_register(void)
>
> clocksource_register_hz(&clocksource_counter, arch_timer_rate);
>
> - err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
> + if (is_irq_percpu)
> + err = request_percpu_irq(arch_timer_ppi, arch_timer_handler,
> "arch_timer", arch_timer_evt);
> + else
> + err = request_irq(arch_timer_ppi, arch_timer_handler, 0,
> + "arch_timer", arch_timer_evt);
So you end up enabling the interrupt earlier than expected. Are you sure
this is safe?
> if (err) {
> pr_err("arch_timer: can't register interrupt %d (%d)\n",
> arch_timer_ppi, err);
> @@ -271,8 +392,13 @@ static int __init arch_timer_register(void)
> }
>
> if (arch_timer_ppi2) {
> - err = request_percpu_irq(arch_timer_ppi2, arch_timer_handler,
> - "arch_timer", arch_timer_evt);
> + if (is_irq_percpu)
> + err = request_percpu_irq(arch_timer_ppi2,
> + arch_timer_handler, "arch_timer",
> + arch_timer_evt);
> + else
> + err = request_irq(arch_timer_ppi2, arch_timer_handler,
> + 0, "arch_timer", arch_timer_evt);
> if (err) {
> pr_err("arch_timer: can't register interrupt %d (%d)\n",
> arch_timer_ppi2, err);
> @@ -314,10 +440,40 @@ static const struct of_device_id
> arch_timer_of_match[] __initconst = {
> {},
> };
>
> +static int __init arch_timer_base_init(void)
> +{
> + struct device_node *np;
> +
> + if (!timer_base) {
> + np = of_find_matching_node(NULL, arch_timer_of_match);
> + if (!np) {
> + pr_err("arch_timer: can't find DT node\n");
> + return -ENODEV;
> + }
> +
> + if (of_get_address(np, 0, NULL, NULL)) {
> + timer_base = of_iomap(np, 0);
> + if (!timer_base) {
> + pr_err("arch_timer: cant map timer base\n");
> + return -ENOMEM;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +static inline void __init arch_timer_base_free(void)
> +{
> + if (timer_base)
> + iounmap(timer_base);
> +}
> +
> int __init arch_timer_of_register(void)
> {
> struct device_node *np;
> u32 freq;
> + int ret;
>
> np = of_find_matching_node(NULL, arch_timer_of_match);
> if (!np) {
> @@ -331,20 +487,37 @@ int __init arch_timer_of_register(void)
>
> arch_timer_ppi = irq_of_parse_and_map(np, 0);
> arch_timer_ppi2 = irq_of_parse_and_map(np, 1);
> +
> + ret = arch_timer_base_init();
> + if (ret)
> + return ret;
> +
> + is_irq_percpu = !(of_property_read_bool(np, "irq-is-not-percpu"));
> +
> pr_info("arch_timer: found %s irqs %d %d\n",
> np->name, arch_timer_ppi, arch_timer_ppi2);
>
> - return arch_timer_register();
> + ret = arch_timer_register();
> + if (ret)
> + arch_timer_base_free();
> +
> + return ret;
> }
>
> int __init arch_timer_sched_clock_init(void)
> {
> int err;
>
> - err = arch_timer_available();
> + err = arch_timer_base_init();
> if (err)
> return err;
>
> + err = arch_timer_available();
> + if (err) {
> + arch_timer_base_free();
> + return err;
> + }
> +
> setup_sched_clock(arch_counter_get_cntvct32, 32, arch_timer_rate);
> return 0;
> }
--
Fast, cheap, reliable. Pick two.
On 8/11/2012 3:04 AM, Marc Zyngier wrote:
> Hi Rohit,
>
> On Fri, 10 Aug 2012 14:58:41 -0700, Rohit Vaswani
> <[email protected]>
> wrote:
>> The current arch_timer only support accessing through CP15 interface.
>> Add support for ARM processors that only support IO mapped register
>> interface
> This is quite a departure from the current implementation, which raises a
> couple of questions:
> - What does CP15 ID_PFR1[19:16] report? Can we easily detect that we do
> not have the CP15 interface?
> - What about HYP mode? Is there any way to control the access of the
> physical timer at PL1?
Yes - we can easily detect if CP15 interface is not present. The PFR1
[19:16] are 0 for this implementation.
How is the access control managed currently for the Physical timer ?
>> Signed-off-by: Rohit Vaswani <[email protected]>
>> ---
>> .../devicetree/bindings/arm/arch_timer.txt | 7 +
>> arch/arm/kernel/arch_timer.c | 259
>> ++++++++++++++++----
>> 2 files changed, 223 insertions(+), 43 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt
>> b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> index 52478c8..1c71799 100644
>> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
>> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> @@ -14,6 +14,13 @@ The timer is attached to a GIC to deliver its
>> per-processor interrupts.
>>
>> - clock-frequency : The frequency of the main counter, in Hz. Optional.
>>
>> +- irq-is-not-percpu: Specify is the timer irq is *NOT* a percpu (PPI)
>> interrupt
>> + In the default case i.e without this property, the timer irq is
> treated
>> as a
>> + PPI interrupt. Optional.
> Ouch! How do you specify the various interrupts for all CPUs? Do you end
> up with 4 SPIs per CPU?
>
>> +- If the node address and reg is specified, the arch_timer will try to
>> use the memory
>> + mapped timer. Optional.
>> +
>> Example:
>>
>> timer {
>> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
>> index 1d0d9df..09604b7 100644
>> --- a/arch/arm/kernel/arch_timer.c
>> +++ b/arch/arm/kernel/arch_timer.c
>> @@ -18,6 +18,7 @@
>> #include <linux/clockchips.h>
>> #include <linux/interrupt.h>
>> #include <linux/of_irq.h>
>> +#include <linux/of_address.h>
>> #include <linux/io.h>
>>
>> #include <asm/cputype.h>
>> @@ -29,8 +30,17 @@
>> static unsigned long arch_timer_rate;
>> static int arch_timer_ppi;
>> static int arch_timer_ppi2;
>> +static int is_irq_percpu;
>>
>> static struct clock_event_device __percpu **arch_timer_evt;
>> +static void __iomem *timer_base;
>> +
>> +struct arch_timer_operations {
>> + void (*reg_write)(int, u32);
>> + u32 (*reg_read)(int);
>> + cycle_t (*get_cntpct)(void);
>> + cycle_t (*get_cntvct)(void);
>> +};
> I already have something similar in a patch series that I'd like to get
> merged in 3.7, implementing support for the virtual timers (needed for
> virtualisation). Have a look at:
> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git
> timers-next
I am working on a patch based on this and will send this out soon.
> <snip>
Thanks,
Rohit Vaswani
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
On 8/10/2012 7:40 PM, Cyril Chemparathy wrote:
> On 8/10/2012 5:58 PM, Rohit Vaswani wrote:
>> The current arch_timer only support accessing through CP15 interface.
>> Add support for ARM processors that only support IO mapped register
>> interface
>>
>
> It looks like this patch attempts to address both (a) non-percpu arch
> timers, and (b) memory mapped arch timers in one go. These should
> probably be broken out into two distinct logical changes.
>
Will split them.
> More below...
>
>> Signed-off-by: Rohit Vaswani <[email protected]>
>> ---
>> .../devicetree/bindings/arm/arch_timer.txt | 7 +
>> arch/arm/kernel/arch_timer.c | 259
>> ++++++++++++++++----
>> 2 files changed, 223 insertions(+), 43 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt
>> b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> index 52478c8..1c71799 100644
>> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
>> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
>> @@ -14,6 +14,13 @@ The timer is attached to a GIC to deliver its
>> per-processor interrupts.
>>
>> - clock-frequency : The frequency of the main counter, in Hz.
>> Optional.
>>
>> +- irq-is-not-percpu: Specify is the timer irq is *NOT* a percpu
>> (PPI) interrupt
>> + In the default case i.e without this property, the timer irq is
>> treated as a
>> + PPI interrupt. Optional.
>> +
>
> The handling of non-percpu IRQs looks broken. The code does
> (enable/disable)_percpu_irq() on IRQs that may no longer be percpu.
I am thinking of adding a wrapper function (function pointer in the
timer_ops) that will call (enable/disable)_percpu_irq() based on if the
said property is defined or not.
>
>> +- If the node address and reg is specified, the arch_timer will try
>> to use the memory
>> + mapped timer. Optional.
>> +
>> Example:
>>
>> timer {
>> diff --git a/arch/arm/kernel/arch_timer.c b/arch/arm/kernel/arch_timer.c
>> index 1d0d9df..09604b7 100644
>> --- a/arch/arm/kernel/arch_timer.c
>> +++ b/arch/arm/kernel/arch_timer.c
>> @@ -18,6 +18,7 @@
>> #include <linux/clockchips.h>
>> #include <linux/interrupt.h>
>> #include <linux/of_irq.h>
>> +#include <linux/of_address.h>
>> #include <linux/io.h>
>>
>> #include <asm/cputype.h>
>> @@ -29,8 +30,17 @@
>> static unsigned long arch_timer_rate;
>> static int arch_timer_ppi;
>> static int arch_timer_ppi2;
>> +static int is_irq_percpu;
>>
>> static struct clock_event_device __percpu **arch_timer_evt;
>> +static void __iomem *timer_base;
>> +
>
> Are percpu memory mapped arch timers an impossibility?
I will make sure to keep this open , as in add memory mapped timers as
well as the possibility of percpu timer interrupts for both memory
mapped and the cp15 interface.
>
>> +struct arch_timer_operations {
>> + void (*reg_write)(int, u32);
>> + u32 (*reg_read)(int);
>> + cycle_t (*get_cntpct)(void);
>> + cycle_t (*get_cntvct)(void);
>> +};
>>
>> /*
>> * Architected system timer support.
>> @@ -44,7 +54,29 @@ static struct clock_event_device __percpu
>> **arch_timer_evt;
>> #define ARCH_TIMER_REG_FREQ 1
>> #define ARCH_TIMER_REG_TVAL 2
>>
>> -static void arch_timer_reg_write(int reg, u32 val)
>> +/* Iomapped Register Offsets */
>> +#define ARCH_TIMER_CNTP_LOW_REG 0x000
>> +#define ARCH_TIMER_CNTP_HIGH_REG 0x004
>> +#define ARCH_TIMER_CNTV_LOW_REG 0x008
>> +#define ARCH_TIMER_CNTV_HIGH_REG 0x00C
>> +#define ARCH_TIMER_CTRL_REG 0x02C
>> +#define ARCH_TIMER_FREQ_REG 0x010
>> +#define ARCH_TIMER_CNTP_TVAL_REG 0x028
>> +#define ARCH_TIMER_CNTV_TVAL_REG 0x038
>> +
>
> ARCH_TIMER_CNTV_TVAL_REG appears to be unused here.
>
>> +static void timer_reg_write_mem(int reg, u32 val)
>> +{
>> + switch (reg) {
>> + case ARCH_TIMER_REG_CTRL:
>> + __raw_writel(val, timer_base + ARCH_TIMER_CTRL_REG);
>> + break;
>> + case ARCH_TIMER_REG_TVAL:
>> + __raw_writel(val, timer_base + ARCH_TIMER_CNTP_TVAL_REG);
>> + break;
>> + }
>> +}
>> +
>
> Wouldn't an array of offsets to map from ARCH_TIMER_REG_* to these
> memory mapped registers eliminate the need to switch-case your way
> through each register?
Done.
>
>> +static void timer_reg_write_cp15(int reg, u32 val)
>> {
>> switch (reg) {
>> case ARCH_TIMER_REG_CTRL:
>> @@ -58,7 +90,28 @@ static void arch_timer_reg_write(int reg, u32 val)
>> isb();
>> }
>>
>> -static u32 arch_timer_reg_read(int reg)
>> +static u32 timer_reg_read_mem(int reg)
>> +{
>> + u32 val;
>> +
>> + switch (reg) {
>> + case ARCH_TIMER_REG_CTRL:
>> + val = __raw_readl(timer_base + ARCH_TIMER_CTRL_REG);
>> + break;
>> + case ARCH_TIMER_REG_FREQ:
>> + val = __raw_readl(timer_base + ARCH_TIMER_FREQ_REG);
>> + break;
>> + case ARCH_TIMER_REG_TVAL:
>> + val = __raw_readl(timer_base + ARCH_TIMER_CNTP_TVAL_REG);
>> + break;
>> + default:
>> + BUG();
>> + }
>> +
>> + return val;
>> +}
>> +
>
> Same as above.
>
>> +static u32 timer_reg_read_cp15(int reg)
>> {
>> u32 val;
>>
>> @@ -79,6 +132,103 @@ static u32 arch_timer_reg_read(int reg)
>> return val;
>> }
>>
>> +static cycle_t arch_counter_get_cntpct_mem(void)
>> +{
>> + u32 cvall, cvalh, thigh;
>> +
>> + do {
>> + cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
>> + cvall = __raw_readl(timer_base + ARCH_TIMER_CNTP_LOW_REG);
>> + thigh = __raw_readl(timer_base + ARCH_TIMER_CNTP_HIGH_REG);
>> + } while (cvalh != thigh);
>> +
>> + return ((cycle_t) cvalh << 32) | cvall;
>> +}
>> +
>> +static cycle_t arch_counter_get_cntpct_cp15(void)
>> +{
>> + u32 cvall, cvalh;
>> +
>> + asm volatile("mrrc p15, 0, %0, %1, c14" : "=r" (cvall), "=r"
>> (cvalh));
>> +
>> + return ((cycle_t) cvalh << 32) | cvall;
>> +}
>> +
>> +static cycle_t arch_counter_get_cntvct_mem(void)
>> +{
>> + u32 cvall, cvalh, thigh;
>> +
>> + do {
>> + cvalh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
>> + cvall = __raw_readl(timer_base + ARCH_TIMER_CNTV_LOW_REG);
>> + thigh = __raw_readl(timer_base + ARCH_TIMER_CNTV_HIGH_REG);
>> + } while (cvalh != thigh);
>> +
>> + return ((cycle_t) cvalh << 32) | cvall;
>> +}
>> +
>
> Repetitive - the logic is identical to arch_counter_get_cntpct_mem()
> above.
>
>> +static cycle_t arch_counter_get_cntvct_cp15(void)
>> +{
>> + u32 cvall, cvalh;
>> +
>> + asm volatile("mrrc p15, 1, %0, %1, c14" : "=r" (cvall), "=r"
>> (cvalh));
>> +
>> + return ((cycle_t) cvalh << 32) | cvall;
>> +}
>> +
>> +static struct arch_timer_operations arch_timer_ops_cp15 = {
>> + .reg_read = &timer_reg_read_cp15,
>> + .reg_write = &timer_reg_write_cp15,
>> + .get_cntpct = &arch_counter_get_cntpct_cp15,
>> + .get_cntvct = &arch_counter_get_cntvct_cp15,
>> +};
>> +
>> +static struct arch_timer_operations arch_timer_ops_mem = {
>> + .reg_read = &timer_reg_read_mem,
>> + .reg_write = &timer_reg_write_mem,
>> + .get_cntpct = &arch_counter_get_cntpct_mem,
>> + .get_cntvct = &arch_counter_get_cntvct_mem,
>> +};
>> +
>> +static struct arch_timer_operations *arch_specific_timer =
>> &arch_timer_ops_cp15;
>> +
>> +static inline void arch_timer_reg_write(int reg, u32 val)
>> +{
>> + arch_specific_timer->reg_write(reg, val);
>> +}
>> +
>> +static inline u32 arch_timer_reg_read(int reg)
>> +{
>> + return arch_specific_timer->reg_read(reg);
>> +}
>> +
>> +static inline cycle_t arch_counter_get_cntpct(void)
>> +{
>> + return arch_specific_timer->get_cntpct();
>> +}
>> +
>> +static inline cycle_t arch_counter_get_cntvct(void)
>> +{
>> + return arch_specific_timer->get_cntvct();
>> +}
>> +
>
> The 4 pointer chasers above could lose some verbosity by being #defines.
I am rebasing this patch on top of Marc's similar patch to add support
for virtual timers and will fix up the problems you have mentioned.
Thanks for your feedback.
>
> <snip>
Thanks,
Rohit Vaswani
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.