Re: [RFC PATCH] cpufreq: Introduce macros for cpufreq_frequency_table iteration
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
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
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
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