Skip to contentSkip to conversation

Replace mlock with private lock and remove unnecessary goto

PATCH Superseded sayli karnik 2 participants 4 messages Mar 13, 2017

Subsystems (4)
Staging IIO Analog Devices IIO drivers Staging - industrial io

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

sayli karnik

author
#0 lore raw

v3 cover Replace mlock with private lock and remove unnecessary goto

The patch set replaces mlock with private lock for driver ad9834 and removes unnecessary goto.

sayli karnik (2):
staging: iio: ad9834: Use private driver lock instead of mlock
staging: iio: ad9834: Remove unnecessary goto statement

 drivers/staging/iio/frequency/ad9834.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH v3 1/2 staging: iio: ad9834: Use private driver lock instead of mlock

iio_dev->mlock should be used by the IIO core only for protecting device operating mode changes. ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
Replace mlock with a lock in the device's global data to protect hardware state changes.

Signed-off-by sayli karnik <[email protected]>

---
v2:
Fixed mistake by changing mutex_lock() to mutex_unlock()

 drivers/staging/iio/frequency/ad9834.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/staging/iio/frequency/ad9834.c +6 -4
index f92ff7f..60461a6 100644
@@ -63,6 +63,7 @@
  * @msg:		default spi message
  * @freq_xfer:		tuning word spi transfer
  * @freq_msg:		tuning word spi message
+ * @lock:		protect sensor state
  * @data:		spi transmit buffer
  * @freq_data:		tuning word spi transmit buffer
  */

@@ -77,6 +78,7 @@ struct ad9834_state {
 	struct spi_message		msg;
 	struct spi_transfer		freq_xfer[2];
 	struct spi_message		freq_msg;
+	struct mutex                    lock;   /* protect sensor state */
 
 	/*
 	 * DMA (thus cache coherency maintenance) requires the

@@ -149,7 +151,7 @@ static ssize_t ad9834_write(struct device *dev,
 	if (ret)
 		goto error_ret;
 
-	mutex_lock(&indio_dev->mlock);
+	mutex_lock(&st->lock);
 	switch ((u32)this_attr->address) {
 	case AD9834_REG_FREQ0:
 	case AD9834_REG_FREQ1:

@@ -207,7 +209,7 @@ static ssize_t ad9834_write(struct device *dev,
 	default:
 		ret = -ENODEV;
 	}
-	mutex_unlock(&indio_dev->mlock);
+	mutex_unlock(&st->lock);
 
 error_ret:
 	return ret ? ret : len;

@@ -224,7 +226,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
 	int ret = 0;
 	bool is_ad9833_7 = (st->devid == ID_AD9833) || (st->devid == ID_AD9837);
 
-	mutex_lock(&indio_dev->mlock);
+	mutex_lock(&st->lock);
 
 	switch ((u32)this_attr->address) {
 	case 0:

@@ -267,7 +269,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
 		st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
 		ret = spi_sync(st->spi, &st->msg);
 	}
-	mutex_unlock(&indio_dev->mlock);
+	mutex_unlock(&st->lock);
 
 	return ret ? ret : len;
 }
Signature · 1 line
2.7.4
#2 lore raw

Re: [PATCH v3 1/2] staging: iio: ad9834: Use private driver lock instead of mlock

Quoted 7 lines · sayli karnik

iio_dev->mlock should be used by the IIO core only for protecting device operating mode changes. ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
Replace mlock with a lock in the device's global data to protect hardware state changes.

Signed-off-by sayli karnik <[email protected]>

This looks OK, but there is one issue with this patch(and all the other lock changes). The lock needs to be initialized before it can be used. You need to call mutex_init() inside the probe function before the lock is used the first time.

Quoted 64 lines

---
v2:
Fixed mistake by changing mutex_lock() to mutex_unlock()

 drivers/staging/iio/frequency/ad9834.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/staging/iio/frequency/ad9834.c +6 -4
index f92ff7f..60461a6 100644
@@ -63,6 +63,7 @@
  * @msg:		default spi message
  * @freq_xfer:		tuning word spi transfer
  * @freq_msg:		tuning word spi message
+ * @lock:		protect sensor state
  * @data:		spi transmit buffer
  * @freq_data:		tuning word spi transmit buffer
  */

@@ -77,6 +78,7 @@ struct ad9834_state {
 	struct spi_message		msg;
 	struct spi_transfer		freq_xfer[2];
 	struct spi_message		freq_msg;
+	struct mutex                    lock;   /* protect sensor state */
 
 	/*
 	 * DMA (thus cache coherency maintenance) requires the

@@ -149,7 +151,7 @@ static ssize_t ad9834_write(struct device *dev,
 	if (ret)
 		goto error_ret;
 
-	mutex_lock(&indio_dev->mlock);
+	mutex_lock(&st->lock);
 	switch ((u32)this_attr->address) {
 	case AD9834_REG_FREQ0:
 	case AD9834_REG_FREQ1:

@@ -207,7 +209,7 @@ static ssize_t ad9834_write(struct device *dev,
 	default:
 		ret = -ENODEV;
 	}
-	mutex_unlock(&indio_dev->mlock);
+	mutex_unlock(&st->lock);
 
 error_ret:
 	return ret ? ret : len;

@@ -224,7 +226,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
 	int ret = 0;
 	bool is_ad9833_7 = (st->devid == ID_AD9833) || (st->devid == ID_AD9837);
 
-	mutex_lock(&indio_dev->mlock);
+	mutex_lock(&st->lock);
 
 	switch ((u32)this_attr->address) {
 	case 0:

@@ -267,7 +269,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
 		st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
 		ret = spi_sync(st->spi, &st->msg);
 	}
-	mutex_unlock(&indio_dev->mlock);
+	mutex_unlock(&st->lock);
 
 	return ret ? ret : len;
 }
author
#3 lore raw

PATCH v3 2/2 staging: iio: ad9834: Remove unnecessary goto statement

In reply to sayli karnik

The patch removes unnecessary use of goto statement.

Signed-off-by sayli karnik <[email protected]>

---
v3:
Uncompressed return logic to avoid calling the function twice

 drivers/staging/iio/frequency/ad9834.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/iio/frequency/ad9834.c +1 -2
index 60461a6..e18e0f2 100644
@@ -149,7 +149,7 @@ static ssize_t ad9834_write(struct device *dev,
 
 	ret = kstrtoul(buf, 10, &val);
 	if (ret)
-		goto error_ret;
+		return ret;
 
 	mutex_lock(&st->lock);
 	switch ((u32)this_attr->address) {

@@ -211,7 +211,6 @@ static ssize_t ad9834_write(struct device *dev,
 	}
 	mutex_unlock(&st->lock);
 
-error_ret:
 	return ret ? ret : len;
 }
Signature · 1 line
2.7.4