Linux USB
 help / color / mirror / Atom feed
* [PATCH v5 0/3] usb: cdns3: Add CIX Sky1 USBSSP controller support
@ 2026-11-03  7:49 Hongliang Yang (杨宏亮)
  2026-11-03  7:49 ` [PATCH v5 1/3] dt-bindings: usb: Add CIX Sky1 USBSSP controller Hongliang Yang (杨宏亮)
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Hongliang Yang (杨宏亮) @ 2026-11-03  7:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Peter Chen, Pawel Laszczak, Roger Quadros,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
	Gary Yang (杨新伟),
	Fugang Duan (段富刚)
  Cc: linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	cix-kernel-upstream, Hongliang Yang (杨宏亮)

This series adds support for the two Cadence USBSSP DRD controllers
integrated in the CIX Sky1 SoC (USB4 and USB5, used in host mode on the
Radxa Orion O6 board).

Patch 1 adds the device tree binding for the CIX glue node. Each
controller owns an AXI bus attribute configuration register and a
controller status register in the SoC RCSU register region. The glue
registers of the two controllers are interleaved inside that region, so
the shared region is described by a simple-bus container with an
address-translating ranges property, and each glue node maps its own
registers as offsets inside it. The interleaving is not a problem:
ranges only translates addresses, it does not require the children to
be contiguous.

Patch 2 adds the glue driver. It enables the controller clocks, runs
the reset sequence, programs the mode strap registers through the Sky1
S5 system controller (cix,usb-syscon) and registers the Cadence core as
a child device.

Patch 3 adds the SoC nodes and enables USB4 and USB5 in host mode on
the Radxa Orion O6 board.

Tested on Radxa Orion O6 (Sky1): both controllers probe, xHCI is
registered and USB 3.x devices enumerate.

Patch 3 references the PHY phandles provided by the CIX Sky1 USB3 PHY
series, which is posted separately; apply that series first.

The series applies on top of v7.4-rc1.

Changes since v4:
- Added the MAINTAINERS entry for the glue driver.
- Fixed continuation line indentation reported by checkpatch.

Hongliang Yang (3):
  dt-bindings: usb: Add CIX Sky1 USBSSP controller
  usb: cdns3: Add CIX Sky1 glue driver
  arm64: dts: cix: Add USB4/USB5 nodes for Sky1

 .../bindings/usb/cix,sky1-usbssp.yaml         | 152 ++++
 MAINTAINERS                                   |   2 +
 arch/arm64/boot/dts/cix/sky1-orion-o6.dts     |  42 +
 arch/arm64/boot/dts/cix/sky1.dtsi             |  95 +++
 drivers/usb/cdns3/Kconfig                     |  13 +
 drivers/usb/cdns3/Makefile                    |   1 +
 drivers/usb/cdns3/cdnsp-sky1.c                | 716 ++++++++++++++++++
 drivers/usb/cdns3/cdnsp-sky1.h                | 128 ++++
 8 files changed, 1149 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml
 create mode 100644 drivers/usb/cdns3/cdnsp-sky1.c
 create mode 100644 drivers/usb/cdns3/cdnsp-sky1.h


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
prerequisite-patch-id: 422da6ec0656173cd308baf6725a7dabfedb0923
prerequisite-patch-id: 4f45048be2c24228268e32481a63724884b0ce02
prerequisite-patch-id: d1fa8aec9de75e8a2b77cf01b65373825749baa2
-- 
2.54.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v5 1/3] dt-bindings: usb: Add CIX Sky1 USBSSP controller
  2026-11-03  7:49 [PATCH v5 0/3] usb: cdns3: Add CIX Sky1 USBSSP controller support Hongliang Yang (杨宏亮)
@ 2026-11-03  7:49 ` Hongliang Yang (杨宏亮)
  2026-11-03  8:01   ` sashiko-bot
  2026-11-03  7:49 ` [PATCH v5 3/3] arm64: dts: cix: Add USB4/USB5 nodes for Sky1 Hongliang Yang (杨宏亮)
  2026-11-03  7:49 ` [PATCH v5 2/3] usb: cdns3: Add CIX Sky1 glue driver Hongliang Yang (杨宏亮)
  2 siblings, 1 reply; 8+ messages in thread
From: Hongliang Yang (杨宏亮) @ 2026-11-03  7:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Peter Chen, Pawel Laszczak, Roger Quadros,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
	Gary Yang (杨新伟),
	Fugang Duan (段富刚)
  Cc: linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	cix-kernel-upstream, Hongliang Yang (杨宏亮)

Add device tree binding for the CIX Sky1 Cadence USBSSP controller
glue layer. Each controller has an AXI bus attribute configuration
register and a controller status register. The glue registers of the
two controllers are interleaved inside a shared RCSU region, which is
represented as a simple-bus container node with an address-translating
ranges property; each glue node describes its registers as offsets
inside that region. Mode strap controls are programmed through the
Sky1 S5 system controller referenced by the cix,usb-syscon phandle.
The CIX glue node wraps a Cadence USBSSP core as a child node.

Signed-off-by: Hongliang Yang <hongliang.yang@cixtech.com>
---
v4 -> v5:
- Added the MAINTAINERS entry for the glue driver.
 .../bindings/usb/cix,sky1-usbssp.yaml         | 152 ++++++++++++++++++
 MAINTAINERS                                   |   2 +
 2 files changed, 154 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml

diff --git a/Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml b/Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml
new file mode 100644
index 000000000000..c29654567e76
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml
@@ -0,0 +1,152 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/cix,sky1-usbssp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CIX Sky1 Cadence USBSSP Controller
+
+maintainers:
+  - Hongliang Yang <hongliang.yang@cixtech.com>
+
+description:
+  The CIX Sky1 USBSSP controller is based on the Cadence USBSSP DRD
+  controller. The integration adds per-controller glue registers in the
+  SoC system control aperture, an AXI bus attribute configuration
+  register and a controller status register, both mapped by the reg
+  property of the glue node. The glue registers of the two controllers
+  are interleaved inside a shared RCSU region, which is represented as
+  a simple-bus container node with an address-translating ranges
+  property; each glue node describes its registers as offsets inside
+  that region. Mode strap controls are programmed through the Sky1 S5
+  system controller referenced by the cix,usb-syscon phandle. The CIX
+  glue node wraps a Cadence USBSSP core as a child node.
+
+properties:
+  compatible:
+    const: cix,sky1-usbssp
+
+  reg:
+    items:
+      - description: AXI bus attribute configuration registers
+      - description: Controller status registers
+
+  reg-names:
+    items:
+      - const: axi
+      - const: status
+
+  clocks:
+    items:
+      - description: Start-of-frame clock
+      - description: AXI bus clock
+      - description: Low-power mode clock
+      - description: APB register interface clock
+
+  clock-names:
+    items:
+      - const: sof
+      - const: axi
+      - const: lpm
+      - const: apb
+
+  resets:
+    items:
+      - description: APB register reset
+      - description: Controller reset
+
+  reset-names:
+    items:
+      - const: apb
+      - const: core
+
+  cix,usb-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to the Sky1 S5 system controller used to program USB mode
+      strap controls.
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 2
+
+  ranges: true
+
+patternProperties:
+  "^usb@[0-9a-f]+$":
+    $ref: cdns,usb3.yaml#
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - cix,usb-syscon
+  - "#address-cells"
+  - "#size-cells"
+  - ranges
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/cix,sky1.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/reset/cix,sky1-s5-system-control.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        usb-rcsu@91c0300 {
+            compatible = "simple-bus";
+            #address-cells = <2>;
+            #size-cells = <2>;
+            ranges = <0x00 0x00000000 0x00 0x091c0300 0x00 0x150>,
+                     <0x00 0x091d0000 0x00 0x091d0000 0x00 0x20000>;
+
+            usb@14 {
+                compatible = "cix,sky1-usbssp";
+                reg = <0x00 0x14 0x00 0x10>,
+                      <0x00 0x100 0x00 0x10>;
+                reg-names = "axi", "status";
+                ranges;
+                #address-cells = <2>;
+                #size-cells = <2>;
+
+                clocks = <&scmi_clk CLK_TREE_USB3A_H0_CLK_SOF>,
+                         <&scmi_clk CLK_TREE_USB3A_0_AXI_GATE>,
+                         <&scmi_clk CLK_TREE_USB3A_H0_CLK_LPM>,
+                         <&scmi_clk CLK_TREE_USB3A_0_APB_GATE>;
+                clock-names = "sof", "axi", "lpm", "apb";
+                resets = <&src SKY1_USBC_SS2_PRST_N>,
+                         <&src SKY1_USBC_SS2_RST_N>;
+                reset-names = "apb", "core";
+                cix,usb-syscon = <&src>;
+
+                usb@91d0000 {
+                    compatible = "cdns,usb3";
+                    reg = <0x00 0x91d0000 0x00 0x4000>,
+                          <0x00 0x91d8000 0x00 0x8000>,
+                          <0x00 0x91d4000 0x00 0x4000>;
+                    reg-names = "otg", "xhci", "dev";
+                    dr_mode = "otg";
+
+                    interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 171 IRQ_TYPE_LEVEL_HIGH>;
+                    interrupt-names = "host", "peripheral", "otg", "wakeup";
+                    maximum-speed = "super-speed-plus";
+
+                    phys = <&usb3_phy4_0>, <&usb2_phy6>;
+                    phy-names = "cdns3,usb3-phy", "cdns3,usb2-phy";
+                };
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index b857e529e5e2..92ff4ccff791 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2904,9 +2904,11 @@ T:	git https://github.com/cixtech/linux-mainline.git
 F:	Documentation/devicetree/bindings/arm/cix.yaml
 F:	Documentation/devicetree/bindings/mailbox/cix,sky1-mbox.yaml
 F:	Documentation/devicetree/bindings/phy/cix,sky1-usb3-phy.yaml
+F:	Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml
 F:	arch/arm64/boot/dts/cix/
 F:	drivers/mailbox/cix-mailbox.c
 F:	drivers/phy/cix/
+F:	drivers/usb/cdns3/cdnsp-sky1.*
 K:	\bcix\b
 
 ARM/CLKDEV SUPPORT
-- 
2.54.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v5 3/3] arm64: dts: cix: Add USB4/USB5 nodes for Sky1
  2026-11-03  7:49 [PATCH v5 0/3] usb: cdns3: Add CIX Sky1 USBSSP controller support Hongliang Yang (杨宏亮)
  2026-11-03  7:49 ` [PATCH v5 1/3] dt-bindings: usb: Add CIX Sky1 USBSSP controller Hongliang Yang (杨宏亮)
@ 2026-11-03  7:49 ` Hongliang Yang (杨宏亮)
  2026-11-03  8:06   ` sashiko-bot
  2026-11-03  7:49 ` [PATCH v5 2/3] usb: cdns3: Add CIX Sky1 glue driver Hongliang Yang (杨宏亮)
  2 siblings, 1 reply; 8+ messages in thread
From: Hongliang Yang (杨宏亮) @ 2026-11-03  7:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Peter Chen, Pawel Laszczak, Roger Quadros,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
	Gary Yang (杨新伟),
	Fugang Duan (段富刚)
  Cc: linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	cix-kernel-upstream, Hongliang Yang (杨宏亮)

Add USB4 and USB5 controller nodes for the CIX Sky1 SoC. Enable USB4
and USB5 in host mode on the Radxa Orion O6 board.

The two controllers share the RCSU register region and their glue
registers are interleaved inside it, so the region is described by a
simple-bus container with an address-translating ranges property and
each controller maps its registers as offsets inside that region.

Signed-off-by: Hongliang Yang <hongliang.yang@cixtech.com>
---
v4 -> v5:
- Fixed continuation line indentation reported by checkpatch.
 arch/arm64/boot/dts/cix/sky1-orion-o6.dts | 42 ++++++++++
 arch/arm64/boot/dts/cix/sky1.dtsi         | 95 +++++++++++++++++++++++
 2 files changed, 137 insertions(+)

diff --git a/arch/arm64/boot/dts/cix/sky1-orion-o6.dts b/arch/arm64/boot/dts/cix/sky1-orion-o6.dts
index 5d57dd9ea299..d07fad302050 100644
--- a/arch/arm64/boot/dts/cix/sky1-orion-o6.dts
+++ b/arch/arm64/boot/dts/cix/sky1-orion-o6.dts
@@ -80,6 +80,22 @@ pins {
 
 		};
 	};
+
+	pinctrl_usb4: usb4-power-on-cfg {
+		pins {
+			bias-pull-down;
+			drive-strength = <8>;
+			pinmux = <CIX_PAD_GPIO041_FUNC_USB_DRIVE_VBUS4>;
+		};
+	};
+
+	pinctrl_usb5: usb5-power-on-cfg {
+		pins {
+			bias-pull-down;
+			drive-strength = <8>;
+			pinmux = <CIX_PAD_GPIO042_FUNC_USB_DRIVE_VBUS5>;
+		};
+	};
 };
 
 &pcie_x8_rc {
@@ -118,6 +134,32 @@ &uart2 {
 	status = "okay";
 };
 
+&usb4 {
+	pinctrl-0 = <&pinctrl_usb4>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&usbss_4 {
+	dr_mode = "host";
+
+	status = "okay";
+};
+
+&usb5 {
+	pinctrl-0 = <&pinctrl_usb5>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&usbss_5 {
+	dr_mode = "host";
+
+	status = "okay";
+};
+
 &usb3_phy4 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
index 06a311ee14ef..c10db0ff9af6 100644
--- a/arch/arm64/boot/dts/cix/sky1.dtsi
+++ b/arch/arm64/boot/dts/cix/sky1.dtsi
@@ -17,6 +17,11 @@ / {
 	#address-cells = <2>;
 	#size-cells = <2>;
 
+	aliases {
+		usb4 = &usb4;
+		usb5 = &usb5;
+	};
+
 	cpus {
 		#address-cells = <2>;
 		#size-cells = <0>;
@@ -606,6 +611,96 @@ mbox_ap2sfh: mailbox@80a0000 {
 			cix,mbox-dir = "tx";
 		};
 
+		usbss_rcsu: usb-rcsu@91c0300 {
+			compatible = "simple-bus";
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges = <0x00 0x00000000 0x00 0x091c0300 0x00 0x150>,
+				<0x00 0x091d0000 0x00 0x091d0000 0x00 0x20000>;
+
+			usb4: usb@14 {
+				compatible = "cix,sky1-usbssp";
+				reg = <0x00 0x14 0x00 0x10>,
+				      <0x00 0x100 0x00 0x10>;
+				reg-names = "axi", "status";
+				ranges;
+				#address-cells = <2>;
+				#size-cells = <2>;
+				clocks = <&scmi_clk CLK_TREE_USB3A_H0_CLK_SOF>,
+					<&scmi_clk CLK_TREE_USB3A_0_AXI_GATE>,
+					<&scmi_clk CLK_TREE_USB3A_H0_CLK_LPM>,
+					<&scmi_clk CLK_TREE_USB3A_0_APB_GATE>;
+				clock-names = "sof", "axi", "lpm", "apb";
+				resets = <&s5_syscon SKY1_USBC_SS2_PRST_N>,
+					<&s5_syscon SKY1_USBC_SS2_RST_N>;
+				reset-names = "apb", "core";
+				cix,usb-syscon = <&s5_syscon>;
+
+				status = "disabled";
+
+				usbss_4: usb@91d0000 {
+					compatible = "cdns,usb3";
+					reg = <0x00 0x91d0000 0x00 0x4000>,
+					      <0x00 0x91d8000 0x00 0x8000>,
+					      <0x00 0x91d4000 0x00 0x4000>;
+					reg-names = "otg", "xhci", "dev";
+					dr_mode = "otg";
+					interrupts = <GIC_SPI 252 IRQ_TYPE_LEVEL_HIGH 0>,
+						<GIC_SPI 252 IRQ_TYPE_LEVEL_HIGH 0>,
+						<GIC_SPI 253 IRQ_TYPE_LEVEL_HIGH 0>,
+						<GIC_SPI 252 IRQ_TYPE_LEVEL_HIGH 0>;
+					interrupt-names = "host", "peripheral",
+						"otg", "wakeup";
+					maximum-speed = "super-speed-plus";
+					phys = <&usb3_phy4_0>;
+					phy-names = "cdns3,usb3-phy";
+
+					status = "disabled";
+				};
+			};
+
+			usb5: usb@24 {
+				compatible = "cix,sky1-usbssp";
+				reg = <0x00 0x24 0x00 0x10>,
+				      <0x00 0x110 0x00 0x10>;
+				reg-names = "axi", "status";
+				ranges;
+				#address-cells = <2>;
+				#size-cells = <2>;
+				clocks = <&scmi_clk CLK_TREE_USB3A_H1_CLK_SOF>,
+					<&scmi_clk CLK_TREE_USB3A_1_AXI_GATE>,
+					<&scmi_clk CLK_TREE_USB3A_H1_CLK_LPM>,
+					<&scmi_clk CLK_TREE_USB3A_1_APB_GATE>;
+				clock-names = "sof", "axi", "lpm", "apb";
+				resets = <&s5_syscon SKY1_USBC_SS3_PRST_N>,
+					<&s5_syscon SKY1_USBC_SS3_RST_N>;
+				reset-names = "apb", "core";
+				cix,usb-syscon = <&s5_syscon>;
+
+				status = "disabled";
+
+				usbss_5: usb@91e0000 {
+					compatible = "cdns,usb3";
+					reg = <0x00 0x91e0000 0x00 0x4000>,
+					      <0x00 0x91e8000 0x00 0x8000>,
+					      <0x00 0x91e4000 0x00 0x4000>;
+					reg-names = "otg", "xhci", "dev";
+					dr_mode = "otg";
+					interrupts = <GIC_SPI 257 IRQ_TYPE_LEVEL_HIGH 0>,
+						<GIC_SPI 257 IRQ_TYPE_LEVEL_HIGH 0>,
+						<GIC_SPI 258 IRQ_TYPE_LEVEL_HIGH 0>,
+						<GIC_SPI 257 IRQ_TYPE_LEVEL_HIGH 0>;
+					interrupt-names = "host", "peripheral",
+						"otg", "wakeup";
+					maximum-speed = "super-speed-plus";
+					phys = <&usb3_phy4_1>;
+					phy-names = "cdns3,usb3-phy";
+
+					status = "disabled";
+				};
+			};
+		};
+
 		usb3_phy4: usb-phy@9210000 {
 			compatible = "cix,sky1-usb3-phy";
 			reg = <0x00 0x09210000 0x00 0x40000>;
-- 
2.54.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v5 2/3] usb: cdns3: Add CIX Sky1 glue driver
  2026-11-03  7:49 [PATCH v5 0/3] usb: cdns3: Add CIX Sky1 USBSSP controller support Hongliang Yang (杨宏亮)
  2026-11-03  7:49 ` [PATCH v5 1/3] dt-bindings: usb: Add CIX Sky1 USBSSP controller Hongliang Yang (杨宏亮)
  2026-11-03  7:49 ` [PATCH v5 3/3] arm64: dts: cix: Add USB4/USB5 nodes for Sky1 Hongliang Yang (杨宏亮)
@ 2026-11-03  7:49 ` Hongliang Yang (杨宏亮)
  2026-11-03  8:15   ` sashiko-bot
  2026-11-03  8:43   ` Philipp Zabel
  2 siblings, 2 replies; 8+ messages in thread
From: Hongliang Yang (杨宏亮) @ 2026-11-03  7:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Peter Chen, Pawel Laszczak, Roger Quadros,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
	Gary Yang (杨新伟),
	Fugang Duan (段富刚)
  Cc: linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	cix-kernel-upstream, Hongliang Yang (杨宏亮)

Add a glue layer driver for the Cadence USBSSP controller integrated
in the CIX Sky1 SoC. The driver manages the controller clocks,
resets, and the mode strap controls located in the Sky1 S5 system
controller. The per-controller glue registers, the AXI bus attribute
configuration register and the controller status register, are mapped
directly through the reg property of the glue node.

Signed-off-by: Hongliang Yang <hongliang.yang@cixtech.com>
---
v4 -> v5:
- No changes.
 drivers/usb/cdns3/Kconfig      |  13 +
 drivers/usb/cdns3/Makefile     |   1 +
 drivers/usb/cdns3/cdnsp-sky1.c | 716 +++++++++++++++++++++++++++++++++
 drivers/usb/cdns3/cdnsp-sky1.h | 128 ++++++
 4 files changed, 858 insertions(+)
 create mode 100644 drivers/usb/cdns3/cdnsp-sky1.c
 create mode 100644 drivers/usb/cdns3/cdnsp-sky1.h

diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
index 39ad23d1ada8..03d8e170a98a 100644
--- a/drivers/usb/cdns3/Kconfig
+++ b/drivers/usb/cdns3/Kconfig
@@ -77,6 +77,19 @@ config USB_CDNSP_PCI
 	  If you choose to build this driver as module it will
 	  be dynamically linked and module will be called cdnsp-pci.ko
 
+config USB_CDNSP_SKY1
+	tristate "Cadence USB3 support on CIX Sky1 SoC platforms"
+	depends on USB_CDNS3
+	depends on ARCH_CIX || COMPILE_TEST
+	default USB_CDNS3
+	help
+	  Glue driver for the Cadence USB dual-role controllers on CIX Sky1
+	  (device tree compatible cix,sky1-usb3). It enables clocks and resets
+	  from the SoC, then uses the shared cdns3 platform core (cdns.ko).
+
+	  If built as a module, the module is named cdnsp-sky1.ko and must be
+	  loaded after the cdns core module when both are loadable modules.
+
 config USB_CDNS3_TI
 	tristate "Cadence USB3 support on TI platforms"
 	depends on ARCH_K3 || COMPILE_TEST
diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
index b2e4ba6a49a3..ab813aaf9940 100644
--- a/drivers/usb/cdns3/Makefile
+++ b/drivers/usb/cdns3/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_USB_CDNSP_PCI)			+= cdnsp-pci.o
 obj-$(CONFIG_USB_CDNS3_TI)			+= cdns3-ti.o
 obj-$(CONFIG_USB_CDNS3_IMX)			+= cdns3-imx.o
 obj-$(CONFIG_USB_CDNS3_STARFIVE)		+= cdns3-starfive.o
+obj-$(CONFIG_USB_CDNSP_SKY1)			+= cdnsp-sky1.o
diff --git a/drivers/usb/cdns3/cdnsp-sky1.c b/drivers/usb/cdns3/cdnsp-sky1.c
new file mode 100644
index 000000000000..1f7870289a35
--- /dev/null
+++ b/drivers/usb/cdns3/cdnsp-sky1.c
@@ -0,0 +1,716 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * cdnsp-sky1.c - CIX Sky1 glue for Cadence USBSSP DRD controller
+ *
+ * Copyright (C) 2026 CIX Technology Group Co., Ltd.
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+
+#include "cdnsp-sky1.h"
+#include "core.h"
+
+static const char *cix_usb_clk_names[CIX_USB_CLK_NUM] = {
+	"sof",
+	"axi",
+	"lpm",
+	"apb",
+};
+
+struct cdnsp_sky1_strap_signal {
+	unsigned int offset, bit;
+};
+
+static const struct cdnsp_sky1_strap_signal strap_signals[SKY1_USB_S5_NUM] = {
+	/* usb config in s5 domain */
+	[U3_TYPEC_DRD_ID]	= { USB_MODE_STRAP_S5_DOMAIN, U3_TYPEC_DRD_MODE_STRAP_BIT },
+	[U3_TYPEC_HOST0_ID]	= { USB_MODE_STRAP_S5_DOMAIN, U3_TYPEC_HOST0_MODE_STRAP_BIT },
+	[U3_TYPEC_HOST1_ID]	= { USB_MODE_STRAP_S5_DOMAIN, U3_TYPEC_HOST1_MODE_STRAP_BIT },
+	[U3_TYPEC_HOST2_ID]	= { USB_MODE_STRAP_S5_DOMAIN, U3_TYPEC_HOST2_MODE_STRAP_BIT },
+	[U3_TYPEA_CTRL0_ID]	= { USB_MODE_STRAP_S5_DOMAIN, U3_TYPEA_CTRL0_MODE_STRAP_BIT },
+	[U3_TYPEA_CTRL1_ID]	= { USB_MODE_STRAP_S5_DOMAIN, U3_TYPEA_CTRL1_MODE_STRAP_BIT},
+	[U2_HOST0_ID]		= { USB_MODE_STRAP_S5_DOMAIN, U2_HOST0_MODE_STRAP_BIT },
+	[U2_HOST1_ID]		= { USB_MODE_STRAP_S5_DOMAIN, U2_HOST1_MODE_STRAP_BIT },
+	[U2_HOST2_ID]		= { USB_MODE_STRAP_S5_DOMAIN, U2_HOST2_MODE_STRAP_BIT },
+	[U2_HOST3_ID]		= { USB_MODE_STRAP_S5_DOMAIN, U2_HOST3_MODE_STRAP_BIT },
+};
+
+static int cdnsp_sky1_set_mode_by_id(struct device *dev, int mode)
+{
+	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
+
+	return regmap_update_bits(data->usb_syscon,
+				  strap_signals[data->id].offset,
+				  GENMASK(strap_signals[data->id].bit + 1,
+					  strap_signals[data->id].bit),
+				  mode << strap_signals[data->id].bit);
+}
+
+/**
+ * cdnsp_sky1_clk_enable_all() - enable all clocks for usb controller
+ * @dev:	Pointer to the device of platform_device
+ *
+ */
+
+static int cdnsp_sky1_clk_enable_all(struct device *dev)
+{
+	int i, ret = 0;
+	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
+	struct clk **cix_usb_clks = data->cix_usb_clks;
+
+	for (i = 0; i < CIX_USB_CLK_NUM; i++) {
+		cix_usb_clks[i] = devm_clk_get(dev, cix_usb_clk_names[i]);
+		if (IS_ERR(cix_usb_clks[i])) {
+			ret = dev_err_probe(dev, PTR_ERR(cix_usb_clks[i]),
+					    "could not get %s clock\n",
+					    cix_usb_clk_names[i]);
+			goto err_usb_clks;
+		}
+		ret = clk_prepare_enable(cix_usb_clks[i]);
+		if (ret) {
+			dev_err(dev, "%s enable failed:%d\n", cix_usb_clk_names[i], ret);
+			goto err_usb_clks;
+		}
+	}
+	dev_dbg(dev, "enable sky1 USB clock done\n");
+	return ret;
+
+err_usb_clks:
+	while (--i >= 0)
+		clk_disable_unprepare(cix_usb_clks[i]);
+	return ret;
+};
+
+/**
+ * cdnsp_sky1_clk_disable_all() - disable all clocks for usb controller
+ * @dev:	Pointer to the device of platform_device
+ *
+ */
+
+static void cdnsp_sky1_clk_disable_all(struct device *dev)
+{
+	int i;
+	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
+	struct clk **cix_usb_clks = data->cix_usb_clks;
+
+	for (i = 0; i < CIX_USB_CLK_NUM; i++)
+		clk_disable_unprepare(cix_usb_clks[i]);
+};
+
+/**
+ * cdnsp_sky1_clk_enable_resume() - enable the clocks that are turned
+ * off while suspend
+ * @dev:	Pointer to the device of platform_device
+ *
+ */
+
+static int cdnsp_sky1_clk_enable_resume(struct device *dev)
+{
+	int i, ret;
+	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
+	struct clk **cix_usb_clks = data->cix_usb_clks;
+
+	for (i = 0; i < CIX_USB_CLK_OFF_NUM; i++) {
+		ret = clk_prepare_enable(cix_usb_clks[i]);
+		if (ret) {
+			dev_err(dev, "failed to enable clock %s: %d\n",
+				cix_usb_clk_names[i], ret);
+			goto err_usb_clks;
+		}
+	}
+	return 0;
+
+err_usb_clks:
+	while (--i >= 0)
+		clk_disable_unprepare(cix_usb_clks[i]);
+	return ret;
+};
+
+/**
+ * cdnsp_sky1_clk_disable_suspend() - disable the clocks which are not
+ * needed when suspend
+ * @dev:	Pointer to the device of platform_device
+ *
+ */
+
+static void cdnsp_sky1_clk_disable_suspend(struct device *dev)
+{
+	int i;
+	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
+	struct clk **cix_usb_clks = data->cix_usb_clks;
+
+	for (i = 0; i < CIX_USB_CLK_OFF_NUM; i++)
+		clk_disable_unprepare(cix_usb_clks[i]);
+};
+
+static int sky1_rcsu_handshake(struct cdnsp_sky1 *data, u32 mask, u32 done,
+			       u64 timeout_us)
+{
+	u32	result;
+	int	ret;
+
+	ret = readl_poll_timeout(data->rcsu_ctst_base, result,
+				 (result & mask) == done ||
+				 result == U32_MAX,
+				 10, timeout_us);
+	if (result == U32_MAX)		/* card removed */
+		return -ENODEV;
+	return ret;
+}
+
+static int cdns_sky1_platform_suspend(struct device *dev,
+				      bool suspend, bool wakeup)
+{
+	struct cdns *cdns = dev_get_drvdata(dev);
+	struct platform_device *xhci_dev = cdns->host_dev;
+	struct usb_hcd  *hcd;
+	struct device *parent = cdns->dev->parent;
+	struct cdnsp_sky1 *data = dev_get_drvdata(parent);
+	u32 value;
+	int poll;
+	int ret = 0;
+	int count = 3;
+
+	data->wakeup = wakeup;
+
+	if (cdns->role != USB_ROLE_HOST)
+		return 0;
+
+	if (!xhci_dev) {
+		dev_dbg(dev, "host controller has not been registered\n");
+		return 0;
+	}
+
+	hcd = dev_get_drvdata(&xhci_dev->dev);
+	if (!hcd) {
+		dev_dbg(dev, "host controller have not registered\n");
+		return 0;
+	}
+
+	if (suspend) {
+		while (count--) {
+			value = readl(hcd->regs + XECP_PM_PMCSR);
+			value &= ~PS_MASK;
+			value |= PS_D3 | PS_PME_En;
+			writel(value, hcd->regs + XECP_PM_PMCSR);
+			/* After controller enters D3, disable AXI and SOF
+			 * until AXI valid flag changes to 0.
+			 */
+			if (sky1_rcsu_handshake(data, AXI_CLOCK_VALID, 0,
+						100ULL * 1000)) {
+				u32 ctst;
+
+				ctst = readl(data->rcsu_ctst_base);
+				dev_dbg(dev, "enter D3 failed, register value:%x\n",
+					ctst);
+			} else {
+				break;
+			}
+		}
+		if (count < 0) {
+			u32 ctst;
+
+			ctst = readl(data->rcsu_ctst_base);
+			dev_err(dev, "enter D3 failed after retries, register value:%x\n",
+				ctst);
+		}
+	} else {
+		while (count--) {
+			value = readl(hcd->regs + XECP_PM_PMCSR);
+			value &= ~PS_MASK;
+			value |= PS_D0;
+			value &= ~PS_PME_En;
+			writel(value, hcd->regs + XECP_PM_PMCSR);
+			/* Wait power state back to D0 */
+			poll = readl_poll_timeout(hcd->regs + XECP_PM_PMCSR,
+						  value,
+						  (value & PS_MASK) == 0 ||
+						  value == U32_MAX,
+						  10, 100ULL * 1000);
+			if (poll) {
+				dev_dbg(dev, "exit D3 timeout, power state=0x%lx\n",
+					value & PS_MASK);
+			} else {
+				break;
+			}
+		}
+		if (count < 0) {
+			dev_err(dev, "exit D3 timeout after retries, power state=0x%lx\n",
+				value & PS_MASK);
+		}
+	}
+
+	return ret;
+}
+
+static void cdnsp_sky1_configure_controller(struct cdnsp_sky1 *data)
+{
+	int clk;
+	int v0, v1, v2;
+	u32 val = 0;
+
+	if (data->u3_disable) {
+		dev_dbg(data->dev, "disable u3 port\n");
+		writel(D_XEC_CFG_3XPORT_MODE_VALUE, data->device_base
+			+ D_XEC_CFG_3XPORT_MODE);
+	}
+	writel(AXI_HALT, data->device_base + D_XEC_AXI_CAP);
+	writel(AXI_HALT, data->xhci_base + D_XEC_AXI_CAP);
+	writel(data->axi_bmax_value, data->device_base + D_XEC_AXI_CTRL0);
+	writel(data->axi_bmax_value, data->xhci_base + D_XEC_AXI_CTRL0);
+	writel((~(u32)(AXI_HALT)), data->device_base + D_XEC_AXI_CAP);
+	writel((~(u32)(AXI_HALT)), data->xhci_base + D_XEC_AXI_CAP);
+	clk = data->sof_clk_freq;
+	v0 =  25 * clk / 100000000;
+	v1 = clk / 10000;
+	v2 = clk / 10;
+	writel(((v0 > 1) ? v0 - 1 : 1), data->device_base
+		+ D_XEC_PRE_REG_250NS);
+	writel((unsigned int)((v1 / 100 > 1) > 0 ? (v1 / 100) - 1 : 1),
+	       data->device_base + D_XEC_PRE_REG_1US);
+	writel((unsigned int)((v1 / 10 > 1) > 0 ? (v1 / 10) - 1 : 1),
+	       data->device_base + D_XEC_PRE_REG_10US);
+	writel(((v1) > 1 ? v1 - 1 : 1), data->device_base
+		+ D_XEC_PRE_REG_100US);
+	writel((unsigned int)((125 * clk / 1000000) > 1 ? (125 * clk / 1000000) : 1),
+	       data->device_base + D_XEC_PRE_REG_125US);
+	writel(((v2 / 100 > 1) ? (v2 / 100) - 1 : 1), data->device_base
+		+ D_XEC_PRE_REG_1MS);
+	writel(((v2 / 10 > 1) ? (v2 / 10) - 1 : 1), data->device_base
+		+ D_XEC_PRE_REG_10MS);
+	writel((v2 > 1 ? v2 - 1 : 1), data->device_base
+		+ D_XEC_PRE_REG_100MS);
+	dev_dbg(data->dev, "readl:%x, %x ,%x, %x, %x, %x, %x, %x\n",
+		readl(data->device_base + D_XEC_PRE_REG_250NS),
+		readl(data->device_base + D_XEC_PRE_REG_1US),
+		readl(data->device_base + D_XEC_PRE_REG_10US),
+		readl(data->device_base + D_XEC_PRE_REG_100US),
+		readl(data->device_base + D_XEC_PRE_REG_125US),
+		readl(data->device_base + D_XEC_PRE_REG_1MS),
+		readl(data->device_base + D_XEC_PRE_REG_10MS),
+		readl(data->device_base + D_XEC_PRE_REG_100MS));
+	clk = data->lpm_clk_freq;
+	v0 =  25 * clk / 100000000;
+	v1 = clk / 10000;
+	v2 = clk / 10;
+	writel(((v0 > 1) ? v0 - 1 : 1), data->device_base
+		+ D_XEC_LPM_PRE_REG_250NS);
+	writel((unsigned int)((v1 / 100 > 1) > 0 ? (v1 / 100) - 1 : 1), data->device_base
+		+ D_XEC_LPM_PRE_REG_1US);
+	writel((unsigned int)((v1 / 10 > 1) > 0 ? (v1 / 10) - 1 : 1), data->device_base
+		+ D_XEC_LPM_PRE_REG_10US);
+	writel(((v1) > 1 ? v1 - 1 : 1), data->device_base
+		+ D_XEC_LPM_PRE_REG_100US);
+	writel((unsigned int)((125 * clk / 1000000) > 1 ? (125 * clk / 1000000) : 1),
+	       data->device_base + D_XEC_LPM_PRE_REG_125US);
+	writel(((v2 / 100 > 1) ? (v2 / 100) - 1 : 1), data->device_base
+		+ D_XEC_LPM_PRE_REG_1MS);
+	writel(((v2 / 10 > 1) ? (v2 / 10) - 1 : 1), data->device_base
+		+ D_XEC_LPM_PRE_REG_10MS);
+	writel((v2 > 1 ? v2 - 1 : 1), data->device_base
+		+ D_XEC_LPM_PRE_REG_100MS);
+	v0 = readl(data->xhci_base + XEC_USBSSP_CHICKEN_BITS_3);
+	v0 &= ~(CFG_APB_TIMEOUT_PSLVERR_EN | CFG_APB_PSLVERR_EN);
+	writel(v0, data->xhci_base + XEC_USBSSP_CHICKEN_BITS_3);
+	if (data->u3_disable) {
+		dev_dbg(data->dev, "disable u3 port\n");
+		writel(XEC_CFG_3XPORT_MODE_VALUE, data->xhci_base
+			+ XEC_CFG_3XPORT_MODE);
+	} else if (data->ssp_disable) {
+		dev_dbg(data->dev, "disable ssp\n");
+		v0 = readl(data->xhci_base + XEC_CFG_3XPORT_MODE);
+		writel(v0  & CFG_3XPORT_MODE_DIS_SSP, data->xhci_base
+			+ XEC_CFG_3XPORT_MODE);
+	}
+	clk = data->sof_clk_freq;
+	v0 =  25 * clk / 100000000;
+	v1 = clk / 10000;
+	v2 = clk / 10;
+	writel(((v0 > 1) ? v0 - 1 : 0), data->xhci_base
+		+ XEC_PRE_REG_250NS);
+	writel((unsigned int)((v1 / 100 > 1) > 0 ? (v1 / 100) - 1 : 0), data->xhci_base
+		+ XEC_PRE_REG_1US);
+	writel((unsigned int)((v1 / 10 > 1) > 0 ? (v1 / 10) - 1 : 0), data->xhci_base
+		+ XEC_PRE_REG_10US);
+	writel(((v1) > 1 ? v1 - 1 : 0), data->xhci_base
+		+ XEC_PRE_REG_100US);
+	writel((unsigned int)((125 * clk / 1000000) > 1 ? (125 * clk / 1000000) : 0),
+	       data->xhci_base + XEC_PRE_REG_125US);
+	writel(((v2 / 100 > 1) ? (v2 / 100) - 1 : 0), data->xhci_base
+		+ XEC_PRE_REG_1MS);
+	writel(((v2 / 10 > 1) ? (v2 / 10) - 1 : 0), data->xhci_base
+		+ XEC_PRE_REG_10MS);
+	writel((v2 > 1 ? v2 - 1 : 0), data->xhci_base
+		+ XEC_PRE_REG_100MS);
+	clk = data->lpm_clk_freq;
+	v0 =  25 * clk / 100000000;
+	v1 = clk / 10000;
+	v2 = clk / 10;
+	writel(((v0 > 1) ? v0 - 1 : 0), data->xhci_base
+		+ XEC_LPM_PRE_REG_250NS);
+	writel((unsigned int)((v1 / 100 > 1) > 0 ? (v1 / 100) - 1 : 0), data->xhci_base
+		+ XEC_LPM_PRE_REG_1US);
+	writel((unsigned int)((v1 / 10 > 1) > 0 ? (v1 / 10) - 1 : 0), data->xhci_base
+		+ XEC_LPM_PRE_REG_10US);
+	writel(((v1) > 1 ? v1 - 1 : 0), data->xhci_base
+		+ XEC_LPM_PRE_REG_100US);
+	writel((unsigned int)((125 * clk / 1000000) > 1 ? (125 * clk / 1000000) : 0),
+	       data->xhci_base + XEC_LPM_PRE_REG_125US);
+	writel(((v2 / 100 > 1) ? (v2 / 100) - 1 : 0), data->xhci_base
+		+ XEC_LPM_PRE_REG_1MS);
+	writel(((v2 / 10 > 1) ? (v2 / 10) - 1 : 0), data->xhci_base
+		+ XEC_LPM_PRE_REG_10MS);
+	writel((v2 > 1 ? v2 - 1 : 0), data->xhci_base
+		+ XEC_LPM_PRE_REG_100MS);
+	val = readl(data->xhci_base + XEC_USBSSP_CLK_GATING_CTRL);
+	val |= HOST20_ACLK_GATING_DISABLE | HOST20_UTMI_GATING_DISABLE;
+	writel(val, data->xhci_base + XEC_USBSSP_CLK_GATING_CTRL);
+}
+
+static int cdnsp_sky1_drd_init(struct cdnsp_sky1 *data)
+{
+	int ret = 0;
+
+	reset_control_assert(data->reset);
+	reset_control_assert(data->preset);
+	cdnsp_sky1_clk_disable_all(data->dev);
+	ret = cdnsp_sky1_clk_enable_all(data->dev);
+	if (ret)
+		return ret;
+	writel(CIX_USB_AXI_WR_CACHE_VALUE, data->rcsu_axi_base);
+	cdnsp_sky1_set_mode_by_id(data->dev, MODE_STRAP_OTG);
+	reset_control_deassert(data->preset);
+	cdnsp_sky1_configure_controller(data);
+	reset_control_deassert(data->reset);
+	return ret;
+}
+
+static void __iomem *sky1_of_get_addr_by_name(struct device_node *parent, const char *name)
+{
+	void __iomem *addr = NULL;
+	struct device_node *node;
+	int index;
+
+	for_each_child_of_node(parent, node) {
+		index = of_property_match_string(node, "reg-names", name);
+		if (index >= 0) {
+			addr = of_iomap(node, index);
+			of_node_put(node);
+			break;
+		}
+	}
+
+	return addr;
+}
+
+static void __iomem *sky1_get_addr_by_name(struct device *dev, const char *name)
+{
+	return sky1_of_get_addr_by_name(dev->of_node, name);
+}
+
+static void sky1_put_addr(void __iomem *regs)
+{
+	if (regs)
+		iounmap(regs);
+}
+
+static int cdnsp_sky1_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct of_dev_auxdata cdns_sky1_auxdata[] = {
+		{ .compatible = "cdns,usb3", },
+		{},
+	};
+	struct cdnsp_sky1 *data;
+	struct cdns3_platform_data *cdns_sky1_pdata;
+	int ret = 0;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+	data->rcsu_axi_base = devm_platform_ioremap_resource_byname(pdev, "axi");
+	if (IS_ERR(data->rcsu_axi_base))
+		return PTR_ERR(data->rcsu_axi_base);
+	data->rcsu_ctst_base = devm_platform_ioremap_resource_byname(pdev, "status");
+	if (IS_ERR(data->rcsu_ctst_base))
+		return PTR_ERR(data->rcsu_ctst_base);
+	data->reset = devm_reset_control_get_exclusive(&pdev->dev, "core");
+	if (IS_ERR(data->reset)) {
+		ret = PTR_ERR(data->reset);
+		dev_err(dev, "get reset error:%d\n", ret);
+		return ret;
+	}
+	data->preset = devm_reset_control_get_exclusive(&pdev->dev, "apb");
+	if (IS_ERR(data->preset)) {
+		ret = PTR_ERR(data->preset);
+		dev_err(dev, "get reset error:%d\n", ret);
+		return ret;
+	}
+	platform_set_drvdata(pdev, data);
+	data->dev = dev;
+	ret = of_alias_get_id(dev->of_node, "usb");
+	if (ret == -ENODEV) {
+		if (device_property_read_u32(dev, "id", &ret))
+			ret = -ENODEV;
+	}
+	if (ret < 0 || ret >=  SKY1_USB_S5_NUM) {
+		dev_err(dev, "get alias failed.\n");
+		return ret;
+	}
+	data->id = ret;
+	data->usb_syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
+							   "cix,usb-syscon");
+	if (IS_ERR(data->usb_syscon)) {
+		dev_err(dev, "Unable to get cix,usb-syscon regmap");
+		return PTR_ERR(data->usb_syscon);
+	}
+	data->u3_disable = device_property_read_bool(dev, "u3-port-disable");
+	data->ssp_disable = device_property_read_bool(dev, "ssp-disable");
+	data->sof_clk_freq = CIX_USB_CLK_8M;
+	data->lpm_clk_freq = CIX_USB_CLK_32K;
+	data->axi_bmax_value = AXI_BMAX_VALUE_DEFAULT;
+	data->xhci_base = sky1_get_addr_by_name(dev, "xhci");
+	if (!data->xhci_base) {
+		ret = -ENODEV;
+		goto err_put_xhci;
+	}
+	data->device_base = sky1_get_addr_by_name(dev, "dev");
+	if (!data->device_base) {
+		ret = -ENODEV;
+		goto err_put_addrs;
+	}
+	ret = cdnsp_sky1_drd_init(data);
+	if (ret == -ETIMEDOUT)
+		ret = -EPROBE_DEFER;
+	if (ret) {
+		/*
+		 * drd_init() leaves the clocks disabled and the resets
+		 * asserted on failure; only the address mappings are
+		 * left to release.
+		 */
+		goto err_put_addrs;
+	}
+	data->oc_gpio = devm_gpiod_get_optional(data->dev, "oc", GPIOD_IN);
+	if (IS_ERR(data->oc_gpio)) {
+		dev_err(data->dev, "can not get oc_gpio\n");
+		ret = PTR_ERR(data->oc_gpio);
+		goto err_quiesce;
+	}
+	if (data->oc_gpio) {
+		ret = gpiod_direction_input(data->oc_gpio);
+		if (ret < 0)
+			dev_err(data->dev, "set oc_gpio input failed:%d\n", ret);
+	}
+	/*
+	 * The platform data is devm-allocated, so it stays alive as long as
+	 * this device; the child devices are depopulated before it is freed.
+	 */
+	cdns_sky1_pdata = devm_kzalloc(dev, sizeof(*cdns_sky1_pdata), GFP_KERNEL);
+	if (!cdns_sky1_pdata) {
+		ret = -ENOMEM;
+		goto err_quiesce;
+	}
+	cdns_sky1_pdata->platform_suspend = cdns_sky1_platform_suspend;
+	cdns_sky1_pdata->quirks = CDNS3_DEFAULT_PM_RUNTIME_ALLOW;
+	cdns_sky1_auxdata[0].platform_data = cdns_sky1_pdata;
+	ret = of_platform_populate(node, NULL, cdns_sky1_auxdata, dev);
+	if (ret) {
+		dev_err(dev, "failed to create children: %d\n", ret);
+		goto err_quiesce;
+	}
+	device_set_wakeup_capable(dev, true);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	return 0;
+
+err_quiesce:
+	reset_control_assert(data->reset);
+	reset_control_assert(data->preset);
+	cdnsp_sky1_clk_disable_all(dev);
+err_put_addrs:
+	sky1_put_addr(data->device_base);
+err_put_xhci:
+	sky1_put_addr(data->xhci_base);
+	return ret;
+}
+
+static void cdnsp_sky1_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(dev);
+	of_platform_depopulate(dev);
+	sky1_put_addr(data->xhci_base);
+	sky1_put_addr(data->device_base);
+	reset_control_assert(data->reset);
+	reset_control_assert(data->preset);
+	cdnsp_sky1_clk_disable_all(dev);
+	pm_runtime_disable(dev);
+	pm_runtime_set_suspended(dev);
+	pm_runtime_put_noidle(dev);
+}
+
+/*
+ * Because the wake-up interrupt and host interrupt are the same interrupt,
+ * closing the axi and sof clock will result in the inability to generate
+ * port status change interrupt.
+ */
+static int cdnsp_sky1_system_suspend(struct device *dev)
+{
+	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
+
+	if (!data->wakeup) {
+		reset_control_assert(data->reset);
+		reset_control_assert(data->preset);
+	}
+	cdnsp_sky1_clk_disable_suspend(dev);
+	return 0;
+}
+
+static int cdnsp_sky1_system_resume(struct device *dev)
+{
+	int ret = 0;
+	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
+
+	ret = cdnsp_sky1_clk_enable_resume(dev);
+	if (ret)
+		return ret;
+	if (!data->wakeup) {
+		writel(CIX_USB_AXI_WR_CACHE_VALUE, data->rcsu_axi_base);
+		cdnsp_sky1_set_mode_by_id(data->dev, MODE_STRAP_OTG);
+		reset_control_deassert(data->preset);
+		cdnsp_sky1_configure_controller(data);
+		reset_control_deassert(data->reset);
+	}
+	return 0;
+}
+
+static const struct dev_pm_ops cdnsp_sky1_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(cdnsp_sky1_system_suspend, cdnsp_sky1_system_resume)
+};
+
+static const struct of_device_id cdns_sky1_of_match[] = {
+	{ .compatible = "cix,sky1-usbssp", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, cdns_sky1_of_match);
+
+static int cdnsp_sky1_find_cdns_dev(struct device *dev, void *data)
+{
+	struct device **cdns_dev = data;
+
+	if (dev->of_node && of_device_is_compatible(dev->of_node, "cdns,usb3")) {
+		/* keep a reference: the pointer is used after the iteration */
+		*cdns_dev = get_device(dev);
+		return 1;
+	}
+	return 0;
+}
+
+static int cdnsp_sky1_find_gadget_match(struct device *dev, void *data)
+{
+	struct device **gadget_dev = data;
+	const char *name = dev_name(dev);
+	static const char gadget_prefix[] = "gadget.";
+
+	/*
+	 * The gadget device is registered on the gadget bus with name
+	 * "gadget.%d" (see usb_add_gadget_udc -> dev_set_name).
+	 * It sits on the gadget bus and has the function driver bound to it.
+	 * Verify the device is on the gadget bus by checking the bus name.
+	 * This prevents matching devices that happen to have "gadget." prefix
+	 * in their name but are not real gadget devices, and also avoids
+	 * NULL pointer dereference when device bus is being removed.
+	 */
+	if (name && dev->bus && !strcmp(dev->bus->name, "gadget") &&
+	    !strncmp(name, gadget_prefix, sizeof(gadget_prefix) - 1)) {
+		/* keep a reference: the pointer is used after the iteration */
+		*gadget_dev = get_device(dev);
+		return 1;
+	}
+	return 0;
+}
+
+static void cdnsp_sky1_shutdown(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
+	struct device *cdns_dev = NULL;
+	struct device *gadget_dev = NULL;
+	struct cdns *cdns = NULL;
+
+	if (!device_may_wakeup(dev)) {
+		/*
+		 * Find the cdns3 child device, then find its gadget device
+		 * and release the function driver before disabling clocks.
+		 * This ensures all register accesses in gadget_unbind_driver
+		 * complete before clocks are turned off.
+		 */
+		device_for_each_child(dev, &cdns_dev, cdnsp_sky1_find_cdns_dev);
+		if (cdns_dev)
+			cdns = dev_get_drvdata(cdns_dev);
+		if (cdns)
+			device_for_each_child(cdns_dev, &gadget_dev,
+					      cdnsp_sky1_find_gadget_match);
+		if (gadget_dev)
+			device_release_driver(gadget_dev);
+		if (cdns && cdns->host_dev) {
+			struct usb_hcd *hcd = platform_get_drvdata(cdns->host_dev);
+
+			if (hcd && hcd->irq > 0) {
+				/*
+				 * Clear HCD_FLAG_HW_ACCESSIBLE before disable_irq.
+				 * This prevents usb_hcd_irq from calling xhci_irq
+				 * (which reads USBSTS) after clocks are disabled.
+				 */
+				clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+				disable_irq(hcd->irq);
+				synchronize_irq(hcd->irq);
+			}
+		}
+		reset_control_assert(data->reset);
+		reset_control_assert(data->preset);
+		cdnsp_sky1_clk_disable_all(dev);
+		put_device(gadget_dev);
+		put_device(cdns_dev);
+	}
+}
+
+static struct platform_driver cdnsp_sky1_driver = {
+	.probe		= cdnsp_sky1_probe,
+	.remove		= cdnsp_sky1_remove,
+	.shutdown	= cdnsp_sky1_shutdown,
+	.driver		= {
+		.name	= "cdnsp-sky1",
+		.of_match_table	= cdns_sky1_of_match,
+		.pm	= pm_sleep_ptr(&cdnsp_sky1_pm_ops),
+	},
+};
+
+module_platform_driver(cdnsp_sky1_driver);
+
+MODULE_ALIAS("platform:cdnsp-sky1");
+MODULE_DESCRIPTION("CIX Sky1 Cadence USBSSP DRD glue driver");
+MODULE_AUTHOR("Hongliang Yang <hongliang.yang@cixtech.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/usb/cdns3/cdnsp-sky1.h b/drivers/usb/cdns3/cdnsp-sky1.h
new file mode 100644
index 000000000000..ac97d3e054e6
--- /dev/null
+++ b/drivers/usb/cdns3/cdnsp-sky1.h
@@ -0,0 +1,128 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _CDNSP_SKY1_H
+#define _CDNSP_SKY1_H
+#include <linux/clk.h>
+#include <linux/gpio/consumer.h>
+#include <linux/reset.h>
+#include <linux/types.h>
+
+#define USB_MODE_STRAP_S5_DOMAIN        0x424
+
+#define MODE_STRAP_OTG                  0
+
+#define U3_TYPEC_DRD_ID                 0
+#define U3_TYPEC_HOST0_ID               1
+#define U3_TYPEC_HOST1_ID               2
+#define U3_TYPEC_HOST2_ID               3
+#define U3_TYPEA_CTRL0_ID               4
+#define U3_TYPEA_CTRL1_ID               5
+#define U2_HOST0_ID                     6
+#define U2_HOST1_ID                     7
+#define U2_HOST2_ID                     8
+#define U2_HOST3_ID                     9
+#define SKY1_USB_S5_NUM                 10
+
+#define U3_TYPEC_DRD_MODE_STRAP_BIT     12
+#define U3_TYPEC_HOST0_MODE_STRAP_BIT   14
+#define U3_TYPEC_HOST1_MODE_STRAP_BIT   16
+#define U3_TYPEC_HOST2_MODE_STRAP_BIT   18
+#define U3_TYPEA_CTRL0_MODE_STRAP_BIT   8
+#define U3_TYPEA_CTRL1_MODE_STRAP_BIT   10
+#define U2_HOST0_MODE_STRAP_BIT         0
+#define U2_HOST1_MODE_STRAP_BIT         2
+#define U2_HOST2_MODE_STRAP_BIT         4
+#define U2_HOST3_MODE_STRAP_BIT         6
+
+#define AXI_HALT                        BIT(31)
+#define AXI_BMAX_VALUE_DEFAULT          0x7
+
+#define D_XEC_CFG_3XPORT_MODE           0x2040
+#define D_XEC_AXI_CAP                   0x2174
+#define D_XEC_AXI_CTRL0                 0x217C
+#define D_XEC_PRE_REG_250NS             0x21E8
+#define D_XEC_PRE_REG_1US               0x21EC
+#define D_XEC_PRE_REG_10US              0x21F0
+#define D_XEC_PRE_REG_100US             0x21F4
+#define D_XEC_PRE_REG_125US             0x21F8
+#define D_XEC_PRE_REG_1MS               0x21FC
+#define D_XEC_PRE_REG_10MS              0x2200
+#define D_XEC_PRE_REG_100MS             0x2204
+#define D_XEC_LPM_PRE_REG_250NS         0x2208
+#define D_XEC_LPM_PRE_REG_1US           0x220C
+#define D_XEC_LPM_PRE_REG_10US          0x2210
+#define D_XEC_LPM_PRE_REG_100US         0x2214
+#define D_XEC_LPM_PRE_REG_125US         0x2218
+#define D_XEC_LPM_PRE_REG_1MS           0x221C
+#define D_XEC_LPM_PRE_REG_10MS          0x2220
+#define D_XEC_LPM_PRE_REG_100MS         0x2224
+
+#define XEC_CFG_3XPORT_MODE             0x2040
+#define XEC_PRE_REG_250NS               0x21E8
+#define XEC_PRE_REG_1US                 0x21EC
+#define XEC_PRE_REG_10US                0x21F0
+#define XEC_PRE_REG_100US               0x21F4
+#define XEC_PRE_REG_125US               0x21F8
+#define XEC_PRE_REG_1MS                 0x21FC
+#define XEC_PRE_REG_10MS                0x2200
+#define XEC_PRE_REG_100MS               0x2204
+#define XEC_LPM_PRE_REG_250NS           0x2208
+#define XEC_LPM_PRE_REG_1US             0x220C
+#define XEC_LPM_PRE_REG_10US            0x2210
+#define XEC_LPM_PRE_REG_100US           0x2214
+#define XEC_LPM_PRE_REG_125US           0x2218
+#define XEC_LPM_PRE_REG_1MS             0x221C
+#define XEC_LPM_PRE_REG_10MS            0x2220
+#define XEC_LPM_PRE_REG_100MS           0x2224
+#define XEC_USBSSP_CHICKEN_BITS_3       0x2230
+#define XEC_USBSSP_CLK_GATING_CTRL      0x2234
+#define XECP_PM_PMCSR                   0x2240
+
+/* XEC_USBSSP_CLK_GATING_CTRL */
+#define HOST20_ACLK_GATING_DISABLE      BIT(4)
+#define HOST20_UTMI_GATING_DISABLE      BIT(5)
+
+/* XECP_PM_PMCSR */
+#define PS_MASK                         GENMASK(1, 0)
+#define PS_D0                           0
+#define PS_D3                           3
+#define PS_PME_En                       BIT(8)
+
+#define D_XEC_CFG_3XPORT_MODE_VALUE     0xa0031e07
+#define XEC_CFG_3XPORT_MODE_VALUE       0xa0031e07
+#define CFG_3XPORT_MODE_DIS_SSP         (~BIT(31))
+
+#define CFG_APB_TIMEOUT_PSLVERR_EN      BIT(22)
+#define CFG_APB_PSLVERR_EN              BIT(23)
+
+#define CIX_USB_CLK_NUM                 (4)
+#define CIX_USB_CLK_OFF_NUM             (2)
+#define CIX_USB_AXI_WR_CACHE_VALUE      0x33
+#define CIX_USB_CLK_32K                 32000
+#define CIX_USB_CLK_8M                  8000000
+
+#define AXI_CLOCK_VALID                 BIT(1)
+
+struct cdnsp_sky1 {
+	struct device *dev;
+	void __iomem *rcsu_axi_base;
+	void __iomem *rcsu_ctst_base;
+	void __iomem *dr_base;
+	void __iomem *xhci_base;
+	void __iomem *device_base;
+	struct platform_device *cdnsp_pdev;
+	struct reset_control *reset;
+	struct reset_control *preset;
+	struct clk *cix_usb_clks[CIX_USB_CLK_NUM];
+	int id;
+	struct regmap	*usb_syscon;
+	int lpm_clk_freq;
+	int sof_clk_freq;
+	bool u3_disable;
+	bool ssp_disable;
+	int axi_bmax_value;
+	struct gpio_desc *oc_gpio;
+	bool wakeup;
+};
+
+#endif /* _CDNSP_SKY1_H */
-- 
2.54.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 1/3] dt-bindings: usb: Add CIX Sky1 USBSSP controller
  2026-11-03  7:49 ` [PATCH v5 1/3] dt-bindings: usb: Add CIX Sky1 USBSSP controller Hongliang Yang (杨宏亮)
@ 2026-11-03  8:01   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-11-03  8:01 UTC (permalink / raw)
  To: Hongliang Yang (杨宏亮)
  Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, robh,
	conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: usb: cix,sky1-usbssp: invalid bus node name and property order
--

commit 26d70ffe3b921fd15f5adca3d546e808116719c0
Author: Hongliang Yang (杨宏亮) <hongliang.yang@cixtech.com>

dt-bindings: usb: Add CIX Sky1 USBSSP controller

This commit adds the Device Tree binding schema for the CIX Sky1 Cadence
USBSSP controller glue layer and updates MAINTAINERS.

> diff --git a/Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml b/Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml
> new file mode 100644
> index 0000000000000..c29654567e765
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml
> @@ -0,0 +1,152 @@
[ ... ]
> +properties:
> +  compatible:
> +    const: cix,sky1-usbssp
> +
> +  reg:
> +    items:
> +      - description: AXI bus attribute configuration registers
> +      - description: Controller status registers
> +
> +  reg-names:
> +    items:
> +      - const: axi
> +      - const: status
[ ... ]
> +  cix,usb-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to the Sky1 S5 system controller used to program USB mode
> +      strap controls.
> +
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 2
> +
> +  ranges: true
> +
> +patternProperties:
> +  "^usb@[0-9a-f]+$":
> +    $ref: cdns,usb3.yaml#
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - cix,usb-syscon
> +  - "#address-cells"
> +  - "#size-cells"
> +  - ranges
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/cix,sky1.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/reset/cix,sky1-s5-system-control.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        usb-rcsu@91c0300 {
> +            compatible = "simple-bus";
> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +            ranges = <0x00 0x00000000 0x00 0x091c0300 0x00 0x150>,
> +                     <0x00 0x091d0000 0x00 0x091d0000 0x00 0x20000>;

[Severity: Low]
Does this pass make dt_binding_check and scripts/dtc/dt-check-style
--mode=strict?

In the example, usb-rcsu@91c0300 has compatible = "simple-bus", which
requires a node name matching ^(bus|soc|axi|ahb|apb|isa|pci-ep-bus)(@[0-9a-f]+)?$
under schemas/simple-bus.yaml (such as bus@91c0300).

Also, in usb-rcsu@91c0300, ranges is placed after #address-cells and
#size-cells, which triggers a property-order warning from
scripts/dtc/dt-check-style --mode=strict since ranges should precede
standard properties per Documentation/devicetree/bindings/dts-coding-style.rst.

Similarly, in the top-level properties and required lists of
Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml, should ranges
be placed right after reg-names rather than after cix,usb-syscon,
#address-cells, and #size-cells per
Documentation/devicetree/bindings/writing-schema.rst?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103074929.1971195-1-hongliang.yang@cixtech.com?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 3/3] arm64: dts: cix: Add USB4/USB5 nodes for Sky1
  2026-11-03  7:49 ` [PATCH v5 3/3] arm64: dts: cix: Add USB4/USB5 nodes for Sky1 Hongliang Yang (杨宏亮)
@ 2026-11-03  8:06   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-11-03  8:06 UTC (permalink / raw)
  To: Hongliang Yang (杨宏亮)
  Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, robh,
	conor+dt

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Medium] arm64: dts: cix: usb4/usb5 aliases used to index syscon mode strap bits
- [Medium] usb: cdns3: cdnsp_sky1_probe() returns positive value on invalid ID
- [Low] arm64: dts: cix: invalid simple-bus node name usb-rcsu@91c0300
- [Low] usb: cdns3: cdnsp_sky1_probe() parses undocumented DT properties
- [Low] usb: cdns3: cdnsp_sky1_drd_init() disables clocks before acquiring them
- [Low] usb: cdns3: Kconfig help text for USB_CDNSP_SKY1 lists wrong compatible
--

