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

From: Jonathan Cameron
Date: Mon Apr 13 2020 - 11:47:34 EST


On Fri, 10 Apr 2020 17:17:25 +0300
Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote:

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

Whilst we are here, can we avoid allocating the chardev at all if
we have neither buffer support, nor events? So don't add the chrdev to the device.

That covers quite a wide range of slow devices and is a nice incidental
improvement (to be honest I'd forgotten we actually created a chardev
in those circumstance :(

Jonathan

> ---
> 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;
>