mbox series

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

Message ID 20200410141729.82834-1-alexandru.ardelean@analog.com (mailing list archive)
Headers show
Series iio: core,buffer: re-organize chardev creation | expand

Message

Alexandru Ardelean April 10, 2020, 2:17 p.m. UTC
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 v2 -> v3:
* removed double init in
  'iio: event: move event-only chardev in industrialio-event.c'

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  | 122 ++++++++++++++++++++++++-
 include/linux/iio/buffer_impl.h   |  10 +++
 include/linux/iio/iio.h           |   4 -
 6 files changed, 285 insertions(+), 132 deletions(-)

Comments

Alexandru Ardelean April 12, 2020, 2:29 p.m. UTC | #1
On Fri, 2020-04-10 at 17:17 +0300, Alexandru Ardelean wrote:
> 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 needs a new V4 now.
The drop of the devm_ APIs is causing some conflicts for this series.

Will send one tomorrow.

Still, any feedback until then [about this] is appreciated.

Thanks
Alex

> 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 v2 -> v3:
> * removed double init in
>   'iio: event: move event-only chardev in industrialio-event.c'
> 
> 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  | 122 ++++++++++++++++++++++++-
>  include/linux/iio/buffer_impl.h   |  10 +++
>  include/linux/iio/iio.h           |   4 -
>  6 files changed, 285 insertions(+), 132 deletions(-)
>