ARM: shmobile: Handle CA7 arch timer delay
Magnus Damm
maintainerPATCH 1/1 ARM: shmobile: Handle CA7 arch timer delay
From: Magnus Damm <[email protected]>
Update the delay code to include arch timer checks for CA7. From a arch timer availability perspective CA7 should be treated same as CA15.
---
Written against renesas-devel-20141002-v3.17-rc7
arch/arm/mach-shmobile/timer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
arch/arm/mach-shmobile/timer.c +15 -8
@@ -45,6 +45,7 @@ void __init shmobile_init_delay(void)struct device_node *np, *cpus;bool is_a7_a8_a9 = false;bool is_a15 = false; +bool has_arch_timer = false;u32 max_freq =0 ; cpus =of_find_node_by_path ("/cpus" );
@@ -57,12 +58,16 @@ void __init shmobile_init_delay(void)if (!of_property_read_u32 (np,"clock-frequency" , &freq)) max_freq =max (max_freq, freq); -if (of_device_is_compatible (np,"arm,cortex-a7" ) || -of_device_is_compatible (np,"arm,cortex-a8" ) || -of_device_is_compatible (np,"arm,cortex-a9" )) +if (of_device_is_compatible (np,"arm,cortex-a8" ) || +of_device_is_compatible (np,"arm,cortex-a9" )) { is_a7_a8_a9 = true; -else if (of_device_is_compatible (np,"arm,cortex-a15" )) + }else if (of_device_is_compatible (np,"arm,cortex-a7" )) { + is_a7_a8_a9 = true; + has_arch_timer = true; + }else if (of_device_is_compatible (np,"arm,cortex-a15" )) { is_a15 = true; + has_arch_timer = true; + } }of_node_put (cpus);
@@ -70,10 +75,12 @@ void __init shmobile_init_delay(void)if (!max_freq)return ; -if (is_a7_a8_a9) -shmobile_setup_delay_hz (max_freq,1 ,3 ); -else if (is_a15 && !IS_ENABLED (CONFIG_ARM_ARCH_TIMER )) -shmobile_setup_delay_hz (max_freq,2 ,4 ); +if (!has_arch_timer || !IS_ENABLED (CONFIG_ARM_ARCH_TIMER )) { +if (is_a7_a8_a9) +shmobile_setup_delay_hz (max_freq,1 ,3 ); +else if (is_a15) +shmobile_setup_delay_hz (max_freq,2 ,4 ); + } }static void __init shmobile_late_time_init (void )
Quoted 10 lines · Magnus Damm
From: Magnus Damm <[email protected]>
Update the delay code to include arch timer checks for CA7. From a arch timer availability perspective CA7 should be treated same as CA15.
Signed-off-by Magnus Damm <[email protected]>---
Written against renesas-devel-20141002-v3.17-rc7
Thanks Magnus, I have queued this up.
There is an outstanding question from Arnd
relating to the last round of init delay updates.
If you have a moment could you look into it?
* Re: [GIT PULL] Renesas ARM Based SoC Init Delay Updates For v3.18
http://www.spinics.net/lists/arm-kernel/msg360580.html
Quoted 52 lines
arch/arm/mach-shmobile/timer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)arch/arm/mach-shmobile/timer.c +15
-8@@ -45,6 +45,7 @@ void __init shmobile_init_delay(void)struct device_node *np, *cpus;bool is_a7_a8_a9 = false;bool is_a15 = false; +bool has_arch_timer = false;u32 max_freq =0 ; cpus =of_find_node_by_path ("/cpus" );@@ -57,12 +58,16 @@ void __init shmobile_init_delay(void)if (!of_property_read_u32 (np,"clock-frequency" , &freq)) max_freq =max (max_freq, freq); -if (of_device_is_compatible (np,"arm,cortex-a7" ) || -of_device_is_compatible (np,"arm,cortex-a8" ) || -of_device_is_compatible (np,"arm,cortex-a9" )) +if (of_device_is_compatible (np,"arm,cortex-a8" ) || +of_device_is_compatible (np,"arm,cortex-a9" )) { is_a7_a8_a9 = true; -else if (of_device_is_compatible (np,"arm,cortex-a15" )) + }else if (of_device_is_compatible (np,"arm,cortex-a7" )) { + is_a7_a8_a9 = true; + has_arch_timer = true; + }else if (of_device_is_compatible (np,"arm,cortex-a15" )) { is_a15 = true; + has_arch_timer = true; + } }of_node_put (cpus);@@ -70,10 +75,12 @@ void __init shmobile_init_delay(void)if (!max_freq)return ; -if (is_a7_a8_a9) -shmobile_setup_delay_hz (max_freq,1 ,3 ); -else if (is_a15 && !IS_ENABLED (CONFIG_ARM_ARCH_TIMER )) -shmobile_setup_delay_hz (max_freq,2 ,4 ); +if (!has_arch_timer || !IS_ENABLED (CONFIG_ARM_ARCH_TIMER )) { +if (is_a7_a8_a9) +shmobile_setup_delay_hz (max_freq,1 ,3 ); +else if (is_a15) +shmobile_setup_delay_hz (max_freq,2 ,4 ); + } }static void __init shmobile_late_time_init (void )
Hi Magnus-san,
Thanks for your patch.
I have 2 points to confirm.
Please kindly share your opinions.
Quoted 31 lines · Magnus Damm
From: Magnus Damm <[email protected]>
Update the delay code to include arch timer checks for CA7. From a arch timer availability perspective CA7 should be treated same as CA15.
Signed-off-by Magnus Damm <[email protected]>---
Written against renesas-devel-20141002-v3.17-rc7
arch/arm/mach-shmobile/timer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)arch/arm/mach-shmobile/timer.c +1
-3@@ -45,6 +45,7 @@ void __init shmobile_init_delay(void)struct device_node *np, *cpus;bool is_a7_a8_a9 = false;bool is_a15 = false; +bool has_arch_timer = false;u32 max_freq =0 ; cpus =of_find_node_by_path ("/cpus" );@@ -57,12 +58,16 @@ void __init shmobile_init_delay(void)if (!of_property_read_u32 (np,"clock-frequency" , &freq)) max_freq =max (max_freq, freq); -if (of_device_is_compatible (np,"arm,cortex-a7" ) || -of_device_is_compatible (np,"arm,cortex-a8" ) || -of_device_is_compatible (np,"arm,cortex-a9" ))
arch/arm/mach-shmobile/timer.c void __init shmobile_init_delay(void) · line 69
Quoted 10 lines
arch/arm/mach-shmobile/timer.c +7
-1+if (of_device_is_compatible (np,"arm,cortex-a8" ) || +of_device_is_compatible (np,"arm,cortex-a9" )) { is_a7_a8_a9 = true; - else if (of_device_is_compatible(np, "arm,cortex-a15")) + }else if (of_device_is_compatible (np,"arm,cortex-a7" )) { + is_a7_a8_a9 = true; + has_arch_timer = true; + }else if (of_device_is_compatible (np,"arm,cortex-a15" )) { is_a15 = true; + has_arch_timer = true;
Because has_arch_timer set true here, below delay setting for a15 will never be entered. However, before this patch, it will be executed in case CONFIG_ARM_ARCH_TIMER is disable. Is it your intention ?
arch/arm/mach-shmobile/timer.c void __init shmobile_init_delay(void) · line 78
Quoted 13 lines
arch/arm/mach-shmobile/timer.c +2
-4+ } }of_node_put (cpus);@@ -70,10 +75,12 @@ void __init shmobile_init_delay(void)if (!max_freq)return ; -if (is_a7_a8_a9) -shmobile_setup_delay_hz (max_freq,1 ,3 ); -else if (is_a15 && !IS_ENABLED (CONFIG_ARM_ARCH_TIMER )) -shmobile_setup_delay_hz (max_freq,2 ,4 ); +if (!has_arch_timer || !IS_ENABLED (CONFIG_ARM_ARCH_TIMER )) {
Above condition checking will apply to all cortex cores, including a8 and a9. However, before this patch, delay setting will be set for a8 and a9, regardless of CONFIG_ARM_ARCH_TIMER.
Is it your intention ?
Quoted 13 lines
arch/arm/mach-shmobile/timer.c +5
-1+if (is_a7_a8_a9) +shmobile_setup_delay_hz (max_freq,1 ,3 ); +else if (is_a15) +shmobile_setup_delay_hz (max_freq,2 ,4 ); + } }static void __init shmobile_late_time_init (void ) --To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Best regards, KHIEM Nguyen
Magnus Damm
maintainerHi Simon,
Quoted 12 lines · Simon Horman
Quoted 10 lines · Magnus Damm
From: Magnus Damm <[email protected]>
Update the delay code to include arch timer checks for CA7. From a arch timer availability perspective CA7 should be treated same as CA15.
Signed-off-by Magnus Damm <[email protected]>---
Written against renesas-devel-20141002-v3.17-rc7
Quoted 8 lines
Thanks Magnus, I have queued this up.
There is an outstanding question from Arnd
relating to the last round of init delay updates.
If you have a moment could you look into it?* Re: [GIT PULL] Renesas ARM Based SoC Init Delay Updates For v3.18
http://www.spinics.net/lists/arm-kernel/msg360580.html
Thanks, I will. It's been on my TODO for quite some time now!
Cheers,
/ magnus
Magnus Damm
maintainerHi Khiem-san,
Quoted 5 lines · Khiem Nguyen
Hi Magnus-san,
Thanks for your patch.
Thanks for your email.
Quoted 3 lines
I have 2 points to confirm.
Please kindly share your opinions.
Sure.
Quoted 38 lines · Magnus Damm
Quoted 37 lines
From: Magnus Damm <[email protected]>
Update the delay code to include arch timer checks for CA7. From a arch timer availability perspective CA7 should be treated same as CA15.
Signed-off-by Magnus Damm <[email protected]>---
Written against renesas-devel-20141002-v3.17-rc7
arch/arm/mach-shmobile/timer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +5
-4@@ -45,6 +45,7 @@ void __init shmobile_init_delay(void) struct device_node *np, *cpus; bool is_a7_a8_a9 = false; bool is_a15 = false; + bool has_arch_timer = false; u32 max_freq = 0; cpus = of_find_node_by_path("/cpus");@@ -57,12 +58,16 @@ void __init shmobile_init_delay(void) if (!of_property_read_u32(np, "clock-frequency", &freq)) max_freq = max(max_freq, freq); - if (of_device_is_compatible(np, "arm,cortex-a7") || - of_device_is_compatible(np, "arm,cortex-a8") || - of_device_is_compatible(np, "arm,cortex-a9")) + if (of_device_is_compatible(np, "arm,cortex-a8") || + of_device_is_compatible(np, "arm,cortex-a9")) { is_a7_a8_a9 = true; - else if (of_device_is_compatible(np, "arm,cortex-a15")) + } else if (of_device_is_compatible(np, "arm,cortex-a7")) { + is_a7_a8_a9 = true;
Quoted 8 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 void __init shmobile_init_delay(void) · line 69
Quoted 4 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +3
-0+ has_arch_timer = true; + } else if (of_device_is_compatible(np, "arm,cortex-a15")) { is_a15 = true; + has_arch_timer = true;Because
has_arch_timerset true here, below delay setting for a15 will never be entered. However, before this patch, it will be executed in caseCONFIG_ARM_ARCH_TIMERis disable. Is it your intention ?
No, it is not my intention. "has_arch_timer" here is used to tell if the arch timer hardware may be available. This only happens on CA7 and CA15.
Quoted 8 lines
Quoted 8 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +1
-0+ } } of_node_put(cpus);@@ -70,10 +75,12 @@ void __init shmobile_init_delay(void) if (!max_freq) return;
Quoted 10 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 void __init shmobile_init_delay(void) · line 78
Quoted 5 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +1
-4- if (is_a7_a8_a9) - shmobile_setup_delay_hz(max_freq, 1, 3); - else if (is_a15 && !IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) - shmobile_setup_delay_hz(max_freq, 2, 4); + if (!has_arch_timer || !IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) {Above condition checking will apply to all cortex cores, including a8 and a9. However, before this patch, delay setting will be set for a8 and a9, regardless of
CONFIG_ARM_ARCH_TIMER.
Is it your intention ?
My intention is to only call shmobile_setup_delay_hz() in the following cases:
A) arch timer hardware is unavailable (has_arch_timer is false) or
B) arch timer is available (has_arch_timer is true) and CONFIG_ARM_ARCH_TIMER is disabled.
Case A) above covers CPU cores without arch timer, like CA8 and CA9.
Is there any problem?
Cheers,
/ magnus
Hi Magnus-san,
Quoted 90 lines · Magnus Damm
Hi Khiem-san,
Quoted 5 lines · Khiem Nguyen
Hi Magnus-san,
Thanks for your patch.
Thanks for your email.
Quoted 3 lines
I have 2 points to confirm.
Please kindly share your opinions.
Sure.
Quoted 38 lines · Magnus Damm
Quoted 37 lines
From: Magnus Damm <[email protected]>
Update the delay code to include arch timer checks for CA7. From a arch timer availability perspective CA7 should be treated same as CA15.
Signed-off-by Magnus Damm <[email protected]>---
Written against renesas-devel-20141002-v3.17-rc7
arch/arm/mach-shmobile/timer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +5
-4@@ -45,6 +45,7 @@ void __init shmobile_init_delay(void) struct device_node *np, *cpus; bool is_a7_a8_a9 = false; bool is_a15 = false; + bool has_arch_timer = false; u32 max_freq = 0; cpus = of_find_node_by_path("/cpus");@@ -57,12 +58,16 @@ void __init shmobile_init_delay(void) if (!of_property_read_u32(np, "clock-frequency", &freq)) max_freq = max(max_freq, freq); - if (of_device_is_compatible(np, "arm,cortex-a7") || - of_device_is_compatible(np, "arm,cortex-a8") || - of_device_is_compatible(np, "arm,cortex-a9")) + if (of_device_is_compatible(np, "arm,cortex-a8") || + of_device_is_compatible(np, "arm,cortex-a9")) { is_a7_a8_a9 = true; - else if (of_device_is_compatible(np, "arm,cortex-a15")) + } else if (of_device_is_compatible(np, "arm,cortex-a7")) { + is_a7_a8_a9 = true;Quoted 8 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 void __init shmobile_init_delay(void) · line 69
Quoted 4 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +3
-0+ has_arch_timer = true; + } else if (of_device_is_compatible(np, "arm,cortex-a15")) { is_a15 = true; + has_arch_timer = true;Because
has_arch_timerset true here, below delay setting for a15 will never be entered. However, before this patch, it will be executed in caseCONFIG_ARM_ARCH_TIMERis disable. Is it your intention ?No, it is not my intention. "
has_arch_timer" here is used to tell if the arch timer hardware may be available. This only happens on CA7 and CA15.Quoted 8 lines
Quoted 8 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +1
-0+ } } of_node_put(cpus);@@ -70,10 +75,12 @@ void __init shmobile_init_delay(void) if (!max_freq) return;Quoted 10 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 void __init shmobile_init_delay(void) · line 78
Quoted 5 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +1
-4- if (is_a7_a8_a9) - shmobile_setup_delay_hz(max_freq, 1, 3); - else if (is_a15 && !IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) - shmobile_setup_delay_hz(max_freq, 2, 4); + if (!has_arch_timer || !IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) {Above condition checking will apply to all cortex cores, including a8 and a9. However, before this patch, delay setting will be set for a8 and a9, regardless of
CONFIG_ARM_ARCH_TIMER.
Is it your intention ?My intention is to only call
shmobile_setup_delay_hz()in the following cases:
Quoted 8 lines
A) arch timer hardware is unavailable (
has_arch_timeris false) or
B) arch timer is available (has_arch_timeris true) andCONFIG_ARM_ARCH_TIMERis disabled.Case A) above covers CPU cores without arch timer, like CA8 and CA9.
Is there any problem?
Your implementation is OK.
At the beginning, I confused the meaning of using 'has_arch_timer' and CONFIG_ARM_ARCH_TIMER.
Now, I understood the implementation via your explanation.
Quoted 4 lines
Cheers,
/ magnus
Signature · 2 lines
Best regards, KHIEM Nguyen
Magnus Damm
maintainerHi Khiem-san,
Quoted 98 lines · Khiem Nguyen
Hi Magnus-san,
Quoted 95 lines · Magnus Damm
Hi Khiem-san,
Quoted 5 lines · Khiem Nguyen
Hi Magnus-san,
Thanks for your patch.
Thanks for your email.
Quoted 3 lines
I have 2 points to confirm.
Please kindly share your opinions.
Sure.
Quoted 38 lines · Magnus Damm
Quoted 37 lines
From: Magnus Damm <[email protected]>
Update the delay code to include arch timer checks for CA7. From a arch timer availability perspective CA7 should be treated same as CA15.
Signed-off-by Magnus Damm <[email protected]>---
Written against renesas-devel-20141002-v3.17-rc7
arch/arm/mach-shmobile/timer.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +5
-4@@ -45,6 +45,7 @@ void __init shmobile_init_delay(void) struct device_node *np, *cpus; bool is_a7_a8_a9 = false; bool is_a15 = false; + bool has_arch_timer = false; u32 max_freq = 0; cpus = of_find_node_by_path("/cpus");@@ -57,12 +58,16 @@ void __init shmobile_init_delay(void) if (!of_property_read_u32(np, "clock-frequency", &freq)) max_freq = max(max_freq, freq); - if (of_device_is_compatible(np, "arm,cortex-a7") || - of_device_is_compatible(np, "arm,cortex-a8") || - of_device_is_compatible(np, "arm,cortex-a9")) + if (of_device_is_compatible(np, "arm,cortex-a8") || + of_device_is_compatible(np, "arm,cortex-a9")) { is_a7_a8_a9 = true; - else if (of_device_is_compatible(np, "arm,cortex-a15")) + } else if (of_device_is_compatible(np, "arm,cortex-a7")) { + is_a7_a8_a9 = true;Quoted 8 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 void __init shmobile_init_delay(void) · line 69
Quoted 4 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +3
-0+ has_arch_timer = true; + } else if (of_device_is_compatible(np, "arm,cortex-a15")) { is_a15 = true; + has_arch_timer = true;Because
has_arch_timerset true here, below delay setting for a15 will never be entered. However, before this patch, it will be executed in caseCONFIG_ARM_ARCH_TIMERis disable. Is it your intention ?No, it is not my intention. "
has_arch_timer" here is used to tell if the arch timer hardware may be available. This only happens on CA7 and CA15.Quoted 8 lines
Quoted 8 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +1
-0+ } } of_node_put(cpus);@@ -70,10 +75,12 @@ void __init shmobile_init_delay(void) if (!max_freq) return;Quoted 10 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 void __init shmobile_init_delay(void) · line 78
Quoted 5 lines
arch/arm/mach-shmobile/timer.c 2014-10-06 08:44:21.000000000 +0900 renamed from arch/arm/mach-shmobile/timer.c +1
-4- if (is_a7_a8_a9) - shmobile_setup_delay_hz(max_freq, 1, 3); - else if (is_a15 && !IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) - shmobile_setup_delay_hz(max_freq, 2, 4); + if (!has_arch_timer || !IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) {Above condition checking will apply to all cortex cores, including a8 and a9. However, before this patch, delay setting will be set for a8 and a9, regardless of
CONFIG_ARM_ARCH_TIMER.
Is it your intention ?My intention is to only call
shmobile_setup_delay_hz()in the following cases:A) arch timer hardware is unavailable (
has_arch_timeris false) or
B) arch timer is available (has_arch_timeris true) andCONFIG_ARM_ARCH_TIMERis disabled.
Quoted 8 lines
Quoted 3 lines
Case A) above covers CPU cores without arch timer, like CA8 and CA9.
Is there any problem?
Your implementation is OK.
At the beginning, I confused the meaning of using 'has_arch_timer' andCONFIG_ARM_ARCH_TIMER.
Now, I understood the implementation via your explanation.
Ok, good! Thanks for checking!
Cheers,
/ magnus