From: Alexandru Ardelean <alexandru.ardelean@analog.com>
To: <linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: <lars@metafoo.de>, <jic23@kernel.org>, <pmeerw@pmeerw.net>,
Alexandru Ardelean <alexandru.ardelean@analog.com>
Subject: [PATCH v6 4/6] iio: buffer,event: duplicate chardev creation for buffers & events
Date: Mon, 27 Apr 2020 16:10:58 +0300 [thread overview]
Message-ID: <20200427131100.50845-5-alexandru.ardelean@analog.com> (raw)
In-Reply-To: <20200427131100.50845-1-alexandru.ardelean@analog.com>
This patch moves the chardev creation into industrialio-buffer.c &
industrialio-event.c. The move has to be in a single step (which makes the
patch a bit big), in order to pass the reference of the chardev to
'indio_dev->chrdev'.
For structure purposes, industrialio-core.c should be the place where
cdev_device_add()/device_add() gets called, and the deletion function as
well.
What happens after this patch is:
- 'indio_dev->chrdev' is converted to a pointer
- if there is an IIO buffer, iio_device_buffer_attach_chrdev() will attach
it's chardev to 'indio_chrdev'
- if it doesn't, the event interface will attach a reference to it's
chardev (if it is instantiated) via iio_device_event_attach_chrdev()
That way, the control of the 'legacy' chardev is still visible in
'industrialio-core.c'. So, each logic file (for buffers & events) shouldn't
hide things too much away from the core file.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/iio_core.h | 16 ++---
drivers/iio/industrialio-buffer.c | 90 ++++++++++++++++++++++++--
drivers/iio/industrialio-core.c | 103 ++++--------------------------
drivers/iio/industrialio-event.c | 103 +++++++++++++++++++++++++++++-
include/linux/iio/buffer_impl.h | 7 ++
include/linux/iio/iio.h | 6 +-
6 files changed, 211 insertions(+), 114 deletions(-)
diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index 39ec0344fb68..a527a66be9e5 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -40,17 +40,14 @@ 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);
+
+void iio_device_buffer_attach_chrdev(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_device_buffers_put(struct iio_dev *indio_dev);
void iio_disable_all_buffers(struct iio_dev *indio_dev);
@@ -58,8 +55,7 @@ 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 void iio_device_buffer_attach_chrdev(struct iio_dev *indio_dev) {}
static inline int iio_buffer_alloc_sysfs_and_mask(struct iio_dev *indio_dev)
{
@@ -77,8 +73,8 @@ static inline void iio_buffer_wakeup_poll(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_event_attach_chrdev(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-buffer.c b/drivers/iio/industrialio-buffer.c
index a66d3fbc2905..0689c100b041 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
@@ -1129,6 +1171,17 @@ void iio_disable_all_buffers(struct iio_dev *indio_dev)
iio_buffer_deactivate_all(indio_dev);
}
+static long iio_buffer_ioctl(struct file *filep, unsigned int cmd,
+ unsigned long arg)
+{
+ struct iio_buffer *buffer = filep->private_data;
+
+ if (!buffer || !buffer->access)
+ return -ENODEV;
+
+ return iio_device_event_ioctl(buffer->indio_dev, filep, cmd, arg);
+}
+
static ssize_t iio_buffer_store_enable(struct device *dev,
struct device_attribute *attr,
const char *buf,
@@ -1355,6 +1408,29 @@ void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev)
iio_free_chan_devattr_list(&buffer->scan_el_dev_attr_list);
}
+static const struct file_operations iio_buffer_fileops = {
+ .owner = THIS_MODULE,
+ .llseek = noop_llseek,
+ .read = iio_buffer_read_outer,
+ .poll = iio_buffer_poll,
+ .unlocked_ioctl = iio_buffer_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
+ .open = iio_buffer_chrdev_open,
+ .release = iio_buffer_chrdev_release,
+};
+
+void iio_device_buffer_attach_chrdev(struct iio_dev *indio_dev)
+{
+ struct iio_buffer *buffer = indio_dev->buffer;
+
+ if (!buffer)
+ return;
+
+ cdev_init(&buffer->chrdev, &iio_buffer_fileops);
+
+ indio_dev->chrdev = &buffer->chrdev;
+}
+
void iio_device_buffers_put(struct iio_dev *indio_dev)
{
struct iio_buffer *buffer = indio_dev->buffer;
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 51e279c60793..e9c37bcd733d 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1584,79 +1584,6 @@ struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
}
EXPORT_SYMBOL_GPL(devm_iio_device_alloc);
-/**
- * iio_chrdev_open() - chrdev file open for buffer access and 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 buffer access and 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;
-}
-
-/* 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)
-{
- struct iio_dev *indio_dev = filp->private_data;
- 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 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,
-};
-
static int iio_check_unique_scan_index(struct iio_dev *indio_dev)
{
int i, j;
@@ -1682,15 +1609,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 = {
- .owner = THIS_MODULE,
- .llseek = noop_llseek,
- .unlocked_ioctl = iio_event_ioctl_wrapper,
- .compat_ioctl = compat_ptr_ioctl,
- .open = iio_chrdev_open,
- .release = iio_chrdev_release,
-};
-
int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
{
int ret;
@@ -1741,16 +1659,17 @@ 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 if (indio_dev->event_interface)
- cdev_init(&indio_dev->chrdev, &iio_event_fileops);
+ iio_device_buffer_attach_chrdev(indio_dev);
- indio_dev->chrdev.owner = this_mod;
+ /* no chrdev attached from buffer, we go with event-only chrdev */
+ if (!indio_dev->chrdev)
+ iio_device_event_attach_chrdev(indio_dev);
- if (indio_dev->buffer || indio_dev->event_interface)
- ret = cdev_device_add(&indio_dev->chrdev, &indio_dev->dev);
- else
+ if (indio_dev->chrdev) {
+ indio_dev->chrdev->owner = this_mod;
+
+ ret = cdev_device_add(indio_dev->chrdev, &indio_dev->dev);
+ } else
ret = device_add(&indio_dev->dev);
if (ret < 0)
@@ -1776,8 +1695,8 @@ EXPORT_SYMBOL(__iio_device_register);
**/
void iio_device_unregister(struct iio_dev *indio_dev)
{
- if (indio_dev->buffer || indio_dev->event_interface)
- cdev_device_del(&indio_dev->chrdev, &indio_dev->dev);
+ if (indio_dev->chrdev)
+ cdev_device_del(indio_dev->chrdev, &indio_dev->dev);
else
device_del(&indio_dev->dev);
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index 5b17c92d3b50..d532a689c2eb 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -30,6 +30,8 @@
* @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
+ * @indio_dev: IIO device to which this event interface belongs to
*/
struct iio_event_interface {
wait_queue_head_t wait;
@@ -39,6 +41,9 @@ struct iio_event_interface {
unsigned long flags;
struct attribute_group group;
struct mutex read_lock;
+
+ struct cdev chrdev;
+ struct iio_dev *indio_dev;
};
bool iio_event_enabled(const struct iio_event_interface *ev_int)
@@ -182,7 +187,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 +220,102 @@ 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;
+
+ /**
+ * The NULL check here is required to prevent crashing when a device
+ * is being removed while userspace would still have open file handles
+ * to try to access this device.
+ */
+ 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_event_fileops = {
+ .owner = THIS_MODULE,
+ .llseek = noop_llseek,
+ .unlocked_ioctl = iio_event_ioctl_wrapper,
+ .compat_ioctl = compat_ptr_ioctl,
+ .open = iio_chrdev_open,
+ .release = iio_chrdev_release,
+};
+
+void iio_device_event_attach_chrdev(struct iio_dev *indio_dev)
+{
+ struct iio_event_interface *ev = indio_dev->event_interface;
+
+ if (!ev)
+ return;
+
+ cdev_init(&ev->chrdev, &iio_event_fileops);
+
+ ev->indio_dev = indio_dev;
+ indio_dev->chrdev = &ev->chrdev;
+}
+
static const char * const iio_ev_type_text[] = {
[IIO_EV_TYPE_THRESH] = "thresh",
[IIO_EV_TYPE_MAG] = "mag",
diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h
index 67d73d465e02..46fc977deae3 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 5f9f439a4f01..52992be44e9e 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -516,10 +516,9 @@ 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: [INTERN] reference to associated character device
* @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.
*/
@@ -559,12 +558,11 @@ struct iio_dev {
clockid_t clock_id;
struct mutex info_exist_lock;
const struct iio_buffer_setup_ops *setup_ops;
- struct cdev chrdev;
+ struct cdev *chrdev;
#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
next prev parent reply other threads:[~2020-04-27 13:11 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-27 13:10 [PATCH v6 0/6] iio: core,buffer: re-organize chardev creation Alexandru Ardelean
2020-04-27 13:10 ` [PATCH v6 1/6] iio: buffer: add back-ref from iio_buffer to iio_dev Alexandru Ardelean
2020-04-27 13:10 ` [PATCH v6 2/6] iio: core,buffer: wrap iio_buffer_put() call into iio_buffers_put() Alexandru Ardelean
2020-04-27 13:10 ` [PATCH v6 3/6] iio: core: register chardev only if needed Alexandru Ardelean
2020-05-03 15:39 ` Jonathan Cameron
2020-05-04 6:46 ` Ardelean, Alexandru
2020-04-27 13:10 ` Alexandru Ardelean [this message]
2020-04-27 13:10 ` [PATCH v6 5/6] iio: core: add simple centralized mechanism for ioctl() handlers Alexandru Ardelean
2020-04-27 13:11 ` [PATCH v6 6/6] iio: core: use new common ioctl() mechanism Alexandru Ardelean
2020-05-03 15:51 ` [PATCH v6 0/6] iio: core,buffer: re-organize chardev creation Jonathan Cameron
2020-05-06 12:52 ` 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=20200427131100.50845-5-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 \
--cc=pmeerw@pmeerw.net \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).