Re: [PATCH v3 5/6] iio: light: ltr501: Add ltr329 driver support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The software trigger handler incorrectly maps the TIMESTAMP channel to the Proximity Sensor for ALS-only chips, causing an I2C polling timeout and trigger failure.
--

--- Patch [5]: [PATCH v3 5/6] iio: light: ltr501: Add ltr329 driver support ---
commit 62d63375d5ea8d7644dcba28928c83f753f6e2cf
Author: Esben Haabendal <[email protected]>

iio: light: ltr501: Add ltr329 driver support
    
This patch adds support for the LTR-329ALS-01 chip, which is similar to
the LTR-303ALS-01 but lacks interrupt support.

> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index b36bd10a02624..9d37872ceec58 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
[ ... ]
> @@ -1257,6 +1271,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
>  		.channels = ltr301_channels,
>  		.no_channels = ARRAY_SIZE(ltr301_channels),
>  	},
> +	[ltr329] = {
> +		.partid = 0x0A,
> +		.als_gain = ltr559_als_gain_tbl,
> +		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
> +		.als_mode_active = BIT(0),
> +		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
> +		.als_gain_shift = 2,
> +		.no_irq_support = true,
> +		.info_no_irq = &ltr301_info_no_irq,
> +		.channels = ltr301_channels,

[Severity: High]
This isn't a bug introduced by this patch, but will the trigger handler fail
for the LTR-329 when using ltr301_channels?

Looking at ltr301_channels, scan_index 2 is defined as the software
timestamp:

drivers/iio/light/ltr501.c:ltr301_channels[] = {
	...
	IIO_CHAN_SOFT_TIMESTAMP(2),
};

However, in ltr501_trigger_handler(), test_bit(2, ...) hardcodes the
assumption that scan_index 2 is the proximity sensor:

drivers/iio/light/ltr501.c:ltr501_trigger_handler() {
	...
	if (test_bit(2, indio_dev->active_scan_mask))
		mask |= LTR501_STATUS_PS_RDY;

	ret = ltr501_drdy(data, mask);
	...
}

If a user enables the IIO timestamp channel (scan_index 2) on an ALS-only
chip like the LTR-329, will ltr501_drdy() timeout waiting for the proximity
sensor ready bit, effectively breaking triggered buffers?

> +		.no_channels = ARRAY_SIZE(ltr301_channels),
> +	},
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5