Re: [PATCH v2 1/9] dt-bindings: iio: dac: ad3552r: add io-backend property

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

 




On 10/09/24 10:16 AM, Nuno Sá wrote:
On Mon, 2024-09-09 at 12:19 -0500, David Lechner wrote:
On 9/9/24 9:03 AM, Nuno Sá wrote:
On Mon, 2024-09-09 at 13:46 +0100, Conor Dooley wrote:
On Sun, Sep 08, 2024 at 01:29:25PM +0100, Jonathan Cameron wrote:
On Thu, 05 Sep 2024 17:17:31 +0200
Angelo Dureghello <adureghello@xxxxxxxxxxxx> wrote:

From: Angelo Dureghello <adureghello@xxxxxxxxxxxx>

There is a version AXI DAC IP block (for FPGAs) that provides
a physical bus for AD3552R and similar chips. This can be used
instead of a typical SPI controller to be able to use the chip
in ways that typical SPI controllers are not capable of.

The binding is modified so that either the device is a SPI
peripheral or it uses an io-backend.

Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
 required:
    - compatible
-  - reg
-  - spi-max-frequency
Sort of feels like both reg and spi-max-frequency
are valid things to specify.

Maybe we have an excellent IP and dodgy wiring so want
to clamp the frequency (long term - don't need to support
in the driver today).

Maybe we have an axi_dac IP that supports multiple
front end devices?  So maybe just keep reg?
I'd like to be convinced that this incarnation of the AXI DAC IP is not
a spi controller and that a ref to spi-controller.yaml is not out of
place here. It may not be something that you'd ever use generally, given
the "weird" interface to it, but it does seem to be one regardless.

Agreed.. As weird as it get's, it's acting as a spi controller.

I'd also really like to know how this fits in with spi-offloads. It
/feels/, and I'd like to reiterate the word feels, like a rather similar
idea just applied to a DAC instead of an ADC.
The offload main principle is to replay a spi transfer periodically given an
input trigger. I'm not so sure we have that same principle in here. In here
I
guess we stream data over the qspi interface based on SCLK which can look
similar. The difference is that this IP does not need any trigger for any
spi
transfer replay (I think).

Looking at the AD3552R from a SPI offload perspective of triggered SPI
messages, I think it still works.

The trigger doesn't have to be a clock/PWM. In this case, the trigger would
be whenever the IIO buffer is full and ready to send a burst of data (not
sure if this would be a hardware or software trigger - but it works either
way).

Right... That's what we already have for DACs with HW buffering.

Also, the DAC_CUSTOM_CTRL::ADDRESS register field in the AXI DAC IP core
acts as an offload to record and play back a SPI write transfer.

If we were using the AXI SPI Engine, this would be one SPI message with
two xfers, one for the address write followed by one for the data write.
Just a nipick comment. At least from the current implementation the address is
only writen once before starting to stream. So I guess we would not want to
replay that xfer for every sample.

The size of the data write would be the size of the IIO buffer - or in
the case of a cyclic DMA, the size of the write data would be channel
data size * num channels and the xfer would have a special cyclic offload
flag set.

So I think we could make a single binding that works for the the AXI DAC
backend/offload and the AXI SPI Engine offload. (I don't think it would
be so easy to integrate the AXI DAC into the SPI framework on the driver
side - and hopefully we won't have to, but the DT still could use the
proposed SPI offload bindings.)

Hopefully not...

As of now, i am proposing to stay in the simpler node as possible for the
current case, like:


    axi_dac: spi@44a70000 {
        compatible = "adi,axi-ad3552r";
        reg = <0x44a70000 0x1000>;
        dmas = <&dac_tx_dma 0>;
        dma-names = "tx";
        #io-backend-cells = <0>;
        clocks = <&ref_clk>;

        #address-cells = <1>;
        #size-cells = <0>;

        dac@0 {
            reg = <0>;
            compatible = "adi,ad3552r";
            reset-gpios = <&gpio0 92 GPIO_ACTIVE_HIGH>;
            io-backends = <&axi_dac>;

            #address-cells = <1>;
            #size-cells = <0>;

            channel@0 {
                reg = <0>;
                adi,output-range-microvolt = <(-10000000) (10000000)>;
            };
        };
    };


and extend things later on, in case.
SPI lines and other info are all obtained from the compatible.

What do you think ?



- Nuno Sá

--
 ,,,      Angelo Dureghello
:: :.     BayLibre -runtime team- Developer
:`___:
 `____:





[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