[PATCH 1/2] ACPI/Processor: Clear unuseful variable count in the acpi_processor_preregister_performance()

[PATCH 1/2] ACPI/Processor: Clear unuseful variable count in the acpi_processor_preregister_performance()

[Lan Tianyu]

The variable count in the acpi_processor_preregister_performance() is only initalized
as one for one cpu and increased by one when find another cpu that share same dependency
domain. Otherwise, it isn't referenced somewhere else.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/acpi/processor_perflib.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index e854582..1e9732d 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -639,7 +639,7 @@ end:
 int acpi_processor_preregister_performance(
 		struct acpi_processor_performance __percpu *performance)
 {
-	int count, count_target;
+	int count_target;
 	int retval = 0;
 	unsigned int i, j;
 	cpumask_var_t covered_cpus;
@@ -711,7 +711,6 @@ int acpi_processor_preregister_performance(
 
 		/* Validate the Domain info */
 		count_target = pdomain->num_processors;
-		count = 1;
 		if (pdomain->coord_type == DOMAIN_COORD_TYPE_SW_ALL)
 			pr->performance->shared_type = CPUFREQ_SHARED_TYPE_ALL;
 		else if (pdomain->coord_type == DOMAIN_COORD_TYPE_HW_ALL)
@@ -745,7 +744,6 @@ int acpi_processor_preregister_performance(
 
 			cpumask_set_cpu(j, covered_cpus);
 			cpumask_set_cpu(j, pr->performance->shared_cpu_map);
-			count++;
 		}
 
 		for_each_possible_cpu(j) {
-- 
1.7.10.4

[PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Lan Tianyu]

Commit aa77a52 and fcf8058 changes the content of "related_cpus" and user
space can't get cpus which are in the same hardware coordination cpu domain
(These info are provided by ACPI AML method _PSD) via "related_cpus". This
change affects some users of original "related_cpus".

This patch is to add a new sysfs attribute "freqdomian_cpus" for acpi-cpufreq
driver which exposes all cpus in the same domain regardless of hardware or
software coordination to make up the info loss of previous change.

Reference:https://bugzilla.kernel.org/show_bug.cgi?id=58761
Reported-by: Jean-Philippe Halimi <jean-philippe.halimi@exascale-computing.eu>
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 Documentation/cpu-freq/user-guide.txt |    4 ++++
 drivers/cpufreq/acpi-cpufreq.c        |   11 +++++++++++
 drivers/cpufreq/cpufreq.c             |    7 ++++---
 include/linux/cpufreq.h               |    3 +++
 4 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/Documentation/cpu-freq/user-guide.txt b/Documentation/cpu-freq/user-guide.txt
index ff2f283..0cc72f7 100644
--- a/Documentation/cpu-freq/user-guide.txt
+++ b/Documentation/cpu-freq/user-guide.txt
@@ -196,6 +196,10 @@ affected_cpus :			List of Online CPUs that require software
 related_cpus :			List of Online + Offline CPUs that need software
 				coordination of frequency.
 
+freqdomain_cpus :		List of Online + Offline CPUs in same CPU dependency
+				domain. (This is only available for acpi-cpufreq
+				driver)
+
 scaling_driver :		Hardware driver for cpufreq.
 
 scaling_cur_freq :		Current frequency of the CPU as determined by
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 17e3496..b859997 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -176,6 +176,16 @@ static struct global_attr global_boost = __ATTR(boost, 0644,
 						show_global_boost,
 						store_global_boost);
 
+static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
+{
+	struct acpi_cpufreq_data *data = per_cpu(acfreq_data, policy->cpu);
+	struct acpi_processor_performance *perf = data->acpi_data;
+
+	return cpufreq_show_cpus(perf->shared_cpu_map, buf);
+}
+
+cpufreq_freq_attr_ro(freqdomain_cpus);
+
 #ifdef CONFIG_X86_ACPI_CPUFREQ_CPB
 static ssize_t store_cpb(struct cpufreq_policy *policy, const char *buf,
 			 size_t count)
@@ -906,6 +916,7 @@ static int acpi_cpufreq_resume(struct cpufreq_policy *policy)
 
 static struct freq_attr *acpi_cpufreq_attr[] = {
 	&cpufreq_freq_attr_scaling_available_freqs,
+	&freqdomain_cpus,
 	NULL,	/* this is a placeholder for cpb, do not remove */
 	NULL,
 };
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index f8c2860..d31feab 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -574,7 +574,7 @@ out:
 	return i;
 }
 
-static ssize_t show_cpus(const struct cpumask *mask, char *buf)
+ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf)
 {
 	ssize_t i = 0;
 	unsigned int cpu;
@@ -589,6 +589,7 @@ static ssize_t show_cpus(const struct cpumask *mask, char *buf)
 	i += sprintf(&buf[i], "\n");
 	return i;
 }
+EXPORT_SYMBOL_GPL(cpufreq_show_cpus);
 
 /**
  * show_related_cpus - show the CPUs affected by each transition even if
@@ -596,7 +597,7 @@ static ssize_t show_cpus(const struct cpumask *mask, char *buf)
  */
 static ssize_t show_related_cpus(struct cpufreq_policy *policy, char *buf)
 {
-	return show_cpus(policy->related_cpus, buf);
+	return cpufreq_show_cpus(policy->related_cpus, buf);
 }
 
 /**
@@ -604,7 +605,7 @@ static ssize_t show_related_cpus(struct cpufreq_policy *policy, char *buf)
  */
 static ssize_t show_affected_cpus(struct cpufreq_policy *policy, char *buf)
 {
-	return show_cpus(policy->cpus, buf);
+	return cpufreq_show_cpus(policy->cpus, buf);
 }
 
 static ssize_t store_scaling_setspeed(struct cpufreq_policy *policy,
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d939056..70021f1 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -438,4 +438,7 @@ void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table,
 void cpufreq_frequency_table_update_policy_cpu(struct cpufreq_policy *policy);
 
 void cpufreq_frequency_table_put_attr(unsigned int cpu);
+
+ssize_t cpufreq_show_cpus(const struct cpumask *mask, char *buf);
+
 #endif /* _LINUX_CPUFREQ_H */
-- 
1.7.10.4

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Viresh Kumar]

On 25 June 2013 07:36, Lan Tianyu <tianyu.lan@intel.com> wrote:
> diff --git a/Documentation/cpu-freq/user-guide.txt b/Documentation/cpu-freq/user-guide.txt
> index ff2f283..0cc72f7 100644
> --- a/Documentation/cpu-freq/user-guide.txt
> +++ b/Documentation/cpu-freq/user-guide.txt
> @@ -196,6 +196,10 @@ affected_cpus :                    List of Online CPUs that require software
>  related_cpus :                 List of Online + Offline CPUs that need software
>                                 coordination of frequency.
>
> +freqdomain_cpus :              List of Online + Offline CPUs in same CPU dependency
> +                               domain. (This is only available for acpi-cpufreq
> +                               driver)
> +

This is generic file, don't add this information here. Add this in
acpi-cpufreq file.

>  scaling_driver :               Hardware driver for cpufreq.
>
>  scaling_cur_freq :             Current frequency of the CPU as determined by
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 17e3496..b859997 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -176,6 +176,16 @@ static struct global_attr global_boost = __ATTR(boost, 0644,
>                                                 show_global_boost,
>                                                 store_global_boost);
>
> +static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
> +{
> +       struct acpi_cpufreq_data *data = per_cpu(acfreq_data, policy->cpu);
> +       struct acpi_processor_performance *perf = data->acpi_data;
> +
> +       return cpufreq_show_cpus(perf->shared_cpu_map, buf);
> +}

I am not sure if this is enough. Check this commit:

aa77a52764a92216b61a6c8079b5c01937c046cd

It had these changes:

diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 937bc28..57a8774 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -730,7 +730,6 @@ static int acpi_cpufreq_cpu_init(struct
cpufreq_policy *policy)
            policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) {
                cpumask_copy(policy->cpus, perf->shared_cpu_map);
        }
