[00/12] iio: add new backend framework

  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:

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.

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á
  

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

On Thu, 2023-11-23 at 18:36 +0100, Olivier MOYSAN wrote:
> 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.
> 

Yeah, I'm keeping the series small (as Jonathan asked in the RFC) and just with basic
stuff needed to get the ad9647 driver in the exact same state as before the
framework. So yes, it's the same deal as with the component approach. You'll need to
add support for it. But, in this case, I believe it should be as straight as:

-/**
- * devm_iio_backend_get - Get a backend device
- * @dev:       Device where to look for the backend.
- * @name:      Backend name.
- *
- * Get's the backend associated with @dev.
- *
- * RETURNS:
- * A backend pointer, negative error pointer otherwise.
- */
-struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
+struct iio_backend *devm_fwnode_iio_backend_get(struct device *dev,
+                                               const struct fwnode_handle *fwnode,
+                                               const char *name)
 {
-       struct fwnode_handle *fwnode;
+       struct fwnode_handle *back_fwnode;
        struct iio_backend *back;
        int index = 0, ret;

@@ -195,20 +187,20 @@ struct iio_backend *devm_iio_backend_get(struct device *dev,
const char *name)
                        return ERR_PTR(index);
        }

-       fwnode = fwnode_find_reference(dev_fwnode(dev), "io-backends", index);
-       if (IS_ERR(fwnode)) {
+       back_fwnode = fwnode_find_reference(fwnode, "io-backends", index);
+       if (IS_ERR(back_fwnode)) {
                dev_err(dev, "Cannot get Firmware reference\n");
-               return ERR_CAST(fwnode);
+               return ERR_CAST(back_fwnode);
        }

        guard(mutex)(&iio_back_lock);
        list_for_each_entry(back, &iio_back_list, entry) {
                struct device_link *link;

-               if (!device_match_fwnode(back->dev, fwnode))
+               if (!device_match_fwnode(back->dev, back_fwnode))
                        continue;

-               fwnode_handle_put(fwnode);
+               fwnode_handle_put(back_fwnode);
                kref_get(&back->ref);
                if (!try_module_get(back->owner)) {
                        dev_err(dev, "Cannot get module reference\n");
@@ -229,9 +221,25 @@ struct iio_backend *devm_iio_backend_get(struct device *dev,
const char *name)
                return back;
        }

-       fwnode_handle_put(fwnode);
+       fwnode_handle_put(back_fwnode);
        return ERR_PTR(-EPROBE_DEFER);
 }
+EXPORT_SYMBOL_GPL(devm_fwnode_iio_backend_get);
+
+/**
+ * devm_iio_backend_get - Get a backend device
+ * @dev:       Device where to look for the backend.
+ * @name:      Backend name.
+ *
+ * Get's the backend associated with @dev.
+ *
+ * RETURNS:
+ * A backend pointer, negative error pointer otherwise.
+ */
+struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
+{
+       return devm_fwnode_iio_backend_get(dev, dev_fwnode(dev), name);
+}
 EXPORT_SYMBOL_GPL(devm_iio_backend_get);

 /**

Completely untested (not even compiled :)). Anyways, the goal is to just have the
minimum accepted and you can then send the needed patches for subnode lookups.


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

Will do.

- Nuno Sá
>
  

On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
<devnull+nuno.sa.analog.com@kernel.org> 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)
>

The "typical provider - consumer stuff" seems pretty straight forward
for finding and connecting two different devices, but the definition
of what is a frontend and what is a backend seems a bit nebulous. It
would be nice to seem some example devicetree to be able to get a
better picture of how this will be used in practices (links to the the
hardware docs for those examples would be nice too).

In addition to the backend ops given in this series, what are some
other expected ops that could be added in the future? Do we need some
kind of spec to say "I need a backend with feature X and feature Y" or
"I need a backend with compatible string" rather than just "I need a
generic backend"?
  

On Thu, 2023-11-30 at 17:54 -0600, David Lechner wrote:
> On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
> <devnull+nuno.sa.analog.com@kernel.org> 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)
> > 
> 
> The "typical provider - consumer stuff" seems pretty straight forward
> for finding and connecting two different devices, but the definition
> of what is a frontend and what is a backend seems a bit nebulous. It
> would be nice to seem some example devicetree to be able to get a
> better picture of how this will be used in practices (links to the the
> hardware docs for those examples would be nice too).
> 
> In addition to the backend ops given in this series, what are some
> other expected ops that could be added in the future? Do we need some
> kind of spec to say "I need a backend with feature X and feature Y" or
> "I need a backend with compatible string" rather than just "I need a
> generic backend"?

Bah, I know saw that I messed up in the cover letter. This was the link [1] I wanted
to add. In there, you'll see the RFC patch which already has lots of info. It also
has a link to a previous discussion  had with Jonathan about this kind off
arrangement.

You already more or less figured the frontend + backend story in one of your replies.
So, yes the data is received or transmitted (if we think DAC/DDS) by the actual
converter which makes sense to be seen as the frontend. This connects to another
device capable of handling the high sample rate (typically a core in a FPGA) of these
converters. But this is ADI usecase, note that STM also wants to have something like
this in order to link devices. Ah, and one of the things that Jonathan wants to see
is that only the frontend device exposes the IIO interface to userspace.




[1]: https://lore.kernel.org/linux-iio/20230804145342.1600136-1-nuno.sa@analog.com/

- Nuno Sá
  

On Fri, 2023-12-01 at 09:41 +0100, Nuno Sá wrote:
> On Thu, 2023-11-30 at 17:54 -0600, David Lechner wrote:
> > On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
> > <devnull+nuno.sa.analog.com@kernel.org> 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)
> > > 
> > 
> > The "typical provider - consumer stuff" seems pretty straight forward
> > for finding and connecting two different devices, but the definition
> > of what is a frontend and what is a backend seems a bit nebulous. It
> > would be nice to seem some example devicetree to be able to get a
> > better picture of how this will be used in practices (links to the the
> > hardware docs for those examples would be nice too).
> > 
> > In addition to the backend ops given in this series, what are some
> > other expected ops that could be added in the future? Do we need some
> > kind of spec to say "I need a backend with feature X and feature Y" or
> > "I need a backend with compatible string" rather than just "I need a
> > generic backend"?

To also reply to this one, I also have somewhere a comment about it (I think in the
code itself). For now, I'm thinking in this to serve all kinds of IIO devices in a
generic way which means, yes, the ops structure can grow badly. I'm not so sure if
that will actually happen in practise but if it does we can always play some OOP
games and leave the backend with the generic stuff (like .enable()/.disable() and so
on) and extend it (for example: having a converters thing that would have ops more
suitable for ADCs/DACs). Anyways, for now I think that would be overcomplicating
things. It's in kernel interfaces so we can always change things.


- Nuno Sá
  

On Thu, Nov 30, 2023 at 5:54 PM David Lechner <dlechner@baylibre.com> wrote:
>
> On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
> <devnull+nuno.sa.analog.com@kernel.org> 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)
> >
>
> The "typical provider - consumer stuff" seems pretty straight forward
> for finding and connecting two different devices, but the definition
> of what is a frontend and what is a backend seems a bit nebulous. It
> would be nice to seem some example devicetree to be able to get a
> better picture of how this will be used in practices (links to the the
> hardware docs for those examples would be nice too).
>

Fulfilling my own request here...

Since AD9467 is being use as the example first user of the IIO offload framework
I did a deep dive into how it is actually being used. It looks like this:

----------------------------------------------------------------------------
Hardware
----------------------------------------------------------------------------

AD9467
------

High-speed ADC that uses SPI for configuration and LVDS for data capture.

https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf

Pins:

    Supplies:
    - AVDD1 (1.8V analog supply)
    - AVDD2 (3.3V analog supply)
    - AVDD3 (1.8V analog supply)
    - SPIVDD (1.8V or 3.3V SPI supply)
    - DRVDD (1.8V digital output driver supply)
    - XVREF (optional reference voltage)

    SPI:
    - CSB
    - SDIO
    - SCLK

    Analog input:
    - VIN+/VIN-

    Clock input:
    - CLK+/CLK-

    Digital output:
    - Dn-/Dn+ (parallel digital output)
    - DCO+/DCO- (data clock output)
    - OR+/OR- (out of range output)


ADI AXI ADC
-----------

FPGA IP core for interfacing an ADC device with a high speed serial (JESD204B)
or source synchronous parallel interface (LVDS/CMOS).

https://wiki.analog.com/resources/fpga/docs/axi_adc_ip

Interfaces:

    LVDS:
    - rx_clk_in_[p|n]   clock input
    - rx_data_in_[p|n]  parallel data input
    - rx_frame_in_[p|n] frame input signal (optional/device specific)
    - rx_or_in_[p|n]    over range input (optional/device specific)

    Write FIFO:
    - see link for details, this consists of multiple signals that connect to a
      "sink module"

    MMIO:
    - memory mapped registers (detailed in link)


"sink module"
-------------

FPGA IP core for piping a data stream to DMA.

https://wiki.analog.com/resources/fpga/docs/util_var_fifo

Interfaces:

    Data Input:
    - data_in           Data to be stored
    - data_in_valid     Valid for the input data

    Data Output:
    - data_out          Data forwarded to the DMA
    - data_out_valid    Valid for the output data

    There are additional interfaces but they probably don't concern devicetree
    since software doesn't interact with them (AFAIK).


Schematic
---------

+----------------------------+    +---------------+    +-------------------+
| SOC/FPGA                   |    | ADC           |    | Signal generator  |
|                            |    |               |    |                   |
|  +--------------------+    |    |       VIN+/- xxxxxxxx VOUT+/-          |
|  | SPI                |    |    |               |    |                   |
|  |               SDIO ------------ SDIO         |    +-------------------+
|  |               SCLK ------------ SCLK         |
|  |                 CS ------------ CSB          |    +-------------------+
|  |                    |    |    |               |    | External clock    |
|  +--------------------+    |    |               |    |                   |
|  | AXI ADC            |    |    |      CLK+/- xxxxxxxxx CLKOUT+/-        |
|  |                    |    |    |               |    |                   |
|  |  rx_data_in_[p|n] xxxxxxxxxxxxx Dn+/-        |    +-------------------+
|  |   rx_clk_in_[p|n] xxxxxxxxxxxxx DCO+/DCO-    |
|  |    rx_or_in_[p|n] xxxxxxxxxxxxx OR+/-        |    +-------------------+
|  |                    |    |    |               |    | Power supplies    |
|  |        Write FIFO ===#  |    |               |    |                   |
|  +--------------------+ H  |    |        AVDD1 --------- 1.8V            |
|  | "sink module"      | H  |    |        AVDD2 --------- 2.2V            |
|  |                    | H  |    |        AVDD3 --------- 1.8V            |
|  |        Data Input ===#  |    |       SPIVDD --------- 3.3V            |
|  |                    |    |    |        DRVDD --------- 1.8V            |
|  |       Data Output ===#  |    |        XVREF --------- 1.2V            |
|  +--------------------+ H  |    |               |    |                   |
|  | DMA controller     | H  |    +---------------+    +-------------------+
|  |                    | H  |
|  |         channel 0 ===#  |
|  |                    |    |
|  +--------------------+    |
|                            |
+----------------------------+

----------------------------------------------------------------------------
Devicetree
----------------------------------------------------------------------------

Current situation:

&soc: {
    spi {
        ...

        spi_adc: adc@0 {
            compatible = "adi,ad9467";
            reg = <0>;
            clocks = <&adc_clk>;
            clock-names = "adc-clk";
        };
    };

    fpga {
        ...

        /* IIO device is associated with this node. */
        axi-adc@44a00000 {
            compatible = "adi,axi-adc-10.0.a";
            reg = <0x44a00000 0x10000>;
            /* Not sure dmas belong here since it is a property of the
             * "sink module" which is separate from AXI ADC module. */
            dmas = <&rx_dma 0>;
            dma-names = "rx";

            adi,adc-dev = <&spi_adc>;
        };
    };
};

