For multicore SoC's, with cores sharing clock line, we are required to set policy->cpus and policy->related_cpus with mask of cpus.
With following patch, we need to set policy->cpus with mask of all possible cpus and policy->related_cpus would be filled automatically by the cpufreq core.
commit 4948b355e90080cd5ec1e91189f65a01e4186ef2 Author: Viresh Kumar viresh.kumar@linaro.org Date: Tue Jan 29 14:39:08 2013 +0000
cpufreq: Simplify cpufreq_add_dev()
Current Tegra driver fills only ->related_cpus and not ->cpus, which looks to be incorrect. Lets fix it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Cc: Stephen Warren swarren@nvidia.com --- arch/arm/mach-tegra/cpu-tegra.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-tegra/cpu-tegra.c b/arch/arm/mach-tegra/cpu-tegra.c index a74d3c7..e7ddcb2 100644 --- a/arch/arm/mach-tegra/cpu-tegra.c +++ b/arch/arm/mach-tegra/cpu-tegra.c @@ -244,7 +244,7 @@ static int tegra_cpu_init(struct cpufreq_policy *policy) policy->cpuinfo.transition_latency = 300 * 1000;
policy->shared_type = CPUFREQ_SHARED_TYPE_ALL; - cpumask_copy(policy->related_cpus, cpu_possible_mask); + cpumask_copy(policy->cpus, cpu_possible_mask);
if (policy->cpu == 0) register_pm_notifier(&tegra_cpu_pm_notifier);
With following patch, we need to set policy->cpus with mask of all possible cpus and policy->related_cpus would be filled automatically by the core.
commit 4948b355e90080cd5ec1e91189f65a01e4186ef2 Author: Viresh Kumar viresh.kumar@linaro.org Date: Tue Jan 29 14:39:08 2013 +0000
cpufreq: Simplify cpufreq_add_dev()
Lets fix it for all single cluster SoCs.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Cc: Linus Walleij linus.walleij@linaro.org Cc: Dmitry Eremin-Solenikov dbaryshkov@gmail.com Cc: spear-devel@list.st.com --- drivers/cpufreq/db8500-cpufreq.c | 2 +- drivers/cpufreq/maple-cpufreq.c | 2 +- drivers/cpufreq/spear-cpufreq.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/db8500-cpufreq.c b/drivers/cpufreq/db8500-cpufreq.c index 4f154bc..e12dff6 100644 --- a/drivers/cpufreq/db8500-cpufreq.c +++ b/drivers/cpufreq/db8500-cpufreq.c @@ -128,7 +128,7 @@ static int __cpuinit db8500_cpufreq_init(struct cpufreq_policy *policy) policy->cpuinfo.transition_latency = 20 * 1000; /* in ns */
/* policy sharing between dual CPUs */ - cpumask_copy(policy->cpus, cpu_present_mask); + cpumask_setall(policy->cpus);
policy->shared_type = CPUFREQ_SHARED_TYPE_ALL;
diff --git a/drivers/cpufreq/maple-cpufreq.c b/drivers/cpufreq/maple-cpufreq.c index 89b178a..d4c4989 100644 --- a/drivers/cpufreq/maple-cpufreq.c +++ b/drivers/cpufreq/maple-cpufreq.c @@ -181,7 +181,7 @@ static int maple_cpufreq_cpu_init(struct cpufreq_policy *policy) /* secondary CPUs are tied to the primary one by the * cpufreq core if in the secondary policy we tell it that * it actually must be one policy together with all others. */ - cpumask_copy(policy->cpus, cpu_online_mask); + cpumask_setall(policy->cpus); cpufreq_frequency_table_get_attr(maple_cpu_freqs, policy->cpu);
return cpufreq_frequency_table_cpuinfo(policy, diff --git a/drivers/cpufreq/spear-cpufreq.c b/drivers/cpufreq/spear-cpufreq.c index fc714a6..8126084 100644 --- a/drivers/cpufreq/spear-cpufreq.c +++ b/drivers/cpufreq/spear-cpufreq.c @@ -188,7 +188,7 @@ static int spear_cpufreq_init(struct cpufreq_policy *policy) policy->cpuinfo.transition_latency = spear_cpufreq.transition_latency; policy->cur = spear_cpufreq_get(0);
- cpumask_copy(policy->cpus, topology_core_cpumask(policy->cpu)); + cpumask_setall(policy->cpus);
return 0; }
On 1 February 2013 12:10, Viresh Kumar viresh.kumar@linaro.org wrote:
With following patch, we need to set policy->cpus with mask of all possible cpus and policy->related_cpus would be filled automatically by the core.
commit 4948b355e90080cd5ec1e91189f65a01e4186ef2 Author: Viresh Kumar viresh.kumar@linaro.org Date: Tue Jan 29 14:39:08 2013 +0000
cpufreq: Simplify cpufreq_add_dev()
Lets fix it for all single cluster SoCs.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Cc: Linus Walleij linus.walleij@linaro.org Cc: Dmitry Eremin-Solenikov dbaryshkov@gmail.com Cc: spear-devel@list.st.com
drivers/cpufreq/db8500-cpufreq.c | 2 +- drivers/cpufreq/maple-cpufreq.c | 2 +- drivers/cpufreq/spear-cpufreq.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
ARM mail servers are broken, please find patch attached.
policy->shared_type field was added only for SoCs with ACPI support:
commit 3b2d99429e3386b6e2ac949fc72486509c8bbe36 Author: Venkatesh Pallipadi venkatesh.pallipadi@intel.com Date: Wed Dec 14 15:05:00 2005 -0500
P-state software coordination for ACPI core
http://bugzilla.kernel.org/show_bug.cgi?id=5737
Many non-ACPI systems are filling this field by mistake, which makes its usage confusing. Lets clean it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Cc: Linus Walleij linus.walleij@linaro.org Cc: Stephen Warren swarren@nvidia.com Cc: Shawn Guo shawn.guo@linaro.org Cc: Santosh Shilimkar santosh.shilimkar@ti.com --- arch/arm/mach-tegra/cpu-tegra.c | 1 - drivers/cpufreq/cpufreq-cpu0.c | 1 - drivers/cpufreq/db8500-cpufreq.c | 2 -- drivers/cpufreq/omap-cpufreq.c | 4 +--- include/linux/cpufreq.h | 3 ++- 5 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-tegra/cpu-tegra.c b/arch/arm/mach-tegra/cpu-tegra.c index e7ddcb2..a36a03d 100644 --- a/arch/arm/mach-tegra/cpu-tegra.c +++ b/arch/arm/mach-tegra/cpu-tegra.c @@ -243,7 +243,6 @@ static int tegra_cpu_init(struct cpufreq_policy *policy) /* FIXME: what's the actual transition time? */ policy->cpuinfo.transition_latency = 300 * 1000;
- policy->shared_type = CPUFREQ_SHARED_TYPE_ALL; cpumask_copy(policy->cpus, cpu_possible_mask);
if (policy->cpu == 0) diff --git a/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index 38ae178..6a538de 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -146,7 +146,6 @@ static int cpu0_cpufreq_init(struct cpufreq_policy *policy) * share the clock and voltage and clock. Use cpufreq affected_cpus * interface to have all CPUs scaled together. */ - policy->shared_type = CPUFREQ_SHARED_TYPE_ANY; cpumask_setall(policy->cpus);
cpufreq_frequency_table_get_attr(freq_table, policy->cpu); diff --git a/drivers/cpufreq/db8500-cpufreq.c b/drivers/cpufreq/db8500-cpufreq.c index e12dff6..79a8486 100644 --- a/drivers/cpufreq/db8500-cpufreq.c +++ b/drivers/cpufreq/db8500-cpufreq.c @@ -130,8 +130,6 @@ static int __cpuinit db8500_cpufreq_init(struct cpufreq_policy *policy) /* policy sharing between dual CPUs */ cpumask_setall(policy->cpus);
- policy->shared_type = CPUFREQ_SHARED_TYPE_ALL; - return 0; }
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c index 97102b0..9128c07 100644 --- a/drivers/cpufreq/omap-cpufreq.c +++ b/drivers/cpufreq/omap-cpufreq.c @@ -214,10 +214,8 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy) * interface to handle this scenario. Additional is_smp() check * is to keep SMP_ON_UP build working. */ - if (is_smp()) { - policy->shared_type = CPUFREQ_SHARED_TYPE_ANY; + if (is_smp()) cpumask_setall(policy->cpus); - }
/* FIXME: what's the actual transition time? */ policy->cpuinfo.transition_latency = 300 * 1000; diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 6bf3f2d..a22944c 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -93,7 +93,7 @@ struct cpufreq_policy { cpumask_var_t cpus; /* Online CPUs only */ cpumask_var_t related_cpus; /* Online + Offline CPUs */
- unsigned int shared_type; /* ANY or ALL affected CPUs + unsigned int shared_type; /* ACPI: ANY or ALL affected CPUs should set cpufreq */ unsigned int cpu; /* cpu nr of CPU managing this policy */ unsigned int last_cpu; /* cpu nr of previous CPU that managed @@ -122,6 +122,7 @@ struct cpufreq_policy { #define CPUFREQ_START (3) #define CPUFREQ_UPDATE_POLICY_CPU (4)
+/* Only for ACPI */ #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */ #define CPUFREQ_SHARED_TYPE_HW (1) /* HW does needed coordination */ #define CPUFREQ_SHARED_TYPE_ALL (2) /* All dependent CPUs should set freq */
On 1 February 2013 12:10, Viresh Kumar viresh.kumar@linaro.org wrote:
policy->shared_type field was added only for SoCs with ACPI support:
commit 3b2d99429e3386b6e2ac949fc72486509c8bbe36 Author: Venkatesh Pallipadi venkatesh.pallipadi@intel.com Date: Wed Dec 14 15:05:00 2005 -0500
P-state software coordination for ACPI core http://bugzilla.kernel.org/show_bug.cgi?id=5737Many non-ACPI systems are filling this field by mistake, which makes its usage confusing. Lets clean it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Cc: Linus Walleij linus.walleij@linaro.org Cc: Stephen Warren swarren@nvidia.com Cc: Shawn Guo shawn.guo@linaro.org Cc: Santosh Shilimkar santosh.shilimkar@ti.com
arch/arm/mach-tegra/cpu-tegra.c | 1 - drivers/cpufreq/cpufreq-cpu0.c | 1 - drivers/cpufreq/db8500-cpufreq.c | 2 -- drivers/cpufreq/omap-cpufreq.c | 4 +--- include/linux/cpufreq.h | 3 ++- 5 files changed, 3 insertions(+), 8 deletions(-)
ARM mail servers are broken, please find patch attached.
On Friday 01 February 2013 12:10 PM, Viresh Kumar wrote:
policy->shared_type field was added only for SoCs with ACPI support:
commit 3b2d99429e3386b6e2ac949fc72486509c8bbe36 Author: Venkatesh Pallipadi venkatesh.pallipadi@intel.com Date: Wed Dec 14 15:05:00 2005 -0500
P-state software coordination for ACPI core http://bugzilla.kernel.org/show_bug.cgi?id=5737Many non-ACPI systems are filling this field by mistake, which makes its usage confusing. Lets clean it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Cc: Linus Walleij linus.walleij@linaro.org Cc: Stephen Warren swarren@nvidia.com Cc: Shawn Guo shawn.guo@linaro.org Cc: Santosh Shilimkar santosh.shilimkar@ti.com
I haven't looked at the cpufreq code recently but remember that it was needed to ensure that all the CPU which share clock/voltage gets updated (affected cpus) on freq change. The CPUs which needs SW co-ordination, should have this flag enabled and OMAP was falling in that category.
May be I miss-understood its use, but can you confirm that SW co-ordination logic continues to work without this flag ?
Regards, Santosh
On 1 February 2013 12:17, Santosh Shilimkar santosh.shilimkar@ti.com wrote:
I haven't looked at the cpufreq code recently but remember that it was needed to ensure that all the CPU which share clock/voltage gets updated (affected cpus) on freq change. The CPUs which needs SW co-ordination, should have this flag enabled and OMAP was falling in that category.
Freq change are done by the target routines of platform cpufreq drivers and they do something like:
for_each_cpu(freqs.cpu, policy->cpus) cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
The only requirement from cpufreq core is to keep cpus sharing clock in policy->cpus.
May be I miss-understood its use, but can you confirm that SW co-ordination logic continues to work without this flag ?
I believe it should work. It works for the systems i worked on:
SPEAr13xx: Dual Cortex A9 ARM TC2: two clusters of A15s and A7s.
On Friday 01 February 2013 12:43 PM, Viresh Kumar wrote:
On 1 February 2013 12:17, Santosh Shilimkar santosh.shilimkar@ti.com wrote:
I haven't looked at the cpufreq code recently but remember that it was needed to ensure that all the CPU which share clock/voltage gets updated (affected cpus) on freq change. The CPUs which needs SW co-ordination, should have this flag enabled and OMAP was falling in that category.
Freq change are done by the target routines of platform cpufreq drivers and they do something like:
for_each_cpu(freqs.cpu, policy->cpus) cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
The only requirement from cpufreq core is to keep cpus sharing clock in policy->cpus.
I am not talking about just notifiers. This is for external users who has subscribed for notifiers. The point is whether the core CPUFReq gets updated without that flag for all affected CPU.
May be I miss-understood its use, but can you confirm that SW co-ordination logic continues to work without this flag ?
I believe it should work. It works for the systems i worked on:
SPEAr13xx: Dual Cortex A9 ARM TC2: two clusters of A15s and A7s.
I will give a try some time next week on OMAP.
Regards Santosh
On 1 February 2013 13:03, Santosh Shilimkar santosh.shilimkar@ti.com wrote:
I am not talking about just notifiers. This is for external users who has subscribed for notifiers. The point is whether the core CPUFReq gets updated without that flag for all affected CPU.
Yes, its safe. Follow this thread, yesterday i explained this to Tomasz Figa:
On Friday 01 February 2013 01:32 PM, Viresh Kumar wrote:
On 1 February 2013 13:03, Santosh Shilimkar santosh.shilimkar@ti.com wrote:
I am not talking about just notifiers. This is for external users who has subscribed for notifiers. The point is whether the core CPUFReq gets updated without that flag for all affected CPU.
Yes, its safe. Follow this thread, yesterday i explained this to Tomasz Figa:
That part was very clear to me Viresh. Anyway thanks for the link. From what I read so far, it might just work but I would want to try it out before acking the approach.
Regards, Santosh
Viresh,
On Friday 01 February 2013 02:22 PM, Santosh Shilimkar wrote:
On Friday 01 February 2013 01:32 PM, Viresh Kumar wrote:
On 1 February 2013 13:03, Santosh Shilimkar santosh.shilimkar@ti.com wrote:
I am not talking about just notifiers. This is for external users who has subscribed for notifiers. The point is whether the core CPUFReq gets updated without that flag for all affected CPU.
Yes, its safe. Follow this thread, yesterday i explained this to Tomasz Figa:
That part was very clear to me Viresh. Anyway thanks for the link. From what I read so far, it might just work but I would want to try it out before acking the approach.
You are correct. Sorry for oversight on your initial point about the usage of the flag. When I added that flag, I just went by the description thinking the cpufreq core booking and stat updates use the flag. Its not the case.
Thanks for the fix. For the patch, Acked-by: Santosh Shilimkar santosh.shilimkar@ti.com
On 1 February 2013 12:10, Viresh Kumar viresh.kumar@linaro.org wrote:
For multicore SoC's, with cores sharing clock line, we are required to set policy->cpus and policy->related_cpus with mask of cpus.
With following patch, we need to set policy->cpus with mask of all possible cpus and policy->related_cpus would be filled automatically by the cpufreq core.
commit 4948b355e90080cd5ec1e91189f65a01e4186ef2 Author: Viresh Kumar viresh.kumar@linaro.org Date: Tue Jan 29 14:39:08 2013 +0000
cpufreq: Simplify cpufreq_add_dev()Current Tegra driver fills only ->related_cpus and not ->cpus, which looks to be incorrect. Lets fix it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Cc: Stephen Warren swarren@nvidia.com
ARM mail servers are broken, please find patch attached.
On 01/31/2013 11:40 PM, Viresh Kumar wrote:
For multicore SoC's, with cores sharing clock line, we are required to set policy->cpus and policy->related_cpus with mask of cpus.
With following patch, we need to set policy->cpus with mask of all possible cpus and policy->related_cpus would be filled automatically by the cpufreq core.
commit 4948b355e90080cd5ec1e91189f65a01e4186ef2 Author: Viresh Kumar viresh.kumar@linaro.org Date: Tue Jan 29 14:39:08 2013 +0000
cpufreq: Simplify cpufreq_add_dev()Current Tegra driver fills only ->related_cpus and not ->cpus, which looks to be incorrect. Lets fix it.
Joseph Lo reviewed/tested this and it looks fine, so,
Acked-by: Stephen Warren swarren@nvidia.com
-
Santosh Shilimkar -
Stephen Warren -
Viresh Kumar