-       cpumask_copy(policy->related_cpus, perf->shared_cpu_map);

 #ifdef CONFIG_SMP
        dmi_check_system(sw_any_bug_dmi_table);
@@ -742,7 +741,6 @@ static int acpi_cpufreq_cpu_init(struct
cpufreq_policy *policy)
        if (check_amd_hwpstate_cpu(cpu) && !acpi_pstate_strict) {
                cpumask_clear(policy->cpus);
                cpumask_set_cpu(cpu, policy->cpus);
-               cpumask_copy(policy->related_cpus, cpu_sibling_mask(cpu));
                policy->shared_type = CPUFREQ_SHARED_TYPE_HW;
                pr_info_once(PFX "overriding BIOS provided _PSD data\n");

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Lan Tianyu]

Hi Viresh
	Thanks for your review.

On 2013年06月25日 11:56, Viresh Kumar wrote:
> On 25 June 2013 07:36, Lan Tianyu <tianyu.lan@intel.com> wrote:
>> diff --git a/Documentation/cpu-freq/user-guide.txt b/Documentation/cpu-freq/user-guide.txt
>> index ff2f283..0cc72f7 100644
>> --- a/Documentation/cpu-freq/user-guide.txt
>> +++ b/Documentation/cpu-freq/user-guide.txt
>> @@ -196,6 +196,10 @@ affected_cpus :                    List of Online CPUs that require software
>>  related_cpus :                 List of Online + Offline CPUs that need software
>>                                 coordination of frequency.
>>
>> +freqdomain_cpus :              List of Online + Offline CPUs in same CPU dependency
>> +                               domain. (This is only available for acpi-cpufreq
>> +                               driver)
>> +
> 
> This is generic file, don't add this information here. Add this in
> acpi-cpufreq file.
I don't find acpi-cpufreq.txt under
Documentation/cpu-freq/acpi-cpufreq.txt. So I should create it?

