From: Angelo Dureghello <adureghello@baylibre.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
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@baylibre.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@44a00000 {
- 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>;
+ dma-names = "tx";
+ #io-backend-cells = <0>;
+ clocks = <&axi_clk>;
+ };
+
+ - |
+ axi_dac: spi@44a70000 {
+ 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 */
};
...
--
2.45.0.rc1On Thu, 19 Sep 2024 11:19:58 +0200
Angelo Dureghello <adureghello@baylibre.com> wrote:
> From: Angelo Dureghello <adureghello@baylibre.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?
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.
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@baylibre.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@44a00000 {
> - 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@44a70000 {
> + 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 */
> };
> ...
>
On 29.09.2024 11:46, Jonathan Cameron wrote:
> On Thu, 19 Sep 2024 11:19:58 +0200
> Angelo Dureghello <adureghello@baylibre.com> wrote:
>
> > From: Angelo Dureghello <adureghello@baylibre.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@baylibre.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@44a00000 {
> > - 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@44a70000 {
> > + 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
On Mon, 2024-09-30 at 14:52 +0200, Angelo Dureghello wrote: > On 29.09.2024 11:46, Jonathan Cameron wrote: > > On Thu, 19 Sep 2024 11:19:58 +0200 > > Angelo Dureghello <adureghello@baylibre.com> wrote: > > > > > From: Angelo Dureghello <adureghello@baylibre.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. Yes but the generic IP does not have this spi bus implementation so the device would be unusable (unless I'm missing something) - Nuno Sá
On Mon, 30 Sep 2024 15:15:03 +0200 Nuno Sá <noname.nuno@gmail.com> wrote: > On Mon, 2024-09-30 at 14:52 +0200, Angelo Dureghello wrote: > > On 29.09.2024 11:46, Jonathan Cameron wrote: > > > On Thu, 19 Sep 2024 11:19:58 +0200 > > > Angelo Dureghello <adureghello@baylibre.com> wrote: > > > > > > > From: Angelo Dureghello <adureghello@baylibre.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. > > Yes but the generic IP does not have this spi bus implementation so the device > would be unusable (unless I'm missing something) Falling back to the generic IP doesn't make sense as they aren't compatible. I'd more expect some future device support that happens to need the same sort of bus support might be able to use this FPGA IP. Anyhow, it is fine to fallback to this specific compatible anyway, so lets go with this rather than trying for a generic name. Jonathan > > - Nuno Sá > > >
On Thu, Sep 19, 2024 at 11:19:58AM +0200, Angelo Dureghello wrote:
> From: Angelo Dureghello <adureghello@baylibre.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
> DDR interface.
>
> The ad3552r device is defined as a child of the AXI DAC, that in
> this case is acting as an SPI controller.
So who acts as an SPI controller? adi,axi-ad3552r? Why this is not
reflected in the binding?
>
> Signed-off-by: Angelo Dureghello <adureghello@baylibre.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])$":
No need for ()
but this is pattern is quite permissive, so looks like you want to
achieve something else. Maybe spi controller? Not sure, I am just
guessing because empty example does not help me.
> + type: object
> + additionalProperties: true
No, this cannot be true.
> + properties:
> + io-backends:
> + description: |
> + AXI backend reference
> + required:
> + - io-backends
I don't get the point. Nodes with only one property are not really
useful.
> +
> additionalProperties: false
>
> examples:
> - |
> dac@44a00000 {
> - compatible = "adi,axi-dac-9.1.b";
> - reg = <0x44a00000 0x10000>;
> - dmas = <&tx_dma 0>;
> + compatible = "adi,axi-dac-9.1.b";
> + reg = <0x44a00000 0x10000>;
Why are you changing this? It's even messier now - other example uses
different indentayion...
> + dmas = <&tx_dma 0>;
> + dma-names = "tx";
> + #io-backend-cells = <0>;
> + clocks = <&axi_clk>;
> + };
> +
> + - |
> + axi_dac: spi@44a70000 {
> + 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 */
Your schema must be complete, example as well (minus repetitive pieces).
Best regards,
Krzysztof
On Thu, 2024-09-19 at 11:19 +0200, Angelo Dureghello wrote: > From: Angelo Dureghello <adureghello@baylibre.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 > 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@baylibre.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 > + I wonder if it makes sense to have these specific bits only for the new compatible? - Nuno Sá
© 2016 - 2024 Red Hat, Inc.