Re: [PATCH v4 1/2] dt-bindings: net: airoha: Add EN7581 ethernet controller

[Rob Herring <robh@kernel.org>]

On Sat, Jun 29, 2024 at 05:01:37PM +0200, Lorenzo Bianconi wrote:
> Introduce device-tree binding documentation for Airoha EN7581 ethernet
> mac controller.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  .../bindings/net/airoha,en7581-eth.yaml       | 171 ++++++++++++++++++
>  1 file changed, 171 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> new file mode 100644
> index 000000000000..e2c0da02ccf2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> @@ -0,0 +1,171 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/airoha,en7581-eth.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Airoha EN7581 Frame Engine Ethernet controller
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +
> +maintainers:
> +  - Lorenzo Bianconi <lorenzo@kernel.org>
> +
> +description:
> +  The frame engine ethernet controller can be found on Airoha SoCs.
> +  These SoCs have dual GMAC ports.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - airoha,en7581-eth
> +
> +  reg:
> +    items:
> +      - description: Frame engine base address
> +      - description: QDMA0 base address
> +      - description: QDMA1 base address
> +
> +  reg-names:
> +    items:
> +      - const: fe
> +      - const: qdma0
> +      - const: qdma1
> +
> +  interrupts:
> +    items:
> +      - description: QDMA lan irq0
> +      - description: QDMA lan irq1
> +      - description: QDMA lan irq2
> +      - description: QDMA lan irq3
> +      - description: QDMA wan irq0
> +      - description: QDMA wan irq1
> +      - description: QDMA wan irq2
> +      - description: QDMA wan irq3
> +      - description: FE error irq
> +      - description: PDMA irq
> +
> +  resets:
> +    maxItems: 8
> +
> +  reset-names:
> +    items:
> +      - const: fe
> +      - const: pdma
> +      - const: qdma
> +      - const: xsi-mac
> +      - const: hsi0-mac
> +      - const: hsi1-mac
> +      - const: hsi-mac
> +      - const: xfp-mac
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^mac@[1-4]$":

ethernet@

> +    type: object
> +    unevaluatedProperties: false
> +    allOf:

Can drop 'allOf'

> +      - $ref: ethernet-controller.yaml#

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

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

Based on the unit-address, you need instead:

minimum: 1
maximum: 4

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

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

mdio {

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

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