---

Proposed IIO backend framework (inferred from v1 patches):

&soc: {
    spi {
        ...

        /* IIO device is associated with this node. */
        adc@0 {
            compatible = "adi,ad9467";
            reg = <0>;
            clocks = <&adc_clk>;
            clock-names = "adc-clk";
            /* As discussed already, this isn't really the right place for
             * dmas either. */
            dmas = <&rx_dma 0>;
            dma-names = "rx";

            io-backends = <&axi_adc>;
        };
    };

    fpga {
        ...

        axi_adc: axi-adc@44a00000 {
            compatible = "adi,axi-adc-10.0.a";
            reg = <0x44a00000 0x10000>;
        };
    };
};

---

Composite device?

/* IIO device is associated with this node. */
adc {
    compatible = "adi,ad9467";

    io-backends = <&adc_spi_backend>, <&adc_lvds_backend>;

    clocks = <&adc_clk>;
    clock-names = "adc-clk";

    xvref-supply = <&ref_voltage>;
    avdd1-supply = <&regulator_1_8V>;
    avdd2-supply = <&regulator_3_3V>;
    avdd3-supply = <&regulator_1_8V>;
};

&soc: {
    spi {
        ...

        /* This node describes only the SPI aspects of the ADC chip */
        adc_spi_backend: adc@0 {
            compatible = "adi,ad9467-spi-io-backend";
            reg = <0>;

            spi-3wire;
            spi-max-frequency = <25000000>;

            spivdd-supply = <&regulator_1_8V>;
        };
    };

    fpga {
        ...

        /* This node is an LVDS bus, analogous to a SPI bus or I2C bus */
        axi-adc@44a00000 {
            compatible = "adi,axi-adc-10.0.a";
            reg = <0x44a00000 0x10000>;

            ...

            /* This node describes all sink modules that are connected to
             * the AXI ADC controller through the FPGA fabric. */
            sink-modules {
                ...

                /* This node describes the FIFO to DMA sink module used by
                 * our ADC. */
                adc_dma: module@0 {
                    compatible = "adi,dma-fifo-1.0.a";
                    reg = <0>;

                    dmas = <&rx_dma 0>;
                    dma-names = "rx";
                };
            };

            /* Then we describe what is connected to each channel of the
             * controller (reg == channel number). */

            /* This node describes only the digital output (LVDS) aspects of
            * the ADC chip */
            adc_lvds_backend: adc@0 {
                compatible = "adi,ad9467-lvds-io-backend";
                reg = <0>;

                drvdd-supply = <&regulator_1_8V>;

                /* This is a LVDS bus peripheral property that can only be used
                 * with peripheral nodes that are children of a compatible =
                 * "adi,axi-adc-10.0.a" node. This works exactly like the
                 * controller-specific SPI bus peripheral properties, e.g.
                 * like samsung,spi-peripheral-props.yaml. */
                adi,sink-modules = <&adc_dma>;
            }
        };
    };
};


----------------------------------------------------------------------------
Discussion
----------------------------------------------------------------------------

After reviewing the existing device tree bindings, it appears the current
adi,ad9467.yaml is incomplete. It lacks the supplies and even though the
example shows that it is a child of a spi node, it is missing a reference to
/schemas/spi/spi-peripheral-props.yaml# and spi properties like spi-3wire
and spi-max-frequency. It also misses a description of what is connected to
the digital output, but that I think that is the main thing we are trying to
solve here - if it belongs there or somewhere else.

Having read more about the AXI ADC IP block, it seems to me like it should just
be considered an LVDS bus controller with generic bindings similar to how we
have SPI and I2C buses.

Following that line of thought, if the compatible = "adi,axi-adc-10.0.a" node
is a bus node, then logically, the ADC device node should be a child node of
that LVDS bus node. But since the ADC is also a SPI device it also needs to
be a child node of the SPI bus node. But is can't be a child of two different
nodes, so where does it go?

This is where the IIO backend framework can come in. We can create a "composite"
device as seen in the example dts above. This is just a platform device (in
Linux-speak) and it composes the two "io-backend" devices from the the two
busses to create a logical iio/adc device.

To solve the mystery of "where does the dmas property belong?", I have taken
a page out of the work I am preparing the AXI SPI Engine offload support [1].
(This hasn't been submitted to a mailing list yet because I'm still working
on the driver, but the bindings are finished and I ran the idea by Rob on IRC
a while back who suggested doing it this way, so maybe it works here too?)

