cpufreq: Introduce macros for cpufreq_frequency_table iteration

RFC PATCH Superseded Stratos Karafotis 2 participants 3 messages Apr 10, 2014 – Apr 11, 2014 CPU frequency scaling framework

lore

Share & tools

lore

Stratos Karafotis

author

2014-04-10 22:01 UTC #0 lore raw

RFC PATCH 1/1 cpufreq: Introduce macros for cpufreq_frequency_table iteration

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 <[email protected]>

---

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,
Stratos Karafotis.
---

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

drivers/cpufreq/freq_table.c +24 ~~-30~~

index 08e7bbc..19bf0c4 100644
@@ -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;
 
-			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;
+
+		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 cpufreq_policy *policy,
 		return -ENOENT;
 	}
 
-	for (i = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
-		if (table[i].frequency == freq)
-			return i;
+	cpufreq_for_each_entry(pos, table) {
+		if (pos->frequency == freq)
+			return pos - table;
 	}
 
 	return -EINVAL;

@@ -208,16 +205,13 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_get_index);
 static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
 				    bool show_boost)
 {
-	unsigned int i = 0;
 	ssize_t count = 0;
-	struct cpufreq_frequency_table *table = policy->freq_table;
+	struct cpufreq_frequency_table *pos, *table = policy->freq_table;
 
 	if (!table)
 		return -ENODEV;
 
-	for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
-		if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
-			continue;
+	cpufreq_for_each_valid_entry(pos, table) {
 		/*
 		 * show_boost = true and driver_data = BOOST freq
 		 * display BOOST freqs

@@ -229,10 +223,10 @@ static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
 		 * show_boost = false and driver_data != BOOST freq
 		 * display NON BOOST freqs
 		 */
-		if (show_boost ^ (table[i].flags & CPUFREQ_BOOST_FREQ))
+		if (show_boost ^ (pos->flags & CPUFREQ_BOOST_FREQ))
 			continue;
 
-		count += sprintf(&buf[count], "%d ", table[i].frequency);
+		count += sprintf(&buf[count], "%d ", pos->frequency);
 	}
 	count += sprintf(&buf[count], "\n");

include/linux/cpufreq.h +29 -0

index 5ae5100..1c221c8 100644
@@ -468,6 +468,35 @@ struct cpufreq_frequency_table {
 				    * order */
 };
 
+static inline bool validate_entry(struct cpufreq_frequency_table *pos)
+{
+	while (pos->frequency != CPUFREQ_TABLE_END)
+		if (pos->frequency == CPUFREQ_ENTRY_INVALID)
+			pos++;
+		else
+			return true;
+	return false;
+}
+
+/*
+ * cpufreq_for_each_entry -	iterate over a cpufreq_frequency_table
+ * @pos:	the cpufreq_frequency_table * to use as a loop cursor.
+ * @table:	the cpufreq_frequency_table * to iterate over.
+ */
+
+#define cpufreq_for_each_entry(pos, table)	\
+	for (pos = table; pos->frequency != CPUFREQ_TABLE_END; pos++)
+
+/*
+ * cpufreq_for_each_valid_entry -     iterate over a cpufreq_frequency_table
+ *	exluding CPUFREQ_ENTRY_INVALID frequencies.
+ * @pos:        the cpufreq_frequency_table * to use as a loop cursor.
+ * @table:      the cpufreq_frequency_table * to iterate over.
+ */
+
+#define cpufreq_for_each_valid_entry(pos, table)	\
+	for (pos = table; validate_entry(pos); pos++)
+
 int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
 				    struct cpufreq_frequency_table *table);

Signature · 1 line

1.9.0

Viresh Kumar

maintainer

2014-04-11 04:24 UTC #1 lore raw

Quoted 9 lines · Stratos Karafotis

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.

Quoted 8 lines

Signed-off-by Stratos Karafotis <[email protected]>
---
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?

Quoted 16 lines

Stratos Karafotis.
---

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

drivers/cpufreq/freq_table.c +1 -0

index 08e7bbc..19bf0c4 100644
@@ -21,22 +21,18 @@
 int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
                                    struct cpufreq_frequency_table *table)
 {
+       struct cpufreq_frequency_table *pos;

drivers/cpufreq/freq_table.c line 29

Quoted 10 lines

drivers/cpufreq/freq_table.c +2 -5

        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 :)

Quoted 47 lines

drivers/cpufreq/freq_table.c +9 -9

-                       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);

drivers/cpufreq/freq_table.c int cpufreq_frequency_table_target(struct cpufreq_policy *policy, · line 134

Quoted 9 lines

drivers/cpufreq/freq_table.c +2 -4

@@ -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.

Quoted 16 lines

drivers/cpufreq/freq_table.c +3 -2

+
+               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)) {

drivers/cpufreq/freq_table.c int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy, · line 193

Quoted 7 lines

drivers/cpufreq/freq_table.c +1 -3

@@ -193,9 +190,9 @@ int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
                return -ENOENT;
        }
 
