Re: [PATCH 2/2] dt-bindings: usb: cdns,usb3: Add support for USBSSP

[Peter Chen]

On 26-03-02 08:28:07, Krzysztof Kozlowski wrote:
> 
> On Mon, Mar 02, 2026 at 11:03:34AM +0800, Peter Chen wrote:
> > Extend the Cadence USBSS DRD binding to also cover the USBSSP
> > controller by adding "cdns,usbssp" to the compatible enum.
> >
> > The USBSSP is the next-generation Cadence USB controller IP. It adds
> > SuperSpeed Plus (USB 3.1 gen2x1, 10 Gbps) support and uses an
> > XHCI-based device controller. The register layout and resource model
> > (otg/xhci/dev memory regions; host/peripheral/otg interrupts) are
> > identical to the USBSS, so both controllers share the same binding
> > and the same platform driver (cdns3-plat.c).
> >
> > Changes to the binding:
> > - compatible: const -> enum with cdns,usb3 and cdns,usbssp
> > - maximum-speed: add super-speed-plus
> > - Add USBSSP example
> >
> > This patch was developed with assistance from Anthropic Claude Opus 4.6.
> 
> Use proper tag, but expect pushback of microslop crap.

Krzysztof, thanks for your reply.

I tried to add Assisted-by or Co-developed-by tag, neither can pass
checkpatch.pl check, it needs a valid email address. See below:

ERROR: Unrecognized email address: 'Claude (Anthropic Claude Opus 4.6)'
#45:
Assisted-by: Claude (Anthropic Claude Opus 4.6)

> 
> >
> > Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/usb/cdns,usb3.yaml    | 36 +++++++++++++++++--
> >  1 file changed, 33 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > index f454ddd9bbaa..f79333e7fc1f 100644
> > --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> > @@ -4,14 +4,22 @@
> >  $id: http://devicetree.org/schemas/usb/cdns,usb3.yaml#
> >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> >
> > -title: Cadence USBSS-DRD controller
> > +title: Cadence USBSS/USBSSP DRD controller
> >
> >  maintainers:
> >    - Pawel Laszczak <pawell@xxxxxxxxxxx>
> >
> > +description:
> > +  Cadence USB dual-role controllers. USBSS (cdns,usb3) supports up to
> > +  SuperSpeed (USB 3.0). USBSSP (cdns,usbssp) is the next generation with
> > +  SuperSpeed Plus (USB 3.1 gen2x1) and XHCI-based device controller. Both
> > +  share the same register layout and resource model.
> 
> So are compatible or not?
> 

Sorry for the misleading description. They are NOT fully compatible.
The register layout (OTG/XHCI/Device) and interrupts
(OTG/XHCI/Device/Wakeup) are the same, but register contents are
different, esp, the device (gadget) controllers are architecturally different:

- USBSS uses a custom gadget controller (cdns3_gadget_init)
- USBSSP uses an XHCI-based gadget controller (cdnsp_gadget_init)

I will fix the description in v2 to clearly state this difference.

> > +
> >  properties:
> >    compatible:
> > -    const: cdns,usb3
> > +    enum:
> > +      - cdns,usb3
> > +      - cdns,usbssp
> 
> Why do we need another generic compatible?
> 
> And why do you add it now to each of device schemas using this one?

Like explain above, the USBSSP has a different device/gadget controller
architecture from USBSS. The platform driver uses the compatible string
to select the correct gadget init function:

  if (device_get_match_data(dev) == &cdnsp_plat)
      cdns->gadget_init = cdnsp_gadget_init;
  else
      cdns->gadget_init = cdns3_gadget_init;

Without a distinct compatible, the driver cannot know which gadget
controller is present. This is a Cadence IP-level distinction (not
SoC-specific), so a generic compatible seems appropriate here. But
please let me know if you'd prefer a different approach.

> >
> >  examples:
> >    - |
> > +    // USBSS example (SuperSpeed)
> >      #include <dt-bindings/interrupt-controller/arm-gic.h>
> >      bus {
> >          #address-cells = <2>;
> > @@ -109,3 +118,24 @@ examples:
> >              dr_mode = "otg";
> >          };
> >      };
> > +  - |
> > +    // USBSSP example (SuperSpeed Plus)
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    bus {
> 
> No, drop entire example. It's the same as other.
> 

I original thought the user may copy SS binding doc to their
SSP dts file, and forget to change maximum-speed property,
so the maximum speed will be fixed at SS. If we don't need
to worry about it, I will delete at v2.

-- 

Best regards,
Peter