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

[Guenter Roeck]

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