Re: [PATCH v4 09/10] thermal/drivers/ti-soc-thermal: Use scope-based cleanup helper

[Krzysztof Kozlowski <krzk@kernel.org>]

On 05/09/2025 08:57, Andreas Kemnade wrote:
> Am Wed,  3 Sep 2025 21:17:32 +0800
> schrieb Zihuan Zhang <zhangzihuan@kylinos.cn>:
> 
>> Replace the manual cpufreq_cpu_put() with __free(put_cpufreq_policy)
>> annotation for policy references. This reduces the risk of reference
>> counting mistakes and aligns the code with the latest kernel style.
>>
>> No functional change intended.
>>
>> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
>> ---
>>  drivers/thermal/ti-soc-thermal/ti-thermal-common.c | 13 ++++---------
>>  1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>> index 0cf0826b805a..37d06468913a 100644
>> --- a/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>> +++ b/drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>> @@ -27,7 +27,6 @@
>>  
>>  /* common data structures */
>>  struct ti_thermal_data {
>> -	struct cpufreq_policy *policy;
>>  	struct thermal_zone_device *ti_thermal;
>>  	struct thermal_zone_device *pcb_tz;
>>  	struct thermal_cooling_device *cool_dev;
>> @@ -218,6 +217,7 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
>>  {
>>  	struct ti_thermal_data *data;
>>  	struct device_node *np = bgp->dev->of_node;
>> +	struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(0);
>>  
> this looks as it changes the lifecycle from the device lifetime to just
> this function...
> 
>>  	/*
>>  	 * We are assuming here that if one deploys the zone
>> @@ -234,19 +234,17 @@ int ti_thermal_register_cpu_cooling(struct ti_bandgap *bgp, int id)
>>  	if (!data)
>>  		return -EINVAL;
>>  
>> -	data->policy = cpufreq_cpu_get(0);
>> -	if (!data->policy) {
>> +	if (!policy) {
>>  		pr_debug("%s: CPUFreq policy not found\n", __func__);
>>  		return -EPROBE_DEFER;
>>  	}
>>  
>>  	/* Register cooling device */
>> -	data->cool_dev = cpufreq_cooling_register(data->policy);
>> +	data->cool_dev = cpufreq_cooling_register(policy);
> 
> and it is passed on to something living beyond this function. I see no
> _get(policy) in cpufreq_cooling_register().
> Am I missing something?
Yeah, " No functional change intended." is clearly incorrect.

I already commented on this series that it is very bad and author should
does not understand how cleanup.h works, and here is another example.

IMO, entire patchset should be dropped.

Best regards,
Krzysztof