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

[Pawel Laszczak]

>
>On 26-03-04 10:02:07, Pawel Laszczak wrote:
>> [Some people who received this message don't often get email from
>> pawell@xxxxxxxxxxx. Learn why this is important at
>> https://urldefense.com/v3/__https://aka.ms/LearnAboutSenderIdentificat
>>
>ion__;!!EHscmS1ygiU1lA!FfCAYegaJYcAGNupDLZxUTOKgLiJVzZMdMe5pK7_Wtxq
>kTM
>> PZbKQnT_1FMrOa_ar7dqoqtpdFIfp__FhXn9a$  ]
>>
>> >> >>>
>> >> >>> +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
>> >>
>> >> Layout cannot be the same if contents is different. Same layout
>> >> means same register is at the same place. If you have different
>> >> register with different contents at given place, how is it "same layout"?
>> >
>> >Sorry. I mean the USBSS and USBSSP share the same resource model
>> >(three memory regions for OTG, XHCI and Device, plus three to four
>> >interrupts). But in each region, eg in OTG region, the layout for
>> >each register are different for both controllers.
>> >
>> >Pawel, I think we could try Krzysztof's suggestion and
>> >differentiating USBSS IP and USBSSP IP at runtime, we could use DID
>> >register (cdns->version) to do that and avoid adding new IP general binding
>doc. What do you think so?
>> >
>> >> Again, driver stuff.
>> >>
>> >> >
>> >> >   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.
>> >>
>> >> Generic compatibles are almost never appropriate and driver code
>> >> rarely convinces.
>> >>
>> >> Best regards,
>> >> Krzysztof
>>
>> The Device (Peripheral) controller in USBSSP is not just an evolution
>> of USBSS; it is a completely different IP core with a distinct
>> register map.
>> While the Host (XHCI) and OTG/DRD portions overlap significantly, the
>> Device register space in USBSSP is architecturally incompatible with USBSS.
>> They are different IP cores integrated into the same subsystem:
>>
>> The register map for the Device controller in USBSSP is entirely
>> different. Offsets that are valid for one controller point to
>> different functions or reserved space in the other. Using a single
>> compatible string would imply register-level compatibility that does not exist.
>> USBSS uses a custom DMA-based peripheral controller, whereas USBSSP
>> uses an entirely different XHCI-based architecture for the device side.
>>
>> These two controllers do not provide a reliable way (e.g., via
>> registers) to distinguish between them in every case.
>> We should treat them as two separate controllers that happen to share
>> some common code, rather than viewing USBSSP simply as a newer version
>> of USBSS.
>>
>> I believe Peter's intention was to use a common DT binding and a
>> single platform driver, as the resource model and parts of the code
>> are nearly identical.
>> However, given the architectural differences on the device side, I
>> believe having distinct compatible strings is the most accurate way to
>> describe the hardware. It prevents the driver from making "blind"
>> accesses to registers that have different meanings depending on the underlying
>IP.
>>
>> Maybe to resolve this, we should have separate DT binding
>> documentation and potentially separate platform drivers to reflect
>> this architectural split?
>>
>
>Pawel, currently, the OTG and host share the same code between USBSS and
>USBSSP, the execution flow is decided by runtime already.
>And for device mode, there are two different drivers for them, we could decide
>which driver's initialization during the runtime, after the initialization, the device
>driver (UDC)'s behaviour has already decided per IP (USBSS vs USBSSP). Are there
>any examples the controller would visit wrong register, eg, USBSS IP visits
>USBSSP registers?
>
>Peter
>

No such case comes to mind.

As long as the code logic is correct and the runtime detection
(via DID/PID, or PCI IDs, or DT match data) functions as intended,
there is no risk of the controller accessing the wrong registers.

Pawel

>
>--
>
>Best regards,
>Peter