linux-kernel - Re: [PATCH 2/3] hwmon: (cros_ec) Add reading target fan RPM function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b9301c9b-9fac-46c4-ae6a-95029a1a2668@roeck-us.net>
Date: Fri, 14 Mar 2025 07:23:22 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Thomas Weißschuh <thomas@...ssschuh.net>
Cc: Sung-Chi Li <lschyi@...omium.org>, Jean Delvare <jdelvare@...e.com>,
 Benson Leung <bleung@...omium.org>, Guenter Roeck <groeck@...omium.org>,
 chrome-platform@...ts.linux.dev, linux-hwmon@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] hwmon: (cros_ec) Add reading target fan RPM function

On 3/14/25 01:52, Thomas Weißschuh wrote:
> On 2025-03-13 16:36:54-0700, Guenter Roeck wrote:
>> On 3/13/25 09:24, Thomas Weißschuh wrote:
>>> On 2025-03-13 12:47:43+0800, Sung-Chi Li wrote:
>>>> Implement the functionality of reading the target fan RPM setting from
>>>> ChromeOS embedded controller under framework.
>>>>
>>>> Signed-off-by: Sung-Chi Li <lschyi@...omium.org>
>>>> ---
>>>>    drivers/hwmon/cros_ec_hwmon.c | 18 ++++++++++++++++++
>>>>    1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
>>>> index b2fec0768301f116f49c57b8dbfb042b98a573e1..73bfcbbaf9531be6b753cfef8045fd5dab5b2ab3 100644
>>>> --- a/drivers/hwmon/cros_ec_hwmon.c
>>>> +++ b/drivers/hwmon/cros_ec_hwmon.c
>>>> @@ -36,6 +36,19 @@ static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index
>>>>    	return 0;
>>>>    }
>>>> +static int cros_ec_hwmon_read_fan_target(struct cros_ec_device *cros_ec, u8 index, int32_t *speed)
>>>
>>> int32_t is a userspace type. In the kernel use i32, or even better u32.
>>>
>>
>> Seems to be pretty widely used to complain about.
> 
> There is even a checkpatch.pl test for it, which should have triggered:
> PREFER_KERNEL_TYPES.
> 
>> $ git grep int32_t drivers/ | wc
>>    43662  192381 3555402
> 
> 33k of those are in generated amdgpu headers.
> This search also happens to include the more frequently used uint32_t.
> 
>> Also, in comparison:
>>
>> $ git grep i32 drivers/ | wc
>>      820    4009   68486
> 
> The numbers for u32 look a bit different:
> 
> $ git grep u32 drivers/ | wc
>      234768 1137059 17410570
> 
> 
> Also this specific driver already consistently uses uNN.
> This does look wrong:
> 
> 	int32_t target_rpm;
>   	u16 speed;
>   	u8 temp;
> 
_That_ is a much better argument.

Thanks,
Guenter