[PATCH v2 0/5] iio: core,buffer: re-organize chardev creation

[PATCH v2 0/5] iio: core,buffer: re-organize chardev creation

[Alexandru Ardelean]

The main intent is to be able to add more chardevs per IIO device, one
for each buffer. To get there, some rework is needed, and one important
facet is to move the creation of the current chardev into
'industrialio-buffer.c', so that control/logic of these chardevs is
localized in that file.

This changeset does that [incrementally] by moving the common chardev
creation from 'industrialio-core.c' to 'industrialio-buffer.c' &
'industrialio-event.c'.
The common chardev is required for both IIO buffers & IIO events.
In order to make this work, the 'iio_device_event_ioctl()' needs to be
passed from 'industrialio-event.c' to 'industrialio-buffer.c' flying past
'industrialio-core.c'. This sounds a bit wrong [at first] but it has the
effect of reducing inter-dependencies between 'industrialio-core.c' to
'industrialio-buffer.c' quite a bit.
The IIO buffer also has a CONFIG_IIO_BUFFER symbol which can turn it
off. No idea how widely this is used [as disabled], but this changeset
also takes that into consideration.

So, now the logic [for __iio_device_register() with regard to chardev
init] is:
1. iio_device_buffers_init() will init buffer and the chardev, if that
   works, the 'iio_device_event_ioctl()' will be attached to the chardev
2. if CONFIG_IIO_BUFFER is not defined or 'indio_dev->buffer == NULL'
   (no buffter attached), -ENOTSUPP should be returned from
   iio_device_buffers_init(), in which case the chardev should be
   initialized in  'industrialio-event.c' via
   'iio_device_register_event_chrdev()'

One neat side effect of this logic, is that we can also move the buffer
sysfs alloc/cleanup into 'industrialio-buffer.c' under the new
'iio_device_buffers_{un}init()' functions.

Changelog v1 -> v2:
* re-reviewed some exit-paths and cleanup some potential leaks on those
  exit paths:
  - for 'iio: buffer: move iio buffer chrdev in industrialio-buffer.c'
    add iio_device_buffers_put() helper and calling iio_buffers_uninit()
    on device un-regsiter
  - for 'move sysfs alloc/free in industrialio-buffer.c'
    call 'iio_buffer_free_sysfs_and_mask()' on exit path if
    cdev_device_add() fails
  - for 'move event-only chardev in industrialio-event.c'
    check if event_interface is NULL in
    iio_device_unregister_event_chrdev()

Alexandru Ardelean (5):
  iio: core: register buffer fileops only if buffer present
  iio: buffer: add back-ref from iio_buffer to iio_dev
  iio: buffer: move iio buffer chrdev in industrialio-buffer.c
  iio: buffer: move sysfs alloc/free in industrialio-buffer.c
  iio: event: move event-only chardev in industrialio-event.c

 drivers/iio/iio_core.h            |  30 +++----
 drivers/iio/industrialio-buffer.c | 144 ++++++++++++++++++++++++++----
 drivers/iio/industrialio-core.c   | 107 +++-------------------
 drivers/iio/industrialio-event.c  | 124 ++++++++++++++++++++++++-
 include/linux/iio/buffer_impl.h   |  10 +++
 include/linux/iio/iio.h           |   4 -
 6 files changed, 287 insertions(+), 132 deletions(-)

-- 
2.17.1

[PATCH v2 1/5] iio: core: register buffer fileops only if buffer present

[Alexandru Ardelean]

The intent is to localize all buffer ops into the industrialio-buffer.c
file, to be able to add support for multiple buffers per IIO device.

We still need to allocate a chardev in __iio_device_register() to be able
to pass event ioctl commands. So, if the IIO device has no buffer, we
create the legacy chardev for the event ioctl() command.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/industrialio-core.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 157d95a24faa..c8c074602709 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1707,6 +1707,15 @@ static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops noop_ring_setup_ops;
 
+static const struct file_operations iio_event_fileops = {
+	.release = iio_chrdev_release,
+	.open = iio_chrdev_open,
+	.owner = THIS_MODULE,
+	.llseek = noop_llseek,
+	.unlocked_ioctl = iio_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
+};
+
 int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 {
 	int ret;
@@ -1757,7 +1766,10 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 		indio_dev->setup_ops == NULL)
 		indio_dev->setup_ops = &noop_ring_setup_ops;
 
-	cdev_init(&indio_dev->chrdev, &iio_buffer_fileops);
+	if (indio_dev->buffer)
+		cdev_init(&indio_dev->chrdev, &iio_buffer_fileops);
+	else
+		cdev_init(&indio_dev->chrdev, &iio_event_fileops);
 
 	indio_dev->chrdev.owner = this_mod;
 
-- 
2.17.1

[PATCH v2 2/5] iio: buffer: add back-ref from iio_buffer to iio_dev

[Alexandru Ardelean]

An IIO device will have multiple buffers, but it shouldn't be allowed that
an IIO buffer should belong to more than 1 IIO device.

Once things get moved more from IIO device to the IIO buffer, and an IIO
device will be able to have more than 1 buffer attached, there will be a
need for a back-ref to the IIO device [from the IIO buffer].

This change adds that.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/industrialio-buffer.c | 2 ++
 include/linux/iio/buffer_impl.h   | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index e6fa1a4e135d..f9ffc7762f6c 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1478,5 +1478,7 @@ void iio_device_attach_buffer(struct iio_dev *indio_dev,
 			      struct iio_buffer *buffer)
 {
 	indio_dev->buffer = iio_buffer_get(buffer);
+
+	indio_dev->buffer->indio_dev = indio_dev;
 }
 EXPORT_SYMBOL_GPL(iio_device_attach_buffer);
diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
index 1e7edf6bed96..8fb92250a190 100644
--- a/include/linux/iio/buffer_impl.h
+++ b/include/linux/iio/buffer_impl.h
@@ -69,6 +69,9 @@ struct iio_buffer_access_funcs {
  * those writing new buffer implementations.
  */
 struct iio_buffer {
+	/** @indio_dev: IIO device to which this buffer belongs to. */
+	struct iio_dev *indio_dev;
+
 	/** @length: Number of datums in buffer. */
 	unsigned int length;
 
-- 
2.17.1

[PATCH v2 3/5] iio: buffer: move iio buffer chrdev in industrialio-buffer.c

[Alexandru Ardelean]

This change makes the first jump to move the buffer chardev from indio_dev
to the buffer attached to indio_dev (i.e. indio_dev->buffer).

This requires that some functions that are shared between
'industrialio-core.c' & industrialio-buffer.c be re-shuffled.
The 'iio_buffer_fileops' is now moved to 'industrialio-buffer.c'. It's also
more appropriate to have it here, but the old 'legacy' iio_ioctl() must be
passed to it.

What happens now is:
* if IS_ENABLED(CONFIG_IIO_BUFFER) and 'indio_dev->buffer != NULL' the
  indio_dev->buffer->chrdev will be initialized (as the old legacy style
  chardev)
* if -ENOTSUPP is returned for either of the conditions above (not being
  met), the chardev will be created directly via 'indio_dev->chrdev', same
  as before; a new field is required now 'indio_dev->chrdev_initialized' to
  mark it true, so that 'indio_dev->chrdev' gets deleted if initialized;

'indio_dev->chrdev_initialized' is of type 'int', because recently
checkpatch complains that 'bool' types on structs can cause alignment
issues.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/iio_core.h            |  25 ++++---
 drivers/iio/industrialio-buffer.c | 119 ++++++++++++++++++++++++++++--
 drivers/iio/industrialio-core.c   |  59 ++++++++-------
 include/linux/iio/buffer_impl.h   |   7 ++
 include/linux/iio/iio.h           |   2 +
 5 files changed, 170 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index fd9a5f1d5e51..4bdadeac2710 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -40,24 +40,31 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals);
 #ifdef CONFIG_IIO_BUFFER
 struct poll_table_struct;
 
-__poll_t iio_buffer_poll(struct file *filp,
-			     struct poll_table_struct *wait);
-ssize_t iio_buffer_read_outer(struct file *filp, char __user *buf,
-			      size_t n, loff_t *f_ps);
+long iio_device_event_ioctl(struct iio_dev *indio_dev, struct file *filp,
+			    unsigned int cmd, unsigned long arg);
+
+int iio_device_buffers_init(struct iio_dev *indio_dev, struct module *this_mod);
+void iio_device_buffers_uninit(struct iio_dev *indio_dev);
+
+void iio_device_buffers_put(struct iio_dev *indio_dev);
 
 int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev);
 void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev);
 
-#define iio_buffer_poll_addr (&iio_buffer_poll)
-#define iio_buffer_read_outer_addr (&iio_buffer_read_outer)
-
 void iio_disable_all_buffers(struct iio_dev *indio_dev);
 void iio_buffer_wakeup_poll(struct iio_dev *indio_dev);
 
 #else
 
-#define iio_buffer_poll_addr NULL
-#define iio_buffer_read_outer_addr NULL
+static inline int iio_device_buffers_init(struct iio_dev *indio_dev,
+					  struct module *this_mod)
+{
+	return -ENOTSUPP;
+}
+
+static inline void iio_device_buffers_uninit(struct iio_dev *indio_dev) {}
+
+static inline void iio_device_buffers_put(struct iio_dev *indio_dev) {}
 
 static inline int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
 {
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index f9ffc7762f6c..4b5c3baadaab 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -99,11 +99,11 @@ static bool iio_buffer_ready(struct iio_dev *indio_dev, struct iio_buffer *buf,
  * Return: negative values corresponding to error codes or ret != 0
  *	   for ending the reading activity
  **/
-ssize_t iio_buffer_read_outer(struct file *filp, char __user *buf,
-			      size_t n, loff_t *f_ps)
+static ssize_t iio_buffer_read_outer(struct file *filp, char __user *buf,
+				     size_t n, loff_t *f_ps)
 {
-	struct iio_dev *indio_dev = filp->private_data;
-	struct iio_buffer *rb = indio_dev->buffer;
+	struct iio_buffer *rb = filp->private_data;
+	struct iio_dev *indio_dev = rb->indio_dev;
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
 	size_t datum_size;
 	size_t to_wait;
@@ -165,11 +165,11 @@ ssize_t iio_buffer_read_outer(struct file *filp, char __user *buf,
  * Return: (EPOLLIN | EPOLLRDNORM) if data is available for reading
  *	   or 0 for other cases
  */
-__poll_t iio_buffer_poll(struct file *filp,
+static __poll_t iio_buffer_poll(struct file *filp,
 			     struct poll_table_struct *wait)
 {
-	struct iio_dev *indio_dev = filp->private_data;
-	struct iio_buffer *rb = indio_dev->buffer;
+	struct iio_buffer *rb = filp->private_data;
+	struct iio_dev *indio_dev = rb->indio_dev;
 
 	if (!indio_dev->info || rb == NULL)
 		return 0;
@@ -180,6 +180,48 @@ __poll_t iio_buffer_poll(struct file *filp,
 	return 0;
 }
 
+/**
+ * iio_buffer_chrdev_open() - chrdev file open for buffer access
+ * @inode:	Inode structure for identifying the device in the file system
+ * @filp:	File structure for iio device used to keep and later access
+ *		private data
+ *
+ * Return: 0 on success or -EBUSY if the device is already opened
+ **/
+static int iio_buffer_chrdev_open(struct inode *inode, struct file *filp)
+{
+	struct iio_buffer *buffer = container_of(inode->i_cdev,
+						 struct iio_buffer, chrdev);
+
+	if (test_and_set_bit(IIO_BUSY_BIT_POS, &buffer->file_ops_flags))
+		return -EBUSY;
+
+	iio_buffer_get(buffer);
+
+	filp->private_data = buffer;
+
+	return 0;
+}
+
+/**
+ * iio_buffer_chrdev_release() - chrdev file close for buffer access
+ * @inode:	Inode structure pointer for the char device
+ * @filp:	File structure pointer for the char device
+ *
+ * Return: 0 for successful release
+ */
+static int iio_buffer_chrdev_release(struct inode *inode, struct file *filp)
+{
+	struct iio_buffer *buffer = container_of(inode->i_cdev,
+						 struct iio_buffer, chrdev);
+
+	clear_bit(IIO_BUSY_BIT_POS, &buffer->file_ops_flags);
+
+	iio_buffer_put(buffer);
+
+	return 0;
+}
+
 /**
  * iio_buffer_wakeup_poll - Wakes up the buffer waitqueue
  * @indio_dev: The IIO device
@@ -1121,6 +1163,14 @@ void iio_disable_all_buffers(struct iio_dev *indio_dev)
 	iio_buffer_deactivate_all(indio_dev);
 }
 
+long iio_buffer_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
+{
+	struct iio_buffer *buffer = filep->private_data;
+	struct iio_dev *indio_dev = buffer->indio_dev;
+
+	return iio_device_event_ioctl(indio_dev, filep, cmd, arg);
+}
+
 static ssize_t iio_buffer_store_enable(struct device *dev,
 				       struct device_attribute *attr,
 				       const char *buf,
@@ -1356,6 +1406,61 @@ void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev)
 	iio_free_chan_devattr_list(&indio_dev->buffer->scan_el_dev_attr_list);
 }
 
+static const struct file_operations iio_buffer_fileops = {
+	.read = iio_buffer_read_outer,
+	.release = iio_buffer_chrdev_release,
+	.open = iio_buffer_chrdev_open,
+	.poll = iio_buffer_poll,
+	.owner = THIS_MODULE,
+	.llseek = noop_llseek,
+	.unlocked_ioctl = iio_buffer_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
+};
+
+int iio_device_buffers_init(struct iio_dev *indio_dev, struct module *this_mod)
+{
+	struct iio_buffer *buffer = indio_dev->buffer;
+	int ret;
+
+	if (!buffer)
+		return -ENOTSUPP;
+
+	cdev_init(&buffer->chrdev, &iio_buffer_fileops);
+
+	buffer->chrdev.owner = this_mod;
+
+	ret = cdev_device_add(&buffer->chrdev, &indio_dev->dev);
+	if (ret)
+		return ret;
+
+	iio_device_get(indio_dev);
+	iio_buffer_get(buffer);
+
+	return 0;
+}
+
+void iio_device_buffers_put(struct iio_dev *indio_dev)
+{
+	struct iio_buffer *buffer = indio_dev->buffer;
+
+	if (!buffer)
+		return;
+
+	iio_buffer_put(buffer);
+}
+
+void iio_device_buffers_uninit(struct iio_dev *indio_dev)
+{
+	struct iio_buffer *buffer = indio_dev->buffer;
+
+	if (!buffer)
+		return;
+
+	cdev_device_del(&buffer->chrdev, &indio_dev->dev);
+	iio_buffer_put(buffer);
+	iio_device_put(indio_dev);
+}
+
 /**
  * iio_validate_scan_mask_onehot() - Validates that exactly one channel is selected
  * @indio_dev: the iio device
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index c8c074602709..2158aeab0bd2 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1475,7 +1475,7 @@ static void iio_dev_release(struct device *device)
 	iio_device_unregister_eventset(indio_dev);
 	iio_device_unregister_sysfs(indio_dev);
 
-	iio_buffer_put(indio_dev->buffer);
+	iio_device_buffers_put(indio_dev);
 
 	ida_simple_remove(&iio_ida, indio_dev->id);
 	kfree(indio_dev);
@@ -1610,7 +1610,7 @@ void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev)
 EXPORT_SYMBOL_GPL(devm_iio_device_free);
 
 /**
- * iio_chrdev_open() - chrdev file open for buffer access and ioctls
+ * iio_chrdev_open() - chrdev file open for event ioctls
  * @inode:	Inode structure for identifying the device in the file system
  * @filp:	File structure for iio device used to keep and later access
  *		private data
@@ -1633,7 +1633,7 @@ static int iio_chrdev_open(struct inode *inode, struct file *filp)
 }
 
 /**
- * iio_chrdev_release() - chrdev file close buffer access and ioctls
+ * iio_chrdev_release() - chrdev file close for event ioctls
  * @inode:	Inode structure pointer for the char device
  * @filp:	File structure pointer for the char device
  *
@@ -1649,11 +1649,9 @@ static int iio_chrdev_release(struct inode *inode, struct file *filp)
 	return 0;
 }
 
-/* Somewhat of a cross file organization violation - ioctls here are actually
- * event related */
-static long iio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+long iio_device_event_ioctl(struct iio_dev *indio_dev, struct file *filp,
+			    unsigned int cmd, unsigned long arg)
 {
-	struct iio_dev *indio_dev = filp->private_data;
 	int __user *ip = (int __user *)arg;
 	int fd;
 
@@ -1671,16 +1669,15 @@ static long iio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	return -EINVAL;
 }
 
-static const struct file_operations iio_buffer_fileops = {
-	.read = iio_buffer_read_outer_addr,
-	.release = iio_chrdev_release,
-	.open = iio_chrdev_open,
-	.poll = iio_buffer_poll_addr,
-	.owner = THIS_MODULE,
-	.llseek = noop_llseek,
-	.unlocked_ioctl = iio_ioctl,
-	.compat_ioctl = compat_ptr_ioctl,
-};
+/* Somewhat of a cross file organization violation - ioctls here are actually
+ * event related */
+static long iio_event_ioctl_wrapper(struct file *filp, unsigned int cmd,
+				    unsigned long arg)
+{
+	struct iio_dev *indio_dev = filp->private_data;
+
+	return iio_device_event_ioctl(indio_dev, filp, cmd, arg);
+}
 
 static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
 {
@@ -1712,7 +1709,7 @@ static const struct file_operations iio_event_fileops = {
 	.open = iio_chrdev_open,
 	.owner = THIS_MODULE,
 	.llseek = noop_llseek,
-	.unlocked_ioctl = iio_ioctl,
+	.unlocked_ioctl = iio_event_ioctl_wrapper,
 	.compat_ioctl = compat_ptr_ioctl,
 };
 
@@ -1766,16 +1763,21 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 		indio_dev->setup_ops == NULL)
 		indio_dev->setup_ops = &noop_ring_setup_ops;
 
-	if (indio_dev->buffer)
-		cdev_init(&indio_dev->chrdev, &iio_buffer_fileops);
-	else
+	ret = iio_device_buffers_init(indio_dev, this_mod);
+	if (ret) {
+		if (ret != -ENOTSUPP)
+			goto error_unreg_eventset;
+
 		cdev_init(&indio_dev->chrdev, &iio_event_fileops);
 
-	indio_dev->chrdev.owner = this_mod;
+		indio_dev->chrdev.owner = this_mod;
 
-	ret = cdev_device_add(&indio_dev->chrdev, &indio_dev->dev);
-	if (ret < 0)
-		goto error_unreg_eventset;
+		ret = cdev_device_add(&indio_dev->chrdev, &indio_dev->dev);
+		if (ret < 0)
+			goto error_unreg_eventset;
+
+		indio_dev->chrdev_initialized = true;
+	}
 
 	return 0;
 
@@ -1797,7 +1799,12 @@ EXPORT_SYMBOL(__iio_device_register);
  **/
 void iio_device_unregister(struct iio_dev *indio_dev)
 {
-	cdev_device_del(&indio_dev->chrdev, &indio_dev->dev);
+	if (indio_dev->chrdev_initialized)
+		cdev_device_del(&indio_dev->chrdev, &indio_dev->dev);
+
+	indio_dev->chrdev_initialized = false;
+
+	iio_device_buffers_uninit(indio_dev);
 
 	mutex_lock(&indio_dev->info_exist_lock);
 
diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
index 8fb92250a190..b4a55c3f556b 100644
--- a/include/linux/iio/buffer_impl.h
+++ b/include/linux/iio/buffer_impl.h
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef _IIO_BUFFER_GENERIC_IMPL_H_
 #define _IIO_BUFFER_GENERIC_IMPL_H_
+#include <linux/cdev.h>
 #include <linux/sysfs.h>
 #include <linux/kref.h>
 
@@ -72,6 +73,12 @@ struct iio_buffer {
 	/** @indio_dev: IIO device to which this buffer belongs to. */
 	struct iio_dev *indio_dev;
 
+	/** @chrdev: associated character device. */
+	struct cdev chrdev;
+
+	/** @file_ops_flags: file ops related flags including busy flag. */
+	unsigned long file_ops_flags;
+
 	/** @length: Number of datums in buffer. */
 	unsigned int length;
 
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index e975020abaa6..e93497f483f7 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -517,6 +517,7 @@ struct iio_buffer_setup_ops {
  * @setup_ops:		[DRIVER] callbacks to call before and after buffer
  *			enable/disable
  * @chrdev:		[INTERN] associated character device
+ * @chrdev_initialized:	[INTERN] true if @chrdev device has been initialized
  * @groups:		[INTERN] attribute groups
  * @groupcounter:	[INTERN] index of next attribute group
  * @flags:		[INTERN] file ops related flags including busy flag.
@@ -560,6 +561,7 @@ struct iio_dev {
 	struct mutex			info_exist_lock;
 	const struct iio_buffer_setup_ops	*setup_ops;
 	struct cdev			chrdev;
+	int				chrdev_initialized;
 #define IIO_MAX_GROUPS 6
 	const struct attribute_group	*groups[IIO_MAX_GROUPS + 1];
 	int				groupcounter;
-- 
2.17.1

[PATCH v2 4/5] iio: buffer: move sysfs alloc/free in industrialio-buffer.c

[Alexandru Ardelean]

Since we now have the 'iio_device_buffers_{un}init()' entry-points into the
industrialio-buffer.c we can now move the sysfs alloc & free in there as
part of that init/uninit.

This changes the order of iio_device_register()/iio_device_unregister() a
bit, but overall this shouldn't matter.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/iio_core.h            | 10 ----------
 drivers/iio/industrialio-buffer.c | 29 +++++++++++++++--------------
 drivers/iio/industrialio-core.c   | 14 +-------------
 3 files changed, 16 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index 4bdadeac2710..1d69071e1426 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -48,9 +48,6 @@ void iio_device_buffers_uninit(struct iio_dev *indio_dev);
 
 void iio_device_buffers_put(struct iio_dev *indio_dev);
 
-int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev);
-void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev);
-
 void iio_disable_all_buffers(struct iio_dev *indio_dev);
 void iio_buffer_wakeup_poll(struct iio_dev *indio_dev);
 
@@ -66,13 +63,6 @@ static inline void iio_device_buffers_uninit(struct iio_dev *indio_dev) {}
 
 static inline void iio_device_buffers_put(struct iio_dev *indio_dev) {}
 
-static inline int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
-{
-	return 0;
-}
-
-static inline void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev) {}
-
 static inline void iio_disable_all_buffers(struct iio_dev *indio_dev) {}
 static inline void iio_buffer_wakeup_poll(struct iio_dev *indio_dev) {}
 
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 4b5c3baadaab..8ab089a9c3bc 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -1284,11 +1284,11 @@ static struct attribute *iio_buffer_attrs[] = {
 	&dev_attr_data_available.attr,
 };
 
-int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
+static int iio_buffer_alloc_sysfs_and_mask(struct iio_buffer *buffer)
 {
+	struct iio_dev *indio_dev = buffer->indio_dev;
 	struct iio_dev_attr *p;
 	struct attribute **attr;
-	struct iio_buffer *buffer = indio_dev->buffer;
 	int ret, i, attrn, attrcount, attrcount_orig = 0;
 	const struct iio_chan_spec *channels;
 
@@ -1301,9 +1301,6 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
 		indio_dev->masklength = ml;
 	}
 
-	if (!buffer)
-		return 0;
-
 	attrcount = 0;
 	if (buffer->attrs) {
 		while (buffer->attrs[attrcount] != NULL)
@@ -1395,15 +1392,12 @@ int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
 	return ret;
 }
 
-void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev)
+static void iio_buffer_free_sysfs_and_mask(struct iio_buffer *buffer)
 {
-	if (!indio_dev->buffer)
-		return;
-
-	bitmap_free(indio_dev->buffer->scan_mask);
-	kfree(indio_dev->buffer->buffer_group.attrs);
-	kfree(indio_dev->buffer->scan_el_group.attrs);
-	iio_free_chan_devattr_list(&indio_dev->buffer->scan_el_dev_attr_list);
+	bitmap_free(buffer->scan_mask);
+	kfree(buffer->buffer_group.attrs);
+	kfree(buffer->scan_el_group.attrs);
+	iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
 }
 
 static const struct file_operations iio_buffer_fileops = {
@@ -1425,13 +1419,19 @@ int iio_device_buffers_init(struct iio_dev *indio_dev, struct module *this_mod)
 	if (!buffer)
 		return -ENOTSUPP;
 
+	ret = iio_buffer_alloc_sysfs_and_mask(buffer);
+	if (ret)
+		return ret;
+
 	cdev_init(&buffer->chrdev, &iio_buffer_fileops);
 
 	buffer->chrdev.owner = this_mod;
 
 	ret = cdev_device_add(&buffer->chrdev, &indio_dev->dev);
-	if (ret)
+	if (ret) {
+		iio_buffer_free_sysfs_and_mask(buffer);
 		return ret;
+	}
 
 	iio_device_get(indio_dev);
 	iio_buffer_get(buffer);
@@ -1457,6 +1457,7 @@ void iio_device_buffers_uninit(struct iio_dev *indio_dev)
 		return;
 
 	cdev_device_del(&buffer->chrdev, &indio_dev->dev);
+	iio_buffer_free_sysfs_and_mask(buffer);
 	iio_buffer_put(buffer);
 	iio_device_put(indio_dev);
 }
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 2158aeab0bd2..794aaa4be832 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1737,18 +1737,11 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 
 	iio_device_register_debugfs(indio_dev);
 
-	ret = iio_buffer_alloc_sysfs_and_mask(indio_dev);
-	if (ret) {
-		dev_err(indio_dev->dev.parent,
-			"Failed to create buffer sysfs interfaces\n");
-		goto error_unreg_debugfs;
-	}
-
 	ret = iio_device_register_sysfs(indio_dev);
 	if (ret) {
 		dev_err(indio_dev->dev.parent,
 			"Failed to register sysfs interfaces\n");
-		goto error_buffer_free_sysfs;
+		goto error_free_sysfs;
 	}
 	ret = iio_device_register_eventset(indio_dev);
 	if (ret) {
@@ -1785,9 +1778,6 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 	iio_device_unregister_eventset(indio_dev);
 error_free_sysfs:
 	iio_device_unregister_sysfs(indio_dev);
-error_buffer_free_sysfs:
-	iio_buffer_free_sysfs_and_mask(indio_dev);
-error_unreg_debugfs:
 	iio_device_unregister_debugfs(indio_dev);
 	return ret;
 }
@@ -1818,8 +1808,6 @@ void iio_device_unregister(struct iio_dev *indio_dev)
 	iio_buffer_wakeup_poll(indio_dev);
 
 	mutex_unlock(&indio_dev->info_exist_lock);
-
-	iio_buffer_free_sysfs_and_mask(indio_dev);
 }
 EXPORT_SYMBOL(iio_device_unregister);
 
-- 
2.17.1

[PATCH v2 5/5] iio: event: move event-only chardev in industrialio-event.c

[Alexandru Ardelean]

At this point, we should be able to move the IIO device's chardev from the
iio_dev struct to the event_interface struct.

The IIO buffer chardev will also take-care of event ioctl() (if it is
initialized. The chardev for the event_interface (which has been moved to
industrialio-event.c) will be initialized only if iio_device_buffers_init()
returns -ENOTSUPP.

This does require an extra pair of iio_device_register_event_chrdev() &
iio_device_unregister_event_chrdev() functions. The
iio_device_register_event_chrdev() should (and will) be  only called if
iio_device_buffers_init() returns -ENOTSUPP.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/iio_core.h           |   5 +-
 drivers/iio/industrialio-core.c  |  94 +----------------------
 drivers/iio/industrialio-event.c | 124 ++++++++++++++++++++++++++++++-
 include/linux/iio/iio.h          |   6 --
 4 files changed, 130 insertions(+), 99 deletions(-)

diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index 1d69071e1426..2249e5b28023 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -68,10 +68,13 @@ static inline void iio_buffer_wakeup_poll(struct iio_dev *indio_dev) {}
 
 #endif
 
+int iio_device_register_event_chrdev(struct iio_dev *indio_dev,
+				     struct module *this_mod);
+void iio_device_unregister_event_chrdev(struct iio_dev *indio_dev);
+
 int iio_device_register_eventset(struct iio_dev *indio_dev);
 void iio_device_unregister_eventset(struct iio_dev *indio_dev);
 void iio_device_wakeup_eventset(struct iio_dev *indio_dev);
-int iio_event_getfd(struct iio_dev *indio_dev);
 
 struct iio_event_interface;
 bool iio_event_enabled(const struct iio_event_interface *ev_int);
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 794aaa4be832..7d3258f4528b 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1609,76 +1609,6 @@ void devm_iio_device_free(struct device *dev, struct iio_dev *iio_dev)
 }
 EXPORT_SYMBOL_GPL(devm_iio_device_free);
 
-/**
- * iio_chrdev_open() - chrdev file open for event ioctls
- * @inode:	Inode structure for identifying the device in the file system
- * @filp:	File structure for iio device used to keep and later access
- *		private data
- *
- * Return: 0 on success or -EBUSY if the device is already opened
- **/
-static int iio_chrdev_open(struct inode *inode, struct file *filp)
-{
-	struct iio_dev *indio_dev = container_of(inode->i_cdev,
-						struct iio_dev, chrdev);
-
-	if (test_and_set_bit(IIO_BUSY_BIT_POS, &indio_dev->flags))
-		return -EBUSY;
-
-	iio_device_get(indio_dev);
-
-	filp->private_data = indio_dev;
-
-	return 0;
-}
-
-/**
- * iio_chrdev_release() - chrdev file close for event ioctls
- * @inode:	Inode structure pointer for the char device
- * @filp:	File structure pointer for the char device
- *
- * Return: 0 for successful release
- */
-static int iio_chrdev_release(struct inode *inode, struct file *filp)
-{
-	struct iio_dev *indio_dev = container_of(inode->i_cdev,
-						struct iio_dev, chrdev);
-	clear_bit(IIO_BUSY_BIT_POS, &indio_dev->flags);
-	iio_device_put(indio_dev);
-
-	return 0;
-}
-
-long iio_device_event_ioctl(struct iio_dev *indio_dev, struct file *filp,
-			    unsigned int cmd, unsigned long arg)
-{
-	int __user *ip = (int __user *)arg;
-	int fd;
-
-	if (!indio_dev->info)
-		return -ENODEV;
-
-	if (cmd == IIO_GET_EVENT_FD_IOCTL) {
-		fd = iio_event_getfd(indio_dev);
-		if (fd < 0)
-			return fd;
-		if (copy_to_user(ip, &fd, sizeof(fd)))
-			return -EFAULT;
-		return 0;
-	}
-	return -EINVAL;
-}
-
-/* Somewhat of a cross file organization violation - ioctls here are actually
- * event related */
-static long iio_event_ioctl_wrapper(struct file *filp, unsigned int cmd,
-				    unsigned long arg)
-{
-	struct iio_dev *indio_dev = filp->private_data;
-
-	return iio_device_event_ioctl(indio_dev, filp, cmd, arg);
-}
-
 static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
 {
 	int i, j;
@@ -1704,15 +1634,6 @@ static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops noop_ring_setup_ops;
 
-static const struct file_operations iio_event_fileops = {
-	.release = iio_chrdev_release,
-	.open = iio_chrdev_open,
-	.owner = THIS_MODULE,
-	.llseek = noop_llseek,
-	.unlocked_ioctl = iio_event_ioctl_wrapper,
-	.compat_ioctl = compat_ptr_ioctl,
-};
-
 int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 {
 	int ret;
@@ -1761,15 +1682,9 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
 		if (ret != -ENOTSUPP)
 			goto error_unreg_eventset;
 
-		cdev_init(&indio_dev->chrdev, &iio_event_fileops);
-
-		indio_dev->chrdev.owner = this_mod;
-
-		ret = cdev_device_add(&indio_dev->chrdev, &indio_dev->dev);
-		if (ret < 0)
+		ret = iio_device_register_event_chrdev(indio_dev, this_mod);
+		if (ret)
 			goto error_unreg_eventset;
-
-		indio_dev->chrdev_initialized = true;
 	}
 
 	return 0;
@@ -1789,10 +1704,7 @@ EXPORT_SYMBOL(__iio_device_register);
  **/
 void iio_device_unregister(struct iio_dev *indio_dev)
 {
-	if (indio_dev->chrdev_initialized)
-		cdev_device_del(&indio_dev->chrdev, &indio_dev->dev);
-
-	indio_dev->chrdev_initialized = false;
+	iio_device_unregister_event_chrdev(indio_dev);
 
 	iio_device_buffers_uninit(indio_dev);
 
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index 5b17c92d3b50..1e74ccfaa512 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -30,6 +30,9 @@
  * @flags:		file operations related flags including busy flag.
  * @group:		event interface sysfs attribute group
  * @read_lock:		lock to protect kfifo read operations
+ * @chrdev:		associated chardev for this event
+ * @chrdev_initialized:	true if this @chrdev was initialized
+ * @indio_dev:		IIO device to which this event interface belongs to
  */
 struct iio_event_interface {
 	wait_queue_head_t	wait;
@@ -39,6 +42,10 @@ struct iio_event_interface {
 	unsigned long		flags;
 	struct attribute_group	group;
 	struct mutex		read_lock;
+
+	struct cdev		chrdev;
+	bool			chrdev_initialized;
+	struct iio_dev		*indio_dev;
 };
 
 bool iio_event_enabled(const struct iio_event_interface *ev_int)
@@ -182,7 +189,7 @@ static const struct file_operations iio_event_chrdev_fileops = {
 	.llseek = noop_llseek,
 };
 
-int iio_event_getfd(struct iio_dev *indio_dev)
+static int iio_event_getfd(struct iio_dev *indio_dev)
 {
 	struct iio_event_interface *ev_int = indio_dev->event_interface;
 	int fd;
@@ -215,6 +222,121 @@ int iio_event_getfd(struct iio_dev *indio_dev)
 	return fd;
 }
 
+/**
+ * iio_chrdev_open() - chrdev file open for event ioctls
+ * @inode:	Inode structure for identifying the device in the file system
+ * @filp:	File structure for iio device used to keep and later access
+ *		private data
+ *
+ * Return: 0 on success or -EBUSY if the device is already opened
+ **/
+static int iio_chrdev_open(struct inode *inode, struct file *filp)
+{
+	struct iio_event_interface *ev =
+		container_of(inode->i_cdev, struct iio_event_interface, chrdev);
+
+	if (test_and_set_bit(IIO_BUSY_BIT_POS, &ev->flags))
+		return -EBUSY;
+
+	iio_device_get(ev->indio_dev);
+
+	filp->private_data = ev;
+
+	return 0;
+}
+
+/**
+ * iio_chrdev_release() - chrdev file close for event ioctls
+ * @inode:	Inode structure pointer for the char device
+ * @filp:	File structure pointer for the char device
+ *
+ * Return: 0 for successful release
+ */
+static int iio_chrdev_release(struct inode *inode, struct file *filp)
+{
+	struct iio_event_interface *ev =
+		container_of(inode->i_cdev, struct iio_event_interface, chrdev);
+
+	clear_bit(IIO_BUSY_BIT_POS, &ev->flags);
+	iio_device_put(ev->indio_dev);
+
+	return 0;
+}
+
+long iio_device_event_ioctl(struct iio_dev *indio_dev, struct file *filp,
+			    unsigned int cmd, unsigned long arg)
+{
+	int __user *ip = (int __user *)arg;
+	int fd;
+
+	if (!indio_dev->info)
+		return -ENODEV;
+
+	if (cmd == IIO_GET_EVENT_FD_IOCTL) {
+		fd = iio_event_getfd(indio_dev);
+		if (fd < 0)
+			return fd;
+		if (copy_to_user(ip, &fd, sizeof(fd)))
+			return -EFAULT;
+		return 0;
+	}
+	return -EINVAL;
+}
+
+static long iio_event_ioctl_wrapper(struct file *filp, unsigned int cmd,
+				    unsigned long arg)
+{
+	struct iio_event_interface *ev = filp->private_data;
+
+	return iio_device_event_ioctl(ev->indio_dev, filp, cmd, arg);
+}
+
+static const struct file_operations iio_device_event_fileops = {
+	.release = iio_chrdev_release,
+	.open = iio_chrdev_open,
+	.owner = THIS_MODULE,
+	.llseek = noop_llseek,
+	.unlocked_ioctl = iio_event_ioctl_wrapper,
+	.compat_ioctl = compat_ptr_ioctl,
+};
+
+int iio_device_register_event_chrdev(struct iio_dev *indio_dev,
+				     struct module *this_mod)
+{
+	struct iio_event_interface *ev = indio_dev->event_interface;
+	int ret;
+
+	if (!ev)
+		return 0;
+
+	cdev_init(&ev->chrdev, &iio_device_event_fileops);
+
+	ev->chrdev.owner = this_mod;
+
+	ret = cdev_device_add(&ev->chrdev, &indio_dev->dev);
+	if (ret < 0)
+		return ret;
+
+	ev->chrdev_initialized = true;
+
+	return 0;
+}
+
+void iio_device_unregister_event_chrdev(struct iio_dev *indio_dev)
+{
+	struct iio_event_interface *ev = indio_dev->event_interface;
+
+	if (!ev)
+		return;
+
+	ev = indio_dev->event_interface;
+
+	if (ev->chrdev_initialized)
+		cdev_device_del(&ev->chrdev, &indio_dev->dev);
+
+	ev->chrdev_initialized = false;
+}
+
 static const char * const iio_ev_type_text[] = {
 	[IIO_EV_TYPE_THRESH] = "thresh",
 	[IIO_EV_TYPE_MAG] = "mag",
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index e93497f483f7..8fa22e4e246f 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -516,11 +516,8 @@ struct iio_buffer_setup_ops {
  * @info_exist_lock:	[INTERN] lock to prevent use during removal
  * @setup_ops:		[DRIVER] callbacks to call before and after buffer
  *			enable/disable
- * @chrdev:		[INTERN] associated character device
- * @chrdev_initialized:	[INTERN] true if @chrdev device has been initialized
  * @groups:		[INTERN] attribute groups
  * @groupcounter:	[INTERN] index of next attribute group
- * @flags:		[INTERN] file ops related flags including busy flag.
  * @debugfs_dentry:	[INTERN] device specific debugfs dentry.
  * @cached_reg_addr:	[INTERN] cached register address for debugfs reads.
  */
@@ -560,13 +557,10 @@ struct iio_dev {
 	clockid_t			clock_id;
 	struct mutex			info_exist_lock;
 	const struct iio_buffer_setup_ops	*setup_ops;
-	struct cdev			chrdev;
-	int				chrdev_initialized;
 #define IIO_MAX_GROUPS 6
 	const struct attribute_group	*groups[IIO_MAX_GROUPS + 1];
 	int				groupcounter;
 
-	unsigned long			flags;
 #if defined(CONFIG_DEBUG_FS)
 	struct dentry			*debugfs_dentry;
 	unsigned			cached_reg_addr;
-- 
2.17.1