| Message ID | 20241014-wip-bl-ad3552r-axi-v0-iio-testing-v6-2-eeef0c1e0e56@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: add support for the ad3552r AXI DAC IP | expand |
On Mon, 14 Oct 2024 12:08:08 +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 a QSPI DDR > (dobule-data-rate) interface. > > The ad3552r device is defined as a child of the AXI DAC, that in > this case is acting as an SPI controller. > > Note, #io-backend is present because it is possible (in theory anyway) > to use a separate controller for the control path than that used > for the datapath. > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > --- > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++-- > 1 file changed, 53 insertions(+), 3 deletions(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.example.dtb: dac@0: spi-max-frequency: 66000000 is greater than the maximum of 30000000 from schema $id: http://devicetree.org/schemas/iio/dac/adi,ad3552r.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241014-wip-bl-ad3552r-axi-v0-iio-testing-v6-2-eeef0c1e0e56@baylibre.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Mon, Oct 14, 2024 at 06:21:02AM -0500, Rob Herring (Arm) wrote: > > On Mon, 14 Oct 2024 12:08:08 +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 a QSPI DDR > > (dobule-data-rate) interface. > > > > The ad3552r device is defined as a child of the AXI DAC, that in > > this case is acting as an SPI controller. > > > > Note, #io-backend is present because it is possible (in theory anyway) > > to use a separate controller for the control path than that used > > for the datapath. > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > --- > > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++-- > > 1 file changed, 53 insertions(+), 3 deletions(-) > > > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.example.dtb: dac@0: spi-max-frequency: 66000000 is greater than the maximum of 30000000 > from schema $id: http://devicetree.org/schemas/iio/dac/adi,ad3552r.yaml# This is at least the third time this issue has been reported. Don't send more versions until you fix it. Rob
Hi Rob, On 14.10.2024 08:38, Rob Herring wrote: > On Mon, Oct 14, 2024 at 06:21:02AM -0500, Rob Herring (Arm) wrote: > > > > On Mon, 14 Oct 2024 12:08:08 +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 a QSPI DDR > > > (dobule-data-rate) interface. > > > > > > The ad3552r device is defined as a child of the AXI DAC, that in > > > this case is acting as an SPI controller. > > > > > > Note, #io-backend is present because it is possible (in theory anyway) > > > to use a separate controller for the control path than that used > > > for the datapath. > > > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > > --- > > > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++-- > > > 1 file changed, 53 insertions(+), 3 deletions(-) > > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > yamllint warnings/errors: > > > > dtschema/dtc warnings/errors: > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.example.dtb: dac@0: spi-max-frequency: 66000000 is greater than the maximum of 30000000 > > from schema $id: http://devicetree.org/schemas/iio/dac/adi,ad3552r.yaml# > > This is at least the third time this issue has been reported. Don't send > more versions until you fix it. > as stated in the patch message, this patch applies to linux-iio testing, where there are no errors, from my tests. Error is due to the spi-max-frequency fix already applied in iio testing, but still not where your bot is testing, proably in mainline. Regards, angelo > Rob
On Mon, 14 Oct 2024 16:04:35 +0200 Angelo Dureghello <adureghello@baylibre.com> wrote: > Hi Rob, > > On 14.10.2024 08:38, Rob Herring wrote: > > On Mon, Oct 14, 2024 at 06:21:02AM -0500, Rob Herring (Arm) wrote: > > > > > > On Mon, 14 Oct 2024 12:08:08 +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 a QSPI DDR > > > > (dobule-data-rate) interface. > > > > > > > > The ad3552r device is defined as a child of the AXI DAC, that in > > > > this case is acting as an SPI controller. > > > > > > > > Note, #io-backend is present because it is possible (in theory anyway) > > > > to use a separate controller for the control path than that used > > > > for the datapath. > > > > > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > > > --- > > > > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++-- > > > > 1 file changed, 53 insertions(+), 3 deletions(-) > > > > > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > > > yamllint warnings/errors: > > > > > > dtschema/dtc warnings/errors: > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.example.dtb: dac@0: spi-max-frequency: 66000000 is greater than the maximum of 30000000 > > > from schema $id: http://devicetree.org/schemas/iio/dac/adi,ad3552r.yaml# > > > > This is at least the third time this issue has been reported. Don't send > > more versions until you fix it. > > > > as stated in the patch message, this patch applies to linux-iio testing, > where there are no errors, from my tests. > > Error is due to the spi-max-frequency fix already applied in iio testing, > but still not where your bot is testing, proably in mainline. Whilst it's a fix, given the fix broadens the accepted range and doesn't matter until this patch (which will behind it) I currently have no intention of sending that fix until next merge window. Cynic in me says just change the example to a value under the old limit and bot will be happy. Example is just that, so doesn't have to reflect the maximum possible or even what people commonly run. Or include that patch again in this series with a note to say it's just here to ensure the base is correct for the bots. Jonathan > > Regards, > angelo > > > Rob
On 14.10.2024 20:20, Jonathan Cameron wrote: > On Mon, 14 Oct 2024 16:04:35 +0200 > Angelo Dureghello <adureghello@baylibre.com> wrote: > > > Hi Rob, > > > > On 14.10.2024 08:38, Rob Herring wrote: > > > On Mon, Oct 14, 2024 at 06:21:02AM -0500, Rob Herring (Arm) wrote: > > > > > > > > On Mon, 14 Oct 2024 12:08:08 +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 a QSPI DDR > > > > > (dobule-data-rate) interface. > > > > > > > > > > The ad3552r device is defined as a child of the AXI DAC, that in > > > > > this case is acting as an SPI controller. > > > > > > > > > > Note, #io-backend is present because it is possible (in theory anyway) > > > > > to use a separate controller for the control path than that used > > > > > for the datapath. > > > > > > > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > > > > --- > > > > > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++-- > > > > > 1 file changed, 53 insertions(+), 3 deletions(-) > > > > > > > > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > > > > > yamllint warnings/errors: > > > > > > > > dtschema/dtc warnings/errors: > > > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.example.dtb: dac@0: spi-max-frequency: 66000000 is greater than the maximum of 30000000 > > > > from schema $id: http://devicetree.org/schemas/iio/dac/adi,ad3552r.yaml# > > > > > > This is at least the third time this issue has been reported. Don't send > > > more versions until you fix it. > > > > > > > as stated in the patch message, this patch applies to linux-iio testing, > > where there are no errors, from my tests. > > > > Error is due to the spi-max-frequency fix already applied in iio testing, > > but still not where your bot is testing, proably in mainline. > > Whilst it's a fix, given the fix broadens the accepted range and doesn't matter > until this patch (which will behind it) I currently have no intention > of sending that fix until next merge window. > > Cynic in me says just change the example to a value under the old limit > and bot will be happy. Example is just that, so doesn't have to reflect > the maximum possible or even what people commonly run. > > Or include that patch again in this series with a note to say it's > just here to ensure the base is correct for the bots. > > Jonathan ack, if a next version is necessry, will do that. Regards, angelo > > > > > Regards, > > angelo > > > > > Rob >
On 10/14/24 5:08 AM, 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 a QSPI DDR > (dobule-data-rate) interface. > > The ad3552r device is defined as a child of the AXI DAC, that in > this case is acting as an SPI controller. > > Note, #io-backend is present because it is possible (in theory anyway) > to use a separate controller for the control path than that used > for the datapath. > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > --- > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++-- > 1 file changed, 53 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..2b7e16717219 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 > @@ -36,7 +38,14 @@ properties: > - const: tx > > clocks: > - maxItems: 1 > + minItems: 1 > + maxItems: 2 > + > + clock-names: > + minItems: 1 > + items: > + - const: s_axi_aclk > + - const: dac_clk > > '#io-backend-cells': > const: 0 > @@ -47,7 +56,16 @@ required: > - reg > - clocks > > -additionalProperties: false > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: adi,axi-ad3552r > + then: > + $ref: /schemas/spi/spi-controller.yaml# + properties: + clocks: + minItems: 2 + clock-names: + minItems: 2 + required: + clock-names + else: + properties: + clocks: + maxItems: 1 + clock-names: + maxItems: 1 We could make the checking of clocks more strict to show the intent: adi,axi-dac-9.1.b only has 1 clock and clock-names is optional. adi,axi-ad3552r always has 2 clocks and clock-names is required. > + > +unevaluatedProperties: false > > examples: > - | > @@ -57,6 +75,38 @@ examples: > dmas = <&tx_dma 0>; > dma-names = "tx"; > #io-backend-cells = <0>; > - clocks = <&axi_clk>; > + clocks = <&clkc 15>; > + clock-names = "s_axi_aclk"; > + }; > + > + - | > + #include <dt-bindings/gpio/gpio.h> > + axi_dac: spi@44a70000 { > + compatible = "adi,axi-ad3552r"; > + reg = <0x44a70000 0x1000>; > + dmas = <&dac_tx_dma 0>; > + dma-names = "tx"; > + #io-backend-cells = <0>; > + clocks = <&clkc 15>, <&ref_clk>; > + clock-names = "s_axi_aclk", "dac_clk"; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + dac@0 { > + compatible = "adi,ad3552r"; > + reg = <0>; > + reset-gpios = <&gpio0 92 GPIO_ACTIVE_HIGH>; > + io-backends = <&axi_dac>; > + spi-max-frequency = <66000000>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + channel@0 { > + reg = <0>; > + adi,output-range-microvolt = <(-10000000) (10000000)>; > + }; > + }; > }; > ... >
On 14.10.2024 16:13, David Lechner wrote: > On 10/14/24 5:08 AM, 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 a QSPI DDR > > (dobule-data-rate) interface. > > > > The ad3552r device is defined as a child of the AXI DAC, that in > > this case is acting as an SPI controller. > > > > Note, #io-backend is present because it is possible (in theory anyway) > > to use a separate controller for the control path than that used > > for the datapath. > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > --- > > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++-- > > 1 file changed, 53 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..2b7e16717219 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 > > @@ -36,7 +38,14 @@ properties: > > - const: tx > > > > clocks: > > - maxItems: 1 > > + minItems: 1 > > + maxItems: 2 > > + > > + clock-names: > > + minItems: 1 > > + items: > > + - const: s_axi_aclk > > + - const: dac_clk > > > > '#io-backend-cells': > > const: 0 > > @@ -47,7 +56,16 @@ required: > > - reg > > - clocks > > > > -additionalProperties: false > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: adi,axi-ad3552r > > + then: > > + $ref: /schemas/spi/spi-controller.yaml# > + properties: > + clocks: > + minItems: 2 > + clock-names: > + minItems: 2 > + required: > + clock-names > + else: > + properties: > + clocks: > + maxItems: 1 > + clock-names: > + maxItems: 1 > > We could make the checking of clocks more strict to show > the intent: > > adi,axi-dac-9.1.b only has 1 clock and clock-names is optional. > > adi,axi-ad3552r always has 2 clocks and clock-names is required. > is this really necessary ? At v.6 would not fix things not reallyh necessary. > > + > > +unevaluatedProperties: false > > > > examples: > > - | > > @@ -57,6 +75,38 @@ examples: > > dmas = <&tx_dma 0>; > > dma-names = "tx"; > > #io-backend-cells = <0>; > > - clocks = <&axi_clk>; > > + clocks = <&clkc 15>; > > + clock-names = "s_axi_aclk"; > > + }; > > + > > + - | > > + #include <dt-bindings/gpio/gpio.h> > > + axi_dac: spi@44a70000 { > > + compatible = "adi,axi-ad3552r"; > > + reg = <0x44a70000 0x1000>; > > + dmas = <&dac_tx_dma 0>; > > + dma-names = "tx"; > > + #io-backend-cells = <0>; > > + clocks = <&clkc 15>, <&ref_clk>; > > + clock-names = "s_axi_aclk", "dac_clk"; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + dac@0 { > > + compatible = "adi,ad3552r"; > > + reg = <0>; > > + reset-gpios = <&gpio0 92 GPIO_ACTIVE_HIGH>; > > + io-backends = <&axi_dac>; > > + spi-max-frequency = <66000000>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + channel@0 { > > + reg = <0>; > > + adi,output-range-microvolt = <(-10000000) (10000000)>; > > + }; > > + }; > > }; > > ... > > > Regards, Angelo
On 10/15/24 2:44 AM, Angelo Dureghello wrote: > On 14.10.2024 16:13, David Lechner wrote: >> On 10/14/24 5:08 AM, 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 a QSPI DDR >>> (dobule-data-rate) interface. >>> >>> The ad3552r device is defined as a child of the AXI DAC, that in >>> this case is acting as an SPI controller. >>> >>> Note, #io-backend is present because it is possible (in theory anyway) >>> to use a separate controller for the control path than that used >>> for the datapath. >>> >>> Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> >>> --- >>> .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 ++++++++++++++++++++-- >>> 1 file changed, 53 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..2b7e16717219 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 >>> @@ -36,7 +38,14 @@ properties: >>> - const: tx >>> >>> clocks: >>> - maxItems: 1 >>> + minItems: 1 >>> + maxItems: 2 >>> + >>> + clock-names: >>> + minItems: 1 >>> + items: >>> + - const: s_axi_aclk >>> + - const: dac_clk >>> >>> '#io-backend-cells': >>> const: 0 >>> @@ -47,7 +56,16 @@ required: >>> - reg >>> - clocks >>> >>> -additionalProperties: false >>> +allOf: >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: adi,axi-ad3552r >>> + then: >>> + $ref: /schemas/spi/spi-controller.yaml# >> + properties: >> + clocks: >> + minItems: 2 >> + clock-names: >> + minItems: 2 >> + required: >> + clock-names >> + else: >> + properties: >> + clocks: >> + maxItems: 1 >> + clock-names: >> + maxItems: 1 >> >> We could make the checking of clocks more strict to show >> the intent: >> >> adi,axi-dac-9.1.b only has 1 clock and clock-names is optional. >> >> adi,axi-ad3552r always has 2 clocks and clock-names is required. >> > is this really necessary ? At v.6 would not fix things > not reallyh necessary. > It is just a suggestion from me. I will leave it to the maintainers to say if it is necessary or not. (If they don't say anything, then we'll take it to mean it isn't necessary.)
On Tue, 2024-10-15 at 09:40 -0500, David Lechner wrote: > On 10/15/24 2:44 AM, Angelo Dureghello wrote: > > On 14.10.2024 16:13, David Lechner wrote: > > > On 10/14/24 5:08 AM, 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 a QSPI DDR > > > > (dobule-data-rate) interface. > > > > > > > > The ad3552r device is defined as a child of the AXI DAC, that in > > > > this case is acting as an SPI controller. > > > > > > > > Note, #io-backend is present because it is possible (in theory anyway) > > > > to use a separate controller for the control path than that used > > > > for the datapath. > > > > > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > > > --- > > > > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 > > > > ++++++++++++++++++++-- > > > > 1 file changed, 53 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..2b7e16717219 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 > > > > @@ -36,7 +38,14 @@ properties: > > > > - const: tx > > > > > > > > clocks: > > > > - maxItems: 1 > > > > + minItems: 1 > > > > + maxItems: 2 > > > > + > > > > + clock-names: > > > > + minItems: 1 > > > > + items: > > > > + - const: s_axi_aclk > > > > + - const: dac_clk > > > > > > > > '#io-backend-cells': > > > > const: 0 > > > > @@ -47,7 +56,16 @@ required: > > > > - reg > > > > - clocks > > > > > > > > -additionalProperties: false > > > > +allOf: > > > > + - if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + const: adi,axi-ad3552r > > > > + then: > > > > + $ref: /schemas/spi/spi-controller.yaml# > > > + properties: > > > + clocks: > > > + minItems: 2 > > > + clock-names: > > > + minItems: 2 > > > + required: > > > + clock-names > > > + else: > > > + properties: > > > + clocks: > > > + maxItems: 1 > > > + clock-names: > > > + maxItems: 1 > > > > > > We could make the checking of clocks more strict to show > > > the intent: > > > > > > adi,axi-dac-9.1.b only has 1 clock and clock-names is optional. > > > > > > adi,axi-ad3552r always has 2 clocks and clock-names is required. > > > > > is this really necessary ? At v.6 would not fix things > > not reallyh necessary. > > > It is just a suggestion from me. I will leave it to the maintainers > to say if it is necessary or not. (If they don't say anything, then > we'll take it to mean it isn't necessary.) > Not a DT maintainer but IMHO, having these kind of checks in the bindings is very useful. - Nuno Sá
On 15.10.2024 16:51, Nuno Sá wrote: > On Tue, 2024-10-15 at 09:40 -0500, David Lechner wrote: > > On 10/15/24 2:44 AM, Angelo Dureghello wrote: > > > On 14.10.2024 16:13, David Lechner wrote: > > > > On 10/14/24 5:08 AM, 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 a QSPI DDR > > > > > (dobule-data-rate) interface. > > > > > > > > > > The ad3552r device is defined as a child of the AXI DAC, that in > > > > > this case is acting as an SPI controller. > > > > > > > > > > Note, #io-backend is present because it is possible (in theory anyway) > > > > > to use a separate controller for the control path than that used > > > > > for the datapath. > > > > > > > > > > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > > > > > --- > > > > > .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 56 > > > > > ++++++++++++++++++++-- > > > > > 1 file changed, 53 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..2b7e16717219 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 > > > > > @@ -36,7 +38,14 @@ properties: > > > > > - const: tx > > > > > > > > > > clocks: > > > > > - maxItems: 1 > > > > > + minItems: 1 > > > > > + maxItems: 2 > > > > > + > > > > > + clock-names: > > > > > + minItems: 1 > > > > > + items: > > > > > + - const: s_axi_aclk > > > > > + - const: dac_clk > > > > > > > > > > '#io-backend-cells': > > > > > const: 0 > > > > > @@ -47,7 +56,16 @@ required: > > > > > - reg > > > > > - clocks > > > > > > > > > > -additionalProperties: false > > > > > +allOf: > > > > > + - if: > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + const: adi,axi-ad3552r > > > > > + then: > > > > > + $ref: /schemas/spi/spi-controller.yaml# > > > > + properties: > > > > + clocks: > > > > + minItems: 2 > > > > + clock-names: > > > > + minItems: 2 > > > > + required: > > > > + clock-names > > > > + else: > > > > + properties: > > > > + clocks: > > > > + maxItems: 1 > > > > + clock-names: > > > > + maxItems: 1 > > > > > > > > We could make the checking of clocks more strict to show > > > > the intent: > > > > > > > > adi,axi-dac-9.1.b only has 1 clock and clock-names is optional. > > > > > > > > adi,axi-ad3552r always has 2 clocks and clock-names is required. > > > > > > > is this really necessary ? At v.6 would not fix things > > > not reallyh necessary. > > > > > It is just a suggestion from me. I will leave it to the maintainers > > to say if it is necessary or not. (If they don't say anything, then > > we'll take it to mean it isn't necessary.) > > > > Not a DT maintainer but IMHO, having these kind of checks in the bindings is very > useful. > added the above checks, but they are producing errors. I propose this: ... clocks: minItems: 1 maxItems: 2 clock-names: minItems: 1 maxItems: 2 '#io-backend-cells': const: 0 required: - compatible - dmas - reg - clocks allOf: - if: properties: compatible: contains: const: adi,axi-ad3552r then: $ref: /schemas/spi/spi-controller.yaml# properties: clocks: minItems: 2 maxItems: 2 clock-names: items: - const: s_axi_aclk - const: dac_clk else: properties: clocks: maxItems: 1 clock-names: items: - const: s_axi_aclk unevaluatedProperties: false examples: ... Keeping clock-names not required, for backward compatibility. Regards, Angelo > - Nuno Sá
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml index a55e9bfc66d7..2b7e16717219 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 @@ -36,7 +38,14 @@ properties: - const: tx clocks: - maxItems: 1 + minItems: 1 + maxItems: 2 + + clock-names: + minItems: 1 + items: + - const: s_axi_aclk + - const: dac_clk '#io-backend-cells': const: 0 @@ -47,7 +56,16 @@ required: - reg - clocks -additionalProperties: false +allOf: + - if: + properties: + compatible: + contains: + const: adi,axi-ad3552r + then: + $ref: /schemas/spi/spi-controller.yaml# + +unevaluatedProperties: false examples: - | @@ -57,6 +75,38 @@ examples: dmas = <&tx_dma 0>; dma-names = "tx"; #io-backend-cells = <0>; - clocks = <&axi_clk>; + clocks = <&clkc 15>; + clock-names = "s_axi_aclk"; + }; + + - | + #include <dt-bindings/gpio/gpio.h> + axi_dac: spi@44a70000 { + compatible = "adi,axi-ad3552r"; + reg = <0x44a70000 0x1000>; + dmas = <&dac_tx_dma 0>; + dma-names = "tx"; + #io-backend-cells = <0>; + clocks = <&clkc 15>, <&ref_clk>; + clock-names = "s_axi_aclk", "dac_clk"; + + #address-cells = <1>; + #size-cells = <0>; + + dac@0 { + compatible = "adi,ad3552r"; + reg = <0>; + reset-gpios = <&gpio0 92 GPIO_ACTIVE_HIGH>; + io-backends = <&axi_dac>; + spi-max-frequency = <66000000>; + + #address-cells = <1>; + #size-cells = <0>; + + channel@0 { + reg = <0>; + adi,output-range-microvolt = <(-10000000) (10000000)>; + }; + }; }; ...