Re: [PATCH v4 02/11] thermal: Store thermal mode in a dedicated enum

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/29/20 8:13 AM, Andrzej Pietrasiewicz wrote:
> Hi Guenter,
> 
> W dniu 29.05.2020 o 16:48, Guenter Roeck pisze:
>> On Thu, May 28, 2020 at 09:20:42PM +0200, Andrzej Pietrasiewicz wrote:
>>> Prepare for storing mode in struct thermal_zone_device.
>>>
>>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>
>>> ---
>>>   drivers/acpi/thermal.c                        | 27 +++++++++----------
>>>   drivers/platform/x86/acerhdf.c                |  8 ++++--
>>>   .../intel/int340x_thermal/int3400_thermal.c   | 18 +++++--------
>>>   3 files changed, 25 insertions(+), 28 deletions(-)
> 
> <snip>
> 
>>> @@ -544,27 +543,25 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
>>>                   enum thermal_device_mode mode)
>>>   {
>>>       struct acpi_thermal *tz = thermal->devdata;
>>> -    int enable;
>>>         if (!tz)
>>>           return -EINVAL;
>>>   +    if (mode != THERMAL_DEVICE_DISABLED &&
>>> +        mode != THERMAL_DEVICE_ENABLED)
>>> +        return -EINVAL;
>>
>> Personally I find this check unnecessary: The enum has no other values,
>> and it is verifyable that the callers provide the enum and not some other
>> value.
> 
> It is getting removed in PATCH 10/11.
> 
> 
>>> +    if (mode != THERMAL_DEVICE_ENABLED &&
>>> +        mode != THERMAL_DEVICE_DISABLED)
>>>           return -EINVAL;
>>
>> Same as above.
> 
> ditto.

Hmm, I think that would be better done with this patch. But I guess
that is a bit of PoV, so

Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

since I don't have any other objections/observations.

Guenter

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-rockchip




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux