Re: [PATCH net-next] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] dt-bindings: net: hns-dsaf: mismatched reg and reg-names constraints
- [Medium] dt-bindings: net: hns-dsaf: additionalProperties breaks port $ref
- [Medium] dt-bindings: net: hns-dsaf: missing desc-num and mc-mac-mask constraints
- [Medium] dt-bindings: net: hns-dsaf: buf-size and desc-num missing from required
--

commit d12acf31486c73a836f5b1585aaa908d8317c9a2
Author: Rob Herring (Arm) <robh@kernel.org>

dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema

This commit converts the HiSilicon hns-dsaf binding to DT schema format. It
drops the unused phy-handle at the top level and adds the undocumented
media-type property.

> diff --git a/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
> new file mode 100644
> index 0000000000000..4edc6cafcbed3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
> @@ -0,0 +1,168 @@

[ ... ]

> +  reg:
> +    minItems: 1
> +    items:
> +      - description:
> +          external interface control register base and size (optional, only
> +          used when subctrl-syscon does not exist)
> +      - description:
> +          SerDes base register and size (optional, only used when serdes-syscon
> +          in port node does not exist)
> +      - description:
> +          PPE register base and size
> +      - description:
> +          DSA Fabric base register and size (not required for single-port mode)
> +
> +  reg-names:
> +    minItems: 1
> +    items:
> +      - const: ppe-base
> +      - const: dsaf-base

[Severity: Medium]
Does this create a mismatch between reg and reg-names constraints?

The reg property is defined as a 4-item list with the first two items being
optional, while reg-names only permits 2 items (ppe-base and dsaf-base).

Due to positional schema matching, if a 2-region configuration is provided, it
might erroneously match against the first two reg descriptions instead of
PPE and DSA Fabric. Additionally, valid 4-region configurations will be
rejected because reg-names restricts the list to 2 items.

[ ... ]

> +  buf-size:
> +    description: RX buffer size (bytes)
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  desc-num:
> +    description: number of descriptors in TX and RX queue
> +    $ref: /schemas/types.yaml#/definitions/uint32

[Severity: Medium]
Did this conversion drop the enum constraint for desc-num?

The original text binding enforced explicit enum values (512, 1024, 2048,
4096) which appear to be missing here.

[ ... ]

> +patternProperties:
> +  "^port@[0-5]$":
> +    description: DSA Fabric port node
> +    $ref: ethernet-switch-port.yaml#
> +    additionalProperties: false

[Severity: Medium]
Will this additionalProperties validation break inheritance from the
referenced ethernet-switch-port.yaml# schema?

In dt-schema, combining a $ref with additionalProperties: false causes all
properties defined in the referenced schema to be rejected unless explicitly
redefined. Should this be unevaluatedProperties: false instead?

> +
> +    properties:
> +      reg:
> +        maximum: 5
> +
> +      phy-handle: true

[Severity: Medium]
Is phy-handle: true added here as a workaround for the inheritance issue
mentioned above?

Since phy-handle is defined in the referenced schema, it should be inherited
automatically if unevaluatedProperties is used instead.

[ ... ]

> +      mc-mac-mask:
> +        description: multicast MAC address mask
> +        $ref: /schemas/types.yaml#/definitions/uint8-array
> +        maxItems: 6

[Severity: Medium]
Can this allow arrays of invalid lengths?

The original text binding expected a 6-byte size for the MAC mask.
Without minItems: 6, shorter arrays (e.g., 1 byte) might pass schema
validation.

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - mode

[Severity: Medium]
Should buf-size and desc-num be included in this required block?

The hns_dsaf_main.c driver strictly requires both properties, failing probe
with -EINVAL if either is missing from the device tree node. Omitting them
here means incomplete device tree nodes will pass schema validation but
silently fail at runtime.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925215848.2949157-2-robh@kernel.org?part=1