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

J. Neuschäfer via B4 Relay posted 3 patches 9 months, 4 weeks ago
There is a newer version of this series
[PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML
Posted by J. Neuschäfer via B4 Relay 9 months, 4 weeks ago
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
Posted by kernel test robot 9 months, 3 weeks ago
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 3/3] dt-bindings: net: Convert fsl,gianfar to YAML
Posted by Rob Herring 9 months, 3 weeks ago
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 3/3] dt-bindings: net: Convert fsl,gianfar to YAML
Posted by J. Neuschäfer 9 months, 3 weeks ago
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 3/3] dt-bindings: net: Convert fsl,gianfar to YAML
Posted by Rob Herring 9 months, 3 weeks ago
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
Re: [PATCH 3/3] dt-bindings: net: Convert fsl,gianfar to YAML
Posted by Rob Herring (Arm) 9 months, 4 weeks ago
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
Posted by Andrew Lunn 9 months, 4 weeks ago
> +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
Posted by J. Neuschäfer 9 months, 4 weeks ago
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