> 
>>  scaling_driver :               Hardware driver for cpufreq.
>>
>>  scaling_cur_freq :             Current frequency of the CPU as determined by
>> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
>> index 17e3496..b859997 100644
>> --- a/drivers/cpufreq/acpi-cpufreq.c
>> +++ b/drivers/cpufreq/acpi-cpufreq.c
>> @@ -176,6 +176,16 @@ static struct global_attr global_boost = __ATTR(boost, 0644,
>>                                                 show_global_boost,
>>                                                 store_global_boost);
>>
>> +static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>> +{
>> +       struct acpi_cpufreq_data *data = per_cpu(acfreq_data, policy->cpu);
>> +       struct acpi_processor_performance *perf = data->acpi_data;
>> +
>> +       return cpufreq_show_cpus(perf->shared_cpu_map, buf);
>> +}
> 
> I am not sure if this is enough. Check this commit:
> 
> aa77a52764a92216b61a6c8079b5c01937c046cd
> 
> It had these changes:

Please see the acpi_processor_preregister_performance() in the
drivers/acpi/processor_perlib.c. All cpus in the same dependency domain
are stored in the perf->shared_cpu_map(including the reference cpu the
perf belongs to). Original code will copy shared_cpu_map to
policy->related_cpus, do cpumask_or between policy->related_cpus and
policy->cpus in the cpufreq_add_dev() and store the result into
policy->related_cpus. Expose the data via related_cpus.

For acpi-cpufreq driver, the shared_cpu_map is the biggest subset since
it regardless the coordination type.

After the commit aa77a5, only the cpus in the software or
software&hardware coordination type dependency domain will copy to
policy->cpus and finally store into policy->related_cpus in the
cpufreq_add_dev() by cpumask_or. And then we miss the cpus in the
hardware coordination type domain. But these info is still stored in the
policy->shared_cpu_map. Does this make sense?

> 
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 937bc28..57a8774 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -730,7 +730,6 @@ static int acpi_cpufreq_cpu_init(struct
> cpufreq_policy *policy)
>             policy->shared_type == CPUFREQ_SHARED_TYPE_ANY) {
>                 cpumask_copy(policy->cpus, perf->shared_cpu_map);
>         }
> -       cpumask_copy(policy->related_cpus, perf->shared_cpu_map);
> 
>  #ifdef CONFIG_SMP
>         dmi_check_system(sw_any_bug_dmi_table);
> @@ -742,7 +741,6 @@ static int acpi_cpufreq_cpu_init(struct
> cpufreq_policy *policy)
>         if (check_amd_hwpstate_cpu(cpu) && !acpi_pstate_strict) {
>                 cpumask_clear(policy->cpus);
>                 cpumask_set_cpu(cpu, policy->cpus);
> -               cpumask_copy(policy->related_cpus, cpu_sibling_mask(cpu));
>                 policy->shared_type = CPUFREQ_SHARED_TYPE_HW;
>                 pr_info_once(PFX "overriding BIOS provided _PSD data\n");
> 


-- 
Best regards
Tianyu Lan

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Viresh Kumar]

On 25 June 2013 12:24, Lan Tianyu <tianyu.lan@intel.com> wrote:
> On 2013年06月25日 11:56, Viresh Kumar wrote:

>> This is generic file, don't add this information here. Add this in
>> acpi-cpufreq file.
> I don't find acpi-cpufreq.txt under
> Documentation/cpu-freq/acpi-cpufreq.txt. So I should create it?

I meant acpi-cpufreq.c file

> Please see the acpi_processor_preregister_performance() in the
> drivers/acpi/processor_perlib.c. All cpus in the same dependency domain
> are stored in the perf->shared_cpu_map(including the reference cpu the
> perf belongs to). Original code will copy shared_cpu_map to
> policy->related_cpus, do cpumask_or between policy->related_cpus and
> policy->cpus in the cpufreq_add_dev() and store the result into
> policy->related_cpus. Expose the data via related_cpus.
>
> For acpi-cpufreq driver, the shared_cpu_map is the biggest subset since
> it regardless the coordination type.
>
> After the commit aa77a5, only the cpus in the software or
> software&hardware coordination type dependency domain will copy to
> policy->cpus and finally store into policy->related_cpus in the
> cpufreq_add_dev() by cpumask_or. And then we miss the cpus in the
> hardware coordination type domain. But these info is still stored in the
> policy->shared_cpu_map. Does this make sense?

I was concerned about this code that was present earlier. That changes
related_cpus based on some conditions.

        if (check_amd_hwpstate_cpu(cpu) && !acpi_pstate_strict) {
                cpumask_clear(policy->cpus);
                cpumask_set_cpu(cpu, policy->cpus);
                cpumask_copy(policy->related_cpus, cpu_sibling_mask(cpu));
                policy->shared_type = CPUFREQ_SHARED_TYPE_HW;
                pr_info_once(PFX "overriding BIOS provided _PSD data\n");
        }

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Lan Tianyu]

On 2013年06月25日 15:48, Viresh Kumar wrote:
> On 25 June 2013 12:24, Lan Tianyu <tianyu.lan@intel.com> wrote:
>> On 2013年06月25日 11:56, Viresh Kumar wrote:
> 
>>> This is generic file, don't add this information here. Add this in
>>> acpi-cpufreq file.
>> I don't find acpi-cpufreq.txt under
>> Documentation/cpu-freq/acpi-cpufreq.txt. So I should create it?
> 
> I meant acpi-cpufreq.c file
Ok. From my opinion, the new attribute is an ABI and it's better to add
descriptor under Document directory. The user can be easy to find how to
use it.

> 
>> Please see the acpi_processor_preregister_performance() in the
>> drivers/acpi/processor_perlib.c. All cpus in the same dependency domain
>> are stored in the perf->shared_cpu_map(including the reference cpu the
>> perf belongs to). Original code will copy shared_cpu_map to
>> policy->related_cpus, do cpumask_or between policy->related_cpus and
>> policy->cpus in the cpufreq_add_dev() and store the result into
>> policy->related_cpus. Expose the data via related_cpus.
>>
>> For acpi-cpufreq driver, the shared_cpu_map is the biggest subset since
>> it regardless the coordination type.
>>
>> After the commit aa77a5, only the cpus in the software or
>> software&hardware coordination type dependency domain will copy to
>> policy->cpus and finally store into policy->related_cpus in the
>> cpufreq_add_dev() by cpumask_or. And then we miss the cpus in the
>> hardware coordination type domain. But these info is still stored in the
>> policy->shared_cpu_map. Does this make sense?
> 
> I was concerned about this code that was present earlier. That changes
> related_cpus based on some conditions.

Please see the commit which add the code. Maybe, we should overwrite
shared_cpu_map by sibling_cpus for this case?

commit acd316248205d553594296f1895ba5196b89ffcc
Author: Andre Przywara <andre.przywara@amd.com>
Date:   Tue Sep 4 08:28:03 2012 +0000

    acpi-cpufreq: Add quirk to disable _PSD usage on all AMD CPUs

    To workaround some Windows specific behavior, the ACPI _PSD table
    on AMD desktop boards advertises all cores as dependent, meaning
    that they all can only use the same P-state. acpi-cpufreq strictly
    obeys this description, instantiating one CPU only and symlinking
    the others. But the hardware can have distinct frequencies for each
    core and powernow-k8 did it that way.
    So, in order to use the hardware to its full potential and keep the
    original powernow-k8 behavior, lets override the _PSD table setting
    on AMD hardware.
    We use the siblings table, as it matches the current hardware
    behavior.

    Signed-off-by: Andre Przywara <andre.przywara@amd.com>
    Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>


> 
>         if (check_amd_hwpstate_cpu(cpu) && !acpi_pstate_strict) {
>                 cpumask_clear(policy->cpus);
>                 cpumask_set_cpu(cpu, policy->cpus);
>                 cpumask_copy(policy->related_cpus, cpu_sibling_mask(cpu));
>                 policy->shared_type = CPUFREQ_SHARED_TYPE_HW;
>                 pr_info_once(PFX "overriding BIOS provided _PSD data\n");
>         }
> 


-- 
Best regards
Tianyu Lan

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Viresh Kumar]

On 25 June 2013 13:49, Lan Tianyu <tianyu.lan@intel.com> wrote:
> Ok. From my opinion, the new attribute is an ABI and it's better to add
> descriptor under Document directory. The user can be easy to find how to
> use it.

Hmm.. So maybe acpi-cpufreq file would be a good starting point. Then
it can have more details about the driver in future.

> Please see the commit which add the code. Maybe, we should overwrite
> shared_cpu_map by sibling_cpus for this case?

Not sure if changing shared_cpu_map has any other implications or not.

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Rafael J. Wysocki]

On Tuesday, June 25, 2013 09:01:03 PM Viresh Kumar wrote:
> On 25 June 2013 13:49, Lan Tianyu <tianyu.lan@intel.com> wrote:
> > Ok. From my opinion, the new attribute is an ABI and it's better to add
> > descriptor under Document directory. The user can be easy to find how to
> > use it.
> 
> Hmm.. So maybe acpi-cpufreq file would be a good starting point. Then
> it can have more details about the driver in future.
> 
> > Please see the commit which add the code. Maybe, we should overwrite
> > shared_cpu_map by sibling_cpus for this case?
> 
> Not sure if changing shared_cpu_map has any other implications or not.

Well, I wouldn't change it, then.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Lan Tianyu]

On 2013年06月26日 07:03, Rafael J. Wysocki wrote:
> On Tuesday, June 25, 2013 09:01:03 PM Viresh Kumar wrote:
>> On 25 June 2013 13:49, Lan Tianyu <tianyu.lan@intel.com> wrote:
>>> Ok. From my opinion, the new attribute is an ABI and it's better to add
>>> descriptor under Document directory. The user can be easy to find how to
>>> use it.
>>
>> Hmm.. So maybe acpi-cpufreq file would be a good starting point. Then
>> it can have more details about the driver in future.
>>
>>> Please see the commit which add the code. Maybe, we should overwrite
>>> shared_cpu_map by sibling_cpus for this case?
>>
>> Not sure if changing shared_cpu_map has any other implications or not.
> 
> Well, I wouldn't change it, then.
Ok. How about add new field "cpufreqdomain_cpus" in the  struct
acpi_cpufreq_data and expose its value for new attribute. For normal
case, copy the shared_cpu_map to it for normal case. For AMD case, copy
sibling_cpus to it.
   Another choice, just keeping what my patch has done. Wait for the new
request since current patch can satisfy reporter and it's not clear
whether the patch is enough for AMD platform.(At last from my view).

