Message-ID: <CAMknhBEcEJ01nO0p5_vy4jVBVTL_rhEk+pvBpXdMtaDurc-05A@mail.gmail.com>
Date:   Fri, 1 Dec 2023 21:53:38 -0600
From:   David Lechner <dlechner@...libre.com>
To:     nuno.sa@...log.com
Cc:     linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-iio@...r.kernel.org,
        Olivier MOYSAN <olivier.moysan@...s.st.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>
Subject: Re: [PATCH 00/12] iio: add new backend framework

On Thu, Nov 30, 2023 at 5:54 PM David Lechner <dlechner@...libre.com> wrote:
>
> On Tue, Nov 21, 2023 at 4:17 AM Nuno Sa via B4 Relay
> <devnull+nuno.sa.analog.com@...nel.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@...00000 {
            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@...00000 {
            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@...00000 {
            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.

Powered by blists - more mailing lists