[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <095f9c64-bcac-e838-ba69-b5df623c444f@foss.st.com>
Date: Thu, 31 Aug 2023 18:14:12 +0200
From: Olivier MOYSAN <olivier.moysan@...s.st.com>
To: Nuno Sá <noname.nuno@...il.com>,
Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>
CC: <linux-kernel@...r.kernel.org>, <linux-iio@...r.kernel.org>,
"Fabrice GASNIER" <fabrice.gasnier@...com>
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@...s.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á
>
>
Powered by blists - more mailing lists