Re: [PATCH v2 net-next 2/7] dt-bindings: net: dsa: mediatek,mt7530: add passthrough mode

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 2

- [Low] The added `description` paragraph in
  Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml (lines…
- [Low] Concern (retained for policy combination, assessed as a false
  positive): the binding advertises a passthrough mode that…

--

dt-bindings: net: dsa: mediatek,mt7530: add passthrough mode

This adds a paragraph to the mediatek,mt7530 binding description explaining
that MT7530/MT7531 switches can be operated in a "passthrough" mode where
traffic moves unconditionally between the CPU port and a DSA link port to a
second switch. The paragraph also states that in this mode the upstream
switch may have no other ports or switch configuration.

> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> index 90b3582b76190..e29fb86934981 100644
> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> @@ -73,6 +73,13 @@ description: |
>  
>        Check out example 7.
>  
> +  The MT7530 and MT7531 switches are capable of being operated in "passthrough"
> +  mode. In this mode, traffic is unconditionally passed between the CPU port
> +  and the DSA port which links to a second switch. To use passthrough mode, the
> +  upstream switch is configured with a DSA link to the downstream switch. In
> +  passthrough mode the upstream switch is restricted, with no other ports or
> +  switch configuration allowed.
> +

[Severity: Low]
The last sentence states a hard device tree constraint, but it only exists
as prose.  Should this restriction also be expressed in the
machine-checkable part of the schema?

As it stands there is no new property, no if/then under allOf, and no $defs
entry that limits the port set when a port carries a link phandle.  The port
container in this same file is permissive:

  patternProperties:
    "^(ethernet-)?ports$":
      type: object
      additionalProperties: true

      patternProperties:
        "^(ethernet-)?port@[0-6]$":
          type: object
          additionalProperties: true

so a device tree describing an upstream MT7530/MT7531 with a CPU port, a DSA
link port and extra user ports, plus gpio-controller/interrupt-controller,
still passes dt_binding_check.  The layout is instead rejected at probe time
by mt753x_check_passthrough() in drivers/net/dsa/mt7530.c, added by the
following patch in the series:

	if (passthrough_ports > 2 ||
	    (passthrough_ports == 2 && enabled_ports != 2)
	) {
		dev_err(dev, "Only two ports allowed in passthrough mode\n");
		return -EINVAL;

Two smaller points on the same paragraph:

Every other topology description in this description block ends with a
pointer to a validated example ("Check out example 7." just above).  Could
this one gain an example too, so the passthrough layout is actually exercised
by dt_binding_check?

The per-port reg description further down still reads:

          reg:
            description:
              Port address described must be 5 or 6 for CPU port and from 0 to 5
              for user ports.

which never mentions the DSA link port this new mode depends on.  The later
patch in the series constrains that link port to reg = 6 via the
mt7530-passthrough-dsa-port $def, which does not match the text above.
Should the reg description be updated at the same time?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915174643.253660-1-cjd%40cjdns.fr