Messages in this thread | | | Date | Thu, 31 Aug 2023 18:14:12 +0200 | From | Olivier MOYSAN <> | Subject | Re: [RFC v2 01/11] iio: introduce iio backend device |
| |
Hi Nuno,
On 7/28/23 10:42, Nuno Sá wrote: > Hi Olivier, > > On Thu, 2023-07-27 at 17:03 +0200, Olivier Moysan wrote: >> Add a new device type in IIO framework. >> This backend device does not compute channel attributes and does not expose >> them through sysfs, as done typically in iio-rescale frontend device. >> Instead, it allows to report information applying to channel >> attributes through callbacks. These backend devices can be cascaded >> to represent chained components. >> An IIO device configured as a consumer of a backend device can compute >> the channel attributes of the whole chain. >> >> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com> >> --- >> drivers/iio/Makefile | 1 + >> drivers/iio/industrialio-backend.c | 107 +++++++++++++++++++++++++++++ >> include/linux/iio/backend.h | 56 +++++++++++++++ >> 3 files changed, 164 insertions(+) >> create mode 100644 drivers/iio/industrialio-backend.c >> create mode 100644 include/linux/iio/backend.h >> >> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile >> index 9622347a1c1b..9b59c6ab1738 100644 >> --- a/drivers/iio/Makefile >> +++ b/drivers/iio/Makefile >> @@ -5,6 +5,7 @@ >> >> obj-$(CONFIG_IIO) += industrialio.o >> industrialio-y := industrialio-core.o industrialio-event.o inkern.o >> +industrialio-$(CONFIG_IIO_BACKEND) += industrialio-backend.o >> industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o >> industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o >> >> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- >> backend.c >> new file mode 100644 >> index 000000000000..7d0625889873 >> --- /dev/null >> +++ b/drivers/iio/industrialio-backend.c >> @@ -0,0 +1,107 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* The industrial I/O core, backend handling functions >> + * >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/device.h> >> +#include <linux/property.h> >> +#include <linux/iio/iio.h> >> +#include <linux/iio/backend.h> >> + >> +static DEFINE_IDA(iio_backend_ida); >> + >> +#define to_iio_backend(_device) container_of((_device), struct iio_backend, >> dev) >> + >> +static void iio_backend_release(struct device *device) >> +{ >> + struct iio_backend *backend = to_iio_backend(device); >> + >> + kfree(backend->name); >> + kfree(backend); >> +} >> + >> +static const struct device_type iio_backend_type = { >> + .release = iio_backend_release, >> + .name = "iio_backend_device", >> +}; >> + >> +struct iio_backend *iio_backend_alloc(struct device *parent) >> +{ >> + struct iio_backend *backend; >> + >> + backend = devm_kzalloc(parent, sizeof(*backend), GFP_KERNEL); >> > > No error checking. > > I guess a lot of cleanings are still missing but the important thing I wanted to > notice is that the above pattern is not ok. > Your 'struct iio_backend *backend'' embeds a 'stuct device' which is a > refcounted object. Nevertheless, you're binding the lifetime of your object to > the parent device and that is wrong. The reason is that as soon as your parent > device get's released or just unbinded from it's driver, all the devres stuff > (including your 'struct iio_backend' object) will be released independentof > your 'struct device' refcount value... > > So, you might argue this won't ever be an issue in here but the pattern is still > wrong. There are some talks about this, the last one was given at the latest > EOSS: > > https://www.youtube.com/watch?v=HCiJL7djGw8&list=PLbzoR-pLrL6pY8a8zSKRC6-AihFrruOkq&index=27&ab_channel=TheLinuxFoundation >
This is a good point. Thanks for pointing it out. Sure, there are still many things to improve.
I have seen the comment from Jonathan on your "Add converter framework" serie. I had a quick look at the serie. It seems that we share the need to aggregate some IIO devices. But I need to read it more carefully to check if we can find some convergences here.
BRs Olivier
> - Nuno Sá > >
| |