Re: [net-next PATCH] dt-bindings: net: marvell,pp2: convert to json-schema

On 22/09/2022 23:10, Michał Grzelak wrote:

properties go first.

+patternProperties:
+

no need for blank line.

+  '^interrupt': true

This is not a pattern..

+  '^#.*-cells$': true

??? Nope. Please start from scratch either from recent similar bindings
or from example-schema.

+
+  '^eth[0-9a-f]*(@.*)?$':
+    type: object
+    properties:
+
+      interrupts:
+        minItems: 1
+        maxItems: 10
+        description: interrupt(s) for the port
+
+      interrupt-names:
+        minItems: 1
+        maxItems: 10
+
+        items:
+          oneOf:
+            - pattern: "^hif[0-8]$"
+            - pattern: "^tx-cpu[0-3]$"
+              deprecated: true
+            - const: link
+            - const: rx-shared
+              deprecated: true

List hast to be specific.

+
+        description: >
+          if more than a single interrupt for is given, must be the
+          name associated to the interrupts listed. Valid names are:
+          "hifX", with X in [0..8], and "link". The names "tx-cpu0",
+          "tx-cpu1", "tx-cpu2", "tx-cpu3" and "rx-shared" are supported
+          for backward compatibility but shouldn't be used for new
+          additions.
+
+      port-id:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: ID of the port from the MAC point of view.
+
+      phy-mode:
+        $ref: "ethernet-controller.yaml#/properties/phy-mode"
+
+      marvell,loopback:
+        $ref: /schemas/types.yaml#/definitions/flag
+        description: port is loopback mode.
+
+      phy:
+        $ref: /schemas/types.yaml#/definitions/phandle
+        description: >
+          a phandle to a phy node defining the PHY address
+          (as the reg property, a single integer).
+
+    required:
+      - interrupts
+      - port-id
+      - phy-mode
+
+properties:
+
+  dma-coherent: true
+
+  compatible:

This goes first.

+    enum:
+      - marvell,armada-375-pp2
+      - marvell,armada-7k-pp2
+
+  reg:
+    minItems: 3
+    maxItems: 4
+
+  marvell,system-controller:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: a phandle to the system controller.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+
+allOf:
+

No need for blank line.

+  - $ref: ethernet-controller.yaml#
+
+  - if:
+      not:
+        patternProperties:
+          '^eth[0-9a-f]*(@.*)?$':
+            properties:
+              interrupts:
+                maxItems: 1
+
+    then:
+      patternProperties:
+        '^eth[0-9a-f]*(@.*)?$':
+          required:
+            - interrupt-names

Skip this.

+
+  - if:
+      properties:
+        compatible:
+          const: marvell,armada-375-pp2
+
+    then:
+      properties:
+

Skip the blank lines after each new block.

+        clocks:
+          items:
+            - description: main controller clock
+            - description: GOP clock
+
+        clock-names:
+          minItems: 2
+          maxItems: 2
+          items:
+            enum:
+              - pp_clk
+              - gop_clk
+
+        reg:
+          description: |
+            For "marvell,armada-375-pp2", must contain the following register sets:
+              - common controller registers
+              - LMS registers
+              - one register area per Ethernet port
+
+    else:
+
+      patternProperties:
+        '^eth[0-9a-f]*(@.*)?$':
+          properties:
+            gop-port-id:
+              $ref: /schemas/types.yaml#/definitions/uint32
+              description: >
+                only for marvell,armada-7k-pp2, ID of the port from the
+                GOP (Group Of Ports) point of view. This ID is used to index the
+                per-port registers in the second register area.
+
+          required:
+            - gop-port-id
+
+      properties:
+
+        clocks:
+          items:
+            - description: main controller clock
+            - description: GOP clock
+            - description: MG clock
+            - description: MG Core clock
+            - description: AXI clock

Why clocks appear only here? All devices require clocks, so this should
be in top level.

+
+        clock-names:
+          minItems: 5
+          maxItems: 5
+          items:
+            enum:
+              - gop_clk
+              - pp_clk
+              - mg_clk
+              - mg_core_clk
+              - axi_clk
+
+        reg:
+          description: |
+            For "marvell,armada-7k-pp2" used by 7K/8K and CN913X, must contain the following register sets:
+              - packet processor registers
+              - networking interfaces registers
+              - CM3 address space used for TX Flow Control

Do not define properties in allOf:if:then, but in top-level place.

Really, start with example-schema. This deviates too much from existing
coding style.

Best regards,
Krzysztof