Re: [PATCH v6 2/8] dt-bindings: iio: dac: adi-axi-dac: add ad3552r axi variant

[David Lechner]

On 10/15/24 2:44 AM, Angelo Dureghello wrote:
> On 14.10.2024 16:13, David Lechner wrote:
>> On 10/14/24 5:08 AM, Angelo Dureghello wrote:
>>> From: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
>>>
>>> Add a new compatible and related bindigns for the fpga-based
>>> "ad3552r" AXI IP core, a variant of the generic AXI DAC IP.
>>>
>>> The AXI "ad3552r" IP is a very similar HDL (fpga) variant of the
>>> generic AXI "DAC" IP, intended to control ad3552r and similar chips,
>>> mainly to reach high speed transfer rates using a QSPI DDR
>>> (dobule-data-rate) interface.
>>>
>>> The ad3552r device is defined as a child of the AXI DAC, that in
>>> this case is acting as an SPI controller.
>>>
>>> Note, #io-backend is present because it is possible (in theory anyway)
>>> to use a separate controller for the control path than that used
>>> for the datapath.
>>>
>>> Signed-off-by: Angelo Dureghello <adureghello@xxxxxxxxxxxx>
>>> ---
>>>  .../devicetree/bindings/iio/dac/adi,axi-dac.yaml   | 56 ++++++++++++++++++++--
>>>  1 file changed, 53 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
>>> index a55e9bfc66d7..2b7e16717219 100644
>>> --- a/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
>>> @@ -19,11 +19,13 @@ description: |
>>>    memory via DMA into the DAC.
>>>  
>>>    https://wiki.analog.com/resources/fpga/docs/axi_dac_ip
>>> +  https://analogdevicesinc.github.io/hdl/library/axi_ad3552r/index.html
>>>  
>>>  properties:
>>>    compatible:
>>>      enum:
>>>        - adi,axi-dac-9.1.b
>>> +      - adi,axi-ad3552r
>>>  
>>>    reg:
>>>      maxItems: 1
>>> @@ -36,7 +38,14 @@ properties:
>>>        - const: tx
>>>  
>>>    clocks:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 2
>>> +
>>> +  clock-names:
>>> +    minItems: 1
>>> +    items:
>>> +      - const: s_axi_aclk
>>> +      - const: dac_clk
>>>  
>>>    '#io-backend-cells':
>>>      const: 0
>>> @@ -47,7 +56,16 @@ required:
>>>    - reg
>>>    - clocks
>>>  
>>> -additionalProperties: false
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: adi,axi-ad3552r
>>> +    then:
>>> +      $ref: /schemas/spi/spi-controller.yaml#
>>   +      properties:
>>   +        clocks:
>>   +          minItems: 2
>>   +        clock-names:
>>   +          minItems: 2
>>   +      required:
>>   +        clock-names
>>   +    else:
>>   +      properties:
>>   +        clocks:
>>   +          maxItems: 1
>>   +        clock-names:
>>   +          maxItems: 1
>>
>> We could make the checking of clocks more strict to show
>> the intent:
>>
>> adi,axi-dac-9.1.b only has 1 clock and clock-names is optional.
>>
>> adi,axi-ad3552r always has 2 clocks and clock-names is required.
>>
> is this really necessary ? At v.6 would not fix things
> not reallyh necessary.
>  
It is just a suggestion from me. I will leave it to the maintainers
to say if it is necessary or not. (If they don't say anything, then
we'll take it to mean it isn't necessary.)