> 
> Thanks,
> Rafael
> 
> 


-- 
Best regards
Tianyu Lan

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Viresh Kumar]

On 26 June 2013 08:11, Lan Tianyu <tianyu.lan@intel.com> wrote:
> On 2013年06月26日 07:03, Rafael J. Wysocki wrote:
>> On Tuesday, June 25, 2013 09:01:03 PM Viresh Kumar wrote:
>>> On 25 June 2013 13:49, Lan Tianyu <tianyu.lan@intel.com> wrote:
>>>> Ok. From my opinion, the new attribute is an ABI and it's better to add
>>>> descriptor under Document directory. The user can be easy to find how to
>>>> use it.
>>>
>>> Hmm.. So maybe acpi-cpufreq file would be a good starting point. Then
>>> it can have more details about the driver in future.
>>>
>>>> Please see the commit which add the code. Maybe, we should overwrite
>>>> shared_cpu_map by sibling_cpus for this case?
>>>
>>> Not sure if changing shared_cpu_map has any other implications or not.
>>
>> Well, I wouldn't change it, then.

Please add a blank line before and after your reply. It makes it more
readable.

> Ok. How about add new field "cpufreqdomain_cpus" in the  struct
> acpi_cpufreq_data and expose its value for new attribute. For normal
> case, copy the shared_cpu_map to it for normal case. For AMD case, copy
> sibling_cpus to it.

This should work I guess.

>    Another choice, just keeping what my patch has done. Wait for the new
> request since current patch can satisfy reporter and it's not clear
> whether the patch is enough for AMD platform.(At last from my view).

NAK. We must keep it as it was before my patch.

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Lan Tianyu]

On 2013年06月26日 14:54, Viresh Kumar wrote:
> On 26 June 2013 08:11, Lan Tianyu <tianyu.lan@intel.com> wrote:
>> On 2013年06月26日 07:03, Rafael J. Wysocki wrote:
>>> On Tuesday, June 25, 2013 09:01:03 PM Viresh Kumar wrote:
>>>> On 25 June 2013 13:49, Lan Tianyu <tianyu.lan@intel.com> wrote:
>>>>> Ok. From my opinion, the new attribute is an ABI and it's better to add
>>>>> descriptor under Document directory. The user can be easy to find how to
>>>>> use it.
>>>>
>>>> Hmm.. So maybe acpi-cpufreq file would be a good starting point. Then
>>>> it can have more details about the driver in future.
>>>>
>>>>> Please see the commit which add the code. Maybe, we should overwrite
>>>>> shared_cpu_map by sibling_cpus for this case?
>>>>
>>>> Not sure if changing shared_cpu_map has any other implications or not.
>>>
>>> Well, I wouldn't change it, then.
> 
> Please add a blank line before and after your reply. It makes it more
> readable.

Thanks for reminder. I will notice this.

> 
>> Ok. How about add new field "cpufreqdomain_cpus" in the  struct
>> acpi_cpufreq_data and expose its value for new attribute. For normal
>> case, copy the shared_cpu_map to it for normal case. For AMD case, copy
>> sibling_cpus to it.
> 
> This should work I guess.

Ok. I will follow this one.

> 
>>    Another choice, just keeping what my patch has done. Wait for the new
>> request since current patch can satisfy reporter and it's not clear
>> whether the patch is enough for AMD platform.(At last from my view).
> 
> NAK. We must keep it as it was before my patch.
> 

OK. Please ignore this one.
-- 
Best regards
Tianyu Lan

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Rafael J. Wysocki]

On Tuesday, June 25, 2013 04:19:14 PM Lan Tianyu wrote:
> On 2013年06月25日 15:48, Viresh Kumar wrote:
> > On 25 June 2013 12:24, Lan Tianyu <tianyu.lan@intel.com> wrote:
> >> On 2013年06月25日 11:56, Viresh Kumar wrote:
> > 
> >>> This is generic file, don't add this information here. Add this in
> >>> acpi-cpufreq file.
> >> I don't find acpi-cpufreq.txt under
> >> Documentation/cpu-freq/acpi-cpufreq.txt. So I should create it?
> > 
> > I meant acpi-cpufreq.c file
> Ok. From my opinion, the new attribute is an ABI and it's better to add
> descriptor under Document directory. The user can be easy to find how to
> use it.

