Messages in this thread | | | Subject | Re: [PATCH v2] iio: core: register chardev only if needed | From | Lars-Peter Clausen <> | Date | Wed, 15 Apr 2020 16:55:40 +0200 |
| |
On 4/15/20 3:56 PM, Ardelean, Alexandru wrote: > On Tue, 2020-04-14 at 19:06 +0100, Jonathan Cameron wrote: >> [External] >> >> On Tue, 14 Apr 2020 11:36:56 +0300 >> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: >> >>> The final 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 only need a chardev if we need to support buffers and/or events. >>> >>> With this change, a chardev will be created: >>> 1. if there is an IIO buffer attached OR >>> 2. if there is an event_interface configured >>> >>> Otherwise, no chardev will be created. >>> Quite a lot of IIO devices don't really need a chardev, so this is a minor >>> improvement to the IIO core, as the IIO device will take up fewer >>> resources. >>> >>> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> >>> --- >>> >>> Changelog v1 -> v2: >>> * split away from series 'iio: core,buffer: re-organize chardev creation'; >>> i'm getting the feeling that this has some value on it's own; >>> no idea if it needs 'Fixes' tag; it is a bit fuzzy to point to a patch >>> which this would be fixed by this; i'm guessing it would be fine >>> without one >> I'd argue it's an 'optimization' rather than a fix :) >> >> Still looks good to me but I'd like it to sit for a little while to >> see if anyone points out something we are both missing! >> > This is not good. > It seems that I did not properly test all cases. > I had to break a device to not have an event_interface to notice that the sysfs > doesn't get instantiated either because device_add is missing. > > Will do another try.
I think you also have to make the `indio_dev->dev.devt = ...` conditional. Or conditionally use device_add() instead of device_add_cdev().
If you go for the former you need to call cdev_device_del() unconditionally, for the latter call device_del() or cdev_device_del() depending on whether the cdev was registered.
- Lars
| |