Message-ID: <a30d4887-c118-4997-8e84-4a269b58b371@baylibre.com>
Date: Wed, 11 Sep 2024 10:45:24 +0200
From: Angelo Dureghello <adureghello@...libre.com>
To: Nuno Sá <noname.nuno@...il.com>,
 David Lechner <dlechner@...libre.com>,
 Conor Dooley <conor.dooley@...rochip.com>,
 Jonathan Cameron <jic23@...nel.org>
Cc: Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Nuno Sá <nuno.sa@...log.com>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Olivier Moysan <olivier.moysan@...s.st.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/9] dt-bindings: iio: dac: ad3552r: add io-backend
 property


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@...libre.com> wrote:
>>>>>
>>>>>> From: Angelo Dureghello <adureghello@...libre.com>
>>>>>>
>>>>>> 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@...libre.com>
>>>>>>   
>>>>>>   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@...70000 {
         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
:`___:
  `____:

Powered by blists - more mailing lists