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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Spurious I2C hardware read of CONF0 register in trigger handler
--

commit bd3b6c2cea8a14ae14e9c9db625cc2c4fc898c75
Author: Javier Carrasco <javier.carrasco.cruz@gmail.com>

iio: light: veml6031x00: add support for triggered buffers

Add triggered buffer functionality for the two channels the device
provides (ALS and IR).

> diff --git a/drivers/iio/light/veml6031x00.c b/drivers/iio/light/veml6031x00.c
> index f3772577a05d5..3d5a27a0baaed 100644
> --- a/drivers/iio/light/veml6031x00.c
> +++ b/drivers/iio/light/veml6031x00.c
[ ... ]
> +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;
> +	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 {
> +		i = 0;
> +		iio_for_each_active_channel(iio, ch) {
> +			ret = regmap_bulk_read(map, iio->channels[ch].address,
> +					       &scan[i++], sizeof(*scan));

[Severity: Medium]
Will this loop perform a spurious I2C hardware read of the CONF0 register?

When userspace enables the timestamp channel and only one of the data channels
(ALS or IR), the first if condition fails and the code falls back to this
else block.

iio_for_each_active_channel() iterates over all set bits in the scan mask,
including the timestamp channel. Since .address is implicitly 0 for the
timestamp channel, does this mean regmap_bulk_read() will read from address
0 (VEML6031X00_REG_CONF0) when processing the timestamp channel?

Should there be a check to skip the timestamp channel before reading from the
hardware, for example by checking if iio->channels[ch].type != IIO_TIMESTAMP?

> +			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/20260901-veml6031x00-v8-0-532cb4f2168a@gmail.com?part=3