[PATCH 0/3] net: Convert Gianfar (Triple Speed Ethernet Controller) bindings to YAML

[PATCH 0/3] net: Convert Gianfar (Triple Speed Ethernet Controller) bindings to YAML

[J. Neuschäfer via B4 Relay]

The aim of this series is to modernize the device tree bindings for the
Freescale "Gianfar" ethernet controller (a.k.a. TSEC, Triple Speed
Ethernet Controller) by converting them to YAML.

Known issues that require a solution:
- fsl,gianfar-mdio.yaml (MDIO bus) and fsl,gianfar.yaml (Ethernet
  controller) both specify "gianfar" as a possible compatible string. If
  compatible = "gianfar" is used, the device_type property determines
  whether the node is an MDIO controller or an Ethernet controller.
  This confuses the DT schema validation tools:

  Warning: Duplicate compatible "gianfar" found in schemas matching "$id":
           http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
           http://devicetree.org/schemas/net/fsl,gianfar.yaml#

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
J. Neuschäfer (3):
      dt-bindings: net: Convert fsl,gianfar-{mdio,tbi} to YAML
      dt-bindings: net: fsl,gianfar-mdio: Update information about TBI
      dt-bindings: net: Convert fsl,gianfar to YAML

 .../devicetree/bindings/net/fsl,gianfar-mdio.yaml  |  93 ++++++++
 .../devicetree/bindings/net/fsl,gianfar.yaml       | 242 +++++++++++++++++++++
 .../devicetree/bindings/net/fsl-tsec-phy.txt       |  80 +------
 3 files changed, 338 insertions(+), 77 deletions(-)
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20250209-gianfar-yaml-a654263b7534

Best regards,
-- 
J. Neuschäfer <j.ne@posteo.net>

[PATCH 1/3] dt-bindings: net: Convert fsl,gianfar-{mdio,tbi} to YAML

[J. Neuschäfer via B4 Relay]

From: "J. Neuschäfer" <j.ne@posteo.net>

Move the information related to the Freescale Gianfar (TSEC) MDIO bus
and the Ten-Bit Interface (TBI) from fsl-tsec-phy.txt to a new binding
file in YAML format, fsl,gianfar-mdio.yaml.

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---

dt-bindings: net: Convert fsl,gianfar-tbi to YAML
---
 .../devicetree/bindings/net/fsl,gianfar-mdio.yaml  | 94 ++++++++++++++++++++++
 .../devicetree/bindings/net/fsl-tsec-phy.txt       | 41 +---------
 2 files changed, 96 insertions(+), 39 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml b/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..2dade7f48c366b7f5c7408e1f7de1a6f5fc80787
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Gianfar (TSEC) MDIO Device
+
+description:
+  This binding describes the MDIO is a bus to which the PHY devices are
+  connected. For each device that exists on this bus, a child node should be
+  created.
+
+  As of this writing, every TSEC is associated with an internal Ten-Bit
+  Interface (TBI) PHY. This PHY is accessed through the local MDIO bus. These
+  buses are defined similarly to the mdio buses, except they are compatible
+  with "fsl,gianfar-tbi". The TBI PHYs underneath them are similar to normal
+  PHYs, but the reg property is considered instructive, rather than
+  descriptive. The reg property should be chosen so it doesn't interfere with
+  other PHYs on the bus.
+
+maintainers:
+  - J. Neuschäfer <j.ne@posteo.net>
+
+properties:
+  compatible:
+    enum:
+      - fsl,gianfar-tbi
+      - fsl,gianfar-mdio
+      - fsl,etsec2-tbi
+      - fsl,etsec2-mdio
+      - fsl,ucc-mdio
+      - gianfar
+      - ucc_geth_phy
+
+  reg:
+    minItems: 1
+    items:
+      - description:
+          Offset and length of the register set for the device
+
+      - description:
+          Optionally, the offset and length of the TBIPA register (TBI PHY
+          address register). If TBIPA register is not specified, the driver
+          will attempt to infer it from the register set specified (your
+          mileage may vary).
+
+  device_type:
+    const: mdio
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+required:
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+allOf:
+  - $ref: mdio.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - gianfar
+              - ucc_geth_phy
+    then:
+      required:
+        - device_type
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        mdio@24520 {
+            reg = <0x24520 0x20>;
+            compatible = "fsl,gianfar-mdio";
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ethernet-phy@0 {
+                reg = <0>;
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 9c9668c1b6a24edff7b7cf625b9f14c3cbc2e0c8..0e55e0af7d6f59cfb571dd3fcff704b7f4c140d2 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -1,47 +1,10 @@
 * MDIO IO device
 
-The MDIO is a bus to which the PHY devices are connected.  For each
-device that exists on this bus, a child node should be created.  See
-the definition of the PHY node in booting-without-of.txt for an example
-of how to define a PHY.
-
-Required properties:
-  - reg : Offset and length of the register set for the device, and optionally
-          the offset and length of the TBIPA register (TBI PHY address
-	  register).  If TBIPA register is not specified, the driver will
-	  attempt to infer it from the register set specified (your mileage may
-	  vary).
-  - compatible : Should define the compatible device type for the
-    mdio. Currently supported strings/devices are:
-	- "fsl,gianfar-tbi"
-	- "fsl,gianfar-mdio"
-	- "fsl,etsec2-tbi"
-	- "fsl,etsec2-mdio"
-	- "fsl,ucc-mdio"
-	- "fsl,fman-mdio"
-    When device_type is "mdio", the following strings are also considered:
-	- "gianfar"
-	- "ucc_geth_phy"
-
-Example:
-
-	mdio@24520 {
-		reg = <24520 20>;
-		compatible = "fsl,gianfar-mdio";
-
-		ethernet-phy@0 {
-			......
-		};
-	};
+Refer to Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
 
 * TBI Internal MDIO bus
 
-As of this writing, every tsec is associated with an internal TBI PHY.
-This PHY is accessed through the local MDIO bus.  These buses are defined
-similarly to the mdio buses, except they are compatible with "fsl,gianfar-tbi".
-The TBI PHYs underneath them are similar to normal PHYs, but the reg property
-is considered instructive, rather than descriptive.  The reg property should
-be chosen so it doesn't interfere with other PHYs on the bus.
+Refer to Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
 
 * Gianfar-compatible ethernet nodes
 

-- 
2.48.0.rc1.219.gb6b6757d772

[PATCH 2/3] dt-bindings: net: fsl,gianfar-mdio: Update information about TBI

[J. Neuschäfer via B4 Relay]

From: "J. Neuschäfer" <j.ne@posteo.net>

When this binding was originally written, all known TSEC Ethernet
controllers had a Ten-Bit Interface (TBI). However, some datasheets such
as for the MPC8315E suggest that this is not universally true:

  The eTSECs do not support TBI, GMII, and FIFO operating modes, so all
  references to these interfaces and features should be ignored for this
  device.

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
 Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml b/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
index 2dade7f48c366b7f5c7408e1f7de1a6f5fc80787..0d2605512c4711a4dcb77620b94ea77a71b45fa8 100644
--- a/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
+++ b/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
@@ -11,13 +11,12 @@ description:
   connected. For each device that exists on this bus, a child node should be
   created.
 
-  As of this writing, every TSEC is associated with an internal Ten-Bit
-  Interface (TBI) PHY. This PHY is accessed through the local MDIO bus. These
-  buses are defined similarly to the mdio buses, except they are compatible
-  with "fsl,gianfar-tbi". The TBI PHYs underneath them are similar to normal
-  PHYs, but the reg property is considered instructive, rather than
-  descriptive. The reg property should be chosen so it doesn't interfere with
-  other PHYs on the bus.
+  Some TSECs are associated with an internal Ten-Bit Interface (TBI) PHY. This
+  PHY is accessed through the local MDIO bus. These buses are defined similarly
+  to the mdio buses, except they are compatible with "fsl,gianfar-tbi". The TBI
+  PHYs underneath them are similar to normal PHYs, but the reg property is
+  considered instructive, rather than descriptive. The reg property should be
+  chosen so it doesn't interfere with other PHYs on the bus.
 
 maintainers:
   - J. Neuschäfer <j.ne@posteo.net>

-- 
2.48.0.rc1.219.gb6b6757d772

[PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML

[J. Neuschäfer via B4 Relay]

From: "J. Neuschäfer" <j.ne@posteo.net>

Add a binding for the "Gianfar" ethernet controller, also known as
TSEC/eTSEC.

Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
 .../devicetree/bindings/net/fsl,gianfar.yaml       | 242 +++++++++++++++++++++
 .../devicetree/bindings/net/fsl-tsec-phy.txt       |  39 +---
 2 files changed, 243 insertions(+), 38 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl,gianfar.yaml b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..dc75ceb5dc6fdee8765bb17273f394d01cce0710
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
@@ -0,0 +1,242 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/fsl,gianfar.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Three-Speed Ethernet Controller (TSEC), "Gianfar"
+
+maintainers:
+  - J. Neuschäfer <j.ne@posteo.net>
+
+properties:
+  compatible:
+    enum:
+      - gianfar
+      - fsl,etsec2
+
+  device_type:
+    const: network
+
+  model:
+    enum:
+      - FEC
+      - TSEC
+      - eTSEC
+
+  reg:
+    maxItems: 1
+
+  ranges: true
+
+  "#address-cells": true
+
+  "#size-cells": true
+
+  cell-index:
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  interrupts:
+    maxItems: 3
+
+  dma-coherent:
+    type: boolean
+
+  fsl,magic-packet:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      If present, indicates that the hardware supports waking up via magic packet.
+
+  fsl,wake-on-filer:
+    type: boolean
+    description:
+      If present, indicates that the hardware supports waking up by Filer
+      General Purpose Interrupt (FGPI) asserted on the Rx int line. This is
+      an advanced power management capability allowing certain packet types
+      (user) defined by filer rules to wake up the system.
+
+  bd-stash:
+    type: boolean
+    description:
+      If present, indicates that the hardware supports stashing buffer
+      descriptors in the L2.
+
+  rx-stash-len:
+    type: boolean
+    description:
+      Denotes the number of bytes of a received buffer to stash in the L2.
+
+  tx-stash-len:
+    type: boolean
+    description:
+      Denotes the index of the first byte from the received buffer to stash in
+      the L2.
+
+  fsl,num_rx_queues:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Number of receive queues
+
+  fsl,num_tx_queues:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Number of transmit queues
+
+  tbi-handle:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Reference (phandle) to the TBI node
+
+required:
+  - compatible
+  - model
+
+patternProperties:
+  "^mdio@[0-9a-f]+$":
+    type: object
+    # TODO: reference to gianfar MDIO binding
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+  # compatible = "gianfar" requires device_type = "network"
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: gianfar
+    then:
+      required:
+        - device_type
+
+  # eTSEC2 controller nodes have "queue group" subnodes and don't need a "reg"
+  # property.
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,etsec2
+    then:
+      patternProperties:
+        "^queue-group@[0-9a-f]+$":
+          type: object
+
+          properties:
+            "#address-cells": true
+
+            "#size-cells": true
+
+            reg:
+              maxItems: 1
+
+            interrupts:
+              maxItems: 3
+
+          required:
+            - reg
+            - interrupts
+
+          additionalProperties: false
+    else:
+      required:
+        - reg
+
+  # TSEC and eTSEC devices require three interrupts
+  - if:
+      properties:
+        model:
+          contains:
+            enum: [ TSEC, eTSEC ]
+    then:
+      properties:
+        interrupts:
+          items:
+            - description: Transmit interrupt
+            - description: Receive interrupt
+            - description: Error interrupt
+
+
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    ethernet@24000 {
+        device_type = "network";
+        model = "TSEC";
+        compatible = "gianfar";
+        reg = <0x24000 0x1000>;
+        local-mac-address = [ 00 E0 0C 00 73 00 ];
+        interrupts = <29 2>, <30 2>, <34 2>;
+        interrupt-parent = <&mpic>;
+        phy-handle = <&phy0>;
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc1 {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        ethernet@24000 {
+            compatible = "gianfar";
+            reg = <0x24000 0x1000>;
+            ranges = <0x0 0x24000 0x1000>;
+            #address-cells = <1>;
+            #size-cells = <1>;
+            cell-index = <0>;
+            device_type = "network";
+            model = "eTSEC";
+            local-mac-address = [ 00 00 00 00 00 00 ];
+            interrupts = <32 IRQ_TYPE_LEVEL_LOW>,
+                         <33 IRQ_TYPE_LEVEL_LOW>,
+                         <34 IRQ_TYPE_LEVEL_LOW>;
+            interrupt-parent = <&ipic>;
+
+            mdio@520 {
+                #address-cells = <1>;
+                #size-cells = <0>;
+                compatible = "fsl,gianfar-mdio";
+                reg = <0x520 0x20>;
+            };
+        };
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc2 {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        ethernet {
+            compatible = "fsl,etsec2";
+            ranges;
+            device_type = "network";
+            #address-cells = <2>;
+            #size-cells = <2>;
+            interrupt-parent = <&gic>;
+            model = "eTSEC";
+            fsl,magic-packet;
+            dma-coherent;
+
+            queue-group@2d10000 {
+                #address-cells = <2>;
+                #size-cells = <2>;
+                reg = <0x0 0x2d10000 0x0 0x1000>;
+                interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>,
+                             <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
+                             <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>;
+            };
+
+            queue-group@2d14000  {
+                #address-cells = <2>;
+                #size-cells = <2>;
+                reg = <0x0 0x2d14000 0x0 0x1000>;
+                interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>,
+                             <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>,
+                             <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
+            };
+        };
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 0e55e0af7d6f59cfb571dd3fcff704b7f4c140d2..b18bb4c997ea3a221e599f694d9a28692cbcaa7c 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -8,44 +8,7 @@ Refer to Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
 
 * Gianfar-compatible ethernet nodes
 
-Properties:
-
-  - device_type : Should be "network"
-  - model : Model of the device.  Can be "TSEC", "eTSEC", or "FEC"
-  - compatible : Should be "gianfar"
-  - reg : Offset and length of the register set for the device
-  - interrupts : For FEC devices, the first interrupt is the device's
-    interrupt.  For TSEC and eTSEC devices, the first interrupt is
-    transmit, the second is receive, and the third is error.
-  - phy-handle : See ethernet.txt file in the same directory.
-  - fixed-link : See fixed-link.txt in the same directory.
-  - phy-connection-type : See ethernet.txt file in the same directory.
-    This property is only really needed if the connection is of type
-    "rgmii-id", as all other connection types are detected by hardware.
-  - fsl,magic-packet : If present, indicates that the hardware supports
-    waking up via magic packet.
-  - fsl,wake-on-filer : If present, indicates that the hardware supports
-    waking up by Filer General Purpose Interrupt (FGPI) asserted on the
-    Rx int line.  This is an advanced power management capability allowing
-    certain packet types (user) defined by filer rules to wake up the system.
-  - bd-stash : If present, indicates that the hardware supports stashing
-    buffer descriptors in the L2.
-  - rx-stash-len : Denotes the number of bytes of a received buffer to stash
-    in the L2.
-  - rx-stash-idx : Denotes the index of the first byte from the received
-    buffer to stash in the L2.
-
-Example:
-	ethernet@24000 {
-		device_type = "network";
-		model = "TSEC";
-		compatible = "gianfar";
-		reg = <0x24000 0x1000>;
-		local-mac-address = [ 00 E0 0C 00 73 00 ];
-		interrupts = <29 2 30 2 34 2>;
-		interrupt-parent = <&mpic>;
-		phy-handle = <&phy0>
-	};
+Refer to Documentation/devicetree/bindings/net/fsl,gianfar.yaml
 
 * Gianfar PTP clock nodes
 

-- 
2.48.0.rc1.219.gb6b6757d772

Re: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML

[Andrew Lunn]

> +examples:
> +  - |
> +    ethernet@24000 {
> +        device_type = "network";
> +        model = "TSEC";
> +        compatible = "gianfar";
> +        reg = <0x24000 0x1000>;
> +        local-mac-address = [ 00 E0 0C 00 73 00 ];

That is a valid Motorola MAC address. It is probably not a good idea
to use it in an example, somebody might copy it into a real .dts file.
Typically you use 00 00 00 00 00 00 so there is an empty property the
bootloader can fill in with a unique MAC address.

	Andrew

Re: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML

[Rob Herring (Arm)]

On Thu, 20 Feb 2025 18:29:23 +0100, J. Neuschäfer wrote:
> Add a binding for the "Gianfar" ethernet controller, also known as
> TSEC/eTSEC.
> 
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
>  .../devicetree/bindings/net/fsl,gianfar.yaml       | 242 +++++++++++++++++++++
>  .../devicetree/bindings/net/fsl-tsec-phy.txt       |  39 +---
>  2 files changed, 243 insertions(+), 38 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Warning: Duplicate compatible "gianfar" found in schemas matching "$id":
	http://devicetree.org/schemas/net/fsl,gianfar.yaml#
	http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: device_type:0: 'mdio' was expected
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: '#address-cells' is a required property
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: '#size-cells' is a required property
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: $nodename:0: 'ethernet@24000' does not match '^mdio(-(bus|external))?(@.+|-([0-9]+))?$'
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: Unevaluated properties are not allowed ('device_type', 'interrupts', 'local-mac-address', 'model', 'phy-handle' were unexpected)
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: device_type:0: 'mdio' was expected
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: #size-cells: 0 was expected
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: $nodename:0: 'ethernet@24000' does not match '^mdio(-(bus|external))?(@.+|-([0-9]+))?$'
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: #size-cells: 0 was expected
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: mdio@520:reg:0:0: 1312 is greater than the maximum of 31
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: mdio@520:reg:0: [1312, 32] is too long
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/fsl,gianfar.example.dtb: ethernet@24000: Unevaluated properties are not allowed ('#size-cells', 'cell-index', 'device_type', 'interrupts', 'local-mac-address', 'mdio@520', 'model', 'ranges' were unexpected)
	from schema $id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250220-gianfar-yaml-v1-3-0ba97fd1ef92@posteo.net

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML

[J. Neuschäfer]

On Thu, Feb 20, 2025 at 07:02:11PM +0100, Andrew Lunn wrote:
> > +examples:
> > +  - |
> > +    ethernet@24000 {
> > +        device_type = "network";
> > +        model = "TSEC";
> > +        compatible = "gianfar";
> > +        reg = <0x24000 0x1000>;
> > +        local-mac-address = [ 00 E0 0C 00 73 00 ];
> 
> That is a valid Motorola MAC address. It is probably not a good idea
> to use it in an example, somebody might copy it into a real .dts file.
> Typically you use 00 00 00 00 00 00 so there is an empty property the
> bootloader can fill in with a unique MAC address.
> 
> 	Andrew

Good point, I'll change it to all-zero.


J. Neuschäfer

Re: [PATCH 1/3] dt-bindings: net: Convert fsl,gianfar-{mdio,tbi} to YAML

[Rob Herring]

On Thu, Feb 20, 2025 at 06:29:21PM +0100, J. Neuschäfer wrote:
> Move the information related to the Freescale Gianfar (TSEC) MDIO bus
> and the Ten-Bit Interface (TBI) from fsl-tsec-phy.txt to a new binding
> file in YAML format, fsl,gianfar-mdio.yaml.
> 
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
> 
> dt-bindings: net: Convert fsl,gianfar-tbi to YAML
> ---
>  .../devicetree/bindings/net/fsl,gianfar-mdio.yaml  | 94 ++++++++++++++++++++++
>  .../devicetree/bindings/net/fsl-tsec-phy.txt       | 41 +---------
>  2 files changed, 96 insertions(+), 39 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml b/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..2dade7f48c366b7f5c7408e1f7de1a6f5fc80787
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
> @@ -0,0 +1,94 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Gianfar (TSEC) MDIO Device
> +
> +description:
> +  This binding describes the MDIO is a bus to which the PHY devices are
> +  connected. For each device that exists on this bus, a child node should be
> +  created.
> +
> +  As of this writing, every TSEC is associated with an internal Ten-Bit
> +  Interface (TBI) PHY. This PHY is accessed through the local MDIO bus. These
> +  buses are defined similarly to the mdio buses, except they are compatible
> +  with "fsl,gianfar-tbi". The TBI PHYs underneath them are similar to normal
> +  PHYs, but the reg property is considered instructive, rather than
> +  descriptive. The reg property should be chosen so it doesn't interfere with
> +  other PHYs on the bus.
> +
> +maintainers:
> +  - J. Neuschäfer <j.ne@posteo.net>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,gianfar-tbi
> +      - fsl,gianfar-mdio
> +      - fsl,etsec2-tbi
> +      - fsl,etsec2-mdio
> +      - fsl,ucc-mdio
> +      - gianfar

Can you just comment out this to avoid the duplicate issue.

Though I think if you write a custom 'select' which looks for 
'device_type = "mdio"' with gianfar compatible and similar in the other 
binding, then the warning will go away. 

> +      - ucc_geth_phy
> +
> +  reg:
> +    minItems: 1
> +    items:
> +      - description:
> +          Offset and length of the register set for the device
> +
> +      - description:
> +          Optionally, the offset and length of the TBIPA register (TBI PHY
> +          address register). If TBIPA register is not specified, the driver
> +          will attempt to infer it from the register set specified (your
> +          mileage may vary).
> +
> +  device_type:
> +    const: mdio
> +

> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0

These are defined in mdio.yaml, so drop them here.

> +
> +required:
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +allOf:
> +  - $ref: mdio.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - gianfar
> +              - ucc_geth_phy
> +    then:
> +      required:
> +        - device_type

Essentially, move this to the 'select' schema and add that property 
device_type must be 'mdio'. You won't need it here anymore because it 
had to be true for the schema to be applied.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        mdio@24520 {
> +            reg = <0x24520 0x20>;
> +            compatible = "fsl,gianfar-mdio";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            ethernet-phy@0 {
> +                reg = <0>;
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
> index 9c9668c1b6a24edff7b7cf625b9f14c3cbc2e0c8..0e55e0af7d6f59cfb571dd3fcff704b7f4c140d2 100644
> --- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
> @@ -1,47 +1,10 @@
>  * MDIO IO device
>  
> -The MDIO is a bus to which the PHY devices are connected.  For each
> -device that exists on this bus, a child node should be created.  See
> -the definition of the PHY node in booting-without-of.txt for an example
> -of how to define a PHY.
> -
> -Required properties:
> -  - reg : Offset and length of the register set for the device, and optionally
> -          the offset and length of the TBIPA register (TBI PHY address
> -	  register).  If TBIPA register is not specified, the driver will
> -	  attempt to infer it from the register set specified (your mileage may
> -	  vary).
> -  - compatible : Should define the compatible device type for the
> -    mdio. Currently supported strings/devices are:
> -	- "fsl,gianfar-tbi"
> -	- "fsl,gianfar-mdio"
> -	- "fsl,etsec2-tbi"
> -	- "fsl,etsec2-mdio"
> -	- "fsl,ucc-mdio"
> -	- "fsl,fman-mdio"
> -    When device_type is "mdio", the following strings are also considered:
> -	- "gianfar"
> -	- "ucc_geth_phy"
> -
> -Example:
> -
> -	mdio@24520 {
> -		reg = <24520 20>;
> -		compatible = "fsl,gianfar-mdio";
> -
> -		ethernet-phy@0 {
> -			......
> -		};
> -	};
> +Refer to Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
>  
>  * TBI Internal MDIO bus
>  
> -As of this writing, every tsec is associated with an internal TBI PHY.
> -This PHY is accessed through the local MDIO bus.  These buses are defined
> -similarly to the mdio buses, except they are compatible with "fsl,gianfar-tbi".
> -The TBI PHYs underneath them are similar to normal PHYs, but the reg property
> -is considered instructive, rather than descriptive.  The reg property should
> -be chosen so it doesn't interfere with other PHYs on the bus.
> +Refer to Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml
>  
>  * Gianfar-compatible ethernet nodes
>  
> 
> -- 
> 2.48.0.rc1.219.gb6b6757d772
> 

Re: [PATCH 2/3] dt-bindings: net: fsl,gianfar-mdio: Update information about TBI

[Rob Herring (Arm)]

On Thu, 20 Feb 2025 18:29:22 +0100, J. Neuschäfer wrote:
> When this binding was originally written, all known TSEC Ethernet
> controllers had a Ten-Bit Interface (TBI). However, some datasheets such
> as for the MPC8315E suggest that this is not universally true:
> 
>   The eTSECs do not support TBI, GMII, and FIFO operating modes, so all
>   references to these interfaces and features should be ignored for this
>   device.
> 
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
>  Documentation/devicetree/bindings/net/fsl,gianfar-mdio.yaml | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML

[Rob Herring]

On Thu, Feb 20, 2025 at 06:29:23PM +0100, J. Neuschäfer wrote:
> Add a binding for the "Gianfar" ethernet controller, also known as
> TSEC/eTSEC.
> 
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
>  .../devicetree/bindings/net/fsl,gianfar.yaml       | 242 +++++++++++++++++++++
>  .../devicetree/bindings/net/fsl-tsec-phy.txt       |  39 +---
>  2 files changed, 243 insertions(+), 38 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/fsl,gianfar.yaml b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..dc75ceb5dc6fdee8765bb17273f394d01cce0710
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
> @@ -0,0 +1,242 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/fsl,gianfar.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale Three-Speed Ethernet Controller (TSEC), "Gianfar"
> +
> +maintainers:
> +  - J. Neuschäfer <j.ne@posteo.net>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - gianfar
> +      - fsl,etsec2
> +
> +  device_type:
> +    const: network
> +
> +  model:
> +    enum:
> +      - FEC
> +      - TSEC
> +      - eTSEC
> +
> +  reg:
> +    maxItems: 1
> +
> +  ranges: true
> +
> +  "#address-cells": true

enum: [ 1, 2 ]

because 3 is not valid here.

> +
> +  "#size-cells": true

enum: [ 1, 2 ]

because 0 is not valid here.


> +
> +  cell-index:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  interrupts:
> +    maxItems: 3

Based on the if/then schema, you need 'minItems' here if the min is not 
3.

Really, move the descriptions here and make them work for the combined 
interrupt case (just a guess).

> +
> +  dma-coherent:
> +    type: boolean

dma-coherent: true

> +
> +  fsl,magic-packet:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      If present, indicates that the hardware supports waking up via magic packet.
> +
> +  fsl,wake-on-filer:
> +    type: boolean
> +    description:
> +      If present, indicates that the hardware supports waking up by Filer
> +      General Purpose Interrupt (FGPI) asserted on the Rx int line. This is
> +      an advanced power management capability allowing certain packet types
> +      (user) defined by filer rules to wake up the system.
> +
> +  bd-stash:
> +    type: boolean
> +    description:
> +      If present, indicates that the hardware supports stashing buffer
> +      descriptors in the L2.
> +
> +  rx-stash-len:
> +    type: boolean
> +    description:
> +      Denotes the number of bytes of a received buffer to stash in the L2.
> +
> +  tx-stash-len:
> +    type: boolean
> +    description:
> +      Denotes the index of the first byte from the received buffer to stash in
> +      the L2.
> +
> +  fsl,num_rx_queues:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Number of receive queues

Constraints? I assume there's at least more than 0.

> +
> +  fsl,num_tx_queues:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Number of transmit queues

Constraints?

> +
> +  tbi-handle:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Reference (phandle) to the TBI node
> +
> +required:
> +  - compatible
> +  - model
> +
> +patternProperties:
> +  "^mdio@[0-9a-f]+$":
> +    type: object
> +    # TODO: reference to gianfar MDIO binding
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +
> +  # compatible = "gianfar" requires device_type = "network"
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: gianfar
> +    then:
> +      required:
> +        - device_type
> +
> +  # eTSEC2 controller nodes have "queue group" subnodes and don't need a "reg"
> +  # property.
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,etsec2
> +    then:
> +      patternProperties:
> +        "^queue-group@[0-9a-f]+$":
> +          type: object
> +
> +          properties:
> +            "#address-cells": true
> +
> +            "#size-cells": true

These have no effect if there are not child nodes or a 'ranges' 
property.

> +
> +            reg:
> +              maxItems: 1
> +
> +            interrupts:
> +              maxItems: 3

Need to define what each one is.

> +
> +          required:
> +            - reg
> +            - interrupts
> +
> +          additionalProperties: false
> +    else:
> +      required:
> +        - reg
> +
> +  # TSEC and eTSEC devices require three interrupts
> +  - if:
> +      properties:
> +        model:
> +          contains:
> +            enum: [ TSEC, eTSEC ]
> +    then:
> +      properties:
> +        interrupts:
> +          items:
> +            - description: Transmit interrupt
> +            - description: Receive interrupt
> +            - description: Error interrupt
> +
> +
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    ethernet@24000 {
> +        device_type = "network";
> +        model = "TSEC";
> +        compatible = "gianfar";
> +        reg = <0x24000 0x1000>;
> +        local-mac-address = [ 00 E0 0C 00 73 00 ];
> +        interrupts = <29 2>, <30 2>, <34 2>;
> +        interrupt-parent = <&mpic>;
> +        phy-handle = <&phy0>;
> +    };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    soc1 {
> +        #address-cells = <1>;
> +        #size-cells = <1>;

You don't need the soc1 node.

> +
> +        ethernet@24000 {
> +            compatible = "gianfar";
> +            reg = <0x24000 0x1000>;
> +            ranges = <0x0 0x24000 0x1000>;
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            cell-index = <0>;
> +            device_type = "network";
> +            model = "eTSEC";
> +            local-mac-address = [ 00 00 00 00 00 00 ];
> +            interrupts = <32 IRQ_TYPE_LEVEL_LOW>,
> +                         <33 IRQ_TYPE_LEVEL_LOW>,
> +                         <34 IRQ_TYPE_LEVEL_LOW>;
> +            interrupt-parent = <&ipic>;
> +
> +            mdio@520 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                compatible = "fsl,gianfar-mdio";
> +                reg = <0x520 0x20>;
> +            };
> +        };
> +    };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    soc2 {

bus {

> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        ethernet {
> +            compatible = "fsl,etsec2";
> +            ranges;
> +            device_type = "network";
> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +            interrupt-parent = <&gic>;
> +            model = "eTSEC";
> +            fsl,magic-packet;
> +            dma-coherent;
> +
> +            queue-group@2d10000 {
> +                #address-cells = <2>;
> +                #size-cells = <2>;
> +                reg = <0x0 0x2d10000 0x0 0x1000>;
> +                interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>,
> +                             <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
> +                             <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>;
> +            };
> +
> +            queue-group@2d14000  {
> +                #address-cells = <2>;
> +                #size-cells = <2>;
> +                reg = <0x0 0x2d14000 0x0 0x1000>;
> +                interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>,
> +                             <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>,
> +                             <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> +            };
> +        };
> +    };
> +
> +...

Re: [PATCH 1/3] dt-bindings: net: Convert fsl,gianfar-{mdio,tbi} to YAML

[J. Neuschäfer]

On Fri, Feb 21, 2025 at 10:36:51AM -0600, Rob Herring wrote:
> On Thu, Feb 20, 2025 at 06:29:21PM +0100, J. Neuschäfer wrote:
> > Move the information related to the Freescale Gianfar (TSEC) MDIO bus
> > and the Ten-Bit Interface (TBI) from fsl-tsec-phy.txt to a new binding
> > file in YAML format, fsl,gianfar-mdio.yaml.
> > 
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > ---
[...]
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - fsl,gianfar-tbi
> > +      - fsl,gianfar-mdio
> > +      - fsl,etsec2-tbi
> > +      - fsl,etsec2-mdio
> > +      - fsl,ucc-mdio
> > +      - gianfar
> 
> Can you just comment out this to avoid the duplicate issue.
> 
> Though I think if you write a custom 'select' which looks for 
> 'device_type = "mdio"' with gianfar compatible and similar in the other 
> binding, then the warning will go away. 

I'm not sure how the 'select' syntax works, is there a reference
document I could read?

> 
> > +      - ucc_geth_phy
> > +
> > +  reg:
> > +    minItems: 1
> > +    items:
> > +      - description:
> > +          Offset and length of the register set for the device
> > +
> > +      - description:
> > +          Optionally, the offset and length of the TBIPA register (TBI PHY
> > +          address register). If TBIPA register is not specified, the driver
> > +          will attempt to infer it from the register set specified (your
> > +          mileage may vary).
> > +
> > +  device_type:
> > +    const: mdio
> > +
> 
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> 
> These are defined in mdio.yaml, so drop them here.

Will do.

> 
> > +
> > +required:
> > +  - reg
> > +  - "#address-cells"
> > +  - "#size-cells"
> > +
> > +allOf:
> > +  - $ref: mdio.yaml#
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - gianfar
> > +              - ucc_geth_phy
> > +    then:
> > +      required:
> > +        - device_type
> 
> Essentially, move this to the 'select' schema and add that property 
> device_type must be 'mdio'. You won't need it here anymore because it 
> had to be true for the schema to be applied.

I'll have to read up on how select works.


Best Regards,
J. Neuschäfer

Re: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML

[J. Neuschäfer]

On Fri, Feb 21, 2025 at 05:35:23PM -0600, Rob Herring wrote:
> On Thu, Feb 20, 2025 at 06:29:23PM +0100, J. Neuschäfer wrote:
> > Add a binding for the "Gianfar" ethernet controller, also known as
> > TSEC/eTSEC.
> > 
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > ---
> >  .../devicetree/bindings/net/fsl,gianfar.yaml       | 242 +++++++++++++++++++++
> >  .../devicetree/bindings/net/fsl-tsec-phy.txt       |  39 +---
> >  2 files changed, 243 insertions(+), 38 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/fsl,gianfar.yaml b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..dc75ceb5dc6fdee8765bb17273f394d01cce0710
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
> > @@ -0,0 +1,242 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/fsl,gianfar.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Freescale Three-Speed Ethernet Controller (TSEC), "Gianfar"
[...]
> > +  "#address-cells": true
> 
> enum: [ 1, 2 ]
> 
> because 3 is not valid here.
> 
> > +
> > +  "#size-cells": true
> 
> enum: [ 1, 2 ]
> 
> because 0 is not valid here.

Good point.

> 
> 
> > +
> > +  cell-index:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +  interrupts:
> > +    maxItems: 3
> 
> Based on the if/then schema, you need 'minItems' here if the min is not 3.
> 
> Really, move the descriptions here and make them work for the combined 
> interrupt case (just a guess).

The difference here (as previously documented in prose) is by device
variant:

 for FEC:

   - one combined interrupt

 for TSEC, eTSEC:

   - transmit interrupt
   - receive interrupt
   - error interrupt

Combining these cases might look like this, not sure if it's good:

  interrupts:
    minItems: 1
    description:
      items:
        - Transmit interrupt or combined interrupt
        - Receive interrupt
        - Error interrupt

> 
> > +
> > +  dma-coherent:
> > +    type: boolean
> 
> dma-coherent: true

Will do.


> > +
> > +  fsl,num_rx_queues:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Number of receive queues
> 
> Constraints? I assume there's at least more than 0.
> 
> > +
> > +  fsl,num_tx_queues:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Number of transmit queues
> 
> Constraints?

Good point, for both of these the only value I can find in use is 8,
which corresponds to the number of queues documented in at least one
hardware manual (MPC8548E).


> > +  # eTSEC2 controller nodes have "queue group" subnodes and don't need a "reg"
> > +  # property.
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: fsl,etsec2
> > +    then:
> > +      patternProperties:
> > +        "^queue-group@[0-9a-f]+$":
> > +          type: object
> > +
> > +          properties:
> > +            "#address-cells": true
> > +
> > +            "#size-cells": true
> 
> These have no effect if there are not child nodes or a 'ranges' 
> property.

Ah, good point, these properties are used in existing DTs, but I see no
reason to keep them. I'll remove them.

> 
> > +
> > +            reg:
> > +              maxItems: 1
> > +
> > +            interrupts:
> > +              maxItems: 3
> 
> Need to define what each one is.

Will do.


> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    soc1 {
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> 
> You don't need the soc1 node.

Ah, true.

> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    soc2 {
> 
> bus {

Will rename.


Thanks,
J. Neuschäfer

Re: [PATCH 1/3] dt-bindings: net: Convert fsl,gianfar-{mdio,tbi} to YAML

[J. Neuschäfer]

On Mon, Feb 24, 2025 at 08:58:19PM +0000, J. Neuschäfer wrote:
> On Fri, Feb 21, 2025 at 10:36:51AM -0600, Rob Herring wrote:
> > On Thu, Feb 20, 2025 at 06:29:21PM +0100, J. Neuschäfer wrote:
> > > Move the information related to the Freescale Gianfar (TSEC) MDIO bus
> > > and the Ten-Bit Interface (TBI) from fsl-tsec-phy.txt to a new binding
> > > file in YAML format, fsl,gianfar-mdio.yaml.
> > > 
> > > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > > ---
> [...]
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - fsl,gianfar-tbi
> > > +      - fsl,gianfar-mdio
> > > +      - fsl,etsec2-tbi
> > > +      - fsl,etsec2-mdio
> > > +      - fsl,ucc-mdio
> > > +      - gianfar
> > 
> > Can you just comment out this to avoid the duplicate issue.
> > 
> > Though I think if you write a custom 'select' which looks for 
> > 'device_type = "mdio"' with gianfar compatible and similar in the other 
> > binding, then the warning will go away. 
> 
> I'm not sure how the 'select' syntax works, is there a reference
> document I could read?

Ok, I think I figured it out, this seems to work as intended:



select:
  oneOf:
    - properties:
        compatible:
          enum:
            - fsl,gianfar-tbi
            - fsl,gianfar-mdio
            - fsl,etsec2-tbi
            - fsl,etsec2-mdio
            - fsl,ucc-mdio

      required:
        - compatible

    - properties:
        compatible:
          enum:
            - gianfar
            - ucc_geth_phy

        device_type:
          const: mdio

      required:
        - compatible
        - device_type

properties:
  compatible:
    enum:
      - fsl,gianfar-tbi
      - fsl,gianfar-mdio
      - fsl,etsec2-tbi
      - fsl,etsec2-mdio
      - fsl,ucc-mdio
      - gianfar
      - ucc_geth_phy

  reg:
    ...



Best regards,
J. Neuschäfer

Re: [PATCH 1/3] dt-bindings: net: Convert fsl,gianfar-{mdio,tbi} to YAML

[Rob Herring]

On Tue, Feb 25, 2025 at 11:12:42AM +0000, J. Neuschäfer wrote:
> On Mon, Feb 24, 2025 at 08:58:19PM +0000, J. Neuschäfer wrote:
> > On Fri, Feb 21, 2025 at 10:36:51AM -0600, Rob Herring wrote:
> > > On Thu, Feb 20, 2025 at 06:29:21PM +0100, J. Neuschäfer wrote:
> > > > Move the information related to the Freescale Gianfar (TSEC) MDIO bus
> > > > and the Ten-Bit Interface (TBI) from fsl-tsec-phy.txt to a new binding
> > > > file in YAML format, fsl,gianfar-mdio.yaml.
> > > > 
> > > > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > > > ---
> > [...]
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - fsl,gianfar-tbi
> > > > +      - fsl,gianfar-mdio
> > > > +      - fsl,etsec2-tbi
> > > > +      - fsl,etsec2-mdio
> > > > +      - fsl,ucc-mdio
> > > > +      - gianfar
> > > 
> > > Can you just comment out this to avoid the duplicate issue.
> > > 
> > > Though I think if you write a custom 'select' which looks for 
> > > 'device_type = "mdio"' with gianfar compatible and similar in the other 
> > > binding, then the warning will go away. 
> > 
> > I'm not sure how the 'select' syntax works, is there a reference
> > document I could read?
> 
> Ok, I think I figured it out, this seems to work as intended:
> 

Looks pretty good.

> 
> select:
>   oneOf:
>     - properties:
>         compatible:

Add "contains" here. That way if someone puts another string in with 
these we still match and then throw a warning.

>           enum:
>             - fsl,gianfar-tbi
>             - fsl,gianfar-mdio
>             - fsl,etsec2-tbi
>             - fsl,etsec2-mdio
>             - fsl,ucc-mdio
> 
>       required:
>         - compatible
> 
>     - properties:
>         compatible:
>           enum:
>             - gianfar
>             - ucc_geth_phy

You could move ucc_geth_phy because there's not a collision with it.

Add a comment somewhere that this is all because of a reuse of gianfar.

>         device_type:
>           const: mdio
> 
>       required:
>         - compatible
>         - device_type

You can move 'required: [compatible]' out of the oneOf.

> 
> properties:
>   compatible:
>     enum:
>       - fsl,gianfar-tbi
>       - fsl,gianfar-mdio
>       - fsl,etsec2-tbi
>       - fsl,etsec2-mdio
>       - fsl,ucc-mdio
>       - gianfar
>       - ucc_geth_phy
> 
>   reg:
>     ...
> 
> 
> 
> Best regards,
> J. Neuschäfer

Re: [PATCH 1/3] dt-bindings: net: Convert fsl,gianfar-{mdio,tbi} to YAML

[J. Neuschäfer]

On Wed, Feb 26, 2025 at 07:31:14AM -0600, Rob Herring wrote:
> On Tue, Feb 25, 2025 at 11:12:42AM +0000, J. Neuschäfer wrote:
> > On Mon, Feb 24, 2025 at 08:58:19PM +0000, J. Neuschäfer wrote:
> > > On Fri, Feb 21, 2025 at 10:36:51AM -0600, Rob Herring wrote:
> > > > On Thu, Feb 20, 2025 at 06:29:21PM +0100, J. Neuschäfer wrote:
> > > > > Move the information related to the Freescale Gianfar (TSEC) MDIO bus
> > > > > and the Ten-Bit Interface (TBI) from fsl-tsec-phy.txt to a new binding
> > > > > file in YAML format, fsl,gianfar-mdio.yaml.
> > > > > 
> > > > > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > > > > ---
> > > [...]
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    enum:
> > > > > +      - fsl,gianfar-tbi
> > > > > +      - fsl,gianfar-mdio
> > > > > +      - fsl,etsec2-tbi
> > > > > +      - fsl,etsec2-mdio
> > > > > +      - fsl,ucc-mdio
> > > > > +      - gianfar
> > > > 
> > > > Can you just comment out this to avoid the duplicate issue.
> > > > 
> > > > Though I think if you write a custom 'select' which looks for 
> > > > 'device_type = "mdio"' with gianfar compatible and similar in the other 
> > > > binding, then the warning will go away. 
> > > 
> > > I'm not sure how the 'select' syntax works, is there a reference
> > > document I could read?
> > 
> > Ok, I think I figured it out, this seems to work as intended:
> > 
> 
> Looks pretty good.
> 
> > 
> > select:
> >   oneOf:
> >     - properties:
> >         compatible:
> 
> Add "contains" here. That way if someone puts another string in with 
> these we still match and then throw a warning.

Good idea.

> 
> >           enum:
> >             - fsl,gianfar-tbi
> >             - fsl,gianfar-mdio
> >             - fsl,etsec2-tbi
> >             - fsl,etsec2-mdio
> >             - fsl,ucc-mdio
> > 
> >       required:
> >         - compatible
> > 
> >     - properties:
> >         compatible:
> >           enum:
> >             - gianfar
> >             - ucc_geth_phy
> 
> You could move ucc_geth_phy because there's not a collision with it.

ucc_geth_phy also requires device_type = "mdio". It is more compact
to write it like this, but perhaps clarity wins out here, and this
requirement should be expressed with an "if:"?

> Add a comment somewhere that this is all because of a reuse of gianfar.

Will do.

> 
> >         device_type:
> >           const: mdio
> > 
> >       required:
> >         - compatible
> >         - device_type
> 
> You can move 'required: [compatible]' out of the oneOf.

Will do.


Thanks,
J. Neuschäfer

Re: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML

[kernel test robot]

Hi Neuschäfer,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 2014c95afecee3e76ca4a56956a936e23283f05b]

url:    https://github.com/intel-lab-lkp/linux/commits/J-Neusch-fer-via-B4-Relay/dt-bindings-net-Convert-fsl-gianfar-mdio-tbi-to-YAML/20250221-013202
base:   2014c95afecee3e76ca4a56956a936e23283f05b
patch link:    https://lore.kernel.org/r/20250220-gianfar-yaml-v1-3-0ba97fd1ef92%40posteo.net
patch subject: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML
config: csky-randconfig-051-20250227 (https://download.01.org/0day-ci/archive/20250227/202502272036.FSFdbXEm-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 14.2.0
dtschema version: 2025.3.dev3+gabf9328
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250227/202502272036.FSFdbXEm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502272036.FSFdbXEm-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Documentation/devicetree/bindings/net/snps,dwmac.yaml: mac-mode: missing type definition
>> Warning: Duplicate compatible "gianfar" found in schemas matching "$id":
   	http://devicetree.org/schemas/net/fsl,gianfar-mdio.yaml#
   	http://devicetree.org/schemas/net/fsl,gianfar.yaml#

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/3] dt-bindings: net: Convert fsl,gianfar-{mdio,tbi} to YAML

[Rob Herring]

On Wed, Feb 26, 2025 at 02:59:06PM +0000, J. Neuschäfer wrote:
> On Wed, Feb 26, 2025 at 07:31:14AM -0600, Rob Herring wrote:
> > On Tue, Feb 25, 2025 at 11:12:42AM +0000, J. Neuschäfer wrote:
> > > On Mon, Feb 24, 2025 at 08:58:19PM +0000, J. Neuschäfer wrote:
> > > > On Fri, Feb 21, 2025 at 10:36:51AM -0600, Rob Herring wrote:
> > > > > On Thu, Feb 20, 2025 at 06:29:21PM +0100, J. Neuschäfer wrote:
> > > > > > Move the information related to the Freescale Gianfar (TSEC) MDIO bus
> > > > > > and the Ten-Bit Interface (TBI) from fsl-tsec-phy.txt to a new binding
> > > > > > file in YAML format, fsl,gianfar-mdio.yaml.
> > > > > > 
> > > > > > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > > > > > ---
> > > > [...]
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    enum:
> > > > > > +      - fsl,gianfar-tbi
> > > > > > +      - fsl,gianfar-mdio
> > > > > > +      - fsl,etsec2-tbi
> > > > > > +      - fsl,etsec2-mdio
> > > > > > +      - fsl,ucc-mdio
> > > > > > +      - gianfar
> > > > > 
> > > > > Can you just comment out this to avoid the duplicate issue.
> > > > > 
> > > > > Though I think if you write a custom 'select' which looks for 
> > > > > 'device_type = "mdio"' with gianfar compatible and similar in the other 
> > > > > binding, then the warning will go away. 
> > > > 
> > > > I'm not sure how the 'select' syntax works, is there a reference
> > > > document I could read?
> > > 
> > > Ok, I think I figured it out, this seems to work as intended:
> > > 
> > 
> > Looks pretty good.
> > 
> > > 
> > > select:
> > >   oneOf:
> > >     - properties:
> > >         compatible:
> > 
> > Add "contains" here. That way if someone puts another string in with 
> > these we still match and then throw a warning.
> 
> Good idea.
> 
> > 
> > >           enum:
> > >             - fsl,gianfar-tbi
> > >             - fsl,gianfar-mdio
> > >             - fsl,etsec2-tbi
> > >             - fsl,etsec2-mdio
> > >             - fsl,ucc-mdio
> > > 
> > >       required:
> > >         - compatible
> > > 
> > >     - properties:
> > >         compatible:
> > >           enum:
> > >             - gianfar
> > >             - ucc_geth_phy
> > 
> > You could move ucc_geth_phy because there's not a collision with it.
> 
> ucc_geth_phy also requires device_type = "mdio". It is more compact
> to write it like this, but perhaps clarity wins out here, and this
> requirement should be expressed with an "if:"?

Yes, an if/then schema outside of the select would be fine.

Rob

Re: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML

[Rob Herring]

On Tue, Feb 25, 2025 at 10:44:53AM +0000, J. Neuschäfer wrote:
> On Fri, Feb 21, 2025 at 05:35:23PM -0600, Rob Herring wrote:
> > On Thu, Feb 20, 2025 at 06:29:23PM +0100, J. Neuschäfer wrote:
> > > Add a binding for the "Gianfar" ethernet controller, also known as
> > > TSEC/eTSEC.
> > > 
> > > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > > ---
> > >  .../devicetree/bindings/net/fsl,gianfar.yaml       | 242 +++++++++++++++++++++
> > >  .../devicetree/bindings/net/fsl-tsec-phy.txt       |  39 +---
> > >  2 files changed, 243 insertions(+), 38 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/fsl,gianfar.yaml b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..dc75ceb5dc6fdee8765bb17273f394d01cce0710
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/fsl,gianfar.yaml
> > > @@ -0,0 +1,242 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/fsl,gianfar.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Freescale Three-Speed Ethernet Controller (TSEC), "Gianfar"
> [...]
> > > +  "#address-cells": true
> > 
> > enum: [ 1, 2 ]
> > 
> > because 3 is not valid here.
> > 
> > > +
> > > +  "#size-cells": true
> > 
> > enum: [ 1, 2 ]
> > 
> > because 0 is not valid here.
> 
> Good point.
> 
> > 
> > 
> > > +
> > > +  cell-index:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > +  interrupts:
> > > +    maxItems: 3
> > 
> > Based on the if/then schema, you need 'minItems' here if the min is not 3.
> > 
> > Really, move the descriptions here and make them work for the combined 
> > interrupt case (just a guess).
> 
> The difference here (as previously documented in prose) is by device
> variant:
> 
>  for FEC:
> 
>    - one combined interrupt
> 
>  for TSEC, eTSEC:
> 
>    - transmit interrupt
>    - receive interrupt
>    - error interrupt
> 
> Combining these cases might look like this, not sure if it's good:
> 
>   interrupts:
>     minItems: 1
>     description:
>       items:
>         - Transmit interrupt or combined interrupt
>         - Receive interrupt
>         - Error interrupt

Yep, that's good. I would say 'single combined' to make it abundantly 
clear.

Rob