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

[Rob Herring <robh@kernel.org>]

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>;
> +            };
> +        };
> +    };
> +
> +...