[PATCH 0/6] thermal/cpu_cooling: Minor cleanups

[PATCH 0/6] thermal/cpu_cooling: Minor cleanups

[Viresh Kumar]

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

-- 
2.4.0

[PATCH 1/6] thermal/cpu_cooling: No need to initialize max_freq to 0

[Viresh Kumar]

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

[PATCH 2/6] thermal/cpu_cooling: quit early after updating policy

[Viresh Kumar]

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

[PATCH 3/6] thermal/cpu_cooling: convert 'switch' block to 'if' block in notifier

[Viresh Kumar]

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;
 }
-- 
2.4.0

[PATCH 4/6] thermal/cpu_cooling: rename cpufreq_val as clipped_freq

[Viresh Kumar]

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

[PATCH 5/6] thermal/cpu_cooling: rename max_freq as clipped_freq in notifier

[Viresh Kumar]

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

[PATCH 6/6] thermal/cpu_cooling: update policy limits if clipped_freq < policy->max

[Viresh Kumar]

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;
 	}
-- 
2.4.0

Re: [PATCH 0/6] thermal/cpu_cooling: Minor cleanups

[Viresh Kumar]

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 !

-- 
viresh

Re: [PATCH 0/6] thermal/cpu_cooling: Minor cleanups

[Viresh Kumar]

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

-- 
viresh

Re: [PATCH 0/6] thermal/cpu_cooling: Minor cleanups

[Eduardo Valentin]

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