This patch trys to convert arch timer to ACPI using GTDT which based on the convert GIC to ACPI patchset.
After convert the arch timer, it works fine when booted up.
root@genericarmv8:~# cat /proc/interrupts CPU0 CPU1 CPU2 CPU3 29: 0 0 0 0 GIC 29 arch_timer 30: 10751 1821 1888 1252 GIC 30 arch_timer 37: 123 0 0 0 GIC 37 uart-pl011 47: 5 0 0 0 GIC 47 eth0 74: 1031 0 0 0 GIC 74 virtio0 IPI0: 2249 1676 1229 1605 Rescheduling interrupts IPI1: 126 123 114 66 Function call interrupts IPI2: 0 0 0 0 Single function call interrupts IPI3: 0 0 0 0 CPU stop interrupts Err:
Hanjun Guo (4): clocksource: arch_timer: Use ACPI GTDT table to initialize arch timer ARMv8 / colcksource: use arch_timer_acpi_init() ARMv7 / clocksource: use arch_timer_acpi_init() Foundation model / dts: remove timer node since it is converted to ACPI
arch/arm/kernel/time.c | 4 + arch/arm64/boot/dts/foundation-v8-acpi.dts | 6 +- arch/arm64/kernel/time.c | 4 + drivers/clocksource/arm_arch_timer.c | 122 ++++++++++++++++++++++++---- include/clocksource/arm_arch_timer.h | 7 +- 5 files changed, 124 insertions(+), 19 deletions(-)
GTDT (Generic Timer Description Table) contains information for arch timer initialization, this patch use this table to probe arm timer.
[Hanjun: modify the patch to use GTDT table presented in ACPI 5.0, and modify the change log]
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/clocksource/arm_arch_timer.c | 122 +++++++++++++++++++++++++++++----- include/clocksource/arm_arch_timer.h | 7 +- 2 files changed, 113 insertions(+), 16 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index fbd9ccd..714d7f9 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -19,6 +19,7 @@ #include <linux/of_address.h> #include <linux/io.h> #include <linux/slab.h> +#include <linux/acpi.h>
#include <asm/arch_timer.h> #include <asm/virt.h> @@ -579,20 +580,8 @@ static void __init arch_timer_common_init(void) arch_timer_arch_init(); }
-static void __init arch_timer_init(struct device_node *np) +static void __init arch_timer_init(void) { - int i; - - if (arch_timers_present & ARCH_CP15_TIMER) { - pr_warn("arch_timer: multiple nodes in dt, skipping\n"); - return; - } - - arch_timers_present |= ARCH_CP15_TIMER; - for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++) - arch_timer_ppi[i] = irq_of_parse_and_map(np, i); - arch_timer_detect_rate(NULL, np); - /* * If HYP mode is available, we know that the physical timer * has been configured to be accessible from PL1. Use it, so @@ -614,8 +603,111 @@ static void __init arch_timer_init(struct device_node *np) arch_timer_register(); arch_timer_common_init(); } -CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_init); -CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_init); + +static void __init arch_timer_of_init(struct device_node *np) +{ + int i; + + if (arch_timers_present & ARCH_CP15_TIMER) { + pr_warn("arch_timer: multiple nodes in dt, skipping\n"); + return; + } + + arch_timers_present |= ARCH_CP15_TIMER; + for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++) + arch_timer_ppi[i] = irq_of_parse_and_map(np, i); + arch_timer_detect_rate(NULL, np); + + arch_timer_init(); +} +CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init); +CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_of_init); + +#ifdef CONFIG_ACPI +void __init arch_timer_acpi_init(void) +{ + struct acpi_table_gtdt *gtdt; + int trigger, polarity; + void __iomem *base = NULL; + + if (arch_timers_present & ARCH_CP15_TIMER) { + pr_warn("arch_timer: already initialized, skipping\n"); + return; + } + + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_GTDT, 0, + (struct acpi_table_header **)>dt))) { + pr_err("arch_timer: GTDT table not defined\n"); + return; + } + + arch_timers_present |= ARCH_CP15_TIMER; + + /* + * Get the timer frequency. Since there is no frequency info + * in the GTDT table, so we should read it from CNTFREG register + * or hard code here to wait for the new ACPI spec available. + */ + if (!gtdt->address) { + arch_timer_rate = arch_timer_get_cntfrq(); + } else { + base = ioremap(gtdt->address, CNTFRQ); + if (!base) { + pr_warn("arch_timer: unable to map arch timer base address\n"); + return; + } + + arch_timer_rate = readl_relaxed(base + CNTFRQ); + iounmap(base); + } + + if (!arch_timer_rate) { + /* Hard code here to set frequence ? */ + pr_warn("arch_timer: Could not get frequency from GTDT table or CNTFREG\n"); + } + + if (gtdt->secure_pl1_interrupt) { + trigger = (gtdt->secure_pl1_flags & ACPI_GTDT_INTERRUPT_MODE) ? + ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE; + polarity = + (gtdt->secure_pl1_flags & ACPI_GTDT_INTERRUPT_POLARITY) + ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH; + arch_timer_ppi[0] = acpi_register_gsi(NULL, + gtdt->secure_pl1_interrupt, trigger, polarity); + } + if (gtdt->non_secure_pl1_interrupt) { + trigger = + (gtdt->non_secure_pl1_flags & ACPI_GTDT_INTERRUPT_MODE) + ? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE; + polarity = + (gtdt->non_secure_pl1_flags & ACPI_GTDT_INTERRUPT_POLARITY) + ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH; + arch_timer_ppi[1] = acpi_register_gsi(NULL, + gtdt->non_secure_pl1_interrupt, trigger, polarity); + } + if (gtdt->virtual_timer_interrupt) { + trigger = (gtdt->virtual_timer_flags & ACPI_GTDT_INTERRUPT_MODE) + ? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE; + polarity = + (gtdt->virtual_timer_flags & ACPI_GTDT_INTERRUPT_POLARITY) + ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH; + arch_timer_ppi[2] = acpi_register_gsi(NULL, + gtdt->virtual_timer_interrupt, trigger, polarity); + } + if (gtdt->non_secure_pl2_interrupt) { + trigger = + (gtdt->non_secure_pl2_flags & ACPI_GTDT_INTERRUPT_MODE) + ? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE; + polarity = + (gtdt->non_secure_pl2_flags & ACPI_GTDT_INTERRUPT_POLARITY) + ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH; + arch_timer_ppi[3] = acpi_register_gsi(NULL, + gtdt->non_secure_pl2_interrupt, trigger, polarity); + } + + arch_timer_init(); +} +#endif
static void __init arch_timer_mem_init(struct device_node *np) { diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h index 93b7f96..eeb9ce3 100644 --- a/include/clocksource/arm_arch_timer.h +++ b/include/clocksource/arm_arch_timer.h @@ -38,7 +38,7 @@ enum arch_timer_reg { extern u32 arch_timer_get_rate(void); extern u64 (*arch_timer_read_counter)(void); extern struct timecounter *arch_timer_get_timecounter(void); - +extern void __init arch_timer_acpi_init(void); #else
static inline u32 arch_timer_get_rate(void) @@ -56,6 +56,11 @@ static inline struct timecounter *arch_timer_get_timecounter(void) return NULL; }
+static inline void arch_timer_acpi_init(void) +{ + return; +} + #endif
#endif
On 11/15/2013 03:04 AM, Hanjun Guo wrote:
GTDT (Generic Timer Description Table) contains information for arch timer initialization, this patch use this table to probe arm timer.
[Hanjun: modify the patch to use GTDT table presented in ACPI 5.0, and modify the change log]
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/clocksource/arm_arch_timer.c | 122 +++++++++++++++++++++++++++++----- include/clocksource/arm_arch_timer.h | 7 +- 2 files changed, 113 insertions(+), 16 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index fbd9ccd..714d7f9 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -19,6 +19,7 @@ #include <linux/of_address.h> #include <linux/io.h> #include <linux/slab.h> +#include <linux/acpi.h>
#include <asm/arch_timer.h> #include <asm/virt.h> @@ -579,20 +580,8 @@ static void __init arch_timer_common_init(void) arch_timer_arch_init(); }
-static void __init arch_timer_init(struct device_node *np) +static void __init arch_timer_init(void) {
- int i;
- if (arch_timers_present & ARCH_CP15_TIMER) {
pr_warn("arch_timer: multiple nodes in dt, skipping\n");return;- }
- arch_timers_present |= ARCH_CP15_TIMER;
- for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)
arch_timer_ppi[i] = irq_of_parse_and_map(np, i);- arch_timer_detect_rate(NULL, np);
- /*
- If HYP mode is available, we know that the physical timer
- has been configured to be accessible from PL1. Use it, so
@@ -614,8 +603,111 @@ static void __init arch_timer_init(struct device_node *np) arch_timer_register(); arch_timer_common_init(); } -CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_init); -CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_init);
+static void __init arch_timer_of_init(struct device_node *np) +{
- int i;
- if (arch_timers_present & ARCH_CP15_TIMER) {
pr_warn("arch_timer: multiple nodes in dt, skipping\n");return;- }
- arch_timers_present |= ARCH_CP15_TIMER;
- for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)
arch_timer_ppi[i] = irq_of_parse_and_map(np, i);- arch_timer_detect_rate(NULL, np);
- arch_timer_init();
+} +CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init); +CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_of_init);
+#ifdef CONFIG_ACPI +void __init arch_timer_acpi_init(void) +{
- struct acpi_table_gtdt *gtdt;
- int trigger, polarity;
- void __iomem *base = NULL;
- if (arch_timers_present & ARCH_CP15_TIMER) {
pr_warn("arch_timer: already initialized, skipping\n");return;- }
- if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_GTDT, 0,
(struct acpi_table_header **)>dt))) {pr_err("arch_timer: GTDT table not defined\n");return;- }
- arch_timers_present |= ARCH_CP15_TIMER;
- /*
* Get the timer frequency. Since there is no frequency info* in the GTDT table, so we should read it from CNTFREG register* or hard code here to wait for the new ACPI spec available.*/- if (!gtdt->address) {
arch_timer_rate = arch_timer_get_cntfrq();- } else {
base = ioremap(gtdt->address, CNTFRQ);if (!base) {pr_warn("arch_timer: unable to map arch timer base address\n");return;}arch_timer_rate = readl_relaxed(base + CNTFRQ);iounmap(base);- }
- if (!arch_timer_rate) {
/* Hard code here to set frequence ? */pr_warn("arch_timer: Could not get frequency from GTDT table or CNTFREG\n");- }
- if (gtdt->secure_pl1_interrupt) {
trigger = (gtdt->secure_pl1_flags & ACPI_GTDT_INTERRUPT_MODE) ?ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->secure_pl1_flags & ACPI_GTDT_INTERRUPT_POLARITY)? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[0] = acpi_register_gsi(NULL,gtdt->secure_pl1_interrupt, trigger, polarity);- }
- if (gtdt->non_secure_pl1_interrupt) {
trigger =(gtdt->non_secure_pl1_flags & ACPI_GTDT_INTERRUPT_MODE)? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->non_secure_pl1_flags & ACPI_GTDT_INTERRUPT_POLARITY)? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[1] = acpi_register_gsi(NULL,gtdt->non_secure_pl1_interrupt, trigger, polarity);- }
- if (gtdt->virtual_timer_interrupt) {
trigger = (gtdt->virtual_timer_flags & ACPI_GTDT_INTERRUPT_MODE)? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->virtual_timer_flags & ACPI_GTDT_INTERRUPT_POLARITY)? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[2] = acpi_register_gsi(NULL,gtdt->virtual_timer_interrupt, trigger, polarity);- }
- if (gtdt->non_secure_pl2_interrupt) {
trigger =(gtdt->non_secure_pl2_flags & ACPI_GTDT_INTERRUPT_MODE)? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->non_secure_pl2_flags & ACPI_GTDT_INTERRUPT_POLARITY)? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[3] = acpi_register_gsi(NULL,gtdt->non_secure_pl2_interrupt, trigger, polarity);- }
- arch_timer_init();
+} +#endif
static void __init arch_timer_mem_init(struct device_node *np) { diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h index 93b7f96..eeb9ce3 100644 --- a/include/clocksource/arm_arch_timer.h +++ b/include/clocksource/arm_arch_timer.h @@ -38,7 +38,7 @@ enum arch_timer_reg { extern u32 arch_timer_get_rate(void); extern u64 (*arch_timer_read_counter)(void); extern struct timecounter *arch_timer_get_timecounter(void);
+extern void __init arch_timer_acpi_init(void); #else
static inline u32 arch_timer_get_rate(void) @@ -56,6 +56,11 @@ static inline struct timecounter *arch_timer_get_timecounter(void) return NULL; }
+static inline void arch_timer_acpi_init(void) +{
- return;
+}
#endif
#endif
I have no objection to this patch but I am curious: why is it that the ACPI code seems so much larger? I understand there's some more code needed to replace arch_timer_detect_rate() and it looks like the rest is to replace irq_of_parse_and_map(); do I have that right?
If that's the case, would it be possible (or even make any sense) to create an irq_parse_and_map() that reads either DT or ACPI in this case? What I'm thinking, I guess, is that what I'd really like to do is minimize the differences between arch_timer_of_init() and arch_timer_acpi_init() somehow such that the only real change in the driver is in how the config info is retrieved (from DT or ACPI) and everything above that stays the same.
Does that make sense? Telling me that I am hopelessly deluded is also one of the possible responses to this question :).
On 16 November 2013 07:51, Al Stone al.stone@linaro.org wrote:
On 11/15/2013 03:04 AM, Hanjun Guo wrote:
GTDT (Generic Timer Description Table) contains information for arch timer initialization, this patch use this table to probe arm timer.
[Hanjun: modify the patch to use GTDT table presented in ACPI 5.0, and modify the change log]
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/clocksource/arm_arch_timer.c | 122 +++++++++++++++++++++++++++++----- include/clocksource/arm_arch_timer.h | 7 +- 2 files changed, 113 insertions(+), 16 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index fbd9ccd..714d7f9 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -19,6 +19,7 @@ #include <linux/of_address.h> #include <linux/io.h> #include <linux/slab.h> +#include <linux/acpi.h>
#include <asm/arch_timer.h> #include <asm/virt.h> @@ -579,20 +580,8 @@ static void __init arch_timer_common_init(void) arch_timer_arch_init(); }
-static void __init arch_timer_init(struct device_node *np) +static void __init arch_timer_init(void) {
int i;if (arch_timers_present & ARCH_CP15_TIMER) {pr_warn("arch_timer: multiple nodes in dt, skipping\n");return;}arch_timers_present |= ARCH_CP15_TIMER;for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)arch_timer_ppi[i] = irq_of_parse_and_map(np, i);arch_timer_detect_rate(NULL, np);/* * If HYP mode is available, we know that the physical timer * has been configured to be accessible from PL1. Use it, so@@ -614,8 +603,111 @@ static void __init arch_timer_init(struct device_node *np) arch_timer_register(); arch_timer_common_init(); } -CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_init); -CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_init);
+static void __init arch_timer_of_init(struct device_node *np) +{
int i;if (arch_timers_present & ARCH_CP15_TIMER) {pr_warn("arch_timer: multiple nodes in dt, skipping\n");return;}arch_timers_present |= ARCH_CP15_TIMER;for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)arch_timer_ppi[i] = irq_of_parse_and_map(np, i);arch_timer_detect_rate(NULL, np);arch_timer_init();+} +CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init); +CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_of_init);
+#ifdef CONFIG_ACPI +void __init arch_timer_acpi_init(void) +{
struct acpi_table_gtdt *gtdt;int trigger, polarity;void __iomem *base = NULL;if (arch_timers_present & ARCH_CP15_TIMER) {pr_warn("arch_timer: already initialized, skipping\n");return;}if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_GTDT, 0,(struct acpi_table_header **)>dt))) {pr_err("arch_timer: GTDT table not defined\n");return;}arch_timers_present |= ARCH_CP15_TIMER;/** Get the timer frequency. Since there is no frequency info* in the GTDT table, so we should read it from CNTFREG register* or hard code here to wait for the new ACPI spec available.*/if (!gtdt->address) {arch_timer_rate = arch_timer_get_cntfrq();} else {base = ioremap(gtdt->address, CNTFRQ);if (!base) {pr_warn("arch_timer: unable to map arch timerbase address\n");
return;}arch_timer_rate = readl_relaxed(base + CNTFRQ);iounmap(base);}if (!arch_timer_rate) {/* Hard code here to set frequence ? */pr_warn("arch_timer: Could not get frequency from GTDTtable or CNTFREG\n");
}if (gtdt->secure_pl1_interrupt) {trigger = (gtdt->secure_pl1_flags &ACPI_GTDT_INTERRUPT_MODE) ?
ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->secure_pl1_flags &ACPI_GTDT_INTERRUPT_POLARITY)
? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[0] = acpi_register_gsi(NULL,gtdt->secure_pl1_interrupt, trigger,polarity);
}if (gtdt->non_secure_pl1_interrupt) {trigger =(gtdt->non_secure_pl1_flags &ACPI_GTDT_INTERRUPT_MODE)
? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->non_secure_pl1_flags &ACPI_GTDT_INTERRUPT_POLARITY)
? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[1] = acpi_register_gsi(NULL,gtdt->non_secure_pl1_interrupt, trigger,polarity);
}if (gtdt->virtual_timer_interrupt) {trigger = (gtdt->virtual_timer_flags &ACPI_GTDT_INTERRUPT_MODE)
? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->virtual_timer_flags & ACPI_GTDT_INTERRUPT_POLARITY)? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[2] = acpi_register_gsi(NULL,gtdt->virtual_timer_interrupt, trigger, polarity);}if (gtdt->non_secure_pl2_interrupt) {trigger =(gtdt->non_secure_pl2_flags &ACPI_GTDT_INTERRUPT_MODE)
? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->non_secure_pl2_flags &ACPI_GTDT_INTERRUPT_POLARITY)
? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[3] = acpi_register_gsi(NULL,gtdt->non_secure_pl2_interrupt, trigger,polarity);
}arch_timer_init();+} +#endif
static void __init arch_timer_mem_init(struct device_node *np) { diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h index 93b7f96..eeb9ce3 100644 --- a/include/clocksource/arm_arch_timer.h +++ b/include/clocksource/arm_arch_timer.h @@ -38,7 +38,7 @@ enum arch_timer_reg { extern u32 arch_timer_get_rate(void); extern u64 (*arch_timer_read_counter)(void); extern struct timecounter *arch_timer_get_timecounter(void);
+extern void __init arch_timer_acpi_init(void); #else
static inline u32 arch_timer_get_rate(void) @@ -56,6 +56,11 @@ static inline struct timecounter *arch_timer_get_timecounter(void) return NULL; }
+static inline void arch_timer_acpi_init(void) +{
return;+}
#endif
#endif
I have no objection to this patch but I am curious: why is it that the ACPI code seems so much larger? I understand there's some more code needed to replace arch_timer_detect_rate() and it looks like the rest is to replace irq_of_parse_and_map(); do I have that right?
Yes, you are right :)
If that's the case, would it be possible (or even make any sense) to create an irq_parse_and_map() that reads either DT or ACPI in this case? What I'm thinking, I guess, is that what I'd really like to do is minimize the differences between arch_timer_of_init() and arch_timer_acpi_init() somehow such that the only real change in the driver is in how the config info is retrieved (from DT or ACPI) and everything above that stays the same.
This will need some update for GIC converting patch. Ok, I will figure out how to do that.
Does that make sense? Telling me that I am hopelessly deluded is also one of the possible responses to this question :).
Of course it make sense :) Thanks for your suggestion.
-Hanjun
On 2013-11-18 21:09, Hanjun Guo wrote:
On 16 November 2013 07:51, Al Stone <al.stone@linaro.org mailto:al.stone@linaro.org> wrote:
On 11/15/2013 03:04 AM, Hanjun Guo wrote:
[...]
I have no objection to this patch but I am curious: why is it that the ACPI code seems so much larger? I understand there's some more code needed to replace arch_timer_detect_rate() and it looks like the rest is to replace irq_of_parse_and_map(); do I have that right?Yes, you are right :)
If that's the case, would it be possible (or even make any sense) to create an irq_parse_and_map() that reads either DT or ACPI in this case? What I'm thinking, I guess, is that what I'd really like to do is minimize the differences between arch_timer_of_init() and arch_timer_acpi_init() somehow such that the only real change in the driver is in how the config info is retrieved (from DT or ACPI) and everything above that stays the same.This will need some update for GIC converting patch. Ok, I will figure out how to do that.
I was trying to do as you said, but I found that it's too complicated to implement irq_parse_and_map() both for ACPI and DT, so I give up and sent a new version of converting arch timer to ACPI with minor changes :)
Thanks for your guidance.
-Hanjun
I
On 11/19/2013 05:47 AM, Hanjun Guo wrote:
On 2013-11-18 21:09, Hanjun Guo wrote:
On 16 November 2013 07:51, Al Stone <al.stone@linaro.org mailto:al.stone@linaro.org> wrote:
On 11/15/2013 03:04 AM, Hanjun Guo wrote:[...]
I have no objection to this patch but I am curious: why is it that the ACPI code seems so much larger? I understand there's some more code needed to replace arch_timer_detect_rate() and it looks like the rest is to replace irq_of_parse_and_map(); do I have that right?Yes, you are right :)
If that's the case, would it be possible (or even make any sense) to create an irq_parse_and_map() that reads either DT or ACPI in this case? What I'm thinking, I guess, is that what I'd really like to do is minimize the differences between arch_timer_of_init() and arch_timer_acpi_init() somehow such that the only real change in the driver is in how the config info is retrieved (from DT or ACPI) and everything above that stays the same.This will need some update for GIC converting patch. Ok, I will figure out how to do that.
I was trying to do as you said, but I found that it's too complicated to implement irq_parse_and_map() both for ACPI and DT, so I give up and sent a new version of converting arch timer to ACPI with minor changes :)
Thanks for your guidance.
-Hanjun
No worries. Thanks for trying :).
On 15 November 2013 18:04, Hanjun Guo hanjun.guo@linaro.org wrote:
GTDT (Generic Timer Description Table) contains information for arch timer initialization, this patch use this table to probe arm timer.
[Hanjun: modify the patch to use GTDT table presented in ACPI 5.0, and modify the change log]
Signed-off-by: Amit Daniel Kachhap amit.daniel@samsung.com Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/clocksource/arm_arch_timer.c | 122 +++++++++++++++++++++++++++++----- include/clocksource/arm_arch_timer.h | 7 +- 2 files changed, 113 insertions(+), 16 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index fbd9ccd..714d7f9 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -19,6 +19,7 @@ #include <linux/of_address.h> #include <linux/io.h> #include <linux/slab.h> +#include <linux/acpi.h>
#include <asm/arch_timer.h> #include <asm/virt.h> @@ -579,20 +580,8 @@ static void __init arch_timer_common_init(void) arch_timer_arch_init(); }
-static void __init arch_timer_init(struct device_node *np) +static void __init arch_timer_init(void) {
int i;if (arch_timers_present & ARCH_CP15_TIMER) {pr_warn("arch_timer: multiple nodes in dt, skipping\n");return;}arch_timers_present |= ARCH_CP15_TIMER;for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)arch_timer_ppi[i] = irq_of_parse_and_map(np, i);arch_timer_detect_rate(NULL, np);/* * If HYP mode is available, we know that the physical timer * has been configured to be accessible from PL1. Use it, so@@ -614,8 +603,111 @@ static void __init arch_timer_init(struct device_node *np) arch_timer_register(); arch_timer_common_init(); } -CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_init); -CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_init);
+static void __init arch_timer_of_init(struct device_node *np) +{
int i;if (arch_timers_present & ARCH_CP15_TIMER) {pr_warn("arch_timer: multiple nodes in dt, skipping\n");return;}arch_timers_present |= ARCH_CP15_TIMER;for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)arch_timer_ppi[i] = irq_of_parse_and_map(np, i);arch_timer_detect_rate(NULL, np);arch_timer_init();+} +CLOCKSOURCE_OF_DECLARE(armv7_arch_timer, "arm,armv7-timer", arch_timer_of_init); +CLOCKSOURCE_OF_DECLARE(armv8_arch_timer, "arm,armv8-timer", arch_timer_of_init);
+#ifdef CONFIG_ACPI +void __init arch_timer_acpi_init(void) +{
struct acpi_table_gtdt *gtdt;int trigger, polarity;void __iomem *base = NULL;if (arch_timers_present & ARCH_CP15_TIMER) {pr_warn("arch_timer: already initialized, skipping\n");return;}if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_GTDT, 0,(struct acpi_table_header **)>dt))) {pr_err("arch_timer: GTDT table not defined\n");return;}
There is a bug here. acpi_get_table() will finally call early_ioremap(); but I didn't call early_iounmap() and lead to early ioremap leak.
Will update in next version.
arch_timers_present |= ARCH_CP15_TIMER;/** Get the timer frequency. Since there is no frequency info* in the GTDT table, so we should read it from CNTFREG register* or hard code here to wait for the new ACPI spec available.*/if (!gtdt->address) {arch_timer_rate = arch_timer_get_cntfrq();} else {base = ioremap(gtdt->address, CNTFRQ);if (!base) {pr_warn("arch_timer: unable to map arch timer baseaddress\n");
return;}arch_timer_rate = readl_relaxed(base + CNTFRQ);iounmap(base);}if (!arch_timer_rate) {/* Hard code here to set frequence ? */pr_warn("arch_timer: Could not get frequency from GTDTtable or CNTFREG\n");
}if (gtdt->secure_pl1_interrupt) {trigger = (gtdt->secure_pl1_flags &ACPI_GTDT_INTERRUPT_MODE) ?
ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->secure_pl1_flags &ACPI_GTDT_INTERRUPT_POLARITY)
? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[0] = acpi_register_gsi(NULL,gtdt->secure_pl1_interrupt, trigger,polarity);
}if (gtdt->non_secure_pl1_interrupt) {trigger =(gtdt->non_secure_pl1_flags &ACPI_GTDT_INTERRUPT_MODE)
? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->non_secure_pl1_flags & ACPI_GTDT_INTERRUPT_POLARITY)? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[1] = acpi_register_gsi(NULL,gtdt->non_secure_pl1_interrupt, trigger, polarity);}if (gtdt->virtual_timer_interrupt) {trigger = (gtdt->virtual_timer_flags &ACPI_GTDT_INTERRUPT_MODE)
? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->virtual_timer_flags & ACPI_GTDT_INTERRUPT_POLARITY)? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[2] = acpi_register_gsi(NULL,gtdt->virtual_timer_interrupt, trigger, polarity);}if (gtdt->non_secure_pl2_interrupt) {trigger =(gtdt->non_secure_pl2_flags &ACPI_GTDT_INTERRUPT_MODE)
? ACPI_EDGE_SENSITIVE : ACPI_LEVEL_SENSITIVE;polarity =(gtdt->non_secure_pl2_flags & ACPI_GTDT_INTERRUPT_POLARITY)? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;arch_timer_ppi[3] = acpi_register_gsi(NULL,gtdt->non_secure_pl2_interrupt, trigger, polarity);}arch_timer_init();+} +#endif
Thanks Hanjun
Use arch_timer_acpi_init() on ARMv8.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/arm64/kernel/time.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm64/kernel/time.c b/arch/arm64/kernel/time.c index 03dc371..4bdaa4c 100644 --- a/arch/arm64/kernel/time.c +++ b/arch/arm64/kernel/time.c @@ -74,6 +74,10 @@ void __init time_init(void)
clocksource_of_init();
+ /* if can't be initialised from DT, try ACPI way */ + if (!arch_timer_get_rate()) + arch_timer_acpi_init(); + arch_timer_rate = arch_timer_get_rate(); if (!arch_timer_rate) panic("Unable to initialise architected timer.\n");
Use arch_timer_acpi_init() on ARMv7.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/arm/kernel/time.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c index 829a96d..14aac22 100644 --- a/arch/arm/kernel/time.c +++ b/arch/arm/kernel/time.c @@ -27,6 +27,8 @@ #include <linux/timex.h> #include <linux/timer.h>
+#include <clocksource/arm_arch_timer.h> + #include <asm/mach/arch.h> #include <asm/mach/time.h> #include <asm/stacktrace.h> @@ -124,5 +126,7 @@ void __init time_init(void) of_clk_init(NULL); #endif clocksource_of_init(); + if (!arch_timer_get_rate()) + arch_timer_acpi_init(); } }
remove timer node since it is converted to ACPI using GTDT table.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/arm64/boot/dts/foundation-v8-acpi.dts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/foundation-v8-acpi.dts b/arch/arm64/boot/dts/foundation-v8-acpi.dts index 7f57c53..b5f432f 100644 --- a/arch/arm64/boot/dts/foundation-v8-acpi.dts +++ b/arch/arm64/boot/dts/foundation-v8-acpi.dts @@ -74,6 +74,9 @@ interrupts = <1 9 0xf04>; };
+ /* + * Removed for ACPI + * timer { compatible = "arm,armv8-timer"; interrupts = <1 13 0xff01>, @@ -83,9 +86,6 @@ clock-frequency = <100000000>; };
- /* - * Removed for ACPI - * pmu { compatible = "arm,armv8-pmuv3"; interrupts = <0 60 4>,
-
Al Stone -
Hanjun Guo