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

[Conor Dooley]

On Tue, May 19, 2026 at 08:47:30AM +0000, Pawel Laszczak wrote:
> >
> >On Fri, May 15, 2026 at 01:24:38PM +0200, Pawel Laszczak via B4 Relay wrote:
> >> From: Pawel Laszczak <pawell@xxxxxxxxxxx>
> >>
> >> Introduce a new compatible string 'cdns,cdnsp-no-drd' for Cadence
> >> USBSS/USBSSP controllers to support hardware configurations where the
> >> Dual-Role Device (DRD) register block is missing or inaccessible.
> >>
> >> This change follows the recommendation to imply hardware limitations
> >> directly from the compatible string rather than using a dedicated
> >> boolean property.
> >>
> >> When 'cdns,cdnsp-no-drd' is used:
> >> - The 'otg' register and interrupt resources are not required.
> >> - The 'reg' and 'interrupts' properties are restricted to 2 items
> >>   (host and device).
> >> - 'dr_mode' must be explicitly set to either 'host' or 'peripheral'.
> >>
> >> The standard 'cdns,usb3' compatible remains unchanged, maintaining
> >> backward compatibility by requiring all 3 resource sets (otg, host, dev).
> >>
> >> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
> >> ---
> >> v8:
> >> - Update commit message to reflect schema changes.
> >> - Removed 'cdns,no-drd' boolean property as per Rob Herring's suggestion.
> >> - Introduced a new compatible string 'cdns,cdnsp-no-drd' for controller
> >>   variants that lack the DRD/OTG register block.
> >>
> >> v7:
> >> - Rename 'no_drd' to 'cdns,no-drd'.
> >> - Update commit message to reflect property renaming and schema
> >changes.
> >> - Simplify 'reg-names' using a single enum.
> >> - Revert 'interrupt-names' to a list of constants.
> >> - Move 'reg' item descriptions to if/else blocks for accuracy.
> >> - Clean up 'if/then' logic (remove redundant checks).
> >> - Add explicit 'items' list for 'interrupt-names' in the 'else' block.
> >>
> >> v6:
> >> - Fixed validation error for 'interrupt-names' by correcting
> >>   the items definition.
> >> - Adjusted 'minItems'/'maxItems' to properly support the optional
> >>  'wakeup' interrupt.
> >> - Fixed 'too long' schema error in examples.
> >>
> >> v5:
> >> - Implemented strict conditional validation using if-then-else logic.
> >> - Enforced 2 register/interrupt items and required 'dr_mode'
> >>   (host or peripheral) when 'no_drd' is present.
> >> - Enforced the standard 3 register/interrupt items (otg, host, dev)
> >>   when 'no_drd' is absent to ensure backward compatibility.
> >> - Updated 'reg-names' and 'interrupt-names' to use enums in the main
> >>   properties section to support flexible resource ordering during
> >>   validation.
> >> ---
> >> ---
> >>  .../devicetree/bindings/usb/cdns,usb3.yaml         | 60
> >++++++++++++++++++----
> >>  1 file changed, 50 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> >b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> >> index 2d95fb7321af..7b0aa9c4a2bd 100644
> >> --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> >> +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> >> @@ -17,22 +17,22 @@ description:
> >>
> >>  properties:
> >>    compatible:
> >> -    const: cdns,usb3
> >> +    enum:
> >> +      - cdns,usb3
> >> +      - cdns,cdnsp-no-drd
> >
> >I doubt this is what Rob meant, he asked for soc-specific compatibles
> >and this is generic. A soc-specific compatible would be something like
> >microchip,newsoc-usb3
> 
> Hi Conor, Rob
> 
> To clarify the hardware situation: I am developing and testing this on
> an FPGA development board connected via pure PCI, without any Device Tree.
> I do not have a specific SoC platform, nor do I know what silicon target
> the end customers will use in the future.
> Introducing a fake SoC-specific compatible string just to pass the DT
> validation feels misleading and unnecessary.
> 
> Since cdns,usb3 is already an established generic compatible, and we
> cannot use a generic configuration string like cdns,cdnsp-no-drd,
> the cleanest way forward is to make this resource-driven instead of
> compatible-driven.
> 
> In v9, I propose to:
>  - Keep only the existing cdns,usb3 compatible.
>  - Update cdns,usb3.yaml to allow minItems: 2 for reg and reg-names
>    if dr_mode is explicitly set to "host" or "peripheral" (indicating
>    that the OTG register block is absent).
>  - In the driver code, determine the lack of DRD dynamically if the "otg"
>    resource/register block is missing.
> 
> This elegantly solves the issue for my PCI/FPGA platform (where I just
> won't pass the "otg" resource), complies with DT practices, and leaves
> the door open for any future customer SoC to use cdns,usb3 with
> standard dr_mode constraints.
> 
> Does this approach look acceptable to you?

Not really. I want enforcement of the correct properties in the binding,
not permission of multiple different combinations.

Perhaps we could go back to the original cdns,usbssp compatible and
leave an empty slot for the soc-specific compatibles, like:
      - items:
          - {}
          - const: sifive,clint2        # SiFive CLINT v2 IP block

That way you can proceed with this effort without a soc-compatible
but we don't end up permitting a generic compatible.

Cheers,
Conor.

> >>    reg:
> >> -    items:
> >> -      - description: OTG controller registers
> >> -      - description: XHCI Host controller registers
> >> -      - description: DEVICE controller registers
> >> +    minItems: 2
> >> +    maxItems: 3
> >>
> >>    reg-names:
> >> +    minItems: 2
> >> +    maxItems: 3
> >>      items:
> >> -      - const: otg
> >> -      - const: xhci
> >> -      - const: dev
> >> +      enum: [ otg, xhci, dev ]
> >>
> >>    interrupts:
> >> -    minItems: 3
> >> +    minItems: 2
> >>      items:
> >>        - description: XHCI host controller interrupt
> >>        - description: Device controller interrupt
> >> @@ -41,7 +41,7 @@ properties:
> >>                       cleared by xhci core, this interrupt is optional
> >>
> >>    interrupt-names:
> >> -    minItems: 3
> >> +    minItems: 2
> >>      items:
> >>        - const: host
> >>        - const: peripheral
> >> @@ -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
> >> +        reg-names:
> >> +          items:
> >> +            - const: xhci
> >> +            - const: dev
> >> +        interrupts:
> >> +          maxItems: 2
> >> +        interrupt-names:
> >> +          items:
> >> +            - const: host
> >> +            - const: peripheral
> >> +        dr_mode:
> >> +          enum: [host, peripheral]
> >> +    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
> >> +        interrupts:
> >> +          minItems: 3
> >> +          maxItems: 4
> >> +        interrupt-names:
> >> +          minItems: 3
> >> +
> >>  unevaluatedProperties: false
> >>
> >>  examples:
> >>
> >> --
> >> 2.43.0
> >>
> >>

Attachment: signature.asc
Description: PGP signature