-       for (i = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
-               if (table[i].frequency == freq)
-                       return i;
+       cpufreq_for_each_entry(pos, table) {

use cpufreq_for_each_valid_entry() here as well.

Quoted 42 lines

drivers/cpufreq/freq_table.c +6 -7

+               if (pos->frequency == freq)
+                       return pos - table;
        }
 
        return -EINVAL;

@@ -208,16 +205,13 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_get_index);
 static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
                                    bool show_boost)
 {
-       unsigned int i = 0;
        ssize_t count = 0;
-       struct cpufreq_frequency_table *table = policy->freq_table;
+       struct cpufreq_frequency_table *pos, *table = policy->freq_table;
 
        if (!table)
                return -ENODEV;
 
-       for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
-               if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
-                       continue;
+       cpufreq_for_each_valid_entry(pos, table) {
                /*
                 * show_boost = true and driver_data = BOOST freq
                 * display BOOST freqs

@@ -229,10 +223,10 @@ static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
                 * show_boost = false and driver_data != BOOST freq
                 * display NON BOOST freqs
                 */
-               if (show_boost ^ (table[i].flags & CPUFREQ_BOOST_FREQ))
+               if (show_boost ^ (pos->flags & CPUFREQ_BOOST_FREQ))
                        continue;
 
-               count += sprintf(&buf[count], "%d ", table[i].frequency);
+               count += sprintf(&buf[count], "%d ", pos->frequency);
        }
        count += sprintf(&buf[count], "\n");

include/linux/cpufreq.h +0 -0

index 5ae5100..1c221c8 100644

include/linux/cpufreq.h struct cpufreq_frequency_table { · line 475

Quoted 8 lines

include/linux/cpufreq.h +5 -0

@@ -468,6 +468,35 @@ struct cpufreq_frequency_table {
                                    * order */
 };
 
+static inline bool validate_entry(struct cpufreq_frequency_table *pos)
+{
+       while (pos->frequency != CPUFREQ_TABLE_END)
+               if (pos->frequency == CPUFREQ_ENTRY_INVALID)
+                       pos++;

This is why I asked you if you have tested it or not.

pos is a local variable here and so pos++ wouldn't reflect in the parent function. But your code will still work as we will check again for the next INVALID frequency.

Another important thing: This routine is going to be used from a Macro and so making it inline is very *inefficient*. We are going to use it from Atleast 20-25 places (as you mentioned) and we definitely aren't looking to have so many copies of this one :)

So, you should move this to cpufreq.c

Quoted 16 lines

include/linux/cpufreq.h +16 -0

+               else
+                       return true;
+       return false;
+}
+
+/*
+ * cpufreq_for_each_entry -    iterate over a cpufreq_frequency_table
+ * @pos:       the cpufreq_frequency_table * to use as a loop cursor.
+ * @table:     the cpufreq_frequency_table * to iterate over.
+ */
+
+#define cpufreq_for_each_entry(pos, table)     \
+       for (pos = table; pos->frequency != CPUFREQ_TABLE_END; pos++)
+
+/*
+ * cpufreq_for_each_valid_entry -     iterate over a cpufreq_frequency_table

include/linux/cpufreq.h struct cpufreq_frequency_table { · line 501

Quoted 10 lines

include/linux/cpufreq.h +8 -0

+ *     exluding CPUFREQ_ENTRY_INVALID frequencies.
+ * @pos:        the cpufreq_frequency_table * to use as a loop cursor.
+ * @table:      the cpufreq_frequency_table * to iterate over.
+ */
+
+#define cpufreq_for_each_valid_entry(pos, table)       \
+       for (pos = table; validate_entry(pos); pos++)
+
 int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
                                    struct cpufreq_frequency_table *table);

Otherwise looks fine. Please update all drivers as well :)

Stratos Karafotis

author

2014-04-11 16:31 UTC #2 lore raw

Quoted 184 lines · Viresh Kumar

Quoted 9 lines · Stratos Karafotis

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.

Quoted 8 lines

Signed-off-by Stratos Karafotis <[email protected]>
---
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?

Quoted 16 lines

Stratos Karafotis.
---

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

drivers/cpufreq/freq_table.c +1 -0

index 08e7bbc..19bf0c4 100644
@@ -21,22 +21,18 @@
 int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
                                    struct cpufreq_frequency_table *table)
 {
+       struct cpufreq_frequency_table *pos;

drivers/cpufreq/freq_table.c line 29

Quoted 10 lines

drivers/cpufreq/freq_table.c +2 -5

        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 :)

Quoted 47 lines

drivers/cpufreq/freq_table.c +9 -9

-                       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);

