The CPU policy struct indicates the co-ordination type for all CPUs of a common freq domain. Initialize it correctly using the CPU specific data gathered from CPPC ACPI lib via acpi_get_psd_map().
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org --- drivers/cpufreq/cppc_cpufreq.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index 93c219f..508809b 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -98,6 +98,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) policy->max = cpu->perf_caps.highest_perf; policy->cpuinfo.min_freq = policy->min; policy->cpuinfo.max_freq = policy->max; + policy->shared_type = cpu->shared_type;
if (policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) cpumask_copy(policy->cpus, cpu->shared_cpu_map);
Hi Rafael,
On 12 November 2015 at 19:52, Ashwin Chaugule ashwin.chaugule@linaro.org wrote:
The CPU policy struct indicates the co-ordination type for all CPUs of a common freq domain. Initialize it correctly using the CPU specific data gathered from CPPC ACPI lib via acpi_get_psd_map().
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
drivers/cpufreq/cppc_cpufreq.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index 93c219f..508809b 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -98,6 +98,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) policy->max = cpu->perf_caps.highest_perf; policy->cpuinfo.min_freq = policy->min; policy->cpuinfo.max_freq = policy->max;
policy->shared_type = cpu->shared_type; if (policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) cpumask_copy(policy->cpus, cpu->shared_cpu_map);
-- 1.9.1
I apologize for not spotting this earlier, but I seem to have gotten overzealous cleaning up code here. Can this make it into v4.4-rc1 along with what you've already pushed out?
Thanks, Ashwin.
On Friday, November 13, 2015 03:47:43 PM Ashwin Chaugule wrote:
Hi Rafael,
On 12 November 2015 at 19:52, Ashwin Chaugule ashwin.chaugule@linaro.org wrote:
The CPU policy struct indicates the co-ordination type for all CPUs of a common freq domain. Initialize it correctly using the CPU specific data gathered from CPPC ACPI lib via acpi_get_psd_map().
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
drivers/cpufreq/cppc_cpufreq.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index 93c219f..508809b 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -98,6 +98,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) policy->max = cpu->perf_caps.highest_perf; policy->cpuinfo.min_freq = policy->min; policy->cpuinfo.max_freq = policy->max;
policy->shared_type = cpu->shared_type; if (policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) cpumask_copy(policy->cpus, cpu->shared_cpu_map);
-- 1.9.1
I apologize for not spotting this earlier, but I seem to have gotten overzealous cleaning up code here. Can this make it into v4.4-rc1 along with what you've already pushed out?
It's too late for -rc1, but I'll queue them up for -rc2.
Thanks, Rafael
Hi Rafael,
On 13 November 2015 at 19:27, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, November 13, 2015 03:47:43 PM Ashwin Chaugule wrote:
Hi Rafael,
On 12 November 2015 at 19:52, Ashwin Chaugule ashwin.chaugule@linaro.org wrote:
The CPU policy struct indicates the co-ordination type for all CPUs of a common freq domain. Initialize it correctly using the CPU specific data gathered from CPPC ACPI lib via acpi_get_psd_map().
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
drivers/cpufreq/cppc_cpufreq.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index 93c219f..508809b 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -98,6 +98,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) policy->max = cpu->perf_caps.highest_perf; policy->cpuinfo.min_freq = policy->min; policy->cpuinfo.max_freq = policy->max;
policy->shared_type = cpu->shared_type; if (policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) cpumask_copy(policy->cpus, cpu->shared_cpu_map);
-- 1.9.1
I apologize for not spotting this earlier, but I seem to have gotten overzealous cleaning up code here. Can this make it into v4.4-rc1 along with what you've already pushed out?
It's too late for -rc1, but I'll queue them up for -rc2.
I was working on a rather quirky system with only one CPU in a domain. In such a case the _PSD object doesn't make sense but its also supposed to be an optional object. This system had a PSD package with Num CPUs = 1. The acpi_get_psd_map() routine (which I adapted from acpi_processor_preregister_performance()) rightfully skips trying to match PSDs when there is only one CPU. But it also skips reading the co-ord type from the PSD if there is one. So what ends up happening is that cpu->shared_type is always 0 and the cppc_cpufreq driver will fail init on this CPU, since I only support TYPE_ANY. This will happen even if there is no PSD object for this CPU.
So I think a better way to deal with this is to explicitly check if shared_type == CPUFREQ_SHARED_TYPE_ALL then print out the Unsupported co-ord type message. I'll send a patch along with this fix as a reply to this mail.
Regards, Ashwin.
The CPU policy struct indicates the co-ordination type for all CPUs of a common freq domain. Initialize it correctly using the CPU specific data gathered from CPPC ACPI lib via acpi_get_psd_map().
The PSD object is optional, so the cpu->shared_type can also be 0. So instead of assuming any value other than SW_ANY(0xFD) is unsupported, explictly check if shared_type is SW_ALL and then bail.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org --- drivers/cpufreq/cppc_cpufreq.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index e8cb334..7c0bdfb 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -98,10 +98,11 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) policy->max = cpu->perf_caps.highest_perf; policy->cpuinfo.min_freq = policy->min; policy->cpuinfo.max_freq = policy->max; + policy->shared_type = cpu->shared_type;
if (policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) cpumask_copy(policy->cpus, cpu->shared_cpu_map); - else { + else if (policy->shared_type == CPUFREQ_SHARED_TYPE_ALL) { /* Support only SW_ANY for now. */ pr_debug("Unsupported CPU co-ord type\n"); return -EFAULT;
On Thursday, November 19, 2015 10:40:07 AM Ashwin Chaugule wrote:
The CPU policy struct indicates the co-ordination type for all CPUs of a common freq domain. Initialize it correctly using the CPU specific data gathered from CPPC ACPI lib via acpi_get_psd_map().
The PSD object is optional, so the cpu->shared_type can also be 0. So instead of assuming any value other than SW_ANY(0xFD) is unsupported, explictly check if shared_type is SW_ALL and then bail.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
I'm assiming that this is a replacement for https://patchwork.kernel.org/patch/7607581/ so I'm going to replace that patch with this one, but that means it will go into -rc3 instead.
drivers/cpufreq/cppc_cpufreq.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index e8cb334..7c0bdfb 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -98,10 +98,11 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) policy->max = cpu->perf_caps.highest_perf; policy->cpuinfo.min_freq = policy->min; policy->cpuinfo.max_freq = policy->max;
- policy->shared_type = cpu->shared_type;
if (policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) cpumask_copy(policy->cpus, cpu->shared_cpu_map);
- else {
- else if (policy->shared_type == CPUFREQ_SHARED_TYPE_ALL) { /* Support only SW_ANY for now. */ pr_debug("Unsupported CPU co-ord type\n"); return -EFAULT;
Thanks, Rafael
On 19 November 2015 at 17:17, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Thursday, November 19, 2015 10:40:07 AM Ashwin Chaugule wrote:
The CPU policy struct indicates the co-ordination type for all CPUs of a common freq domain. Initialize it correctly using the CPU specific data gathered from CPPC ACPI lib via acpi_get_psd_map().
The PSD object is optional, so the cpu->shared_type can also be 0. So instead of assuming any value other than SW_ANY(0xFD) is unsupported, explictly check if shared_type is SW_ALL and then bail.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
I'm assiming that this is a replacement for https://patchwork.kernel.org/patch/7607581/ so I'm going to replace that patch with this one, but that means it will go into -rc3 instead.
Replacement is correct. I guess I didn't realize you had pulled it in already. :)
Thanks, Ashwin.