Re: [PATCH net-next v3 3/8] dt-bindings: net: dsa: Add SoC-e SWIP switch

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Documentation/devicetree/bindings/net/dsa/soce,swip.yaml accepts
  the MMIO child nodes mdio@204 and mdio-mux@202 but does not require…
- [Low] In Documentation/devicetree/bindings/net/dsa/soce,swip.yaml, the
  per-port constraints (the dsa-port.yaml $ref, `reg` maximum 30,…

--

dt-bindings: net: dsa: Add SoC-e SWIP switch

This commit adds a DT binding for SoC-e FPGA-based Ethernet switch IP
cores. The compatible string encodes the layout of the identification
registers. The binding describes the Ethernet ports and the optional
integrated MDIO controller with its MMIO mux.

> diff --git a/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml b/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml
> new file mode 100644
> index 0000000000000..d614fb2a29d3d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml

[ ... ]

> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +  ranges: true
> +
> +  mdio@204:
> +    $ref: /schemas/net/soce,swip-mdio.yaml#
> +    unevaluatedProperties: false
> +    description:
> +      Integrated MDIO controller bus on the master side of the mux.
> +
> +  mdio-mux@202:
> +    $ref: /schemas/net/mdio-mux-mmioreg.yaml#
> +    unevaluatedProperties: false
> +
> +  ethernet-ports:
> +    type: object
> +    patternProperties:
> +      '^ethernet-port@[0-9a-f]+$':
> +        type: object
> +        $ref: dsa-port.yaml#
> +        unevaluatedProperties: false

[Severity: Low]
Should this pattern be '^(ethernet-)?port@[0-9a-f]+$'?

This schema includes ethernet-switch.yaml through dsa.yaml. That file
also accepts port@N nodes under ethernet-ports, with only a type check:

    '^(ethernet-)?port@[0-9a-f]+$':
      type: object
      description: Ethernet switch ports

As a result, a node named port@3 counts as evaluated and passes
validation. None of the checks here apply to it: the dsa-port.yaml
reference, the reg maximum, the required reg and phy-mode, and the
phy-mode enum.

dsa_switch_parse_ports_of() in net/dsa/dsa.c still uses such a node. It
walks every available child with for_each_available_child_of_node() and
does not check node names. An unvalidated phy-mode can then reach
soce_phylink_get_caps(), which only handles the listed modes.

The other DSA bindings that constrain port nodes (lantiq,gswip, mt7530,
ksz, sja1105, qca8k and others) all use '^(ethernet-)?port@[0-9a-f]+$'.

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - ethernet-ports

[Severity: Low]
When the mdio@204 or mdio-mux@202 children are present, should ranges,
#address-cells and #size-cells be required?

As written, a switch node that has those MMIO children but no ranges
still passes dt_binding_check. The kernel then cannot translate the
children's reg:

drivers/of/address.c:__of_translate_address() {
    ...
	ranges = of_get_property(parent, rprop, &rlen);
	if (ranges == NULL && !of_empty_ranges_quirk(parent) &&
	    strcmp(rprop, "dma-ranges")) {
		pr_debug("no ranges; cannot translate\n");
		return 1;
	}
    ...
}

So mdio_mux_mmioreg_probe() fails in of_address_to_resource() with
"could not obtain memory map". soce_mdio_iomap() in
drivers/net/mdio/mdio-soce.c fails the same way. No PHY behind the mux
can then be reached.

dtc already warns about missing cell properties, but it does not warn
about a missing ranges.

Would a dependentRequired (or dependencies) entry help here? It could
make mdio-mux@202 and mdio@204 require ranges, #address-cells and
#size-cells.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923-devel-vstrassheim-soce-dsa-ml-v3-0-ddebafcb9ba7%40linutronix.de