Re: [PATCH v7 1/2] dt-bindings: usb: cdns3: Add cdns,no-drd property

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The schema fails to enforce that `dr_mode` is required when `cdns,no-drd` is present.
- [Medium] The conditional blocks enforce strict array ordering for `reg-names`, directly contradicting the commit message's claim of supporting flexible ordering.
- [Medium] The `if/then/else` logic fails to fully enforce array length constraints for `reg` and `reg-names`, breaking backward compatibility and allowing invalid configurations.
--

commit 27ad576607394b6bdaff814912f5101ce8966dd5
Author: Pawel Laszczak <pawell@cadence.com>

dt-bindings: usb: cdns3: Add cdns,no-drd property

This commit introduces the cdns,no-drd property for Cadence USB controllers
where the dual-role device register block is missing or inaccessible. It
updates the schema to conditionally modify the requirements for reg,
reg-names, interrupts, and dr_mode based on whether this property is present.

> 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 +98,45 @@ allOf:
>    - $ref: usb-drd.yaml#
>    - $ref: usb-xhci.yaml#
>  
> +  - if:
> +      required:
> +        - cdns,no-drd
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - description: XHCI Host controller registers
> +            - description: DEVICE controller registers

Since the top-level schema relaxes maxItems to 3, does this then block
need an explicit maxItems of 2 for reg? 

In JSON schema, defining an items list inside a conditional block does not
implicitly restrict the maximum length, which could allow an invalid third
register item to silently pass validation.

> +        reg-names:
> +          items:
> +            - const: xhci
> +            - const: dev

The commit message states reg-names uses enums to allow flexible ordering
during validation. However, does defining items as an array of constants in
this block enforce a strict positional sequence? 

Tuple validation requires xhci to be the first element and dev to be the
second, which seems to contradict the intended flexibility.

Similarly, does this block also need an explicit maxItems of 2 to match
the expected length?

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

The commit message notes dr_mode must be explicitly set to either host or
peripheral. Does the then block need a required list containing dr_mode to
enforce this? 

Currently, the schema restricts the allowed values but will still pass
validation if a device tree node omits dr_mode entirely.

> +    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

Just like in the then block, does defining items as an array of constants
here prevent flexible ordering by enforcing otg, xhci, and dev in that
exact sequence?

Also, since the top-level schema was relaxed to allow a minimum of 2 items,
do reg and reg-names need an explicit minItems of 3 here? 

Without it, an array of just 2 items could pass validation when cdns,no-drd
is absent, which breaks the intended backward compatibility of requiring all
3 resource sets.

> +        interrupts:
> +          minItems: 3
> +          maxItems: 4

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514-b4-no_drd_config-v7-0-18cc40e363fd@cadence.com?part=1