| Message ID | 902bed453159832925df76e24806f3b919fdfc74.1359700706.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
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=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(-) ARM mail servers are broken, please find patch attached. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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=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> > --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: http://www.spinics.net/lists/arm-kernel/msg221629.html -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: > > http://www.spinics.net/lists/arm-kernel/msg221629.html > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: >> >> http://www.spinics.net/lists/arm-kernel/msg221629.html >> > 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> -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */
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(-)