[1]: https://github.com/analogdevicesinc/linux/pull/2341/commits/57bb1998371f61f4144689136aba5dd6d16a2a66

Since the "sink module" is really a separate IP block from the AXI ADC, it gets
its own node and compatible string. Since these "sink modules" are connected to
the AXI ADC, they get listed as child nodes, but we group them together under a
single sink-modules node to separate them from the LVDS peripherals nodes. Then
they get associated with a peripheral with the adi,sink-modules property (also
see comment on that property in the example above).

My "composite" device example evolved quite a bit as I was writing this but I'm
pretty happy with where it ended up. I think adding child nodes to the axi-adc
node answers Nuno's concerns about how to keep a generic axi-adc binding while
accounting for the fact that it changes slightly depending on what it is
attached to. And having a separate platform device solves my questions about
the ambiguity of which should be the front end, the spi node or the axi-adc
node. It turns out, perhaps the answer is neither.
  

On Fri, 2023-12-01 at 21:53 -0600, David Lechner wrote:
> On Thu, Nov 30, 2023 at 5:54 PM David Lechner <dlechner@baylibre.com> wrote:
> > 
> > On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
> > <devnull+nuno.sa.analog.com@kernel.org> 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)
> > > 
> > 
> > The "typical provider - consumer stuff" seems pretty straight forward
> > for finding and connecting two different devices, but the definition
> > of what is a frontend and what is a backend seems a bit nebulous. It
> > would be nice to seem some example devicetree to be able to get a
> > better picture of how this will be used in practices (links to the the
> > hardware docs for those examples would be nice too).
> > 
> 
> Fulfilling my own request here...
> 
> Since AD9467 is being use as the example first user of the IIO offload framework
> I did a deep dive into how it is actually being used. It looks like this:
> 

This is not an offload framework... I think somehow you're connecting this to the
spi_engine offload and these are two completely different things. Maybe they can
intersect at some point but as of now, I don't see any benefit in doing it. The goal
of this patchseries is to have a simple and generic framework so we can connect IIO
devices (frontends) to a backend device having kind of an IIO aggregate device so to
say. Moreover, we just want to have the ad9467 driver in the same state it was before
to keep things simple. I'm already fixing some things but I don't want extend that
too much as the primary goal is to have the framework in. Cleanups can come
afterwards.

