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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect HID attribute usage ID passed to `sensor_hub_input_get_attribute_info()`, breaking device initialization.
- [Low] Invalid struct member `.format` used in `struct iio_scan_type` initialization.
--

commit b8c3d6dce107c50c4bd37c0ec71526208891109a
Author: Natália Salvino André <natalia.andre@ime.usp.br>

iio: accel: HID: hid-sensor-accel-3d: Refactor channel initialization

This commit cleans up the channel initialization loop in the HID 3D
accelerometer driver. It replaces a local helper function with a compound
literal to directly initialize the channel scan type configuration.

> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> index 2ff591b3458f7..99ca67732bbe2 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c

[ ... ]

> @@ -297,19 +287,20 @@ static int accel_3d_parse_report(struct platform_device *pdev,
>  				struct accel_3d_state *st)
>  {
>  	int ret;
> -	int i;
>  
> -	for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) {
> +	for (unsigned int ch = CHANNEL_SCAN_INDEX_X; ch <= CHANNEL_SCAN_INDEX_Z; ch++) {
>  		ret = sensor_hub_input_get_attribute_info(hsdev,
>  				HID_INPUT_REPORT,
>  				usage_id,
> -				HID_USAGE_SENSOR_ACCEL_X_AXIS + i,
> -				&st->accel[CHANNEL_SCAN_INDEX_X + i]);
> +				ch,

The fourth argument to sensor_hub_input_get_attribute_info() is the
attribute usage ID. Previously, this passed HID_USAGE_SENSOR_ACCEL_X_AXIS + i.
Does passing the loop variable ch (which represents internal index values 
like 0, 1, 2) prevent the driver from finding the accelerometer fields in 
the HID report descriptor?

> +				&st->accel[ch]);
>  		if (ret < 0)
>  			break;
> -		accel_3d_adjust_channel_bit_mask(channels,
> -				CHANNEL_SCAN_INDEX_X + i,
> -				st->accel[CHANNEL_SCAN_INDEX_X + i].size);
> +		channels[ch].scan_type = (struct iio_scan_type) {
> +			.format = 's',

Does struct iio_scan_type have a format member? Looking at 
include/linux/iio/iio.h, it appears the correct member name for this
field is sign.

> +			.realbits = BYTES_TO_BITS(st->accel[ch].size),
> +			.storagebits = BITS_PER_TYPE(u32),
> +		};
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512012302.20883-1-natalia.andre@ime.usp.br?part=1