Re: [RFC PATCH] cpufreq: Introduce macros for cpufreq_frequency_table iteration

2014-04-11 Thread Stratos Karafotis
On 11/04/2014 07:24 πμ, Viresh Kumar wrote:
> On 11 April 2014 03:31, Stratos Karafotis  wrote:
>> This patch intoduces 2 macros which can be used for iteration
>> over cpufreq_frequency_table:
>>
>> - cpufreq_for_each_entry: iterate over each entry of the table
>> - cpufreq_for_each_valid_entry: iterate over each entry of the
>> table that contains a valid frequency.
>>
>> It should have no functional changes.
>>
>> Signed-off-by: Stratos Karafotis 
>> ---
>>
>> I found about 20 occurrences in various drivers that these macros
>> can be used. I will proceed with the necessary changes if you
>> approve something like this.
>>
>> Thanks in advance for your time,
> 
> Thanks for your time and I find it useful enough. So its a yes from
> me :)
> 
> But, have you tested this yet?
> 
>> Stratos Karafotis.
>> ---
>>
>>  drivers/cpufreq/freq_table.c | 54 
>> 
>>  include/linux/cpufreq.h  | 29 
>>  2 files changed, 53 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
>> index 08e7bbc..19bf0c4 100644
>> --- a/drivers/cpufreq/freq_table.c
>> +++ b/drivers/cpufreq/freq_table.c
>> @@ -21,22 +21,18 @@
>>  int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
>> struct cpufreq_frequency_table *table)
>>  {
>> +   struct cpufreq_frequency_table *pos;
>> unsigned int min_freq = ~0;
>> unsigned int max_freq = 0;
>> -   unsigned int i;
>>
>> -   for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
>> -   unsigned int freq = table[i].frequency;
>> -   if (freq == CPUFREQ_ENTRY_INVALID) {
>> -   pr_debug("table entry %u is invalid, skipping\n", i);
>> +   cpufreq_for_each_valid_entry(pos, table) {
>> +   unsigned int freq = pos->frequency;
> 
> move the definition part to the top of this routine, we don't need
> to create freq for every iteration here. It was bad earlier as well :)
> 
>>
>> -   continue;
>> -   }
>> if (!cpufreq_boost_enabled()
>> -   && (table[i].flags & CPUFREQ_BOOST_FREQ))
>> +   && (pos->flags & CPUFREQ_BOOST_FREQ))
>> continue;
>>
>> -   pr_debug("table entry %u: %u kHz\n", i, freq);
>> +   pr_debug("table entry %lu: %u kHz\n", pos - table, freq);
>> if (freq < min_freq)
>> min_freq = freq;
>> if (freq > max_freq)
>> @@ -57,7 +53,8 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_cpuinfo);
>>  int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
>>struct cpufreq_frequency_table *table)
>>  {
>> -   unsigned int next_larger = ~0, freq, i = 0;
>> +   struct cpufreq_frequency_table *pos;
>> +   unsigned int next_larger = ~0, freq;
>> bool found = false;
>>
>> pr_debug("request for verification of policy (%u - %u kHz) for cpu 
>> %u\n",
>> @@ -65,9 +62,9 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy 
>> *policy,
>>
>> cpufreq_verify_within_cpu_limits(policy);
>>
>> -   for (; freq = table[i].frequency, freq != CPUFREQ_TABLE_END; i++) {
>> -   if (freq == CPUFREQ_ENTRY_INVALID)
>> -   continue;
>> +   cpufreq_for_each_valid_entry(pos, table) {
>> +   freq = pos->frequency;
>> +
>> if ((freq >= policy->min) && (freq <= policy->max)) {
>> found = true;
>> break;
>> @@ -118,7 +115,8 @@ int cpufreq_frequency_table_target(struct cpufreq_policy 
>> *policy,
>> .driver_data = ~0,
>> .frequency = 0,
>> };
>> -   unsigned int i;
>> +   struct cpufreq_frequency_table *pos;
>> +   unsigned int i = 0;
>>
>> pr_debug("request for target %u kHz (relation: %u) for cpu %u\n",
>> target_freq, relation, policy->cpu);
>> @@ -132,10 +130,10 @@ int cpufreq_frequency_table_target(struct 
>> cpufreq_policy *policy,
>> break;
>> }
>>
>> -   for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
>> -   unsigned int freq = table[i].frequency;
>> -   if (freq == CPUFREQ_ENTRY_INVALID)
>> -   continue;
>> +   cpufreq_for_each_valid_entry(pos, table) {
>> +   unsigned int freq = pos->frequency;
> 
> same here.
> 
>> +
>> +   i = pos - table;
>> if ((freq < policy->min) || (freq > policy->max))
>> continue;
>> switch (relation) {
>> @@ -184,8 +182,7 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_target);
>>  int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
>> unsigned int freq)
>>  {
>> 

Re: [RFC PATCH] cpufreq: Introduce macros for cpufreq_frequency_table iteration

2014-04-11 Thread Stratos Karafotis
On 11/04/2014 07:24 πμ, Viresh Kumar wrote:
 On 11 April 2014 03:31, Stratos Karafotis strat...@semaphore.gr wrote:
 This patch intoduces 2 macros which can be used for iteration
 over cpufreq_frequency_table:

 - cpufreq_for_each_entry: iterate over each entry of the table
 - cpufreq_for_each_valid_entry: iterate over each entry of the
 table that contains a valid frequency.

 It should have no functional changes.

 Signed-off-by: Stratos Karafotis strat...@semaphore.gr
 ---

 I found about 20 occurrences in various drivers that these macros
 can be used. I will proceed with the necessary changes if you
 approve something like this.

 Thanks in advance for your time,
 
 Thanks for your time and I find it useful enough. So its a yes from
 me :)
 
 But, have you tested this yet?
 
 Stratos Karafotis.
 ---

  drivers/cpufreq/freq_table.c | 54 
 
  include/linux/cpufreq.h  | 29 
  2 files changed, 53 insertions(+), 30 deletions(-)

 diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
 index 08e7bbc..19bf0c4 100644
 --- a/drivers/cpufreq/freq_table.c
 +++ b/drivers/cpufreq/freq_table.c
 @@ -21,22 +21,18 @@
  int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
 struct cpufreq_frequency_table *table)
  {
 +   struct cpufreq_frequency_table *pos;
 unsigned int min_freq = ~0;
 unsigned int max_freq = 0;
 -   unsigned int i;

 -   for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
 -   unsigned int freq = table[i].frequency;
 -   if (freq == CPUFREQ_ENTRY_INVALID) {
 -   pr_debug(table entry %u is invalid, skipping\n, i);
 +   cpufreq_for_each_valid_entry(pos, table) {
 +   unsigned int freq = pos-frequency;
 
 move the definition part to the top of this routine, we don't need
 to create freq for every iteration here. It was bad earlier as well :)
 

 -   continue;
 -   }
 if (!cpufreq_boost_enabled()
 -(table[i].flags  CPUFREQ_BOOST_FREQ))
 +(pos-flags  CPUFREQ_BOOST_FREQ))
 continue;

 -   pr_debug(table entry %u: %u kHz\n, i, freq);
 +   pr_debug(table entry %lu: %u kHz\n, pos - table, freq);
 if (freq  min_freq)
 min_freq = freq;
 if (freq  max_freq)
 @@ -57,7 +53,8 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_cpuinfo);
  int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *table)
  {
 -   unsigned int next_larger = ~0, freq, i = 0;
 +   struct cpufreq_frequency_table *pos;
 +   unsigned int next_larger = ~0, freq;
 bool found = false;

 pr_debug(request for verification of policy (%u - %u kHz) for cpu 
 %u\n,
 @@ -65,9 +62,9 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy 
 *policy,

 cpufreq_verify_within_cpu_limits(policy);

 -   for (; freq = table[i].frequency, freq != CPUFREQ_TABLE_END; i++) {
 -   if (freq == CPUFREQ_ENTRY_INVALID)
 -   continue;
 +   cpufreq_for_each_valid_entry(pos, table) {
 +   freq = pos-frequency;
 +
 if ((freq = policy-min)  (freq = policy-max)) {
 found = true;
 break;
 @@ -118,7 +115,8 @@ int cpufreq_frequency_table_target(struct cpufreq_policy 
 *policy,
 .driver_data = ~0,
 .frequency = 0,
 };
 -   unsigned int i;
 +   struct cpufreq_frequency_table *pos;
 +   unsigned int i = 0;

 pr_debug(request for target %u kHz (relation: %u) for cpu %u\n,
 target_freq, relation, policy-cpu);
 @@ -132,10 +130,10 @@ int cpufreq_frequency_table_target(struct 
 cpufreq_policy *policy,
 break;
 }

 -   for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
 -   unsigned int freq = table[i].frequency;
 -   if (freq == CPUFREQ_ENTRY_INVALID)
 -   continue;
 +   cpufreq_for_each_valid_entry(pos, table) {
 +   unsigned int freq = pos-frequency;
 
 same here.
 
 +
 +   i = pos - table;
 if ((freq  policy-min) || (freq  policy-max))
 continue;
 switch (relation) {
 @@ -184,8 +182,7 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_target);
  int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
 unsigned int freq)
  {
 -   struct cpufreq_frequency_table *table;
 -   int i;
 +   struct cpufreq_frequency_table *pos, *table;

 table = cpufreq_frequency_get_table(policy-cpu);
 if (unlikely(!table)) {
 @@ -193,9 +190,9 @@ 

Re: [RFC PATCH] cpufreq: Introduce macros for cpufreq_frequency_table iteration

2014-04-10 Thread Viresh Kumar
On 11 April 2014 03:31, Stratos Karafotis  wrote:
> This patch intoduces 2 macros which can be used for iteration
> over cpufreq_frequency_table:
>
> - cpufreq_for_each_entry: iterate over each entry of the table
> - cpufreq_for_each_valid_entry: iterate over each entry of the
> table that contains a valid frequency.
>
> It should have no functional changes.
>
> Signed-off-by: Stratos Karafotis 
> ---
>
> I found about 20 occurrences in various drivers that these macros
> can be used. I will proceed with the necessary changes if you
> approve something like this.
>
> Thanks in advance for your time,

Thanks for your time and I find it useful enough. So its a yes from
me :)

But, have you tested this yet?

> Stratos Karafotis.
> ---
>
>  drivers/cpufreq/freq_table.c | 54 
> 
>  include/linux/cpufreq.h  | 29 
>  2 files changed, 53 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
> index 08e7bbc..19bf0c4 100644
> --- a/drivers/cpufreq/freq_table.c
> +++ b/drivers/cpufreq/freq_table.c
> @@ -21,22 +21,18 @@
>  int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
> struct cpufreq_frequency_table *table)
>  {
> +   struct cpufreq_frequency_table *pos;
> unsigned int min_freq = ~0;
> unsigned int max_freq = 0;
> -   unsigned int i;
>
> -   for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
> -   unsigned int freq = table[i].frequency;
> -   if (freq == CPUFREQ_ENTRY_INVALID) {
> -   pr_debug("table entry %u is invalid, skipping\n", i);
> +   cpufreq_for_each_valid_entry(pos, table) {
> +   unsigned int freq = pos->frequency;

move the definition part to the top of this routine, we don't need
to create freq for every iteration here. It was bad earlier as well :)

>
> -   continue;
> -   }
> if (!cpufreq_boost_enabled()
> -   && (table[i].flags & CPUFREQ_BOOST_FREQ))
> +   && (pos->flags & CPUFREQ_BOOST_FREQ))
> continue;
>
> -   pr_debug("table entry %u: %u kHz\n", i, freq);
> +   pr_debug("table entry %lu: %u kHz\n", pos - table, freq);
> if (freq < min_freq)
> min_freq = freq;
> if (freq > max_freq)
> @@ -57,7 +53,8 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_cpuinfo);
>  int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
>struct cpufreq_frequency_table *table)
>  {
> -   unsigned int next_larger = ~0, freq, i = 0;
> +   struct cpufreq_frequency_table *pos;
> +   unsigned int next_larger = ~0, freq;
> bool found = false;
>
> pr_debug("request for verification of policy (%u - %u kHz) for cpu 
> %u\n",
> @@ -65,9 +62,9 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy 
> *policy,
>
> cpufreq_verify_within_cpu_limits(policy);
>
> -   for (; freq = table[i].frequency, freq != CPUFREQ_TABLE_END; i++) {
> -   if (freq == CPUFREQ_ENTRY_INVALID)
> -   continue;
> +   cpufreq_for_each_valid_entry(pos, table) {
> +   freq = pos->frequency;
> +
> if ((freq >= policy->min) && (freq <= policy->max)) {
> found = true;
> break;
> @@ -118,7 +115,8 @@ int cpufreq_frequency_table_target(struct cpufreq_policy 
> *policy,
> .driver_data = ~0,
> .frequency = 0,
> };
> -   unsigned int i;
> +   struct cpufreq_frequency_table *pos;
> +   unsigned int i = 0;
>
> pr_debug("request for target %u kHz (relation: %u) for cpu %u\n",
> target_freq, relation, policy->cpu);
> @@ -132,10 +130,10 @@ int cpufreq_frequency_table_target(struct 
> cpufreq_policy *policy,
> break;
> }
>
> -   for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
> -   unsigned int freq = table[i].frequency;
> -   if (freq == CPUFREQ_ENTRY_INVALID)
> -   continue;
> +   cpufreq_for_each_valid_entry(pos, table) {
> +   unsigned int freq = pos->frequency;

same here.

> +
> +   i = pos - table;
> if ((freq < policy->min) || (freq > policy->max))
> continue;
> switch (relation) {
> @@ -184,8 +182,7 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_target);
>  int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
> unsigned int freq)
>  {
> -   struct cpufreq_frequency_table *table;
> -   int i;
> +   struct cpufreq_frequency_table *pos, *table;
>
> table = cpufreq_frequency_get_table(policy->cpu);
>

Re: [RFC PATCH] cpufreq: Introduce macros for cpufreq_frequency_table iteration

2014-04-10 Thread Viresh Kumar
On 11 April 2014 03:31, Stratos Karafotis strat...@semaphore.gr wrote:
 This patch intoduces 2 macros which can be used for iteration
 over cpufreq_frequency_table:

 - cpufreq_for_each_entry: iterate over each entry of the table
 - cpufreq_for_each_valid_entry: iterate over each entry of the
 table that contains a valid frequency.

 It should have no functional changes.

 Signed-off-by: Stratos Karafotis strat...@semaphore.gr
 ---

 I found about 20 occurrences in various drivers that these macros
 can be used. I will proceed with the necessary changes if you
 approve something like this.

 Thanks in advance for your time,

Thanks for your time and I find it useful enough. So its a yes from
me :)

But, have you tested this yet?

 Stratos Karafotis.
 ---

  drivers/cpufreq/freq_table.c | 54 
 
  include/linux/cpufreq.h  | 29 
  2 files changed, 53 insertions(+), 30 deletions(-)

 diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
 index 08e7bbc..19bf0c4 100644
 --- a/drivers/cpufreq/freq_table.c
 +++ b/drivers/cpufreq/freq_table.c
 @@ -21,22 +21,18 @@
  int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
 struct cpufreq_frequency_table *table)
  {
 +   struct cpufreq_frequency_table *pos;
 unsigned int min_freq = ~0;
 unsigned int max_freq = 0;
 -   unsigned int i;

 -   for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
 -   unsigned int freq = table[i].frequency;
 -   if (freq == CPUFREQ_ENTRY_INVALID) {
 -   pr_debug(table entry %u is invalid, skipping\n, i);
 +   cpufreq_for_each_valid_entry(pos, table) {
 +   unsigned int freq = pos-frequency;

move the definition part to the top of this routine, we don't need
to create freq for every iteration here. It was bad earlier as well :)


 -   continue;
 -   }
 if (!cpufreq_boost_enabled()
 -(table[i].flags  CPUFREQ_BOOST_FREQ))
 +(pos-flags  CPUFREQ_BOOST_FREQ))
 continue;

 -   pr_debug(table entry %u: %u kHz\n, i, freq);
 +   pr_debug(table entry %lu: %u kHz\n, pos - table, freq);
 if (freq  min_freq)
 min_freq = freq;
 if (freq  max_freq)
 @@ -57,7 +53,8 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_cpuinfo);
  int cpufreq_frequency_table_verify(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *table)
  {
 -   unsigned int next_larger = ~0, freq, i = 0;
 +   struct cpufreq_frequency_table *pos;
 +   unsigned int next_larger = ~0, freq;
 bool found = false;

 pr_debug(request for verification of policy (%u - %u kHz) for cpu 
 %u\n,
 @@ -65,9 +62,9 @@ int cpufreq_frequency_table_verify(struct cpufreq_policy 
 *policy,

 cpufreq_verify_within_cpu_limits(policy);

 -   for (; freq = table[i].frequency, freq != CPUFREQ_TABLE_END; i++) {
 -   if (freq == CPUFREQ_ENTRY_INVALID)
 -   continue;
 +   cpufreq_for_each_valid_entry(pos, table) {
 +   freq = pos-frequency;
 +
 if ((freq = policy-min)  (freq = policy-max)) {
 found = true;
 break;
 @@ -118,7 +115,8 @@ int cpufreq_frequency_table_target(struct cpufreq_policy 
 *policy,
 .driver_data = ~0,
 .frequency = 0,
 };
 -   unsigned int i;
 +   struct cpufreq_frequency_table *pos;
 +   unsigned int i = 0;

 pr_debug(request for target %u kHz (relation: %u) for cpu %u\n,
 target_freq, relation, policy-cpu);
 @@ -132,10 +130,10 @@ int cpufreq_frequency_table_target(struct 
 cpufreq_policy *policy,
 break;
 }

 -   for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
 -   unsigned int freq = table[i].frequency;
 -   if (freq == CPUFREQ_ENTRY_INVALID)
 -   continue;
 +   cpufreq_for_each_valid_entry(pos, table) {
 +   unsigned int freq = pos-frequency;

same here.

 +
 +   i = pos - table;
 if ((freq  policy-min) || (freq  policy-max))
 continue;
 switch (relation) {
 @@ -184,8 +182,7 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_target);
  int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
 unsigned int freq)
  {
 -   struct cpufreq_frequency_table *table;
 -   int i;
 +   struct cpufreq_frequency_table *pos, *table;

 table = cpufreq_frequency_get_table(policy-cpu);
 if (unlikely(!table)) {
 @@ -193,9 +190,9 @@ int cpufreq_frequency_table_get_index(struct