Re: [PATCH v4 2/4] dt-bindings: usb: Add CIX Sky1 USBSSP controller

[Hongliang Yang]

Hi Krzysztof,

Thanks for the review.

On 06/09/2026 08:55, Krzysztof Kozlowski wrote:
> No MMIO here?
>
> Then this is your device.
>
> Last time you claimed you have TWO registers. ONLY. Now you claim you
> have entire address space, but somewhere else.
>
> If you do not have here MMIO, then this glue layer is not a separate
> device node and you should have one node only, just like we did for
> DWC. One node for glue layer and CDNS USB.
>
> If you have here MMIO for some glue over IP block, then it is unlikely
> to be a syscon.

The glue layer does have its own MMIO: each controller has a small set
of RCSU registers (an AXI attribute configuration register and a
controller status register), so I dropped the syscon node, the
cix,usb-rcsu phandle and the separate cix,sky1-usbss-rcsu binding
entirely. The glue registers are now mapped through the reg property of
the glue node itself.

One complication is that the two controllers (usb4 and usb5) share a
single RCSU register region and their registers are interleaved inside
it, so neither controller owns a contiguous slice:

    0x091c0300 + 0x14 : usb4 AXI attribute configuration
    0x091c0300 + 0x24 : usb5 AXI attribute configuration
    0x091c0300 + 0x100: usb4 controller status
    0x091c0300 + 0x110: usb5 controller status

To express this in DT I made the shared RCSU region a simple-bus
container node with an address-translating ranges, and moved both glue
nodes under it. The glue nodes then describe their registers as offsets
inside the RCSU window:

    usb-rcsu@91c0300 {
        compatible = "simple-bus";
        #address-cells = <2>;
        #size-cells = <2>;
        ranges = <0x00 0x091c0300 0x00 0x091c0300 0x00 0x150>;

        usb@14 {
            compatible = "cix,sky1-usbssp";
            reg = <0x00 0x14 0x00 0x10>,
                  <0x00 0x100 0x00 0x10>;
            reg-names = "axi", "status";
            ...
        };

        usb@24 {
            ...
        };
    };

The Cadence core stays a child of each glue node with an empty ranges,
so its addresses are unchanged. The driver maps the two glue registers
by name via devm_platform_ioremap_resource_byname(), so no driver
changes are needed.

Would you be fine with such representation of the shared RCSU region?
The alternative would be to keep the glue nodes directly under the soc
node with absolute addresses in reg:

    usb@91c0314 {
        compatible = "cix,sky1-usbssp";
        reg = <0x00 0x091c0314 0x00 0x10>,
              <0x00 0x091c0400 0x00 0x10>;
        ...
    };

Both work with the current driver; I slightly prefer the ranges variant
because it documents the shared region and constrains the glue
registers to the RCSU window, but I am happy to go either way,
whatever you consider more appropriate.

> One more syscon?
>
> ...
>     cix,usb-syscon:
>
> One more syscon?

The cix,usb-syscon phandle points to the Sky1 S5 system controller,
which holds the USB mode strap controls shared with other USB
instances of the SoC (it is a genuine system controller, not a
register block created for this driver). I will clarify that in the
binding description.

Best regards,
Hongliang Yang