Re: [PATCH v2 1/2] staging: iio: ad7816: serialize ad7816_spi_read() with a mutex

[Joshua Crofts]

On Mon,  3 Aug 2026 15:15:19 +0300
Abdelnasser Hussein <abdelnasserhussein11@xxxxxxxxx> wrote:

> The ad7816_spi_read() path performs a sequence of SPI transfers and GPIO
> state changes that must not be interleaved with another read operation.
> 
> Without serialization, concurrent callers can interfere with each other,
> leading to inconsistent device state and incorrect data being returned.
> 
> Add a mutex to struct ad7816_chip_info and hold it across the entire read
> sequence to ensure exclusive access to the device.
> 
> Signed-off-by: Abdelnasser Hussein <abdelnasserhussein11@xxxxxxxxx>
> ---
>  drivers/staging/iio/adc/ad7816.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index 0e32a2295990..b5a0c2871e00 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -50,6 +50,7 @@ struct ad7816_chip_info {
>  	u8  oti_data[AD7816_CS_MAX + 1];
>  	u8  channel_id;	/* 0 always be temperature */
>  	u8  mode;
> +	struct mutex lock; /* protect device state during SPI transfers */
>  };
>  
>  enum ad7816_type {
> @@ -67,11 +68,14 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
>  	int ret;
>  	__be16 buf;
>  
> +	mutex_lock(&chip->lock);

This patch would benefit from using guard(mutex) from the <linux/cleanup.h>
header. 
> +
>  	gpiod_set_value(chip->rdwr_pin, 1);
>  	gpiod_set_value(chip->rdwr_pin, 0);
>  	ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip->channel_id));
>  	if (ret < 0) {
>  		dev_err(&spi_dev->dev, "SPI channel setting error\n");
> +		mutex_unlock(&chip->lock);
>  		return ret;
>  	}
>  	gpiod_set_value(chip->rdwr_pin, 1);
> @@ -94,11 +98,13 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
>  	ret = spi_read(spi_dev, &buf, sizeof(*data));
>  	if (ret < 0) {
>  		dev_err(&spi_dev->dev, "SPI data read error\n");
> +		mutex_unlock(&chip->lock);
> +
>  		return ret;
>  	}
>  
>  	*data = be16_to_cpu(buf);
> -
> +	mutex_unlock(&chip->lock);
>  	return ret;
>  }
>  
> @@ -359,7 +365,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
>  	if (!indio_dev)
>  		return -ENOMEM;
>  	chip = iio_priv(indio_dev);
> -
> +	mutex_init(&chip->lock);

devm_mutex_init() since the driver uses managed resources. Additionally,
please check the return value of the function and just return on failure.

>  	chip->spi_dev = spi_dev;
>  	for (i = 0; i <= AD7816_CS_MAX; i++)
>  		chip->oti_data[i] = 203;



-- 
Kind regards,
Joshua Crofts