Replace mlock with private lock and improve code flow

PATCH Superseded sayli karnik 3 participants 6 messages Mar 12, 2017 – Mar 13, 2017

Subsystems (4)

Staging IIO Analog Devices IIO drivers Staging - industrial io

lore

Share & tools

lore

sayli karnik

author

2017-03-12 22:15 UTC #0 lore raw

v2 cover Replace mlock with private lock and improve code flow

The patch series replaces mlock with a private lock for driver ad9834 and improves code flow by eliminating excessive goto.

sayli karnik (2):
staging: iio: ad9834: Use private driver lock instead of mlock
staging: iio: ad9834: Remove unnecessary goto label and compress
return logic

 drivers/staging/iio/frequency/ad9834.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Signature · 1 line

2.7.4

sayli karnik

author

2017-03-12 22:17 UTC #1 lore raw

PATCH v2 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

sayli karnik

author

2017-03-12 22:19 UTC #2 lore raw

PATCH v2 2/2 staging: iio: ad9834: Remove unnecessary goto label and compress return logic

In reply to sayli karnik

The patch removes excessive goto label and compresses return logic.

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

---
 drivers/staging/iio/frequency/ad9834.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/staging/iio/frequency/ad9834.c +2 -4

index 60461a6..18f3e8f 100644
@@ -147,9 +147,8 @@ static ssize_t ad9834_write(struct device *dev,
 	int ret;
 	unsigned long val;
 
-	ret = kstrtoul(buf, 10, &val);
-	if (ret)
-		goto error_ret;
+	if (kstrtoul(buf, 10, &val))
+		return kstrtoul(buf, 10, &val);
 
 	mutex_lock(&st->lock);
 	switch ((u32)this_attr->address) {

@@ -211,7 +210,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

Alison Schofield

2017-03-12 22:27 UTC #3 lore raw

Re: [Outreachy kernel] [PATCH v2 2/2] staging: iio: ad9834: Remove unnecessary goto label and compress return logic

Quoted 12 lines · sayli karnik

The patch removes excessive goto label and compresses return logic.
Signed-off-by sayli karnik <[email protected]>
---
 drivers/staging/iio/frequency/ad9834.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/staging/iio/frequency/ad9834.c +0 -0
index 60461a6..18f3e8f 100644

drivers/staging/iio/frequency/ad9834.c static ssize_t ad9834_write(struct device *dev, · line 151

Quoted 8 lines

drivers/staging/iio/frequency/ad9834.c +2 -3

@@ -147,9 +147,8 @@ static ssize_t ad9834_write(struct device *dev,
 	int ret;
 	unsigned long val;
 
-	ret = kstrtoul(buf, 10, &val);
-	if (ret)
-		goto error_ret;
+	if (kstrtoul(buf, 10, &val))
+		return kstrtoul(buf, 10, &val);

Will a smaller change suffice?

ret = kstrtoul(buf, 10, &val);
if (ret)
- goto error_ret;
+ return ret;

alisons

Quoted 20 lines

drivers/staging/iio/frequency/ad9834.c +0 -1

 	mutex_lock(&st->lock);
 	switch ((u32)this_attr->address) {

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

Signature · 8 lines

2.7.4

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/59ddccc600cfd1594dcb4dadb44b80328fc06008.1489356140.git.karniksayli1995%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

sayli karnik

author

2017-03-13 07:17 UTC #4 lore raw

Re: [Outreachy kernel] [PATCH v2 2/2] staging: iio: ad9834: Remove unnecessary goto label and compress return logic

Quoted 16 lines · Alison Schofield

Quoted 15 lines · sayli karnik
The patch removes excessive goto label and compresses return logic.
Signed-off-by sayli karnik <[email protected]>
---
 drivers/staging/iio/frequency/ad9834.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/staging/iio/frequency/ad9834.c +0 -0
index 60461a6..18f3e8f 100644
@@ -147,9 +147,8 @@ static ssize_t ad9834_write(struct device *dev,
      int ret;
      unsigned long val;
 

Quoted 8 lines

drivers/staging/iio/frequency/ad9834.c static ssize_t ad9834_write(struct device *dev, · line 151
Quoted 5 lines
drivers/staging/iio/frequency/ad9834.c +2 -3
-     ret = kstrtoul(buf, 10, &val);
-     if (ret)
-             goto error_ret;
+     if (kstrtoul(buf, 10, &val))
+             return kstrtoul(buf, 10, &val);
Will a smaller change suffice?

Yes, sending a solution without having to call the function twice.

Quoted 34 lines

ret = kstrtoul(buf, 10, &val);
if (ret)
(quoted diff) +1 -1
-       goto error_ret;
+       return ret;
alisons
Quoted 14 lines
drivers/staging/iio/frequency/ad9834.c +0 -2
      mutex_lock(&st->lock);
      switch ((u32)this_attr->address) {
@@ -211,7 +210,6 @@ static ssize_t ad9834_write(struct device *dev,
      }
      mutex_unlock(&st->lock);
 
-error_ret:
      return ret ? ret : len;
 }
 
--
2.7.4
Quoted 6 lines
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/59ddccc600cfd1594dcb4dadb44b80328fc06008.1489356140.git.karniksayli1995%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170312222738.GB14070%40d830.WORKGROUP.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2017-03-12 22:28 UTC #5 lore raw

Re: [Outreachy kernel] [PATCH v2 2/2] staging: iio: ad9834: Remove unnecessary goto label and compress return logic

In reply to sayli karnik

Quoted 12 lines · sayli karnik

The patch removes excessive goto label and compresses return logic.
Signed-off-by sayli karnik <[email protected]>
---
 drivers/staging/iio/frequency/ad9834.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/staging/iio/frequency/ad9834.c +0 -0
index 60461a6..18f3e8f 100644

drivers/staging/iio/frequency/ad9834.c static ssize_t ad9834_write(struct device *dev, · line 151

Quoted 8 lines

drivers/staging/iio/frequency/ad9834.c +2 -3

@@ -147,9 +147,8 @@ static ssize_t ad9834_write(struct device *dev,
 	int ret;
 	unsigned long val;
 
-	ret = kstrtoul(buf, 10, &val);
-	if (ret)
-		goto error_ret;
+	if (kstrtoul(buf, 10, &val))
+		return kstrtoul(buf, 10, &val);

Not such a good idea. The function does some work, and there is no need to repeat it.

julia

Quoted 21 lines

drivers/staging/iio/frequency/ad9834.c +0 -2
 	mutex_lock(&st->lock);
 	switch ((u32)this_attr->address) {
@@ -211,7 +210,6 @@ static ssize_t ad9834_write(struct device *dev,
 	}
 	mutex_unlock(&st->lock);
 
-error_ret:
 	return ret ? ret : len;
 }
 
--
2.7.4
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/59ddccc600cfd1594dcb4dadb44b80328fc06008.1489356140.git.karniksayli1995%40gmail.com.
For more options, visit https://groups.google.com/d/optout.