Re: [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The schema does not enforce the presence of the `dr_mode` property when `cdns,cdnsp` is matched, despite the commit message explicitly requiring it.
- [Medium] Array bounds (`minItems` and `maxItems`) for `reg`, `reg-names`, and `interrupt-names` are not explicitly constrained inside the conditionals.
- [Medium] Use of the empty schema `{}` as a wildcard for compatible strings breaks dt-schema validation and explicit documentation rules.
--

commit 1f787c873a0b8a21200340bba7dffd95f1916193
Author: Pawel Laszczak <pawell@cadence.com>

dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string

Introduce a new generic fallback compatible string cdns,cdnsp for
Cadence USBSSP controllers without the Dual-Role Device (DRD) register
block. It restricts the resources to host and device configurations,
requiring dr_mode to be explicitly set.

> diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> @@ -17,22 +17,24 @@ description:
>  
>  properties:
>    compatible:
> -    const: cdns,usb3
> +    oneOf:
> +      - const: cdns,usb3
> +      - items:
> +          - {}

[Severity: Medium]
The commit message mentions using an empty schema wildcard {} to allow
future vendor SoC compatibles to be prepended. Does this break dt-schema
meta-validation during dt_binding_check? The tooling usually expects
concrete compatible strings to be explicitly documented via an enum rather
than using a wildcard.

> +          - const: cdns,cdnsp

[ ... ]

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: cdns,cdnsp
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - description: XHCI Host controller registers
> +            - description: DEVICE controller registers
> +        reg-names:
> +          items:
> +            - const: xhci
> +            - const: dev

[Severity: Medium]
In dt-schema, specifying an items list inside conditional branches does not
automatically override the top-level minItems and maxItems length constraints.

Since the top-level schema allows 2 to 3 items, would omitting maxItems: 2
for reg and reg-names in this 'then' branch allow a 3-item configuration
to incorrectly pass validation? Similarly, does omitting minItems: 3 in the
'else' branch allow a 2-item configuration to pass for cdns,usb3?

> +        interrupts:
> +          maxItems: 2
> +        interrupt-names:
> +          items:
> +            - const: host
> +            - const: peripheral
> +        dr_mode:
> +          enum: [host, peripheral]

[Severity: Medium]
The commit message states that dr_mode must be explicitly set to either
'host' or 'peripheral' when this compatible is matched.

Should dr_mode be added to a required list in this block? Without it, the
property remains optional, and device trees missing dr_mode could silently
pass validation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-no_drd_config_v9-v9-0-2512cef10104@cadence.com?part=1