diff mbox series

[v3,4/4] thermal: cpuidle: Register cpuidle cooling device

Message ID 20200414220837.9284-4-daniel.lezcano@linaro.org (mailing list archive)
State New, archived
Delegated to: Daniel Lezcano
Headers show
Series [v3,1/4] powercap/drivers/idle_inject: Specify idle state max latency | expand

Commit Message

Daniel Lezcano April 14, 2020, 10:08 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/cpuidle-arm.c  | 5 +++++
 drivers/cpuidle/cpuidle-psci.c | 5 +++++
 2 files changed, 10 insertions(+)

Comments

Daniel Lezcano April 21, 2020, 8:15 a.m. UTC | #1
Hi Lorenzo, Sudeep,

other patches of the series are acked / reviewed.

If you are ok with these changes, could you add your acked-by so I can
merge all the series via the thermal tree?

Thanks

  -- Daniel

On 15/04/2020 00:08, 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/cpuidle-arm.c  | 5 +++++
>  drivers/cpuidle/cpuidle-psci.c | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index 9e5156d39627..2406ac0ae134 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -8,6 +8,7 @@
>  
>  #define pr_fmt(fmt) "CPUidle arm: " fmt
>  
> +#include <linux/cpu_cooling.h>
>  #include <linux/cpuidle.h>
>  #include <linux/cpumask.h>
>  #include <linux/cpu_pm.h>
> @@ -124,6 +125,10 @@ static int __init arm_idle_init_cpu(int cpu)
>  	if (ret)
>  		goto out_kfree_drv;
>  
> +	ret = cpuidle_cooling_register(drv);
> +	if (ret)
> +		pr_err("Failed to register the idle cooling device: %d\n", ret);
> +
>  	return 0;
>  
>  out_kfree_drv:
> diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
> index edd7a54ef0d3..8e805bff646f 100644
> --- a/drivers/cpuidle/cpuidle-psci.c
> +++ b/drivers/cpuidle/cpuidle-psci.c
> @@ -9,6 +9,7 @@
>  #define pr_fmt(fmt) "CPUidle PSCI: " fmt
>  
>  #include <linux/cpuhotplug.h>
> +#include <linux/cpu_cooling.h>
>  #include <linux/cpuidle.h>
>  #include <linux/cpumask.h>
>  #include <linux/cpu_pm.h>
> @@ -305,6 +306,10 @@ static int __init psci_idle_init_cpu(int cpu)
>  	if (ret)
>  		goto out_kfree_drv;
>  
> +	ret = cpuidle_cooling_register(drv);
> +	if (ret)
> +		pr_err("Failed to register the idle cooling device: %d\n", ret);
> +
>  	return 0;
>  
>  out_kfree_drv:
>
Daniel Lezcano April 27, 2020, 4:50 p.m. UTC | #2
Hi guys,

any chance you ack this patch ?


On 21/04/2020 10:15, Daniel Lezcano wrote:
> 
> Hi Lorenzo, Sudeep,
> 
> other patches of the series are acked / reviewed.
> 
> If you are ok with these changes, could you add your acked-by so I can
> merge all the series via the thermal tree?
> 
> Thanks
> 
>   -- Daniel
> 
> On 15/04/2020 00:08, 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/cpuidle-arm.c  | 5 +++++
>>  drivers/cpuidle/cpuidle-psci.c | 5 +++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
>> index 9e5156d39627..2406ac0ae134 100644
>> --- a/drivers/cpuidle/cpuidle-arm.c
>> +++ b/drivers/cpuidle/cpuidle-arm.c
>> @@ -8,6 +8,7 @@
>>  
>>  #define pr_fmt(fmt) "CPUidle arm: " fmt
>>  
>> +#include <linux/cpu_cooling.h>
>>  #include <linux/cpuidle.h>
>>  #include <linux/cpumask.h>
>>  #include <linux/cpu_pm.h>
>> @@ -124,6 +125,10 @@ static int __init arm_idle_init_cpu(int cpu)
>>  	if (ret)
>>  		goto out_kfree_drv;
>>  
>> +	ret = cpuidle_cooling_register(drv);
>> +	if (ret)
>> +		pr_err("Failed to register the idle cooling device: %d\n", ret);
>> +
>>  	return 0;
>>  
>>  out_kfree_drv:
>> diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
>> index edd7a54ef0d3..8e805bff646f 100644
>> --- a/drivers/cpuidle/cpuidle-psci.c
>> +++ b/drivers/cpuidle/cpuidle-psci.c
>> @@ -9,6 +9,7 @@
>>  #define pr_fmt(fmt) "CPUidle PSCI: " fmt
>>  
>>  #include <linux/cpuhotplug.h>
>> +#include <linux/cpu_cooling.h>
>>  #include <linux/cpuidle.h>
>>  #include <linux/cpumask.h>
>>  #include <linux/cpu_pm.h>
>> @@ -305,6 +306,10 @@ static int __init psci_idle_init_cpu(int cpu)
>>  	if (ret)
>>  		goto out_kfree_drv;
>>  
>> +	ret = cpuidle_cooling_register(drv);
>> +	if (ret)
>> +		pr_err("Failed to register the idle cooling device: %d\n", ret);
>> +
>>  	return 0;
>>  
>>  out_kfree_drv:
>>
> 
>
Lukasz Luba April 28, 2020, 3:31 p.m. UTC | #3
On 4/14/20 11:08 PM, 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/cpuidle-arm.c  | 5 +++++
>   drivers/cpuidle/cpuidle-psci.c | 5 +++++
>   2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
> index 9e5156d39627..2406ac0ae134 100644
> --- a/drivers/cpuidle/cpuidle-arm.c
> +++ b/drivers/cpuidle/cpuidle-arm.c
> @@ -8,6 +8,7 @@
>   
>   #define pr_fmt(fmt) "CPUidle arm: " fmt
>   
> +#include <linux/cpu_cooling.h>
>   #include <linux/cpuidle.h>
>   #include <linux/cpumask.h>
>   #include <linux/cpu_pm.h>
> @@ -124,6 +125,10 @@ static int __init arm_idle_init_cpu(int cpu)
>   	if (ret)
>   		goto out_kfree_drv;
>   
> +	ret = cpuidle_cooling_register(drv);
> +	if (ret)
> +		pr_err("Failed to register the idle cooling device: %d\n", ret);

