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

Olivier MOYSAN
2023-11-23T17:37:24Z

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@foss.st.com/
> [2]: https://lore.kernel.org/linux-iio/20230727150324.1157933-1-olivier.moysan@foss.st.com/
>
> ---
> 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á
>