That said, is fine to have this kind of discussion but I honestly think you're over
engineering the whole thing. Maybe you're already too ahead of me :), but where we
stand right now, I don't see any reason for anything so complicated as the below.
Also note this should be simple and generic. As I already said, this is not supposed
to be an ADI only thing and STM also wants to make use of this infrastructure. But
see below some of my comments on why I think it's too much...
 
> ----------------------------------------------------------------------------
> Hardware
> ----------------------------------------------------------------------------
> 
> AD9467
> ------
> 
> High-speed ADC that uses SPI for configuration and LVDS for data capture.
> 
> https://www.analog.com/media/en/technical-documentation/data-sheets/AD9467.pdf
> 
> Pins:
> 
>     Supplies:
>     - AVDD1 (1.8V analog supply)
>     - AVDD2 (3.3V analog supply)
>     - AVDD3 (1.8V analog supply)
>     - SPIVDD (1.8V or 3.3V SPI supply)
>     - DRVDD (1.8V digital output driver supply)
>     - XVREF (optional reference voltage)
> 
>     SPI:
>     - CSB
>     - SDIO
>     - SCLK
> 
>     Analog input:
>     - VIN+/VIN-
> 
>     Clock input:
>     - CLK+/CLK-
> 
>     Digital output:
>     - Dn-/Dn+ (parallel digital output)
>     - DCO+/DCO- (data clock output)
>     - OR+/OR- (out of range output)
> 
> 
> ADI AXI ADC
> -----------
> 
> FPGA IP core for interfacing an ADC device with a high speed serial (JESD204B)
> or source synchronous parallel interface (LVDS/CMOS).
> 
> https://wiki.analog.com/resources/fpga/docs/axi_adc_ip
> 
> Interfaces:
> 
>     LVDS:
>     - rx_clk_in_[p|n]   clock input
>     - rx_data_in_[p|n]  parallel data input
>     - rx_frame_in_[p|n] frame input signal (optional/device specific)
>     - rx_or_in_[p|n]    over range input (optional/device specific)
> 
>     Write FIFO:
>     - see link for details, this consists of multiple signals that connect to a
>       "sink module"
> 
>     MMIO:
>     - memory mapped registers (detailed in link)
> 
> 
> "sink module"
> -------------
> 
> FPGA IP core for piping a data stream to DMA.
> 
> https://wiki.analog.com/resources/fpga/docs/util_var_fifo
> 
> Interfaces:
> 
>     Data Input:
>     - data_in           Data to be stored
>     - data_in_valid     Valid for the input data
> 
>     Data Output:
>     - data_out          Data forwarded to the DMA
>     - data_out_valid    Valid for the output data
> 
>     There are additional interfaces but they probably don't concern devicetree
>     since software doesn't interact with them (AFAIK).
> 
> 
> Schematic
> ---------
> 
> +----------------------------+    +---------------+    +-------------------+
> > SOC/FPGA                   |    | ADC           |    | Signal generator  |
> >                            |    |               |    |                   |
> >  +--------------------+    |    |       VIN+/- xxxxxxxx VOUT+/-          |
> >  | SPI                |    |    |               |    |                   |
> >  |               SDIO ------------ SDIO         |    +-------------------+
> >  |               SCLK ------------ SCLK         |
> >  |                 CS ------------ CSB          |    +-------------------+
> >  |                    |    |    |               |    | External clock    |
> >  +--------------------+    |    |               |    |                   |
> >  | AXI ADC            |    |    |      CLK+/- xxxxxxxxx CLKOUT+/-        |
> >  |                    |    |    |               |    |                   |
> >  |  rx_data_in_[p|n] xxxxxxxxxxxxx Dn+/-        |    +-------------------+
> >  |   rx_clk_in_[p|n] xxxxxxxxxxxxx DCO+/DCO-    |
> >  |    rx_or_in_[p|n] xxxxxxxxxxxxx OR+/-        |    +-------------------+
> >  |                    |    |    |               |    | Power supplies    |
> >  |        Write FIFO ===#  |    |               |    |                   |
> >  +--------------------+ H  |    |        AVDD1 --------- 1.8V            |
> >  | "sink module"      | H  |    |        AVDD2 --------- 2.2V            |
> >  |                    | H  |    |        AVDD3 --------- 1.8V            |
> >  |        Data Input ===#  |    |       SPIVDD --------- 3.3V            |
> >  |                    |    |    |        DRVDD --------- 1.8V            |
> >  |       Data Output ===#  |    |        XVREF --------- 1.2V            |
> >  +--------------------+ H  |    |               |    |                   |
> >  | DMA controller     | H  |    +---------------+    +-------------------+
> >  |                    | H  |
> >  |         channel 0 ===#  |
> >  |                    |    |
> >  +--------------------+    |
> >                            |
> +----------------------------+
> 
> ----------------------------------------------------------------------------
> Devicetree
> ----------------------------------------------------------------------------
> 
> Current situation:
> 
> &soc: {
>     spi {
>         ...
> 
>         spi_adc: adc@0 {
>             compatible = "adi,ad9467";
>             reg = <0>;
>             clocks = <&adc_clk>;
>             clock-names = "adc-clk";
>         };
>     };
> 
>     fpga {
>         ...
> 
>         /* IIO device is associated with this node. */
>         axi-adc@44a00000 {
>             compatible = "adi,axi-adc-10.0.a";
>             reg = <0x44a00000 0x10000>;
>             /* Not sure dmas belong here since it is a property of the
>              * "sink module" which is separate from AXI ADC module. */
>             dmas = <&rx_dma 0>;
>             dma-names = "rx";
> 
>             adi,adc-dev = <&spi_adc>;
>         };
>     };
> };
> 
> ---
> 
> Proposed IIO backend framework (inferred from v1 patches):
> 
> &soc: {
>     spi {
>         ...
> 
>         /* IIO device is associated with this node. */
>         adc@0 {
>             compatible = "adi,ad9467";
>             reg = <0>;
>             clocks = <&adc_clk>;
>             clock-names = "adc-clk";
>             /* As discussed already, this isn't really the right place for
>              * dmas either. */
>             dmas = <&rx_dma 0>;
>             dma-names = "rx";

Not ideally but if they stay here is also not a big deal/problem...
> 
>             io-backends = <&axi_adc>;
>         };
>     };
> 
>     fpga {
>         ...
> 
>         axi_adc: axi-adc@44a00000 {
>             compatible = "adi,axi-adc-10.0.a";
>             reg = <0x44a00000 0x10000>;
>         };
>     };
> };
> 
> ---
> 
> Composite device?
> 
> /* IIO device is associated with this node. */
> adc {
>     compatible = "adi,ad9467";
> 
>     io-backends = <&adc_spi_backend>, <&adc_lvds_backend>;
> 
>     clocks = <&adc_clk>;
>     clock-names = "adc-clk";
> 
>     xvref-supply = <&ref_voltage>;
>     avdd1-supply = <&regulator_1_8V>;
>     avdd2-supply = <&regulator_3_3V>;
>     avdd3-supply = <&regulator_1_8V>;
> };
> 
> &soc: {
>     spi {
>         ...
> 
>         /* This node describes only the SPI aspects of the ADC chip */
>         adc_spi_backend: adc@0 {
>             compatible = "adi,ad9467-spi-io-backend";
>             reg = <0>;
> 
>             spi-3wire;
>             spi-max-frequency = <25000000>;
> 
>             spivdd-supply = <&regulator_1_8V>;
>         };
>     };
> 
>     fpga {
>         ...
> 
>         /* This node is an LVDS bus, analogous to a SPI bus or I2C bus */
>         axi-adc@44a00000 {
>             compatible = "adi,axi-adc-10.0.a";
>             reg = <0x44a00000 0x10000>;
> 
>             ...
> 
>             /* This node describes all sink modules that are connected to
>              * the AXI ADC controller through the FPGA fabric. */
>             sink-modules {
>                 ...
> 
>                 /* This node describes the FIFO to DMA sink module used by
>                  * our ADC. */
>                 adc_dma: module@0 {
>                     compatible = "adi,dma-fifo-1.0.a";

Why a different compatible?
> 

>                     reg = <0>;
> 
>                     dmas = <&rx_dma 0>;
>                     dma-names = "rx";
>                 };
>             };
> 
>             /* Then we describe what is connected to each channel of the
>              * controller (reg == channel number). */

I don't think we need to describe that in here. We might get to a point where we
might need some clever properties in the backend device but also note that the
frontend device should also know what to do with the device. Most of the knowledge
should be there (of course we should not have properties in the frontend that don't
belong to that device).

> 
>             /* This node describes only the digital output (LVDS) aspects of
>             * the ADC chip */
>             adc_lvds_backend: adc@0 {
>                 compatible = "adi,ad9467-lvds-io-backend";

This is just adding complexity... why different compatibles? 

>                 reg = <0>;
> 
>                 drvdd-supply = <&regulator_1_8V>;
> 
>                 /* This is a LVDS bus peripheral property that can only be used
>                  * with peripheral nodes that are children of a compatible =
>                  * "adi,axi-adc-10.0.a" node. This works exactly like the
>                  * controller-specific SPI bus peripheral properties, e.g.
>                  * like samsung,spi-peripheral-props.yaml. */
>                 adi,sink-modules = <&adc_dma>;
>             }
>         };
>     };
> };
> 
> 
> ----------------------------------------------------------------------------
> Discussion
> ----------------------------------------------------------------------------
> 
> After reviewing the existing device tree bindings, it appears the current
> adi,ad9467.yaml is incomplete. It lacks the supplies and even though the
> example shows that it is a child of a spi node, it is missing a reference to
> /schemas/spi/spi-peripheral-props.yaml# and spi properties like spi-3wire
> and spi-max-frequency. It also misses a description of what is connected to
> the digital output, but that I think that is the main thing we are trying to
> solve here - if it belongs there or somewhere else.

