| 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
| ||
|
Message-ID: <fa27dc74-7b1f-4ef5-81dc-cc434da4ff89@kernel.org>
Date: Wed, 25 Sep 2024 09:22:10 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>,
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 24/09/2024 14:27, Nuno Sá wrote:
> On Tue, 2024-09-24 at 10:02 +0200, Krzysztof Kozlowski wrote:
>> 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?
>>
>
> The backend is a provider-consumer type of API. On the consumer side (which is the
> driver the child node will probe on), we need to call devm_iio_backend_get() to get
> the backend object (which obviously is the parent). For that, 'io-backends' is being
You described the driver, so how does it matter? Driver can call
get_backend_from_parent(), right? Or get_backend_from_fwnode(parent)?
> used. We do have another API called __devm_iio_backend_get_from_fwnode_lookup() that
> could be used with the parent fwnode and should work. However that was only added to
> keep backward compatibility in the first user of the IIO backend framework and it's
> not really meant to be used again. We are aware this is awkward at the very least [1]
> but hopefully still acceptable.
Don't use driver reasons for discussing hardware. Answer my first
question, because it is still ignored.
Best regards,
Krzysztof
Powered by blists - more mailing lists