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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The 'dr_mode' property is not explicitly enforced as required for 'cdns,cdnsp-no-drd', contradicting the commit message.
- [Medium] Missing `minItems` and `maxItems` limits in the conditional blocks allow incomplete or over-specified property arrays to silently pass validation.
--

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

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

Introduce a new compatible string cdns,cdnsp-no-drd for controllers
missing the DRD register block. This restricts register and interrupt
resources to 2 items and specifies dr_mode requirements. The standard
cdns,usb3 maintains its existing 3-resource requirement.

> 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
[ ... ]
> @@ -93,6 +93,46 @@ allOf:
>    - $ref: usb-drd.yaml#
>    - $ref: usb-xhci.yaml#
>  
> +  - if:
> +      properties:
> +        compatible:
> +          const: cdns,cdnsp-no-drd
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - description: XHCI Host controller registers
> +            - description: DEVICE controller registers

The top-level schema allows up to 3 items for reg. Since providing an items
list doesn't implicitly restrict the overall array length in dt-schema,
could a node with 3 registers incorrectly pass validation here?

Should maxItems: 2 be explicitly added to reg and reg-names in this block?

> +        reg-names:
> +          items:
> +            - const: xhci
> +            - const: dev
> +        interrupts:
> +          maxItems: 2
> +        interrupt-names:
> +          items:
> +            - const: host
> +            - const: peripheral

Similarly, could interrupt-names also pass validation with 3 items because
the top-level allows it? Should maxItems: 2 be added here as well?

> +        dr_mode:
> +          enum: [host, peripheral]

The commit message states that dr_mode must be explicitly set to either host
or peripheral. Does the schema enforce this requirement?

Since dr_mode is only constrained by an enum and not included in a required
list, a device tree omitting the property entirely will still pass
validation. This allows an invalid configuration where the USB core might
fall back to the default otg mode on hardware that lacks DRD capabilities.
Should a required list containing dr_mode be added to this block?

> +    else:
> +      properties:
> +        reg:
> +          items:
> +            - description: OTG controller registers
> +            - description: XHCI Host controller registers
> +            - description: DEVICE controller registers
> +        reg-names:
> +          items:
> +            - const: otg
> +            - const: xhci
> +            - const: dev

The top-level schema was relaxed to allow a minimum of 2 items for reg and
reg-names. Without minItems: 3 specified here, could a malformed legacy
cdns,usb3 node with only 2 registers pass validation?

Should minItems: 3 be added to reg and reg-names in this else block to
maintain backward compatibility?

> +        interrupts:
> +          minItems: 3
> +          maxItems: 4
> +        interrupt-names:
> +          minItems: 3
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-b4-no_drd_config-v8-0-75cb05317679@cadence.com?part=1