Re: [PATCH] iio: gyro: adis16260: remove indio_dev mlock

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 19 Sep 2019 14:57:16 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> The internal lock that is by the ADIS library should be sufficient to keep
> state consistent. There is no need for an extra lock.

I'm not sure that's true.  In theory we could get two different attempts
to set the sampling frequency running concurrently. That could lead to
a race between the point where we set the spi frequency for future
messages and the point where we set the devices sampling frequency.

Bang it stops working.  So, whilst it is arguably paranoid I think
you do need a lock here, but it should be something that is driver
local rather than mlock.

thanks,

Jonathan


> 
> This patch removes it.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>  drivers/iio/gyro/adis16260.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/iio/gyro/adis16260.c b/drivers/iio/gyro/adis16260.c
> index 207a0ce13439..0fa93d02062a 100644
> --- a/drivers/iio/gyro/adis16260.c
> +++ b/drivers/iio/gyro/adis16260.c
> @@ -293,7 +293,6 @@ static int adis16260_write_raw(struct iio_dev *indio_dev,
>  		addr = adis16260_addresses[chan->scan_index][1];
>  		return adis_write_reg_16(adis, addr, val);
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		mutex_lock(&indio_dev->mlock);
>  		if (spi_get_device_id(adis->spi)->driver_data)
>  			t = 256 / val;
>  		else
> @@ -310,7 +309,6 @@ static int adis16260_write_raw(struct iio_dev *indio_dev,
>  			adis->spi->max_speed_hz = ADIS16260_SPI_FAST;
>  		ret = adis_write_reg_8(adis, ADIS16260_SMPL_PRD, t);
>  
> -		mutex_unlock(&indio_dev->mlock);
>  		return ret;
>  	}
>  	return -EINVAL;