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