Re: [PATCH 00/12] iio: add new backend framework

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

 



Hi Nuno,

On 11/21/23 11:20, Nuno Sa via B4 Relay wrote:
Hi all,

This is a Framework to handle complex IIO aggregate devices.

The typical architecture is to have one device as the frontend device which
can be "linked" against one or multiple backend devices. All the IIO and
userspace interface is expected to be registers/managed by the frontend
device which will callback into the backends when needed (to get/set
some configuration that it does not directly control).

The basic framework interface is pretty simple:
  - Backends should register themselves with @devm_iio_backend_register()
  - Frontend devices should get backends with @devm_iio_backend_get()

(typical provider - consumer stuff)

This is the result of the discussions in [1] and [2]. In short, both ADI
and STM wanted some way to control/get configurations from a kind of
IIO aggregate device. So discussions were made to have something that
serves and can be used by everyone.

The main differences with the converter framework RFC [1]:

1) Dropped the component framework. One can get more overview about
the concerns on the references but the main reasons were:
  * Relying on providing .remove() callbacks to be allowed to use device
    managed functions. I was not even totally sure about the correctness
    of it and in times where everyone tries to avoid that driver
    callback, it could lead to some maintenance burden.
  * Scalability issues. As mentioned in [2], to support backends defined
    in FW child nodes was not so straightforward with the component
    framework.
  * Device links can already do some of the things that made me
    try the component framework (eg: removing consumers on suppliers
    unbind).

2) Only support the minimal set of functionality to have the devices in
    the same state as before using the backend framework. New features
    will be added afterwards.

3) Moved the API docs into the .c files.

4) Moved the framework to the IIO top dir and renamed it to
    industrialio-backend.c.

Also, as compared with the RFC in [2], I don't think there are that many
similarities other than the filename. However, it should now be pretty
straight for Olivier to build on top of it. Also to mention that I did
grabbed patch 1 ("of: property: add device link support for
io-backends") from that series and just did some minor changes:


I did not already look at the framework patches in detail, but at first glance it looks fine.

I confirm that it has been quite straightforward to build on top of this framework, as it remains close to my first approach. I had only some small changes to do, to match the API, and to get it alive. This is great.

I see just one concern regarding ADC generic channel bindings support.
Here we have no devices associated to the channel sub-nodes. So, I cannot figure out we could use the devm_iio_backend_get() API, when looking for backend handle in channels sub-nodes. I have to think about it.

1) Renamed the property from "io-backend" to "io-backends".
2) No '#io-backend-cells' as it's not supported/needed by the framework
(at least for now) .

Regarding the driver core patch
("driver: core: allow modifying device_links flags"), it is more like a
RFC one. I'm not really sure if the current behavior isn't just
expected/wanted. Since I could not really understand if it is or not
(or why the different handling DL_FLAG_AUTOREMOVE_CONSUMER vs
DL_FLAG_AUTOREMOVE_SUPPLIER), I'm sending out the patch.

Jonathan,

I also have some fixes and cleanups for the ad9467 driver. I added
Fixes tags but I'm not sure if it's really worth it to backport them (given
what we already discussed about these drivers). I'll leave that to you
:).

I'm also not sure if I'm missing some tags (even though the series
is frankly different from [2]).

Olivier,

If you want to be included as a Reviewer let me know and I'll happily do
so in the next version.


Yes, please add me as reviewer.
Thanks.
Olivier

Also regarding the new IIO fw schemas. Should I send patches/PR to:

https://github.com/devicetree-org/dt-schema/

? Or is there any other workflow for it?

[1]: https://lore.kernel.org/linux-iio/20230727150324.1157933-1-olivier.moysan@xxxxxxxxxxx/
[2]: https://lore.kernel.org/linux-iio/20230727150324.1157933-1-olivier.moysan@xxxxxxxxxxx/

---
Nuno Sa (11):
       driver: core: allow modifying device_links flags
       iio: add the IIO backend framework
       iio: adc: ad9467: fix reset gpio handling
       iio: adc: ad9467: don't ignore error codes
       iio: adc: ad9467: add mutex to struct ad9467_state
       iio: adc: ad9467: fix scale setting
       iio: adc: ad9467: use spi_get_device_match_data()
       iio: adc: ad9467: use chip_info variables instead of array
       iio: adc: ad9467: convert to backend framework
       iio: adc: adi-axi-adc: convert to regmap
       iio: adc: adi-axi-adc: move to backend framework

Olivier Moysan (1):
       of: property: add device link support for io-backends

  MAINTAINERS                         |   7 +
  drivers/base/core.c                 |  14 +-
  drivers/iio/Kconfig                 |   5 +
  drivers/iio/Makefile                |   1 +
  drivers/iio/adc/Kconfig             |   3 +-
  drivers/iio/adc/ad9467.c            | 382 +++++++++++++++++++++-----------
  drivers/iio/adc/adi-axi-adc.c       | 429 +++++++-----------------------------
  drivers/iio/industrialio-backend.c  | 302 +++++++++++++++++++++++++
  drivers/of/property.c               |   2 +
  include/linux/iio/adc/adi-axi-adc.h |   4 +
  include/linux/iio/backend.h         |  58 +++++
  11 files changed, 723 insertions(+), 484 deletions(-)

Thanks!
- 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