drivers/cpufreq/freq_table.c int cpufreq_frequency_table_target(struct cpufreq_policy *policy, · line 134

Quoted 9 lines

drivers/cpufreq/freq_table.c +2 -4

@@ -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.

Quoted 16 lines

drivers/cpufreq/freq_table.c +3 -2

+
+               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)) {

drivers/cpufreq/freq_table.c int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy, · line 193

Quoted 7 lines

drivers/cpufreq/freq_table.c +1 -3

@@ -193,9 +190,9 @@ int cpufreq_frequency_table_get_index(struct cpufreq_policy *policy,
                return -ENOENT;
        }
 
-       for (i = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
-               if (table[i].frequency == freq)
-                       return i;
+       cpufreq_for_each_entry(pos, table) {

use cpufreq_for_each_valid_entry() here as well.

Quoted 45 lines

drivers/cpufreq/freq_table.c +6 -7

+               if (pos->frequency == freq)
+                       return pos - table;
        }
 
        return -EINVAL;

@@ -208,16 +205,13 @@ EXPORT_SYMBOL_GPL(cpufreq_frequency_table_get_index);
 static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
                                    bool show_boost)
 {
-       unsigned int i = 0;
        ssize_t count = 0;
-       struct cpufreq_frequency_table *table = policy->freq_table;
+       struct cpufreq_frequency_table *pos, *table = policy->freq_table;
 
        if (!table)
                return -ENODEV;
 
-       for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
-               if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
-                       continue;
+       cpufreq_for_each_valid_entry(pos, table) {
                /*
                 * show_boost = true and driver_data = BOOST freq
                 * display BOOST freqs

@@ -229,10 +223,10 @@ static ssize_t show_available_freqs(struct cpufreq_policy *policy, char *buf,
                 * show_boost = false and driver_data != BOOST freq
                 * display NON BOOST freqs
                 */
-               if (show_boost ^ (table[i].flags & CPUFREQ_BOOST_FREQ))
+               if (show_boost ^ (pos->flags & CPUFREQ_BOOST_FREQ))
                        continue;
 
-               count += sprintf(&buf[count], "%d ", table[i].frequency);
+               count += sprintf(&buf[count], "%d ", pos->frequency);
        }
        count += sprintf(&buf[count], "\n");

include/linux/cpufreq.h +0 -0

index 5ae5100..1c221c8 100644
@@ -468,6 +468,35 @@ struct cpufreq_frequency_table {
                                    * order */
 };

Quoted 10 lines

include/linux/cpufreq.h struct cpufreq_frequency_table { · line 475
Quoted 5 lines
include/linux/cpufreq.h +5 -0
+static inline bool validate_entry(struct cpufreq_frequency_table *pos)
+{
+       while (pos->frequency != CPUFREQ_TABLE_END)
+               if (pos->frequency == CPUFREQ_ENTRY_INVALID)
+                       pos++;
This is why I asked you if you have tested it or not.
pos is a local variable here and so pos++ wouldn't reflect in the parent function. But your code will still work as we will check again for the next INVALID frequency.

Of course, you are right!
Yes, I partially tested it. But my frequency table didn't contain an invalid entry. I will fix it.

Quoted 6 lines

Another important thing: This routine is going to be used from a Macro and so making it inline is very *inefficient*. We are going to use it from Atleast 20-25 places (as you mentioned) and we definitely aren't looking to have so many copies of this one :)
So, you should move this to cpufreq.c

OK, I will do it. I was thinking about performance but as you mentioned 20 copies are too much. :)

Quoted 20 lines

include/linux/cpufreq.h +20 -0

+               else
+                       return true;
+       return false;
+}
+
+/*
+ * cpufreq_for_each_entry -    iterate over a cpufreq_frequency_table
+ * @pos:       the cpufreq_frequency_table * to use as a loop cursor.
+ * @table:     the cpufreq_frequency_table * to iterate over.
+ */
+
+#define cpufreq_for_each_entry(pos, table)     \
+       for (pos = table; pos->frequency != CPUFREQ_TABLE_END; pos++)
+
+/*
+ * cpufreq_for_each_valid_entry -     iterate over a cpufreq_frequency_table
+ *     exluding CPUFREQ_ENTRY_INVALID frequencies.
+ * @pos:        the cpufreq_frequency_table * to use as a loop cursor.
+ * @table:      the cpufreq_frequency_table * to iterate over.
+ */

Quoted 9 lines

include/linux/cpufreq.h struct cpufreq_frequency_table { · line 501
Quoted 6 lines
include/linux/cpufreq.h +4 -0
+
+#define cpufreq_for_each_valid_entry(pos, table)       \
+       for (pos = table; validate_entry(pos); pos++)
+
 int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
                                    struct cpufreq_frequency_table *table);
Otherwise looks fine. Please update all drivers as well :)

I will prepare the patches with your suggestions.
Thank you very much for your review!

Stratos