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@xxxxxxxxxxxx> >>>>> >>>>> 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@xxxxxxxxxxxx> >>>>> --- >>>>> .../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@44a70000 { >>>>> + 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
