[2/2] thermal: cpuidle: Register cpuidle cooling device
diff mbox series

Message ID 20191219221932.15930-2-daniel.lezcano@linaro.org
State Mainlined, archived
Delegated to: Zhang Rui
Headers show
Series
  • [1/2] thermal/drivers/of-thermal: Make of_thermal_destroy_zones static
Related show

Commit Message

Daniel Lezcano Dec. 19, 2019, 10:19 p.m. UTC
The cpuidle driver can be used as a cooling device by injecting idle
cycles. The DT binding for the idle state added an optional

When the property is set, register the cpuidle driver with the idle
state node pointer as a cooling device. The thermal framework will do
the association automatically with the thermal zone via the
cooling-device defined in the device tree cooling-maps section.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/cpuidle/dt_idle_states.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Matthias Kaehlcke Dec. 19, 2019, 10:51 p.m. UTC | #1
Hi Daniel,

On Thu, Dec 19, 2019 at 11:19:28PM +0100, Daniel Lezcano wrote:
> The cpuidle driver can be used as a cooling device by injecting idle
> cycles. The DT binding for the idle state added an optional
> 
> When the property is set, register the cpuidle driver with the idle
> state node pointer as a cooling device. The thermal framework will do
> the association automatically with the thermal zone via the
> cooling-device defined in the device tree cooling-maps section.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/cpuidle/dt_idle_states.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/cpuidle/dt_idle_states.c b/drivers/cpuidle/dt_idle_states.c
> index d06d21a9525d..34bd65197342 100644
> --- a/drivers/cpuidle/dt_idle_states.c
> +++ b/drivers/cpuidle/dt_idle_states.c
> @@ -8,6 +8,7 @@
>  
>  #define pr_fmt(fmt) "DT idle-states: " fmt
>  
> +#include <linux/cpu_cooling.h>
>  #include <linux/cpuidle.h>
>  #include <linux/cpumask.h>
>  #include <linux/errno.h>
> @@ -205,6 +206,13 @@ int dt_init_idle_driver(struct cpuidle_driver *drv,
>  			err = -EINVAL;
>  			break;
>  		}
> +
> +		if (of_find_property(state_node, "#cooling-cells", NULL)) {
> +			err = cpuidle_of_cooling_register(state_node, drv);

cpuidle_of_cooling_register() returns a struct thermal_cooling_device *,
so you probably want to use PTR_ERR() here.

Could it be a problem that the cooling device isn't unregistered even when all
associated cores are taken offline?
Daniel Lezcano Dec. 19, 2019, 10:57 p.m. UTC | #2
On 19/12/2019 23:51, Matthias Kaehlcke wrote:
> Hi Daniel,
> 
> On Thu, Dec 19, 2019 at 11:19:28PM +0100, Daniel Lezcano wrote:
>> The cpuidle driver can be used as a cooling device by injecting idle
>> cycles. The DT binding for the idle state added an optional
>>
>> When the property is set, register the cpuidle driver with the idle
>> state node pointer as a cooling device. The thermal framework will do
>> the association automatically with the thermal zone via the
>> cooling-device defined in the device tree cooling-maps section.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/cpuidle/dt_idle_states.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/cpuidle/dt_idle_states.c b/drivers/cpuidle/dt_idle_states.c
>> index d06d21a9525d..34bd65197342 100644
>> --- a/drivers/cpuidle/dt_idle_states.c
>> +++ b/drivers/cpuidle/dt_idle_states.c
>> @@ -8,6 +8,7 @@
>>  
>>  #define pr_fmt(fmt) "DT idle-states: " fmt
>>  
>> +#include <linux/cpu_cooling.h>
>>  #include <linux/cpuidle.h>
>>  #include <linux/cpumask.h>
>>  #include <linux/errno.h>
>> @@ -205,6 +206,13 @@ int dt_init_idle_driver(struct cpuidle_driver *drv,
>>  			err = -EINVAL;
>>  			break;
>>  		}
>> +
>> +		if (of_find_property(state_node, "#cooling-cells", NULL)) {
>> +			err = cpuidle_of_cooling_register(state_node, drv);
> 
> cpuidle_of_cooling_register() returns a struct thermal_cooling_device *,
> so you probably want to use PTR_ERR() here.

Right, I'm about the send the V6 which returns an int.

> Could it be a problem that the cooling device isn't unregistered even when all
> associated cores are taken offline?

The cooling device relies on the powercap/idle_inject.c which is based
on the smpboot API. This one takes care of parking the per cpu pinned
tasks. So AFAICS, it is fine.

Patch
diff mbox series

diff --git a/drivers/cpuidle/dt_idle_states.c b/drivers/cpuidle/dt_idle_states.c
index d06d21a9525d..34bd65197342 100644
--- a/drivers/cpuidle/dt_idle_states.c
+++ b/drivers/cpuidle/dt_idle_states.c
@@ -8,6 +8,7 @@ 
 
 #define pr_fmt(fmt) "DT idle-states: " fmt
 
+#include <linux/cpu_cooling.h>
 #include <linux/cpuidle.h>
 #include <linux/cpumask.h>
 #include <linux/errno.h>
@@ -205,6 +206,13 @@  int dt_init_idle_driver(struct cpuidle_driver *drv,
 			err = -EINVAL;
 			break;
 		}
+
+		if (of_find_property(state_node, "#cooling-cells", NULL)) {
+			err = cpuidle_of_cooling_register(state_node, drv);
+			if (err)
+				break;
+		}
+
 		of_node_put(state_node);
 	}