As stated, bindings can be improved/cleaned afterwards...

> 
> Having read more about the AXI ADC IP block, it seems to me like it should just
> be considered an LVDS bus controller with generic bindings similar to how we
> have SPI and I2C buses.

No... That core can handle LVDS interfaces, yes. But it can also do CMOS and JESD. So
you can see already that having a bus for it is completely over-kill. And those cores
have a 1:1 connection. It's just a serial interface to transfer data at high-speed
rates so I'm not really seeing how we would benefit from having a bus for it. Of
course if the day comes where this gets somehow used outside of converters and IIO
devices, we could think in making it a generic bus. As of now, I'm honestly not
seeing it.

And to add a bit more on the 1:1 stuff, what sometimes happens for highly complex
devices (like RF transceivers) with complex RX/TX data paths where each path as a
serial data interface (LVDS/CMOS/JESD) is that you have an instance of the
AXI_ADC/AXI_DAC cores per data path. So, effectively you end up with one frontend
(the transceiver) with multiple backends which also fits nicely in the current
proposal. Multiple frontends for one backend is something that I'm not aware (at
least ADI wise) but it might be a thing for other usecases. Also doable, but then you
already need to add support for it (like adding counters for enable/disable states
and things like that).

Since I'm talking usecases, one that we have at ADI that is upstreamable is a
cascaded one where the AXI_DAC core can serve as backend and frontend at the same
time. That might happen in cases the DAC core has an interpolation core behind it.
So, some fun ahead of us :) (should also be fairly straight with what we have).

