Re: [PATCH 2/4] dt-bindings: net: dsa: Add SoC-e switch IP and DSA bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Medium] The `mdio` bus models a Linux software-synthetic topology rather than the actual hardware layout, misusing `reg` and inventing custom physical address properties.
- [Medium] Setting `unevaluatedProperties: false` on the child `ethernet-phy` node breaks validation for vendor-specific PHY properties.
- [Low] The device tree example in `soce,switch-dsa.yaml` fails to compile because it references undefined phandle labels (`&soce_switch_ip` and `&gem3`).
- [Low] The device tree example in `soce,switch-ip.yaml` provides a 4-cell `reg` property without a parent node specifying `#address-cells` and `#size-cells`, leading to validation failures.
- [Low] The `ports` sub-node pattern property description claims to define vendor-specific properties, but none are actually listed or defined.
- [Low] Unnecessary `#address-cells` and `#size-cells` defined at the root node level, causing dtc warnings.
- [Low] The `soce,num-ports` property is missing constraints that are explicitly stated in its description.
--

--- Patch [2]: [PATCH 2/4] dt-bindings: net: dsa: Add SoC-e switch IP and DSA bindings ---
commit 28c590589436788e107ab82bf7fbd0fdcad40087
Author: Vasilij Strassheim <v.strassheim@linutronix.de>
Subject: dt-bindings: net: dsa: Add SoC-e switch IP and DSA bindings

This commit adds DT schema documentation for SoC-e FPGA switch devices.
The binding is split into a switch IP node describing the MMIO register
block and a DSA MDIO node describing the DSA-facing switch instance.

> diff --git a/Documentation/devicetree/bindings/net/dsa/soce,switch-dsa.yaml b/Documentation/devicetree/bindings/net/dsa/soce,switch-dsa.yaml
> new file mode 100644
> index 0000000000000..a04e2838d14eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/soce,switch-dsa.yaml
> @@ -0,0 +1,195 @@

[ ... ]

> +properties:
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0

[Severity: Low]
Are #address-cells and #size-cells necessary at the root node level?
Since the only allowed sub-nodes (ports and mdio) do not use a @
unit-address, compiling device trees conforming to this schema might emit
avoid_unnecessary_addr_size dtc warnings.

[ ... ]

> +  mdio:
> +    $ref: /schemas/net/mdio.yaml#
> +    description: |
> +      Optional child MDIO bus exposing PHY devices attached to switch user
> +      ports. PHY nodes referenced by port "phy-handle" properties should be
> +      placed here. The PHY address seen by Linux on this bus is the switch port
> +      index, not necessarily the hardware PHY address selected internally by
> +      the switch IP.

[Severity: Medium]
Does this model a Linux software-synthetic topology rather than the actual
hardware layout? Device tree bindings are expected to describe the hardware,
rather than modeling internal software abstractions like Linux's synthetic
MDIO bus mapping. Using custom properties for the hardware bus index instead
of natural mdio subnodes might violate core DT design principles.

> +    unevaluatedProperties: false
> +
> +    patternProperties:
> +      "^ethernet-phy@[0-9a-f]+$":
> +        type: object
> +        $ref: /schemas/net/ethernet-phy.yaml#
> +        unevaluatedProperties: false

[Severity: Medium]
Will setting unevaluatedProperties: false on the child ethernet-phy node
break validation for valid vendor-specific PHY properties? Device trees using
specific PHY hardware with valid vendor properties (like ti,rx-internal-delay)
might fail schema validation because the parent schema strictly forbids them.

[ ... ]

> +  ports:
> +    type: object
> +    description: Container node for the switch port sub-nodes.
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      "^port@[0-9a-f]+$":
> +        $ref: dsa-port.yaml#
> +        unevaluatedProperties: false
> +        description: |
> +          Switch port. In addition to the standard DSA port properties
> +          (label, phy-mode, ethernet, fixed-link) the following
> +          vendor-specific properties are used for data ports connected to
> +          external PHYs.

[Severity: Low]
Could this description be updated? It claims to define vendor-specific
properties used for data ports connected to external PHYs, but the description
abruptly ends and no such properties are actually listed or defined.

[ ... ]

> +examples:
> +  - |
> +    /* Excerpt from a Xilinx ZynqMP design (ARM64) */
> +    mdio {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        soce_switch: switch@10 {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            compatible = "soce,switch-dsa";
> +            reg = <0x10>;
> +            soce,switch-ip = <&soce_switch_ip>;

[Severity: Low]
Does this example fail to compile? The phandle label &soce_switch_ip
does not appear to be defined in this example snippet, which might cause
dt_binding_check to fail when dtc attempts to resolve it.

[ ... ]

> +                port@2 {
> +                    reg = <0x2>;
> +                    label = "INTERNAL_PORT";
> +                    ethernet = <&gem3>;

[Severity: Low]
Similar to &soce_switch_ip, the &gem3 ethernet controller label also
appears to be undefined in this example.

> diff --git a/Documentation/devicetree/bindings/net/dsa/soce,switch-ip.yaml b/Documentation/devicetree/bindings/net/dsa/soce,switch-ip.yaml
> new file mode 100644
> index 0000000000000..ba9bb396235f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/soce,switch-ip.yaml
> @@ -0,0 +1,54 @@

[ ... ]

> +  soce,num-ports:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Total number of switch ports (external/user ports plus the CPU uplink
> +      port). Must be greater than zero and not exceed the selected IP variant.

[Severity: Low]
Should there be explicit JSON schema constraints (such as minimum: 1)
added for this property? The description mentions it must be greater than
zero, but invalid device trees specifying soce,num-ports = <0>; might
pass validation without the constraint.

[ ... ]

> +examples:
> +  - |
> +    switch_ip: switch@80020000 {
> +        compatible = "soce,mrs-24-01";
> +        reg = <0x0 0x80020000 0x0 0x10000>;

[Severity: Low]
Will this 4-cell reg property cause a validation failure? The implicit
parent provides #address-cells = <1>, so this might parse as 2 items and
exceed the maxItems: 1 constraint. It likely requires a parent node
(e.g., a bus node) defining #address-cells = <2> and #size-cells = <2>.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-devel-vstrassheim-soce-dsa-ml-v1-0-be569dae1b20@linutronix.de?part=2