thermal/cpu_cooling: Minor cleanups

PATCH In review Viresh Kumar 2 participants 10 messages Jul 30, 2015 – Aug 14, 2015 Thermal

lore

Share & tools

lore

Viresh Kumar

author

2015-07-30 07:10 UTC #0 lore raw

cover thermal/cpu_cooling: Minor cleanups

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

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-07-30 07:10 UTC #1 lore raw

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

Its always set before getting used, don't initialize it.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/thermal/cpu_cooling.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/thermal/cpu_cooling.c +1 -1

index 6509c61b9648..d010c6d0d722 100644
@@ -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) {

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-07-30 07:10 UTC #2 lore raw

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

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

---
 drivers/thermal/cpu_cooling.c | 1 +
 1 file changed, 1 insertion(+)

drivers/thermal/cpu_cooling.c +1 -0

index d010c6d0d722..276cec5a1fb7 100644
@@ -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;

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-07-30 07:10 UTC #3 lore raw

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

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

---
 drivers/thermal/cpu_cooling.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

drivers/thermal/cpu_cooling.c +10 ~~-16~~

index 276cec5a1fb7..bf2673bb3cf5 100644
@@ -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;
 }

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-07-30 07:10 UTC #4 lore raw

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

In reply to Viresh Kumar

That's what it is for, lets name it properly.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/thermal/cpu_cooling.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/thermal/cpu_cooling.c +5 -5

index bf2673bb3cf5..61ee726ede34 100644
@@ -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);

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-07-30 07:10 UTC #5 lore raw

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

In reply to Viresh Kumar

That's what it is for, lets name it properly.

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/thermal/cpu_cooling.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/thermal/cpu_cooling.c +4 -4

index 61ee726ede34..9209c324a7fc 100644
@@ -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);

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-07-30 07:10 UTC #6 lore raw

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

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

---
 drivers/thermal/cpu_cooling.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

drivers/thermal/cpu_cooling.c +12 -1

index 9209c324a7fc..d9a603588cb5 100644
@@ -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;
 	}

Signature · 1 line

2.4.0

Viresh Kumar

author

2015-08-04 08:56 UTC #7 lore raw

In reply to Viresh Kumar

Quoted 5 lines · 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.

Ping !

Signature · 1 line

viresh

Viresh Kumar

author

2015-08-12 10:17 UTC #8 lore raw

Quoted 8 lines · Viresh Kumar

Quoted 5 lines · 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.
Ping !

Another Ping :)

Signature · 1 line

viresh

Eduardo Valentin

2015-08-14 04:59 UTC #9 lore raw

Quoted 11 lines · Viresh Kumar

Quoted 8 lines · Viresh Kumar
Quoted 5 lines · 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.
Ping !
Another Ping :)

In the way.

Quoted 3 lines

Signature · 2 lines
-- 
viresh