Re: [PATCH v3 04/10] dt-bindings: iio: dac: ad3552r: add io-backend support

[David Lechner <dlechner@baylibre.com>]

On 9/30/24 10:08 AM, Angelo Dureghello wrote:
> On 30.09.2024 15:49, Jonathan Cameron wrote:
>> On Mon, 30 Sep 2024 16:15:41 +0200
>> Angelo Dureghello <adureghello@baylibre.com> wrote:
>>
>>> On 29.09.2024 11:51, Jonathan Cameron wrote:
>>>> On Thu, 19 Sep 2024 11:20:00 +0200
>>>> Angelo Dureghello <adureghello@baylibre.com> wrote:
>>>>   
>>>>> From: Angelo Dureghello <adureghello@baylibre.com>
>>>>>
>>>>> There is a version AXI DAC IP block (for FPGAs) that provides
>>>>> a physical bus for AD3552R and similar chips, and acts as
>>>>> an SPI controller.  
>>>>
>>>> Wrap is a bit short. Aim for < 75 chars for patch descriptions.
>>>>   
>>>>>
>>>>> For this case, the binding is modified to include some
>>>>> additional properties.
>>>>>
>>>>> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com>
>>>>> ---
>>>>>  .../devicetree/bindings/iio/dac/adi,ad3552r.yaml   | 42 ++++++++++++++++++++++
>>>>>  1 file changed, 42 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
>>>>> index 41fe00034742..aca4a41c2633 100644
>>>>> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
>>>>> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
>>>>> @@ -60,6 +60,18 @@ properties:
>>>>>      $ref: /schemas/types.yaml#/definitions/uint32
>>>>>      enum: [0, 1, 2, 3]
>>>>>  
>>>>> +  io-backends:
>>>>> +    description: The iio backend reference.  
>>>>
>>>> Give a description of what the backend does in this case.  I.e. that it is
>>>> a qspi DDR backend with ...
>>>>   
>>>>> +      An example backend can be found at
>>>>> +        https://analogdevicesinc.github.io/hdl/library/axi_ad3552r/index.html
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  adi,synchronous-mode:
>>>>> +    description: Enable waiting for external synchronization signal.
>>>>> +      Some AXI IP configuration can implement a dual-IP layout, with internal
>>>>> +      wirings for streaming synchronization.  
>>>>
>>>> I've no idea what a dual-IP layout is.  Can you provide a little more info
>>>> here?  What are the two IPs?
>>>>  
>>> IP is a term used in fpga design as "intellectual property", that is
>>> intended as a functional block of logic or data used to make a 
>>> field-programmable gate array module.
>>>
>>> A dual layout is just 2 same fpga modules in place of one.
>>>  
>>> I can add a "fpga" regerence to be more clear.
>>
>> IP I was familiar with.  I'm more interested in what each IP is doing in this
>> case.  Or at least an example of what sort of split of functionality might
>> make use of this.
>>
> 
> I have an image of the project (that is under development or testing now),
> not sure how to attach the image here, btw, something as
>  
>           axi_ad3552r_0  ----------->---- qspi0
>               sync_ext_device --.
>        .- external_sync          |
>        |                         |
>        |-------------<-----------                        
>        |
>        |   axi_ad3552r_1 ----------->---- qspi1
>        `- external_sync
>  
> My understanding is that it's just a method to use a octal spi,
> duplicating the transfer rate. I can collect more info in case.
> 

No, it's not for octal SPI. It is for synchronizing the data
transfer to two different DAC chips.

I think we need a bit more in the DT bindings for this to fully
describe the wiring shown. We need to indicate that both of the
two AXI AD3552R IP blocks have external_sync connected, so a
adi,external-sync flag could be used for this. Then we also need
to describe that sync_ext_device is only wired up on one of the
IP blocks. So we would need a separate adi,sync-ext-device flag.

Then the driver would use this information to A) know that we
need to set the external sync arm bit when starting buffered
reads and B) know that the buffered read for the IP block
instance with sync_ext_device needs to be started last so that
the data streams for both DACs will be synchronized.