Re: [PATCH net-next v4 06/14] dt-bindings: net: dsa: renesas,rzn1-a5psw: Add RZ/T2H and RZ/N2H ETHSW support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: net: dsa: rzn1-a5psw: split ETHSW into separate schema
--

commit 58ade097798a6648dae56580c5e1d16da26de07a
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

dt-bindings: net: dsa: renesas,rzn1-a5psw: Add RZ/T2H and RZ/N2H ETHSW support

This commit extends the Renesas A5PSW DSA binding schema to cover the ETHSW
variant found on RZ/T2H and RZ/N2H SoCs, adding compatible strings and
per-variant constraints for interrupts, clocks, resets, and ports.

> diff --git a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
> index 250c59e58439b..338e499b42b1c 100644
> --- a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
> @@ -4,43 +4,108 @@
>  $id: http://devicetree.org/schemas/net/dsa/renesas,rzn1-a5psw.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Renesas RZ/N1 Advanced 5 ports ethernet switch
> +title: Renesas RZ/N1 A5PSW and RZ/T2H, RZ/N2H ETHSW Ethernet Switch
>  
>  maintainers:
>    - Clément Léger <clement.leger@bootlin.com>
> +  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>  
> -description: |
> -  The advanced 5 ports switch is present on the Renesas RZ/N1 SoC family and
> -  handles 4 ports + 1 CPU management port.
> +description: >
> +  This binding describes the Ethernet switch IPs used on Renesas SoCs:
>  
> -allOf:
> -  - $ref: dsa.yaml#/$defs/ethernet-ports
> +  - The A5PSW (Advanced 5-Port Switch) found on the RZ/N1 family, which
> +    provides 4 external ports and 1 CPU/management port.
> +  - The ETHSW (Ethernet Switch) found on the RZ/T2H and RZ/N2H families,
> +    which is derived from the A5PSW IP with some register layout
> +    differences, additional timestamping support, and a total of 4 ports
> +    including the CPU/management port.
>  
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -          - renesas,r9a06g032-a5psw
> -      - const: renesas,rzn1-a5psw
> +    oneOf:
> +      - items:
> +          - enum:
> +              - renesas,r9a06g032-a5psw
> +          - const: renesas,rzn1-a5psw
> +
> +      - const: renesas,r9a09g077-ethsw
> +
> +      - items:
> +          - const: renesas,r9a09g087-ethsw
> +          - const: renesas,r9a09g077-ethsw

[ ... ]

> @@ -94,6 +166,48 @@ required:
>    - clock-names
>    - power-domains
>    - ethernet-ports
> +  - interrupts
> +  - interrupt-names
> +
> +allOf:
> +  - $ref: dsa.yaml#/$defs/ethernet-ports
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a09g077-ethsw
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 24
> +        interrupt-names:
> +          minItems: 24
> +        clocks:
> +          minItems: 3
> +        clock-names:
> +          minItems: 3
> +        ethernet-ports:
> +          patternProperties:
> +            "^(ethernet-)?port@[0-3]$":
> +              properties:
> +                reg:
> +                  items:
> +                    - maximum: 3
> +            "^(ethernet-)?port@4$": false
> +      required:
> +        - resets
> +    else:
> +      properties:
> +        interrupts:
> +          maxItems: 5
> +        interrupt-names:
> +          maxItems: 5
> +        clocks:
> +          maxItems: 2
> +        clock-names:
> +          maxItems: 2
> +        resets: false

[Severity: Low]
This isn't a bug, but would it be cleaner to split the RZ/T2H and RZ/N2H
ETHSW binding into a separate schema document (such as
renesas,r9a09g077-ethsw.yaml) instead of combining it with
renesas,rzn1-a5psw.yaml?

In Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml, the
ETHSW compatibles (renesas,r9a09g077-ethsw and renesas,r9a09g087-ethsw) do
not share a fallback compatible with renesas,rzn1-a5psw, and nearly every
property in the schema differs between the two variants:

- interrupts and interrupt-names have different item counts (24 vs 5),
  names, and ordering (switch then dlr on ETHSW vs dlr then switch on
  A5PSW), requiring separate oneOf branches.
- clocks and clock-names have different item counts (3 including ts vs 2).
- resets is required on ETHSW and disallowed on A5PSW.
- ethernet-ports allows ports 0-3 (disallowing port@4 and restricting reg
  to maximum 3) on ETHSW versus ports 0-4 on A5PSW.

Because the allOf if-then-else conditional has to override almost every
property in the file, splitting ETHSW into its own schema file would avoid
the per-variant conditional complexity.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104182105.1102555-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=6