This and similar from cpuidle-psci.c might produce a lot of error log
entries. The 'return 0' below does not take into account that we failed
to register cpuidle cooling. Thus, I would rather ignore the return from 
cpuidle_cooling_register and move this print into
cpuidle_cooling_register function, changing it also to debug level.

> +
>   	return 0;
>   
>   out_kfree_drv:
> diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
> index edd7a54ef0d3..8e805bff646f 100644
> --- a/drivers/cpuidle/cpuidle-psci.c
> +++ b/drivers/cpuidle/cpuidle-psci.c
> @@ -9,6 +9,7 @@
>   #define pr_fmt(fmt) "CPUidle PSCI: " fmt
>   
>   #include <linux/cpuhotplug.h>
> +#include <linux/cpu_cooling.h>
>   #include <linux/cpuidle.h>
>   #include <linux/cpumask.h>
>   #include <linux/cpu_pm.h>
> @@ -305,6 +306,10 @@ static int __init psci_idle_init_cpu(int cpu)
>   	if (ret)
>   		goto out_kfree_drv;
>   
> +	ret = cpuidle_cooling_register(drv);
> +	if (ret)
> +		pr_err("Failed to register the idle cooling device: %d\n", ret);
> +

The same here. I would change it into one line:
+	cpuidle_cooling_register(drv);

>   	return 0;
>   
>   out_kfree_drv:
> 

Regards,
Lukasz
diff mbox series

Patch

diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c
index 9e5156d39627..2406ac0ae134 100644
--- a/drivers/cpuidle/cpuidle-arm.c
+++ b/drivers/cpuidle/cpuidle-arm.c
@@ -8,6 +8,7 @@ 
 
 #define pr_fmt(fmt) "CPUidle arm: " fmt
 
+#include <linux/cpu_cooling.h>
 #include <linux/cpuidle.h>
 #include <linux/cpumask.h>
 #include <linux/cpu_pm.h>
@@ -124,6 +125,10 @@  static int __init arm_idle_init_cpu(int cpu)
 	if (ret)
 		goto out_kfree_drv;
 
+	ret = cpuidle_cooling_register(drv);
+	if (ret)
+		pr_err("Failed to register the idle cooling device: %d\n", ret);
+
 	return 0;
 
 out_kfree_drv:
diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
index edd7a54ef0d3..8e805bff646f 100644
--- a/drivers/cpuidle/cpuidle-psci.c
+++ b/drivers/cpuidle/cpuidle-psci.c
@@ -9,6 +9,7 @@ 
 #define pr_fmt(fmt) "CPUidle PSCI: " fmt
 
 #include <linux/cpuhotplug.h>
+#include <linux/cpu_cooling.h>
 #include <linux/cpuidle.h>
 #include <linux/cpumask.h>
 #include <linux/cpu_pm.h>
@@ -305,6 +306,10 @@  static int __init psci_idle_init_cpu(int cpu)
 	if (ret)
 		goto out_kfree_drv;
 
+	ret = cpuidle_cooling_register(drv);
+	if (ret)
+		pr_err("Failed to register the idle cooling device: %d\n", ret);
+
 	return 0;
 
 out_kfree_drv: