Re: [PATCH net-next v10 1/4] dt-bindings: net: dsa: microchip: Add KSZ8995XA

[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: 1 · Low: 1

- [Medium] In
  Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml the new…
- [Low] Reported concern (validated as NOT a defect):
  `microchip,ksz8995xa` is documented in…

--

dt-bindings: net: dsa: microchip: Add KSZ8995XA

This adds the compatible microchip,ksz8995xa to the Microchip KSZ DSA
switch binding, described as having the same properties as the KS8995XA
and being like the KSZ8795 and KSZ8864.

> diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> index 8d4a3a9a33fcc..ed39530b6cbbb 100644
> --- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> @@ -23,6 +23,7 @@ properties:
>        - microchip,ksz8864  # 4-port version of KSZ8895 family switch
>        - microchip,ksz8873
>        - microchip,ksz8895  # 5-port version of KSZ8895 family switch
> +      - microchip,ksz8995xa
>        - microchip,ksz9477
>        - microchip,ksz9897
>        - microchip,ksz9896
> @@ -122,6 +123,7 @@ allOf:
>              enum:
>                - microchip,ksz8863
>                - microchip,ksz8873
> +              - microchip,ksz8995xa
>      then:
>        $ref: dsa.yaml#/$defs/ethernet-ports
>      else:

[Severity: Medium]
Is this second hunk intentional?  The enum being extended here sits under
a "not:", so adding a compatible to it selects the "else" branch rather
than the "then" branch:

  - if:
      not:
        properties:
          compatible:
            enum:
              - microchip,ksz8863
              - microchip,ksz8873
              - microchip,ksz8995xa
    then:
      $ref: dsa.yaml#/$defs/ethernet-ports
    else:
      patternProperties:
        "^(ethernet-)?ports$":
          patternProperties:
            "^(ethernet-)?port@[0-2]$":

The "else" branch is the hand-rolled KSZ88x3 schema.  It only matches
port@0 through port@2, does not reference dsa.yaml#, and additionally
allows the KSZ88x3/REFCLKO-only flag microchip,rmii-clk-internal.

The KSZ8995XA is a 5-port part with the CPU port at index 4.  The chip
data added later in this series, ksz_switch_chips[KSZ8995XA] in
drivers/net/dsa/microchip/ksz_common.c, has:

	.cpu_ports = 0x10,	/* can be configured as cpu port */
	.port_cnt = 5,		/* total cpu and user ports */

So for microchip,ksz8995xa, would port@3 and port@4 (including the MII
CPU port with ethernet/phy-mode/fixed-link) match no subschema and go
unvalidated, and would a node with no ports/ethernet-ports at all also
pass since the dsa.yaml base requirements are no longer applied?

The same silicon is also described by
Documentation/devicetree/bindings/net/dsa/micrel,ks8995.yaml, which
drivers/net/dsa/microchip/ksz_spi.c maps to the same
ksz_switch_chips[KSZ8995XA] entry.  That file uses:

  allOf:
    - $ref: dsa.yaml#/$defs/ethernet-ports

and its example documents ethernet-port@0 through ethernet-port@4, which
is the opposite of the 3-port constraint now applied to
microchip,ksz8995xa.

This also seems to disagree with the commit message, which says the part
is "just like the KSZ8795 and KSZ8864" and "has the same properties as
the KS8995XA" - both microchip,ksz8795 and microchip,ksz8864 take the
"then" branch and use the generic dsa.yaml ethernet-ports schema.

Would dropping the second hunk and keeping only the top-level compatible
enum addition give the described behaviour?

The same negated enum entry is still present at the end of the series
(c34290e766e1566f450a527435a9398d135e21d4), so no later patch appears to
change it.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-ks8995-to-ksz8-v10-0-7379239eb65f%40kernel.org