Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1346882 > unrolled thread

[RFC PATCH 0/2] iio: introduce iio_{claim|release}_direct_mode()

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 0/2] iio: introduce iio_{claim|release}_direct_mode() Alison Schofield <amsfield22@gmail.com> - 2016-03-01 20:00 +0100
    [RFC PATCH 2/2] staging: iio: adc7192: use iio_{claim|release}_direct_mode() Alison Schofield <amsfield22@gmail.com> - 2016-03-01 20:10 +0100
    [RFC PATCH 1/2] iio: core: implement iio_{claim|release}_direct_mode() Alison Schofield <amsfield22@gmail.com> - 2016-03-01 20:10 +0100
      Re: [RFC PATCH 1/2] iio: core: implement  iio_{claim|release}_direct_mode() Lars-Peter Clausen <lars@metafoo.de> - 2016-03-02 14:30 +0100

#1346882 — [RFC PATCH 0/2] iio: introduce iio_{claim|release}_direct_mode()

This patchset introduces two helper functions to simplify driver code
requiring the device to be locked in direct mode during execution of a
code path. The staging driver ad7192 is updated to demonstrate usage.

This could be applied to approximately 18 known cases where the driver
is holding the lock in direct mode.  Unknown cases might be those that
should, but don't, hold the lock.

Alternate implementation: Generalize to support a claim on any mode.
Do iio_claim_mode(device,mode) where if the device is in *mode*, it
is guaranteed to stay that way until release is called. I considered
and rejected this option because a) not sure other modes would ever
need to be locked, and b) the semantic improvement is less when it
is generalized.
 
This patchset was inspired by a discussion on linux-iio:
http://www.spinics.net/lists/linux-iio/msg18540.html

Alison Schofield (2):
  iio: core: implement iio_{claim|release}_direct_mode()
  staging: iio: adc7192: use iio_{claim|release}_direct_mode()

 drivers/iio/industrialio-core.c  | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/staging/iio/adc/ad7192.c | 24 +++++++++---------------
 include/linux/iio/iio.h          |  2 ++
 3 files changed, 50 insertions(+), 15 deletions(-)

-- 
2.1.4

[toc] | [next] | [standalone]

#1346900 — [RFC PATCH 2/2] staging: iio: adc7192: use iio_{claim|release}_direct_mode()

Replace the code that guarantees the device stays in direct mode with
iio_{claim|release}_direct_mode() which does same.

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/staging/iio/adc/ad7192.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index f843f19..401ec91 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -349,11 +349,9 @@ static ssize_t ad7192_write_frequency(struct device *dev,
 	if (lval == 0)
 		return -EINVAL;
 
-	mutex_lock(&indio_dev->mlock);
-	if (iio_buffer_enabled(indio_dev)) {
-		mutex_unlock(&indio_dev->mlock);
+	ret = iio_claim_direct_mode(indio_dev);
+	if (ret)
 		return -EBUSY;
-	}
 
 	div = st->mclk / (lval * st->f_order * 1024);
 	if (div < 1 || div > 1023) {
@@ -366,7 +364,7 @@ static ssize_t ad7192_write_frequency(struct device *dev,
 	ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
 
 out:
-	mutex_unlock(&indio_dev->mlock);
+	iio_release_direct_mode(indio_dev);
 
 	return ret ? ret : len;
 }
@@ -434,11 +432,9 @@ static ssize_t ad7192_set(struct device *dev,
 	if (ret < 0)
 		return ret;
 
-	mutex_lock(&indio_dev->mlock);
-	if (iio_buffer_enabled(indio_dev)) {
-		mutex_unlock(&indio_dev->mlock);
+	ret = iio_claim_direct_mode(indio_dev);
+	if (ret)
 		return -EBUSY;
-	}
 
 	switch ((u32)this_attr->address) {
 	case AD7192_REG_GPOCON:
@@ -461,7 +457,7 @@ static ssize_t ad7192_set(struct device *dev,
 		ret = -EINVAL;
 	}
 
-	mutex_unlock(&indio_dev->mlock);
+	iio_release_direct_mode(indio_dev);
 
 	return ret ? ret : len;
 }
@@ -555,11 +551,9 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
 	int ret, i;
 	unsigned int tmp;
 
-	mutex_lock(&indio_dev->mlock);
-	if (iio_buffer_enabled(indio_dev)) {
-		mutex_unlock(&indio_dev->mlock);
+	ret = iio_claim_direct_mode(indio_dev);
+	if (ret)
 		return -EBUSY;
-	}
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
@@ -582,7 +576,7 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
 		ret = -EINVAL;
 	}
 
-	mutex_unlock(&indio_dev->mlock);
+	iio_release_direct_mode(indio_dev);
 
 	return ret;
 }
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1346908 — [RFC PATCH 1/2] iio: core: implement iio_{claim|release}_direct_mode()

It is often the case that the driver wants to be sure a device stays
in direct mode while it is executing a task or series of tasks.  To
accomplish this today, the driver performs this sequence: 1) take the
device state lock, 2)verify it is not in a buffered mode, 3) execute
some tasks, and 4) release that lock.

This patch introduces a pair of helper functions that simplify these
steps and make it more semantically expressive.

