Re: [PATCH v2] iio: orientation: hid-sensor-rotation: use ext_scan_type

[srinivas pandruvada <srinivas.pandruvada@linux.intel.com>]

+Lixu

On Sun, 2026-04-12 at 15:26 +0100, Jonathan Cameron wrote:
> On Sun, 01 Mar 2026 17:46:48 -0600
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > Make use of ext_scan_type to handle the dynamic realbits size of
> > the
> > quaternion data. This lets us implement it using static data rather
> > than
> > having to duplicate the channel info for each driver instance.
> > 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---
> I'm going to apply this now, but would welcome any additional
> feedback
> from Srinivas or others.
> 
> Note, given this is next cycle material now I'll only push the tree
> out
> as testing until I can rebase on rc1.
> 

In real world I think report size is always 16 copying from spec.

    Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Thanks,
Srinivas


> Thanks,
> 
> Jonathan
> 
> > This is something I noticed we could do while looking at an
> > unrelated
> > bug. I've tested this using the same script from [1] and confirmed
> > that
> > that the scan type didn't change. Before and after are both:
> > 
> > $ cat in_rot_quaternion_type
> > le:s16/32X4>>0  
> > 
> > [1]:
> > https://lore.kernel.org/linux-iio/20260301-iio-fix-timestamp-alignment-v1-1-1a54980bfb90@baylibre.com/
> > ---
> > Changes in v2:
> > - Dropped DEV_ROT_SCAN_TYPE_8BIT.
> > - Tested using /dev/uhid.
> > - Link to v1:
> > https://lore.kernel.org/r/20260214-iio-hid-sensor-rotation-cleanup-v1-1-3aec9a533c0f@baylibre.com
> > ---
> >  drivers/iio/orientation/hid-sensor-rotation.c | 71
> > ++++++++++++++++-----------
> >  1 file changed, 43 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/iio/orientation/hid-sensor-rotation.c
> > b/drivers/iio/orientation/hid-sensor-rotation.c
> > index e759f91a710a..3cfd0b323514 100644
> > --- a/drivers/iio/orientation/hid-sensor-rotation.c
> > +++ b/drivers/iio/orientation/hid-sensor-rotation.c
> > @@ -34,6 +34,27 @@ static const u32
> > rotation_sensitivity_addresses[] = {
> >  	HID_USAGE_SENSOR_ORIENT_QUATERNION,
> >  };
> >  
> > +enum {
> > +	DEV_ROT_SCAN_TYPE_16BIT,
> > +	DEV_ROT_SCAN_TYPE_32BIT,
> > +};
> > +
> > +static const struct iio_scan_type dev_rot_scan_types[] = {
> > +	[DEV_ROT_SCAN_TYPE_16BIT] = {
> > +		.sign = 's',
> > +		.realbits = 16,
> > +		/* Storage bits has to stay 32 to not break
> > userspace. */
> > +		.storagebits = 32,
> > +		.repeat = 4,
> > +	},
> > +	[DEV_ROT_SCAN_TYPE_32BIT] = {
> > +		.sign = 's',
> > +		.realbits = 32,
> > +		.storagebits = 32,
> > +		.repeat = 4,
> > +	},
> > +};
> > +
> >  /* Channel definitions */
> >  static const struct iio_chan_spec dev_rot_channels[] = {
> >  	{
> > @@ -45,23 +66,14 @@ static const struct iio_chan_spec
> > dev_rot_channels[] = {
> >  					BIT(IIO_CHAN_INFO_OFFSET)
> > |
> >  					BIT(IIO_CHAN_INFO_SCALE) |
> >  					BIT(IIO_CHAN_INFO_HYSTERES
> > IS),
> > -		.scan_index = 0
> > +		.scan_index = 0,
> > +		.has_ext_scan_type = 1,
> > +		.ext_scan_type = dev_rot_scan_types,
> > +		.num_ext_scan_type =
> > ARRAY_SIZE(dev_rot_scan_types),
> >  	},
> >  	IIO_CHAN_SOFT_TIMESTAMP(1)
> >  };
> >  
> > -/* Adjust channel real bits based on report descriptor */
> > -static void dev_rot_adjust_channel_bit_mask(struct iio_chan_spec
> > *chan,
> > -						int size)
> > -{
> > -	chan->scan_type.sign = 's';
> > -	/* Real storage bits will change based on the report desc.
> > */
> > -	chan->scan_type.realbits = size * 8;
> > -	/* Maximum size of a sample to capture is u32 */
> > -	chan->scan_type.storagebits = sizeof(u32) * 8;
> > -	chan->scan_type.repeat = 4;
> > -}
> > -
> >  /* Channel read_raw handler */
> >  static int dev_rot_read_raw(struct iio_dev *indio_dev,
> >  				struct iio_chan_spec const *chan,
> > @@ -136,9 +148,25 @@ static int dev_rot_write_raw(struct iio_dev
> > *indio_dev,
> >  	return ret;
> >  }
> >  
> > +static int dev_rot_get_current_scan_type(const struct iio_dev
> > *indio_dev,
> > +					 const struct
> > iio_chan_spec *chan)
> > +{
> > +	struct dev_rot_state *rot_state = iio_priv(indio_dev);
> > +
> > +	switch (rot_state->quaternion.size / 4) {
> > +	case sizeof(s16):
> > +		return DEV_ROT_SCAN_TYPE_16BIT;
> > +	case sizeof(s32):
> > +		return DEV_ROT_SCAN_TYPE_32BIT;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> >  static const struct iio_info dev_rot_info = {
> >  	.read_raw_multi = &dev_rot_read_raw,
> >  	.write_raw = &dev_rot_write_raw,
> > +	.get_current_scan_type = &dev_rot_get_current_scan_type,
> >  };
> >  
> >  /* Callback handler to send event after all samples are received
> > and captured */
> > @@ -196,7 +224,6 @@ static int dev_rot_capture_sample(struct
> > hid_sensor_hub_device *hsdev,
> >  /* Parse report which is specific to an usage id*/
> >  static int dev_rot_parse_report(struct platform_device *pdev,
> >  				struct hid_sensor_hub_device
> > *hsdev,
> > -				struct iio_chan_spec *channels,
> >  				unsigned usage_id,
> >  				struct dev_rot_state *st)
> >  {
> > @@ -210,9 +237,6 @@ static int dev_rot_parse_report(struct
> > platform_device *pdev,
> >  	if (ret)
> >  		return ret;
> >  
> > -	dev_rot_adjust_channel_bit_mask(&channels[0],
> > -		st->quaternion.size / 4);
> > -
> >  	dev_dbg(&pdev->dev, "dev_rot %x:%x\n", st-
> > >quaternion.index,
> >  		st->quaternion.report_id);
> >  
> > @@ -271,22 +295,13 @@ static int hid_dev_rot_probe(struct
> > platform_device *pdev)
> >  		return ret;
> >  	}
> >  
> > -	indio_dev->channels = devm_kmemdup(&pdev->dev,
> > dev_rot_channels,
> > -					  
> > sizeof(dev_rot_channels),
> > -					   GFP_KERNEL);
> > -	if (!indio_dev->channels) {
> > -		dev_err(&pdev->dev, "failed to duplicate
> > channels\n");
> > -		return -ENOMEM;
> > -	}
> > -
> > -	ret = dev_rot_parse_report(pdev, hsdev,
> > -				   (struct iio_chan_spec
> > *)indio_dev->channels,
> > -					hsdev->usage, rot_state);
> > +	ret = dev_rot_parse_report(pdev, hsdev, hsdev->usage,
> > rot_state);
> >  	if (ret) {
> >  		dev_err(&pdev->dev, "failed to setup
> > attributes\n");
> >  		return ret;
> >  	}
> >  
> > +	indio_dev->channels = dev_rot_channels;
> >  	indio_dev->num_channels = ARRAY_SIZE(dev_rot_channels);
> >  	indio_dev->info = &dev_rot_info;
> >  	indio_dev->name = name;
> > 
> > ---
> > base-commit: 3fa5e5702a82d259897bd7e209469bc06368bf31
> > change-id: 20260214-iio-hid-sensor-rotation-cleanup-84e8410926ef
> > 
> > Best regards,