staging: adis16060_core: Replace mlock with buf_lock and refactor code

The IIO subsystem is redefining iio_dev->mlock to be used by the IIO core only for protecting device operating mode changes. ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.

In this driver, mlock was being used to protect hardware state changes. Replace it with buf_lock in the devices global data.

As buf_lock protects both the adis16060_spi_write() and adis16060_spi_read() functions and both are always called in pair. First write, then read. Thus, refactor the code to have one single function adis16060_spi_write_than_read() which is protected by the existing buf_lock.

Removed nested locks as the function adis16060_read_raw call a lock on &st->buf_lock and then calls the function adis16060_spi_write which again tries to get hold of the same lock.

The locks in adis16060_read_raw are dropped and now have a single safe function adis16060_spi_write_than_read with the locks inside it being called.

Signed-off-by simran singhal <[email protected]>

---

v7:
-Change subject
-Remove lock from read_raw instead of from

    function adis16060_spi_write_than_read().

 drivers/staging/iio/gyro/adis16060_core.c | 38 +++++++++----------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

drivers/staging/iio/gyro/adis16060_core.c +5 -12

index c9d46e7..193587c 100644
@@ -40,25 +40,18 @@ struct adis16060_state {
 
 static struct iio_dev *adis16060_iio_dev;
 
-static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
+static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+					 u8 conf, u16 *val)
 {
 	int ret;
 	struct adis16060_state *st = iio_priv(indio_dev);
 
 	mutex_lock(&st->buf_lock);
-	st->buf[2] = val; /* The last 8 bits clocked in are latched */
+	st->buf[2] = conf; /* The last 8 bits clocked in are latched */
 	ret = spi_write(st->us_w, st->buf, 3);
-	mutex_unlock(&st->buf_lock);
-
-	return ret;
-}
-
-static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
-{
-	int ret;
-	struct adis16060_state *st = iio_priv(indio_dev);
 
-	mutex_lock(&st->buf_lock);
+	if (ret < 0)
+		return ret;
 
 	ret = spi_read(st->us_r, st->buf, 3);
 

drivers/staging/iio/gyro/adis16060_core.c +4 -13

 	mutex_unlock(&st->buf_lock);
 
 	return ret;

@@ -83,20 +76,15 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 {
 	u16 tval = 0;
 	int ret;
+	struct adis16060_state *st = iio_priv(indio_dev);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		/* Take the iio_dev status lock */
-		mutex_lock(&indio_dev->mlock);
-		ret = adis16060_spi_write(indio_dev, chan->address);
+		ret = adis16060_spi_write_than_read(indio_dev,
+						    chan->address, &tval);
 		if (ret < 0)
-			goto out_unlock;
+			return ret;
 
-		ret = adis16060_spi_read(indio_dev, &tval);
-		if (ret < 0)
-			goto out_unlock;
-
-		mutex_unlock(&indio_dev->mlock);
 		*val = tval;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_OFFSET:

@@ -110,10 +98,6 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
 	}
 
 	return -EINVAL;
-
-out_unlock:
-	mutex_unlock(&indio_dev->mlock);
-	return ret;
 }
 
 static const struct iio_info adis16060_info = {

Hi simran,

[auto build test WARNING on iio/togreg]
[also build test WARNING on v4.11-rc3 next-20170322]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/simran-singhal/staging-adis16060_core-Replace-mlock-with-buf_lock-and-refactor-code/20170323-031726
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-randconfig-s0-03230201 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/staging/iio/gyro/adis16060_core.c: In function 'adis16060_read_raw':

Quoted 1 line

Quoted 1 line

drivers/staging/iio/gyro/adis16060_core.c:79: warning: unused variable 'st'

vim +/st +79 drivers/staging/iio/gyro/adis16060_core.c

    63		if (!ret)
    64			*val = ((st->buf[0] & 0x3) << 12) |
    65				 (st->buf[1] << 4) |
    66				 ((st->buf[2] >> 4) & 0xF);
    67		mutex_unlock(&st->buf_lock);
    68	
    69		return ret;
    70	}
    71	
    72	static int adis16060_read_raw(struct iio_dev *indio_dev,
    73				      struct iio_chan_spec const *chan,
    74				      int *val, int *val2,
    75				      long mask)
    76	{
    77		u16 tval = 0;
    78		int ret;

Quoted 1 line

79 struct adis16060_state *st = iio_priv(indio_dev);

    80	
    81		switch (mask) {
    82		case IIO_CHAN_INFO_RAW:
    83			ret = adis16060_spi_write_than_read(indio_dev,
    84							    chan->address, &tval);
    85			if (ret < 0)
    86				return ret;
    87	

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation