Re: [PATCH 1/2] dt-bindings: remoteproc: Add bindings for new Ingenic SoCs.

From: Zhou Yanjie
Date: Sat Jul 24 2021 - 09:06:35 EST


Hi Paul,

On 2021/7/24 下午7:02, Paul Cercueil wrote:
Hi Zhou,

Le sam., juil. 24 2021 at 17:11:37 +0800, 周琰杰 (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx> a écrit :
Add the remoteproc bindings for the JZ4760 SoC, the JZ4760B SoC,
the JZ4775 SoC, and the JZ4780 SoC from Ingenic.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx>
---
 .../bindings/remoteproc/ingenic,vpu.yaml           | 74 ++++++++++++++++------
 1 file changed, 56 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml b/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml
index d0aa91b..6154596 100644
--- a/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/ingenic,vpu.yaml
@@ -17,31 +17,52 @@ maintainers:

 properties:
   compatible:
-    const: ingenic,jz4770-vpu-rproc
+    enum:
+      - ingenic,jz4760-vpu-rproc
+      - ingenic,jz4760b-vpu-rproc
+      - ingenic,jz4770-vpu-rproc
+      - ingenic,jz4775-vpu-rproc
+      - ingenic,jz4780-vpu-rproc

   reg:
-    items:
-      - description: aux registers
-      - description: tcsm0 registers
-      - description: tcsm1 registers
-      - description: sram registers
+    oneOf:
+      - items:
+          - description: aux registers
+          - description: tcsm0 registers
+          - description: tcsm1 registers
+          - description: sram registers
+      - items:
+          - description: aux registers
+          - description: tcsm registers
+          - description: sram registers

Since we have "reg-names" already, we don't really need any description, so you could just have:

reg:
 minItems: 3
 maxItems: 4


Sure, I will do it in the next version.




   reg-names:
-    items:
-      - const: aux
-      - const: tcsm0
-      - const: tcsm1
-      - const: sram
+    oneOf:
+      - items:
+          - const: aux
+          - const: tcsm0
+          - const: tcsm1
+          - const: sram
+      - items:
+          - const: aux
+          - const: tcsm
+          - const: sram

You could just add "tcsm" to the items list, and add:
minItems: 3
maxItems: 4


Sure.




   clocks:
-    items:
-      - description: aux clock
-      - description: vpu clock
+    oneOf:
+      - items:
+          - description: aux clock
+          - description: vpu clock
+      - items:
+          - description: vpu clock

Same as above, since we already have clock-names, the descriptions don't bring much.

You can replace with:

clocks:
 minItems: 1
 maxItems: 2


Sure.




   clock-names:
-    items:
-      - const: aux
-      - const: vpu
+    oneOf:
+      - items:
+          - const: aux
+          - const: vpu
+      - items:
+          - const: vpu

I think you could just add:
minItems: 1


Sure.


Thanks and best regards!



Cheers,
-Paul


   interrupts:
     maxItems: 1
@@ -60,7 +81,7 @@ examples:
   - |
     #include <dt-bindings/clock/jz4770-cgu.h>

-    vpu: video-decoder@132a0000 {
+    video-decoder@132a0000 {
       compatible = "ingenic,jz4770-vpu-rproc";

       reg = <0x132a0000 0x20>, /* AUX */
@@ -75,3 +96,20 @@ examples:
       interrupt-parent = <&cpuintc>;
       interrupts = <3>;
     };
+  - |
+    #include <dt-bindings/clock/jz4780-cgu.h>
+
+    video-decoder@132a0000 {
+      compatible = "ingenic,jz4780-vpu-rproc";
+
+      reg = <0x132a0000 0x20>, /* AUX */
+            <0x132c0000 0x8000>, /* TCSM */
+            <0x132f0000 0x4000>; /* SRAM */
+      reg-names = "aux", "tcsm", "sram";
+
+      clocks = <&cgu JZ4780_CLK_VPU>;
+      clock-names = "vpu";
+
+      interrupt-parent = <&intc>;
+      interrupts = <62>;
+    };
--
2.7.4