Re: [PATCH v5 4/4] tools/thermal/thermal-engine: Take into account the thresholds API

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

On 21/10/2024 22:10, Lukasz Luba wrote:
> 
> 
> On 10/14/24 10:43, Daniel Lezcano wrote:
>> Enhance the thermal-engine skeleton with the thresholds added in the
>> kernel and use the API exported by the thermal library.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>   tools/thermal/thermal-engine/thermal-engine.c | 105 +++++++++++++++---
>>   1 file changed, 92 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/thermal/thermal-engine/thermal-engine.c b/tools/ 
>> thermal/thermal-engine/thermal-engine.c
>> index 9b1476a2680f..0764dc754771 100644
>> --- a/tools/thermal/thermal-engine/thermal-engine.c
>> +++ b/tools/thermal/thermal-engine/thermal-engine.c
>> @@ -38,6 +38,14 @@ struct thermal_data {
>>       struct thermal_handler *th;
>>   };
>> +static int show_threshold(struct thermal_threshold *th, 
>> __maybe_unused void *arg)
>> +{
>> +    INFO("threshold temp=%d, direction=%d\n",
>> +         th->temperature, th->direction);
>> +
>> +    return 0;
>> +}
>> +
>>   static int show_trip(struct thermal_trip *tt, __maybe_unused void *arg)
>>   {
>>       INFO("trip id=%d, type=%d, temp=%d, hyst=%d\n",
>> @@ -70,6 +78,8 @@ static int show_tz(struct thermal_zone *tz, 
>> __maybe_unused void *arg)
>>       for_each_thermal_trip(tz->trip, show_trip, NULL);
>> +    for_each_thermal_threshold(tz->thresholds, show_threshold, NULL);
>> +
>>       show_temp(tz, arg);
>>       show_governor(tz, arg);
>> @@ -77,6 +87,30 @@ static int show_tz(struct thermal_zone *tz, 
>> __maybe_unused void *arg)
>>       return 0;
>>   }
>> +static int set_threshold(struct thermal_zone *tz, __maybe_unused void 
>> *arg)
>> +{
>> +    struct thermal_handler *th = arg;
>> +    int thresholds[] = { 43000, 65000, 49000, 55000, 57000 };
>> +    size_t i;
> 
> minor: I would just make it normal 'int'.

Why ? :)

The for loop compares i < sizeof(thresholds) / sizeof(thresholds[0])

sizeof() returns size_t

So we are comparing the same type.


>> +
>> +    INFO("Setting threshold for thermal zone '%s', id=%d\n", tz- 
>> >name, tz->id);
>> +

[ ... ]

> 
> LGTM,
> 
> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>


Thanks for the review



-- 
<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