On Sat, 5 Oct 2019 15:38:45 +0100 Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On Thu, 19 Sep 2019 14:57:16 +0300 > Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> 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. Thinking a bit more on this do we have a potential issue where we race with a read on the spi bus as we are changing this frequency? They might all be 'safe' but I haven't thought it through properly yet. > > thanks, > > Jonathan > > > > > > This patch removes it. > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> > > --- > > 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; >