One thing that at some point I might ask our hdl guys is to have an ID register so we
can identify for which frontend the backend was built. This could be useful to pass
come config to the core so we could do some safety checks. A nice thing but also not
a super duper crucial one...

> 
> Following that line of thought, if the compatible = "adi,axi-adc-10.0.a" node
> is a bus node, then logically, the ADC device node should be a child node of
> that LVDS bus node. But since the ADC is also a SPI device it also needs to
> be a child node of the SPI bus node. But is can't be a child of two different
> nodes, so where does it go?
> 

I think the above "kills" this line of thought :).

> This is where the IIO backend framework can come in. We can create a "composite"
> device as seen in the example dts above. This is just a platform device (in
> Linux-speak) and it composes the two "io-backend" devices from the the two
> busses to create a logical iio/adc device.
> 
> To solve the mystery of "where does the dmas property belong?", I have taken
> a page out of the work I am preparing the AXI SPI Engine offload support [1].
> (This hasn't been submitted to a mailing list yet because I'm still working
> on the driver, but the bindings are finished and I ran the idea by Rob on IRC
> a while back who suggested doing it this way, so maybe it works here too?)
> 
> [1]:
> https://github.com/analogdevicesinc/linux/pull/2341/commits/57bb1998371f61f4144689136aba5dd6d16a2a66
> 
> Since the "sink module" is really a separate IP block from the AXI ADC, it gets
> its own node and compatible string. Since these "sink modules" are connected to
> the AXI ADC, they get listed as child nodes, but we group them together under a
> single sink-modules node to separate them from the LVDS peripherals nodes. Then
> they get associated with a peripheral with the adi,sink-modules property (also
> see comment on that property in the example above).
> 

I never really looked at that IP but if it is something sitting behind the axi_adc
might also be a candidate for a cascaded configuration. Anyways, as I said, it's fine
to discuss it but that is all stuff that will only go in once we have users for it.
I'm also not aware if we do have an usecase for it or if it's something we control at
all from linux (might be a pure hw thing).

> My "composite" device example evolved quite a bit as I was writing this but I'm
> pretty happy with where it ended up. I think adding child nodes to the axi-adc
> node answers Nuno's concerns about how to keep a generic axi-adc binding while
> accounting for the fact that it changes slightly depending on what it is
> attached to. And having a separate platform device solves my questions about
> the ambiguity of which should be the front end, the spi node or the axi-adc
> node. It turns out, perhaps the answer is neither.

