Re: [PATCH v3 0/8] iio: add new backend framework

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

 



Hi Nuno,

On 12/15/23 16:18, Nuno Sá wrote:
On Thu, 2023-12-14 at 11:03 -0600, Rob Herring wrote:
On Thu, Dec 14, 2023 at 05:05:20PM +0100, Nuno Sá wrote:
On Thu, 2023-12-14 at 08:16 -0600, Rob Herring wrote:
On Wed, Dec 13, 2023 at 04:02:31PM +0100, Nuno Sa wrote:
v1:
https://lore.kernel.org/linux-iio/20231204144925.4fe9922f@jic23-huawei/T/#m222f517
5273b81dbfe40b7f0daffcdc67d6cb8ff

v2:
https://lore.kernel.org/r/20231208-dev-iio-backend-v2-0-5450951895e1@xxxxxxxxx
m

Changes in v3:
- Patch 1:
  * Use proposed generic schema [1]. Also make it a required property;
  * Improved the commit message.
- Patch 2:
  * Improved commit message.
- Patch 4:
  * Namespace all IIO DMAENGINE buffer exports;
  * Removed unrelated new line removal change.
- Patch 5:
  * Namespace all IIO backend exports.
- Patch 6:
  * Set backend.h in alphabetical order;
  * Import IIO backend namespace.
- Patch 7:
  * Don't depend on OF in kbuild anymore;
  * Import IIO backend namespace.

For the bindings patches, I tried not to enter into much details about
the IIO framework as I think specifics of the implementation don't care
from the bindings perspective. Hopefully the commit messages are good
enough.

I'm also aware that patch 1 is not backward compatible but we are
anyways doing it on the driver side (and on the driver the property is
indeed required). Anyways, just let me know if making the property
required is not acceptable (I'm fairly confident no one was using the
upstream version of the driver and so validating devicetrees for it).

Keeping the block diagram in v3's cover so we don't have to follow links
to check the one of the typicals setups.

                                            ----------------------------------
----
-----------------
  ------------------                        | -----------         ------------
       -------  FPGA |
  |     ADC        |------------------------| | AXI ADC |---------| DMA CORE
|----
--| RAM |       |
  | (Frontend/IIO) | Serial Data (eg: LVDS) | |(backend)|---------|
|----
--|     |       |
  |                |------------------------| -----------         ------------
       -------       |
  ------------------                        ----------------------------------
----
-----------------

Why doesn't axi-adc just have an io-channels property to adc? It's the
opposite direction for the link, but it seems more logical to me that
axi-adc depends on adc rather than the other way around.


We are not interested on a single channel but on the complete device. >From the
axi-
adc point of view, there's not much we could do with the adc channel. I mean,
maybe
we could still do something but it would be far from ideal (see below).

Will this hold up for everyone? Could there be a backend device that
handles multiple ADCs? IOW, do you need #io-backend-cells? It's somewhat
better if we add that up front rather than later and have to treat
missing as 0 cells. It is also the only way to generically identify the
providers (well, there's also 'foo-controller' properties, but we've
gone away from those).


For the axi-adc backend, it's very unlikely. The way the core connects to the
converters is through a serial data interface (LVDS, CMOS or JESD in ADI usecases).
This interface is not really a bus so it's kind of a 1:1 connection. Now, in more
complicated devices (like highly integrated RF transceivers) what we have is that we
have multiple of these cores (one per RX/TX port) connected to the frontend. So,
effectively 1 frontend could have multiple backends. So, yes, your first "doubts" are
not that "crazy" as this is also not the "typical" provider - consumer relationship.
However, for all of what I've said in the previous email, even in these cases,
thinking in these cores as the provider, makes things much more easier to handle.

However, the above is just ADI usecases. In theory, yes, it can be very possible for
another backend other than axi-adc to have multiple frontends connected so I guess we
can make #io-backend-cells already available in the schema.

For the axi-adc bindings this would be 'const: 0', right?


The opposite direction is exactly what we had (look at patch 2) just with another
custom property. The problem with that is that we would then need a bidirectional
link (we would need to callback into the provider and the provider would need to
access the consumer) between consumers and providers and that would be far from
optimal. The bidirectional link would exist because if we want to support
fundamental
things like LVDS/CMOS interface tuning we need to set/get settings from the axi-
adc
core. And as Jonathan suggested, the real data is captured or sent on the
converters
(ADC or DACs) and that is why we have the IIO device/interface in there and why
we
call them "frontends". In ADI usecases, backends are these FPGA cores providing
"services" to the "real" high speed converters. To put it in another way, the
real
converter is the one who knows how to use the axi-adc core and not the other way
around. That's also one of the reasons why it would be way more difficult to
handle
things with the opposite link. That's how we have done it so far and the mess we
have
out of tree is massive :sweat_smile: We ended up doing raw writes and reads on
the
axi-adc MMIO registers from the converter driver just because we had to configure
or
get something from the axi-adc device but the link was backwards.

The direction (for the binding) doesn't really matter. It doesn't
dictate the direction in the OS. In the ad9467 driver, you can search
the DT for 'adi,adc-dev' and find the node which matches your node's
phandle. It's not exactly efficient, but you are doing it once. It would
also prevent the DT ABI break you are introducing.


Hmm, I think I see your idea. So you mean something like
devm_iio_backend_get_optional() and if not present, then we would look for nodes
having the 'adi,adc-dev' property and look for the one pointing at us... Then, we
would need another API in the backend to look for registered backends matching that
fwnode. Right?

I mean, I guess this could work but we would already have to start a fresh framework
with API's that are not really meant to be used anymore other than the ad9467 driver
(not devm_iio_backend_get_optional() because sooner or later I think we'll need that
one). We are already breaking ABI in the driver and I'm still fairly confident no one
is really using the upstream driver because it's lacking support for devices and
important features (not even in ADI fork we're using it).

Anyways, if you still insist on having something like this (and feel more comfortable
in not breaking DT ABI), I can see how this would look like in the next version...


And if there's another consumer in the chain, then a node could
certainly be both an io-channels consumer and producer.


This should also be possible with this architecture. A node can be both a backend
(provider) and a consumer and we have an out of tree design that fits this (that
I
surely want to upstream after the foundations are done).

The architecture of the drivers seems odd to me. It looks similar to
making a phy driver handle all the state and protocol with the host
controller being a backend.

In this case, it's not really a controller. It's more like an extension of the
device
because we need a way to handle the high sample rates this ADCs can do. Then we
can
also do test tones with the backend which is useful for interface tuning (as
mentioned above).

To give you a bit more context, I'm adding the generic property because we will
have
more users for it (from ADI - the next should be the axi-dac core) but STM is
also
interested in this (likely the next user).

Hope this makes it a bit more clear...

Yes, thanks.

I generally ask for 2 users on new common bindings. I've accepted too
many only to have the 2nd user come in a month later and need additions.
An ack on the binding from the STM folks would be nice here. And
Jonathan too.


Olivier, could we get an ack on the bindings patch? Do you also have any idea about
how long it would take for you to send patches so we have another user of the schema?

On my side, it might very well take a month or so (given we have holidays nearby) as
the axi-dac core is more complex than the axi-adc. Bah it might take less than a
month to have the first version of it posted in the lists but I can't make any
promises.


Sorry for late answer.
Regarding bindings patch, I assume you refer to [1].
After adding the #io-backend-cells property in v5 you can add my
Ack-by Olivier Moysan <olivier.moysan@xxxxxxxxxxx>

I will prepare a serie based on these bindings. Hopefully it should be possible this month.

BRs
Olivier

[1] https://github.com/devicetree-org/dt-schema/pull/120


- Nuno Sá





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux