Re: [PATCH v4 1/2] dt-bindings: usb: cdns3: Add no_drd property

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Schema logic error: lowering `minItems` does not make leading or intermediate array items optional, breaking schema validation when `otg` resources are omitted.
- [Low] The new property `no_drd` violates standard Device Tree property naming conventions.
- [Low] The schema fails to enforce the operational mode restrictions introduced by the `no_drd` property.
--

commit 56705b83ac8b4d7657f3b9eca696b4b8d5225cc7
Author: Pawel Laszczak <pawell@cadence.com>

dt-bindings: usb: cdns3: Add no_drd property

This commit introduces a new boolean property 'no_drd' for Cadence USBSS/USBSSP
controllers to support hardware configurations without a Dual-Role Device (DRD)
register block. It also relaxes the 'reg' and 'interrupts' properties
requirements to make the 'otg' resources optional.

> diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> index 2d95fb7321aff..8c7dc81708fb5 100644
> --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> @@ -20,19 +20,21 @@ properties:
>      const: cdns,usb3
>  
>    reg:
> +    minItems: 2
>      items:
>        - description: OTG controller registers
>        - description: XHCI Host controller registers
>        - description: DEVICE controller registers
>  
>    reg-names:
> +    minItems: 2
>      items:
>        - const: otg
>        - const: xhci
>        - const: dev

Does lowering minItems actually make the leading 'otg' resource optional?

In JSON Schema, array tuple validation matches elements strictly by their
index order. For reg-names, the schema expects 'otg' at index 0 and 'xhci'
at index 1. Lowering minItems to 2 only makes the trailing element ('dev')
optional.

If a device tree omits the 'otg' resource to indicate no DRD block, and
provides reg-names = "xhci", "dev", validation will fail because the first
provided element ("xhci") will be evaluated against const: otg.

To correctly allow omitting specific positional resources, does the schema
need a oneOf construct with explicitly defined arrays of allowed combinations?

[ ... ]
>    interrupt-names:
> -    minItems: 3
> +    minItems: 2
>      items:
>        - const: host
>        - const: peripheral

Similarly, if 'otg' is omitted but the optional 'wakeup' interrupt is
provided, validation will fail because 'wakeup' will be evaluated against
the third element ('otg').

[ ... ]
> @@ -79,6 +81,13 @@ properties:
>      description: Enable resetting of PHY if Rx fail is detected
>      type: boolean
>  
> +  no_drd:

This isn't a bug, but device tree property names typically use hyphens
instead of underscores.

Since this property describes a hardware-specific design omission for the
Cadence USB controller rather than a standard core USB subsystem property,
should it use a vendor prefix and hyphens, such as cdns,no-drd, to comply
with DT schema standards?

> +    description:
> +      Indicates that the Dual-Role Device (DRD) register block is not
> +      implemented or is inaccessible. In this case, the controller
> +      must operate in a fixed peripheral or host mode.
> +    type: boolean

Does the schema enforce the operational mode restrictions introduced by
this property?

The description explicitly states that the controller must operate in a
fixed peripheral or host mode. However, the schema does not enforce this
functional requirement.

Could an allOf block be added to the schema to restrict the dr_mode
property to enum: [host, peripheral] and explicitly forbid usb-role-switch
when this property is present?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-b4-no_drd_config-v4-0-7b4e5dc96f81@cadence.com?part=1