Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1625840 > unrolled thread

[PATCH V3 00/17] thermal: cpu_cooling: improve interaction with cpufreq core

Back to article view | Back to linux.kernel

Contents

  [PATCH V3 00/17] thermal: cpu_cooling: improve interaction with cpufreq core Viresh Kumar <viresh.kumar@linaro.org> - 2017-04-19 07:40 +0200
    [PATCH V3 01/17] thermal: cpu_cooling: Avoid accessing potentially freed structures Viresh Kumar <viresh.kumar@linaro.org> - 2017-04-19 07:40 +0200
    [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus' Viresh Kumar <viresh.kumar@linaro.org> - 2017-04-19 07:40 +0200
      Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus' Lukasz Luba <lukasz.luba@arm.com> - 2017-04-24 19:00 +0200
        Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus' Viresh Kumar <viresh.kumar@linaro.org> - 2017-04-25 07:00 +0200
          Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus' Lukasz Luba <lukasz.luba@arm.com> - 2017-04-25 12:30 +0200
            Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus' Viresh Kumar <viresh.kumar@linaro.org> - 2017-04-25 12:30 +0200
    [PATCH V3 05/17] thermal: cpu_cooling: remove cpufreq_cooling_get_level() Viresh Kumar <viresh.kumar@linaro.org> - 2017-04-19 07:40 +0200
    [PATCH V3 15/17] thermal: cpu_cooling: don't store cpu_dev in cpufreq_cdev Viresh Kumar <viresh.kumar@linaro.org> - 2017-04-19 07:40 +0200

#1625840 — [PATCH V3 00/17] thermal: cpu_cooling: improve interaction with cpufreq core

Hi Guys,

The cpu_cooling driver is designed to use CPU frequency scaling to avoid
high thermal states for a platform. But it wasn't glued really well with
cpufreq core. For example clipped-cpus is copied from the policy
structure and its much better to use the policy->cpus (or related_cpus)
fields directly as they may have got updated. Not that things were
broken before this series, but they can be optimized a bit more.

This series tries to improve interactions between cpufreq core and
cpu_cooling driver and does some fixes/cleanups to the cpu_cooling
driver.

I have tested it on ARM 32 (exynos) and 64 bit (hikey) boards and have
pushed them for 0-day build bot and kernel CI testing as well. We should
know if something is broken with these.

@Lukasz: It would be good if you can give them a test, specially because
of your work on the "power" specific bits in the driver. This series
already has the improvements you suggested.

Pushed here as well:

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git thermal/cooling

V2->V3:
- Additional check to guarantee that policy is valid.
- Initialize freq-table and cpufreq_cdev->policy fields before they are
  used by the power-cooling functionality.
- Thanks Lukasz for testing out and suggesting these changes.

V1->V2:
- Name cpufreq cooling dev as cpufreq_cdev everywhere (Eduardo).

--
viresh

Viresh Kumar (17):
  thermal: cpu_cooling: Avoid accessing potentially freed structures
  thermal: cpu_cooling: rearrange globals
  thermal: cpu_cooling: Name cpufreq cooling devices as cpufreq_cdev
  thermal: cpu_cooling: replace cool_dev with cdev
  thermal: cpu_cooling: remove cpufreq_cooling_get_level()
  thermal: cpu_cooling: get rid of a variable in cpufreq_set_cur_state()
  thermal: cpu_cooling: use cpufreq_policy to register cooling device
  cpufreq: create cpufreq_table_count_valid_entries()
  thermal: cpu_cooling: store cpufreq policy
  thermal: cpu_cooling: OPPs are registered for all CPUs
  thermal: cpu_cooling: get rid of 'allowed_cpus'
  thermal: cpu_cooling: merge frequency and power tables
  thermal: cpu_cooling: create structure for idle time stats
  thermal: cpu_cooling: get_level() can't fail
  thermal: cpu_cooling: don't store cpu_dev in cpufreq_cdev
  thermal: cpu_cooling: 'freq' can't be zero in cpufreq_state2power()
  thermal: cpu_cooling: Rearrange struct cpufreq_cooling_device

 drivers/cpufreq/arm_big_little.c                   |   2 +-
 drivers/cpufreq/cpufreq-dt.c                       |   2 +-
 drivers/cpufreq/cpufreq_stats.c                    |  13 +-
 drivers/cpufreq/dbx500-cpufreq.c                   |   2 +-
 drivers/cpufreq/mt8173-cpufreq.c                   |   4 +-
 drivers/cpufreq/qoriq-cpufreq.c                    |   3 +-
 drivers/thermal/cpu_cooling.c                      | 602 +++++++++------------
 drivers/thermal/imx_thermal.c                      |  22 +-
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c |  22 +-
 include/linux/cpu_cooling.h                        |  32 +-
 include/linux/cpufreq.h                            |  14 +
 11 files changed, 311 insertions(+), 407 deletions(-)

-- 
2.12.0.432.g71c3a4f4ba37

[toc] | [next] | [standalone]

#1625842 — [PATCH V3 01/17] thermal: cpu_cooling: Avoid accessing potentially freed structures

After the lock is dropped, it is possible that the cpufreq_dev gets
freed before we call get_level() and that can cause kernel to crash.

Drop the lock after we are done using the structure.

Cc: 4.2+ <stable@vger.kernel.org> # 4.2+
Fixes: 02373d7c69b4 ("thermal: cpu_cooling: fix lockdep problems in cpu_cooling")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/thermal/cpu_cooling.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 69d0f430b2d1..be29489dd247 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -153,8 +153,10 @@ unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
 	mutex_lock(&cooling_list_lock);
 	list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) {
 		if (cpumask_test_cpu(cpu, &cpufreq_dev->allowed_cpus)) {
+			unsigned long level = get_level(cpufreq_dev, freq);
+
 			mutex_unlock(&cooling_list_lock);
-			return get_level(cpufreq_dev, freq);
+			return level;
 		}
 	}
 	mutex_unlock(&cooling_list_lock);
