Re: [PATCH v3 2/6] iio: gyro: HID: hid-sensor-gyro-3d: Refactor channel initialization

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 15 May 2026 09:48:34 -0700
srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> On Mon, 2026-05-11 at 22:16 -0300, Natália Salvino André wrote:
> > Replace the local gyro_3d_adjust_channel_bit_mask() function with a
> > compound literal for scan_type initialization to improve code
> > readability.
> > 
> > Signed-off-by: Natália Salvino André <natalia.andre@ime.usp.br>
> > Co-developed-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
> > Signed-off-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
> > ---
> >  drivers/iio/gyro/hid-sensor-gyro-3d.c | 24 +++++++++---------------
> >  1 file changed, 9 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > index c340cc899a7c..4cbe4d93a0a1 100644
> > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > @@ -3,6 +3,7 @@
> >   * HID Sensors Driver
> >   * Copyright (c) 2012, Intel Corporation.
> >   */
> > +#include <linux/bitops.h>
> >  #include <linux/device.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/module.h>
> > @@ -82,17 +83,6 @@ static const struct iio_chan_spec
> > gyro_3d_channels[] = {
> >  	IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
> >  };
> >  
> > -/* Adjust channel real bits based on report descriptor */
> > -static void gyro_3d_adjust_channel_bit_mask(struct iio_chan_spec
> > *channels,
> > -						int channel, int
> > size)
> > -{
> > -	channels[channel].scan_type.sign = 's';
> > -	/* Real storage bits will change based on the report desc.
> > */
> > -	channels[channel].scan_type.realbits = size * 8;
> > -	/* Maximum size of a sample to capture is u32 */
> > -	channels[channel].scan_type.storagebits = sizeof(u32) * 8;
> > -}
> > -
> >  /* Channel read_raw handler */
> >  static int gyro_3d_read_raw(struct iio_dev *indio_dev,
> >  			      struct iio_chan_spec const *chan,
> > @@ -251,16 +241,20 @@ static int gyro_3d_parse_report(struct
> > platform_device *pdev,
> >  	int i;
> >  
> >  	for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) {
> > +		unsigned int ch = CHANNEL_SCAN_INDEX_X + i;
> > +  
> 
> When you do code change more than what you described, then you should
> add to commit description that you are also doing this at the same
> time.
Particularly true when it is less than ideal.  Like in previous
patch the loop shouldbe from CHANNEL_SCAN_INDEX_X to CHANNEL_SCAN_INDEX_Z
not 0 (the value is 0 but if we assume that then the sum for ch makes little
sense!)

> 
> Thanks,
> Srinivas
> 
> >  		ret = sensor_hub_input_get_attribute_info(hsdev,
> >  				HID_INPUT_REPORT,
> >  				usage_id,
> >  				HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXI
> > S + i,
> > -				&st->gyro[CHANNEL_SCAN_INDEX_X +
> > i]);
> > +				&st->gyro[ch]);
> >  		if (ret < 0)
> >  			break;
> > -		gyro_3d_adjust_channel_bit_mask(channels,
> > -				CHANNEL_SCAN_INDEX_X + i,
> > -				st->gyro[CHANNEL_SCAN_INDEX_X +
> > i].size);
> > +		channels[ch].scan_type = (struct iio_scan_type) {
> > +			.format = 's',
> > +			.realbits = BYTES_TO_BITS(st-  
> > >gyro[ch].size),  
> > +			.storagebits = BITS_PER_TYPE(u32),
> > +		};
> >  	}
> >  	dev_dbg(&pdev->dev, "gyro_3d %x:%x, %x:%x, %x:%x\n",
> >  			st->gyro[0].index,  
>