Messages in this thread | | | Date | Thu, 23 Nov 2023 18:36:45 +0100 | Subject | Re: [PATCH 00/12] iio: add new backend framework | From | Olivier MOYSAN <> |
| |
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á >
| |