-- 
2.12.0.432.g71c3a4f4ba37

[toc] | [prev] | [next] | [standalone]

#1625843 — [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus'

'allowed_cpus' is a copy of policy->related_cpus and can be replaced by
it directly. At some places we are only concerned about online CPUs and
policy->cpus can be used there.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/thermal/cpu_cooling.c | 77 ++++++++++++-------------------------------
 1 file changed, 21 insertions(+), 56 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index ce387f62c93e..1097162f7f8a 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -74,7 +74,6 @@ struct power_table {
  *	frequency.
  * @max_level: maximum cooling level. One less than total number of valid
  *	cpufreq frequencies.
- * @allowed_cpus: all the cpus involved for this cpufreq_cooling_device.
  * @node: list_head to link all cpufreq_cooling_device together.
  * @last_load: load measured by the latest call to cpufreq_get_requested_power()
  * @time_in_idle: previous reading of the absolute time that this cpu was idle
@@ -97,7 +96,6 @@ struct cpufreq_cooling_device {
 	unsigned int clipped_freq;
 	unsigned int max_level;
 	unsigned int *freq_table;	/* In descending order */
-	struct cpumask allowed_cpus;
 	struct list_head node;
 	u32 last_load;
 	u64 *time_in_idle;
@@ -161,7 +159,7 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb,
 
 	mutex_lock(&cooling_list_lock);
 	list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
-		if (!cpumask_test_cpu(policy->cpu, &cpufreq_cdev->allowed_cpus))
+		if (policy != cpufreq_cdev->policy)
 			continue;
 
 		/*
@@ -304,7 +302,7 @@ static u32 cpu_power_to_freq(struct cpufreq_cooling_device *cpufreq_cdev,
  * get_load() - get load for a cpu since last updated
  * @cpufreq_cdev:	&struct cpufreq_cooling_device for this cpu
  * @cpu:	cpu number
- * @cpu_idx:	index of the cpu in cpufreq_cdev->allowed_cpus
+ * @cpu_idx:	index of the cpu in time_in_idle*
  *
  * Return: The average load of cpu @cpu in percentage since this
  * function was last called.
@@ -351,7 +349,7 @@ static int get_static_power(struct cpufreq_cooling_device *cpufreq_cdev,
 {
 	struct dev_pm_opp *opp;
 	unsigned long voltage;
-	struct cpumask *cpumask = &cpufreq_cdev->allowed_cpus;
+	struct cpumask *cpumask = cpufreq_cdev->policy->related_cpus;
 	unsigned long freq_hz = freq * 1000;
 
 	if (!cpufreq_cdev->plat_get_static_power || !cpufreq_cdev->cpu_dev) {
@@ -468,7 +466,7 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
 	cpufreq_cdev->cpufreq_state = state;
 	cpufreq_cdev->clipped_freq = clip_freq;
 
-	cpufreq_update_policy(cpumask_any(&cpufreq_cdev->allowed_cpus));
+	cpufreq_update_policy(cpufreq_cdev->policy->cpu);
 
 	return 0;
 }
@@ -504,28 +502,18 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
 	int i = 0, cpu, ret;
 	u32 static_power, dynamic_power, total_load = 0;
 	struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
+	struct cpufreq_policy *policy = cpufreq_cdev->policy;
 	u32 *load_cpu = NULL;
 
-	cpu = cpumask_any_and(&cpufreq_cdev->allowed_cpus, cpu_online_mask);
-
-	/*
-	 * All the CPUs are offline, thus the requested power by
-	 * the cdev is 0
-	 */
-	if (cpu >= nr_cpu_ids) {
-		*power = 0;
-		return 0;
-	}
-
-	freq = cpufreq_quick_get(cpu);
+	freq = cpufreq_quick_get(policy->cpu);
 
 	if (trace_thermal_power_cpu_get_power_enabled()) {
-		u32 ncpus = cpumask_weight(&cpufreq_cdev->allowed_cpus);
+		u32 ncpus = cpumask_weight(policy->related_cpus);
 
 		load_cpu = kcalloc(ncpus, sizeof(*load_cpu), GFP_KERNEL);
 	}
 
-	for_each_cpu(cpu, &cpufreq_cdev->allowed_cpus) {
+	for_each_cpu(cpu, policy->related_cpus) {
 		u32 load;
 
 		if (cpu_online(cpu))
@@ -550,9 +538,9 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
 	}
 
 	if (load_cpu) {
-		trace_thermal_power_cpu_get_power(
-			&cpufreq_cdev->allowed_cpus,
-			freq, load_cpu, i, dynamic_power, static_power);
+		trace_thermal_power_cpu_get_power(policy->related_cpus, freq,
+						  load_cpu, i, dynamic_power,
+						  static_power);
 
 		kfree(load_cpu);
 	}
@@ -581,38 +569,22 @@ static int cpufreq_state2power(struct thermal_cooling_device *cdev,
 			       unsigned long state, u32 *power)
 {
 	unsigned int freq, num_cpus;
-	cpumask_var_t cpumask;
 	u32 static_power, dynamic_power;
 	int ret;
 	struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
 
-	if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
-		return -ENOMEM;
-
-	cpumask_and(cpumask, &cpufreq_cdev->allowed_cpus, cpu_online_mask);
-	num_cpus = cpumask_weight(cpumask);
-
-	/* None of our cpus are online, so no power */
-	if (num_cpus == 0) {
-		*power = 0;
-		ret = 0;
-		goto out;
-	}
+	num_cpus = cpumask_weight(cpufreq_cdev->policy->cpus);
 
 	freq = cpufreq_cdev->freq_table[state];
-	if (!freq) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (!freq)
+		return -EINVAL;
 
 	dynamic_power = cpu_freq_to_power(cpufreq_cdev, freq) * num_cpus;
 	ret = get_static_power(cpufreq_cdev, tz, freq, &static_power);
 	if (ret)
-		goto out;
+		return ret;
 
 	*power = static_power + dynamic_power;
-out:
-	free_cpumask_var(cpumask);
 	return ret;
 }
 
@@ -640,19 +612,14 @@ static int cpufreq_power2state(struct thermal_cooling_device *cdev,
 			       struct thermal_zone_device *tz, u32 power,
 			       unsigned long *state)
 {
-	unsigned int cpu, cur_freq, target_freq;
+	unsigned int cur_freq, target_freq;
 	int ret;
 	s32 dyn_power;
 	u32 last_load, normalised_power, static_power;
 	struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
+	struct cpufreq_policy *policy = cpufreq_cdev->policy;
 
-	cpu = cpumask_any_and(&cpufreq_cdev->allowed_cpus, cpu_online_mask);
-
-	/* None of our cpus are online */
-	if (cpu >= nr_cpu_ids)
-		return -ENODEV;
-
-	cur_freq = cpufreq_quick_get(cpu);
+	cur_freq = cpufreq_quick_get(policy->cpu);
 	ret = get_static_power(cpufreq_cdev, tz, cur_freq, &static_power);
 	if (ret)
 		return ret;
@@ -667,12 +634,12 @@ static int cpufreq_power2state(struct thermal_cooling_device *cdev,
 	if (*state == THERMAL_CSTATE_INVALID) {
 		dev_err_ratelimited(&cdev->device,
 				    "Failed to convert %dKHz for cpu %d into a cdev state\n",
-				    target_freq, cpu);
+				    target_freq, policy->cpu);
 		return -EINVAL;
 	}
 
-	trace_thermal_power_cpu_limit(&cpufreq_cdev->allowed_cpus,
-				      target_freq, *state, power);
+	trace_thermal_power_cpu_limit(policy->related_cpus, target_freq, *state,
+				      power);
 	return 0;
 }
 
@@ -786,8 +753,6 @@ __cpufreq_cooling_register(struct device_node *np,
 		goto free_time_in_idle_timestamp;
 	}
 
-	cpumask_copy(&cpufreq_cdev->allowed_cpus, policy->related_cpus);
-
 	if (capacitance) {
 		cpufreq_cdev->plat_get_static_power = plat_static_func;
 
-- 
2.12.0.432.g71c3a4f4ba37

[toc] | [prev] | [next] | [standalone]

#1629785 — Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus'

Hi Viresh,

I have been testing the patch set and found one of the issues.
Please see the comment below.

On 19/04/17 06:29, Viresh Kumar wrote:
> 'allowed_cpus' is a copy of policy->related_cpus and can be replaced by
> it directly. At some places we are only concerned about online CPUs and
> policy->cpus can be used there.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/thermal/cpu_cooling.c | 77 ++++++++++++-------------------------------
>  1 file changed, 21 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index ce387f62c93e..1097162f7f8a 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -74,7 +74,6 @@ struct power_table {
>   *	frequency.
>   * @max_level: maximum cooling level. One less than total number of valid
>   *	cpufreq frequencies.
> - * @allowed_cpus: all the cpus involved for this cpufreq_cooling_device.
>   * @node: list_head to link all cpufreq_cooling_device together.
>   * @last_load: load measured by the latest call to cpufreq_get_requested_power()
>   * @time_in_idle: previous reading of the absolute time that this cpu was idle
> @@ -97,7 +96,6 @@ struct cpufreq_cooling_device {
>  	unsigned int clipped_freq;
>  	unsigned int max_level;
>  	unsigned int *freq_table;	/* In descending order */
> -	struct cpumask allowed_cpus;
>  	struct list_head node;
>  	u32 last_load;
>  	u64 *time_in_idle;
> @@ -161,7 +159,7 @@ static int cpufreq_thermal_notifier(struct notifier_block *nb,
>
>  	mutex_lock(&cooling_list_lock);
>  	list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
> -		if (!cpumask_test_cpu(policy->cpu, &cpufreq_cdev->allowed_cpus))
> +		if (policy != cpufreq_cdev->policy)
The policy pointer forwarded from cpufreq_update_policy()
is a local variable 'new_policy' so cannot be compared with pinned
policy pointer in the cooling device.
You should do the cpumask test like before:
	if (!cpumask_test_cpu(policy->cpu,
			      cpufreq_cdev->policy->related_cpus))


But there is something still in the patch set...
I will try to check it tomorrow.

Best regards,
Lukasz

[toc] | [prev] | [next] | [standalone]

#1630185 — Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus'

On 24-04-17, 17:53, Lukasz Luba wrote:
> The policy pointer forwarded from cpufreq_update_policy()
> is a local variable 'new_policy' so cannot be compared with pinned
> policy pointer in the cooling device.
> You should do the cpumask test like before:
> 	if (!cpumask_test_cpu(policy->cpu,
> 			      cpufreq_cdev->policy->related_cpus))

Right. I have fixed it a bit differently now.

> But there is something still in the patch set...
> I will try to check it tomorrow.

I reviewed all the patches very carefully again, trying to find out the culprit
(I don't have the right hardware to test it like you have).

Found out that max_level isn't used properly at few places, fixed and pushed my
branch now. See if it works fine now.

HEAD: 6a883ddf73cd

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1630372 — Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus'

Hi Viresh,

I have run the newest version (6a883ddf73cd).
It looks correct (tests are passing so far).
Feel free to add
Tested-by: Lukasz Luba <lukasz.luba@arm.com>

I would like to go through the code before
it got merged, though.

If you are planing to post v4, I can test and review it
this week.

Regards,
Lukasz


On 25/04/17 05:57, Viresh Kumar wrote:
> On 24-04-17, 17:53, Lukasz Luba wrote:
>> The policy pointer forwarded from cpufreq_update_policy()
>> is a local variable 'new_policy' so cannot be compared with pinned
>> policy pointer in the cooling device.
>> You should do the cpumask test like before:
>> 	if (!cpumask_test_cpu(policy->cpu,
>> 			      cpufreq_cdev->policy->related_cpus))
>
> Right. I have fixed it a bit differently now.
>
>> But there is something still in the patch set...
>> I will try to check it tomorrow.
>
> I reviewed all the patches very carefully again, trying to find out the culprit
> (I don't have the right hardware to test it like you have).
>
> Found out that max_level isn't used properly at few places, fixed and pushed my
> branch now. See if it works fine now.
>
> HEAD: 6a883ddf73cd
>

[toc] | [prev] | [next] | [standalone]

#1630375 — Re: [PATCH V3 11/17] thermal: cpu_cooling: get rid of 'allowed_cpus'

On 25-04-17, 11:19, Lukasz Luba wrote:
> Hi Viresh,
> 
> I have run the newest version (6a883ddf73cd).
> It looks correct (tests are passing so far).
> Feel free to add
> Tested-by: Lukasz Luba <lukasz.luba@arm.com>
> 
> I would like to go through the code before
> it got merged, though.
> 
> If you are planing to post v4, I can test and review it
> this week.

Thanks a lot. I have just sent that.

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1625844 — [PATCH V3 05/17] thermal: cpu_cooling: remove cpufreq_cooling_get_level()

There is only one user of cpufreq_cooling_get_level() and that already
has pointer to the cpufreq_cdev structure. It can directly call
get_level() instead and we can get rid of cpufreq_cooling_get_level().

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/thermal/cpu_cooling.c | 33 +--------------------------------
 include/linux/cpu_cooling.h   |  6 ------
 2 files changed, 1 insertion(+), 38 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index f1e784c22c5a..1f4b6a719d05 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -136,37 +136,6 @@ static unsigned long get_level(struct cpufreq_cooling_device *cpufreq_cdev,
 }
 
 /**
- * cpufreq_cooling_get_level - for a given cpu, return the cooling level.
- * @cpu: cpu for which the level is required
- * @freq: the frequency of interest
- *
- * This function will match the cooling level corresponding to the
- * requested @freq and return it.
- *
- * Return: The matched cooling level on success or THERMAL_CSTATE_INVALID
- * otherwise.
- */
-unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
-{
-	struct cpufreq_cooling_device *cpufreq_cdev;
-
-	mutex_lock(&cooling_list_lock);
-	list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
-		if (cpumask_test_cpu(cpu, &cpufreq_cdev->allowed_cpus)) {
-			unsigned long level = get_level(cpufreq_cdev, freq);
-
-			mutex_unlock(&cooling_list_lock);
-			return level;
-		}
-	}
-	mutex_unlock(&cooling_list_lock);
-
-	pr_err("%s: cpu:%d not part of any cooling device\n", __func__, cpu);
-	return THERMAL_CSTATE_INVALID;
-}
-EXPORT_SYMBOL_GPL(cpufreq_cooling_get_level);
-
-/**
  * cpufreq_thermal_notifier - notifier callback for cpufreq policy change.
  * @nb:	struct notifier_block * with callback info.
  * @event: value showing cpufreq event for which this function invoked.
@@ -697,7 +666,7 @@ static int cpufreq_power2state(struct thermal_cooling_device *cdev,
 	normalised_power = (dyn_power * 100) / last_load;
 	target_freq = cpu_power_to_freq(cpufreq_cdev, normalised_power);
 
-	*state = cpufreq_cooling_get_level(cpu, target_freq);
+	*state = get_level(cpufreq_cdev, target_freq);
 	if (*state == THERMAL_CSTATE_INVALID) {
 		dev_err_ratelimited(&cdev->device,
 				    "Failed to convert %dKHz for cpu %d into a cdev state\n",
diff --git a/include/linux/cpu_cooling.h b/include/linux/cpu_cooling.h
index c156f5082758..96c5e4c2f9c8 100644
--- a/include/linux/cpu_cooling.h
+++ b/include/linux/cpu_cooling.h
@@ -82,7 +82,6 @@ of_cpufreq_power_cooling_register(struct device_node *np,
  */
 void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev);
 
-unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq);
 #else /* !CONFIG_CPU_THERMAL */
 static inline struct thermal_cooling_device *
 cpufreq_cooling_register(const struct cpumask *clip_cpus)
@@ -117,11 +116,6 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
 {
 	return;
 }
-static inline
-unsigned long cpufreq_cooling_get_level(unsigned int cpu, unsigned int freq)
-{
-	return THERMAL_CSTATE_INVALID;
-}
 #endif	/* CONFIG_CPU_THERMAL */
 
 #endif /* __CPU_COOLING_H__ */
-- 
2.12.0.432.g71c3a4f4ba37

[toc] | [prev] | [next] | [standalone]

#1625845 — [PATCH V3 15/17] thermal: cpu_cooling: don't store cpu_dev in cpufreq_cdev

'cpu_dev' is used by only one function, get_static_power(), and it
wouldn't be time consuming to get the cpu device structure within it.
This would help removing cpu_dev from struct cpufreq_cooling_device.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/thermal/cpu_cooling.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 762ddfc4e654..c85b217d16c8 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -87,7 +87,6 @@ struct time_in_idle {
  * @node: list_head to link all cpufreq_cooling_device together.
  * @last_load: load measured by the latest call to cpufreq_get_requested_power()
  * @idle_time: idle time stats
- * @cpu_dev: the cpu_device of policy->cpu.
  * @plat_get_static_power: callback to calculate the static power
  *
  * This structure is required for keeping information of each registered
@@ -104,7 +103,6 @@ struct cpufreq_cooling_device {
 	struct list_head node;
 	u32 last_load;
 	struct time_in_idle *idle_time;
-	struct device *cpu_dev;
 	get_static_t plat_get_static_power;
 };
 
@@ -255,8 +253,6 @@ static int update_freq_table(struct cpufreq_cooling_device *cpufreq_cdev,
 		freq_table[i].power = power;
 	}
 
-	cpufreq_cdev->cpu_dev = dev;
-
 	return 0;
 }
 
@@ -338,19 +334,22 @@ static int get_static_power(struct cpufreq_cooling_device *cpufreq_cdev,
 {
 	struct dev_pm_opp *opp;
 	unsigned long voltage;
-	struct cpumask *cpumask = cpufreq_cdev->policy->related_cpus;
+	struct cpufreq_policy *policy = cpufreq_cdev->policy;
+	struct cpumask *cpumask = policy->related_cpus;
 	unsigned long freq_hz = freq * 1000;
+	struct device *dev;
 
-	if (!cpufreq_cdev->plat_get_static_power || !cpufreq_cdev->cpu_dev) {
+	if (!cpufreq_cdev->plat_get_static_power) {
 		*power = 0;
 		return 0;
 	}
 
-	opp = dev_pm_opp_find_freq_exact(cpufreq_cdev->cpu_dev, freq_hz,
-					 true);
+	dev = get_cpu_device(policy->cpu);
+	WARN_ON(!dev);
+
+	opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true);
 	if (IS_ERR(opp)) {
-		dev_warn_ratelimited(cpufreq_cdev->cpu_dev,
-				     "Failed to find OPP for frequency %lu: %ld\n",
+		dev_warn_ratelimited(dev, "Failed to find OPP for frequency %lu: %ld\n",
 				     freq_hz, PTR_ERR(opp));
 		return -EINVAL;
 	}
@@ -359,8 +358,7 @@ static int get_static_power(struct cpufreq_cooling_device *cpufreq_cdev,
 	dev_pm_opp_put(opp);
 
 	if (voltage == 0) {
-		dev_err_ratelimited(cpufreq_cdev->cpu_dev,
-				    "Failed to get voltage for frequency %lu\n",
+		dev_err_ratelimited(dev, "Failed to get voltage for frequency %lu\n",
 				    freq_hz);
 		return -EINVAL;
 	}
-- 
2.12.0.432.g71c3a4f4ba37

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web