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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The schema incorrectly uses `additionalProperties: false` while referencing a common schema (`snps,dwc3-common.yaml#`), breaking property inheritance and leading to redundant property declarations.
- [Low] Mandating software workaround quirks (`snps,dis_u2_susphy_quirk`, `snps,dis_u3_susphy_quirk`) in the `required` list.
--

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

dt-bindings: usb: add Altera Agilex5 DWC3 controller

This commit adds a device tree binding for the Altera Agilex5 SoC integration
of the Synopsys DWC3 USB controller.

> 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

[ ... ]

> +  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
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - phys
> +  - phy-names
> +  - resets
> +  - reset-names
> +  - snps,dis_u2_susphy_quirk
> +  - snps,dis_u3_susphy_quirk

[Severity: Low]
Are these software workaround quirks strictly required for all hardware configurations?

Quirks are meant to be optional software workarounds for specific boards or
silicon revisions. If every device matching altr,agilex5-dwc3 intrinsically
requires these suspend PHY quirks, shouldn't the driver apply them natively
in code when it matches the compatible string, rather than forcing device
tree authors to manually include these workaround flags in every node?

> +
> +additionalProperties: false

[Severity: Low]
Does this strict inheritance restriction cause the redundant definitions above?

In JSON schema, additionalProperties: false ignores properties validated by
allOf $ref schemas like snps,dwc3-common.yaml. This appears to force
redundantly redefining standard properties like dr_mode, maximum-speed, and
the suspend PHY quirks as true in the local properties block just to pass
validation.

Could this be changed to unevaluatedProperties: false to allow removing the
redundant local declarations while properly inheriting the schema definitions?

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