Re: [PATCH v6 3/3] arm64: dts: cix: Add USB4/USB5 nodes for Sky1

[Hongliang Yang]

On Thu, Nov 05, 2026 at 08:48:27AM +0100, Krzysztof Kozlowski wrote:
> So you keep ignoring review or parts of review and never responding to it.
> NAK last time, I will be NAKing this.

Fair, and it is the same mistake as on the binding: I answered the v5
cover letter and treated that as answering the review. The full
point-by-point answer is in the reply to [PATCH v6 1/3]; this one covers
only what is specific to the dts.

1) "Shared region is not a simple bus." / "Drop node. Not relevant."
   (b28d5657)

Gone in v6, together with the ranges translation. v5 had a
usb-rcsu@91c0300 node that was a bus for a block of registers; v6 has
neither the node nor the translation.

2) "So this is @14 / But this is at @91d0000 / You have completely fake
   representation of hardware." (b28d5657)

That was the consequence of the container: a node at a translated
offset 0x14 with a child at an absolute address. In v7 the unit address
of every node is the first address of its own reg, so nothing is
translated and nothing is invented.

3) "NAK, undocumented ABI. Drop entire alias and do not sneak such stuff
   in the future." (6029d402)

The usbN aliases and the id property are gone since v6. The mode strap
is now an argument of the syscon phandle, and it points at the S5 system
controller, which is where that register is:

	cix,usb-syscon = <&s5_syscon 0x424 8>;   /* USB4 */
	cix,usb-syscon = <&s5_syscon 0x424 10>;  /* USB5 */

Controller 0 of the Type-A block is used by USB4, controller 1 is used
by USB5.

4) "No improvements, since v2." (7c35c198)

Correct for the register model; v6 changed the container and the aliases
but left the two 16-byte register slots, which was not what you asked
for. v7 changes that part:

	usb4: usb@91c0314 {
		compatible = "cix,sky1-usbssp";
		reg = <0x00 0x091c0314 0x00 0x100>;
		...

		usbss_4: usb@91d0000 {
			compatible = "cdns,usb3";
			...
		};
	};

	usb5: usb@91c0324 {
		compatible = "cix,sky1-usbssp";
		reg = <0x00 0x091c0324 0x00 0x100>;
		...

		usbss_5: usb@91e0000 {
			compatible = "cdns,usb3";
			...
		};
	};

One range per controller, from its AXI cache attribute register to its
AXI clock status register, instead of two register slots. The two ranges
overlap, because the two controllers' registers are interleaved in the
shared 0x091c0000 block and 0x0328 inside the second range belongs to
controller 0. I would rather have the dts say that than pretend each
controller has a private block; the reason the driver uses
devm_ioremap() instead of devm_platform_ioremap_resource() is the same
overlap.

5) The PHY is a separate series since v4 ("do not combine USB patches
   with other subsystems", 6ca4ba08), and v7 touches only bindings/usb/
   and this dtsi. v6 also modified the S5 system control binding to add
   #syscon-cells, which is not a property that exists in the kernel; that
   hunk is dropped and the changelog no longer claims it.

6) "Odd style, please read git commit help page" (0df645f6): the commit
   messages are what git format-patch produced, nothing hand-edited.

As in the binding reply, I am implementing v7 now instead of asking you
another question and waiting.

Thanks,
Hongliang