Re: [PATCH v7 3/4] iio: light: veml6031x00: add support for triggered buffers

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The trigger handler performs a spurious I2C read of the configuration register when the soft timestamp channel is enabled.
--

--- Patch [3]: [PATCH v7 3/4] iio: light: veml6031x00: add support for triggered buffers ---
commit ad14fde3718bccd7b3598fb39633f0d8bcdf444b
Author: Javier Carrasco <[email protected]>

iio: light: veml6031x00: add support for triggered buffers

This commit adds triggered buffer functionality for the ALS and IR channels
provided by the device.

> diff --git a/drivers/iio/light/veml6031x00.c b/drivers/iio/light/veml6031x00.c
> index 348bc28859d6f..e6ca98414a01c 100644
> --- a/drivers/iio/light/veml6031x00.c
> +++ b/drivers/iio/light/veml6031x00.c

[ ... ]

> @@ -482,6 +514,79 @@ static const struct iio_info veml6031x00_info = {

[ ... ]

> +static irqreturn_t veml6031x00_trig_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *iio = pf->indio_dev;
> +	struct veml6031x00_data *data = iio_priv(iio);
> +	struct regmap *map = data->regmap;
> +	IIO_DECLARE_BUFFER_WITH_TS(__le16, scan, 2) = { };
> +	unsigned int i = 0;
> +	int ch, ret;
> +
> +	if (test_bit(VEML6031X00_SCAN_ALS, iio->active_scan_mask) &&
> +	    test_bit(VEML6031X00_SCAN_IR, iio->active_scan_mask)) {
> +		ret = regmap_bulk_read(map, VEML6031X00_REG_ALS_L,
> +				       scan, 2 * sizeof(*scan));
> +		if (ret)
> +			goto done;
> +	} else {
> +		iio_for_each_active_channel(iio, ch) {
> +			ret = regmap_bulk_read(map, iio->channels[ch].address,
> +					       &scan[i++], sizeof(*scan));

[Severity: Medium]
Does this loop also execute for the timestamp channel if it is enabled?

If the soft timestamp channel is enabled without both ALS and IR being
enabled simultaneously, iio_for_each_active_channel() will iterate over it.
Because the timestamp channel's address field defaults to 0, does this
result in a spurious I2C read from address 0x00 (VEML6031X00_REG_CONF0) into
the buffer padding on every trigger event?

> +			if (ret)
> +				goto done;
> +		}
> +	}
> +
> +	iio_push_to_buffers_with_ts(iio, scan, sizeof(scan), pf->timestamp);
> +
> +done:
> +	iio_trigger_notify_done(iio->trig);
> +
> +	return IRQ_HANDLED;
> +}

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