commit f03c94102bbd5a8d24fc4ea8c5c765d7b2a4ab36
Author: Hongliang Yang (杨宏亮) <hongliang.yang@cixtech.com>

arm64: dts: cix: Add USB4/USB5 nodes for Sky1

This commit adds the USB4 and USB5 Cadence USBSSP controller nodes wrapped in
a shared RCSU simple-bus region for the CIX Sky1 SoC in sky1.dtsi, and
enables both controllers in host mode on the Radxa Orion O6 board.

> diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
> index 06a311ee14ef8..c10db0ff9af64 100644
> --- a/arch/arm64/boot/dts/cix/sky1.dtsi
> +++ b/arch/arm64/boot/dts/cix/sky1.dtsi
> @@ -17,6 +17,11 @@ / {
>  	#address-cells = <2>;
>  	#size-cells = <2>;
>  
> +	aliases {
> +		usb4 = &usb4;
> +		usb5 = &usb5;
> +	};

[Severity: Medium]
Are these usb4 and usb5 aliases being used to index hardware mode strap bits
in the S5 syscon?

In drivers/usb/cdns3/cdnsp-sky1.c, cdnsp_sky1_probe() looks up the alias ID
or an undocumented id property and passes it to cdnsp_sky1_set_mode_by_id():

drivers/usb/cdns3/cdnsp-sky1.c:cdnsp_sky1_probe() {
	...
	ret = of_alias_get_id(dev->of_node, "usb");
	if (ret == -ENODEV) {
		if (device_property_read_u32(dev, "id", &ret))
			ret = -ENODEV;
	}
	if (ret < 0 || ret >=  SKY1_USB_S5_NUM) {
		dev_err(dev, "get alias failed.\n");
		return ret;
	}
	data->id = ret;
	...
}

drivers/usb/cdns3/cdnsp-sky1.c:cdnsp_sky1_set_mode_by_id() {
	struct cdnsp_sky1 *data = dev_get_drvdata(dev);

	return regmap_update_bits(data->usb_syscon,
				  strap_signals[data->id].offset,
				  GENMASK(strap_signals[data->id].bit + 1,
					  strap_signals[data->id].bit),
				  mode << strap_signals[data->id].bit);
}

Documentation/devicetree/bindings/writing-bindings.rst states that nodes
should not rely on custom aliases or instance ID properties to parameterize
how they act on a syscon, and should instead pass the register or bit offset
cells directly in the cix,usb-syscon phandle arguments.

With the current approach, if a board dts omits the alias (as in the example
in Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml, which also
does not document any alias or id requirement), cdnsp_sky1_probe() fails with
-ENODEV. Conversely, if a board renumbers its aliases (for example,
usb0 = &usb4), cdnsp_sky1_set_mode_by_id() will program the wrong
controller's strap bits in s5_syscon. Also, placing aliases in the SoC dtsi
exposes aliases to disabled nodes on boards that do not enable these
controllers.

Could the syscon register offset and bit position be passed as phandle
arguments in cix,usb-syscon instead of using usb aliases?

[Severity: Medium]
Related to the alias lookup in cdnsp_sky1_probe() shown above, what happens
if the alias ID or id property is greater than or equal to SKY1_USB_S5_NUM?

	if (ret < 0 || ret >=  SKY1_USB_S5_NUM) {
		dev_err(dev, "get alias failed.\n");
		return ret;
	}

When ret >= SKY1_USB_S5_NUM, does cdnsp_sky1_probe() return a positive
integer to really_probe() instead of a negative errno such as -EINVAL?

> +
>  	cpus {
>  		#address-cells = <2>;
>  		#size-cells = <0>;
> @@ -606,6 +611,96 @@ mbox_ap2sfh: mailbox@80a0000 {
>  			cix,mbox-dir = "tx";
>  		};
>  
> +		usbss_rcsu: usb-rcsu@91c0300 {
> +			compatible = "simple-bus";

[Severity: Low]
Does the node name usb-rcsu@91c0300 pass dtbs_check and dt_binding_check
when paired with compatible = "simple-bus"?

The dtschema simple-bus.yaml schema requires nodes with only
compatible = "simple-bus" to use a node name matching
^(bus|soc|axi|ahb|apb)(@[0-9a-f]+)?$. Should this node (and the corresponding
node in the example in Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml)
be named bus@91c0300 instead?

> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges = <0x00 0x00000000 0x00 0x091c0300 0x00 0x150>,
> +				<0x00 0x091d0000 0x00 0x091d0000 0x00 0x20000>;
> +
> +			usb4: usb@14 {
> +				compatible = "cix,sky1-usbssp";

[Severity: Low]
In drivers/usb/cdns3/Kconfig, the help text for config USB_CDNSP_SKY1 says:

	  Glue driver for the Cadence USB dual-role controllers on CIX Sky1
	  (device tree compatible cix,sky1-usb3). It enables clocks and resets

Should that help text say cix,sky1-usbssp to match cdns_sky1_of_match,
cix,sky1-usbssp.yaml, and sky1.dtsi?

[Severity: Low]
When cdnsp_sky1_probe() binds to these cix,sky1-usbssp nodes, it also queries
several properties that are not documented in
Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml:

drivers/usb/cdns3/cdnsp-sky1.c:cdnsp_sky1_probe() {
	...
	if (ret == -ENODEV) {
		if (device_property_read_u32(dev, "id", &ret))
			ret = -ENODEV;
	}
	...
	data->u3_disable = device_property_read_bool(dev, "u3-port-disable");
	data->ssp_disable = device_property_read_bool(dev, "ssp-disable");
	...
	data->oc_gpio = devm_gpiod_get_optional(data->dev, "oc", GPIOD_IN);
	...
}

Because cix,sky1-usbssp.yaml sets unevaluatedProperties: false, any dts using
id, u3-port-disable, ssp-disable, or oc-gpios will fail schema validation, and
data->oc_gpio is never referenced again after cdnsp_sky1_probe(). Should
these undocumented property reads and the unused oc_gpio handling be removed
from cdnsp_sky1_probe(), or documented in the binding if needed?

[Severity: Low]
Also during probe of these nodes, cdnsp_sky1_probe() calls
cdnsp_sky1_drd_init(), which disables all clocks before enabling them:

drivers/usb/cdns3/cdnsp-sky1.c:cdnsp_sky1_drd_init() {
	int ret = 0;

	reset_control_assert(data->reset);
	reset_control_assert(data->preset);
	cdnsp_sky1_clk_disable_all(data->dev);
	ret = cdnsp_sky1_clk_enable_all(data->dev);
	...
}

Since data->cix_usb_clks[] is only populated with devm_clk_get() inside
cdnsp_sky1_clk_enable_all(), isn't cdnsp_sky1_clk_disable_all() just calling
clk_disable_unprepare(NULL) on all four uninitialized array entries here?
Should the cdnsp_sky1_clk_disable_all() call in cdnsp_sky1_drd_init() be
dropped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103074929.1971195-1-hongliang.yang@cixtech.com?part=3

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 2/3] usb: cdns3: Add CIX Sky1 glue driver
  2026-11-03  7:49 ` [PATCH v5 2/3] usb: cdns3: Add CIX Sky1 glue driver Hongliang Yang (杨宏亮)
@ 2026-11-03  8:15   ` sashiko-bot
  2026-11-03  8:43   ` Philipp Zabel
  1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-11-03  8:15 UTC (permalink / raw)
  To: Hongliang Yang (杨宏亮)
  Cc: linux-usb@vger.kernel.org, devicetree@vger.kernel.org, robh,
	conor+dt

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] usb: cdns3: unclocked MMIO access and host_dev race in cdnsp_sky1_shutdown()
- [High] usb: cdns3: controller reset in cdnsp_sky1_system_suspend() breaks system resume
- [Medium] usb: cdns3: positive probe return, pointer-sign mismatch, and DT mismatch
- [Medium] usb: cdns3: ignored D3 errors and broken D0 poll in cdns_sky1_platform_suspend()
- [Medium] usb: cdns3: cdnsp_sky1_system_suspend() disables wakeup clocks when wakeup is set
- [Medium] usb: cdns3: clock disable underflow on remove/shutdown after resume failure
- [Medium] usb: cdns3: raw ~AXI_HALT write in cdnsp_sky1_configure_controller()
- [Medium] usb: cdns3: 125us prescaler off-by-one in cdnsp_sky1_configure_controller()
- [Low] usb: cdns3: cdnsp_sky1_drd_init() disables clocks before devm_clk_get()
- [Low] usb: cdns3: dead fields, unused oc_gpio, and wrong compatible in Kconfig
--

commit fcb2f6f1fc38d7bbecb98bf69c74a74c7c5ceec3
Author: Hongliang Yang (杨宏亮) <hongliang.yang@cixtech.com>

usb: cdns3: Add CIX Sky1 glue driver

This commit adds a platform glue driver for the Cadence USBSSP dual-role
controller on the CIX Sky1 SoC, managing clocks, resets, S5 system
controller mode straps, and per-controller glue registers.

> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
> index 39ad23d1ada89..03d8e170a98af 100644
> --- a/drivers/usb/cdns3/Kconfig
> +++ b/drivers/usb/cdns3/Kconfig
> @@ -77,6 +77,19 @@ config USB_CDNSP_PCI
>  	  If you choose to build this driver as module it will
>  	  be dynamically linked and module will be called cdnsp-pci.ko
>  
> +config USB_CDNSP_SKY1
> +	tristate "Cadence USB3 support on CIX Sky1 SoC platforms"
> +	depends on USB_CDNS3
> +	depends on ARCH_CIX || COMPILE_TEST
> +	default USB_CDNS3
> +	help
> +	  Glue driver for the Cadence USB dual-role controllers on CIX Sky1
> +	  (device tree compatible cix,sky1-usb3). It enables clocks and resets
> +	  from the SoC, then uses the shared cdns3 platform core (cdns.ko).

[Severity: Low]
This isn't a bug, but the help text for USB_CDNSP_SKY1 mentions compatible
cix,sky1-usb3 whereas cdns_sky1_of_match and
Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml use
cix,sky1-usbssp, and the depends on USB_CDNS3 line is inside the enclosing if
USB_CDNS3 block. Also, in struct cdnsp_sky1 in drivers/usb/cdns3/cdnsp-sky1.h,
are the dr_base and cdnsp_pdev members, as well as oc_gpio in
cdnsp_sky1_probe() (where gpiod_direction_input() is called after requesting
with GPIOD_IN, and the descriptor is never referenced again), still needed?

> diff --git a/drivers/usb/cdns3/cdnsp-sky1.c b/drivers/usb/cdns3/cdnsp-sky1.c
> new file mode 100644
> index 0000000000000..1f7870289a35f
> --- /dev/null
> +++ b/drivers/usb/cdns3/cdnsp-sky1.c

[ ... ]

> +static int cdns_sky1_platform_suspend(struct device *dev,
> +				      bool suspend, bool wakeup)
> +{
> +	struct cdns *cdns = dev_get_drvdata(dev);
> +	struct platform_device *xhci_dev = cdns->host_dev;
> +	struct usb_hcd  *hcd;
> +	struct device *parent = cdns->dev->parent;
> +	struct cdnsp_sky1 *data = dev_get_drvdata(parent);
> +	u32 value;
> +	int poll;
> +	int ret = 0;
> +	int count = 3;
> +
> +	data->wakeup = wakeup;
> +
> +	if (cdns->role != USB_ROLE_HOST)
> +		return 0;
> +
> +	if (!xhci_dev) {
> +		dev_dbg(dev, "host controller has not been registered\n");
> +		return 0;
> +	}
> +
> +	hcd = dev_get_drvdata(&xhci_dev->dev);
> +	if (!hcd) {
> +		dev_dbg(dev, "host controller have not registered\n");
> +		return 0;
> +	}
> +
> +	if (suspend) {
> +		while (count--) {
> +			value = readl(hcd->regs + XECP_PM_PMCSR);
> +			value &= ~PS_MASK;
> +			value |= PS_D3 | PS_PME_En;
> +			writel(value, hcd->regs + XECP_PM_PMCSR);
> +			/* After controller enters D3, disable AXI and SOF
> +			 * until AXI valid flag changes to 0.
> +			 */
> +			if (sky1_rcsu_handshake(data, AXI_CLOCK_VALID, 0,
> +						100ULL * 1000)) {
> +				u32 ctst;
> +
> +				ctst = readl(data->rcsu_ctst_base);
> +				dev_dbg(dev, "enter D3 failed, register value:%x\n",
> +					ctst);
> +			} else {
> +				break;
> +			}
> +		}
> +		if (count < 0) {
> +			u32 ctst;
> +
> +			ctst = readl(data->rcsu_ctst_base);
> +			dev_err(dev, "enter D3 failed after retries, register value:%x\n",
> +				ctst);
> +		}
> +	} else {
> +		while (count--) {
> +			value = readl(hcd->regs + XECP_PM_PMCSR);
> +			value &= ~PS_MASK;
> +			value |= PS_D0;
> +			value &= ~PS_PME_En;
> +			writel(value, hcd->regs + XECP_PM_PMCSR);
> +			/* Wait power state back to D0 */
> +			poll = readl_poll_timeout(hcd->regs + XECP_PM_PMCSR,
> +						  value,
> +						  (value & PS_MASK) == 0 ||
> +						  value == U32_MAX,
> +						  10, 100ULL * 1000);
> +			if (poll) {
> +				dev_dbg(dev, "exit D3 timeout, power state=0x%lx\n",
> +					value & PS_MASK);
> +			} else {
> +				break;
> +			}
> +		}
> +		if (count < 0) {
> +			dev_err(dev, "exit D3 timeout after retries, power state=0x%lx\n",
> +				value & PS_MASK);
> +		}
> +	}
> +
> +	return ret;
> +}

[Severity: Medium]
In cdns_sky1_platform_suspend(), ret is initialized to 0 and never updated, so
when sky1_rcsu_handshake() or readl_poll_timeout() fails all retries (count <
0), the function logs an error but still returns 0 to
cdns3_controller_suspend() or cdns3_controller_resume().

In addition, in the resume (!suspend) branch, readl_poll_timeout() stops when
(value & PS_MASK) == 0 || value == U32_MAX, but unlike sky1_rcsu_handshake()
it does not treat value == U32_MAX as an error, so reading U32_MAX (where
value & PS_MASK is 3) is treated as a successful transition to D0.

Also, in the suspend branch, value |= PS_D3 | PS_PME_En enables PS_PME_En
even when wakeup is false, and when cdns->role != USB_ROLE_HOST the function
returns early without clearing AXI_CLOCK_VALID before
cdnsp_sky1_system_suspend() disables the sof and axi clocks. Should
cdns_sky1_platform_suspend() propagate an error code on failure, check for
U32_MAX on D0 polling, and only set PS_PME_En when wakeup is enabled?

> +static void cdnsp_sky1_configure_controller(struct cdnsp_sky1 *data)
> +{
> +	int clk;
> +	int v0, v1, v2;
> +	u32 val = 0;
> +
> +	if (data->u3_disable) {
> +		dev_dbg(data->dev, "disable u3 port\n");
> +		writel(D_XEC_CFG_3XPORT_MODE_VALUE, data->device_base
> +			+ D_XEC_CFG_3XPORT_MODE);
> +	}
> +	writel(AXI_HALT, data->device_base + D_XEC_AXI_CAP);
> +	writel(AXI_HALT, data->xhci_base + D_XEC_AXI_CAP);
> +	writel(data->axi_bmax_value, data->device_base + D_XEC_AXI_CTRL0);
> +	writel(data->axi_bmax_value, data->xhci_base + D_XEC_AXI_CTRL0);
> +	writel((~(u32)(AXI_HALT)), data->device_base + D_XEC_AXI_CAP);
> +	writel((~(u32)(AXI_HALT)), data->xhci_base + D_XEC_AXI_CAP);

[Severity: Medium]
In cdnsp_sky1_configure_controller(), AXI_HALT is defined as BIT(31). Writing
AXI_HALT and then (~(u32)(AXI_HALT)) directly to D_XEC_AXI_CAP first clears
bits [30:0] to 0 and then writes 0x7fffffff, setting all lower 31 bits to 1.
Should these accesses to D_XEC_AXI_CAP be read-modify-write operations using
readl() and writel() to set and clear only AXI_HALT?

> +	clk = data->sof_clk_freq;
> +	v0 =  25 * clk / 100000000;
> +	v1 = clk / 10000;
> +	v2 = clk / 10;
> +	writel(((v0 > 1) ? v0 - 1 : 1), data->device_base
> +		+ D_XEC_PRE_REG_250NS);
> +	writel((unsigned int)((v1 / 100 > 1) > 0 ? (v1 / 100) - 1 : 1),
> +	       data->device_base + D_XEC_PRE_REG_1US);
> +	writel((unsigned int)((v1 / 10 > 1) > 0 ? (v1 / 10) - 1 : 1),
> +	       data->device_base + D_XEC_PRE_REG_10US);
> +	writel(((v1) > 1 ? v1 - 1 : 1), data->device_base
> +		+ D_XEC_PRE_REG_100US);
> +	writel((unsigned int)((125 * clk / 1000000) > 1 ? (125 * clk / 1000000) : 1),
> +	       data->device_base + D_XEC_PRE_REG_125US);

[Severity: Medium]
In cdnsp_sky1_configure_controller(), all of the other prescaler registers
subtract 1 from the cycle count when the value is greater than 1 (for example,
for the 8 MHz SOF clock, 1us writes 7, 10us writes 79, 100us writes 799, and
1ms writes 7999). However, all four 125us register writes
(D_XEC_PRE_REG_125US, D_XEC_LPM_PRE_REG_125US, XEC_PRE_REG_125US, and
XEC_LPM_PRE_REG_125US) check ((125 * clk / 1000000) > 1) and write
(125 * clk / 1000000) without subtracting 1 (writing 1000 instead of 999 for
8 MHz, and 4 instead of 3 for 32 kHz).

Also, XEC_CFG_3XPORT_MODE on xhci_base handles both data->u3_disable and
data->ssp_disable, whereas D_XEC_CFG_3XPORT_MODE on device_base only checks
data->u3_disable. Should the four 125us prescaler calculations subtract 1, and
should data->ssp_disable also be applied to D_XEC_CFG_3XPORT_MODE?

[ ... ]

> +static int cdnsp_sky1_drd_init(struct cdnsp_sky1 *data)
> +{
> +	int ret = 0;
> +
> +	reset_control_assert(data->reset);
> +	reset_control_assert(data->preset);
> +	cdnsp_sky1_clk_disable_all(data->dev);
> +	ret = cdnsp_sky1_clk_enable_all(data->dev);

[Severity: Low]
In cdnsp_sky1_drd_init(), cdnsp_sky1_clk_disable_all() is called before
cdnsp_sky1_clk_enable_all(). Because devm_clk_get() is called inside
cdnsp_sky1_clk_enable_all() rather than once in cdnsp_sky1_probe(),
data->cix_usb_clks[] still contains NULL pointers when
cdnsp_sky1_clk_disable_all() runs during probe. Should the clocks be acquired
with devm_clk_get() in cdnsp_sky1_probe() before calling
cdnsp_sky1_drd_init()?

[ ... ]

> +static int cdnsp_sky1_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct of_dev_auxdata cdns_sky1_auxdata[] = {
> +		{ .compatible = "cdns,usb3", },
> +		{},
> +	};
> +	struct cdnsp_sky1 *data;
> +	struct cdns3_platform_data *cdns_sky1_pdata;
> +	int ret = 0;
> +

[ ... ]

> +	platform_set_drvdata(pdev, data);
> +	data->dev = dev;
> +	ret = of_alias_get_id(dev->of_node, "usb");
> +	if (ret == -ENODEV) {
> +		if (device_property_read_u32(dev, "id", &ret))
> +			ret = -ENODEV;
> +	}
> +	if (ret < 0 || ret >=  SKY1_USB_S5_NUM) {
> +		dev_err(dev, "get alias failed.\n");
> +		return ret;
> +	}
> +	data->id = ret;
> +	data->usb_syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
> +							   "cix,usb-syscon");
> +	if (IS_ERR(data->usb_syscon)) {
> +		dev_err(dev, "Unable to get cix,usb-syscon regmap");
> +		return PTR_ERR(data->usb_syscon);
> +	}
> +	data->u3_disable = device_property_read_bool(dev, "u3-port-disable");
> +	data->ssp_disable = device_property_read_bool(dev, "ssp-disable");

[Severity: Medium]
In cdnsp_sky1_probe(), can this ID lookup and property parsing cause a few
issues?

1. ret is declared as int and &ret is passed to device_property_read_u32(),
which expects u32 * and triggers a -Wpointer-sign warning.

2. If ret >= SKY1_USB_S5_NUM, cdnsp_sky1_probe() returns a positive value to
really_probe() instead of a negative errno such as -EINVAL.

3. When CONFIG_OF is disabled (under COMPILE_TEST), of_alias_get_id() returns
-ENOSYS rather than -ENODEV, skipping the device_property_read_u32() fallback.

4. Documentation/devicetree/bindings/usb/cix,sky1-usbssp.yaml sets
unevaluatedProperties: false and does not define id, u3-port-disable,
ssp-disable, or oc-gpios, nor does its example include a usb alias, so probing
the DT binding example will fail with -ENODEV.

[ ... ]

> +static void cdnsp_sky1_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
> +
> +	pm_runtime_get_sync(dev);
> +	of_platform_depopulate(dev);
> +	sky1_put_addr(data->xhci_base);
> +	sky1_put_addr(data->device_base);
> +	reset_control_assert(data->reset);
> +	reset_control_assert(data->preset);
> +	cdnsp_sky1_clk_disable_all(dev);
> +	pm_runtime_disable(dev);
> +	pm_runtime_set_suspended(dev);
> +	pm_runtime_put_noidle(dev);
> +}
> +
> +/*
> + * Because the wake-up interrupt and host interrupt are the same interrupt,
> + * closing the axi and sof clock will result in the inability to generate
> + * port status change interrupt.
> + */
> +static int cdnsp_sky1_system_suspend(struct device *dev)
> +{
> +	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
> +
> +	if (!data->wakeup) {
> +		reset_control_assert(data->reset);
> +		reset_control_assert(data->preset);
> +	}
> +	cdnsp_sky1_clk_disable_suspend(dev);
> +	return 0;
> +}

[Severity: Medium]
The comment above cdnsp_sky1_system_suspend() notes that closing the axi and
sof clocks prevents generating the port status change wakeup interrupt.
However, cdnsp_sky1_clk_disable_suspend(dev) (which disables the sof and axi
clocks) is called unconditionally outside the if (!data->wakeup) block. Should
cdnsp_sky1_clk_disable_suspend(dev) and the corresponding
cdnsp_sky1_clk_enable_resume(dev) in cdnsp_sky1_system_resume() also be
guarded by !data->wakeup, or should different clocks be gated when wakeup is
enabled?

> +
> +static int cdnsp_sky1_system_resume(struct device *dev)
> +{
> +	int ret = 0;
> +	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
> +
> +	ret = cdnsp_sky1_clk_enable_resume(dev);
> +	if (ret)
> +		return ret;
> +	if (!data->wakeup) {
> +		writel(CIX_USB_AXI_WR_CACHE_VALUE, data->rcsu_axi_base);
> +		cdnsp_sky1_set_mode_by_id(data->dev, MODE_STRAP_OTG);
> +		reset_control_deassert(data->preset);
> +		cdnsp_sky1_configure_controller(data);
> +		reset_control_deassert(data->reset);
> +	}
> +	return 0;
> +}

[Severity: High]
When !data->wakeup during system sleep, cdnsp_sky1_system_suspend() asserts
data->reset and data->preset, which resets all DRD/OTG, xHCI, and gadget
controller registers. On resume, cdnsp_sky1_system_resume() deasserts the
resets and runs cdnsp_sky1_configure_controller(), leaving the DRD block in
its post-reset state with host mode (OTGCMD_HOST_BUS_REQ) disabled.

When the child device then resumes in cdns3_plat_resume() ->
cdns3_controller_resume(), it first calls cdns_sky1_platform_suspend(dev,
false, false), which accesses hcd->regs + XECP_PM_PMCSR before
cdns_drd_host_on() has re-enabled the xHCI block, and then calls
cdns_resume() under spin_lock_irqsave(&cdns->lock, flags) (where
cdns_power_is_lost() for CDNSP_CONTROLLER_V2 either triggers sleeping calls
under the spinlock or skips reinitializing the DRD/host registers). How is the
child cdns3 controller expected to recover its DRD and host state after a full
reset in cdnsp_sky1_system_suspend()?

[Severity: Medium]
Also in cdnsp_sky1_system_resume(), if cdnsp_sky1_clk_enable_resume(dev) fails
to re-enable the first CIX_USB_CLK_OFF_NUM clocks (sof and axi), those two
clocks remain disabled while lpm and apb remain enabled. When
cdnsp_sky1_remove() or cdnsp_sky1_shutdown() later calls
cdnsp_sky1_clk_disable_all(dev), won't it call clk_disable_unprepare() on all
four clocks and underflow the enable/prepare counts for sof and axi?

[ ... ]

> +static void cdnsp_sky1_shutdown(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cdnsp_sky1 *data = dev_get_drvdata(dev);
> +	struct device *cdns_dev = NULL;
> +	struct device *gadget_dev = NULL;
> +	struct cdns *cdns = NULL;
> +
> +	if (!device_may_wakeup(dev)) {
> +		/*
> +		 * Find the cdns3 child device, then find its gadget device
> +		 * and release the function driver before disabling clocks.
> +		 * This ensures all register accesses in gadget_unbind_driver
> +		 * complete before clocks are turned off.
> +		 */
> +		device_for_each_child(dev, &cdns_dev, cdnsp_sky1_find_cdns_dev);
> +		if (cdns_dev)
> +			cdns = dev_get_drvdata(cdns_dev);
> +		if (cdns)
> +			device_for_each_child(cdns_dev, &gadget_dev,
> +					      cdnsp_sky1_find_gadget_match);
> +		if (gadget_dev)
> +			device_release_driver(gadget_dev);
> +		if (cdns && cdns->host_dev) {
> +			struct usb_hcd *hcd = platform_get_drvdata(cdns->host_dev);
> +
> +			if (hcd && hcd->irq > 0) {
> +				/*
> +				 * Clear HCD_FLAG_HW_ACCESSIBLE before disable_irq.
> +				 * This prevents usb_hcd_irq from calling xhci_irq
> +				 * (which reads USBSTS) after clocks are disabled.
> +				 */
> +				clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> +				disable_irq(hcd->irq);
> +				synchronize_irq(hcd->irq);
> +			}
> +		}
> +		reset_control_assert(data->reset);
> +		reset_control_assert(data->preset);
> +		cdnsp_sky1_clk_disable_all(dev);
> +		put_device(gadget_dev);
> +		put_device(cdns_dev);
> +	}
> +}

[Severity: High]
Can cdnsp_sky1_shutdown() cause unclocked MMIO accesses or race with role
switching during system shutdown?

1. device_shutdown() does not runtime-resume devices before invoking
->shutdown(). If the controller is runtime-suspended when
device_release_driver(gadget_dev) is called, gadget_unbind_driver() ->
cdnsp_gadget_udc_stop() will access controller registers while the device is
in low-power state.

2. cdns->host_dev is accessed without holding cdns->mutex, which can race with
a concurrent role switch running cdns_host_exit() ->
platform_device_unregister(cdns->host_dev).

3. Releasing gadget_dev only unbinds the gadget function driver; cdns->otg_irq
and cdns->dev_irq remain enabled (and when cdns->role == USB_ROLE_DEVICE,
cdns->host_dev is NULL so the controller interrupt is not disabled via
hcd->irq either). After reset_control_assert() and
cdnsp_sky1_clk_disable_all(dev) turn off the APB and AXI clocks, any shared or
pending OTG or gadget interrupt will call cdns_drd_irq() or
cdnsp_irq_handler() and read MMIO registers while the controller is unclocked
and held in reset.

Would it be safer to runtime-resume and depopulate the child device (as in
cdnsp_sky1_remove()) so cdns3 tears down host, gadget, and OTG interrupts
cleanly before asserting resets and disabling clocks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103074929.1971195-1-hongliang.yang@cixtech.com?part=2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v5 2/3] usb: cdns3: Add CIX Sky1 glue driver
  2026-11-03  7:49 ` [PATCH v5 2/3] usb: cdns3: Add CIX Sky1 glue driver Hongliang Yang (杨宏亮)
  2026-11-03  8:15   ` sashiko-bot
@ 2026-11-03  8:43   ` Philipp Zabel
  1 sibling, 0 replies; 8+ messages in thread
From: Philipp Zabel @ 2026-11-03  8:43 UTC (permalink / raw)
  To: Hongliang Yang (杨宏亮), Greg Kroah-Hartman,
	Peter Chen, Pawel Laszczak, Roger Quadros, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley,
	Gary Yang (杨新伟),
	Fugang Duan (段富刚)
  Cc: linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	cix-kernel-upstream

On Di, 2026-11-03 at 07:49 +0000, Hongliang Yang (杨宏亮) wrote:
> Add a glue layer driver for the Cadence USBSSP controller integrated
> in the CIX Sky1 SoC. The driver manages the controller clocks,
> resets, and the mode strap controls located in the Sky1 S5 system
> controller. The per-controller glue registers, the AXI bus attribute
> configuration register and the controller status register, are mapped
> directly through the reg property of the glue node.
> 
> Signed-off-by: Hongliang Yang <hongliang.yang@cixtech.com>
> ---
> v4 -> v5:
> - No changes.
>  drivers/usb/cdns3/Kconfig      |  13 +
>  drivers/usb/cdns3/Makefile     |   1 +
>  drivers/usb/cdns3/cdnsp-sky1.c | 716 +++++++++++++++++++++++++++++++++
>  drivers/usb/cdns3/cdnsp-sky1.h | 128 ++++++
>  4 files changed, 858 insertions(+)
>  create mode 100644 drivers/usb/cdns3/cdnsp-sky1.c
>  create mode 100644 drivers/usb/cdns3/cdnsp-sky1.h
> 
[...]
> diff --git a/drivers/usb/cdns3/cdnsp-sky1.c b/drivers/usb/cdns3/cdnsp-sky1.c
> new file mode 100644
> index 000000000000..1f7870289a35
> --- /dev/null
> +++ b/drivers/usb/cdns3/cdnsp-sky1.c
> @@ -0,0 +1,716 @@
[...]
> +static int cdnsp_sky1_probe(struct platform_device *pdev)
> +{
[...]
> +       data->reset = devm_reset_control_get_exclusive(&pdev->dev, "core");
> +       if (IS_ERR(data->reset)) {
> +               ret = PTR_ERR(data->reset);
> +               dev_err(dev, "get reset error:%d\n", ret);
> +               return ret;

		return dev_err_probe(dev, PTR_ERR(data->reset),
"...");

> +       }
> +       data->preset = devm_reset_control_get_exclusive(&pdev->dev, "apb");
> +       if (IS_ERR(data->preset)) {
> +               ret = PTR_ERR(data->preset);
> +               dev_err(dev, "get reset error:%d\n", ret);
> +               return ret;

Same as above.

regards
Philipp

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-11-03  8:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03  7:49 [PATCH v5 0/3] usb: cdns3: Add CIX Sky1 USBSSP controller support Hongliang Yang (杨宏亮)
2026-11-03  7:49 ` [PATCH v5 1/3] dt-bindings: usb: Add CIX Sky1 USBSSP controller Hongliang Yang (杨宏亮)
2026-11-03  8:01   ` sashiko-bot
2026-11-03  7:49 ` [PATCH v5 3/3] arm64: dts: cix: Add USB4/USB5 nodes for Sky1 Hongliang Yang (杨宏亮)
2026-11-03  8:06   ` sashiko-bot
2026-11-03  7:49 ` [PATCH v5 2/3] usb: cdns3: Add CIX Sky1 glue driver Hongliang Yang (杨宏亮)
2026-11-03  8:15   ` sashiko-bot
2026-11-03  8:43   ` Philipp Zabel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox