Convert the Marvell MMP DMA binding to YAML.
The TXT binding mentions that the controller may have one IRQ per DMA
channel. Examples of this were dropped in the YAML binding because of
dt_binding_check complaints (either too many interrupt cells or
interrupts) and the fact that this is not used in any of the in-tree
device trees.
Signed-off-by: Duje Mihanović <[email protected]>
---
.../devicetree/bindings/dma/marvell,mmp-dma.yaml | 82 ++++++++++++++++++++++
Documentation/devicetree/bindings/dma/mmp-dma.txt | 81 ---------------------
2 files changed, 82 insertions(+), 81 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
new file mode 100644
index 000000000000..fe94ba9143e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/marvell,mmp-dma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell MMP DMA controller
+
+maintainers:
+ - Duje Mihanović <[email protected]>
+
+description:
+ Marvell MMP SoCs may have two types of DMA controllers, peripheral and audio.
+
+properties:
+ compatible:
+ enum:
+ - marvell,pdma-1.0
+ - marvell,adma-1.0
+ - marvell,pxa910-squ
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description:
+ Interrupt lines for the controller, may be shared or one per DMA channel
+ minItems: 1
+
+ '#dma-channels':
+ deprecated: true
+
+ '#dma-requests':
+ deprecated: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - '#dma-cells'
+
+allOf:
+ - $ref: dma-controller.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - marvell,adma-1.0
+ - marvell,pxa910-squ
+ then:
+ properties:
+ asram:
+ description:
+ phandle to the SRAM pool
+ minItems: 1
+ maxItems: 1
+ iram:
+ maxItems: 1
+
+unevaluatedProperties: false
+
+examples:
+ # Peripheral controller
+ - |
+ pdma0: dma-controller@d4000000 {
+ compatible = "marvell,pdma-1.0";
+ reg = <0xd4000000 0x10000>;
+ interrupts = <47>;
+ #dma-cells = <2>;
+ dma-channels = <16>;
+ };
+
+ # Audio controller
+ - |
+ squ: dma-controller@d42a0800 {
+ compatible = "marvell,pxa910-squ";
+ reg = <0xd42a0800 0x100>;
+ interrupts = <46>;
+ #dma-cells = <2>;
+ dma-channels = <2>;
+ };
diff --git a/Documentation/devicetree/bindings/dma/mmp-dma.txt b/Documentation/devicetree/bindings/dma/mmp-dma.txt
deleted file mode 100644
index ec18bf0a802a..000000000000
--- a/Documentation/devicetree/bindings/dma/mmp-dma.txt
+++ /dev/null
@@ -1,81 +0,0 @@
-* MARVELL MMP DMA controller
-
-Marvell Peripheral DMA Controller
-Used platforms: pxa688, pxa910, pxa3xx, etc
-
-Required properties:
-- compatible: Should be "marvell,pdma-1.0"
-- reg: Should contain DMA registers location and length.
-- interrupts: Either contain all of the per-channel DMA interrupts
- or one irq for pdma device
-
-Optional properties:
-- dma-channels: Number of DMA channels supported by the controller (defaults
- to 32 when not specified)
-- #dma-channels: deprecated
-- dma-requests: Number of DMA requestor lines supported by the controller
- (defaults to 32 when not specified)
-- #dma-requests: deprecated
-
-"marvell,pdma-1.0"
-Used platforms: pxa25x, pxa27x, pxa3xx, pxa93x, pxa168, pxa910, pxa688.
-
-Examples:
-
-/*
- * Each channel has specific irq
- * ICU parse out irq channel from ICU register,
- * while DMA controller may not able to distinguish the irq channel
- * Using this method, interrupt-parent is required as demuxer
- * For example, pxa688 icu register 0x128, bit 0~15 is PDMA channel irq,
- * 18~21 is ADMA irq
- */
-pdma: dma-controller@d4000000 {
- compatible = "marvell,pdma-1.0";
- reg = <0xd4000000 0x10000>;
- interrupts = <0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15>;
- interrupt-parent = <&intcmux32>;
- dma-channels = <16>;
- };
-
-/*
- * One irq for all channels
- * Dmaengine driver (DMA controller) distinguish irq channel via
- * parsing internal register
- */
-pdma: dma-controller@d4000000 {
- compatible = "marvell,pdma-1.0";
- reg = <0xd4000000 0x10000>;
- interrupts = <47>;
- dma-channels = <16>;
- };
-
-
-Marvell Two Channel DMA Controller used specifically for audio
-Used platforms: pxa688, pxa910
-
-Required properties:
-- compatible: Should be "marvell,adma-1.0" or "marvell,pxa910-squ"
-- reg: Should contain DMA registers location and length.
-- interrupts: Either contain all of the per-channel DMA interrupts
- or one irq for dma device
-
-"marvell,adma-1.0" used on pxa688
-"marvell,pxa910-squ" used on pxa910
-
-Examples:
-
-/* each channel has specific irq */
-adma0: dma-controller@d42a0800 {
- compatible = "marvell,adma-1.0";
- reg = <0xd42a0800 0x100>;
- interrupts = <18 19>;
- interrupt-parent = <&intcmux32>;
- };
-
-/* One irq for all channels */
-squ: dma-controller@d42a0800 {
- compatible = "marvell,pxa910-squ";
- reg = <0xd42a0800 0x100>;
- interrupts = <46>;
- };
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240125-pxa-dma-yaml-0b62e10824f4
Best regards,
--
Duje Mihanović <[email protected]>
Hey,
On Sat, Jan 27, 2024 at 05:53:45PM +0100, Duje Mihanović wrote:
> Convert the Marvell MMP DMA binding to YAML.
>
> The TXT binding mentions that the controller may have one IRQ per DMA
> channel. Examples of this were dropped in the YAML binding because of
> dt_binding_check complaints (either too many interrupt cells or
> interrupts) and the fact that this is not used in any of the in-tree
> device trees.
>
> Signed-off-by: Duje Mihanović <[email protected]>
> ---
> .../devicetree/bindings/dma/marvell,mmp-dma.yaml | 82 ++++++++++++++++++++++
> Documentation/devicetree/bindings/dma/mmp-dma.txt | 81 ---------------------
> 2 files changed, 82 insertions(+), 81 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
> new file mode 100644
> index 000000000000..fe94ba9143e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dma/marvell,mmp-dma.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell MMP DMA controller
> +
> +maintainers:
> + - Duje Mihanović <[email protected]>
> +
> +description:
> + Marvell MMP SoCs may have two types of DMA controllers, peripheral and audio.
> +
> +properties:
> + compatible:
> + enum:
> + - marvell,pdma-1.0
> + - marvell,adma-1.0
> + - marvell,pxa910-squ
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + description:
> + Interrupt lines for the controller, may be shared or one per DMA channel
> + minItems: 1
> +
> + '#dma-channels':
> + deprecated: true
> +
> + '#dma-requests':
> + deprecated: true
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - '#dma-cells'
> +
> +allOf:
> + - $ref: dma-controller.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - marvell,adma-1.0
> + - marvell,pxa910-squ
> + then:
> + properties:
> + asram:
> + description:
> + phandle to the SRAM pool
> + minItems: 1
> + maxItems: 1
> + iram:
> + maxItems:
These properties are not mentioned in the text binding, nor commit
message. Where did they come from?
That said, for properties that are only usable on some platforms, please
define them at the top level and conditionally permit/constrain them.
> +unevaluatedProperties: false
> +
> +examples:
> + # Peripheral controller
> + - |
> + pdma0: dma-controller@d4000000 {
The label is not needed here or below.
In fact, I'd probably delete the second example as it shows nothing that
the first one does not.
thanks,
Conor.
> + compatible = "marvell,pdma-1.0";
> + reg = <0xd4000000 0x10000>;
> + interrupts = <47>;
> + #dma-cells = <2>;
> + dma-channels = <16>;
> + };
> +
> + # Audio controller
> + - |
> + squ: dma-controller@d42a0800 {
> + compatible = "marvell,pxa910-squ";
> + reg = <0xd42a0800 0x100>;
> + interrupts = <46>;
> + #dma-cells = <2>;
> + dma-channels = <2>;
> + };
On Sunday, January 28, 2024 6:28:03 PM CET Conor Dooley wrote:
> On Sat, Jan 27, 2024 at 05:53:45PM +0100, Duje Mihanović wrote:
> > +allOf:
> > + - $ref: dma-controller.yaml#
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - marvell,adma-1.0
> > + - marvell,pxa910-squ
> > + then:
> > + properties:
> > + asram:
> > + description:
> > + phandle to the SRAM pool
> > + minItems: 1
> > + maxItems: 1
> > + iram:
>
> > + maxItems:
> These properties are not mentioned in the text binding, nor commit
> message. Where did they come from?
Both of them can be found in arch/arm/boot/dts/marvell/mmp2.dtsi. There is one
major difference between the two: iram is not mentioned at all by the mmp_tdma
driver (on the other hand, asram is not only used but also required for a
successful probe), but I left it here as it's still found in the MMP2 dtsi. On
second thought it should probably be dropped both here and in the dtsi.
> That said, for properties that are only usable on some platforms, please
> define them at the top level and conditionally permit/constrain them.
Could you please point me to how to do so if this if/then does not do it
properly?
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + # Peripheral controller
> > + - |
> > + pdma0: dma-controller@d4000000 {
>
> The label is not needed here or below.
> In fact, I'd probably delete the second example as it shows nothing that
> the first one does not.
I'd rather add the asram property in the second node (adding onto the above
comment, I now see that it shouldn't have even passed dt_binding_check because
of the missing asram, but it did).
Regards,
--
Duje
On Sun, Jan 28, 2024 at 07:01:36PM +0100, Duje Mihanović wrote:
> On Sunday, January 28, 2024 6:28:03 PM CET Conor Dooley wrote:
> > On Sat, Jan 27, 2024 at 05:53:45PM +0100, Duje Mihanović wrote:
> > > +allOf:
> > > + - $ref: dma-controller.yaml#
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + enum:
> > > + - marvell,adma-1.0
> > > + - marvell,pxa910-squ
> > > + then:
> > > + properties:
> > > + asram:
> > > + description:
> > > + phandle to the SRAM pool
> > > + minItems: 1
> > > + maxItems: 1
> > > + iram:
> >
> > > + maxItems:
> > These properties are not mentioned in the text binding, nor commit
> > message. Where did they come from?
>
> Both of them can be found in arch/arm/boot/dts/marvell/mmp2.dtsi. There is one
> major difference between the two: iram is not mentioned at all by the mmp_tdma
> driver (on the other hand, asram is not only used but also required for a
> successful probe), but I left it here as it's still found in the MMP2 dtsi. On
> second thought it should probably be dropped both here and in the dtsi.
>
> > That said, for properties that are only usable on some platforms, please
> > define them at the top level and conditionally permit/constrain them.
>
> Could you please point me to how to do so if this if/then does not do it
> properly?
Negate the if and then:
then:
properties:
asram: false
There are lots of examples in the tree.
>
> > > +unevaluatedProperties: false
> > > +
> > > +examples:
> > > + # Peripheral controller
> > > + - |
> > > + pdma0: dma-controller@d4000000 {
> >
> > The label is not needed here or below.
> > In fact, I'd probably delete the second example as it shows nothing that
> > the first one does not.
>
> I'd rather add the asram property in the second node (adding onto the above
> comment, I now see that it shouldn't have even passed dt_binding_check because
> of the missing asram, but it did).
It passed because 'required' is what checks for property presence and
nowhere is asram required. It is missing a type definition which should
have warned, but may not since it is under an 'if'.
Rob
On Wednesday, January 31, 2024 6:18:57 PM CET Rob Herring wrote:
> On Sun, Jan 28, 2024 at 07:01:36PM +0100, Duje Mihanović wrote:
> > On Sunday, January 28, 2024 6:28:03 PM CET Conor Dooley wrote:
> > > That said, for properties that are only usable on some platforms, please
> > > define them at the top level and conditionally permit/constrain them.
> >
> > Could you please point me to how to do so if this if/then does not do it
> > properly?
>
> Negate the if and then:
>
> then:
> properties:
> asram: false
>
> There are lots of examples in the tree.
This works as expected, thanks.
> > > > +unevaluatedProperties: false
> > > > +
> > > > +examples:
> > > > + # Peripheral controller
> > > > + - |
> > > > + pdma0: dma-controller@d4000000 {
> > >
> > > The label is not needed here or below.
> > > In fact, I'd probably delete the second example as it shows nothing that
> > > the first one does not.
> >
> > I'd rather add the asram property in the second node (adding onto the
above
> > comment, I now see that it shouldn't have even passed dt_binding_check
> > because of the missing asram, but it did).
>
> It passed because 'required' is what checks for property presence and
> nowhere is asram required. It is missing a type definition which should
> have warned, but may not since it is under an 'if'.
I thought specifying minItems would have the same effect, but that turned out
not to be the case. Also, the 'if' did indeed suppress the missing type
warning.
Regards,
--
Duje