[PATCH 1/4] staging:iio: Factor out event handling into its own file

[PATCH 1/4] staging:iio: Factor out event handling into its own file

[Lars-Peter Clausen]

The core iio file has gotten quite cluttered over time. This patch moves
the event handling code into its own file.

This has also the advantage that industrialio-core.c is now closer again to
its counterpart in the outofstaging branch.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/Makefile             |    2 +-
 drivers/staging/iio/iio_core.h           |    4 +
 drivers/staging/iio/industrialio-core.c  |  458 ----------------------------
 drivers/staging/iio/industrialio-event.c |  484 ++++++++++++++++++++++++++++++
 4 files changed, 489 insertions(+), 459 deletions(-)
 create mode 100644 drivers/staging/iio/industrialio-event.c

diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
index 1340aea..657710b 100644
--- a/drivers/staging/iio/Makefile
+++ b/drivers/staging/iio/Makefile
@@ -3,7 +3,7 @@
 #
 
 obj-$(CONFIG_IIO) += industrialio.o
-industrialio-y := industrialio-core.o
+industrialio-y := industrialio-core.o industrialio-event.o
 industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
 industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
 
diff --git a/drivers/staging/iio/iio_core.h b/drivers/staging/iio/iio_core.h
index ff27f13..3b20de3 100644
--- a/drivers/staging/iio/iio_core.h
+++ b/drivers/staging/iio/iio_core.h
@@ -60,4 +60,8 @@ static inline void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
 
 #endif
 
+int iio_device_register_eventset(struct iio_dev *indio_dev);
+void iio_device_unregister_eventset(struct iio_dev *indio_dev);
+int iio_event_getfd(struct iio_dev *indio_dev);
+
 #endif
diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
index 2eef85f..3089307 100644
--- a/drivers/staging/iio/industrialio-core.c
+++ b/drivers/staging/iio/industrialio-core.c
@@ -100,71 +100,6 @@ const struct iio_chan_spec
 	return NULL;
 }
 
