Re: [PATCH 3/7] iio: light: hid-sensor-als: use u32 instead of unsigned

[Maxwell Doose]

On Mon, 8 Jun 2026 19:40:47 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

> On Mon, 8 Jun 2026 08:37:38 +0200
> Joshua Crofts <joshua.crofts1@xxxxxxxxx> wrote:
> 
> > On Sun, 7 Jun 2026 at 22:54, Maxwell Doose <m32285159@xxxxxxxxx> wrote:  
> > >
> > > On Sat, Jun 6, 2026 at 7:19 AM Sanjay Chitroda
> > > <sanjayembeddedse@xxxxxxxxx> wrote:    
> > > >
> > > > From: Sanjay Chitroda <sanjayembeddedse@xxxxxxxxx>
> > > >
> > > > Prefer 'u32' instead of bare 'unsigned' variable to improve code
> > > > clarity and consistency with kernel style.
> > > >
> > > > Signed-off-by: Sanjay Chitroda <sanjayembeddedse@xxxxxxxxx>
> > > > ---
> > > >  drivers/iio/light/hid-sensor-als.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > >    
> > >
> > > Do we *need* u32 though? Usually these types are for places where we
> > > require specific bit-widths for a reason (e.g., reading a hardware
> > > register) but I'm not sure that's the case here. I could be totally
> > > wrong, in that case please correct me but otherwise I unfortunately
> > > don't see much value in this. That's just my personal opinion though,
> > > Jonathan may think otherwise.    
> > 
> > Aside from the array of usage ids that are u32 defined here:
> > https://elixir.bootlin.com/linux/v7.1-rc6/source/drivers/iio/light/hid-sensor-als.c#L46
> > 
> > there are additional structs used in the HID drivers that take u32 (I
> > had a similar
> > patch moving `unsigned` to `unsigned int`, but it was recommended to use the
> > types that the structs use, hence u32).
> >   
> Here the reason for the change is even simpler. These are callbacks assigned to:
> 
> struct hid_sensor_hub_callbacks {
> 	struct platform_device *pdev;
> 	int (*suspend)(struct hid_sensor_hub_device *hsdev, void *priv);
> 	int (*resume)(struct hid_sensor_hub_device *hsdev, void *priv);
> 	int (*capture_sample)(struct hid_sensor_hub_device *hsdev,
> 			u32 usage_id, size_t raw_len, char *raw_data,
> 			void *priv);
> 	int (*send_event)(struct hid_sensor_hub_device *hsdev, u32 usage_id,
> 			 void *priv);
> };
> 
> So given those signatures, these should always have been u32.
>

Ah then perhaps I was a bit harsh on Sanjay.

>
> I would like that clearly stated in the patch descriptions. Consistency
> is a bit too vague.
>

Agree though. But given this

Reviewed-by: Maxwell Doose <m32285159@xxxxxxxxx>

-- 
best regards,
max