So, in conclusion, let's make this simple for now. When we have enough users and we
are aware of the real complexity of the whole thing we can see how to proceed. It
might even be you're right from the beginning and we end up in something like this
(but at least for the bus stuff I'm very sceptical). Now, if we add too much
complexity right from the beginning, it might be way harder to change/scale it down
the road.

Anyways, thanks for your feedback and for some bugs issues you already found in the
series!

- Nuno Sá
  

On Sat, Dec 2, 2023 at 3:37 AM Nuno Sá <noname.nuno@gmail.com> wrote:
>
> On Fri, 2023-12-01 at 21:53 -0600, David Lechner wrote:
> > On Thu, Nov 30, 2023 at 5:54 PM David Lechner <dlechner@baylibre.com> wrote:
> > >
> > > On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
> > > <devnull+nuno.sa.analog.com@kernel.org> 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)
> > > >
> > >
> > > The "typical provider - consumer stuff" seems pretty straight forward
> > > for finding and connecting two different devices, but the definition
> > > of what is a frontend and what is a backend seems a bit nebulous. It
> > > would be nice to seem some example devicetree to be able to get a
> > > better picture of how this will be used in practices (links to the the
> > > hardware docs for those examples would be nice too).
> > >
> >
> > Fulfilling my own request here...
> >
> > Since AD9467 is being use as the example first user of the IIO offload framework
> > I did a deep dive into how it is actually being used. It looks like this:
> >
>
> This is not an offload framework... I think somehow you're connecting this to the
> spi_engine offload and these are two completely different things. Maybe they can
> intersect at some point but as of now, I don't see any benefit in doing it. The goal
> of this patchseries is to have a simple and generic framework so we can connect IIO
> devices (frontends) to a backend device having kind of an IIO aggregate device so to
> say. Moreover, we just want to have the ad9467 driver in the same state it was before
> to keep things simple. I'm already fixing some things but I don't want extend that
> too much as the primary goal is to have the framework in. Cleanups can come
> afterwards.
>
> That said, is fine to have this kind of discussion but I honestly think you're over
> engineering the whole thing. Maybe you're already too ahead of me :), but where we
> stand right now, I don't see any reason for anything so complicated as the below.
> Also note this should be simple and generic. As I already said, this is not supposed
> to be an ADI only thing and STM also wants to make use of this infrastructure. But
> see below some of my comments on why I think it's too much...

This is a very fair point. I do have a tendency to overthink things. :-)

At the very least, being able to see the schematic of how it all fits
together filled in the holes of my understanding and now everything
you are doing in this series makes sense to me. And I totally agree
with keeping it simpler is better.
  

On Sat, 2 Dec 2023 10:16:52 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On Sat, Dec 2, 2023 at 3:37 AM Nuno Sá <noname.nuno@gmail.com> wrote:
> >
> > On Fri, 2023-12-01 at 21:53 -0600, David Lechner wrote:  
> > > On Thu, Nov 30, 2023 at 5:54 PM David Lechner <dlechner@baylibre.com> wrote:  
> > > >
> > > > On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
> > > > <devnull+nuno.sa.analog.com@kernel.org> 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)
> > > > >  
> > > >
> > > > The "typical provider - consumer stuff" seems pretty straight forward
> > > > for finding and connecting two different devices, but the definition
> > > > of what is a frontend and what is a backend seems a bit nebulous. It
> > > > would be nice to seem some example devicetree to be able to get a
> > > > better picture of how this will be used in practices (links to the the
> > > > hardware docs for those examples would be nice too).
> > > >  
> > >
> > > Fulfilling my own request here...
> > >
> > > Since AD9467 is being use as the example first user of the IIO offload framework
> > > I did a deep dive into how it is actually being used. It looks like this:
> > >  
> >
> > This is not an offload framework... I think somehow you're connecting this to the
> > spi_engine offload and these are two completely different things. Maybe they can
> > intersect at some point but as of now, I don't see any benefit in doing it. The goal
> > of this patchseries is to have a simple and generic framework so we can connect IIO
> > devices (frontends) to a backend device having kind of an IIO aggregate device so to
> > say. Moreover, we just want to have the ad9467 driver in the same state it was before
> > to keep things simple. I'm already fixing some things but I don't want extend that
> > too much as the primary goal is to have the framework in. Cleanups can come
> > afterwards.
> >
> > That said, is fine to have this kind of discussion but I honestly think you're over
> > engineering the whole thing. Maybe you're already too ahead of me :), but where we
> > stand right now, I don't see any reason for anything so complicated as the below.
> > Also note this should be simple and generic. As I already said, this is not supposed
> > to be an ADI only thing and STM also wants to make use of this infrastructure. But
> > see below some of my comments on why I think it's too much...  
> 
> This is a very fair point. I do have a tendency to overthink things. :-)
> 
> At the very least, being able to see the schematic of how it all fits
> together filled in the holes of my understanding and now everything
> you are doing in this series makes sense to me. And I totally agree
> with keeping it simpler is better.

Interesting discussion. One key thing it has highlighted for me is that
even the simpler proposal that Nuno has put forward deserves some
more documentation!  Preferably with some asci art - though maybe not as complex
as David's pretty picture.  I keep forgetting which is the backend and which
is the front end for this discussion which isn't helping me get my head
around it.

Jonathan