Re: [PATCH v7 2/3] staging: iio: adc: ad7816: Serialize SPI operations

[Joshua Crofts <joshua.crofts1@gmail.com>]

On Tue, 15 Sep 2026 10:59:38 +0300
Abdelnasser Hussein <abdelnasserhussein11@gmail.com> wrote:

...

> +	ret = devm_mutex_init(&spi_dev->dev, &chip->lock);
> +	if (ret)
> +		return ret;
> +
>  	chip->spi_dev = spi_dev;
>  	for (i = 0; i <= AD7816_CS_MAX; i++)
>  		chip->oti_data[i] = 203;

The patch in itself is fine, but Sashiko points out that the mutex
could be added in the ad7816_store_mode/channel() functions. Nevertheless,
this patch only focuses on SPI transfers so you could add guards to the
GPIO functions in another patch (I don't think you need to send a v8 though,
just another patch after this series gets merged).

Does this newly added lock also need to be acquired in the sysfs store
functions?
If a userspace process concurrently writes to the mode or channel sysfs
attributes, the GPIO pin and device state can be modified without acquiring
chip->lock:
drivers/staging/iio/adc/ad7816.c:ad7816_store_mode() {
    ...
	if (strcmp(buf, "full") == 0) {
		gpiod_set_value(chip->rdwr_pin, 1);
		chip->mode = AD7816_FULL;
	} else {
    ...
}
drivers/staging/iio/adc/ad7816.c:ad7816_store_channel() {
    ...
	chip->channel_id = data;
    ...
}
Could this concurrent access corrupt the rdwr_pin state, chip->mode, or
chip->channel_id variables during an ongoing SPI transfer, leading to
malformed transactions or corrupted ADC readings?

-- 
Kind regards,
Joshua Crofts