Re: [PATCH v6 1/4] dt-bindings: usb: add Altera Agilex5 DWC3 controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The binding uses `additionalProperties: false` alongside an `allOf: - $ref` to a common schema, which breaks inheritance of common properties.
--

commit 6228c1d4fc3d3337f401a374921c27cd7ee4aba2
Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

dt-bindings: usb: add Altera Agilex5 DWC3 controller

Add a binding for the Altera Agilex5 SoC integration of the Synopsys DWC3
USB controller (compatible "altr,agilex5-dwc3"), covering clocks, resets,
PHYs, and optional IOMMU support.

> diff --git a/Documentation/devicetree/bindings/usb/altr,agilex5-dwc3.yaml b/Documentation/devicetree/bindings/usb/altr,agilex5-dwc3.yaml
> new file mode 100644
> index 0000000000000..610ddcb632e28
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/altr,agilex5-dwc3.yaml

[ ... ]

> +allOf:
> +  - $ref: snps,dwc3-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: altr,agilex5-dwc3

[ ... ]

> +  reset-names:
> +    items:
> +      - const: core
> +      - const: ecc
> +
> +  dr_mode: true
> +
> +  maximum-speed: true
> +
> +  snps,dis_u2_susphy_quirk: true
> +
> +  snps,dis_u3_susphy_quirk: true

[Severity: Medium]
Are these properties redundantly declared as true to work around the
limitations of additionalProperties being set to false?

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - phys
> +  - phy-names
> +  - resets
> +  - reset-names
> +  - snps,dis_u2_susphy_quirk
> +  - snps,dis_u3_susphy_quirk
> +
> +additionalProperties: false

[Severity: Medium]
Does this strict constraint incorrectly reject inherited properties from the
DWC3 common schema?

Since this binding uses an allOf inclusion for snps,dwc3-common.yaml, using
additionalProperties: false breaks the inheritance of common properties. Valid
common properties from generic DWC3 or USB schemas will be wrongly rejected by
dtbs_check unless explicitly duplicated in this file.

Should this be unevaluatedProperties: false instead, which would allow
evaluating properties from the included common schema and remove the need to
redundantly declare the inherited properties above?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788886615.git.adrian.ho.yin.ng@altera.com?part=1