[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <sowmuxfsedwdshyothf7jc6mcrbzqbs2vzw7x4p3tg3iqnlnjt@5qa3kazkce46>
Date: Mon, 30 Sep 2024 14:52:10 +0200
From: Angelo Dureghello <adureghello@...libre.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Lars-Peter Clausen <lars@...afoo.de>,
Michael Hennerich <Michael.Hennerich@...log.com>, Nuno Sa <nuno.sa@...log.com>, 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, Mark Brown <broonie@...nel.org>,
linux-spi@...r.kernel.org
Subject: Re: [PATCH v3 02/10] dt-bindings: iio: dac: axi-dac: add ad3552r axi
variant
On 29.09.2024 11:46, Jonathan Cameron wrote:
> On Thu, 19 Sep 2024 11:19:58 +0200
> Angelo Dureghello <adureghello@...libre.com> wrote:
>
> > From: Angelo Dureghello <adureghello@...libre.com>
> >
> > 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 an additional QSPI
>
> I'd drop the word additional as I assume it is an 'either/or' situation
> for the interfaces.
>
> Do we have other devices using this same IP? I.e. does it make
> sense to provide a more generic compatible as a fallback for this one
> so that other devices would work without the need for explicit support?
>
>
no, actually ad3552r-axi is only interfacing to ad3552r.
I could eventually set adi,axi-dac-9.1.b as a fallback, since it
is the "gneric" AXI implementation.
> I'd also ideally like a view point from Mark Brown as SPI maintainer
> on how we should deal with this highly specialized spi controller.
> Is he happy with us using an SPI like binding but not figuring out how
> to fit this engine into the SPI subsystem.
>
> Please +CC Mark and the spi list (done here) on future versions + provide
> a clear description of what is going on for them.
>
Ok.
Actually i fixed the bindings for v4 setting axi-ad3552r as an
spi-controller, and the target ad3552r as a spi-peripheral (child node).
This axi-ad3552r is not only a pure spi-controller since providing
some synchronization features not typical of a spi-controller.
> Maybe with the binding fixed as spi compliant, we can figure out the
> if we eventually want to treat this as an SPI controller from the
> kernel driver point of view even if we initially do something 'special'.
>
> Jonathan
>
>
> > DDR interface.
> >
> > The ad3552r device is defined as a child of the AXI DAC, that in
> > this case is acting as an SPI controller.
> >
> > Signed-off-by: Angelo Dureghello <adureghello@...libre.com>
> > ---
> > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 40 ++++++++++++++++++++--
> > 1 file changed, 37 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..6cf0c2cb84e7 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
> > @@ -41,22 +43,54 @@ properties:
> > '#io-backend-cells':
> > const: 0
> >
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > required:
> > - compatible
> > - dmas
> > - reg
> > - clocks
> >
> > +patternProperties:
> > + "^.*@([0-9])$":
> > + type: object
> > + additionalProperties: true
> > + properties:
> > + io-backends:
> > + description: |
> > + AXI backend reference
> > + required:
> > + - io-backends
> > +
> > additionalProperties: false
> >
> > examples:
> > - |
> > dac@...00000 {
> > - compatible = "adi,axi-dac-9.1.b";
> > - reg = <0x44a00000 0x10000>;
> > - dmas = <&tx_dma 0>;
> > + compatible = "adi,axi-dac-9.1.b";
> > + reg = <0x44a00000 0x10000>;
> > + dmas = <&tx_dma 0>;
>
> If it makes sense to reformat then separate patch
> please as this is hard to read as a result of this
> change. Also, as pointed out, be consistent with spacing.
>
> > + dma-names = "tx";
> > + #io-backend-cells = <0>;
> > + clocks = <&axi_clk>;
> > + };
> > +
> > + - |
> > + axi_dac: spi@...70000 {
> > + compatible = "adi,axi-ad3552r";
> > + reg = <0x44a70000 0x1000>;
> > + dmas = <&dac_tx_dma 0>;
> > dma-names = "tx";
> > #io-backend-cells = <0>;
> > clocks = <&axi_clk>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + /* DAC devices */
> > };
> > ...
> >
>
--
o/ QW5nZWxvIER1cmVnaGVsbG8=
www.kernel-space.org
e: angelo at kernel-space.org
c: +39 388 8550663
Powered by blists - more mailing lists