[PATCH v3 0/3] marvell,pp2.yaml and .dtsi improvements

[PATCH v3 0/3] marvell,pp2.yaml and .dtsi improvements

[Michał Grzelak]

Hi,

This patch series introduces changes of port names from ethX to 
ethernet-port@X in all relevant .dtsi files. It includes also all
considerations from thread about v2. 

Would appreciate if you had time to review that version.

Best regards,
Michał

---
Changelog:
v2->v3
- move 'reg:description' to 'allOf:if:then'
- change '#size-cells: true' and '#address-cells: true'
  to '#size-cells: const: 0' and '#address-cells: const: 1'
- replace all occurences of pattern "^eth\{hex_num}*"
  with "^(ethernet-)?port@[0-9]+$"
- add description in 'patternProperties:^...'
- add 'patternProperties:^...:interrupt-names:minItems: 1'
- add 'patternProperties:^...:reg:description'
- update 'patternProperties:^...:port-id:description'
- add 'patternProperties:^...:required: - reg'
- update '*:description:' to uppercase
- add 'allOf:then:required:marvell,system-controller'
- skip quotation marks from 'allOf:$ref'
- add 'else' schema to match 'allOf:if:then'
- restrict 'clocks' in 'allOf:if:then'
- restrict 'clock-names' in 'allOf:if:then'
- add #address-cells=<1>; #size-cells=<0>; in 'examples:'
- change every "ethX" to "ethernet-port@X" in 'examples:'
- add "reg" and comment in all ports in 'examples:'
- change /ethernet/eth0/phy-mode in examples://Armada-375
  to "rgmii-id"
- replace each cpm_ with cp0_ in 'examples:'
- replace each _syscon0 with _clk0 in 'examples:'
- remove each eth0X label in 'examples:'
- update armada-375.dtsi and armada-cp11x.dtsi to match
  marvell,pp2.yaml

v1->v2
- move 'properties' to the front of the file
- remove blank line after 'properties'
- move 'compatible' to the front of 'properties'
- move 'clocks', 'clock-names' and 'reg' definitions to 'properties' 
- substitute all occurences of 'marvell,armada-7k-pp2' with
  'marvell,armada-7k-pp22'
- add properties:#size-cells and properties:#address-cells 
- specify list in 'interrupt-names'
- remove blank lines after 'patternProperties'
- remove '^interrupt' and '^#.*-cells$' patterns
- remove blank line after 'allOf'
- remove first 'if-then-else' block from 'allOf'
- negate the condition in allOf:if schema
- delete 'interrupt-controller' from section 'examples'
- delete '#interrupt-cells' from section 'examples'

Marcin Wojtas (2):
  arm64: dts: marvell: Update network description to match schema
  ARM: dts: armada-375: Update network description to match schema

Michał Grzelak (1):
  dt-bindings: net: marvell,pp2: convert to json-schema

 .../devicetree/bindings/net/marvell,pp2.yaml  | 286 ++++++++++++++++++
 .../devicetree/bindings/net/marvell-pp2.txt   | 141 ---------
 MAINTAINERS                                   |   2 +-
 arch/arm/boot/dts/armada-375.dtsi             |  12 +-
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi |  17 +-
 5 files changed, 306 insertions(+), 152 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,pp2.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/marvell-pp2.txt

-- 
2.25.1

[PATCH v3 1/3] dt-bindings: net: marvell,pp2: convert to json-schema

[Michał Grzelak]

Convert the marvell,pp2 bindings from text to proper schema.

Move 'marvell,system-controller' and 'dma-coherent' properties from
port up to the controller node, to match what is actually done in DT.

Signed-off-by: Michał Grzelak <mig@semihalf.com>
---
 .../devicetree/bindings/net/marvell,pp2.yaml  | 286 ++++++++++++++++++
 .../devicetree/bindings/net/marvell-pp2.txt   | 141 ---------
 MAINTAINERS                                   |   2 +-
 3 files changed, 287 insertions(+), 142 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/marvell,pp2.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/marvell-pp2.txt

