Re: [PATCH v5 1/4] thermal/netlink: Add the commands and the events for the thresholds

[Lukasz Luba <lukasz.luba@arm.com>]

Hi Daniel,

On 10/22/24 08:09, Daniel Lezcano wrote:
> On 22/10/2024 00:02, Lukasz Luba wrote:
>>
>>
>> On 10/14/24 10:43, Daniel Lezcano wrote:
>>> The thresholds exist but there is no notification neither action code
>>> related to them yet.
>>>
>>> These changes implement the netlink for the notifications when the
>>> thresholds are crossed, added, deleted or flushed as well as the
>>> commands which allows to get the list of the thresholds, flush them,
>>> add and delete.
>>>
>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> ---
> 
> [ ... ]
> 
>>> +static inline int thermal_notify_threshold_up(const struct 
>>> thermal_zone_device *tz)
>>> +{
>>> +    return 0;
>>> +}
>>
>> These 'return 0' look a bit odd. We usually use 'return -EINVAL' in
>> not defined. Although, since we don't check the output of those
>> functions  - we are OK. We just have to remember about these zeros,
>> one day when we would like to add the check of the return.
> 
> The return error really depends on the context of the call site. There 
> are other subsystems returning 0 when the service is not enabled (eg. 
> cpufreq.h, devfreq.h, device_cgroup.h, etc ...)
> 
> 

Fair enough. As I said, we would just keep them in mind if we one
day decide to add the checks of the returns.

I'm waiting for your next version with the new locking scheme that
Rafael asked and I will add my review tags.

Regards,
Lukasz