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@xxxxxxxxxx> --- 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