diff --git a/Documentation/devicetree/bindings/net/marvell,pp2.yaml b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
new file mode 100644
index 000000000000..24c6aeb46814
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
@@ -0,0 +1,286 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/marvell,pp2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell CN913X / Marvell Armada 375, 7K, 8K Ethernet Controller
+
+maintainers:
+  - Marcin Wojtas <mw@semihalf.com>
+  - Russell King <linux@armlinux.org>
+
+description: |
+  Marvell Armada 375 Ethernet Controller (PPv2.1)
+  Marvell Armada 7K/8K Ethernet Controller (PPv2.2)
+  Marvell CN913X Ethernet Controller (PPv2.3)
+
+properties:
+  compatible:
+    enum:
+      - marvell,armada-375-pp2
+      - marvell,armada-7k-pp22
+
+  reg:
+    minItems: 3
+    maxItems: 4
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  clocks:
+    minItems: 2
+    items:
+      - description: main controller clock
+      - description: GOP clock
+      - description: MG clock
+      - description: MG Core clock
+      - description: AXI clock
+
+  clock-names:
+    minItems: 2
+    items:
+      - const: pp_clk
+      - const: gop_clk
+      - const: mg_clk
+      - const: mg_core_clk
+      - const: axi_clk
+
+  dma-coherent: true
+
+  marvell,system-controller:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: a phandle to the system controller.
+
+patternProperties:
+  '^(ethernet-)?port@[0-9]+$':
+    type: object
+    description: subnode for each ethernet port.
+
+    properties:
+      interrupts:
+        minItems: 1
+        maxItems: 10
+        description: interrupt(s) for the port
+
+      interrupt-names:
+        minItems: 1
+        items:
+          - const: hif0
+          - const: hif1
+          - const: hif2
+          - const: hif3
+          - const: hif4
+          - const: hif5
+          - const: hif6
+          - const: hif7
+          - const: hif8
+          - const: link
+
+        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.
+
+      reg:
+        description: ID of the port from the MAC point of view.
+
+      port-id:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: >
+          ID of the port from the MAC point of view.
+          Legacy binding for backward compatibility.
+
+      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).
+
+      phy-mode:
+        $ref: ethernet-controller.yaml#/properties/phy-mode
+
+      marvell,loopback:
+        $ref: /schemas/types.yaml#/definitions/flag
+        description: port is loopback mode.
+
+      gop-port-id:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: >
+          only for marvell,armada-7k-pp22, 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:
+      - interrupts
+      - port-id
+      - phy-mode
+      - reg
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          const: marvell,armada-7k-pp22
+    then:
+      properties:
+        reg:
+          items:
+            - description: Packet Processor registers
+            - description: Networking interfaces registers
+            - description: CM3 address space used for TX Flow Control
+
+        clocks:
+          minItems: 5
+
+        clock-names:
+          minItems: 5
+
+      patternProperties:
+        '^(ethernet-)?port@[0-9]+$':
+          required:
+            - gop-port-id
+
+      required:
+        - marvell,system-controller
+    else:
+      properties:
+        reg:
+          items:
+            - description: Packet Processor registers
+            - description: LMS registers
+            - description: Register area per eth0
+            - description: Register area per eth1
+
+        clocks:
+          maxItems: 2
+
+        clock-names:
+          maxItems: 2
+
+      patternProperties:
+        '^(ethernet-)?port@[0-9]+$':
+          properties:
+            gop-port-id: false
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    // For Armada 375 variant
+    #include <dt-bindings/interrupt-controller/mvebu-icu.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    ethernet@f0000 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "marvell,armada-375-pp2";
+      reg = <0xf0000 0xa000>,
+            <0xc0000 0x3060>,
+            <0xc4000 0x100>,
+            <0xc5000 0x100>;
+      clocks = <&gateclk 3>, <&gateclk 19>;
+      clock-names = "pp_clk", "gop_clk";
+
+      ethernet-port@0 {
+        interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+        reg = <0>;
+        port-id = <0>; /* For backward compatibility. */
+        phy = <&phy0>;
+        phy-mode = "rgmii-id";
+      };
+
+      ethernet-port@1 {
+        interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+        reg = <1>;
+        port-id = <1>; /* For backward compatibility. */
+        phy = <&phy3>;
+        phy-mode = "gmii";
+      };
+    };
+
+  - |
+    // For Armada 7k/8k and Cn913x variants
+    #include <dt-bindings/interrupt-controller/mvebu-icu.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    ethernet@0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "marvell,armada-7k-pp22";
+      reg = <0x0 0x100000>, <0x129000 0xb000>, <0x220000 0x800>;
+      clocks = <&cp0_clk 1 3>, <&cp0_clk 1 9>,
+               <&cp0_clk 1 5>, <&cp0_clk 1 6>, <&cp0_clk 1 18>;
+      clock-names = "pp_clk", "gop_clk", "mg_clk", "mg_core_clk", "axi_clk";
+      marvell,system-controller = <&cp0_syscon0>;
+
+      ethernet-port@0 {
+        interrupts = <ICU_GRP_NSR 39 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 43 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 47 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 51 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 55 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 59 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 63 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 67 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 71 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 129 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "hif0", "hif1", "hif2", "hif3", "hif4",
+                          "hif5", "hif6", "hif7", "hif8", "link";
+        phy-mode = "10gbase-r";
+        reg = <0>;
+        port-id = <0>; /* For backward compatibility. */
+        gop-port-id = <0>;
+      };
+
+      ethernet-port@1 {
+        interrupts = <ICU_GRP_NSR 40 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 44 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 48 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 52 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 56 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 60 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 64 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 68 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 72 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 128 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "hif0", "hif1", "hif2", "hif3", "hif4",
+                          "hif5", "hif6", "hif7", "hif8", "link";
+        phy-mode = "rgmii-id";
+        reg = <1>;
+        port-id = <1>; /* For backward compatibility. */
+        gop-port-id = <2>;
+      };
+
+      ethernet-port@2 {
+        interrupts = <ICU_GRP_NSR 41 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 45 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 49 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 53 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 57 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 61 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 65 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 69 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 73 IRQ_TYPE_LEVEL_HIGH>,
+                     <ICU_GRP_NSR 127 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "hif0", "hif1", "hif2", "hif3", "hif4",
+                          "hif5", "hif6", "hif7", "hif8", "link";
+        phy-mode = "gmii";
+        reg = <2>;
+        port-id = <2>; /* For backward compatibility. */
+        gop-port-id = <3>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt
deleted file mode 100644
index ce15c173f43f..000000000000
--- a/Documentation/devicetree/bindings/net/marvell-pp2.txt
+++ /dev/null
@@ -1,141 +0,0 @@
-* Marvell Armada 375 Ethernet Controller (PPv2.1)
-  Marvell Armada 7K/8K Ethernet Controller (PPv2.2)
-  Marvell CN913X Ethernet Controller (PPv2.3)
-
-Required properties:
-
-- compatible: should be one of:
-    "marvell,armada-375-pp2"
-    "marvell,armada-7k-pp2"
-- reg: addresses and length of the register sets for the device.
-  For "marvell,armada-375-pp2", must contain the following register
-  sets:
-	- common controller registers
-	- LMS registers
-	- one register area per Ethernet port
-  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
-
-- clocks: pointers to the reference clocks for this device, consequently:
-	- main controller clock (for both armada-375-pp2 and armada-7k-pp2)
-	- GOP clock (for both armada-375-pp2 and armada-7k-pp2)
-	- MG clock (only for armada-7k-pp2)
-	- MG Core clock (only for armada-7k-pp2)
-	- AXI clock (only for armada-7k-pp2)
-- clock-names: names of used clocks, must be "pp_clk", "gop_clk", "mg_clk",
-  "mg_core_clk" and "axi_clk" (the 3 latter only for armada-7k-pp2).
-
-The ethernet ports are represented by subnodes. At least one port is
-required.
-
-Required properties (port):
-
-- interrupts: interrupt(s) for the port
-- port-id: ID of the port from the MAC point of view
-- gop-port-id: 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.
-- phy-mode: See ethernet.txt file in the same directory
-
-Optional properties (port):
-
-- marvell,loopback: port is loopback mode
-- phy: a phandle to a phy node defining the PHY address (as the reg
-  property, a single integer).
-- interrupt-names: 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.
-- marvell,system-controller: a phandle to the system controller.
-
-Example for marvell,armada-375-pp2:
-
-ethernet@f0000 {
-	compatible = "marvell,armada-375-pp2";
-	reg = <0xf0000 0xa000>,
-	      <0xc0000 0x3060>,
-	      <0xc4000 0x100>,
-	      <0xc5000 0x100>;
-	clocks = <&gateclk 3>, <&gateclk 19>;
-	clock-names = "pp_clk", "gop_clk";
-
-	eth0: eth0@c4000 {
-		interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
-		port-id = <0>;
-		phy = <&phy0>;
-		phy-mode = "gmii";
-	};
-
-	eth1: eth1@c5000 {
-		interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
-		port-id = <1>;
-		phy = <&phy3>;
-		phy-mode = "gmii";
-	};
-};
-
-Example for marvell,armada-7k-pp2:
-
-cpm_ethernet: ethernet@0 {
-	compatible = "marvell,armada-7k-pp22";
-	reg = <0x0 0x100000>, <0x129000 0xb000>, <0x220000 0x800>;
-	clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>,
-		 <&cpm_syscon0 1 5>, <&cpm_syscon0 1 6>, <&cpm_syscon0 1 18>;
-	clock-names = "pp_clk", "gop_clk", "mg_clk", "mg_core_clk", "axi_clk";
-
-	eth0: eth0 {
-		interrupts = <ICU_GRP_NSR 39 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 43 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 47 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 51 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 55 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 59 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 63 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 67 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 71 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 129 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "hif0", "hif1", "hif2", "hif3", "hif4",
-				  "hif5", "hif6", "hif7", "hif8", "link";
-		port-id = <0>;
-		gop-port-id = <0>;
-	};
-
-	eth1: eth1 {
-		interrupts = <ICU_GRP_NSR 40 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 44 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 48 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 52 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 56 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 60 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 64 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 68 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 72 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 128 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "hif0", "hif1", "hif2", "hif3", "hif4",
-				  "hif5", "hif6", "hif7", "hif8", "link";
-		port-id = <1>;
-		gop-port-id = <2>;
-	};
-
-	eth2: eth2 {
-		interrupts = <ICU_GRP_NSR 41 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 45 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 49 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 53 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 57 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 61 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 65 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 69 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 73 IRQ_TYPE_LEVEL_HIGH>,
-			     <ICU_GRP_NSR 127 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-names = "hif0", "hif1", "hif2", "hif3", "hif4",
-				  "hif5", "hif6", "hif7", "hif8", "link";
-		port-id = <2>;
-		gop-port-id = <3>;
-	};
-};
diff --git a/MAINTAINERS b/MAINTAINERS
index e68a0804394d..51da1b56d87e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12292,7 +12292,7 @@ M:	Marcin Wojtas <mw@semihalf.com>
 M:	Russell King <linux@armlinux.org.uk>
 L:	netdev@vger.kernel.org
 S:	Maintained
-F:	Documentation/devicetree/bindings/net/marvell-pp2.txt
+F:	Documentation/devicetree/bindings/net/marvell,pp2.yaml
 F:	drivers/net/ethernet/marvell/mvpp2/
 
 MARVELL MWIFIEX WIRELESS DRIVER
-- 
2.25.1

[PATCH v3 2/3] arm64: dts: marvell: Update network description to match schema

[Michał Grzelak]

From: Marcin Wojtas <mw@semihalf.com>

Update the PP2 ethernet ports subnodes' names to match
schema enforced by the marvell,pp2.yaml contents.

Add new required properties ('reg') which contains information
about the port ID, keeping 'port-id' ones for backward
compatibility.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index d6c0990a267d..7d0043824f2a 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -58,6 +58,8 @@ config-space@CP11X_BASE {
 		ranges = <0x0 0x0 ADDRESSIFY(CP11X_BASE) 0x2000000>;
 
 		CP11X_LABEL(ethernet): ethernet@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
 			compatible = "marvell,armada-7k-pp22";
 			reg = <0x0 0x100000>, <0x129000 0xb000>, <0x220000 0x800>;
 			clocks = <&CP11X_LABEL(clk) 1 3>, <&CP11X_LABEL(clk) 1 9>,
@@ -69,7 +71,7 @@ CP11X_LABEL(ethernet): ethernet@0 {
 			status = "disabled";
 			dma-coherent;
 
-			CP11X_LABEL(eth0): eth0 {
+			CP11X_LABEL(eth0): ethernet-port@0 {
 				interrupts = <39 IRQ_TYPE_LEVEL_HIGH>,
 					<43 IRQ_TYPE_LEVEL_HIGH>,
 					<47 IRQ_TYPE_LEVEL_HIGH>,
@@ -83,12 +85,13 @@ CP11X_LABEL(eth0): eth0 {
 				interrupt-names = "hif0", "hif1", "hif2",
 					"hif3", "hif4", "hif5", "hif6", "hif7",
 					"hif8", "link";
-				port-id = <0>;
+				reg = <0>;
+				port-id = <0>; /* For backward compatibility. */
 				gop-port-id = <0>;
 				status = "disabled";
 			};
 
-			CP11X_LABEL(eth1): eth1 {
+			CP11X_LABEL(eth1): ethernet-port@1 {
 				interrupts = <40 IRQ_TYPE_LEVEL_HIGH>,
 					<44 IRQ_TYPE_LEVEL_HIGH>,
 					<48 IRQ_TYPE_LEVEL_HIGH>,
@@ -102,12 +105,13 @@ CP11X_LABEL(eth1): eth1 {
 				interrupt-names = "hif0", "hif1", "hif2",
 					"hif3", "hif4", "hif5", "hif6", "hif7",
 					"hif8", "link";
-				port-id = <1>;
+				reg = <1>;
+				port-id = <1>; /* For backward compatibility. */
 				gop-port-id = <2>;
 				status = "disabled";
 			};
 
-			CP11X_LABEL(eth2): eth2 {
+			CP11X_LABEL(eth2): ethernet-port@2 {
 				interrupts = <41 IRQ_TYPE_LEVEL_HIGH>,
 					<45 IRQ_TYPE_LEVEL_HIGH>,
 					<49 IRQ_TYPE_LEVEL_HIGH>,
@@ -121,7 +125,8 @@ CP11X_LABEL(eth2): eth2 {
 				interrupt-names = "hif0", "hif1", "hif2",
 					"hif3", "hif4", "hif5", "hif6", "hif7",
 					"hif8", "link";
-				port-id = <2>;
+				reg = <2>;
+				port-id = <2>; /* For backward compatibility. */
 				gop-port-id = <3>;
 				status = "disabled";
 			};
-- 
2.25.1

[PATCH v3 3/3] ARM: dts: armada-375: Update network description to match schema

[Michał Grzelak]

From: Marcin Wojtas <mw@semihalf.com>

Update the PP2 ethernet ports subnodes' names to match
schema enforced by the marvell,pp2.yaml contents.

Add new required properties ('reg') which contains information
about the port ID, keeping 'port-id' ones for backward
compatibility.

Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 arch/arm/boot/dts/armada-375.dtsi | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/armada-375.dtsi b/arch/arm/boot/dts/armada-375.dtsi
index 929deaf312a5..9fbe0cfec48f 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -178,6 +178,8 @@ mdio: mdio@c0054 {
 
 			/* Network controller */
 			ethernet: ethernet@f0000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
 				compatible = "marvell,armada-375-pp2";
 				reg = <0xf0000 0xa000>, /* Packet Processor regs */
 				      <0xc0000 0x3060>, /* LMS regs */
@@ -187,15 +189,17 @@ ethernet: ethernet@f0000 {
 				clock-names = "pp_clk", "gop_clk";
 				status = "disabled";
 
-				eth0: eth0 {
+				eth0: ethernet-port@0 {
 					interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
-					port-id = <0>;
+					reg = <0>;
+					port-id = <0>; /* For backward compatibility. */
 					status = "disabled";
 				};
 
-				eth1: eth1 {
+				eth1: ethernet-port@1 {
 					interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
-					port-id = <1>;
+					reg = <1>;
+					port-id = <1>; /* For backward compatibility. */
 					status = "disabled";
 				};
 			};
-- 
2.25.1

Re: [PATCH v3 1/3] dt-bindings: net: marvell,pp2: convert to json-schema

[Krzysztof Kozlowski]

On 11/10/2022 15:06, Michał Grzelak wrote:
> Convert the marvell,pp2 bindings from text to proper schema.
> 
> Move 'marvell,system-controller' and 'dma-coherent' properties from
> port up to the controller node, to match what is actually done in DT.

You need to also mention other changes done during conversion -
requiring subnodes to be named "(ethernet-)?ports", deprecating port-id.

> 
> Signed-off-by: Michał Grzelak <mig@semihalf.com>
> ---
>  .../devicetree/bindings/net/marvell,pp2.yaml  | 286 ++++++++++++++++++
>  .../devicetree/bindings/net/marvell-pp2.txt   | 141 ---------
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 287 insertions(+), 142 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/marvell,pp2.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/marvell-pp2.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/marvell,pp2.yaml b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> new file mode 100644
> index 000000000000..24c6aeb46814
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> @@ -0,0 +1,286 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/marvell,pp2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell CN913X / Marvell Armada 375, 7K, 8K Ethernet Controller
> +
> +maintainers:
> +  - Marcin Wojtas <mw@semihalf.com>
> +  - Russell King <linux@armlinux.org>
> +
> +description: |
> +  Marvell Armada 375 Ethernet Controller (PPv2.1)
> +  Marvell Armada 7K/8K Ethernet Controller (PPv2.2)
> +  Marvell CN913X Ethernet Controller (PPv2.3)
> +
> +properties:
> +  compatible:
> +    enum:
> +      - marvell,armada-375-pp2
> +      - marvell,armada-7k-pp22
> +
> +  reg:
> +    minItems: 3
> +    maxItems: 4
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  clocks:
> +    minItems: 2
> +    items:
> +      - description: main controller clock
> +      - description: GOP clock
> +      - description: MG clock
> +      - description: MG Core clock
> +      - description: AXI clock
> +
> +  clock-names:
> +    minItems: 2
> +    items:
> +      - const: pp_clk
> +      - const: gop_clk
> +      - const: mg_clk
> +      - const: mg_core_clk
> +      - const: axi_clk
> +
> +  dma-coherent: true
> +
> +  marvell,system-controller:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: a phandle to the system controller.
> +
> +patternProperties:
> +  '^(ethernet-)?port@[0-9]+$':
> +    type: object
> +    description: subnode for each ethernet port.
> +
> +    properties:
> +      interrupts:
> +        minItems: 1
> +        maxItems: 10
> +        description: interrupt(s) for the port
> +
> +      interrupt-names:
> +        minItems: 1
> +        items:
> +          - const: hif0
> +          - const: hif1
> +          - const: hif2
> +          - const: hif3
> +          - const: hif4
> +          - const: hif5
> +          - const: hif6
> +          - const: hif7
> +          - const: hif8
> +          - const: link
> +
> +        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.
> +
> +      reg:
> +        description: ID of the port from the MAC point of view.
> +
> +      port-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32

        deprecated: true

> +        description: >
> +          ID of the port from the MAC point of view.
> +          Legacy binding for backward compatibility.
> +
> +      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).
> +
> +      phy-mode:
> +        $ref: ethernet-controller.yaml#/properties/phy-mode
> +
> +      marvell,loopback:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: port is loopback mode.
> +
> +      gop-port-id:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: >
> +          only for marvell,armada-7k-pp22, 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:
> +      - interrupts
> +      - port-id
> +      - phy-mode
> +      - reg

Keep the same order of items here as in list of properties

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#

Hmm, are you sure this applies to top-level properties, not to
ethernet-port subnodes? Your ports have phy-mode and phy - just like
ethernet-controller. If I understand correctly, your Armada Ethernet
Controller actually consists of multiple ethernet controllers?

If so, this should be moved to proper place inside patternProperties.
Maybe the subnodes should also be renamed from ports to just "ethernet"
(as ethernet-controller.yaml expects), but other schemas do not follow
this convention,

> +  - if:
> +      properties:
> +        compatible:
> +          const: marvell,armada-7k-pp22


Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: net: marvell,pp2: convert to json-schema

[Marcin Wojtas]

Hi Krzysztof,

Let me chime in.

wt., 11 paź 2022 o 21:50 Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> napisał(a):
>
> On 11/10/2022 15:06, Michał Grzelak wrote:
> > Convert the marvell,pp2 bindings from text to proper schema.
> >
> > Move 'marvell,system-controller' and 'dma-coherent' properties from
> > port up to the controller node, to match what is actually done in DT.
>
> You need to also mention other changes done during conversion -
> requiring subnodes to be named "(ethernet-)?ports", deprecating port-id.
>
> >
> > Signed-off-by: Michał Grzelak <mig@semihalf.com>
> > ---
> >  .../devicetree/bindings/net/marvell,pp2.yaml  | 286 ++++++++++++++++++
> >  .../devicetree/bindings/net/marvell-pp2.txt   | 141 ---------
> >  MAINTAINERS                                   |   2 +-
> >  3 files changed, 287 insertions(+), 142 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/net/marvell,pp2.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/net/marvell-pp2.txt
> >
> > diff --git a/Documentation/devicetree/bindings/net/marvell,pp2.yaml b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> > new file mode 100644
> > index 000000000000..24c6aeb46814
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> > @@ -0,0 +1,286 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/marvell,pp2.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell CN913X / Marvell Armada 375, 7K, 8K Ethernet Controller
> > +
> > +maintainers:
> > +  - Marcin Wojtas <mw@semihalf.com>
> > +  - Russell King <linux@armlinux.org>
> > +
> > +description: |
> > +  Marvell Armada 375 Ethernet Controller (PPv2.1)
> > +  Marvell Armada 7K/8K Ethernet Controller (PPv2.2)
> > +  Marvell CN913X Ethernet Controller (PPv2.3)
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - marvell,armada-375-pp2
> > +      - marvell,armada-7k-pp22
> > +
> > +  reg:
> > +    minItems: 3
> > +    maxItems: 4
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +  clocks:
> > +    minItems: 2
> > +    items:
> > +      - description: main controller clock
> > +      - description: GOP clock
> > +      - description: MG clock
> > +      - description: MG Core clock
> > +      - description: AXI clock
> > +
> > +  clock-names:
> > +    minItems: 2
> > +    items:
> > +      - const: pp_clk
> > +      - const: gop_clk
> > +      - const: mg_clk
> > +      - const: mg_core_clk
> > +      - const: axi_clk
> > +
> > +  dma-coherent: true
> > +
> > +  marvell,system-controller:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: a phandle to the system controller.
> > +
> > +patternProperties:
> > +  '^(ethernet-)?port@[0-9]+$':
> > +    type: object
> > +    description: subnode for each ethernet port.
> > +
> > +    properties:
> > +      interrupts:
> > +        minItems: 1
> > +        maxItems: 10
> > +        description: interrupt(s) for the port
> > +
> > +      interrupt-names:
> > +        minItems: 1
> > +        items:
> > +          - const: hif0
> > +          - const: hif1
> > +          - const: hif2
> > +          - const: hif3
> > +          - const: hif4
> > +          - const: hif5
> > +          - const: hif6
> > +          - const: hif7
> > +          - const: hif8
> > +          - const: link
> > +
> > +        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.
> > +
> > +      reg:
> > +        description: ID of the port from the MAC point of view.
> > +
> > +      port-id:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
>
>         deprecated: true
>
> > +        description: >
> > +          ID of the port from the MAC point of view.
> > +          Legacy binding for backward compatibility.
> > +
> > +      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).
> > +
> > +      phy-mode:
> > +        $ref: ethernet-controller.yaml#/properties/phy-mode
> > +
> > +      marvell,loopback:
> > +        $ref: /schemas/types.yaml#/definitions/flag
> > +        description: port is loopback mode.
> > +
> > +      gop-port-id:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: >
> > +          only for marvell,armada-7k-pp22, 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:
> > +      - interrupts
> > +      - port-id
> > +      - phy-mode
> > +      - reg
>
> Keep the same order of items here as in list of properties
>
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +
> > +allOf:
> > +  - $ref: ethernet-controller.yaml#
>
> Hmm, are you sure this applies to top-level properties, not to
> ethernet-port subnodes? Your ports have phy-mode and phy - just like
> ethernet-controller. If I understand correctly, your Armada Ethernet
> Controller actually consists of multiple ethernet controllers?
>

PP2 is a single controller with common HW blocks, such as queue/buffer
management, parser/classifier, register space, and more. It controls
up to 3 MAC's (ports) that can be connected to phys, sfp cages, etc.
The latter cannot exist on their own and IMO the current hierarchy -
the main controller with subnodes (ports) properly reflects the
hardware.

Anyway, the ethernet-controller.yaml properties fit to the subnodes.
Apart from the name. The below is IMO a good description:.

> If so, this should be moved to proper place inside patternProperties.
> Maybe the subnodes should also be renamed from ports to just "ethernet"
> (as ethernet-controller.yaml expects), but other schemas do not follow
> this convention,

ethernet@
{
    ethernet-port@0
    {
     }
     ethernet-port@1
     {
     }
}

What do you recommend?

Thanks,
Marcin

Re: [PATCH v3 1/3] dt-bindings: net: marvell,pp2: convert to json-schema

[Marcin Wojtas]

wt., 11 paź 2022 o 22:34 Marcin Wojtas <mw@semihalf.com> napisał(a):
>
> Hi Krzysztof,
>
> Let me chime in.
>
> wt., 11 paź 2022 o 21:50 Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> napisał(a):
> >
> > On 11/10/2022 15:06, Michał Grzelak wrote:
> > > Convert the marvell,pp2 bindings from text to proper schema.
> > >
> > > Move 'marvell,system-controller' and 'dma-coherent' properties from
> > > port up to the controller node, to match what is actually done in DT.
> >
> > You need to also mention other changes done during conversion -
> > requiring subnodes to be named "(ethernet-)?ports", deprecating port-id.
> >
> > >
> > > Signed-off-by: Michał Grzelak <mig@semihalf.com>
> > > ---
> > >  .../devicetree/bindings/net/marvell,pp2.yaml  | 286 ++++++++++++++++++
> > >  .../devicetree/bindings/net/marvell-pp2.txt   | 141 ---------
> > >  MAINTAINERS                                   |   2 +-
> > >  3 files changed, 287 insertions(+), 142 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/net/marvell,pp2.yaml
> > >  delete mode 100644 Documentation/devicetree/bindings/net/marvell-pp2.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/marvell,pp2.yaml b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> > > new file mode 100644
> > > index 000000000000..24c6aeb46814
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> > > @@ -0,0 +1,286 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/marvell,pp2.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Marvell CN913X / Marvell Armada 375, 7K, 8K Ethernet Controller
> > > +
> > > +maintainers:
> > > +  - Marcin Wojtas <mw@semihalf.com>
> > > +  - Russell King <linux@armlinux.org>
> > > +
> > > +description: |
> > > +  Marvell Armada 375 Ethernet Controller (PPv2.1)
> > > +  Marvell Armada 7K/8K Ethernet Controller (PPv2.2)
> > > +  Marvell CN913X Ethernet Controller (PPv2.3)
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - marvell,armada-375-pp2
> > > +      - marvell,armada-7k-pp22
> > > +
> > > +  reg:
> > > +    minItems: 3
> > > +    maxItems: 4
> > > +
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> > > +
> > > +  clocks:
> > > +    minItems: 2
> > > +    items:
> > > +      - description: main controller clock
> > > +      - description: GOP clock
> > > +      - description: MG clock
> > > +      - description: MG Core clock
> > > +      - description: AXI clock
> > > +
> > > +  clock-names:
> > > +    minItems: 2
> > > +    items:
> > > +      - const: pp_clk
> > > +      - const: gop_clk
> > > +      - const: mg_clk
> > > +      - const: mg_core_clk
> > > +      - const: axi_clk
> > > +
> > > +  dma-coherent: true
> > > +
> > > +  marvell,system-controller:
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description: a phandle to the system controller.
> > > +
> > > +patternProperties:
> > > +  '^(ethernet-)?port@[0-9]+$':
> > > +    type: object
> > > +    description: subnode for each ethernet port.
> > > +
> > > +    properties:
> > > +      interrupts:
> > > +        minItems: 1
> > > +        maxItems: 10
> > > +        description: interrupt(s) for the port
> > > +
> > > +      interrupt-names:
> > > +        minItems: 1
> > > +        items:
> > > +          - const: hif0
> > > +          - const: hif1
> > > +          - const: hif2
> > > +          - const: hif3
> > > +          - const: hif4
> > > +          - const: hif5
> > > +          - const: hif6
> > > +          - const: hif7
> > > +          - const: hif8
> > > +          - const: link
> > > +
> > > +        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.
> > > +
> > > +      reg:
> > > +        description: ID of the port from the MAC point of view.
> > > +
> > > +      port-id:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> >
> >         deprecated: true
> >
> > > +        description: >
> > > +          ID of the port from the MAC point of view.
> > > +          Legacy binding for backward compatibility.
> > > +
> > > +      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).
> > > +
> > > +      phy-mode:
> > > +        $ref: ethernet-controller.yaml#/properties/phy-mode
> > > +
> > > +      marvell,loopback:
> > > +        $ref: /schemas/types.yaml#/definitions/flag
> > > +        description: port is loopback mode.
> > > +
> > > +      gop-port-id:
> > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > +        description: >
> > > +          only for marvell,armada-7k-pp22, 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:
> > > +      - interrupts
> > > +      - port-id
> > > +      - phy-mode
> > > +      - reg
> >
> > Keep the same order of items here as in list of properties
> >
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - clocks
> > > +  - clock-names
> > > +
> > > +allOf:
> > > +  - $ref: ethernet-controller.yaml#
> >
> > Hmm, are you sure this applies to top-level properties, not to
> > ethernet-port subnodes? Your ports have phy-mode and phy - just like
> > ethernet-controller. If I understand correctly, your Armada Ethernet
> > Controller actually consists of multiple ethernet controllers?
> >
>
> PP2 is a single controller with common HW blocks, such as queue/buffer
> management, parser/classifier, register space, and more. It controls
> up to 3 MAC's (ports) that can be connected to phys, sfp cages, etc.
> The latter cannot exist on their own and IMO the current hierarchy -
> the main controller with subnodes (ports) properly reflects the
> hardware.
>
> Anyway, the ethernet-controller.yaml properties fit to the subnodes.
> Apart from the name. The below is IMO a good description:.
>
> > If so, this should be moved to proper place inside patternProperties.
> > Maybe the subnodes should also be renamed from ports to just "ethernet"
> > (as ethernet-controller.yaml expects), but other schemas do not follow
> > this convention,
>
> ethernet@
> {
>     ethernet-port@0
>     {
>      }
>      ethernet-port@1
>      {
>      }
> }
>
> What do you recommend?
>

I moved the ethernet-controller.yaml reference to under the subnode
(this allowed me to remove phy and phy-mode description)) and it
doesn't complain about the node naming. Please let me know if below
would be acceptable.

--- a/Documentation/devicetree/bindings/net/marvell,pp2.yaml
+++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
@@ -61,7 +61,11 @@ patternProperties:
     type: object
     description: subnode for each ethernet port.

+    allOf:
+      - $ref: ethernet-controller.yaml#
+
     properties:
       interrupts:
         minItems: 1
         maxItems: 10
@@ -95,19 +99,11 @@ patternProperties:

       port-id:
         $ref: /schemas/types.yaml#/definitions/uint32
+        deprecated: true
         description: >
           ID of the port from the MAC point of view.
           Legacy binding for backward compatibility.

-      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).
-
-      phy-mode:
-        $ref: ethernet-controller.yaml#/properties/phy-mode
-
       marvell,loopback:
         $ref: /schemas/types.yaml#/definitions/flag
         description: port is loopback mode.
@@ -132,7 +128,6 @@ required:
   - clock-names

 allOf:
-  - $ref: ethernet-controller.yaml#
   - if:

Best regards,
Marcin

Re: [PATCH v3 1/3] dt-bindings: net: marvell,pp2: convert to json-schema

[Krzysztof Kozlowski]

On 11/10/2022 16:34, Marcin Wojtas wrote:
>>
>> Keep the same order of items here as in list of properties
>>
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - clocks
>>> +  - clock-names
>>> +
>>> +allOf:
>>> +  - $ref: ethernet-controller.yaml#
>>
>> Hmm, are you sure this applies to top-level properties, not to
>> ethernet-port subnodes? Your ports have phy-mode and phy - just like
>> ethernet-controller. If I understand correctly, your Armada Ethernet
>> Controller actually consists of multiple ethernet controllers?
>>
> 
> PP2 is a single controller with common HW blocks, such as queue/buffer
> management, parser/classifier, register space, and more. It controls
> up to 3 MAC's (ports) that can be connected to phys, sfp cages, etc.
> The latter cannot exist on their own and IMO the current hierarchy -
> the main controller with subnodes (ports) properly reflects the
> hardware.
> 
> Anyway, the ethernet-controller.yaml properties fit to the subnodes.
> Apart from the name. The below is IMO a good description:.

It also starts to look a bit like a switch (see bindings/net/dsa).

> 
>> If so, this should be moved to proper place inside patternProperties.
>> Maybe the subnodes should also be renamed from ports to just "ethernet"
>> (as ethernet-controller.yaml expects), but other schemas do not follow
>> this convention,
> 
> ethernet@
> {
>     ethernet-port@0
>     {
>      }
>      ethernet-port@1
>      {
>      }
> }
> 
> What do you recommend?

Yes, keep it like this and reference the ethernet-controller.yaml in
each port.

Best regards,
Krzysztof

Re: [PATCH v3 1/3] dt-bindings: net: marvell,pp2: convert to json-schema

[Krzysztof Kozlowski]

On 11/10/2022 19:01, Marcin Wojtas wrote:
,
>>
>> ethernet@
>> {
>>     ethernet-port@0
>>     {
>>      }
>>      ethernet-port@1
>>      {
>>      }
>> }
>>
>> What do you recommend?
>>
> 
> I moved the ethernet-controller.yaml reference to under the subnode
> (this allowed me to remove phy and phy-mode description)) and it
> doesn't complain about the node naming. Please let me know if below
> would be acceptable.
> 
> --- a/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> +++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
> @@ -61,7 +61,11 @@ patternProperties:
>      type: object
>      description: subnode for each ethernet port.
> 
> +    allOf:

Skip the allOf, just $ref is enough.

> +      - $ref: ethernet-controller.yaml#
> +
>      properties:
>        interrupts:
>          minItems: 1
>          maxItems: 10
> @@ -95,19 +99,11 @@ patternProperties:
> 
>        port-id:
>          $ref: /schemas/types.yaml#/definitions/uint32
> +        deprecated: true
>          description: >
>            ID of the port from the MAC point of view.
>            Legacy binding for backward compatibility.
> 
> -      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).
> -
> -      phy-mode:
> -        $ref: ethernet-controller.yaml#/properties/phy-mode
> -
>        marvell,loopback:
>          $ref: /schemas/types.yaml#/definitions/flag
>          description: port is loopback mode.
> @@ -132,7 +128,6 @@ required:
>    - clock-names
> 
>  allOf:
> -  - $ref: ethernet-controller.yaml#
>    - if:
> 

Yes, except:

1. top-level (so with no indentation) unevaluatedProperties: false
should be now additionalProperties: false.
2. You need unevaluatedProperties here:

+    type: object
+    description: subnode for each ethernet port.
+    $ref: ethernet-controller.yaml#
+    unevaluatedProperties: false

Best regards,
Krzysztof