All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandru Ardelean <alexandru.ardelean@analog.com>
To: <linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: <jic23@kernel.org>, <lars@metafoo.de>,
	Alexandru Ardelean <alexandru.ardelean@analog.com>
Subject: [PATCH v3 3/5] iio: buffer: move iio buffer chrdev in industrialio-buffer.c
Date: Fri, 10 Apr 2020 17:17:27 +0300	[thread overview]
Message-ID: <20200410141729.82834-4-alexandru.ardelean@analog.com> (raw)
In-Reply-To: <20200410141729.82834-1-alexandru.ardelean@analog.com>

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


  parent reply	other threads:[~2020-04-10 14:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-10 14:17 [PATCH v3 0/5] iio: core,buffer: re-organize chardev creation Alexandru Ardelean
2020-04-10 14:17 ` [PATCH v3 1/5] iio: core: register buffer fileops only if buffer present Alexandru Ardelean
2020-04-13 15:47   ` Jonathan Cameron
2020-04-14  5:20     ` Ardelean, Alexandru
2020-04-14  6:07       ` Ardelean, Alexandru
2020-04-10 14:17 ` [PATCH v3 2/5] iio: buffer: add back-ref from iio_buffer to iio_dev Alexandru Ardelean
2020-04-13 15:48   ` Jonathan Cameron
2020-04-10 14:17 ` Alexandru Ardelean [this message]
2020-04-13 15:58   ` [PATCH v3 3/5] iio: buffer: move iio buffer chrdev in industrialio-buffer.c Jonathan Cameron
2020-04-14  5:43     ` Ardelean, Alexandru
2020-04-14 17:50       ` Jonathan Cameron
2020-04-10 14:17 ` [PATCH v3 4/5] iio: buffer: move sysfs alloc/free " Alexandru Ardelean
2020-04-13 16:00   ` Jonathan Cameron
2020-04-10 14:17 ` [PATCH v3 5/5] iio: event: move event-only chardev in industrialio-event.c Alexandru Ardelean
2020-04-12 14:29 ` [PATCH v3 0/5] iio: core,buffer: re-organize chardev creation Ardelean, Alexandru

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200410141729.82834-4-alexandru.ardelean@analog.com \
    --to=alexandru.ardelean@analog.com \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.