lkml.org 
[lkml]   [2020]   [May]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v6 0/6] iio: core,buffer: re-organize chardev creation
Date
On Sun, 2020-05-03 at 16:51 +0100, Jonathan Cameron wrote:
> [External]
>
> On Mon, 27 Apr 2020 16:10:54 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> 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.
> > This tries to re-organize the init of the chardev.
>
> Hmm. I'd like this set to sit and ideally gather a few acks before
> I move ahead with it.
>
> The protections against problems around remove have always been
> somewhat fiddly and I suspect don't cover everything.
>
> I'm fairly sure taking the exist lock is 'sufficient' but I'm not
> actually sure it's necessary. We only otherwise take it for
> place where the inkern interface is in use so we can race
> against removal of a provider driver.
>
> We don't have such heavy weight protections in the buffer code
> and I'll be honest I can't remember why. Original patch mentions
> that it was about avoiding taking additional new references to the
> struct iio_dev. We aren't doing that as such here so perhaps
> we don't need to take the lock..
>
> Lars, I suspect you may have been involved in that stuff originally
> so I'd appreciate you taking a quick look at this if you have
> time!
>

Well, it will take me a bit to re-do this series.
I've got to a point where some of these patches will need some re-tuning.
.
Adding multiple buffers per IIO device seems a bit more difficult than I
initially thought.

> Thanks,
>
> Jonathan
>
> >
> > Changelog v5 -> v6:
> > - patch 'iio: core: register chardev only if needed'
> > - sort file_operations fields for iio_event_fileops
> > - patch 'iio: buffer,event: duplicate chardev creation for buffers & events'
> > - fixed-up '**/' -> '*/' for 2 block comments
> > - sorted file_operations for iio_buffer_fileops, after move
> > - removed 'indio_dev->chrdev = NULL' on IIO device unregister
> > - added comment about 'indio_dev->info' NULL check in
> > iio_device_event_ioctl()
> > - patch 'iio: core: add simple centralized mechanism for ioctl() handlers'
> > - re-using lock 'indio_dev->info_exist_lock' for new ioctl register
> > mechanism in iio_device_ioctl()
> > - simplified exit condition from the loop; only need to check
> > `ret != IIO_IOCTL_UNHANDLED` to continue looping;
> > everything else is just return/break
> > - patch 'iio: core: use new common ioctl() mechanism'
> > - the comment for 'indio_dev->info' NULL check is being moved here to
> > highlight why the null-check is being removed; or where it's being
> > moved
> >
> > Changelog v4 -> v5:
> > - dropped patch 'iio: Use an early return in iio_device_alloc to simplify
> > code.'
> > is applied upstream
> >
> > Changelog v3 -> v4:
> > - added patch [1] 'iio: Use an early return in iio_device_alloc to simplify
> > code.'
> > it's main purpose is so that this patch applies:
> > [2]'iio: core: add simple centralized mechanism for ioctl() handlers'
> > depending on the final version of patch [1], patch [2] needs some
> > minor fixup
> > - added patch 'iio: core,buffer: wrap iio_buffer_put() call into
> > iio_buffers_put()'
> > - patch 'iio: core: register buffer fileops only if buffer present'
> > is now: 'iio: core: register chardev only if needed'
> > - dropped 'iio: buffer: move sysfs alloc/free in industrialio-buffer.c'
> > it's likely we won't be doing this patch anymore
> > - patches:
> > 'iio: buffer: move iio buffer chrdev in industrialio-buffer.c'
> > 'iio: event: move event-only chardev in industrialio-event.c'
> > have been merged into 'iio: buffer,event: duplicate chardev creation for
> > buffers & events'
> > since now, the logic is a bit different, and 'indio_dev->chrdev' is
> > now a reference to either the buffer's chrdev & or the events-only
> > chrdev
> > - added simple mechanism to register ioctl() handlers for IIO device
> > which is currently used only by events mechanism
> >
> > 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 (6):
> > iio: buffer: add back-ref from iio_buffer to iio_dev
> > iio: core,buffer: wrap iio_buffer_put() call into iio_buffers_put()
> > iio: core: register chardev only if needed
> > iio: buffer,event: duplicate chardev creation for buffers & events
> > iio: core: add simple centralized mechanism for ioctl() handlers
> > iio: core: use new common ioctl() mechanism
> >
> > drivers/iio/iio_core.h | 29 +++++---
> > drivers/iio/industrialio-buffer.c | 102 ++++++++++++++++++++++++--
> > drivers/iio/industrialio-core.c | 116 +++++++++++++-----------------
> > drivers/iio/industrialio-event.c | 100 +++++++++++++++++++++++++-
> > include/linux/iio/buffer_impl.h | 10 +++
> > include/linux/iio/iio.h | 8 +--
> > 6 files changed, 276 insertions(+), 89 deletions(-)
> >
\
 
 \ /
  Last update: 2020-05-06 14:53    [W:0.310 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site