Re: [RFC v2 01/11] iio: introduce iio backend device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
---
  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á





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux