Re: [PATCH 2/2] thermal/drivers/thermal_helpers: Adjust output format

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 08/04/2022 13:09, Alexander Stein wrote:
> Outputs like this where -1 is printed as unsigned is somewhat misleading
>   thermal thermal_zone1: Trip3[type=0,temp=48000]:trend=4,throttle=1
>   thermal cooling_device3: cur_state=1
>   thermal cooling_device3: old_target=-1, target=2
>   thermal cooling_device3: zone1->target=1
>   thermal cooling_device3: zone1->target=2
>   thermal cooling_device3: zone1->target=18446744073709551615
>   thermal cooling_device3: set to state 2
> 
> With THERMAL_NO_TARGET assigning -1 as unsigned it make sense to print
> the target as signed integer, even if the type is actually unsigned.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> An alternative would be to change thermal_instance::target from unsigned
> long to long, but this would entail a lot of API & driver changes as well
> which looks less intriguing.
> 
>   drivers/thermal/thermal_helpers.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
> index 3edd047e144f..0d0da6670267 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -199,7 +199,7 @@ void __thermal_cdev_update(struct thermal_cooling_device *cdev)
>   
>   	/* Make sure cdev enters the deepest cooling state */
>   	list_for_each_entry(instance, &cdev->thermal_instances, cdev_node) {
> -		dev_dbg(&cdev->device, "zone%d->target=%lu\n",
> +		dev_dbg(&cdev->device, "zone%d->target=%ld\n",
>   			instance->tz->id, instance->target);
>   		if (instance->target == THERMAL_NO_TARGET)
>   			continue;

Actually you pointed out something fuzzy in the target values.

The unsigned long type for the target and THERMAL_NO_TARGET are not 
compatible.

It would be much simpler to have THERMAL_NO_TARGET = 0 which 
semantically makes more sense than a negative value.



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog