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

[Jonathan Cameron <jic23@kernel.org>]

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.

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_HYSTERESIS),
> -		.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,