Re: [PATCH] iio: gyro: adis16130: remove mlock usage

["Ardelean, Alexandru" <alexandru.Ardelean@analog.com>]

On Sat, 2019-10-05 at 13:45 +0100, Jonathan Cameron wrote:
> [External]
> 
> On Tue, 17 Sep 2019 19:10:23 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> 
> > The use of indio_dev's mlock is discouraged. The driver already defines
> > it's own `bus_lock` in `adis16130_spi_read()`, so using the mlock is
> > redundant.
> > 
> > The parts supported by this chip are obsoleted anyway, so for now we
> > just
> > remove mlock as part of a general cleanup, until the driver gets
> > removed.
> Hmm. Removing a device driver like this which isn't in staging is going
> to be controversial.  There may well be long term supported devices out
> there using it.  We have no way of knowing, so my inclination will be
> to leave it there unless it is a significant maintenance burden.

I'll admit I'm sometimes a bit trigger-happy [still] about removing drivers
that are marked obsolete on the company product-site.
This usually [for me] goes away with time [the trigger-happiness].

> 
> The drivers in staging are a different matter as we never made any
> 'promise' of supporting those!
> 
> I'll hazard a guess that mlock here was cut and paste from another driver
> and that driver supported buffered modes.  In those cases it would need
> to move to the utility functions to ensure we are not in buffered mode.
> 
> Here your fix is right though given the driver only support sysfs reads.
> 
> Applied to the togreg branch of iio.git and pushed out as testing for
> the autobuilders to play with it.
> 
> Thanks,
> 
> Jonathan
> 
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> >  drivers/iio/gyro/adis16130.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/iio/gyro/adis16130.c
> > b/drivers/iio/gyro/adis16130.c
> > index de3f66f89496..79e63c8a2ea8 100644
> > --- a/drivers/iio/gyro/adis16130.c
> > +++ b/drivers/iio/gyro/adis16130.c
> > @@ -76,9 +76,7 @@ static int adis16130_read_raw(struct iio_dev
> > *indio_dev,
> >  	switch (mask) {
> >  	case IIO_CHAN_INFO_RAW:
> >  		/* Take the iio_dev status lock */
> > -		mutex_lock(&indio_dev->mlock);
> >  		ret = adis16130_spi_read(indio_dev, chan->address, &temp);
> > -		mutex_unlock(&indio_dev->mlock);
> >  		if (ret)
> >  			return ret;
> >  		*val = temp;