Eduardo,
Radivoje and Punit asked me to look into a patch Radivoje has posted recently and that got me to fix few things. It doesn't do anything related to what Radivoje had in his patch though.
Viresh Kumar (6): thermal/cpu_cooling: No need to initialize max_freq to 0 thermal/cpu_cooling: quit early after updating policy thermal/cpu_cooling: convert 'switch' block to 'if' block in notifier thermal/cpu_cooling: rename cpufreq_val as clipped_freq thermal/cpu_cooling: rename max_freq as clipped_freq in notifier thermal/cpu_cooling: update policy limits if clipped_freq < policy->max
drivers/thermal/cpu_cooling.c | 46 ++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 20 deletions(-)
Its always set before getting used, don't initialize it.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/cpu_cooling.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 6509c61b9648..d010c6d0d722 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -215,7 +215,7 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, unsigned long event, void *data) { struct cpufreq_policy *policy = data; - unsigned long max_freq = 0; + unsigned long max_freq; struct cpufreq_cooling_device *cpufreq_dev;
switch (event) {
If a valid cpufreq_dev is found for policy->cpu, we should update the policy and quit the for loop. There is no need to keep traversing the list of cpufreq_dev's.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/cpu_cooling.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index d010c6d0d722..276cec5a1fb7 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -232,6 +232,7 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, if (policy->max != max_freq) cpufreq_verify_within_limits(policy, 0, max_freq); + break; } mutex_unlock(&cooling_cpufreq_lock); break;
We just need to take care of single event here and there is no need to increase indentation level of most of the code (which causes lines longer that 80 columns to break).
Kill the switch block.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/cpu_cooling.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 276cec5a1fb7..bf2673bb3cf5 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -218,27 +218,21 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, unsigned long max_freq; struct cpufreq_cooling_device *cpufreq_dev;
- switch (event) { + if (event != CPUFREQ_ADJUST) + return NOTIFY_DONE;
- case CPUFREQ_ADJUST: - mutex_lock(&cooling_cpufreq_lock); - list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) { - if (!cpumask_test_cpu(policy->cpu, - &cpufreq_dev->allowed_cpus)) - continue; + mutex_lock(&cooling_cpufreq_lock); + list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) { + if (!cpumask_test_cpu(policy->cpu, &cpufreq_dev->allowed_cpus)) + continue;
- max_freq = cpufreq_dev->cpufreq_val; + max_freq = cpufreq_dev->cpufreq_val;
- if (policy->max != max_freq) - cpufreq_verify_within_limits(policy, 0, - max_freq); - break; - } - mutex_unlock(&cooling_cpufreq_lock); + if (policy->max != max_freq) + cpufreq_verify_within_limits(policy, 0, max_freq); break; - default: - return NOTIFY_DONE; } + mutex_unlock(&cooling_cpufreq_lock);
return NOTIFY_OK; }
That's what it is for, lets name it properly.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/cpu_cooling.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index bf2673bb3cf5..61ee726ede34 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -68,7 +68,7 @@ struct power_table { * registered cooling device. * @cpufreq_state: integer value representing the current state of cpufreq * cooling devices. - * @cpufreq_val: integer value representing the absolute value of the clipped + * @clipped_freq: integer value representing the absolute value of the clipped * frequency. * @max_level: maximum cooling level. One less than total number of valid * cpufreq frequencies. @@ -91,7 +91,7 @@ struct cpufreq_cooling_device { int id; struct thermal_cooling_device *cool_dev; unsigned int cpufreq_state; - unsigned int cpufreq_val; + unsigned int clipped_freq; unsigned int max_level; unsigned int *freq_table; /* In descending order */ struct cpumask allowed_cpus; @@ -226,7 +226,7 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, if (!cpumask_test_cpu(policy->cpu, &cpufreq_dev->allowed_cpus)) continue;
- max_freq = cpufreq_dev->cpufreq_val; + max_freq = cpufreq_dev->clipped_freq;
if (policy->max != max_freq) cpufreq_verify_within_limits(policy, 0, max_freq); @@ -514,7 +514,7 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
clip_freq = cpufreq_device->freq_table[state]; cpufreq_device->cpufreq_state = state; - cpufreq_device->cpufreq_val = clip_freq; + cpufreq_device->clipped_freq = clip_freq;
cpufreq_update_policy(cpu);
@@ -856,7 +856,7 @@ __cpufreq_cooling_register(struct device_node *np, pr_debug("%s: freq:%u KHz\n", __func__, freq); }
- cpufreq_dev->cpufreq_val = cpufreq_dev->freq_table[0]; + cpufreq_dev->clipped_freq = cpufreq_dev->freq_table[0]; cpufreq_dev->cool_dev = cool_dev;
mutex_lock(&cooling_cpufreq_lock);
That's what it is for, lets name it properly.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/cpu_cooling.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 61ee726ede34..9209c324a7fc 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -215,7 +215,7 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, unsigned long event, void *data) { struct cpufreq_policy *policy = data; - unsigned long max_freq; + unsigned long clipped_freq; struct cpufreq_cooling_device *cpufreq_dev;
if (event != CPUFREQ_ADJUST) @@ -226,10 +226,10 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, if (!cpumask_test_cpu(policy->cpu, &cpufreq_dev->allowed_cpus)) continue;
- max_freq = cpufreq_dev->clipped_freq; + clipped_freq = cpufreq_dev->clipped_freq;
- if (policy->max != max_freq) - cpufreq_verify_within_limits(policy, 0, max_freq); + if (policy->max != clipped_freq) + cpufreq_verify_within_limits(policy, 0, clipped_freq); break; } mutex_unlock(&cooling_cpufreq_lock);
policy->max is the maximum allowed frequency defined by user and clipped_freq is the maximum that thermal constraints allow.
If clipped_freq is lower than policy->max, then we need to readjust policy->max.
But, if clipped_freq is greater than policy->max, we don't need to do anything. We used to call cpufreq_verify_within_limits() in this case, but it doesn't change anything in this case.
Lets skip this unnecessary call and write a comment that explains this.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/thermal/cpu_cooling.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c index 9209c324a7fc..d9a603588cb5 100644 --- a/drivers/thermal/cpu_cooling.c +++ b/drivers/thermal/cpu_cooling.c @@ -226,9 +226,20 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb, if (!cpumask_test_cpu(policy->cpu, &cpufreq_dev->allowed_cpus)) continue;
+ /* + * policy->max is the maximum allowed frequency defined by user + * and clipped_freq is the maximum that thermal constraints + * allow. + * + * If clipped_freq is lower than policy->max, then we need to + * readjust policy->max. + * + * But, if clipped_freq is greater than policy->max, we don't + * need to do anything. + */ clipped_freq = cpufreq_dev->clipped_freq;
- if (policy->max != clipped_freq) + if (policy->max > clipped_freq) cpufreq_verify_within_limits(policy, 0, clipped_freq); break; }
On 04-08-15, 14:26, Viresh Kumar wrote:
On 30-07-15, 12:40, Viresh Kumar wrote:
Eduardo,
Radivoje and Punit asked me to look into a patch Radivoje has posted recently and that got me to fix few things. It doesn't do anything related to what Radivoje had in his patch though.
Ping !
Another Ping :)
On Wed, Aug 12, 2015 at 03:47:19PM +0530, Viresh Kumar wrote:
On 04-08-15, 14:26, Viresh Kumar wrote:
On 30-07-15, 12:40, Viresh Kumar wrote:
Eduardo,
Radivoje and Punit asked me to look into a patch Radivoje has posted recently and that got me to fix few things. It doesn't do anything related to what Radivoje had in his patch though.
Ping !
Another Ping :)
In the way.
-- viresh
linaro-kernel@lists.linaro.org