[PATCH] Thermal cpu_cooling: fix inconsistent use of CPU frequencytable

From: Zhang Rui
Date: Fri Feb 01 2013 - 02:41:47 EST


there are three kinds of entries in CPU frequency table.
1. invalid entry, its frequency is CPUFREQ_ENTRY_INVALID.
2. duplicate entry, two entry may share the same frequency,
And we should treat it as on valid entry.
3. valid entry with a proper frequency.

And when talking about cpufreq cooling device cooling state,
it should be the same thing, in both cpu_cooling.c and its users,
AKA, max_cooling_state of a cpufreq cooling device equals the
number of VALID entries (type #3 above) in CPU frequency table.
And when setting a cpufreq cooling device to state X, it means set
the cpufreq cooling device to Xth maximum frequency in the
cpufreq frequency table.

This patch does a cleanup in both drivers/thermal/cpu_cooling.c
and drivers/thermal/exynos_thermal.c to make them be consistent
in using cpu level/cpufreq cooling state.

Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
---
drivers/thermal/cpu_cooling.c | 122 ++++++++++++++++++++++++--------------
drivers/thermal/exynos_thermal.c | 17 +-----
include/linux/cpu_cooling.h | 12 ++++
3 files changed, 91 insertions(+), 60 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 836828e..c16795b 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -116,6 +116,40 @@ static int is_cpufreq_valid(int cpu)
}

/**
+ * cpufreq_cooling_get_max_level - function to get max valid cpufreq levels
+ * @cpu: cpu for which frequency is fetched.
+ */
+int cpufreq_cooling_get_max_level(unsigned int cpu)
+{
+ int i, level;
+ unsigned int freq = CPUFREQ_ENTRY_INVALID;
+ struct cpufreq_frequency_table *table =
+ cpufreq_frequency_get_table(cpu);
+
+ if (!table)
+ return -EINVAL;
+
+ for (i = 0, level = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
+ /* Invalid entry */
+ if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
+ continue;
+
+ /* Duplicate entry */
+ if (freq == table[i].frequency)
+ continue;
+
+ /* First valid entry */
+ if (freq == CPUFREQ_ENTRY_INVALID)
+ freq = table[i].frequency;
+
+ level++;
+ }
+
+ return level;
+}
+EXPORT_SYMBOL(cpufreq_cooling_get_max_level);
+
+/**
* get_cpu_frequency - get the absolute value of frequency from level.
* @cpu: cpu for which frequency is fetched.
* @level: level of frequency of the CPU
@@ -123,46 +157,58 @@ static int is_cpufreq_valid(int cpu)
*/
static unsigned int get_cpu_frequency(unsigned int cpu, unsigned long level)
{
- int ret = 0, i = 0;
- unsigned long level_index;
+ int i, count;
+ int max_level;
+ unsigned int freq = CPUFREQ_ENTRY_INVALID;
bool descend = false;
struct cpufreq_frequency_table *table =
cpufreq_frequency_get_table(cpu);
- if (!table)
- return ret;

- while (table[i].frequency != CPUFREQ_TABLE_END) {
+ max_level = cpufreq_cooling_get_max_level(cpu);
+
+ if (max_level < 0)
+ return max_level;
+
+ if (level > max_level)
+ return -EINVAL;
+
+ for (i = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
continue;

- /*check if table in ascending or descending order*/
- if ((table[i + 1].frequency != CPUFREQ_TABLE_END) &&
- (table[i + 1].frequency < table[i].frequency)
- && !descend) {
- descend = true;
+ if (freq == CPUFREQ_ENTRY_INVALID)
+ /* first valid entry */
+ freq = table[i].frequency;
+ else if (freq == table[i].frequency)
+ /* duplicate entry */
+ continue;
+ else {
+ /* two valid entries, check frequency order */
+ descend = !!(freq > table[i].frequency);
+ break;
}
-
- /*return if level matched and table in descending order*/
- if (descend && i == level)
- return table[i].frequency;
- i++;
}
- i--;

- if (level > i || descend)
- return ret;
- level_index = i - level;
+ /* level equals "the index of valid entries" in cpufreq table */
+ level = descend ? level : max_level - level + 1;

- /*Scan the table in reverse order and match the level*/
- while (i >= 0) {
+ freq = CPUFREQ_ENTRY_INVALID;
+ for (i = 0, count = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
+ /* ignore invalid entry */
if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
continue;
- /*return if level matched*/
- if (i == level_index)
+
+ if (freq == CPUFREQ_ENTRY_INVALID)
+ /* first valid entry */
+ freq = table[i].frequency;
+ else if (freq == table[i].frequency)
+ /* ignore duplicate entry */
+ continue;
+ count++;
+ if (level == count)
return table[i].frequency;
- i--;
}
- return ret;
+ return -ENODEV;
}

/**
@@ -244,29 +290,17 @@ static int cpufreq_get_max_state(struct thermal_cooling_device *cdev,
struct cpufreq_cooling_device *cpufreq_device = cdev->devdata;
struct cpumask *maskPtr = &cpufreq_device->allowed_cpus;
unsigned int cpu;
- struct cpufreq_frequency_table *table;
- unsigned long count = 0;
- int i = 0;
+ int max_level;

cpu = cpumask_any(maskPtr);
- table = cpufreq_frequency_get_table(cpu);
- if (!table) {
- *state = 0;
- return 0;
- }
-
- for (i = 0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
- if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
- continue;
- count++;
- }

- if (count > 0) {
- *state = --count;
- return 0;
- }
+ max_level = cpufreq_cooling_get_max_level(cpu);

- return -EINVAL;
+ if (max_level < 0)
+ return max_level;
+ else
+ *state = max_level;
+ return 0;
}

/**
diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
index cd71e24..5ac2de1 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -241,22 +241,7 @@ static int exynos_get_crit_temp(struct thermal_zone_device *thermal,

static int exynos_get_frequency_level(unsigned int cpu, unsigned int freq)
{
- int i = 0, ret = -EINVAL;
- struct cpufreq_frequency_table *table = NULL;
-#ifdef CONFIG_CPU_FREQ
- table = cpufreq_frequency_get_table(cpu);
-#endif
- if (!table)
- return ret;
-
- while (table[i].frequency != CPUFREQ_TABLE_END) {
- if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
- continue;
- if (table[i].frequency == freq)
- return i;
- i++;
- }
- return ret;
+ return cpufreq_cooling_get_max_level(cpu);
}

/* Bind callback functions for thermal zone */
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index 40b4ef5..2de9319 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -42,6 +42,14 @@ struct thermal_cooling_device *cpufreq_cooling_register(
* @cdev: thermal cooling device pointer.
*/
void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
+
+/**
+ * cpufreq_cooling_get_max_level - function to get maxinum cooling state
+ * of a cpufreq cooling device
+ * @cpu: cpu of which frequency is fetched.
+ */
+int cpufreq_cooling_get_max_level(unsigned int cpu);
+
#else /* !CONFIG_CPU_THERMAL */
static inline struct thermal_cooling_device *cpufreq_cooling_register(
const struct cpumask *clip_cpus)
@@ -53,6 +61,10 @@ static inline void cpufreq_cooling_unregister(
{
return;
}
+static inline int cpufreq_cooling_get_max_level(unsigned int cpu)
+{
+ return -ENODEV;
+}
#endif /* CONFIG_CPU_THERMAL */

#endif /* __CPU_COOLING_H__ */
--
1.7.9.5



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/