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

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

On 10/22/24 11:01, Rafael J. Wysocki wrote:
> Hi Lukasz,
> 
> On Tue, Oct 22, 2024 at 11:39 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
>>
>> 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.
> 
> My plan was to take this patch and replace the open-coded locking with
> guards when applying it (that would be a mechanical change AFAICS).
> 
> I guess it's OK to add a R-by from you in that case?

Yes, please add. I know what you are going to add there, so:

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