On Wed, Jan 29, 2025 at 05:52:31PM -0500, Frank Li wrote:

On Sun, Jan 26, 2025 at 07:59:00PM +0100, J. Neuschäfer wrote:

quoted

The devicetree bindings for Freescale DMA engines have so far existed as
a text file. This patch converts them to YAML, and specifies all the
compatible strings currently in use in arch/powerpc/boot/dts.

Signed-off-by: J. Neuschäfer <redacted>
---
 .../devicetree/bindings/dma/fsl,elo-dma.yaml       | 129 +++++++++++++
 .../devicetree/bindings/dma/fsl,elo3-dma.yaml      | 105 +++++++++++
 .../devicetree/bindings/dma/fsl,eloplus-dma.yaml   | 120 ++++++++++++
 .../devicetree/bindings/powerpc/fsl/dma.txt        | 204 ---------------------
 4 files changed, 354 insertions(+), 204 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/fsl,elo-dma.yaml b/Documentation/devicetree/bindings/dma/fsl,elo-dma.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..d1f4978a672c1217c322c27f243470b2de8c99d4
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/fsl,elo-dma.yaml

@@ -0,0 +1,129 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/fsl,elo-dma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Elo DMA Controller
+
+maintainers:
+  - J. Neuschäfer <j.ne@posteo.net>
+
+description: |

needn't | here

Will remove.

quoted

+  This is a little-endian 4-channel DMA controller, used in Freescale mpc83xx
+  series chips such as mpc8315, mpc8349, mpc8379 etc.
+
+  Note on DMA channel compatible properties: The compatible property must say
+  "fsl,elo-dma-channel" or "fsl,eloplus-dma-channel" to be used by the Elo DMA

There are not 'fsl,eloplus-dma-channel' under "^dma-channel@.*$". I suggest
remove this because 'compatible': items already show such information.

Good point, I'll trim this text down.

quoted

+  driver (fsldma).  Any DMA channel used by fsldma cannot be used by another
+  DMA driver, such as the SSI sound drivers for the MPC8610.  Therefore, any
+  DMA channel that should be used for another driver should not use
+  "fsl,elo-dma-channel" or "fsl,eloplus-dma-channel".  For the SSI drivers, for
+  example, the compatible property should be "fsl,ssi-dma-channel".  See
+  ssi.txt for more information.

I noticed fsl,ssi.txt has been converted to YAML since this text was
originally written, so I'll make reference to that.

[...]

quoted

+examples:
+  - |
+    dma@82a8 {
+        #address-cells = <1>;
+        #size-cells = <1>;
+        compatible = "fsl,mpc8349-dma", "fsl,elo-dma";
+        reg = <0x82a8 4>;

compatible and reg should be first two property.

Will fix.

quoted

+        ranges = <0 0x8100 0x1a4>;
+        interrupt-parent = <&ipic>;
+        interrupts = <71 8>;
+        cell-index = <0>;

need space line here.

Will fix.

quoted

+        dma-channel@0 {
+            compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
+            cell-index = <0>;
+            reg = <0 0x80>;
+            interrupt-parent = <&ipic>;
+            interrupts = <71 8>;
+        };

need space line here. check other's example dts

Will fix in all files.


[...]

quoted

+patternProperties:
+  "^dma-channel@.*$":
+    type: object
+
+    properties:
+      compatible:
+        items:
+          - enum:
+              - fsl,mpc8540-dma-channel
+              - fsl,mpc8541-dma-channel
+              - fsl,mpc8548-dma-channel
+              - fsl,mpc8555-dma-channel
+              - fsl,mpc8560-dma-channel
+              - fsl,mpc8572-dma-channel
+          - const: fsl,eloplus-dma-channel

I think you can merge this fsl,mpc83xx-dma yaml file

+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,elo-dma
+    then:
+      patternProperties:
+        "^dma-channel@.*$":
+          properties:
+            compatible:
+              items:
+                - enum:
			....
+    else
+      patternProperties:
+        "^dma-channel@.*$":
+          properties:
+            compatible:
+              items:
+                - enum:
                        ....
+                - const: fsl,eloplus-dma-channel

I suppose that works, but I'm not entirely convinced it would help with
readability, compared to leaving the three variants separate.


Thank you for your review!

Best regards,
J. Neuschäfer