iio_claim_direct_mode()
        If the device is not in any buffered mode it is guaranteed
        to stay that way until iio_release_direct_mode() is called.

iio_release_direct_mode()
        Release the claim. Device is no longer guaranteed to stay
        in direct mode.

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
---
 drivers/iio/industrialio-core.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/iio/iio.h         |  2 ++
 2 files changed, 41 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 70cb7eb..f6f0c89 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -25,6 +25,7 @@
 #include <linux/slab.h>
 #include <linux/anon_inodes.h>
 #include <linux/debugfs.h>
+#include <linux/mutex.h>
 #include <linux/iio/iio.h>
 #include "iio_core.h"
 #include "iio_core_trigger.h"
@@ -1375,6 +1376,44 @@ void devm_iio_device_unregister(struct device *dev, struct iio_dev *indio_dev)
 }
 EXPORT_SYMBOL_GPL(devm_iio_device_unregister);
 
+/**
+ * iio_claim_direct_mode - Keep device in direct mode
+ * @indio_dev:	the iio_dev associated with the device
+ *
+ * If the device is in direct mode it is guaranteed to
+ * stay that way until iio_release_direct_mode() is called.
+ *
+ * Use with iio_release_direct_mode()
+ *
+ * Returns: 0 on success, -EINVAL on failure
+ */
+int iio_claim_direct_mode(struct iio_dev *indio_dev)
+{
+	mutex_lock(&indio_dev->mlock);
+
+	if (iio_buffer_enabled(indio_dev)) {
+		mutex_unlock(&indio_dev->mlock);
+		return -EINVAL;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(iio_claim_direct_mode);
+
+/**
+ * iio_release_direct_mode - releases claim on direct mode
+ * @indio_dev:	the iio_dev associated with the device
+ *
+ * Release the claim. Device is no longer guaranteed to stay
+ * in direct mode.
+ *
+ * Use with iio_claim_direct_mode()
+ */
+void iio_release_direct_mode(struct iio_dev *indio_dev)
+{
+	mutex_unlock(&indio_dev->mlock);
+}
+EXPORT_SYMBOL_GPL(iio_release_direct_mode);
+
 subsys_initcall(iio_init);
 module_exit(iio_exit);
 
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index ce9e9c1..9efe2af 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -527,6 +527,8 @@ void iio_device_unregister(struct iio_dev *indio_dev);
 int devm_iio_device_register(struct device *dev, struct iio_dev *indio_dev);
 void devm_iio_device_unregister(struct device *dev, struct iio_dev *indio_dev);
 int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp);
+int iio_claim_direct_mode(struct iio_dev *indio_dev);
+void iio_release_direct_mode(struct iio_dev *indio_dev);
 
 extern struct bus_type iio_bus_type;
 
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1348028 — Re: [RFC PATCH 1/2] iio: core: implement iio_{claim|release}_direct_mode()

On 03/01/2016 08:02 PM, Alison Schofield wrote:
> It is often the case that the driver wants to be sure a device stays
> in direct mode while it is executing a task or series of tasks.  To
> accomplish this today, the driver performs this sequence: 1) take the
> device state lock, 2)verify it is not in a buffered mode, 3) execute
> some tasks, and 4) release that lock.
> 
> This patch introduces a pair of helper functions that simplify these
> steps and make it more semantically expressive.
> 
> iio_claim_direct_mode()
>         If the device is not in any buffered mode it is guaranteed
>         to stay that way until iio_release_direct_mode() is called.
> 
> iio_release_direct_mode()
>         Release the claim. Device is no longer guaranteed to stay
>         in direct mode.
> 
> Signed-off-by: Alison Schofield <amsfield22@gmail.com>

Looks basically good.

> ---
>  drivers/iio/industrialio-core.c | 39 +++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/iio.h         |  2 ++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 70cb7eb..f6f0c89 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -25,6 +25,7 @@
>  #include <linux/slab.h>
>  #include <linux/anon_inodes.h>
>  #include <linux/debugfs.h>
> +#include <linux/mutex.h>
>  #include <linux/iio/iio.h>
>  #include "iio_core.h"
>  #include "iio_core_trigger.h"
> @@ -1375,6 +1376,44 @@ void devm_iio_device_unregister(struct device *dev, struct iio_dev *indio_dev)
>  }
>  EXPORT_SYMBOL_GPL(devm_iio_device_unregister);
>  
> +/**
> + * iio_claim_direct_mode - Keep device in direct mode
> + * @indio_dev:	the iio_dev associated with the device
> + *
> + * If the device is in direct mode it is guaranteed to
> + * stay that way until iio_release_direct_mode() is called.
> + *
> + * Use with iio_release_direct_mode()
> + *
> + * Returns: 0 on success, -EINVAL on failure
> + */
> +int iio_claim_direct_mode(struct iio_dev *indio_dev)

To be consistent with the reset of the API I'd use the iio_device_... prefix
here, same for the release function.

> +{
> +	mutex_lock(&indio_dev->mlock);
> +
> +	if (iio_buffer_enabled(indio_dev)) {
> +		mutex_unlock(&indio_dev->mlock);
> +		return -EINVAL;

-EINVAL doesn't make much sense here, -EBUSY is better.

> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(iio_claim_direct_mode);
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web