Re: [PATCH V4 4/4] thermal/drivers/cpu_cooling: Rename to cpufreq_cooling

[Martin Kepplinger <martin.kepplinger@puri.sm>]

On 06.12.19 15:15, Daniel Lezcano wrote:
> On 06/12/2019 12:33, Martin Kepplinger wrote:
>> I tested this on the librem5-devkit and see the
>> cooling devices in sysfs. I configure ARM_PSCI_CPUIDLE, not ARM_CPUIDLE and
>> add the patch below in register the cooling device there. "psci_idle"
>> is listed as the cpuidle_driver.
>>
>> That's what I'm running, in case you want to see it all:
>> https://source.puri.sm/martin.kepplinger/linux-next/commits/next-20191205/librem5_cpuidle_mainline_atf
>>
>> so I add a trip temperature description like this:
>> https://source.puri.sm/martin.kepplinger/linux-next/commit/361f49f93ae2c477fd012790831cabd0ed976660
>>
>> When I let the SoC heat up, cpuidle cooling won't kick it. In sysfs:
>>
>> catting the relevant files in /sys/class/thermal after heating up,
>> if that makes sense:
>>
>> 87000
>> 85000
>> 85000
>> thermal-cpufreq-0
>> 1
>> thermal-idle-0
>> 0
>> thermal-idle-1                                                                  
>> 0                                                                               
>> thermal-idle-2
>> 0
>> thermal-idle-3
>> 0
>>
>> with ARM_CPUIDLE instead of ARM_PSCI_CPUIDLE (and registering the cooling dev
>> during cpuidle-arm.c init) I won't have a cpuidle driver and thus no cpu-sleep
>> state at all.
>>
>> Can you see where the problem here lies?
> 
> Yes, I removed the registration via the DT.
> 
> Can you try the following:
> 
> diff --git a/drivers/cpuidle/dt_idle_states.c
> b/drivers/cpuidle/dt_idle_states.c
> index d06d21a9525d..01367ddec49a 100644
> --- a/drivers/cpuidle/dt_idle_states.c
> +++ b/drivers/cpuidle/dt_idle_states.c
> @@ -13,6 +13,7 @@
>  #include <linux/errno.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/cpu_cooling.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> 
> @@ -205,6 +206,9 @@ int dt_init_idle_driver(struct cpuidle_driver *drv,
>  			err = -EINVAL;
>  			break;
>  		}
> +
> +		cpuidle_of_cooling_register(state_node, drv);
> +
>  		of_node_put(state_node);
>  	}
> 
> That's a hack for the moment.
> 

thanks. I could test that successfully. The only question would be: Is
is intentional how "non-aggressive" the cooling driver cools? I would
have expected it to basically inject more idle cycles earlier. I'd set
75 degrees as trip point and at 85 degress is would only inject about 30
(of 100).

You describe the "config values" in question in the documentation, but
I'm not sure what's the correct way to change them.

thanks,

                                      martin