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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e8af0f3f-a09c-42d7-b8ca-dd633539af73@kernel.org>
Date: Tue, 24 Sep 2024 10:02:37 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Angelo Dureghello <adureghello@...libre.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>,
 Nuno Sa <nuno.sa@...log.com>, Jonathan Cameron <jic23@...nel.org>,
 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,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 dlechner@...libre.com
Subject: Re: [PATCH v3 04/10] dt-bindings: iio: dac: ad3552r: add io-backend
 support

On 23/09/2024 17:50, Angelo Dureghello wrote:
> Hi Krzysztof,
> 
> On 22/09/24 23:02, Krzysztof Kozlowski wrote:
>> On Thu, Sep 19, 2024 at 11:20:00AM +0200, Angelo Dureghello 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, and acts as
>>> an SPI controller.
>>>
>>> For this case, the binding is modified to include some
>>> additional properties.
>>>
>>> Signed-off-by: Angelo Dureghello <adureghello@...libre.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.
>>> +      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.
>>> +    type: boolean
>>> +
>>>     '#address-cells':
>>>       const: 1
>>>   
>>> @@ -128,6 +140,7 @@ patternProperties:
>>>             - custom-output-range-config
>>>   
>>>   allOf:
>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>     - if:
>>>         properties:
>>>           compatible:
>>> @@ -238,4 +251,33 @@ examples:
>>>               };
>>>           };
>>>       };
>>> +
>>> +  - |
>>> +    axi_dac: spi@...70000 {
>>> +        compatible = "adi,axi-ad3552r";
>> That is either redundant or entire example should go to the parent node,
>> if this device is fixed child of complex device (IOW, adi,ad3552r cannot
>> be used outside of adi,axi-ad3552r).
> 
> ad3552r can still be used by a generic "classic" spi
> controller (SCLK/CS/MISO) but at a slower samplerate, fpga
> controller only (axi-ad3552r) can reach 33MUPS.

OK, then this is just redundant. Drop the node. Parent example should
contain the children, though.
> 
>>
>>> +        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 {
>>> +            compatible = "adi,ad3552r";
>>> +            reg = <0>;
>>> +            reset-gpios = <&gpio0 92 0>;
>> Use standard defines for GPIO flags.
> 
> fixed, thanks
> 
>>> +            io-backends = <&axi_dac>;
>> Why do you need to point to the parent? How much coupled are these
>> devices? Child pointing to parent is not usually expected, because
>> that's obvious.
> 
> 
> "io-backends" is actually the way to refer to the backend module,
> (used already for i.e. ad9739a),
> it is needed because the backend is not only acting as spi-controller,
> but is also providing some APIs for synchronization and bus setup support.


But if backend is the parent, then this is redundant. You can take it
from the child-parent relationship. Is this pointing to other devices
(non-parent) in other ad3552r configurations?

Best regards,
Krzysztof