On Tue, Aug 24, 2021 at 06:00:25PM +0800, Moudy Ho wrote:

If we wanted h/w indexes in DT, we'd have a standard property. Why do 
you need this?

+
+  reg:
+    description: |
+      Physical base address and length of the function block
+      register space, the number aligns with the component
+      and its own subcomponent.

Drop and add 'maxItems: 1'

+
+  mediatek,gce-client-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: |
+      sub-system id corresponding to the global command engine (GCE)

You mean 'phandle to GCE and sub-system id'?

+      register address.
+      $ref: /schemas/mailbox/mtk-gce.txt

Kind of looks like jsonschema but in the description and to a .txt 
file...

Should be indented 2 more spaces. Install yamllint and check with 'make 
dt_binding_check'.

+          # controller node
+          - mediatek,mt8183-mdp3

And then 2 more here.

+        - enum:
+          - mediatek,mt8183-mdp3-rdma
+
+      - items:
+        - enum:
+          # read DMA
+          - mediatek,mt8183-mdp3-rdma
+
+  mediatek,scp:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    maxItems: 1
+    description: |
+      The node of system control processor (SCP), using
+      the remoteproc & rpmsg framework.
+      $ref: /schemas/remoteproc/mtk,scp.yaml
+
+  mediatek,mdp3-id:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    maxItems: 1
+    description: |
+      In MDP3, it can allocate multiple identical modules for
+      different data path selection or multi-pipeline execution.
+      This node is used to indicate the ID of each module.
+
+  mdp3-comps:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    items:
+        - enum:

And this is 2 too many spaces...

+          # MDP direct-link input path selection, create a
+          # component for path connectedness of HW pipe control
+          - mediatek,mt8183-mdp3-dl1
+        - enum:
+          - mediatek,mt8183-mdp3-dl2
+        - enum:
+          # MDP direct-link output path selection, create a
+          # component for path connectedness of HW pipe control
+          - mediatek,mt8183-mdp3-path1
+        - enum:
+          - mediatek,mt8183-mdp3-path2
+        - enum:
+          # Input DMA of ISP PASS2 (DIP) module for raw image input
+          - mediatek,mt8183-mdp3-imgi
+        - enum:
+          # Output DMA of ISP PASS2 (DIP) module for YUV image output
+          - mediatek,mt8183-mdp3-exto
+
+  mdp3-comp-ids:
+    maxItems: 1
+    $ref: /schemas/types.yaml#/definitions/uint32-array

If only a single item, then it's a 'uint32' not an array.

+    description: |
+      Pipeline ID of MDP direct-link or DIP.
+
+  reg:
+    description: |
+      Physical base address and length of the function block
+      register space, the number aligns with the component
+      and its own subcomponent.
+
+  mediatek,gce-client-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: |
+      sub-system id corresponding to the global command engine (GCE)
+      register address.
+      $ref: /schemas/mailbox/mtk-gce.txt
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    maxItems: 6

Need to define what they are.

+
+  iommus:
+    $ref: /schemas/types.yaml#/definitions/phandle

iommus already has a type definition.

+    description: |
+      Should point to the respective IOMMU block with master
+      port as argument.
+      $ref: /schemas/iommu/mediatek,iommu.yaml

No. Drop (the whole description because you don't need generic 
descriptions for common properties).

What's needed is how many entries (maxItems: 1).

+
+  mediatek,mmsys:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    maxItems: 1
+    description: |
+      The node of mux(multiplexer) controller for HW connections.
+
+  mediatek,mm-mutex:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    maxItems: 1
+    description: |
+      The node of sof(start of frame) signal controller.
+
+  mediatek,mailbox-gce:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      The node of global command engine (GCE), used to read/write
+      registers with critical time limitation.
+      $ref: /schemas/mailbox/mtk-gce.txt
+
+  mboxes:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: |
+      $ref: /schemas/mailbox/mailbox.txt
+
+  gce-subsys:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: |
+      sub-system id corresponding to the global command engine (GCE)
+      register address.
+      $ref: /schemas/mailbox/mtk-gce.txt
+
+if:
+  properties:
+    compatible:
+      items:
+        - enum:
+          - mediatek,mt8183-mdp3
+        - enum:
+          - mediatek,mt8183-mdp3-rdma

Normally, you want to use 'contains' for if/then schemas:

compatible:
  contains:
    const: mediatek,mt8183-mdp3

+
+then:
+  required:
+    - mediatek,scp
+    - mediatek,mmsys
+    - mediatek,mm-mutex
+    - mediatek,mailbox-gce
+    - mboxes
+    - gce-subsys
+
+required:
+  - compatible
+  - mediatek,mdp3-id
+  - reg
+  - clocks
+  - mediatek,gce-client-reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8183-clk.h>
+    #include <dt-bindings/gce/mt8183-gce.h>
+    #include <dt-bindings/power/mt8183-power.h>
+    #include <dt-bindings/memory/mt8183-larb-port.h>
+
+    mdp3_rdma0: mdp3_rdma0@14001000 {
+      compatible = "mediatek,mt8183-mdp3",
+                   "mediatek,mt8183-mdp3-rdma";
+      mediatek,scp = <&scp>;
+      mediatek,mdp3-id = <0>;
+      mdp3-comps = "mediatek,mt8183-mdp3-dl1", "mediatek,mt8183-mdp3-dl2",
+                   "mediatek,mt8183-mdp3-path1", "mediatek,mt8183-mdp3-path2",
+                   "mediatek,mt8183-mdp3-imgi", "mediatek,mt8183-mdp3-exto";
+      mdp3-comp-ids = <0 1 0 1 0 1>;
+      reg = <0x14001000 0x1000>,
+            <0x14000000 0x1000>,
+            <0x14005000 0x1000>,
+            <0x14006000 0x1000>,
+            <0x15020000 0x1000>;
+      mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x1000 0x1000>,
+                                <&gce SUBSYS_1400XXXX 0 0x1000>,
+                                <&gce SUBSYS_1400XXXX 0x5000 0x1000>,
+                                <&gce SUBSYS_1400XXXX 0x6000 0x1000>,
+                                <&gce SUBSYS_1502XXXX 0 0x1000>;
+      power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
+      clocks = <&mmsys CLK_MM_MDP_RDMA0>,
+               <&mmsys CLK_MM_MDP_RSZ1>,
+               <&mmsys CLK_MM_MDP_DL_TXCK>,
+               <&mmsys CLK_MM_MDP_DL_RX>,
+               <&mmsys CLK_MM_IPU_DL_TXCK>,
+               <&mmsys CLK_MM_IPU_DL_RX>;
+      iommus = <&iommu>;
+      mediatek,mmsys = <&mmsys>;
+      mediatek,mm-mutex = <&mutex>;
+      mediatek,mailbox-gce = <&gce>;
+      mboxes = <&gce 20 CMDQ_THR_PRIO_LOWEST 0>,
+               <&gce 21 CMDQ_THR_PRIO_LOWEST 0>,
+               <&gce 22 CMDQ_THR_PRIO_LOWEST 0>,
+               <&gce 23 CMDQ_THR_PRIO_LOWEST 0>;
+      gce-subsys = <&gce 0x14000000 SUBSYS_1400XXXX>,
+                   <&gce 0x14010000 SUBSYS_1401XXXX>,
+                   <&gce 0x14020000 SUBSYS_1402XXXX>,
+                   <&gce 0x15020000 SUBSYS_1502XXXX>;
+    };
\ No newline at end of file

Fix this.

Similar comments on the rest.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel