On Sat, Jun 29, 2024 at 05:01:37PM +0200, Lorenzo Bianconi wrote:

quoted

Introduce device-tree binding documentation for Airoha EN7581 ethernet
mac controller.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 .../bindings/net/airoha,en7581-eth.yaml       | 171 ++++++++++++++++++
 1 file changed, 171 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml

diff --git a/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
new file mode 100644
index 000000000000..e2c0da02ccf2
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml

@@ -0,0 +1,171 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/airoha,en7581-eth.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha EN7581 Frame Engine Ethernet controller
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+maintainers:
+  - Lorenzo Bianconi <lorenzo@kernel.org>
+
+description:
+  The frame engine ethernet controller can be found on Airoha SoCs.
+  These SoCs have dual GMAC ports.
+
+properties:
+  compatible:
+    enum:
+      - airoha,en7581-eth
+
+  reg:
+    items:
+      - description: Frame engine base address
+      - description: QDMA0 base address
+      - description: QDMA1 base address
+
+  reg-names:
+    items:
+      - const: fe
+      - const: qdma0
+      - const: qdma1
+
+  interrupts:
+    items:
+      - description: QDMA lan irq0
+      - description: QDMA lan irq1
+      - description: QDMA lan irq2
+      - description: QDMA lan irq3
+      - description: QDMA wan irq0
+      - description: QDMA wan irq1
+      - description: QDMA wan irq2
+      - description: QDMA wan irq3
+      - description: FE error irq
+      - description: PDMA irq
+
+  resets:
+    maxItems: 8
+
+  reset-names:
+    items:
+      - const: fe
+      - const: pdma
+      - const: qdma
+      - const: xsi-mac
+      - const: hsi0-mac
+      - const: hsi1-mac
+      - const: hsi-mac
+      - const: xfp-mac
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^mac@[1-4]$":

ethernet@

Should it be like mtk_eth_soc binding?
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/mediatek%2Cnet.yaml#L371

quoted

+    type: object
+    unevaluatedProperties: false
+    allOf:

Can drop 'allOf'

ack, I will fix it in v5

quoted

+      - $ref: ethernet-controller.yaml#

Which node represents an ethernet controller? This one or the parent? 
Most likely it is not both.

This node reprensts the GMAC port associated to the parent enternet controller.

quoted

+    description:
+      Ethernet MAC node
+    properties:
+      compatible:
+        const: airoha,eth-mac
+
+      reg:
+        maxItems: 1

Based on the unit-address, you need instead:

minimum: 1
maximum: 4

ack, I will fix it in v5

But what does 1-4 represent? There are no MMIO registers associated with 
the MACs? Please describe.

This is the GMAC port ID.

quoted

+
+    required:
+      - reg
+      - compatible
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - resets
+  - reset-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/en7523-clk.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      eth0: ethernet@1fb50000 {
+        compatible = "airoha,en7581-eth";
+        reg = <0 0x1fb50000 0 0x2600>,
+              <0 0x1fb54000 0 0x2000>,
+              <0 0x1fb56000 0 0x2000>;
+        reg-names = "fe", "qdma0", "qdma1";
+
+        resets = <&scuclk 44>,
+                 <&scuclk 30>,
+                 <&scuclk 31>,
+                 <&scuclk 6>,
+                 <&scuclk 15>,
+                 <&scuclk 16>,
+                 <&scuclk 17>,
+                 <&scuclk 26>;
+        reset-names = "fe", "pdma", "qdma", "xsi-mac",
+                      "hsi0-mac", "hsi1-mac", "hsi-mac",
+                      "xfp-mac";
+
+        interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mac1: mac@1 {
+          compatible = "airoha,eth-mac";
+          reg = <1>;
+          phy-mode = "2500base-x";
+          phy-handle = <&phy0>;
+        };
+
+        mac2: mac@2 {
+          compatible = "airoha,eth-mac";
+          reg = <2>;
+          phy-mode = "2500base-x";
+          phy-handle = <&phy1>;
+        };
+      };
+
+      mdio: mdio-bus {

mdio {

But really, drop this if it is not part of this device (binding). What 
is the control interface for this MDIO bus? If it is part of this 
device, then the mdio node needs to be within the device's node.

ack, I will drop it in v5.

Regards,
Lorenzo

quoted

+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        phy0: ethernet-phy@0 {
+            compatible = "ethernet-phy-id67c9.de0a";
+            reg = <0>;
+            phy-mode = "2500base-x";
+        };
+
+        phy1: ethernet-phy@1 {
+            compatible = "ethernet-phy-id67c9.de0a";
+            reg = <1>;
+            phy-mode = "2500base-x";
+        };
+      };
+    };
-- 
2.45.2