Re: [PATCH net-next 6/6] dt-bindings: net: dsa: mv88e6xxx: add mv88e6xxx-avb-mode property

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Encoding runtime network configuration (traffic policy) in the Device Tree.
- [Low] Schema validation fails for all valid inputs due to an array/integer type mismatch.
- [Low] The Device Tree binding description violates OS-agnostic rules by referencing Linux-specific netlink flags.
--

commit aae733d9f490b168e212410379f1d8278d34ee3c
Author: Luke Howard <lukeh@padl.com>

dt-bindings: net: dsa: mv88e6xxx: add mv88e6xxx-avb-mode property

This commit adds a vendor-specific marvell,mv88e6xxx-avb-mode property to the
Marvell MV88E6xxx DSA switch binding. The property configures the handling of
frames with non-AVB priorities and destination addresses into Standard,
Enhanced, or Secure modes.

> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell,mv88e6xxx.yaml b/Documentation/devicetree/bindings/net/dsa/marvell,mv88e6xxx.yaml
> index 19ae600e93394..1a01810e7682c 100644
> --- a/Documentation/devicetree/bindings/net/dsa/marvell,mv88e6xxx.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/marvell,mv88e6xxx.yaml
> @@ -97,6 +97,32 @@ properties:
>      required:
>        - compatible
>  
> +  marvell,mv88e6xxx-avb-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32

[Severity: Medium]
Does adding this property encode dynamic network configuration policy into the
Device Tree?

Device trees are typically meant for static hardware description. Setting how
AVB frames are handled and whether frames are dropped appears to be a runtime
traffic policy.

Shouldn't this be managed via standard Linux networking UAPIs like devlink,
bridge attributes, or tc, rather than being hardcoded in the DT?

> +    description:
> +      Marvell MV88E6xxx switches that support Audio Video Bridging /
> +      Time-Sensitive Networking (AVB/TSN) traffic prioritization can have
> +      ports configured in one of several modes. These modes control the
> +      handling of frames with non-AVB frame priorities and destination
> +      addresses.
> +    oneOf:
> +      - description:
> +          Standard Mode. Frames whose priority is mapped to an AVB
> +          traffic class (TC) are considered AVB frames; other frames are
> +          considered legacy (non-AVB).
> +        const: 0
> +      - description:
> +          Enhanced Mode. Frames whose priority is mapped to an AVB TC
> +          and for which a static FDB or MDB entry exists are considered
> +          AVB frames. Frames with an AVB TC but no matching FDB or MDB
> +          entry are dropped. AVB MDB entries can be added using the
> +          MDB_FLAGS_STREAM_RESERVED netlink flag.

[Severity: Low]
Does referencing MDB_FLAGS_STREAM_RESERVED violate the OS-agnostic rules for
Device Tree bindings?

This leaks Linux-specific netlink implementation details directly into the
hardware specification.

> +        const: 1

[Severity: Low]
Will this schema fail validation for all valid inputs due to an array and
integer type mismatch?

The uint32 reference above expects the property to be parsed as a
single-element array, but this applies a const constraint for an integer.
In JSON schema, an array does not match an integer.

Should this use enum: [0, 1, 2] instead, or place the const inside an items
array so it passes the binding checks?

> +      - description:
> +          Secure Mode. As Enhanced Mode, but the matching FDB or MDB
> +          entry must also have the source port's bit set.
> +        const: 2

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1779841530.git.lukeh@padl.com?part=6