Yes, this should be documented under Documentation/ABI/testing/.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 2/2] CPUFreq: Add new sysfs attribute freqdomain_cpus for acpi-freq driver

[Lan Tianyu]

On 2013年06月26日 07:02, Rafael J. Wysocki wrote:
> On Tuesday, June 25, 2013 04:19:14 PM Lan Tianyu wrote:
>> On 2013年06月25日 15:48, Viresh Kumar wrote:
>>> On 25 June 2013 12:24, Lan Tianyu <tianyu.lan@intel.com> wrote:
>>>> On 2013年06月25日 11:56, Viresh Kumar wrote:
>>>
>>>>> This is generic file, don't add this information here. Add this in
>>>>> acpi-cpufreq file.
>>>> I don't find acpi-cpufreq.txt under
>>>> Documentation/cpu-freq/acpi-cpufreq.txt. So I should create it?
>>>
>>> I meant acpi-cpufreq.c file
>> Ok. From my opinion, the new attribute is an ABI and it's better to add
>> descriptor under Document directory. The user can be easy to find how to
>> use it.
> 
> Yes, this should be documented under Documentation/ABI/testing/.

I find the following descriptor in the
Documentation/ABI/testing/sysfs-devices-system-cpu. So I originally put
the new attribute descriptor in the user-guide.txt. Now, creating new
acpi-cpufreq file under Documentation/cpu-freq/ and adding the
descriptor in the new file maybe a good choice?


What:           /sys/devices/system/cpu/cpu#/cpufreq/*
Date:           pre-git history
Contact:        cpufreq@vger.kernel.org
Description:    Discover and change clock speed of CPUs

                Clock scaling allows you to change the clock speed of the
                CPUs on the fly. This is a nice method to save battery
                power, because the lower the clock speed, the less power
                the CPU consumes.

                There are many knobs to tweak in this directory.

                See files in Documentation/cpu-freq/ for more information.

                In particular, read Documentation/cpu-freq/user-guide.txt
                to learn how to control the knobs.


> 
> Thanks,
> Rafael
> 
> 


-- 
Best regards
Tianyu Lan
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] ACPI/Processor: Clear unuseful variable count in the acpi_processor_preregister_performance()

[Viresh Kumar]

On 25 June 2013 07:36, Lan Tianyu <tianyu.lan@intel.com> wrote:
> The variable count in the acpi_processor_preregister_performance() is only initalized
> as one for one cpu and increased by one when find another cpu that share same dependency
> domain. Otherwise, it isn't referenced somewhere else.
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  drivers/acpi/processor_perflib.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH 1/2] ACPI/Processor: Clear unuseful variable count in the acpi_processor_preregister_performance()

[Lan Tianyu]

On 2013年06月25日 15:45, Viresh Kumar wrote:
> On 25 June 2013 07:36, Lan Tianyu <tianyu.lan@intel.com> wrote:
>> The variable count in the acpi_processor_preregister_performance() is only initalized
>> as one for one cpu and increased by one when find another cpu that share same dependency
>> domain. Otherwise, it isn't referenced somewhere else.
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>>  drivers/acpi/processor_perflib.c |    4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> 
Thanks.


-- 
Best regards
Tianyu Lan
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] ACPI/Processor: Clear unuseful variable count in the acpi_processor_preregister_performance()

[Rafael J. Wysocki]

On Tuesday, June 25, 2013 04:42:09 PM Lan Tianyu wrote:
> On 2013年06月25日 15:45, Viresh Kumar wrote:
> > On 25 June 2013 07:36, Lan Tianyu <tianyu.lan@intel.com> wrote:
> >> The variable count in the acpi_processor_preregister_performance() is only initalized
> >> as one for one cpu and increased by one when find another cpu that share same dependency
> >> domain. Otherwise, it isn't referenced somewhere else.
> >>
> >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> >> ---
> >>  drivers/acpi/processor_perflib.c |    4 +---
> >>  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > 
> Thanks.

Queued up for 3.11.

Thanks,
Rafael