-/**
- * struct iio_detected_event_list - list element for events that have occurred
- * @list:		linked list header
- * @ev:			the event itself
- */
-struct iio_detected_event_list {
-	struct list_head		list;
-	struct iio_event_data		ev;
-};
-
-/**
- * struct iio_event_interface - chrdev interface for an event line
- * @dev:		device assocated with event interface
- * @wait:		wait queue to allow blocking reads of events
- * @event_list_lock:	mutex to protect the list of detected events
- * @det_events:		list of detected events
- * @max_events:		maximum number of events before new ones are dropped
- * @current_events:	number of events in detected list
- * @flags:		file operations related flags including busy flag.
- */
-struct iio_event_interface {
-	wait_queue_head_t			wait;
-	struct mutex				event_list_lock;
-	struct list_head			det_events;
-	int					max_events;
-	int					current_events;
-	struct list_head dev_attr_list;
-	unsigned long flags;
-	struct attribute_group			group;
-};
-
-int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
-{
-	struct iio_event_interface *ev_int = indio_dev->event_interface;
-	struct iio_detected_event_list *ev;
-	int ret = 0;
-
-	/* Does anyone care? */
-	mutex_lock(&ev_int->event_list_lock);
-	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
-		if (ev_int->current_events == ev_int->max_events) {
-			mutex_unlock(&ev_int->event_list_lock);
-			return 0;
-		}
-		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
-		if (ev == NULL) {
-			ret = -ENOMEM;
-			mutex_unlock(&ev_int->event_list_lock);
-			goto error_ret;
-		}
-		ev->ev.id = ev_code;
-		ev->ev.timestamp = timestamp;
-
-		list_add_tail(&ev->list, &ev_int->det_events);
-		ev_int->current_events++;
-		mutex_unlock(&ev_int->event_list_lock);
-		wake_up_interruptible(&ev_int->wait);
-	} else
-		mutex_unlock(&ev_int->event_list_lock);
-
-error_ret:
-	return ret;
-}
-EXPORT_SYMBOL(iio_push_event);
-
 /* This turns up an awful lot */
 ssize_t iio_read_const_attr(struct device *dev,
 			    struct device_attribute *attr,
@@ -174,110 +109,6 @@ ssize_t iio_read_const_attr(struct device *dev,
 }
 EXPORT_SYMBOL(iio_read_const_attr);
 
-static ssize_t iio_event_chrdev_read(struct file *filep,
-				     char __user *buf,
-				     size_t count,
-				     loff_t *f_ps)
-{
-	struct iio_event_interface *ev_int = filep->private_data;
-	struct iio_detected_event_list *el;
-	size_t len = sizeof(el->ev);
-	int ret;
-
-	if (count < len)
-		return -EINVAL;
-
-	mutex_lock(&ev_int->event_list_lock);
-	if (list_empty(&ev_int->det_events)) {
-		if (filep->f_flags & O_NONBLOCK) {
-			ret = -EAGAIN;
-			goto error_mutex_unlock;
-		}
-		mutex_unlock(&ev_int->event_list_lock);
-		/* Blocking on device; waiting for something to be there */
-		ret = wait_event_interruptible(ev_int->wait,
-					       !list_empty(&ev_int
-							   ->det_events));
-		if (ret)
-			goto error_ret;
-		/* Single access device so no one else can get the data */
-		mutex_lock(&ev_int->event_list_lock);
-	}
-
-	el = list_first_entry(&ev_int->det_events,
-			      struct iio_detected_event_list,
-			      list);
-	if (copy_to_user(buf, &(el->ev), len)) {
-		ret = -EFAULT;
-		goto error_mutex_unlock;
-	}
-	list_del(&el->list);
-	ev_int->current_events--;
-	mutex_unlock(&ev_int->event_list_lock);
-	kfree(el);
-
-	return len;
-
-error_mutex_unlock:
-	mutex_unlock(&ev_int->event_list_lock);
-error_ret:
-
-	return ret;
-}
-
-static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
-{
-	struct iio_event_interface *ev_int = filep->private_data;
-	struct iio_detected_event_list *el, *t;
-
-	mutex_lock(&ev_int->event_list_lock);
-	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
-	/*
-	 * In order to maintain a clean state for reopening,
-	 * clear out any awaiting events. The mask will prevent
-	 * any new __iio_push_event calls running.
-	 */
-	list_for_each_entry_safe(el, t, &ev_int->det_events, list) {
-		list_del(&el->list);
-		kfree(el);
-	}
-	ev_int->current_events = 0;
-	mutex_unlock(&ev_int->event_list_lock);
-
-	return 0;
-}
-
-static const struct file_operations iio_event_chrdev_fileops = {
-	.read =  iio_event_chrdev_read,
-	.release = iio_event_chrdev_release,
-	.owner = THIS_MODULE,
-	.llseek = noop_llseek,
-};
-
-static int iio_event_getfd(struct iio_dev *indio_dev)
-{
-	struct iio_event_interface *ev_int = indio_dev->event_interface;
-	int fd;
-
-	if (ev_int == NULL)
-		return -ENODEV;
-
-	mutex_lock(&ev_int->event_list_lock);
-	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
-		mutex_unlock(&ev_int->event_list_lock);
-		return -EBUSY;
-	}
-	mutex_unlock(&ev_int->event_list_lock);
-	fd = anon_inode_getfd("iio:event",
-				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
-	if (fd < 0) {
-		mutex_lock(&ev_int->event_list_lock);
-		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
-		mutex_unlock(&ev_int->event_list_lock);
-	}
-	return fd;
-}
-
 static int __init iio_init(void)
 {
 	int ret;
@@ -726,295 +557,6 @@ static void iio_device_unregister_sysfs(struct iio_dev *indio_dev)
 	kfree(indio_dev->chan_attr_group.attrs);
 }
 
-static const char * const iio_ev_type_text[] = {
-	[IIO_EV_TYPE_THRESH] = "thresh",
-	[IIO_EV_TYPE_MAG] = "mag",
-	[IIO_EV_TYPE_ROC] = "roc",
-	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
-	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
-};
-
-static const char * const iio_ev_dir_text[] = {
-	[IIO_EV_DIR_EITHER] = "either",
-	[IIO_EV_DIR_RISING] = "rising",
-	[IIO_EV_DIR_FALLING] = "falling"
-};
-
-static ssize_t iio_ev_state_store(struct device *dev,
-				  struct device_attribute *attr,
-				  const char *buf,
-				  size_t len)
-{
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	int ret;
-	bool val;
-
-	ret = strtobool(buf, &val);
-	if (ret < 0)
-		return ret;
-
-	ret = indio_dev->info->write_event_config(indio_dev,
-						  this_attr->address,
-						  val);
-	return (ret < 0) ? ret : len;
-}
-
-static ssize_t iio_ev_state_show(struct device *dev,
-				 struct device_attribute *attr,
-				 char *buf)
-{
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	int val = indio_dev->info->read_event_config(indio_dev,
-						     this_attr->address);
-
-	if (val < 0)
-		return val;
-	else
-		return sprintf(buf, "%d\n", val);
-}
-
-static ssize_t iio_ev_value_show(struct device *dev,
-				 struct device_attribute *attr,
-				 char *buf)
-{
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	int val, ret;
-
-	ret = indio_dev->info->read_event_value(indio_dev,
-						this_attr->address, &val);
-	if (ret < 0)
-		return ret;
-
-	return sprintf(buf, "%d\n", val);
-}
-
-static ssize_t iio_ev_value_store(struct device *dev,
-				  struct device_attribute *attr,
-				  const char *buf,
-				  size_t len)
-{
-	struct iio_dev *indio_dev = dev_get_drvdata(dev);
-	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	unsigned long val;
-	int ret;
-
-	if (!indio_dev->info->write_event_value)
-		return -EINVAL;
-
-	ret = strict_strtoul(buf, 10, &val);
-	if (ret)
-		return ret;
-
-	ret = indio_dev->info->write_event_value(indio_dev, this_attr->address,
-						 val);
-	if (ret < 0)
-		return ret;
-
-	return len;
-}
-
-static int iio_device_add_event_sysfs(struct iio_dev *indio_dev,
-				      struct iio_chan_spec const *chan)
-{
-	int ret = 0, i, attrcount = 0;
-	u64 mask = 0;
-	char *postfix;
-	if (!chan->event_mask)
-		return 0;
-
-	for_each_set_bit(i, &chan->event_mask, sizeof(chan->event_mask)*8) {
-		postfix = kasprintf(GFP_KERNEL, "%s_%s_en",
-				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
-				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
-		if (postfix == NULL) {
-			ret = -ENOMEM;
-			goto error_ret;
-		}
-		if (chan->modified)
-			mask = IIO_MOD_EVENT_CODE(chan->type, 0, chan->channel,
-						  i/IIO_EV_DIR_MAX,
-						  i%IIO_EV_DIR_MAX);
-		else if (chan->differential)
-			mask = IIO_EVENT_CODE(chan->type,
-					      0, 0,
-					      i%IIO_EV_DIR_MAX,
-					      i/IIO_EV_DIR_MAX,
-					      0,
-					      chan->channel,
-					      chan->channel2);
-		else
-			mask = IIO_UNMOD_EVENT_CODE(chan->type,
-						    chan->channel,
-						    i/IIO_EV_DIR_MAX,
-						    i%IIO_EV_DIR_MAX);
-
-		ret = __iio_add_chan_devattr(postfix,
-					     chan,
-					     &iio_ev_state_show,
-					     iio_ev_state_store,
-					     mask,
-					     0,
-					     &indio_dev->dev,
-					     &indio_dev->event_interface->
-					     dev_attr_list);
-		kfree(postfix);
-		if (ret)
-			goto error_ret;
-		attrcount++;
-		postfix = kasprintf(GFP_KERNEL, "%s_%s_value",
-				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
-				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
-		if (postfix == NULL) {
-			ret = -ENOMEM;
-			goto error_ret;
-		}
-		ret = __iio_add_chan_devattr(postfix, chan,
-					     iio_ev_value_show,
-					     iio_ev_value_store,
-					     mask,
-					     0,
-					     &indio_dev->dev,
-					     &indio_dev->event_interface->
-					     dev_attr_list);
-		kfree(postfix);
-		if (ret)
-			goto error_ret;
-		attrcount++;
-	}
-	ret = attrcount;
-error_ret:
-	return ret;
-}
-
-static inline void __iio_remove_event_config_attrs(struct iio_dev *indio_dev)
-{
-	struct iio_dev_attr *p, *n;
-	list_for_each_entry_safe(p, n,
-				 &indio_dev->event_interface->
-				 dev_attr_list, l) {
-		kfree(p->dev_attr.attr.name);
-		kfree(p);
-	}
-}
-
-static inline int __iio_add_event_config_attrs(struct iio_dev *indio_dev)
-{
-	int j, ret, attrcount = 0;
-
-	INIT_LIST_HEAD(&indio_dev->event_interface->dev_attr_list);
-	/* Dynically created from the channels array */
-	for (j = 0; j < indio_dev->num_channels; j++) {
-		ret = iio_device_add_event_sysfs(indio_dev,
-						 &indio_dev->channels[j]);
-		if (ret < 0)
-			goto error_clear_attrs;
-		attrcount += ret;
-	}
-	return attrcount;
-
-error_clear_attrs:
-	__iio_remove_event_config_attrs(indio_dev);
-
-	return ret;
-}
-
-static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
-{
-	int j;
-
-	for (j = 0; j < indio_dev->num_channels; j++)
-		if (indio_dev->channels[j].event_mask != 0)
-			return true;
-	return false;
-}
-
-static void iio_setup_ev_int(struct iio_event_interface *ev_int)
-{
-	mutex_init(&ev_int->event_list_lock);
-	/* discussion point - make this variable? */
-	ev_int->max_events = 10;
-	ev_int->current_events = 0;
-	INIT_LIST_HEAD(&ev_int->det_events);
-	init_waitqueue_head(&ev_int->wait);
-}
-
-static const char *iio_event_group_name = "events";
-static int iio_device_register_eventset(struct iio_dev *indio_dev)
-{
-	struct iio_dev_attr *p;
-	int ret = 0, attrcount_orig = 0, attrcount, attrn;
-	struct attribute **attr;
-
-	if (!(indio_dev->info->event_attrs ||
-	      iio_check_for_dynamic_events(indio_dev)))
-		return 0;
-
-	indio_dev->event_interface =
-		kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
-	if (indio_dev->event_interface == NULL) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
-
-	iio_setup_ev_int(indio_dev->event_interface);
-	if (indio_dev->info->event_attrs != NULL) {
-		attr = indio_dev->info->event_attrs->attrs;
-		while (*attr++ != NULL)
-			attrcount_orig++;
-	}
-	attrcount = attrcount_orig;
-	if (indio_dev->channels) {
-		ret = __iio_add_event_config_attrs(indio_dev);
-		if (ret < 0)
-			goto error_free_setup_event_lines;
-		attrcount += ret;
-	}
-
-	indio_dev->event_interface->group.name = iio_event_group_name;
-	indio_dev->event_interface->group.attrs = kcalloc(attrcount + 1,
-							  sizeof(indio_dev->event_interface->group.attrs[0]),
-							  GFP_KERNEL);
-	if (indio_dev->event_interface->group.attrs == NULL) {
-		ret = -ENOMEM;
-		goto error_free_setup_event_lines;
-	}
-	if (indio_dev->info->event_attrs)
-		memcpy(indio_dev->event_interface->group.attrs,
-		       indio_dev->info->event_attrs->attrs,
-		       sizeof(indio_dev->event_interface->group.attrs[0])
-		       *attrcount_orig);
-	attrn = attrcount_orig;
-	/* Add all elements from the list. */
-	list_for_each_entry(p,
-			    &indio_dev->event_interface->dev_attr_list,
-			    l)
-		indio_dev->event_interface->group.attrs[attrn++] =
-			&p->dev_attr.attr;
-	indio_dev->groups[indio_dev->groupcounter++] =
-		&indio_dev->event_interface->group;
-
-	return 0;
-
-error_free_setup_event_lines:
-	__iio_remove_event_config_attrs(indio_dev);
-	kfree(indio_dev->event_interface);
-error_ret:
-
-	return ret;
-}
-
-static void iio_device_unregister_eventset(struct iio_dev *indio_dev)
-{
-	if (indio_dev->event_interface == NULL)
-		return;
-	__iio_remove_event_config_attrs(indio_dev);
-	kfree(indio_dev->event_interface->group.attrs);
-	kfree(indio_dev->event_interface);
-}
-
 static void iio_dev_release(struct device *device)
 {
 	struct iio_dev *indio_dev = container_of(device, struct iio_dev, dev);
diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
new file mode 100644
index 0000000..17ed582
--- /dev/null
+++ b/drivers/staging/iio/industrialio-event.c
@@ -0,0 +1,484 @@
+/* The industrial I/O core
+ *
+ * Copyright (c) 2008 Jonathan Cameron
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * Based on elements of hwmon and input subsystems.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/poll.h>
+#include <linux/wait.h>
+#include <linux/cdev.h>
+#include <linux/slab.h>
+#include <linux/kfifo.h>
+#include <linux/anon_inodes.h>
+#include "iio.h"
+#include "iio_core.h"
+#include "sysfs.h"
+#include "events.h"
+
+/**
+ * struct iio_detected_event_list - list element for events that have occurred
+ * @list:		linked list header
+ * @ev:			the event itself
+ */
+struct iio_detected_event_list {
+	struct list_head		list;
+	struct iio_event_data		ev;
+};
+
+/**
+ * struct iio_event_interface - chrdev interface for an event line
+ * @dev:		device assocated with event interface
+ * @wait:		wait queue to allow blocking reads of events
+ * @event_list_lock:	mutex to protect the list of detected events
+ * @det_events:		list of detected events
+ * @max_events:		maximum number of events before new ones are dropped
+ * @current_events:	number of events in detected list
+ * @flags:		file operations related flags including busy flag.
+ */
+struct iio_event_interface {
+	wait_queue_head_t			wait;
+	struct mutex				event_list_lock;
+	struct list_head			det_events;
+	int					max_events;
+	int					current_events;
+	struct list_head dev_attr_list;
+	unsigned long flags;
+	struct attribute_group			group;
+};
+
+int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
+{
+	struct iio_event_interface *ev_int = indio_dev->event_interface;
+	struct iio_detected_event_list *ev;
+	int ret = 0;
+
+	/* Does anyone care? */
+	mutex_lock(&ev_int->event_list_lock);
+	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
+		if (ev_int->current_events == ev_int->max_events) {
+			mutex_unlock(&ev_int->event_list_lock);
+			return 0;
+		}
+		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
+		if (ev == NULL) {
+			ret = -ENOMEM;
+			mutex_unlock(&ev_int->event_list_lock);
+			goto error_ret;
+		}
+		ev->ev.id = ev_code;
+		ev->ev.timestamp = timestamp;
+
+		list_add_tail(&ev->list, &ev_int->det_events);
+		ev_int->current_events++;
+		mutex_unlock(&ev_int->event_list_lock);
+		wake_up_interruptible(&ev_int->wait);
+	} else
+		mutex_unlock(&ev_int->event_list_lock);
+
+error_ret:
+	return ret;
+}
+EXPORT_SYMBOL(iio_push_event);
+
+static ssize_t iio_event_chrdev_read(struct file *filep,
+				     char __user *buf,
+				     size_t count,
+				     loff_t *f_ps)
+{
+	struct iio_event_interface *ev_int = filep->private_data;
+	struct iio_detected_event_list *el;
+	size_t len = sizeof(el->ev);
+	int ret;
+
+	if (count < len)
+		return -EINVAL;
+
+	mutex_lock(&ev_int->event_list_lock);
+	if (list_empty(&ev_int->det_events)) {
+		if (filep->f_flags & O_NONBLOCK) {
+			ret = -EAGAIN;
+			goto error_mutex_unlock;
+		}
+		mutex_unlock(&ev_int->event_list_lock);
+		/* Blocking on device; waiting for something to be there */
+		ret = wait_event_interruptible(ev_int->wait,
+					       !list_empty(&ev_int
+							   ->det_events));
+		if (ret)
+			goto error_ret;
+		/* Single access device so no one else can get the data */
+		mutex_lock(&ev_int->event_list_lock);
+	}
+
+	el = list_first_entry(&ev_int->det_events,
+			      struct iio_detected_event_list,
+			      list);
+	if (copy_to_user(buf, &(el->ev), len)) {
+		ret = -EFAULT;
+		goto error_mutex_unlock;
+	}
+	list_del(&el->list);
+	ev_int->current_events--;
+	mutex_unlock(&ev_int->event_list_lock);
+	kfree(el);
+
+	return len;
+
+error_mutex_unlock:
+	mutex_unlock(&ev_int->event_list_lock);
+error_ret:
+
+	return ret;
+}
+
+static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
+{
+	struct iio_event_interface *ev_int = filep->private_data;
+	struct iio_detected_event_list *el, *t;
+
+	mutex_lock(&ev_int->event_list_lock);
+	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
+	/*
+	 * In order to maintain a clean state for reopening,
+	 * clear out any awaiting events. The mask will prevent
+	 * any new __iio_push_event calls running.
+	 */
+	list_for_each_entry_safe(el, t, &ev_int->det_events, list) {
+		list_del(&el->list);
+		kfree(el);
+	}
+	ev_int->current_events = 0;
+	mutex_unlock(&ev_int->event_list_lock);
+
+	return 0;
+}
+
+static const struct file_operations iio_event_chrdev_fileops = {
+	.read =  iio_event_chrdev_read,
+	.release = iio_event_chrdev_release,
+	.owner = THIS_MODULE,
+	.llseek = noop_llseek,
+};
+
+int iio_event_getfd(struct iio_dev *indio_dev)
+{
+	struct iio_event_interface *ev_int = indio_dev->event_interface;
+	int fd;
+
+	if (ev_int == NULL)
+		return -ENODEV;
+
+	mutex_lock(&ev_int->event_list_lock);
+	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
+		mutex_unlock(&ev_int->event_list_lock);
+		return -EBUSY;
+	}
+	mutex_unlock(&ev_int->event_list_lock);
+	fd = anon_inode_getfd("iio:event",
+				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
+	if (fd < 0) {
+		mutex_lock(&ev_int->event_list_lock);
+		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
+		mutex_unlock(&ev_int->event_list_lock);
+	}
+	return fd;
+}
+
+static const char * const iio_ev_type_text[] = {
+	[IIO_EV_TYPE_THRESH] = "thresh",
+	[IIO_EV_TYPE_MAG] = "mag",
+	[IIO_EV_TYPE_ROC] = "roc",
+	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
+	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
+};
+
+static const char * const iio_ev_dir_text[] = {
+	[IIO_EV_DIR_EITHER] = "either",
+	[IIO_EV_DIR_RISING] = "rising",
+	[IIO_EV_DIR_FALLING] = "falling"
+};
+
+static ssize_t iio_ev_state_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t len)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+	int ret;
+	bool val;
+
+	ret = strtobool(buf, &val);
+	if (ret < 0)
+		return ret;
+
+	ret = indio_dev->info->write_event_config(indio_dev,
+						  this_attr->address,
+						  val);
+	return (ret < 0) ? ret : len;
+}
+
+static ssize_t iio_ev_state_show(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+	int val = indio_dev->info->read_event_config(indio_dev,
+						     this_attr->address);
+
+	if (val < 0)
+		return val;
+	else
+		return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t iio_ev_value_show(struct device *dev,
+				 struct device_attribute *attr,
+				 char *buf)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+	int val, ret;
+
+	ret = indio_dev->info->read_event_value(indio_dev,
+						this_attr->address, &val);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t iio_ev_value_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf,
+				  size_t len)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+	unsigned long val;
+	int ret;
+
+	if (!indio_dev->info->write_event_value)
+		return -EINVAL;
+
+	ret = strict_strtoul(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	ret = indio_dev->info->write_event_value(indio_dev, this_attr->address,
+						 val);
+	if (ret < 0)
+		return ret;
+
+	return len;
+}
+
+static int iio_device_add_event_sysfs(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan)
+{
+	int ret = 0, i, attrcount = 0;
+	u64 mask = 0;
+	char *postfix;
+	if (!chan->event_mask)
+		return 0;
+
+	for_each_set_bit(i, &chan->event_mask, sizeof(chan->event_mask)*8) {
+		postfix = kasprintf(GFP_KERNEL, "%s_%s_en",
+				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
+				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
+		if (postfix == NULL) {
+			ret = -ENOMEM;
+			goto error_ret;
+		}
+		if (chan->modified)
+			mask = IIO_MOD_EVENT_CODE(chan->type, 0, chan->channel,
+						  i/IIO_EV_DIR_MAX,
+						  i%IIO_EV_DIR_MAX);
+		else if (chan->differential)
+			mask = IIO_EVENT_CODE(chan->type,
+					      0, 0,
+					      i%IIO_EV_DIR_MAX,
+					      i/IIO_EV_DIR_MAX,
+					      0,
+					      chan->channel,
+					      chan->channel2);
+		else
+			mask = IIO_UNMOD_EVENT_CODE(chan->type,
+						    chan->channel,
+						    i/IIO_EV_DIR_MAX,
+						    i%IIO_EV_DIR_MAX);
+
+		ret = __iio_add_chan_devattr(postfix,
+					     chan,
+					     &iio_ev_state_show,
+					     iio_ev_state_store,
+					     mask,
+					     0,
+					     &indio_dev->dev,
+					     &indio_dev->event_interface->
+					     dev_attr_list);
+		kfree(postfix);
+		if (ret)
+			goto error_ret;
+		attrcount++;
+		postfix = kasprintf(GFP_KERNEL, "%s_%s_value",
+				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
+				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
+		if (postfix == NULL) {
+			ret = -ENOMEM;
+			goto error_ret;
+		}
+		ret = __iio_add_chan_devattr(postfix, chan,
+					     iio_ev_value_show,
+					     iio_ev_value_store,
+					     mask,
+					     0,
+					     &indio_dev->dev,
+					     &indio_dev->event_interface->
+					     dev_attr_list);
+		kfree(postfix);
+		if (ret)
+			goto error_ret;
+		attrcount++;
+	}
+	ret = attrcount;
+error_ret:
+	return ret;
+}
+
+static inline void __iio_remove_event_config_attrs(struct iio_dev *indio_dev)
+{
+	struct iio_dev_attr *p, *n;
+	list_for_each_entry_safe(p, n,
+				 &indio_dev->event_interface->
+				 dev_attr_list, l) {
+		kfree(p->dev_attr.attr.name);
+		kfree(p);
+	}
+}
+
+static inline int __iio_add_event_config_attrs(struct iio_dev *indio_dev)
+{
+	int j, ret, attrcount = 0;
+
+	INIT_LIST_HEAD(&indio_dev->event_interface->dev_attr_list);
+	/* Dynically created from the channels array */
+	for (j = 0; j < indio_dev->num_channels; j++) {
+		ret = iio_device_add_event_sysfs(indio_dev,
+						 &indio_dev->channels[j]);
+		if (ret < 0)
+			goto error_clear_attrs;
+		attrcount += ret;
+	}
+	return attrcount;
+
+error_clear_attrs:
+	__iio_remove_event_config_attrs(indio_dev);
+
+	return ret;
+}
+
+static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
+{
+	int j;
+
+	for (j = 0; j < indio_dev->num_channels; j++)
+		if (indio_dev->channels[j].event_mask != 0)
+			return true;
+	return false;
+}
+
+static void iio_setup_ev_int(struct iio_event_interface *ev_int)
+{
+	mutex_init(&ev_int->event_list_lock);
+	/* discussion point - make this variable? */
+	ev_int->max_events = 10;
+	ev_int->current_events = 0;
+	INIT_LIST_HEAD(&ev_int->det_events);
+	init_waitqueue_head(&ev_int->wait);
+}
+
+static const char *iio_event_group_name = "events";
+int iio_device_register_eventset(struct iio_dev *indio_dev)
+{
+	struct iio_dev_attr *p;
+	int ret = 0, attrcount_orig = 0, attrcount, attrn;
+	struct attribute **attr;
+
+	if (!(indio_dev->info->event_attrs ||
+	      iio_check_for_dynamic_events(indio_dev)))
+		return 0;
+
+	indio_dev->event_interface =
+		kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
+	if (indio_dev->event_interface == NULL) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+
+	iio_setup_ev_int(indio_dev->event_interface);
+	if (indio_dev->info->event_attrs != NULL) {
+		attr = indio_dev->info->event_attrs->attrs;
+		while (*attr++ != NULL)
+			attrcount_orig++;
+	}
+	attrcount = attrcount_orig;
+	if (indio_dev->channels) {
+		ret = __iio_add_event_config_attrs(indio_dev);
+		if (ret < 0)
+			goto error_free_setup_event_lines;
+		attrcount += ret;
+	}
+
+	indio_dev->event_interface->group.name = iio_event_group_name;
+	indio_dev->event_interface->group.attrs = kcalloc(attrcount + 1,
+							  sizeof(indio_dev->event_interface->group.attrs[0]),
+							  GFP_KERNEL);
+	if (indio_dev->event_interface->group.attrs == NULL) {
+		ret = -ENOMEM;
+		goto error_free_setup_event_lines;
+	}
+	if (indio_dev->info->event_attrs)
+		memcpy(indio_dev->event_interface->group.attrs,
+		       indio_dev->info->event_attrs->attrs,
+		       sizeof(indio_dev->event_interface->group.attrs[0])
+		       *attrcount_orig);
+	attrn = attrcount_orig;
+	/* Add all elements from the list. */
+	list_for_each_entry(p,
+			    &indio_dev->event_interface->dev_attr_list,
+			    l)
+		indio_dev->event_interface->group.attrs[attrn++] =
+			&p->dev_attr.attr;
+	indio_dev->groups[indio_dev->groupcounter++] =
+		&indio_dev->event_interface->group;
+
+	return 0;
+
+error_free_setup_event_lines:
+	__iio_remove_event_config_attrs(indio_dev);
+	kfree(indio_dev->event_interface);
+error_ret:
+
+	return ret;
+}
+
+void iio_device_unregister_eventset(struct iio_dev *indio_dev)
+{
+	if (indio_dev->event_interface == NULL)
+		return;
+	__iio_remove_event_config_attrs(indio_dev);
+	kfree(indio_dev->event_interface->group.attrs);
+	kfree(indio_dev->event_interface);
+}
-- 
1.7.7.3

[PATCH 2/4] staging:iio:events: Use kfifo for event queue

[Lars-Peter Clausen]

The current IIO event code uses a list to emulate FIFO like behaviour.
Just use a kfifo directly instead to implement the event queue.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/industrialio-event.c |   89 ++++++------------------------
 1 files changed, 18 insertions(+), 71 deletions(-)

diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
index 17ed582..50d03bd 100644
--- a/drivers/staging/iio/industrialio-event.c
+++ b/drivers/staging/iio/industrialio-event.c
@@ -26,31 +26,17 @@
 #include "events.h"
 
 /**
- * struct iio_detected_event_list - list element for events that have occurred
- * @list:		linked list header
- * @ev:			the event itself
- */
-struct iio_detected_event_list {
-	struct list_head		list;
-	struct iio_event_data		ev;
-};
-
-/**
  * struct iio_event_interface - chrdev interface for an event line
  * @dev:		device assocated with event interface
  * @wait:		wait queue to allow blocking reads of events
- * @event_list_lock:	mutex to protect the list of detected events
  * @det_events:		list of detected events
- * @max_events:		maximum number of events before new ones are dropped
- * @current_events:	number of events in detected list
  * @flags:		file operations related flags including busy flag.
  */
 struct iio_event_interface {
 	wait_queue_head_t			wait;
 	struct mutex				event_list_lock;
-	struct list_head			det_events;
-	int					max_events;
-	int					current_events;
+	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
+
 	struct list_head dev_attr_list;
 	unsigned long flags;
 	struct attribute_group			group;
@@ -59,33 +45,24 @@ struct iio_event_interface {
 int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
 {
 	struct iio_event_interface *ev_int = indio_dev->event_interface;
-	struct iio_detected_event_list *ev;
+	struct iio_event_data ev;
 	int ret = 0;
 
 	/* Does anyone care? */
 	mutex_lock(&ev_int->event_list_lock);
 	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
-		if (ev_int->current_events == ev_int->max_events) {
-			mutex_unlock(&ev_int->event_list_lock);
-			return 0;
-		}
-		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
-		if (ev == NULL) {
-			ret = -ENOMEM;
-			mutex_unlock(&ev_int->event_list_lock);
-			goto error_ret;
-		}
-		ev->ev.id = ev_code;
-		ev->ev.timestamp = timestamp;
 
-		list_add_tail(&ev->list, &ev_int->det_events);
-		ev_int->current_events++;
+		ev.id = ev_code;
+		ev.timestamp = timestamp;
+
+		ret = kfifo_put(&ev_int->det_events, &ev);
+
 		mutex_unlock(&ev_int->event_list_lock);
-		wake_up_interruptible(&ev_int->wait);
+		if (ret != 0)
+			wake_up_interruptible(&ev_int->wait);
 	} else
 		mutex_unlock(&ev_int->event_list_lock);
 
-error_ret:
 	return ret;
 }
 EXPORT_SYMBOL(iio_push_event);
@@ -96,15 +73,14 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
 				     loff_t *f_ps)
 {
 	struct iio_event_interface *ev_int = filep->private_data;
-	struct iio_detected_event_list *el;
-	size_t len = sizeof(el->ev);
+	unsigned int copied;
 	int ret;
 
-	if (count < len)
+	if (count < sizeof(struct iio_event_data))
 		return -EINVAL;
 
 	mutex_lock(&ev_int->event_list_lock);
-	if (list_empty(&ev_int->det_events)) {
+	if (kfifo_is_empty(&ev_int->det_events)) {
 		if (filep->f_flags & O_NONBLOCK) {
 			ret = -EAGAIN;
 			goto error_mutex_unlock;
@@ -112,52 +88,29 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
 		mutex_unlock(&ev_int->event_list_lock);
 		/* Blocking on device; waiting for something to be there */
 		ret = wait_event_interruptible(ev_int->wait,
-					       !list_empty(&ev_int
-							   ->det_events));
+					!kfifo_is_empty(&ev_int->det_events));
 		if (ret)
 			goto error_ret;
 		/* Single access device so no one else can get the data */
 		mutex_lock(&ev_int->event_list_lock);
 	}
 
-	el = list_first_entry(&ev_int->det_events,
-			      struct iio_detected_event_list,
-			      list);
-	if (copy_to_user(buf, &(el->ev), len)) {
-		ret = -EFAULT;
-		goto error_mutex_unlock;
-	}
-	list_del(&el->list);
-	ev_int->current_events--;
 	mutex_unlock(&ev_int->event_list_lock);
-	kfree(el);
-
-	return len;
+	ret = kfifo_to_user(&ev_int->det_events, buf, count, &copied);
 
 error_mutex_unlock:
 	mutex_unlock(&ev_int->event_list_lock);
 error_ret:
-
-	return ret;
+	return ret ? ret : copied;
 }
 
 static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
 {
 	struct iio_event_interface *ev_int = filep->private_data;
-	struct iio_detected_event_list *el, *t;
 
 	mutex_lock(&ev_int->event_list_lock);
 	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
-	/*
-	 * In order to maintain a clean state for reopening,
-	 * clear out any awaiting events. The mask will prevent
-	 * any new __iio_push_event calls running.
-	 */
-	list_for_each_entry_safe(el, t, &ev_int->det_events, list) {
-		list_del(&el->list);
-		kfree(el);
-	}
-	ev_int->current_events = 0;
+	kfifo_reset_out(&ev_int->det_events);
 	mutex_unlock(&ev_int->event_list_lock);
 
 	return 0;
@@ -269,9 +222,6 @@ static ssize_t iio_ev_value_store(struct device *dev,
 	unsigned long val;
 	int ret;
 
-	if (!indio_dev->info->write_event_value)
-		return -EINVAL;
-
 	ret = strict_strtoul(buf, 10, &val);
 	if (ret)
 		return ret;
@@ -402,10 +352,7 @@ static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
 static void iio_setup_ev_int(struct iio_event_interface *ev_int)
 {
 	mutex_init(&ev_int->event_list_lock);
-	/* discussion point - make this variable? */
-	ev_int->max_events = 10;
-	ev_int->current_events = 0;
-	INIT_LIST_HEAD(&ev_int->det_events);
+	INIT_KFIFO(ev_int->det_events);
 	init_waitqueue_head(&ev_int->wait);
 }
 
-- 
1.7.7.3

[PATCH 3/4] staging:iio:events: Use waitqueue lock to protect event queue

[Lars-Peter Clausen]

Use the waitqueue lock to protect the event queue instead of a custom mutex.
This has the advantage that we can call the waitqueue operations with the lock
held, which simplifies the code flow a bit.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/industrialio-event.c |   43 ++++++++++++-----------------
 1 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
index 50d03bd..ef4943a 100644
--- a/drivers/staging/iio/industrialio-event.c
+++ b/drivers/staging/iio/industrialio-event.c
@@ -34,7 +34,6 @@
  */
 struct iio_event_interface {
 	wait_queue_head_t			wait;
-	struct mutex				event_list_lock;
 	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
 
 	struct list_head dev_attr_list;
@@ -49,19 +48,17 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
 	int ret = 0;
 
 	/* Does anyone care? */
-	mutex_lock(&ev_int->event_list_lock);
+	spin_lock(&ev_int->wait.lock);
 	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
 
 		ev.id = ev_code;
 		ev.timestamp = timestamp;
 
 		ret = kfifo_put(&ev_int->det_events, &ev);
-
-		mutex_unlock(&ev_int->event_list_lock);
 		if (ret != 0)
-			wake_up_interruptible(&ev_int->wait);
-	} else
-		mutex_unlock(&ev_int->event_list_lock);
+			wake_up_locked(&ev_int->wait);
+	}
+	spin_unlock(&ev_int->wait.lock);
 
 	return ret;
 }
@@ -79,28 +76,25 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
 	if (count < sizeof(struct iio_event_data))
 		return -EINVAL;
 
-	mutex_lock(&ev_int->event_list_lock);
+	spin_lock(&ev_int->wait.lock);
 	if (kfifo_is_empty(&ev_int->det_events)) {
 		if (filep->f_flags & O_NONBLOCK) {
 			ret = -EAGAIN;
-			goto error_mutex_unlock;
+			goto error_unlock;
 		}
-		mutex_unlock(&ev_int->event_list_lock);
 		/* Blocking on device; waiting for something to be there */
-		ret = wait_event_interruptible(ev_int->wait,
+		ret = wait_event_interruptible_locked(ev_int->wait,
 					!kfifo_is_empty(&ev_int->det_events));
 		if (ret)
-			goto error_ret;
+			goto error_unlock;
 		/* Single access device so no one else can get the data */
-		mutex_lock(&ev_int->event_list_lock);
 	}
 
-	mutex_unlock(&ev_int->event_list_lock);
 	ret = kfifo_to_user(&ev_int->det_events, buf, count, &copied);
 
-error_mutex_unlock:
-	mutex_unlock(&ev_int->event_list_lock);
-error_ret:
+error_unlock:
+	spin_unlock(&ev_int->wait.lock);
+
 	return ret ? ret : copied;
 }
 
@@ -108,10 +102,10 @@ static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
 {
 	struct iio_event_interface *ev_int = filep->private_data;
 
-	mutex_lock(&ev_int->event_list_lock);
+	spin_lock(&ev_int->wait.lock);
 	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
 	kfifo_reset_out(&ev_int->det_events);
-	mutex_unlock(&ev_int->event_list_lock);
+	spin_unlock(&ev_int->wait.lock);
 
 	return 0;
 }
@@ -131,18 +125,18 @@ int iio_event_getfd(struct iio_dev *indio_dev)
 	if (ev_int == NULL)
 		return -ENODEV;
 
-	mutex_lock(&ev_int->event_list_lock);
+	spin_lock(&ev_int->wait.lock);
 	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
-		mutex_unlock(&ev_int->event_list_lock);
+		spin_unlock(&ev_int->wait.lock);
 		return -EBUSY;
 	}
-	mutex_unlock(&ev_int->event_list_lock);
+	spin_unlock(&ev_int->wait.lock);
 	fd = anon_inode_getfd("iio:event",
 				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
 	if (fd < 0) {
-		mutex_lock(&ev_int->event_list_lock);
+		spin_lock(&ev_int->wait.lock);
 		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
-		mutex_unlock(&ev_int->event_list_lock);
+		spin_unlock(&ev_int->wait.lock);
 	}
 	return fd;
 }
@@ -351,7 +345,6 @@ static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
 
 static void iio_setup_ev_int(struct iio_event_interface *ev_int)
 {
-	mutex_init(&ev_int->event_list_lock);
 	INIT_KFIFO(ev_int->det_events);
 	init_waitqueue_head(&ev_int->wait);
 }
-- 
1.7.7.3

[PATCH 4/4] staging:iio:events: Add poll support

[Lars-Peter Clausen]

Add poll support to the event queue. This will allow us to check for pending
events in a application's event loop using poll() or similar. Since we already
have support for blocking reads adding poll support as well is trivial.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/staging/iio/industrialio-event.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
index ef4943a..2263269 100644
--- a/drivers/staging/iio/industrialio-event.c
+++ b/drivers/staging/iio/industrialio-event.c
@@ -56,7 +56,7 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
 
 		ret = kfifo_put(&ev_int->det_events, &ev);
 		if (ret != 0)
-			wake_up_locked(&ev_int->wait);
+			wake_up_locked_poll(&ev_int->wait, POLLIN);
 	}
 	spin_unlock(&ev_int->wait.lock);
 
@@ -64,6 +64,25 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
 }
 EXPORT_SYMBOL(iio_push_event);
 
+/**
+ * iio_event_poll() - poll the event queue to find out if it has data
+ */
+static unsigned int iio_event_poll(struct file *filep,
+			     struct poll_table_struct *wait)
+{
+	struct iio_event_interface *ev_int = filep->private_data;
+	unsigned int events = 0;
+
+	poll_wait(filep, &ev_int->wait, wait);
+
+	spin_lock(&ev_int->wait.lock);
+	if (!kfifo_is_empty(&ev_int->det_events))
+		events = POLLIN | POLLRDNORM;
+	spin_unlock(&ev_int->wait.lock);
+
+	return events;
+}
+
 static ssize_t iio_event_chrdev_read(struct file *filep,
 				     char __user *buf,
 				     size_t count,
@@ -112,6 +131,7 @@ static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
 
 static const struct file_operations iio_event_chrdev_fileops = {
 	.read =  iio_event_chrdev_read,
+	.poll =  iio_event_poll,
 	.release = iio_event_chrdev_release,
 	.owner = THIS_MODULE,
 	.llseek = noop_llseek,
-- 
1.7.7.3

Re: [PATCH 1/4] staging:iio: Factor out event handling into its own file

[Jonathan Cameron]

On 12/16/2011 05:12 PM, Lars-Peter Clausen wrote:
> The core iio file has gotten quite cluttered over time. This patch moves
> the event handling code into its own file.
A small comment here wrt to ease of review. When doing
a whole scale move like this, it is helpful to state
where any necessary code changes were... That way I can
just assume you cut and paste the rest rather than having
to check that.
> 
> This has also the advantage that industrialio-core.c is now closer again to
> its counterpart in the outofstaging branch.
Definitely in favour of this change.

Assuming it isn't in a later patch, there are a few undocumented
structure elements that could do with documenting ;)
More 'interestingly' there are a few that don't exist and are
documented.  All my fault of course, but seeing as you are
cleaning this code up... (looks hopeful)
Obviously shouldn't be part of this patch.  Either insert
one fixing it first, or do it at the end as a cleanup patch.

So all in all, the patch is fine, I just noticed some unrelated bits
whilst reading it ;)  Events as you have no doubt noticed are
probably our most dubious corner (or were until this set).
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/staging/iio/Makefile             |    2 +-
>  drivers/staging/iio/iio_core.h           |    4 +
>  drivers/staging/iio/industrialio-core.c  |  458 ----------------------------
>  drivers/staging/iio/industrialio-event.c |  484 ++++++++++++++++++++++++++++++
>  4 files changed, 489 insertions(+), 459 deletions(-)
>  create mode 100644 drivers/staging/iio/industrialio-event.c
> 
> diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
> index 1340aea..657710b 100644
> --- a/drivers/staging/iio/Makefile
> +++ b/drivers/staging/iio/Makefile
> @@ -3,7 +3,7 @@
>  #
>  
>  obj-$(CONFIG_IIO) += industrialio.o
> -industrialio-y := industrialio-core.o
> +industrialio-y := industrialio-core.o industrialio-event.o
>  industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
>  industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
>  
> diff --git a/drivers/staging/iio/iio_core.h b/drivers/staging/iio/iio_core.h
> index ff27f13..3b20de3 100644
> --- a/drivers/staging/iio/iio_core.h
> +++ b/drivers/staging/iio/iio_core.h
> @@ -60,4 +60,8 @@ static inline void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
>  
>  #endif
>  
> +int iio_device_register_eventset(struct iio_dev *indio_dev);
> +void iio_device_unregister_eventset(struct iio_dev *indio_dev);
> +int iio_event_getfd(struct iio_dev *indio_dev);
> +
>  #endif
> diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
> index 2eef85f..3089307 100644
> --- a/drivers/staging/iio/industrialio-core.c
> +++ b/drivers/staging/iio/industrialio-core.c
> @@ -100,71 +100,6 @@ const struct iio_chan_spec
>  	return NULL;
>  }
>  
> -/**
> - * struct iio_detected_event_list - list element for events that have occurred
> - * @list:		linked list header
> - * @ev:			the event itself
> - */
> -struct iio_detected_event_list {
> -	struct list_head		list;
> -	struct iio_event_data		ev;
> -};
> -
> -/**
> - * struct iio_event_interface - chrdev interface for an event line
> - * @dev:		device assocated with event interface
> - * @wait:		wait queue to allow blocking reads of events
> - * @event_list_lock:	mutex to protect the list of detected events
> - * @det_events:		list of detected events
> - * @max_events:		maximum number of events before new ones are dropped
> - * @current_events:	number of events in detected list
> - * @flags:		file operations related flags including busy flag.
> - */
> -struct iio_event_interface {
> -	wait_queue_head_t			wait;
> -	struct mutex				event_list_lock;
> -	struct list_head			det_events;
> -	int					max_events;
> -	int					current_events;
> -	struct list_head dev_attr_list;
> -	unsigned long flags;
> -	struct attribute_group			group;
> -};
> -
> -int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
> -{
> -	struct iio_event_interface *ev_int = indio_dev->event_interface;
> -	struct iio_detected_event_list *ev;
> -	int ret = 0;
> -
> -	/* Does anyone care? */
> -	mutex_lock(&ev_int->event_list_lock);
> -	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
> -		if (ev_int->current_events == ev_int->max_events) {
> -			mutex_unlock(&ev_int->event_list_lock);
> -			return 0;
> -		}
> -		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
> -		if (ev == NULL) {
> -			ret = -ENOMEM;
> -			mutex_unlock(&ev_int->event_list_lock);
> -			goto error_ret;
> -		}
> -		ev->ev.id = ev_code;
> -		ev->ev.timestamp = timestamp;
> -
> -		list_add_tail(&ev->list, &ev_int->det_events);
> -		ev_int->current_events++;
> -		mutex_unlock(&ev_int->event_list_lock);
> -		wake_up_interruptible(&ev_int->wait);
> -	} else
> -		mutex_unlock(&ev_int->event_list_lock);
> -
> -error_ret:
> -	return ret;
> -}
> -EXPORT_SYMBOL(iio_push_event);
> -
>  /* This turns up an awful lot */
>  ssize_t iio_read_const_attr(struct device *dev,
>  			    struct device_attribute *attr,
> @@ -174,110 +109,6 @@ ssize_t iio_read_const_attr(struct device *dev,
>  }
>  EXPORT_SYMBOL(iio_read_const_attr);
>  
> -static ssize_t iio_event_chrdev_read(struct file *filep,
> -				     char __user *buf,
> -				     size_t count,
> -				     loff_t *f_ps)
> -{
> -	struct iio_event_interface *ev_int = filep->private_data;
> -	struct iio_detected_event_list *el;
> -	size_t len = sizeof(el->ev);
> -	int ret;
> -
> -	if (count < len)
> -		return -EINVAL;
> -
> -	mutex_lock(&ev_int->event_list_lock);
> -	if (list_empty(&ev_int->det_events)) {
> -		if (filep->f_flags & O_NONBLOCK) {
> -			ret = -EAGAIN;
> -			goto error_mutex_unlock;
> -		}
> -		mutex_unlock(&ev_int->event_list_lock);
> -		/* Blocking on device; waiting for something to be there */
> -		ret = wait_event_interruptible(ev_int->wait,
> -					       !list_empty(&ev_int
> -							   ->det_events));
> -		if (ret)
> -			goto error_ret;
> -		/* Single access device so no one else can get the data */
> -		mutex_lock(&ev_int->event_list_lock);
> -	}
> -
> -	el = list_first_entry(&ev_int->det_events,
> -			      struct iio_detected_event_list,
> -			      list);
> -	if (copy_to_user(buf, &(el->ev), len)) {
> -		ret = -EFAULT;
> -		goto error_mutex_unlock;
> -	}
> -	list_del(&el->list);
> -	ev_int->current_events--;
> -	mutex_unlock(&ev_int->event_list_lock);
> -	kfree(el);
> -
> -	return len;
> -
> -error_mutex_unlock:
> -	mutex_unlock(&ev_int->event_list_lock);
> -error_ret:
> -
> -	return ret;
> -}
> -
> -static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
> -{
> -	struct iio_event_interface *ev_int = filep->private_data;
> -	struct iio_detected_event_list *el, *t;
> -
> -	mutex_lock(&ev_int->event_list_lock);
> -	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
> -	/*
> -	 * In order to maintain a clean state for reopening,
> -	 * clear out any awaiting events. The mask will prevent
> -	 * any new __iio_push_event calls running.
> -	 */
> -	list_for_each_entry_safe(el, t, &ev_int->det_events, list) {
> -		list_del(&el->list);
> -		kfree(el);
> -	}
> -	ev_int->current_events = 0;
> -	mutex_unlock(&ev_int->event_list_lock);
> -
> -	return 0;
> -}
> -
> -static const struct file_operations iio_event_chrdev_fileops = {
> -	.read =  iio_event_chrdev_read,
> -	.release = iio_event_chrdev_release,
> -	.owner = THIS_MODULE,
> -	.llseek = noop_llseek,
> -};
> -
> -static int iio_event_getfd(struct iio_dev *indio_dev)
> -{
> -	struct iio_event_interface *ev_int = indio_dev->event_interface;
> -	int fd;
> -
> -	if (ev_int == NULL)
> -		return -ENODEV;
> -
> -	mutex_lock(&ev_int->event_list_lock);
> -	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
> -		mutex_unlock(&ev_int->event_list_lock);
> -		return -EBUSY;
> -	}
> -	mutex_unlock(&ev_int->event_list_lock);
> -	fd = anon_inode_getfd("iio:event",
> -				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
> -	if (fd < 0) {
> -		mutex_lock(&ev_int->event_list_lock);
> -		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
> -		mutex_unlock(&ev_int->event_list_lock);
> -	}
> -	return fd;
> -}
> -
>  static int __init iio_init(void)
>  {
>  	int ret;
> @@ -726,295 +557,6 @@ static void iio_device_unregister_sysfs(struct iio_dev *indio_dev)
>  	kfree(indio_dev->chan_attr_group.attrs);
>  }
>  
> -static const char * const iio_ev_type_text[] = {
> -	[IIO_EV_TYPE_THRESH] = "thresh",
> -	[IIO_EV_TYPE_MAG] = "mag",
> -	[IIO_EV_TYPE_ROC] = "roc",
> -	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
> -	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
> -};
> -
> -static const char * const iio_ev_dir_text[] = {
> -	[IIO_EV_DIR_EITHER] = "either",
> -	[IIO_EV_DIR_RISING] = "rising",
> -	[IIO_EV_DIR_FALLING] = "falling"
> -};
> -
> -static ssize_t iio_ev_state_store(struct device *dev,
> -				  struct device_attribute *attr,
> -				  const char *buf,
> -				  size_t len)
> -{
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> -	int ret;
> -	bool val;
> -
> -	ret = strtobool(buf, &val);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = indio_dev->info->write_event_config(indio_dev,
> -						  this_attr->address,
> -						  val);
> -	return (ret < 0) ? ret : len;
> -}
> -
> -static ssize_t iio_ev_state_show(struct device *dev,
> -				 struct device_attribute *attr,
> -				 char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> -	int val = indio_dev->info->read_event_config(indio_dev,
> -						     this_attr->address);
> -
> -	if (val < 0)
> -		return val;
> -	else
> -		return sprintf(buf, "%d\n", val);
> -}
> -
> -static ssize_t iio_ev_value_show(struct device *dev,
> -				 struct device_attribute *attr,
> -				 char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> -	int val, ret;
> -
> -	ret = indio_dev->info->read_event_value(indio_dev,
> -						this_attr->address, &val);
> -	if (ret < 0)
> -		return ret;
> -
> -	return sprintf(buf, "%d\n", val);
> -}
> -
> -static ssize_t iio_ev_value_store(struct device *dev,
> -				  struct device_attribute *attr,
> -				  const char *buf,
> -				  size_t len)
> -{
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> -	unsigned long val;
> -	int ret;
> -
> -	if (!indio_dev->info->write_event_value)
> -		return -EINVAL;
> -
> -	ret = strict_strtoul(buf, 10, &val);
> -	if (ret)
> -		return ret;
> -
> -	ret = indio_dev->info->write_event_value(indio_dev, this_attr->address,
> -						 val);
> -	if (ret < 0)
> -		return ret;
> -
> -	return len;
> -}
> -
> -static int iio_device_add_event_sysfs(struct iio_dev *indio_dev,
> -				      struct iio_chan_spec const *chan)
> -{
> -	int ret = 0, i, attrcount = 0;
> -	u64 mask = 0;
> -	char *postfix;
> -	if (!chan->event_mask)
> -		return 0;
> -
> -	for_each_set_bit(i, &chan->event_mask, sizeof(chan->event_mask)*8) {
> -		postfix = kasprintf(GFP_KERNEL, "%s_%s_en",
> -				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
> -				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
> -		if (postfix == NULL) {
> -			ret = -ENOMEM;
> -			goto error_ret;
> -		}
> -		if (chan->modified)
> -			mask = IIO_MOD_EVENT_CODE(chan->type, 0, chan->channel,
> -						  i/IIO_EV_DIR_MAX,
> -						  i%IIO_EV_DIR_MAX);
> -		else if (chan->differential)
> -			mask = IIO_EVENT_CODE(chan->type,
> -					      0, 0,
> -					      i%IIO_EV_DIR_MAX,
> -					      i/IIO_EV_DIR_MAX,
> -					      0,
> -					      chan->channel,
> -					      chan->channel2);
> -		else
> -			mask = IIO_UNMOD_EVENT_CODE(chan->type,
> -						    chan->channel,
> -						    i/IIO_EV_DIR_MAX,
> -						    i%IIO_EV_DIR_MAX);
> -
> -		ret = __iio_add_chan_devattr(postfix,
> -					     chan,
> -					     &iio_ev_state_show,
> -					     iio_ev_state_store,
> -					     mask,
> -					     0,
> -					     &indio_dev->dev,
> -					     &indio_dev->event_interface->
> -					     dev_attr_list);
> -		kfree(postfix);
> -		if (ret)
> -			goto error_ret;
> -		attrcount++;
> -		postfix = kasprintf(GFP_KERNEL, "%s_%s_value",
> -				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
> -				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
> -		if (postfix == NULL) {
> -			ret = -ENOMEM;
> -			goto error_ret;
> -		}
> -		ret = __iio_add_chan_devattr(postfix, chan,
> -					     iio_ev_value_show,
> -					     iio_ev_value_store,
> -					     mask,
> -					     0,
> -					     &indio_dev->dev,
> -					     &indio_dev->event_interface->
> -					     dev_attr_list);
> -		kfree(postfix);
> -		if (ret)
> -			goto error_ret;
> -		attrcount++;
> -	}
> -	ret = attrcount;
> -error_ret:
> -	return ret;
> -}
> -
> -static inline void __iio_remove_event_config_attrs(struct iio_dev *indio_dev)
> -{
> -	struct iio_dev_attr *p, *n;
> -	list_for_each_entry_safe(p, n,
> -				 &indio_dev->event_interface->
> -				 dev_attr_list, l) {
> -		kfree(p->dev_attr.attr.name);
> -		kfree(p);
> -	}
> -}
> -
> -static inline int __iio_add_event_config_attrs(struct iio_dev *indio_dev)
> -{
> -	int j, ret, attrcount = 0;
> -
> -	INIT_LIST_HEAD(&indio_dev->event_interface->dev_attr_list);
> -	/* Dynically created from the channels array */
> -	for (j = 0; j < indio_dev->num_channels; j++) {
> -		ret = iio_device_add_event_sysfs(indio_dev,
> -						 &indio_dev->channels[j]);
> -		if (ret < 0)
> -			goto error_clear_attrs;
> -		attrcount += ret;
> -	}
> -	return attrcount;
> -
> -error_clear_attrs:
> -	__iio_remove_event_config_attrs(indio_dev);
> -
> -	return ret;
> -}
> -
> -static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
> -{
> -	int j;
> -
> -	for (j = 0; j < indio_dev->num_channels; j++)
> -		if (indio_dev->channels[j].event_mask != 0)
> -			return true;
> -	return false;
> -}
> -
> -static void iio_setup_ev_int(struct iio_event_interface *ev_int)
> -{
> -	mutex_init(&ev_int->event_list_lock);
> -	/* discussion point - make this variable? */
> -	ev_int->max_events = 10;
> -	ev_int->current_events = 0;
> -	INIT_LIST_HEAD(&ev_int->det_events);
> -	init_waitqueue_head(&ev_int->wait);
> -}
> -
> -static const char *iio_event_group_name = "events";
> -static int iio_device_register_eventset(struct iio_dev *indio_dev)
> -{
> -	struct iio_dev_attr *p;
> -	int ret = 0, attrcount_orig = 0, attrcount, attrn;
> -	struct attribute **attr;
> -
> -	if (!(indio_dev->info->event_attrs ||
> -	      iio_check_for_dynamic_events(indio_dev)))
> -		return 0;
> -
> -	indio_dev->event_interface =
> -		kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> -	if (indio_dev->event_interface == NULL) {
> -		ret = -ENOMEM;
> -		goto error_ret;
> -	}
> -
> -	iio_setup_ev_int(indio_dev->event_interface);
> -	if (indio_dev->info->event_attrs != NULL) {
> -		attr = indio_dev->info->event_attrs->attrs;
> -		while (*attr++ != NULL)
> -			attrcount_orig++;
> -	}
> -	attrcount = attrcount_orig;
> -	if (indio_dev->channels) {
> -		ret = __iio_add_event_config_attrs(indio_dev);
> -		if (ret < 0)
> -			goto error_free_setup_event_lines;
> -		attrcount += ret;
> -	}
> -
> -	indio_dev->event_interface->group.name = iio_event_group_name;
> -	indio_dev->event_interface->group.attrs = kcalloc(attrcount + 1,
> -							  sizeof(indio_dev->event_interface->group.attrs[0]),
> -							  GFP_KERNEL);
> -	if (indio_dev->event_interface->group.attrs == NULL) {
> -		ret = -ENOMEM;
> -		goto error_free_setup_event_lines;
> -	}
> -	if (indio_dev->info->event_attrs)
> -		memcpy(indio_dev->event_interface->group.attrs,
> -		       indio_dev->info->event_attrs->attrs,
> -		       sizeof(indio_dev->event_interface->group.attrs[0])
> -		       *attrcount_orig);
> -	attrn = attrcount_orig;
> -	/* Add all elements from the list. */
> -	list_for_each_entry(p,
> -			    &indio_dev->event_interface->dev_attr_list,
> -			    l)
> -		indio_dev->event_interface->group.attrs[attrn++] =
> -			&p->dev_attr.attr;
> -	indio_dev->groups[indio_dev->groupcounter++] =
> -		&indio_dev->event_interface->group;
> -
> -	return 0;
> -
> -error_free_setup_event_lines:
> -	__iio_remove_event_config_attrs(indio_dev);
> -	kfree(indio_dev->event_interface);
> -error_ret:
> -
> -	return ret;
> -}
> -
> -static void iio_device_unregister_eventset(struct iio_dev *indio_dev)
> -{
> -	if (indio_dev->event_interface == NULL)
> -		return;
> -	__iio_remove_event_config_attrs(indio_dev);
> -	kfree(indio_dev->event_interface->group.attrs);
> -	kfree(indio_dev->event_interface);
> -}
> -
>  static void iio_dev_release(struct device *device)
>  {
>  	struct iio_dev *indio_dev = container_of(device, struct iio_dev, dev);
> diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
> new file mode 100644
> index 0000000..17ed582
> --- /dev/null
> +++ b/drivers/staging/iio/industrialio-event.c
> @@ -0,0 +1,484 @@
> +/* The industrial I/O core
> + *
> + * Copyright (c) 2008 Jonathan Cameron
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * Based on elements of hwmon and input subsystems.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/poll.h>
> +#include <linux/wait.h>
> +#include <linux/cdev.h>
> +#include <linux/slab.h>
> +#include <linux/kfifo.h>
> +#include <linux/anon_inodes.h>
> +#include "iio.h"
> +#include "iio_core.h"
> +#include "sysfs.h"
> +#include "events.h"
> +
> +/**
> + * struct iio_detected_event_list - list element for events that have occurred
> + * @list:		linked list header
> + * @ev:			the event itself
> + */
> +struct iio_detected_event_list {
> +	struct list_head		list;
> +	struct iio_event_data		ev;
> +};
> +
> +/**
> + * struct iio_event_interface - chrdev interface for an event line
> + * @dev:		device assocated with event interface
Doesn't exist.
> + * @wait:		wait queue to allow blocking reads of events
> + * @event_list_lock:	mutex to protect the list of detected events
> + * @det_events:		list of detected events
> + * @max_events:		maximum number of events before new ones are dropped
> + * @current_events:	number of events in detected list
> + * @flags:		file operations related flags including busy flag.
> + */
> +struct iio_event_interface {
> +	wait_queue_head_t			wait;
> +	struct mutex				event_list_lock;
> +	struct list_head			det_events;
> +	int					max_events;
> +	int					current_events;
undocumented
> +	struct list_head dev_attr_list;
> +	unsigned long flags;
undocumented
> +	struct attribute_group			group;
> +};
> +
> +int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
> +{
> +	struct iio_event_interface *ev_int = indio_dev->event_interface;
> +	struct iio_detected_event_list *ev;
> +	int ret = 0;
> +
> +	/* Does anyone care? */
> +	mutex_lock(&ev_int->event_list_lock);
> +	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
> +		if (ev_int->current_events == ev_int->max_events) {
> +			mutex_unlock(&ev_int->event_list_lock);
> +			return 0;
> +		}
> +		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
> +		if (ev == NULL) {
> +			ret = -ENOMEM;
> +			mutex_unlock(&ev_int->event_list_lock);
> +			goto error_ret;
> +		}
> +		ev->ev.id = ev_code;
> +		ev->ev.timestamp = timestamp;
> +
> +		list_add_tail(&ev->list, &ev_int->det_events);
> +		ev_int->current_events++;
> +		mutex_unlock(&ev_int->event_list_lock);
> +		wake_up_interruptible(&ev_int->wait);
> +	} else
> +		mutex_unlock(&ev_int->event_list_lock);
> +
> +error_ret:
> +	return ret;
> +}
> +EXPORT_SYMBOL(iio_push_event);
> +
> +static ssize_t iio_event_chrdev_read(struct file *filep,
> +				     char __user *buf,
> +				     size_t count,
> +				     loff_t *f_ps)
> +{
> +	struct iio_event_interface *ev_int = filep->private_data;
> +	struct iio_detected_event_list *el;
> +	size_t len = sizeof(el->ev);
> +	int ret;
> +
> +	if (count < len)
> +		return -EINVAL;
> +
> +	mutex_lock(&ev_int->event_list_lock);
> +	if (list_empty(&ev_int->det_events)) {
> +		if (filep->f_flags & O_NONBLOCK) {
> +			ret = -EAGAIN;
> +			goto error_mutex_unlock;
> +		}
> +		mutex_unlock(&ev_int->event_list_lock);
> +		/* Blocking on device; waiting for something to be there */
> +		ret = wait_event_interruptible(ev_int->wait,
> +					       !list_empty(&ev_int
> +							   ->det_events));
> +		if (ret)
> +			goto error_ret;
> +		/* Single access device so no one else can get the data */
> +		mutex_lock(&ev_int->event_list_lock);
> +	}
> +
> +	el = list_first_entry(&ev_int->det_events,
> +			      struct iio_detected_event_list,
> +			      list);
> +	if (copy_to_user(buf, &(el->ev), len)) {
> +		ret = -EFAULT;
> +		goto error_mutex_unlock;
> +	}
> +	list_del(&el->list);
> +	ev_int->current_events--;
> +	mutex_unlock(&ev_int->event_list_lock);
> +	kfree(el);
> +
> +	return len;
> +
> +error_mutex_unlock:
> +	mutex_unlock(&ev_int->event_list_lock);
> +error_ret:
> +
> +	return ret;
> +}
> +
> +static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
> +{
> +	struct iio_event_interface *ev_int = filep->private_data;
> +	struct iio_detected_event_list *el, *t;
> +
> +	mutex_lock(&ev_int->event_list_lock);
> +	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
> +	/*
> +	 * In order to maintain a clean state for reopening,
> +	 * clear out any awaiting events. The mask will prevent
> +	 * any new __iio_push_event calls running.
> +	 */
> +	list_for_each_entry_safe(el, t, &ev_int->det_events, list) {
> +		list_del(&el->list);
> +		kfree(el);
> +	}
> +	ev_int->current_events = 0;
> +	mutex_unlock(&ev_int->event_list_lock);
> +
> +	return 0;
> +}
> +
> +static const struct file_operations iio_event_chrdev_fileops = {
> +	.read =  iio_event_chrdev_read,
> +	.release = iio_event_chrdev_release,
> +	.owner = THIS_MODULE,
> +	.llseek = noop_llseek,
> +};
> +
> +int iio_event_getfd(struct iio_dev *indio_dev)
> +{
> +	struct iio_event_interface *ev_int = indio_dev->event_interface;
> +	int fd;
> +
> +	if (ev_int == NULL)
> +		return -ENODEV;
> +
> +	mutex_lock(&ev_int->event_list_lock);
> +	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
> +		mutex_unlock(&ev_int->event_list_lock);
> +		return -EBUSY;
> +	}
> +	mutex_unlock(&ev_int->event_list_lock);
> +	fd = anon_inode_getfd("iio:event",
> +				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
> +	if (fd < 0) {
> +		mutex_lock(&ev_int->event_list_lock);
> +		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
> +		mutex_unlock(&ev_int->event_list_lock);
> +	}
> +	return fd;
> +}
> +
> +static const char * const iio_ev_type_text[] = {
> +	[IIO_EV_TYPE_THRESH] = "thresh",
> +	[IIO_EV_TYPE_MAG] = "mag",
> +	[IIO_EV_TYPE_ROC] = "roc",
> +	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
> +	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
> +};
> +
> +static const char * const iio_ev_dir_text[] = {
> +	[IIO_EV_DIR_EITHER] = "either",
> +	[IIO_EV_DIR_RISING] = "rising",
> +	[IIO_EV_DIR_FALLING] = "falling"
> +};
> +
> +static ssize_t iio_ev_state_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf,
> +				  size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +	int ret;
> +	bool val;
> +
> +	ret = strtobool(buf, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = indio_dev->info->write_event_config(indio_dev,
> +						  this_attr->address,
> +						  val);
> +	return (ret < 0) ? ret : len;
> +}
> +
> +static ssize_t iio_ev_state_show(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +	int val = indio_dev->info->read_event_config(indio_dev,
> +						     this_attr->address);
> +
> +	if (val < 0)
> +		return val;
> +	else
> +		return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t iio_ev_value_show(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +	int val, ret;
> +
> +	ret = indio_dev->info->read_event_value(indio_dev,
> +						this_attr->address, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t iio_ev_value_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf,
> +				  size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +	unsigned long val;
> +	int ret;
> +
> +	if (!indio_dev->info->write_event_value)
> +		return -EINVAL;
> +
> +	ret = strict_strtoul(buf, 10, &val);
Again as you are in here, good oportunity to get rid
of this... (again a separate patch).
> +	if (ret)
> +		return ret;
> +
> +	ret = indio_dev->info->write_event_value(indio_dev, this_attr->address,
> +						 val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return len;
> +}
> +
> +static int iio_device_add_event_sysfs(struct iio_dev *indio_dev,
> +				      struct iio_chan_spec const *chan)
> +{
> +	int ret = 0, i, attrcount = 0;
> +	u64 mask = 0;
> +	char *postfix;
> +	if (!chan->event_mask)
> +		return 0;
> +
> +	for_each_set_bit(i, &chan->event_mask, sizeof(chan->event_mask)*8) {
> +		postfix = kasprintf(GFP_KERNEL, "%s_%s_en",
> +				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
> +				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
> +		if (postfix == NULL) {
> +			ret = -ENOMEM;
> +			goto error_ret;
> +		}
> +		if (chan->modified)
> +			mask = IIO_MOD_EVENT_CODE(chan->type, 0, chan->channel,
> +						  i/IIO_EV_DIR_MAX,
> +						  i%IIO_EV_DIR_MAX);
> +		else if (chan->differential)
> +			mask = IIO_EVENT_CODE(chan->type,
> +					      0, 0,
> +					      i%IIO_EV_DIR_MAX,
> +					      i/IIO_EV_DIR_MAX,
> +					      0,
> +					      chan->channel,
> +					      chan->channel2);
> +		else
> +			mask = IIO_UNMOD_EVENT_CODE(chan->type,
> +						    chan->channel,
> +						    i/IIO_EV_DIR_MAX,
> +						    i%IIO_EV_DIR_MAX);
> +
> +		ret = __iio_add_chan_devattr(postfix,
> +					     chan,
> +					     &iio_ev_state_show,
> +					     iio_ev_state_store,
> +					     mask,
> +					     0,
> +					     &indio_dev->dev,
> +					     &indio_dev->event_interface->
> +					     dev_attr_list);
> +		kfree(postfix);
> +		if (ret)
> +			goto error_ret;
> +		attrcount++;
> +		postfix = kasprintf(GFP_KERNEL, "%s_%s_value",
> +				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
> +				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
> +		if (postfix == NULL) {
> +			ret = -ENOMEM;
> +			goto error_ret;
> +		}
> +		ret = __iio_add_chan_devattr(postfix, chan,
> +					     iio_ev_value_show,
> +					     iio_ev_value_store,
> +					     mask,
> +					     0,
> +					     &indio_dev->dev,
> +					     &indio_dev->event_interface->
> +					     dev_attr_list);
> +		kfree(postfix);
> +		if (ret)
> +			goto error_ret;
> +		attrcount++;
> +	}
> +	ret = attrcount;
> +error_ret:
> +	return ret;
> +}
> +
> +static inline void __iio_remove_event_config_attrs(struct iio_dev *indio_dev)
> +{
> +	struct iio_dev_attr *p, *n;
> +	list_for_each_entry_safe(p, n,
> +				 &indio_dev->event_interface->
> +				 dev_attr_list, l) {
> +		kfree(p->dev_attr.attr.name);
> +		kfree(p);
> +	}
> +}
> +
> +static inline int __iio_add_event_config_attrs(struct iio_dev *indio_dev)
> +{
> +	int j, ret, attrcount = 0;
> +
> +	INIT_LIST_HEAD(&indio_dev->event_interface->dev_attr_list);
> +	/* Dynically created from the channels array */
> +	for (j = 0; j < indio_dev->num_channels; j++) {
> +		ret = iio_device_add_event_sysfs(indio_dev,
> +						 &indio_dev->channels[j]);
> +		if (ret < 0)
> +			goto error_clear_attrs;
> +		attrcount += ret;
> +	}
> +	return attrcount;
> +
> +error_clear_attrs:
> +	__iio_remove_event_config_attrs(indio_dev);
> +
> +	return ret;
> +}
> +
> +static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
> +{
> +	int j;
> +
> +	for (j = 0; j < indio_dev->num_channels; j++)
> +		if (indio_dev->channels[j].event_mask != 0)
> +			return true;
> +	return false;
> +}
> +
> +static void iio_setup_ev_int(struct iio_event_interface *ev_int)
> +{
> +	mutex_init(&ev_int->event_list_lock);
> +	/* discussion point - make this variable? */
> +	ev_int->max_events = 10;
> +	ev_int->current_events = 0;
> +	INIT_LIST_HEAD(&ev_int->det_events);
> +	init_waitqueue_head(&ev_int->wait);
> +}
> +
> +static const char *iio_event_group_name = "events";
> +int iio_device_register_eventset(struct iio_dev *indio_dev)
> +{
> +	struct iio_dev_attr *p;
> +	int ret = 0, attrcount_orig = 0, attrcount, attrn;
> +	struct attribute **attr;
> +
> +	if (!(indio_dev->info->event_attrs ||
> +	      iio_check_for_dynamic_events(indio_dev)))
> +		return 0;
> +
> +	indio_dev->event_interface =
> +		kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> +	if (indio_dev->event_interface == NULL) {
> +		ret = -ENOMEM;
> +		goto error_ret;
> +	}
> +
> +	iio_setup_ev_int(indio_dev->event_interface);
> +	if (indio_dev->info->event_attrs != NULL) {
> +		attr = indio_dev->info->event_attrs->attrs;
> +		while (*attr++ != NULL)
> +			attrcount_orig++;
> +	}
> +	attrcount = attrcount_orig;
> +	if (indio_dev->channels) {
> +		ret = __iio_add_event_config_attrs(indio_dev);
> +		if (ret < 0)
> +			goto error_free_setup_event_lines;
> +		attrcount += ret;
> +	}
> +
> +	indio_dev->event_interface->group.name = iio_event_group_name;
> +	indio_dev->event_interface->group.attrs = kcalloc(attrcount + 1,
> +							  sizeof(indio_dev->event_interface->group.attrs[0]),
> +							  GFP_KERNEL);
> +	if (indio_dev->event_interface->group.attrs == NULL) {
> +		ret = -ENOMEM;
> +		goto error_free_setup_event_lines;
> +	}
> +	if (indio_dev->info->event_attrs)
> +		memcpy(indio_dev->event_interface->group.attrs,
> +		       indio_dev->info->event_attrs->attrs,
> +		       sizeof(indio_dev->event_interface->group.attrs[0])
> +		       *attrcount_orig);
> +	attrn = attrcount_orig;
> +	/* Add all elements from the list. */
> +	list_for_each_entry(p,
> +			    &indio_dev->event_interface->dev_attr_list,
> +			    l)
> +		indio_dev->event_interface->group.attrs[attrn++] =
> +			&p->dev_attr.attr;
> +	indio_dev->groups[indio_dev->groupcounter++] =
> +		&indio_dev->event_interface->group;
> +
> +	return 0;
> +
> +error_free_setup_event_lines:
> +	__iio_remove_event_config_attrs(indio_dev);
> +	kfree(indio_dev->event_interface);
> +error_ret:
> +
> +	return ret;
> +}
> +
> +void iio_device_unregister_eventset(struct iio_dev *indio_dev)
> +{
> +	if (indio_dev->event_interface == NULL)
> +		return;
> +	__iio_remove_event_config_attrs(indio_dev);
> +	kfree(indio_dev->event_interface->group.attrs);
> +	kfree(indio_dev->event_interface);
> +}

Re: [PATCH 1/4] staging:iio: Factor out event handling into its own file

[Jonathan Cameron]

On 12/18/2011 06:19 PM, Jonathan Cameron wrote:
> On 12/16/2011 05:12 PM, Lars-Peter Clausen wrote:
>> The core iio file has gotten quite cluttered over time. This patch moves
>> the event handling code into its own file.
> A small comment here wrt to ease of review. When doing
> a whole scale move like this, it is helpful to state
> where any necessary code changes were... That way I can
> just assume you cut and paste the rest rather than having
> to check that.
>>
>> This has also the advantage that industrialio-core.c is now closer again to
>> its counterpart in the outofstaging branch.
> Definitely in favour of this change.
> 
> Assuming it isn't in a later patch, there are a few undocumented
> structure elements that could do with documenting ;)
> More 'interestingly' there are a few that don't exist and are
> documented.  All my fault of course, but seeing as you are
> cleaning this code up... (looks hopeful)
> Obviously shouldn't be part of this patch.  Either insert
> one fixing it first, or do it at the end as a cleanup patch.
> 
> So all in all, the patch is fine, I just noticed some unrelated bits
> whilst reading it ;)  Events as you have no doubt noticed are
> probably our most dubious corner (or were until this set).
>>
I really ought not to leave my build tests running in the background
whilst sending emails acking things.  Looks like you have some issues...

drivers/staging/iio/industrialio-event.c:84:17: error: undefined
identifier 'TASK_INTERRUPTIBLE'
drivers/staging/iio/industrialio-event.c:114:23: error: undefined
identifier 'TASK_INTERRUPTIBLE'
drivers/staging/iio/industrialio-event.c:114:23: error: undefined
identifier 'signal_pending'
drivers/staging/iio/industrialio-event.c:114:23: error: undefined
identifier 'schedule'

Can't chase this down right now, but I'm guessing missing header
at least on arm.

adding include of sched.h does the job.
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Acked-by: Jonathan Cameron <jic23@kernel.org>
>> ---
>>  drivers/staging/iio/Makefile             |    2 +-
>>  drivers/staging/iio/iio_core.h           |    4 +
>>  drivers/staging/iio/industrialio-core.c  |  458 ----------------------------
>>  drivers/staging/iio/industrialio-event.c |  484 ++++++++++++++++++++++++++++++
>>  4 files changed, 489 insertions(+), 459 deletions(-)
>>  create mode 100644 drivers/staging/iio/industrialio-event.c
>>
>> diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
>> index 1340aea..657710b 100644
>> --- a/drivers/staging/iio/Makefile
>> +++ b/drivers/staging/iio/Makefile
>> @@ -3,7 +3,7 @@
>>  #
>>  
>>  obj-$(CONFIG_IIO) += industrialio.o
>> -industrialio-y := industrialio-core.o
>> +industrialio-y := industrialio-core.o industrialio-event.o
>>  industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
>>  industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
>>  
>> diff --git a/drivers/staging/iio/iio_core.h b/drivers/staging/iio/iio_core.h
>> index ff27f13..3b20de3 100644
>> --- a/drivers/staging/iio/iio_core.h
>> +++ b/drivers/staging/iio/iio_core.h
>> @@ -60,4 +60,8 @@ static inline void iio_chrdev_buffer_release(struct iio_dev *indio_dev)
>>  
>>  #endif
>>  
>> +int iio_device_register_eventset(struct iio_dev *indio_dev);
>> +void iio_device_unregister_eventset(struct iio_dev *indio_dev);
>> +int iio_event_getfd(struct iio_dev *indio_dev);
>> +
>>  #endif
>> diff --git a/drivers/staging/iio/industrialio-core.c b/drivers/staging/iio/industrialio-core.c
>> index 2eef85f..3089307 100644
>> --- a/drivers/staging/iio/industrialio-core.c
>> +++ b/drivers/staging/iio/industrialio-core.c
>> @@ -100,71 +100,6 @@ const struct iio_chan_spec
>>  	return NULL;
>>  }
>>  
>> -/**
>> - * struct iio_detected_event_list - list element for events that have occurred
>> - * @list:		linked list header
>> - * @ev:			the event itself
>> - */
>> -struct iio_detected_event_list {
>> -	struct list_head		list;
>> -	struct iio_event_data		ev;
>> -};
>> -
>> -/**
>> - * struct iio_event_interface - chrdev interface for an event line
>> - * @dev:		device assocated with event interface
>> - * @wait:		wait queue to allow blocking reads of events
>> - * @event_list_lock:	mutex to protect the list of detected events
>> - * @det_events:		list of detected events
>> - * @max_events:		maximum number of events before new ones are dropped
>> - * @current_events:	number of events in detected list
>> - * @flags:		file operations related flags including busy flag.
>> - */
>> -struct iio_event_interface {
>> -	wait_queue_head_t			wait;
>> -	struct mutex				event_list_lock;
>> -	struct list_head			det_events;
>> -	int					max_events;
>> -	int					current_events;
>> -	struct list_head dev_attr_list;
>> -	unsigned long flags;
>> -	struct attribute_group			group;
>> -};
>> -
>> -int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
>> -{
>> -	struct iio_event_interface *ev_int = indio_dev->event_interface;
>> -	struct iio_detected_event_list *ev;
>> -	int ret = 0;
>> -
>> -	/* Does anyone care? */
>> -	mutex_lock(&ev_int->event_list_lock);
>> -	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>> -		if (ev_int->current_events == ev_int->max_events) {
>> -			mutex_unlock(&ev_int->event_list_lock);
>> -			return 0;
>> -		}
>> -		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
>> -		if (ev == NULL) {
>> -			ret = -ENOMEM;
>> -			mutex_unlock(&ev_int->event_list_lock);
>> -			goto error_ret;
>> -		}
>> -		ev->ev.id = ev_code;
>> -		ev->ev.timestamp = timestamp;
>> -
>> -		list_add_tail(&ev->list, &ev_int->det_events);
>> -		ev_int->current_events++;
>> -		mutex_unlock(&ev_int->event_list_lock);
>> -		wake_up_interruptible(&ev_int->wait);
>> -	} else
>> -		mutex_unlock(&ev_int->event_list_lock);
>> -
>> -error_ret:
>> -	return ret;
>> -}
>> -EXPORT_SYMBOL(iio_push_event);
>> -
>>  /* This turns up an awful lot */
>>  ssize_t iio_read_const_attr(struct device *dev,
>>  			    struct device_attribute *attr,
>> @@ -174,110 +109,6 @@ ssize_t iio_read_const_attr(struct device *dev,
>>  }
>>  EXPORT_SYMBOL(iio_read_const_attr);
>>  
>> -static ssize_t iio_event_chrdev_read(struct file *filep,
>> -				     char __user *buf,
>> -				     size_t count,
>> -				     loff_t *f_ps)
>> -{
>> -	struct iio_event_interface *ev_int = filep->private_data;
>> -	struct iio_detected_event_list *el;
>> -	size_t len = sizeof(el->ev);
>> -	int ret;
>> -
>> -	if (count < len)
>> -		return -EINVAL;
>> -
>> -	mutex_lock(&ev_int->event_list_lock);
>> -	if (list_empty(&ev_int->det_events)) {
>> -		if (filep->f_flags & O_NONBLOCK) {
>> -			ret = -EAGAIN;
>> -			goto error_mutex_unlock;
>> -		}
>> -		mutex_unlock(&ev_int->event_list_lock);
>> -		/* Blocking on device; waiting for something to be there */
>> -		ret = wait_event_interruptible(ev_int->wait,
>> -					       !list_empty(&ev_int
>> -							   ->det_events));
>> -		if (ret)
>> -			goto error_ret;
>> -		/* Single access device so no one else can get the data */
>> -		mutex_lock(&ev_int->event_list_lock);
>> -	}
>> -
>> -	el = list_first_entry(&ev_int->det_events,
>> -			      struct iio_detected_event_list,
>> -			      list);
>> -	if (copy_to_user(buf, &(el->ev), len)) {
>> -		ret = -EFAULT;
>> -		goto error_mutex_unlock;
>> -	}
>> -	list_del(&el->list);
>> -	ev_int->current_events--;
>> -	mutex_unlock(&ev_int->event_list_lock);
>> -	kfree(el);
>> -
>> -	return len;
>> -
>> -error_mutex_unlock:
>> -	mutex_unlock(&ev_int->event_list_lock);
>> -error_ret:
>> -
>> -	return ret;
>> -}
>> -
>> -static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
>> -{
>> -	struct iio_event_interface *ev_int = filep->private_data;
>> -	struct iio_detected_event_list *el, *t;
>> -
>> -	mutex_lock(&ev_int->event_list_lock);
>> -	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>> -	/*
>> -	 * In order to maintain a clean state for reopening,
>> -	 * clear out any awaiting events. The mask will prevent
>> -	 * any new __iio_push_event calls running.
>> -	 */
>> -	list_for_each_entry_safe(el, t, &ev_int->det_events, list) {
>> -		list_del(&el->list);
>> -		kfree(el);
>> -	}
>> -	ev_int->current_events = 0;
>> -	mutex_unlock(&ev_int->event_list_lock);
>> -
>> -	return 0;
>> -}
>> -
>> -static const struct file_operations iio_event_chrdev_fileops = {
>> -	.read =  iio_event_chrdev_read,
>> -	.release = iio_event_chrdev_release,
>> -	.owner = THIS_MODULE,
>> -	.llseek = noop_llseek,
>> -};
>> -
>> -static int iio_event_getfd(struct iio_dev *indio_dev)
>> -{
>> -	struct iio_event_interface *ev_int = indio_dev->event_interface;
>> -	int fd;
>> -
>> -	if (ev_int == NULL)
>> -		return -ENODEV;
>> -
>> -	mutex_lock(&ev_int->event_list_lock);
>> -	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>> -		mutex_unlock(&ev_int->event_list_lock);
>> -		return -EBUSY;
>> -	}
>> -	mutex_unlock(&ev_int->event_list_lock);
>> -	fd = anon_inode_getfd("iio:event",
>> -				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
>> -	if (fd < 0) {
>> -		mutex_lock(&ev_int->event_list_lock);
>> -		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>> -		mutex_unlock(&ev_int->event_list_lock);
>> -	}
>> -	return fd;
>> -}
>> -
>>  static int __init iio_init(void)
>>  {
>>  	int ret;
>> @@ -726,295 +557,6 @@ static void iio_device_unregister_sysfs(struct iio_dev *indio_dev)
>>  	kfree(indio_dev->chan_attr_group.attrs);
>>  }
>>  
>> -static const char * const iio_ev_type_text[] = {
>> -	[IIO_EV_TYPE_THRESH] = "thresh",
>> -	[IIO_EV_TYPE_MAG] = "mag",
>> -	[IIO_EV_TYPE_ROC] = "roc",
>> -	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
>> -	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
>> -};
>> -
>> -static const char * const iio_ev_dir_text[] = {
>> -	[IIO_EV_DIR_EITHER] = "either",
>> -	[IIO_EV_DIR_RISING] = "rising",
>> -	[IIO_EV_DIR_FALLING] = "falling"
>> -};
>> -
>> -static ssize_t iio_ev_state_store(struct device *dev,
>> -				  struct device_attribute *attr,
>> -				  const char *buf,
>> -				  size_t len)
>> -{
>> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> -	int ret;
>> -	bool val;
>> -
>> -	ret = strtobool(buf, &val);
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	ret = indio_dev->info->write_event_config(indio_dev,
>> -						  this_attr->address,
>> -						  val);
>> -	return (ret < 0) ? ret : len;
>> -}
>> -
>> -static ssize_t iio_ev_state_show(struct device *dev,
>> -				 struct device_attribute *attr,
>> -				 char *buf)
>> -{
>> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> -	int val = indio_dev->info->read_event_config(indio_dev,
>> -						     this_attr->address);
>> -
>> -	if (val < 0)
>> -		return val;
>> -	else
>> -		return sprintf(buf, "%d\n", val);
>> -}
>> -
>> -static ssize_t iio_ev_value_show(struct device *dev,
>> -				 struct device_attribute *attr,
>> -				 char *buf)
>> -{
>> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> -	int val, ret;
>> -
>> -	ret = indio_dev->info->read_event_value(indio_dev,
>> -						this_attr->address, &val);
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	return sprintf(buf, "%d\n", val);
>> -}
>> -
>> -static ssize_t iio_ev_value_store(struct device *dev,
>> -				  struct device_attribute *attr,
>> -				  const char *buf,
>> -				  size_t len)
>> -{
>> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> -	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> -	unsigned long val;
>> -	int ret;
>> -
>> -	if (!indio_dev->info->write_event_value)
>> -		return -EINVAL;
>> -
>> -	ret = strict_strtoul(buf, 10, &val);
>> -	if (ret)
>> -		return ret;
>> -
>> -	ret = indio_dev->info->write_event_value(indio_dev, this_attr->address,
>> -						 val);
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	return len;
>> -}
>> -
>> -static int iio_device_add_event_sysfs(struct iio_dev *indio_dev,
>> -				      struct iio_chan_spec const *chan)
>> -{
>> -	int ret = 0, i, attrcount = 0;
>> -	u64 mask = 0;
>> -	char *postfix;
>> -	if (!chan->event_mask)
>> -		return 0;
>> -
>> -	for_each_set_bit(i, &chan->event_mask, sizeof(chan->event_mask)*8) {
>> -		postfix = kasprintf(GFP_KERNEL, "%s_%s_en",
>> -				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
>> -				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
>> -		if (postfix == NULL) {
>> -			ret = -ENOMEM;
>> -			goto error_ret;
>> -		}
>> -		if (chan->modified)
>> -			mask = IIO_MOD_EVENT_CODE(chan->type, 0, chan->channel,
>> -						  i/IIO_EV_DIR_MAX,
>> -						  i%IIO_EV_DIR_MAX);
>> -		else if (chan->differential)
>> -			mask = IIO_EVENT_CODE(chan->type,
>> -					      0, 0,
>> -					      i%IIO_EV_DIR_MAX,
>> -					      i/IIO_EV_DIR_MAX,
>> -					      0,
>> -					      chan->channel,
>> -					      chan->channel2);
>> -		else
>> -			mask = IIO_UNMOD_EVENT_CODE(chan->type,
>> -						    chan->channel,
>> -						    i/IIO_EV_DIR_MAX,
>> -						    i%IIO_EV_DIR_MAX);
>> -
>> -		ret = __iio_add_chan_devattr(postfix,
>> -					     chan,
>> -					     &iio_ev_state_show,
>> -					     iio_ev_state_store,
>> -					     mask,
>> -					     0,
>> -					     &indio_dev->dev,
>> -					     &indio_dev->event_interface->
>> -					     dev_attr_list);
>> -		kfree(postfix);
>> -		if (ret)
>> -			goto error_ret;
>> -		attrcount++;
>> -		postfix = kasprintf(GFP_KERNEL, "%s_%s_value",
>> -				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
>> -				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
>> -		if (postfix == NULL) {
>> -			ret = -ENOMEM;
>> -			goto error_ret;
>> -		}
>> -		ret = __iio_add_chan_devattr(postfix, chan,
>> -					     iio_ev_value_show,
>> -					     iio_ev_value_store,
>> -					     mask,
>> -					     0,
>> -					     &indio_dev->dev,
>> -					     &indio_dev->event_interface->
>> -					     dev_attr_list);
>> -		kfree(postfix);
>> -		if (ret)
>> -			goto error_ret;
>> -		attrcount++;
>> -	}
>> -	ret = attrcount;
>> -error_ret:
>> -	return ret;
>> -}
>> -
>> -static inline void __iio_remove_event_config_attrs(struct iio_dev *indio_dev)
>> -{
>> -	struct iio_dev_attr *p, *n;
>> -	list_for_each_entry_safe(p, n,
>> -				 &indio_dev->event_interface->
>> -				 dev_attr_list, l) {
>> -		kfree(p->dev_attr.attr.name);
>> -		kfree(p);
>> -	}
>> -}
>> -
>> -static inline int __iio_add_event_config_attrs(struct iio_dev *indio_dev)
>> -{
>> -	int j, ret, attrcount = 0;
>> -
>> -	INIT_LIST_HEAD(&indio_dev->event_interface->dev_attr_list);
>> -	/* Dynically created from the channels array */
>> -	for (j = 0; j < indio_dev->num_channels; j++) {
>> -		ret = iio_device_add_event_sysfs(indio_dev,
>> -						 &indio_dev->channels[j]);
>> -		if (ret < 0)
>> -			goto error_clear_attrs;
>> -		attrcount += ret;
>> -	}
>> -	return attrcount;
>> -
>> -error_clear_attrs:
>> -	__iio_remove_event_config_attrs(indio_dev);
>> -
>> -	return ret;
>> -}
>> -
>> -static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
>> -{
>> -	int j;
>> -
>> -	for (j = 0; j < indio_dev->num_channels; j++)
>> -		if (indio_dev->channels[j].event_mask != 0)
>> -			return true;
>> -	return false;
>> -}
>> -
>> -static void iio_setup_ev_int(struct iio_event_interface *ev_int)
>> -{
>> -	mutex_init(&ev_int->event_list_lock);
>> -	/* discussion point - make this variable? */
>> -	ev_int->max_events = 10;
>> -	ev_int->current_events = 0;
>> -	INIT_LIST_HEAD(&ev_int->det_events);
>> -	init_waitqueue_head(&ev_int->wait);
>> -}
>> -
>> -static const char *iio_event_group_name = "events";
>> -static int iio_device_register_eventset(struct iio_dev *indio_dev)
>> -{
>> -	struct iio_dev_attr *p;
>> -	int ret = 0, attrcount_orig = 0, attrcount, attrn;
>> -	struct attribute **attr;
>> -
>> -	if (!(indio_dev->info->event_attrs ||
>> -	      iio_check_for_dynamic_events(indio_dev)))
>> -		return 0;
>> -
>> -	indio_dev->event_interface =
>> -		kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
>> -	if (indio_dev->event_interface == NULL) {
>> -		ret = -ENOMEM;
>> -		goto error_ret;
>> -	}
>> -
>> -	iio_setup_ev_int(indio_dev->event_interface);
>> -	if (indio_dev->info->event_attrs != NULL) {
>> -		attr = indio_dev->info->event_attrs->attrs;
>> -		while (*attr++ != NULL)
>> -			attrcount_orig++;
>> -	}
>> -	attrcount = attrcount_orig;
>> -	if (indio_dev->channels) {
>> -		ret = __iio_add_event_config_attrs(indio_dev);
>> -		if (ret < 0)
>> -			goto error_free_setup_event_lines;
>> -		attrcount += ret;
>> -	}
>> -
>> -	indio_dev->event_interface->group.name = iio_event_group_name;
>> -	indio_dev->event_interface->group.attrs = kcalloc(attrcount + 1,
>> -							  sizeof(indio_dev->event_interface->group.attrs[0]),
>> -							  GFP_KERNEL);
>> -	if (indio_dev->event_interface->group.attrs == NULL) {
>> -		ret = -ENOMEM;
>> -		goto error_free_setup_event_lines;
>> -	}
>> -	if (indio_dev->info->event_attrs)
>> -		memcpy(indio_dev->event_interface->group.attrs,
>> -		       indio_dev->info->event_attrs->attrs,
>> -		       sizeof(indio_dev->event_interface->group.attrs[0])
>> -		       *attrcount_orig);
>> -	attrn = attrcount_orig;
>> -	/* Add all elements from the list. */
>> -	list_for_each_entry(p,
>> -			    &indio_dev->event_interface->dev_attr_list,
>> -			    l)
>> -		indio_dev->event_interface->group.attrs[attrn++] =
>> -			&p->dev_attr.attr;
>> -	indio_dev->groups[indio_dev->groupcounter++] =
>> -		&indio_dev->event_interface->group;
>> -
>> -	return 0;
>> -
>> -error_free_setup_event_lines:
>> -	__iio_remove_event_config_attrs(indio_dev);
>> -	kfree(indio_dev->event_interface);
>> -error_ret:
>> -
>> -	return ret;
>> -}
>> -
>> -static void iio_device_unregister_eventset(struct iio_dev *indio_dev)
>> -{
>> -	if (indio_dev->event_interface == NULL)
>> -		return;
>> -	__iio_remove_event_config_attrs(indio_dev);
>> -	kfree(indio_dev->event_interface->group.attrs);
>> -	kfree(indio_dev->event_interface);
>> -}
>> -
>>  static void iio_dev_release(struct device *device)
>>  {
>>  	struct iio_dev *indio_dev = container_of(device, struct iio_dev, dev);
>> diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
>> new file mode 100644
>> index 0000000..17ed582
>> --- /dev/null
>> +++ b/drivers/staging/iio/industrialio-event.c
>> @@ -0,0 +1,484 @@
>> +/* The industrial I/O core
>> + *
>> + * Copyright (c) 2008 Jonathan Cameron
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + *
>> + * Based on elements of hwmon and input subsystems.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/err.h>
>> +#include <linux/device.h>
>> +#include <linux/fs.h>
>> +#include <linux/poll.h>
>> +#include <linux/wait.h>
>> +#include <linux/cdev.h>
>> +#include <linux/slab.h>
>> +#include <linux/kfifo.h>
>> +#include <linux/anon_inodes.h>
>> +#include "iio.h"
>> +#include "iio_core.h"
>> +#include "sysfs.h"
>> +#include "events.h"
>> +
>> +/**
>> + * struct iio_detected_event_list - list element for events that have occurred
>> + * @list:		linked list header
>> + * @ev:			the event itself
>> + */
>> +struct iio_detected_event_list {
>> +	struct list_head		list;
>> +	struct iio_event_data		ev;
>> +};
>> +
>> +/**
>> + * struct iio_event_interface - chrdev interface for an event line
>> + * @dev:		device assocated with event interface
> Doesn't exist.
>> + * @wait:		wait queue to allow blocking reads of events
>> + * @event_list_lock:	mutex to protect the list of detected events
>> + * @det_events:		list of detected events
>> + * @max_events:		maximum number of events before new ones are dropped
>> + * @current_events:	number of events in detected list
>> + * @flags:		file operations related flags including busy flag.
>> + */
>> +struct iio_event_interface {
>> +	wait_queue_head_t			wait;
>> +	struct mutex				event_list_lock;
>> +	struct list_head			det_events;
>> +	int					max_events;
>> +	int					current_events;
> undocumented
>> +	struct list_head dev_attr_list;
>> +	unsigned long flags;
> undocumented
>> +	struct attribute_group			group;
>> +};
>> +
>> +int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
>> +{
>> +	struct iio_event_interface *ev_int = indio_dev->event_interface;
>> +	struct iio_detected_event_list *ev;
>> +	int ret = 0;
>> +
>> +	/* Does anyone care? */
>> +	mutex_lock(&ev_int->event_list_lock);
>> +	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>> +		if (ev_int->current_events == ev_int->max_events) {
>> +			mutex_unlock(&ev_int->event_list_lock);
>> +			return 0;
>> +		}
>> +		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
>> +		if (ev == NULL) {
>> +			ret = -ENOMEM;
>> +			mutex_unlock(&ev_int->event_list_lock);
>> +			goto error_ret;
>> +		}
>> +		ev->ev.id = ev_code;
>> +		ev->ev.timestamp = timestamp;
>> +
>> +		list_add_tail(&ev->list, &ev_int->det_events);
>> +		ev_int->current_events++;
>> +		mutex_unlock(&ev_int->event_list_lock);
>> +		wake_up_interruptible(&ev_int->wait);
>> +	} else
>> +		mutex_unlock(&ev_int->event_list_lock);
>> +
>> +error_ret:
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(iio_push_event);
>> +
>> +static ssize_t iio_event_chrdev_read(struct file *filep,
>> +				     char __user *buf,
>> +				     size_t count,
>> +				     loff_t *f_ps)
>> +{
>> +	struct iio_event_interface *ev_int = filep->private_data;
>> +	struct iio_detected_event_list *el;
>> +	size_t len = sizeof(el->ev);
>> +	int ret;
>> +
>> +	if (count < len)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&ev_int->event_list_lock);
>> +	if (list_empty(&ev_int->det_events)) {
>> +		if (filep->f_flags & O_NONBLOCK) {
>> +			ret = -EAGAIN;
>> +			goto error_mutex_unlock;
>> +		}
>> +		mutex_unlock(&ev_int->event_list_lock);
>> +		/* Blocking on device; waiting for something to be there */
>> +		ret = wait_event_interruptible(ev_int->wait,
>> +					       !list_empty(&ev_int
>> +							   ->det_events));
>> +		if (ret)
>> +			goto error_ret;
>> +		/* Single access device so no one else can get the data */
>> +		mutex_lock(&ev_int->event_list_lock);
>> +	}
>> +
>> +	el = list_first_entry(&ev_int->det_events,
>> +			      struct iio_detected_event_list,
>> +			      list);
>> +	if (copy_to_user(buf, &(el->ev), len)) {
>> +		ret = -EFAULT;
>> +		goto error_mutex_unlock;
>> +	}
>> +	list_del(&el->list);
>> +	ev_int->current_events--;
>> +	mutex_unlock(&ev_int->event_list_lock);
>> +	kfree(el);
>> +
>> +	return len;
>> +
>> +error_mutex_unlock:
>> +	mutex_unlock(&ev_int->event_list_lock);
>> +error_ret:
>> +
>> +	return ret;
>> +}
>> +
>> +static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
>> +{
>> +	struct iio_event_interface *ev_int = filep->private_data;
>> +	struct iio_detected_event_list *el, *t;
>> +
>> +	mutex_lock(&ev_int->event_list_lock);
>> +	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>> +	/*
>> +	 * In order to maintain a clean state for reopening,
>> +	 * clear out any awaiting events. The mask will prevent
>> +	 * any new __iio_push_event calls running.
>> +	 */
>> +	list_for_each_entry_safe(el, t, &ev_int->det_events, list) {
>> +		list_del(&el->list);
>> +		kfree(el);
>> +	}
>> +	ev_int->current_events = 0;
>> +	mutex_unlock(&ev_int->event_list_lock);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct file_operations iio_event_chrdev_fileops = {
>> +	.read =  iio_event_chrdev_read,
>> +	.release = iio_event_chrdev_release,
>> +	.owner = THIS_MODULE,
>> +	.llseek = noop_llseek,
>> +};
>> +
>> +int iio_event_getfd(struct iio_dev *indio_dev)
>> +{
>> +	struct iio_event_interface *ev_int = indio_dev->event_interface;
>> +	int fd;
>> +
>> +	if (ev_int == NULL)
>> +		return -ENODEV;
>> +
>> +	mutex_lock(&ev_int->event_list_lock);
>> +	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>> +		mutex_unlock(&ev_int->event_list_lock);
>> +		return -EBUSY;
>> +	}
>> +	mutex_unlock(&ev_int->event_list_lock);
>> +	fd = anon_inode_getfd("iio:event",
>> +				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
>> +	if (fd < 0) {
>> +		mutex_lock(&ev_int->event_list_lock);
>> +		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>> +		mutex_unlock(&ev_int->event_list_lock);
>> +	}
>> +	return fd;
>> +}
>> +
>> +static const char * const iio_ev_type_text[] = {
>> +	[IIO_EV_TYPE_THRESH] = "thresh",
>> +	[IIO_EV_TYPE_MAG] = "mag",
>> +	[IIO_EV_TYPE_ROC] = "roc",
>> +	[IIO_EV_TYPE_THRESH_ADAPTIVE] = "thresh_adaptive",
>> +	[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
>> +};
>> +
>> +static const char * const iio_ev_dir_text[] = {
>> +	[IIO_EV_DIR_EITHER] = "either",
>> +	[IIO_EV_DIR_RISING] = "rising",
>> +	[IIO_EV_DIR_FALLING] = "falling"
>> +};
>> +
>> +static ssize_t iio_ev_state_store(struct device *dev,
>> +				  struct device_attribute *attr,
>> +				  const char *buf,
>> +				  size_t len)
>> +{
>> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> +	int ret;
>> +	bool val;
>> +
>> +	ret = strtobool(buf, &val);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = indio_dev->info->write_event_config(indio_dev,
>> +						  this_attr->address,
>> +						  val);
>> +	return (ret < 0) ? ret : len;
>> +}
>> +
>> +static ssize_t iio_ev_state_show(struct device *dev,
>> +				 struct device_attribute *attr,
>> +				 char *buf)
>> +{
>> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> +	int val = indio_dev->info->read_event_config(indio_dev,
>> +						     this_attr->address);
>> +
>> +	if (val < 0)
>> +		return val;
>> +	else
>> +		return sprintf(buf, "%d\n", val);
>> +}
>> +
>> +static ssize_t iio_ev_value_show(struct device *dev,
>> +				 struct device_attribute *attr,
>> +				 char *buf)
>> +{
>> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> +	int val, ret;
>> +
>> +	ret = indio_dev->info->read_event_value(indio_dev,
>> +						this_attr->address, &val);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return sprintf(buf, "%d\n", val);
>> +}
>> +
>> +static ssize_t iio_ev_value_store(struct device *dev,
>> +				  struct device_attribute *attr,
>> +				  const char *buf,
>> +				  size_t len)
>> +{
>> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> +	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> +	unsigned long val;
>> +	int ret;
>> +
>> +	if (!indio_dev->info->write_event_value)
>> +		return -EINVAL;
>> +
>> +	ret = strict_strtoul(buf, 10, &val);
> Again as you are in here, good oportunity to get rid
> of this... (again a separate patch).
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = indio_dev->info->write_event_value(indio_dev, this_attr->address,
>> +						 val);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return len;
>> +}
>> +
>> +static int iio_device_add_event_sysfs(struct iio_dev *indio_dev,
>> +				      struct iio_chan_spec const *chan)
>> +{
>> +	int ret = 0, i, attrcount = 0;
>> +	u64 mask = 0;
>> +	char *postfix;
>> +	if (!chan->event_mask)
>> +		return 0;
>> +
>> +	for_each_set_bit(i, &chan->event_mask, sizeof(chan->event_mask)*8) {
>> +		postfix = kasprintf(GFP_KERNEL, "%s_%s_en",
>> +				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
>> +				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
>> +		if (postfix == NULL) {
>> +			ret = -ENOMEM;
>> +			goto error_ret;
>> +		}
>> +		if (chan->modified)
>> +			mask = IIO_MOD_EVENT_CODE(chan->type, 0, chan->channel,
>> +						  i/IIO_EV_DIR_MAX,
>> +						  i%IIO_EV_DIR_MAX);
>> +		else if (chan->differential)
>> +			mask = IIO_EVENT_CODE(chan->type,
>> +					      0, 0,
>> +					      i%IIO_EV_DIR_MAX,
>> +					      i/IIO_EV_DIR_MAX,
>> +					      0,
>> +					      chan->channel,
>> +					      chan->channel2);
>> +		else
>> +			mask = IIO_UNMOD_EVENT_CODE(chan->type,
>> +						    chan->channel,
>> +						    i/IIO_EV_DIR_MAX,
>> +						    i%IIO_EV_DIR_MAX);
>> +
>> +		ret = __iio_add_chan_devattr(postfix,
>> +					     chan,
>> +					     &iio_ev_state_show,
>> +					     iio_ev_state_store,
>> +					     mask,
>> +					     0,
>> +					     &indio_dev->dev,
>> +					     &indio_dev->event_interface->
>> +					     dev_attr_list);
>> +		kfree(postfix);
>> +		if (ret)
>> +			goto error_ret;
>> +		attrcount++;
>> +		postfix = kasprintf(GFP_KERNEL, "%s_%s_value",
>> +				    iio_ev_type_text[i/IIO_EV_DIR_MAX],
>> +				    iio_ev_dir_text[i%IIO_EV_DIR_MAX]);
>> +		if (postfix == NULL) {
>> +			ret = -ENOMEM;
>> +			goto error_ret;
>> +		}
>> +		ret = __iio_add_chan_devattr(postfix, chan,
>> +					     iio_ev_value_show,
>> +					     iio_ev_value_store,
>> +					     mask,
>> +					     0,
>> +					     &indio_dev->dev,
>> +					     &indio_dev->event_interface->
>> +					     dev_attr_list);
>> +		kfree(postfix);
>> +		if (ret)
>> +			goto error_ret;
>> +		attrcount++;
>> +	}
>> +	ret = attrcount;
>> +error_ret:
>> +	return ret;
>> +}
>> +
>> +static inline void __iio_remove_event_config_attrs(struct iio_dev *indio_dev)
>> +{
>> +	struct iio_dev_attr *p, *n;
>> +	list_for_each_entry_safe(p, n,
>> +				 &indio_dev->event_interface->
>> +				 dev_attr_list, l) {
>> +		kfree(p->dev_attr.attr.name);
>> +		kfree(p);
>> +	}
>> +}
>> +
>> +static inline int __iio_add_event_config_attrs(struct iio_dev *indio_dev)
>> +{
>> +	int j, ret, attrcount = 0;
>> +
>> +	INIT_LIST_HEAD(&indio_dev->event_interface->dev_attr_list);
>> +	/* Dynically created from the channels array */
>> +	for (j = 0; j < indio_dev->num_channels; j++) {
>> +		ret = iio_device_add_event_sysfs(indio_dev,
>> +						 &indio_dev->channels[j]);
>> +		if (ret < 0)
>> +			goto error_clear_attrs;
>> +		attrcount += ret;
>> +	}
>> +	return attrcount;
>> +
>> +error_clear_attrs:
>> +	__iio_remove_event_config_attrs(indio_dev);
>> +
>> +	return ret;
>> +}
>> +
>> +static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
>> +{
>> +	int j;
>> +
>> +	for (j = 0; j < indio_dev->num_channels; j++)
>> +		if (indio_dev->channels[j].event_mask != 0)
>> +			return true;
>> +	return false;
>> +}
>> +
>> +static void iio_setup_ev_int(struct iio_event_interface *ev_int)
>> +{
>> +	mutex_init(&ev_int->event_list_lock);
>> +	/* discussion point - make this variable? */
>> +	ev_int->max_events = 10;
>> +	ev_int->current_events = 0;
>> +	INIT_LIST_HEAD(&ev_int->det_events);
>> +	init_waitqueue_head(&ev_int->wait);
>> +}
>> +
>> +static const char *iio_event_group_name = "events";
>> +int iio_device_register_eventset(struct iio_dev *indio_dev)
>> +{
>> +	struct iio_dev_attr *p;
>> +	int ret = 0, attrcount_orig = 0, attrcount, attrn;
>> +	struct attribute **attr;
>> +
>> +	if (!(indio_dev->info->event_attrs ||
>> +	      iio_check_for_dynamic_events(indio_dev)))
>> +		return 0;
>> +
>> +	indio_dev->event_interface =
>> +		kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
>> +	if (indio_dev->event_interface == NULL) {
>> +		ret = -ENOMEM;
>> +		goto error_ret;
>> +	}
>> +
>> +	iio_setup_ev_int(indio_dev->event_interface);
>> +	if (indio_dev->info->event_attrs != NULL) {
>> +		attr = indio_dev->info->event_attrs->attrs;
>> +		while (*attr++ != NULL)
>> +			attrcount_orig++;
>> +	}
>> +	attrcount = attrcount_orig;
>> +	if (indio_dev->channels) {
>> +		ret = __iio_add_event_config_attrs(indio_dev);
>> +		if (ret < 0)
>> +			goto error_free_setup_event_lines;
>> +		attrcount += ret;
>> +	}
>> +
>> +	indio_dev->event_interface->group.name = iio_event_group_name;
>> +	indio_dev->event_interface->group.attrs = kcalloc(attrcount + 1,
>> +							  sizeof(indio_dev->event_interface->group.attrs[0]),
>> +							  GFP_KERNEL);
>> +	if (indio_dev->event_interface->group.attrs == NULL) {
>> +		ret = -ENOMEM;
>> +		goto error_free_setup_event_lines;
>> +	}
>> +	if (indio_dev->info->event_attrs)
>> +		memcpy(indio_dev->event_interface->group.attrs,
>> +		       indio_dev->info->event_attrs->attrs,
>> +		       sizeof(indio_dev->event_interface->group.attrs[0])
>> +		       *attrcount_orig);
>> +	attrn = attrcount_orig;
>> +	/* Add all elements from the list. */
>> +	list_for_each_entry(p,
>> +			    &indio_dev->event_interface->dev_attr_list,
>> +			    l)
>> +		indio_dev->event_interface->group.attrs[attrn++] =
>> +			&p->dev_attr.attr;
>> +	indio_dev->groups[indio_dev->groupcounter++] =
>> +		&indio_dev->event_interface->group;
>> +
>> +	return 0;
>> +
>> +error_free_setup_event_lines:
>> +	__iio_remove_event_config_attrs(indio_dev);
>> +	kfree(indio_dev->event_interface);
>> +error_ret:
>> +
>> +	return ret;
>> +}
>> +
>> +void iio_device_unregister_eventset(struct iio_dev *indio_dev)
>> +{
>> +	if (indio_dev->event_interface == NULL)
>> +		return;
>> +	__iio_remove_event_config_attrs(indio_dev);
>> +	kfree(indio_dev->event_interface->group.attrs);
>> +	kfree(indio_dev->event_interface);
>> +}
> 

Re: [PATCH 1/4] staging:iio: Factor out event handling into its own file

[Lars-Peter Clausen]

On 12/18/2011 07:24 PM, Jonathan Cameron wrote:
> On 12/18/2011 06:19 PM, Jonathan Cameron wrote:
>> On 12/16/2011 05:12 PM, Lars-Peter Clausen wrote:
>>> The core iio file has gotten quite cluttered over time. This patch moves
>>> the event handling code into its own file.
>> A small comment here wrt to ease of review. When doing
>> a whole scale move like this, it is helpful to state
>> where any necessary code changes were... That way I can
>> just assume you cut and paste the rest rather than having
>> to check that.
>>>
>>> This has also the advantage that industrialio-core.c is now closer again to
>>> its counterpart in the outofstaging branch.
>> Definitely in favour of this change.
>>
>> Assuming it isn't in a later patch, there are a few undocumented
>> structure elements that could do with documenting ;)
>> More 'interestingly' there are a few that don't exist and are
>> documented.  All my fault of course, but seeing as you are
>> cleaning this code up... (looks hopeful)
>> Obviously shouldn't be part of this patch.  Either insert
>> one fixing it first, or do it at the end as a cleanup patch.
>>
>> So all in all, the patch is fine, I just noticed some unrelated bits
>> whilst reading it ;)  Events as you have no doubt noticed are
>> probably our most dubious corner (or were until this set).
>>>
> I really ought not to leave my build tests running in the background
> whilst sending emails acking things.  Looks like you have some issues...
> 
> drivers/staging/iio/industrialio-event.c:84:17: error: undefined
> identifier 'TASK_INTERRUPTIBLE'
> drivers/staging/iio/industrialio-event.c:114:23: error: undefined
> identifier 'TASK_INTERRUPTIBLE'
> drivers/staging/iio/industrialio-event.c:114:23: error: undefined
> identifier 'signal_pending'
> drivers/staging/iio/industrialio-event.c:114:23: error: undefined
> identifier 'schedule'
> 
> Can't chase this down right now, but I'm guessing missing header
> at least on arm.
> 
> adding include of sched.h does the job.

Makes me wonder if wait.h should include sched.h. But I'll add it the this
patch as well.

Re: [PATCH 2/4] staging:iio:events: Use kfifo for event queue

[Jonathan Cameron]

On 12/16/2011 05:12 PM, Lars-Peter Clausen wrote:
> The current IIO event code uses a list to emulate FIFO like behaviour.
> Just use a kfifo directly instead to implement the event queue.
Please also add that this changes number of elements queue to 16 from
(I think) 10.

One bit to do with remove comment but not code that needs fixing...
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/industrialio-event.c |   89 ++++++------------------------
>  1 files changed, 18 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
> index 17ed582..50d03bd 100644
> --- a/drivers/staging/iio/industrialio-event.c
> +++ b/drivers/staging/iio/industrialio-event.c
> @@ -26,31 +26,17 @@
>  #include "events.h"
>  
>  /**
> - * struct iio_detected_event_list - list element for events that have occurred
> - * @list:		linked list header
> - * @ev:			the event itself
> - */
> -struct iio_detected_event_list {
> -	struct list_head		list;
> -	struct iio_event_data		ev;
> -};
> -
> -/**
>   * struct iio_event_interface - chrdev interface for an event line
>   * @dev:		device assocated with event interface
>   * @wait:		wait queue to allow blocking reads of events
> - * @event_list_lock:	mutex to protect the list of detected events
The comment goes in this patch, but the element is still there.  The
next patch removes the users for this but not I think the actual element.
>   * @det_events:		list of detected events
> - * @max_events:		maximum number of events before new ones are dropped
> - * @current_events:	number of events in detected list
>   * @flags:		file operations related flags including busy flag.
>   */
>  struct iio_event_interface {
>  	wait_queue_head_t			wait;
>  	struct mutex				event_list_lock;
> -	struct list_head			det_events;
> -	int					max_events;
> -	int					current_events;
> +	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
> +
>  	struct list_head dev_attr_list;
>  	unsigned long flags;
>  	struct attribute_group			group;
> @@ -59,33 +45,24 @@ struct iio_event_interface {
>  int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
>  {
>  	struct iio_event_interface *ev_int = indio_dev->event_interface;
> -	struct iio_detected_event_list *ev;
> +	struct iio_event_data ev;
>  	int ret = 0;
>  
>  	/* Does anyone care? */
>  	mutex_lock(&ev_int->event_list_lock);
>  	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
> -		if (ev_int->current_events == ev_int->max_events) {
> -			mutex_unlock(&ev_int->event_list_lock);
> -			return 0;
> -		}
> -		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
> -		if (ev == NULL) {
> -			ret = -ENOMEM;
> -			mutex_unlock(&ev_int->event_list_lock);
> -			goto error_ret;
> -		}
> -		ev->ev.id = ev_code;
> -		ev->ev.timestamp = timestamp;
>  
> -		list_add_tail(&ev->list, &ev_int->det_events);
> -		ev_int->current_events++;
> +		ev.id = ev_code;
> +		ev.timestamp = timestamp;
> +
> +		ret = kfifo_put(&ev_int->det_events, &ev);
> +
>  		mutex_unlock(&ev_int->event_list_lock);
> -		wake_up_interruptible(&ev_int->wait);
> +		if (ret != 0)
> +			wake_up_interruptible(&ev_int->wait);
perhaps rename ret to something indicating that it is the number
of elements copied?  That way it is obvious we are checking if
the kfifo is full rather than for errors.
>  	} else
>  		mutex_unlock(&ev_int->event_list_lock);
>  
> -error_ret:
>  	return ret;
>  }
>  EXPORT_SYMBOL(iio_push_event);
> @@ -96,15 +73,14 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
>  				     loff_t *f_ps)
>  {
>  	struct iio_event_interface *ev_int = filep->private_data;
> -	struct iio_detected_event_list *el;
> -	size_t len = sizeof(el->ev);
> +	unsigned int copied;
>  	int ret;
>  
> -	if (count < len)
> +	if (count < sizeof(struct iio_event_data))
>  		return -EINVAL;
>  
>  	mutex_lock(&ev_int->event_list_lock);
> -	if (list_empty(&ev_int->det_events)) {
> +	if (kfifo_is_empty(&ev_int->det_events)) {
>  		if (filep->f_flags & O_NONBLOCK) {
>  			ret = -EAGAIN;
>  			goto error_mutex_unlock;
> @@ -112,52 +88,29 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
>  		mutex_unlock(&ev_int->event_list_lock);
>  		/* Blocking on device; waiting for something to be there */
>  		ret = wait_event_interruptible(ev_int->wait,
> -					       !list_empty(&ev_int
> -							   ->det_events));
> +					!kfifo_is_empty(&ev_int->det_events));
>  		if (ret)
>  			goto error_ret;
>  		/* Single access device so no one else can get the data */
>  		mutex_lock(&ev_int->event_list_lock);
>  	}
>  
> -	el = list_first_entry(&ev_int->det_events,
> -			      struct iio_detected_event_list,
> -			      list);
> -	if (copy_to_user(buf, &(el->ev), len)) {
> -		ret = -EFAULT;
> -		goto error_mutex_unlock;
> -	}
> -	list_del(&el->list);
> -	ev_int->current_events--;
>  	mutex_unlock(&ev_int->event_list_lock);
> -	kfree(el);
> -
> -	return len;
> +	ret = kfifo_to_user(&ev_int->det_events, buf, count, &copied);
>  
>  error_mutex_unlock:
>  	mutex_unlock(&ev_int->event_list_lock);
>  error_ret:
> -
> -	return ret;
> +	return ret ? ret : copied;
>  }
>  
>  static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
>  {
>  	struct iio_event_interface *ev_int = filep->private_data;
> -	struct iio_detected_event_list *el, *t;
>  
>  	mutex_lock(&ev_int->event_list_lock);
>  	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
> -	/*
> -	 * In order to maintain a clean state for reopening,
> -	 * clear out any awaiting events. The mask will prevent
> -	 * any new __iio_push_event calls running.
> -	 */
Comment is still relevant (if updated appropriately)
> -	list_for_each_entry_safe(el, t, &ev_int->det_events, list) {
> -		list_del(&el->list);
> -		kfree(el);
> -	}
> -	ev_int->current_events = 0;
> +	kfifo_reset_out(&ev_int->det_events);
>  	mutex_unlock(&ev_int->event_list_lock);
>  
>  	return 0;
> @@ -269,9 +222,6 @@ static ssize_t iio_ev_value_store(struct device *dev,
>  	unsigned long val;
>  	int ret;
>  
> -	if (!indio_dev->info->write_event_value)
> -		return -EINVAL;
> -
>  	ret = strict_strtoul(buf, 10, &val);
>  	if (ret)
>  		return ret;
> @@ -402,10 +352,7 @@ static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
>  static void iio_setup_ev_int(struct iio_event_interface *ev_int)
>  {
>  	mutex_init(&ev_int->event_list_lock);
> -	/* discussion point - make this variable? */
> -	ev_int->max_events = 10;
> -	ev_int->current_events = 0;
> -	INIT_LIST_HEAD(&ev_int->det_events);
> +	INIT_KFIFO(ev_int->det_events);
>  	init_waitqueue_head(&ev_int->wait);
>  }
>  

Re: [PATCH 2/4] staging:iio:events: Use kfifo for event queue

[Jonathan Cameron]

On 12/18/2011 09:42 PM, Jonathan Cameron wrote:
> On 12/16/2011 05:12 PM, Lars-Peter Clausen wrote:
>> The current IIO event code uses a list to emulate FIFO like behaviour.
>> Just use a kfifo directly instead to implement the event queue.
> Please also add that this changes number of elements queue to 16 from
> (I think) 10.
> 
> One bit to do with remove comment but not code that needs fixing...
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>>  drivers/staging/iio/industrialio-event.c |   89 ++++++------------------------
>>  1 files changed, 18 insertions(+), 71 deletions(-)
>>
>> diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
>> index 17ed582..50d03bd 100644
>> --- a/drivers/staging/iio/industrialio-event.c
>> +++ b/drivers/staging/iio/industrialio-event.c
>> @@ -26,31 +26,17 @@
>>  #include "events.h"
>>  
>>  /**
>> - * struct iio_detected_event_list - list element for events that have occurred
>> - * @list:		linked list header
>> - * @ev:			the event itself
>> - */
>> -struct iio_detected_event_list {
>> -	struct list_head		list;
>> -	struct iio_event_data		ev;
>> -};
>> -
>> -/**
>>   * struct iio_event_interface - chrdev interface for an event line
>>   * @dev:		device assocated with event interface
>>   * @wait:		wait queue to allow blocking reads of events
>> - * @event_list_lock:	mutex to protect the list of detected events
> The comment goes in this patch, but the element is still there.  The
> next patch removes the users for this but not I think the actual element.
Ah my mistake, it is removed in the next patch.  Please shift this
comment removal to there.
>>   * @det_events:		list of detected events
>> - * @max_events:		maximum number of events before new ones are dropped
>> - * @current_events:	number of events in detected list
>>   * @flags:		file operations related flags including busy flag.
>>   */
>>  struct iio_event_interface {
>>  	wait_queue_head_t			wait;
>>  	struct mutex				event_list_lock;
>> -	struct list_head			det_events;
>> -	int					max_events;
>> -	int					current_events;
>> +	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
>> +
>>  	struct list_head dev_attr_list;
>>  	unsigned long flags;
>>  	struct attribute_group			group;
>> @@ -59,33 +45,24 @@ struct iio_event_interface {
>>  int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
>>  {
>>  	struct iio_event_interface *ev_int = indio_dev->event_interface;
>> -	struct iio_detected_event_list *ev;
>> +	struct iio_event_data ev;
>>  	int ret = 0;
>>  
>>  	/* Does anyone care? */
>>  	mutex_lock(&ev_int->event_list_lock);
>>  	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>> -		if (ev_int->current_events == ev_int->max_events) {
>> -			mutex_unlock(&ev_int->event_list_lock);
>> -			return 0;
>> -		}
>> -		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
>> -		if (ev == NULL) {
>> -			ret = -ENOMEM;
>> -			mutex_unlock(&ev_int->event_list_lock);
>> -			goto error_ret;
>> -		}
>> -		ev->ev.id = ev_code;
>> -		ev->ev.timestamp = timestamp;
>>  
>> -		list_add_tail(&ev->list, &ev_int->det_events);
>> -		ev_int->current_events++;
>> +		ev.id = ev_code;
>> +		ev.timestamp = timestamp;
>> +
>> +		ret = kfifo_put(&ev_int->det_events, &ev);
>> +
>>  		mutex_unlock(&ev_int->event_list_lock);
>> -		wake_up_interruptible(&ev_int->wait);
>> +		if (ret != 0)
>> +			wake_up_interruptible(&ev_int->wait);
> perhaps rename ret to something indicating that it is the number
> of elements copied?  That way it is obvious we are checking if
> the kfifo is full rather than for errors.
>>  	} else
>>  		mutex_unlock(&ev_int->event_list_lock);
>>  
>> -error_ret:
>>  	return ret;
>>  }
>>  EXPORT_SYMBOL(iio_push_event);
>> @@ -96,15 +73,14 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
>>  				     loff_t *f_ps)
>>  {
>>  	struct iio_event_interface *ev_int = filep->private_data;
>> -	struct iio_detected_event_list *el;
>> -	size_t len = sizeof(el->ev);
>> +	unsigned int copied;
>>  	int ret;
>>  
>> -	if (count < len)
>> +	if (count < sizeof(struct iio_event_data))
>>  		return -EINVAL;
>>  
>>  	mutex_lock(&ev_int->event_list_lock);
>> -	if (list_empty(&ev_int->det_events)) {
>> +	if (kfifo_is_empty(&ev_int->det_events)) {
>>  		if (filep->f_flags & O_NONBLOCK) {
>>  			ret = -EAGAIN;
>>  			goto error_mutex_unlock;
>> @@ -112,52 +88,29 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
>>  		mutex_unlock(&ev_int->event_list_lock);
>>  		/* Blocking on device; waiting for something to be there */
>>  		ret = wait_event_interruptible(ev_int->wait,
>> -					       !list_empty(&ev_int
>> -							   ->det_events));
>> +					!kfifo_is_empty(&ev_int->det_events));
>>  		if (ret)
>>  			goto error_ret;
>>  		/* Single access device so no one else can get the data */
>>  		mutex_lock(&ev_int->event_list_lock);
>>  	}
>>  
>> -	el = list_first_entry(&ev_int->det_events,
>> -			      struct iio_detected_event_list,
>> -			      list);
>> -	if (copy_to_user(buf, &(el->ev), len)) {
>> -		ret = -EFAULT;
>> -		goto error_mutex_unlock;
>> -	}
>> -	list_del(&el->list);
>> -	ev_int->current_events--;
>>  	mutex_unlock(&ev_int->event_list_lock);
>> -	kfree(el);
>> -
>> -	return len;
>> +	ret = kfifo_to_user(&ev_int->det_events, buf, count, &copied);
>>  
>>  error_mutex_unlock:
>>  	mutex_unlock(&ev_int->event_list_lock);
>>  error_ret:
>> -
>> -	return ret;
>> +	return ret ? ret : copied;
>>  }
>>  
>>  static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
>>  {
>>  	struct iio_event_interface *ev_int = filep->private_data;
>> -	struct iio_detected_event_list *el, *t;
>>  
>>  	mutex_lock(&ev_int->event_list_lock);
>>  	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>> -	/*
>> -	 * In order to maintain a clean state for reopening,
>> -	 * clear out any awaiting events. The mask will prevent
>> -	 * any new __iio_push_event calls running.
>> -	 */
> Comment is still relevant (if updated appropriately)
>> -	list_for_each_entry_safe(el, t, &ev_int->det_events, list) {
>> -		list_del(&el->list);
>> -		kfree(el);
>> -	}
>> -	ev_int->current_events = 0;
>> +	kfifo_reset_out(&ev_int->det_events);
>>  	mutex_unlock(&ev_int->event_list_lock);
>>  
>>  	return 0;
>> @@ -269,9 +222,6 @@ static ssize_t iio_ev_value_store(struct device *dev,
>>  	unsigned long val;
>>  	int ret;
>>  
>> -	if (!indio_dev->info->write_event_value)
>> -		return -EINVAL;
>> -
>>  	ret = strict_strtoul(buf, 10, &val);
>>  	if (ret)
>>  		return ret;
>> @@ -402,10 +352,7 @@ static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
>>  static void iio_setup_ev_int(struct iio_event_interface *ev_int)
>>  {
>>  	mutex_init(&ev_int->event_list_lock);
>> -	/* discussion point - make this variable? */
>> -	ev_int->max_events = 10;
>> -	ev_int->current_events = 0;
>> -	INIT_LIST_HEAD(&ev_int->det_events);
>> +	INIT_KFIFO(ev_int->det_events);
>>  	init_waitqueue_head(&ev_int->wait);
>>  }
>>  
> 

Re: [PATCH 3/4] staging:iio:events: Use waitqueue lock to protect event queue

[Jonathan Cameron]

On 12/16/2011 05:12 PM, Lars-Peter Clausen wrote:
> Use the waitqueue lock to protect the event queue instead of a custom mutex.
> This has the advantage that we can call the waitqueue operations with the lock
> held, which simplifies the code flow a bit.
Didn't realise this was an option. I'm not finding all that many places
doing this.  Could you point out some examples to compare with?
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/staging/iio/industrialio-event.c |   43 ++++++++++++-----------------
>  1 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
> index 50d03bd..ef4943a 100644
> --- a/drivers/staging/iio/industrialio-event.c
> +++ b/drivers/staging/iio/industrialio-event.c
> @@ -34,7 +34,6 @@
>   */
>  struct iio_event_interface {
>  	wait_queue_head_t			wait;
> -	struct mutex				event_list_lock;
>  	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
>  
>  	struct list_head dev_attr_list;
> @@ -49,19 +48,17 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
>  	int ret = 0;
>  
>  	/* Does anyone care? */
> -	mutex_lock(&ev_int->event_list_lock);
> +	spin_lock(&ev_int->wait.lock);
>  	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>  
>  		ev.id = ev_code;
>  		ev.timestamp = timestamp;
>  
>  		ret = kfifo_put(&ev_int->det_events, &ev);
> -
> -		mutex_unlock(&ev_int->event_list_lock);
>  		if (ret != 0)
> -			wake_up_interruptible(&ev_int->wait);
> -	} else
> -		mutex_unlock(&ev_int->event_list_lock);
> +			wake_up_locked(&ev_int->wait);
> +	}
> +	spin_unlock(&ev_int->wait.lock);
>  
>  	return ret;
>  }
> @@ -79,28 +76,25 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
>  	if (count < sizeof(struct iio_event_data))
>  		return -EINVAL;
>  
> -	mutex_lock(&ev_int->event_list_lock);
> +	spin_lock(&ev_int->wait.lock);
>  	if (kfifo_is_empty(&ev_int->det_events)) {
>  		if (filep->f_flags & O_NONBLOCK) {
>  			ret = -EAGAIN;
> -			goto error_mutex_unlock;
> +			goto error_unlock;
>  		}
> -		mutex_unlock(&ev_int->event_list_lock);
>  		/* Blocking on device; waiting for something to be there */
> -		ret = wait_event_interruptible(ev_int->wait,
> +		ret = wait_event_interruptible_locked(ev_int->wait,
>  					!kfifo_is_empty(&ev_int->det_events));
>  		if (ret)
> -			goto error_ret;
> +			goto error_unlock;
>  		/* Single access device so no one else can get the data */
> -		mutex_lock(&ev_int->event_list_lock);
>  	}
>  
> -	mutex_unlock(&ev_int->event_list_lock);
>  	ret = kfifo_to_user(&ev_int->det_events, buf, count, &copied);
>  
> -error_mutex_unlock:
> -	mutex_unlock(&ev_int->event_list_lock);
> -error_ret:
> +error_unlock:
> +	spin_unlock(&ev_int->wait.lock);
> +
>  	return ret ? ret : copied;
>  }
>  
> @@ -108,10 +102,10 @@ static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
>  {
>  	struct iio_event_interface *ev_int = filep->private_data;
>  
> -	mutex_lock(&ev_int->event_list_lock);
> +	spin_lock(&ev_int->wait.lock);
>  	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>  	kfifo_reset_out(&ev_int->det_events);
> -	mutex_unlock(&ev_int->event_list_lock);
> +	spin_unlock(&ev_int->wait.lock);
>  
>  	return 0;
>  }
> @@ -131,18 +125,18 @@ int iio_event_getfd(struct iio_dev *indio_dev)
>  	if (ev_int == NULL)
>  		return -ENODEV;
>  
> -	mutex_lock(&ev_int->event_list_lock);
> +	spin_lock(&ev_int->wait.lock);
>  	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
> -		mutex_unlock(&ev_int->event_list_lock);
> +		spin_unlock(&ev_int->wait.lock);
>  		return -EBUSY;
>  	}
> -	mutex_unlock(&ev_int->event_list_lock);
> +	spin_unlock(&ev_int->wait.lock);
>  	fd = anon_inode_getfd("iio:event",
>  				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
>  	if (fd < 0) {
> -		mutex_lock(&ev_int->event_list_lock);
> +		spin_lock(&ev_int->wait.lock);
>  		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
> -		mutex_unlock(&ev_int->event_list_lock);
> +		spin_unlock(&ev_int->wait.lock);
>  	}
>  	return fd;
>  }
> @@ -351,7 +345,6 @@ static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
>  
>  static void iio_setup_ev_int(struct iio_event_interface *ev_int)
>  {
> -	mutex_init(&ev_int->event_list_lock);
>  	INIT_KFIFO(ev_int->det_events);
>  	init_waitqueue_head(&ev_int->wait);
>  }

Re: [PATCH 3/4] staging:iio:events: Use waitqueue lock to protect event queue

[Lars-Peter Clausen]

On 12/18/2011 10:54 PM, Jonathan Cameron wrote:
> On 12/16/2011 05:12 PM, Lars-Peter Clausen wrote:
>> Use the waitqueue lock to protect the event queue instead of a custom mutex.
>> This has the advantage that we can call the waitqueue operations with the lock
>> held, which simplifies the code flow a bit.
> Didn't realise this was an option. I'm not finding all that many places
> doing this.  Could you point out some examples to compare with?

fs/timerfd.c and also see commit 22c43c81a51
("wait_event_interruptible_locked() interface")

>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>>  drivers/staging/iio/industrialio-event.c |   43 ++++++++++++-----------------
>>  1 files changed, 18 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
>> index 50d03bd..ef4943a 100644
>> --- a/drivers/staging/iio/industrialio-event.c
>> +++ b/drivers/staging/iio/industrialio-event.c
>> @@ -34,7 +34,6 @@
>>   */
>>  struct iio_event_interface {
>>  	wait_queue_head_t			wait;
>> -	struct mutex				event_list_lock;
>>  	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
>>  
>>  	struct list_head dev_attr_list;
>> @@ -49,19 +48,17 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
>>  	int ret = 0;
>>  
>>  	/* Does anyone care? */
>> -	mutex_lock(&ev_int->event_list_lock);
>> +	spin_lock(&ev_int->wait.lock);
>>  	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>>  
>>  		ev.id = ev_code;
>>  		ev.timestamp = timestamp;
>>  
>>  		ret = kfifo_put(&ev_int->det_events, &ev);
>> -
>> -		mutex_unlock(&ev_int->event_list_lock);
>>  		if (ret != 0)
>> -			wake_up_interruptible(&ev_int->wait);
>> -	} else
>> -		mutex_unlock(&ev_int->event_list_lock);
>> +			wake_up_locked(&ev_int->wait);
>> +	}
>> +	spin_unlock(&ev_int->wait.lock);
>>  
>>  	return ret;
>>  }
>> @@ -79,28 +76,25 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
>>  	if (count < sizeof(struct iio_event_data))
>>  		return -EINVAL;
>>  
>> -	mutex_lock(&ev_int->event_list_lock);
>> +	spin_lock(&ev_int->wait.lock);
>>  	if (kfifo_is_empty(&ev_int->det_events)) {
>>  		if (filep->f_flags & O_NONBLOCK) {
>>  			ret = -EAGAIN;
>> -			goto error_mutex_unlock;
>> +			goto error_unlock;
>>  		}
>> -		mutex_unlock(&ev_int->event_list_lock);
>>  		/* Blocking on device; waiting for something to be there */
>> -		ret = wait_event_interruptible(ev_int->wait,
>> +		ret = wait_event_interruptible_locked(ev_int->wait,
>>  					!kfifo_is_empty(&ev_int->det_events));
>>  		if (ret)
>> -			goto error_ret;
>> +			goto error_unlock;
>>  		/* Single access device so no one else can get the data */
>> -		mutex_lock(&ev_int->event_list_lock);
>>  	}
>>  
>> -	mutex_unlock(&ev_int->event_list_lock);
>>  	ret = kfifo_to_user(&ev_int->det_events, buf, count, &copied);
>>  
>> -error_mutex_unlock:
>> -	mutex_unlock(&ev_int->event_list_lock);
>> -error_ret:
>> +error_unlock:
>> +	spin_unlock(&ev_int->wait.lock);
>> +
>>  	return ret ? ret : copied;
>>  }
>>  
>> @@ -108,10 +102,10 @@ static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
>>  {
>>  	struct iio_event_interface *ev_int = filep->private_data;
>>  
>> -	mutex_lock(&ev_int->event_list_lock);
>> +	spin_lock(&ev_int->wait.lock);
>>  	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>>  	kfifo_reset_out(&ev_int->det_events);
>> -	mutex_unlock(&ev_int->event_list_lock);
>> +	spin_unlock(&ev_int->wait.lock);
>>  
>>  	return 0;
>>  }
>> @@ -131,18 +125,18 @@ int iio_event_getfd(struct iio_dev *indio_dev)
>>  	if (ev_int == NULL)
>>  		return -ENODEV;
>>  
>> -	mutex_lock(&ev_int->event_list_lock);
>> +	spin_lock(&ev_int->wait.lock);
>>  	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>> -		mutex_unlock(&ev_int->event_list_lock);
>> +		spin_unlock(&ev_int->wait.lock);
>>  		return -EBUSY;
>>  	}
>> -	mutex_unlock(&ev_int->event_list_lock);
>> +	spin_unlock(&ev_int->wait.lock);
>>  	fd = anon_inode_getfd("iio:event",
>>  				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
>>  	if (fd < 0) {
>> -		mutex_lock(&ev_int->event_list_lock);
>> +		spin_lock(&ev_int->wait.lock);
>>  		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>> -		mutex_unlock(&ev_int->event_list_lock);
>> +		spin_unlock(&ev_int->wait.lock);
>>  	}
>>  	return fd;
>>  }
>> @@ -351,7 +345,6 @@ static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
>>  
>>  static void iio_setup_ev_int(struct iio_event_interface *ev_int)
>>  {
>> -	mutex_init(&ev_int->event_list_lock);
>>  	INIT_KFIFO(ev_int->det_events);
>>  	init_waitqueue_head(&ev_int->wait);
>>  }
> 
> 

Re: [PATCH 3/4] staging:iio:events: Use waitqueue lock to protect event queue

[J.I. Cameron]

On Dec 19 2011, Lars-Peter Clausen wrote:

>On 12/18/2011 10:54 PM, Jonathan Cameron wrote:
>> On 12/16/2011 05:12 PM, Lars-Peter Clausen wrote:
>>> Use the waitqueue lock to protect the event queue instead of a custom mutex.
>>> This has the advantage that we can call the waitqueue operations with the lock
>>> held, which simplifies the code flow a bit.
>> Didn't realise this was an option. I'm not finding all that many places
>> doing this.  Could you point out some examples to compare with?
>
>fs/timerfd.c and also see commit 22c43c81a51
>("wait_event_interruptible_locked() interface")
Looks 'by the book' so fine with me (and Greg signed off
on that patch so it should catch him by suprise!)
>
>>>
>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org>
>>> ---
>>>  drivers/staging/iio/industrialio-event.c |   43 ++++++++++++-----------------
>>>  1 files changed, 18 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
>>> index 50d03bd..ef4943a 100644
>>> --- a/drivers/staging/iio/industrialio-event.c
>>> +++ b/drivers/staging/iio/industrialio-event.c
>>> @@ -34,7 +34,6 @@
>>>   */
>>>  struct iio_event_interface {
>>>  	wait_queue_head_t			wait;
>>> -	struct mutex				event_list_lock;
>>>  	DECLARE_KFIFO(det_events, struct iio_event_data, 16);
>>>  
>>>  	struct list_head dev_attr_list;
>>> @@ -49,19 +48,17 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
>>>  	int ret = 0;
>>>  
>>>  	/* Does anyone care? */
>>> -	mutex_lock(&ev_int->event_list_lock);
>>> +	spin_lock(&ev_int->wait.lock);
>>>  	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>>>  
>>>  		ev.id = ev_code;
>>>  		ev.timestamp = timestamp;
>>>  
>>>  		ret = kfifo_put(&ev_int->det_events, &ev);
>>> -
>>> -		mutex_unlock(&ev_int->event_list_lock);
>>>  		if (ret != 0)
>>> -			wake_up_interruptible(&ev_int->wait);
>>> -	} else
>>> -		mutex_unlock(&ev_int->event_list_lock);
>>> +			wake_up_locked(&ev_int->wait);
>>> +	}
>>> +	spin_unlock(&ev_int->wait.lock);
>>>  
>>>  	return ret;
>>>  }
>>> @@ -79,28 +76,25 @@ static ssize_t iio_event_chrdev_read(struct file *filep,
>>>  	if (count < sizeof(struct iio_event_data))
>>>  		return -EINVAL;
>>>  
>>> -	mutex_lock(&ev_int->event_list_lock);
>>> +	spin_lock(&ev_int->wait.lock);
>>>  	if (kfifo_is_empty(&ev_int->det_events)) {
>>>  		if (filep->f_flags & O_NONBLOCK) {
>>>  			ret = -EAGAIN;
>>> -			goto error_mutex_unlock;
>>> +			goto error_unlock;
>>>  		}
>>> -		mutex_unlock(&ev_int->event_list_lock);
>>>  		/* Blocking on device; waiting for something to be there */
>>> -		ret = wait_event_interruptible(ev_int->wait,
>>> +		ret = wait_event_interruptible_locked(ev_int->wait,
>>>  					!kfifo_is_empty(&ev_int->det_events));
>>>  		if (ret)
>>> -			goto error_ret;
>>> +			goto error_unlock;
>>>  		/* Single access device so no one else can get the data */
>>> -		mutex_lock(&ev_int->event_list_lock);
>>>  	}
>>>  
>>> -	mutex_unlock(&ev_int->event_list_lock);
>>>  	ret = kfifo_to_user(&ev_int->det_events, buf, count, &copied);
>>>  
>>> -error_mutex_unlock:
>>> -	mutex_unlock(&ev_int->event_list_lock);
>>> -error_ret:
>>> +error_unlock:
>>> +	spin_unlock(&ev_int->wait.lock);
>>> +
>>>  	return ret ? ret : copied;
>>>  }
>>>  
>>> @@ -108,10 +102,10 @@ static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
>>>  {
>>>  	struct iio_event_interface *ev_int = filep->private_data;
>>>  
>>> -	mutex_lock(&ev_int->event_list_lock);
>>> +	spin_lock(&ev_int->wait.lock);
>>>  	clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>>>  	kfifo_reset_out(&ev_int->det_events);
>>> -	mutex_unlock(&ev_int->event_list_lock);
>>> +	spin_unlock(&ev_int->wait.lock);
>>>  
>>>  	return 0;
>>>  }
>>> @@ -131,18 +125,18 @@ int iio_event_getfd(struct iio_dev *indio_dev)
>>>  	if (ev_int == NULL)
>>>  		return -ENODEV;
>>>  
>>> -	mutex_lock(&ev_int->event_list_lock);
>>> +	spin_lock(&ev_int->wait.lock);
>>>  	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>>> -		mutex_unlock(&ev_int->event_list_lock);
>>> +		spin_unlock(&ev_int->wait.lock);
>>>  		return -EBUSY;
>>>  	}
>>> -	mutex_unlock(&ev_int->event_list_lock);
>>> +	spin_unlock(&ev_int->wait.lock);
>>>  	fd = anon_inode_getfd("iio:event",
>>>  				&iio_event_chrdev_fileops, ev_int, O_RDONLY);
>>>  	if (fd < 0) {
>>> -		mutex_lock(&ev_int->event_list_lock);
>>> +		spin_lock(&ev_int->wait.lock);
>>>  		clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>>> -		mutex_unlock(&ev_int->event_list_lock);
>>> +		spin_unlock(&ev_int->wait.lock);
>>>  	}
>>>  	return fd;
>>>  }
>>> @@ -351,7 +345,6 @@ static bool iio_check_for_dynamic_events(struct iio_dev *indio_dev)
>>>  
>>>  static void iio_setup_ev_int(struct iio_event_interface *ev_int)
>>>  {
>>> -	mutex_init(&ev_int->event_list_lock);
>>>  	INIT_KFIFO(ev_int->det_events);
>>>  	init_waitqueue_head(&ev_int->wait);
>>>  }
>> 
>> 
>

Re: [PATCH 4/4] staging:iio:events: Add poll support

[jic23]

Lars-Peter Clausen writes: 

> Add poll support to the event queue. This will allow us to check for pending
> events in a application's event loop using poll() or similar. Since we already
> have support for blocking reads adding poll support as well is trivial. 
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Jonathan Cameron <jic23@kernel.org> 

Some corners here I'm not particularly familiar with, but is fine
as far as I can see! 

Thanks for these cleanups / functionality improvements. 

Jonathan
> ---
>  drivers/staging/iio/industrialio-event.c |   22 +++++++++++++++++++++-
>  1 files changed, 21 insertions(+), 1 deletions(-) 
> 
> diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/staging/iio/industrialio-event.c
> index ef4943a..2263269 100644
> --- a/drivers/staging/iio/industrialio-event.c
> +++ b/drivers/staging/iio/industrialio-event.c
> @@ -56,7 +56,7 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
>  
>  		ret = kfifo_put(&ev_int->det_events, &ev);
>  		if (ret != 0)
> -			wake_up_locked(&ev_int->wait);
> +			wake_up_locked_poll(&ev_int->wait, POLLIN);
>  	}
>  	spin_unlock(&ev_int->wait.lock);
>  
> @@ -64,6 +64,25 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp)
>  }
>  EXPORT_SYMBOL(iio_push_event);
>  
> +/**
> + * iio_event_poll() - poll the event queue to find out if it has data
> + */
> +static unsigned int iio_event_poll(struct file *filep,
> +			     struct poll_table_struct *wait)
> +{
> +	struct iio_event_interface *ev_int = filep->private_data;
> +	unsigned int events = 0;
> +
> +	poll_wait(filep, &ev_int->wait, wait);
> +
> +	spin_lock(&ev_int->wait.lock);
> +	if (!kfifo_is_empty(&ev_int->det_events))
> +		events = POLLIN | POLLRDNORM;
> +	spin_unlock(&ev_int->wait.lock);
> +
> +	return events;
> +}
> +
>  static ssize_t iio_event_chrdev_read(struct file *filep,
>  				     char __user *buf,
>  				     size_t count,
> @@ -112,6 +131,7 @@ static int iio_event_chrdev_release(struct inode *inode, struct file *filep)
>  
>  static const struct file_operations iio_event_chrdev_fileops = {
>  	.read =  iio_event_chrdev_read,
> +	.poll =  iio_event_poll,
>  	.release = iio_event_chrdev_release,
>  	.owner = THIS_MODULE,
>  	.llseek = noop_llseek,
> -- 
> 1.7.7.3 
> 
> 

Re: [PATCH 3/4] staging:iio:events: Use waitqueue lock to protect event queue

[J.I. Cameron]

On Dec 19 2011, J.I. Cameron wrote:

>On Dec 19 2011, Lars-Peter Clausen wrote:
>
>>On 12/18/2011 10:54 PM, Jonathan Cameron wrote:
>>> On 12/16/2011 05:12 PM, Lars-Peter Clausen wrote:
>>>> Use the waitqueue lock to protect the event queue instead of a custom =
mutex.
>>>> This has the advantage that we can call the waitqueue operations with =
the lock
>>>> held, which simplifies the code flow a bit.
>>> Didn't realise this was an option. I'm not finding all that many places
>>> doing this.  Could you point out some examples to compare with?
>>
>>fs/timerfd.c and also see commit 22c43c81a51
>>("wait_event_interruptible_locked() interface")
>Looks 'by the book' so fine with me (and Greg signed off
>on that patch so it should catch him by suprise!)
should not...
>>
>>>>
>>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>Acked-by: Jonathan Cameron <jic23@kernel.org>
>>>> ---
>>>>  drivers/staging/iio/industrialio-event.c |   43 ++++++++++++---------=
--------
>>>>  1 files changed, 18 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/iio/industrialio-event.c b/drivers/stagin=
g/iio/industrialio-event.c
>>>> index 50d03bd..ef4943a 100644
>>>> --- a/drivers/staging/iio/industrialio-event.c
>>>> +++ b/drivers/staging/iio/industrialio-event.c
>>>> @@ -34,7 +34,6 @@
>>>>   */
>>>>  struct iio_event_interface {
>>>>  =09wait_queue_head_t=09=09=09wait;
>>>> -=09struct mutex=09=09=09=09event_list_lock;
>>>>  =09DECLARE_KFIFO(det_events, struct iio_event_data, 16);
>>>> =20
>>>>  =09struct list_head dev_attr_list;
>>>> @@ -49,19 +48,17 @@ int iio_push_event(struct iio_dev *indio_dev, u64 =
ev_code, s64 timestamp)
>>>>  =09int ret =3D 0;
>>>> =20
>>>>  =09/* Does anyone care? */
>>>> -=09mutex_lock(&ev_int->event_list_lock);
>>>> +=09spin_lock(&ev_int->wait.lock);
>>>>  =09if (test_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>>>> =20
>>>>  =09=09ev.id =3D ev_code;
>>>>  =09=09ev.timestamp =3D timestamp;
>>>> =20
>>>>  =09=09ret =3D kfifo_put(&ev_int->det_events, &ev);
>>>> -
>>>> -=09=09mutex_unlock(&ev_int->event_list_lock);
>>>>  =09=09if (ret !=3D 0)
>>>> -=09=09=09wake_up_interruptible(&ev_int->wait);
>>>> -=09} else
>>>> -=09=09mutex_unlock(&ev_int->event_list_lock);
>>>> +=09=09=09wake_up_locked(&ev_int->wait);
>>>> +=09}
>>>> +=09spin_unlock(&ev_int->wait.lock);
>>>> =20
>>>>  =09return ret;
>>>>  }
>>>> @@ -79,28 +76,25 @@ static ssize_t iio_event_chrdev_read(struct file *=
filep,
>>>>  =09if (count < sizeof(struct iio_event_data))
>>>>  =09=09return -EINVAL;
>>>> =20
>>>> -=09mutex_lock(&ev_int->event_list_lock);
>>>> +=09spin_lock(&ev_int->wait.lock);
>>>>  =09if (kfifo_is_empty(&ev_int->det_events)) {
>>>>  =09=09if (filep->f_flags & O_NONBLOCK) {
>>>>  =09=09=09ret =3D -EAGAIN;
>>>> -=09=09=09goto error_mutex_unlock;
>>>> +=09=09=09goto error_unlock;
>>>>  =09=09}
>>>> -=09=09mutex_unlock(&ev_int->event_list_lock);
>>>>  =09=09/* Blocking on device; waiting for something to be there */
>>>> -=09=09ret =3D wait_event_interruptible(ev_int->wait,
>>>> +=09=09ret =3D wait_event_interruptible_locked(ev_int->wait,
>>>>  =09=09=09=09=09!kfifo_is_empty(&ev_int->det_events));
>>>>  =09=09if (ret)
>>>> -=09=09=09goto error_ret;
>>>> +=09=09=09goto error_unlock;
>>>>  =09=09/* Single access device so no one else can get the data */
>>>> -=09=09mutex_lock(&ev_int->event_list_lock);
>>>>  =09}
>>>> =20
>>>> -=09mutex_unlock(&ev_int->event_list_lock);
>>>>  =09ret =3D kfifo_to_user(&ev_int->det_events, buf, count, &copied);
>>>> =20
>>>> -error_mutex_unlock:
>>>> -=09mutex_unlock(&ev_int->event_list_lock);
>>>> -error_ret:
>>>> +error_unlock:
>>>> +=09spin_unlock(&ev_int->wait.lock);
>>>> +
>>>>  =09return ret ? ret : copied;
>>>>  }
>>>> =20
>>>> @@ -108,10 +102,10 @@ static int iio_event_chrdev_release(struct inode=
 *inode, struct file *filep)
>>>>  {
>>>>  =09struct iio_event_interface *ev_int =3D filep->private_data;
>>>> =20
>>>> -=09mutex_lock(&ev_int->event_list_lock);
>>>> +=09spin_lock(&ev_int->wait.lock);
>>>>  =09clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>>>>  =09kfifo_reset_out(&ev_int->det_events);
>>>> -=09mutex_unlock(&ev_int->event_list_lock);
>>>> +=09spin_unlock(&ev_int->wait.lock);
>>>> =20
>>>>  =09return 0;
>>>>  }
>>>> @@ -131,18 +125,18 @@ int iio_event_getfd(struct iio_dev *indio_dev)
>>>>  =09if (ev_int =3D=3D NULL)
>>>>  =09=09return -ENODEV;
>>>> =20
>>>> -=09mutex_lock(&ev_int->event_list_lock);
>>>> +=09spin_lock(&ev_int->wait.lock);
>>>>  =09if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev_int->flags)) {
>>>> -=09=09mutex_unlock(&ev_int->event_list_lock);
>>>> +=09=09spin_unlock(&ev_int->wait.lock);
>>>>  =09=09return -EBUSY;
>>>>  =09}
>>>> -=09mutex_unlock(&ev_int->event_list_lock);
>>>> +=09spin_unlock(&ev_int->wait.lock);
>>>>  =09fd =3D anon_inode_getfd("iio:event",
>>>>  =09=09=09=09&iio_event_chrdev_fileops, ev_int, O_RDONLY);
>>>>  =09if (fd < 0) {
>>>> -=09=09mutex_lock(&ev_int->event_list_lock);
>>>> +=09=09spin_lock(&ev_int->wait.lock);
>>>>  =09=09clear_bit(IIO_BUSY_BIT_POS, &ev_int->flags);
>>>> -=09=09mutex_unlock(&ev_int->event_list_lock);
>>>> +=09=09spin_unlock(&ev_int->wait.lock);
>>>>  =09}
>>>>  =09return fd;
>>>>  }
>>>> @@ -351,7 +345,6 @@ static bool iio_check_for_dynamic_events(struct ii=
o_dev *indio_dev)
>>>> =20
>>>>  static void iio_setup_ev_int(struct iio_event_interface *ev_int)
>>>>  {
>>>> -=09mutex_init(&ev_int->event_list_lock);
>>>>  =09INIT_KFIFO(ev_int->det_events);
>>>>  =09init_waitqueue_head(&ev_int->wait);
>>>>  }
>>>=20
>>>=20
>>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>