[PATCH 0/2] usb: cdns3: USBSSP platform driver support

[PATCH 0/2] usb: cdns3: USBSSP platform driver support

[Peter Chen]

Hi Pawel,

The Cadence USBSSP (CDNSP) controller was previously only accessible
through PCI, coupling the gadget driver with the PCI glue layer into a
single monolithic module (cdnsp-udc-pci). This prevented using the
CDNSP IP on SoC/platform designs that expose the controller through
device tree, eg, CIX Sky1 SoC.

In this series, it restructures the driver to decouple the CDNSP gadget
from PCI, and refactor cdnsp-pci.c into a thin PCI-to-platform wrapper (similar
to cdns3-pci-wrap.c) that registers a platform device and passes
PCI resources and platform data to the common platform driver. So, please
help test it in your platform.

The changes are tested with random configuration combination tests.

============================================================
ALL TESTS COMPLETE. Summary:
============================================================
Starting cdns3 config combination tests...
Timestamp: 2026年 03月 02日 星期一 09:34:47 CST

PASS: all-builtin (SUPPORT=y CDNS3=y GADGET=y HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
PASS: support-y_cdns3-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
PASS: support-y_cdns3-m_gadget-n (SUPPORT=y CDNS3=m GADGET=n HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
PASS: support-y_cdnsp-m_gadget-y (SUPPORT=y CDNS3=y GADGET=y HOST=y CDNSP=m CDNSP_G=y CDNSP_H=y)
PASS: all-module (SUPPORT=m CDNS3=m GADGET=y HOST=y CDNSP=m CDNSP_G=y CDNSP_H=y)
PASS: no-cdns3 (SUPPORT=y CDNS3=n GADGET=n HOST=n CDNSP=y CDNSP_G=y CDNSP_H=y)
PASS: no-gadget (SUPPORT=y CDNS3=y GADGET=n HOST=y CDNSP=y CDNSP_G=n CDNSP_H=y)
PASS: support-y_both-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y HOST=y CDNSP=m CDNSP_G=y CDNSP_H=y)
PASS: minimal-module (SUPPORT=m CDNS3=m GADGET=n HOST=n CDNSP=m CDNSP_G=n CDNSP_H=n)

This patch was developed with assistance from Anthropic Claude Opus 4.6.

Peter Chen (2):
  usb: cdns3: Add USBSSP platform driver support
  dt-bindings: usb: cdns,usb3: Add support for USBSSP

 .../devicetree/bindings/usb/cdns,usb3.yaml    |  36 ++-
 drivers/usb/cdns3/Kconfig                     |  44 ++--
 drivers/usb/cdns3/Makefile                    |  30 +--
 drivers/usb/cdns3/cdns3-gadget.c              |   4 +
 drivers/usb/cdns3/cdns3-plat.c                |  14 +-
 drivers/usb/cdns3/cdnsp-gadget.c              |   4 +
 drivers/usb/cdns3/cdnsp-pci.c                 | 221 ++++++++----------
 drivers/usb/cdns3/core.h                      |   1 +
 drivers/usb/cdns3/gadget-export.h             |   4 +-
 9 files changed, 184 insertions(+), 174 deletions(-)

-- 
2.50.1

[PATCH 1/2] usb: cdns3: Add USBSSP platform driver support

[Peter Chen]

The Cadence USBSSP (CDNSP) controller was previously only accessible
through PCI, coupling the gadget driver with the PCI glue layer into a
single monolithic module (cdnsp-udc-pci). This prevented using the
CDNSP IP on SoC/platform designs that expose the controller through
device tree.

Restructure the driver to decouple the CDNSP gadget from PCI:

- Introduce CONFIG_USB_CDNSP as a standalone tristate (analogous to
  CONFIG_USB_CDNS3), with USB_CDNSP_GADGET and USB_CDNSP_HOST as
  bool sub-options. The gadget code builds as a separate cdnsp.ko
  module.

- Refactor cdnsp-pci.c into a thin PCI-to-platform wrapper (similar
  to cdns3-pci-wrap.c) that registers a platform device and passes
  PCI resources and platform data to the common platform driver.

- Add "cdns,usbssp" compatible string to the shared platform driver
  (cdns3-plat.c). The probe function uses of_device_id match data to
  select cdnsp_gadget_init vs cdns3_gadget_init.

- Move cdns3-plat.o into cdns-usb-common, since it now serves as the
  shared platform driver entry point for both CDNS3 and CDNSP.

- Fix gadget-export.h to use IS_REACHABLE() keyed on the tristate
  module config (CONFIG_USB_CDNS3/CONFIG_USB_CDNSP) instead of
  IS_ENABLED() on the bool gadget config. The bool configs are always
  'y' when enabled, causing IS_ENABLED/IS_REACHABLE to always return
  true and resulting in link errors when cdns-usb-common is built-in
  but the gadget module is loadable.

- Add missing MODULE_LICENSE()/MODULE_DESCRIPTION() and
  EXPORT_SYMBOL_GPL() to the cdns3 and cdnsp gadget modules, required
  by modpost.

- Pass override_apb_timeout through cdns3_platform_data so the PCI
  wrapper can communicate PCI-specific APB timeout values to the
  common driver.

This patch was developed with assistance from Anthropic Claude Opus 4.6.

Signed-off-by: Peter Chen <peter.chen@cixtech.com>
---
 drivers/usb/cdns3/Kconfig         |  44 +++---
 drivers/usb/cdns3/Makefile        |  30 ++--
 drivers/usb/cdns3/cdns3-gadget.c  |   4 +
 drivers/usb/cdns3/cdns3-plat.c    |  14 +-
 drivers/usb/cdns3/cdnsp-gadget.c  |   4 +
 drivers/usb/cdns3/cdnsp-pci.c     | 221 +++++++++++++-----------------
 drivers/usb/cdns3/core.h          |   1 +
 drivers/usb/cdns3/gadget-export.h |   4 +-
 8 files changed, 151 insertions(+), 171 deletions(-)

diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
index 0a514b591527..80c740f5c755 100644
--- a/drivers/usb/cdns3/Kconfig
+++ b/drivers/usb/cdns3/Kconfig
@@ -20,8 +20,13 @@ config USB_CDNS3
 	  Say Y here if your system has a Cadence USB3 dual-role controller.
 	  It supports: dual-role switch, Host-only, and Peripheral-only.
 
-	  If you choose to build this driver is a dynamically linked
-	  as module, the module will be called cdns3.ko.
+config USB_CDNSP
+	tristate "Cadence USBSSP Dual-Role Controller"
+	depends on USB_CDNS_SUPPORT
+	help
+	  Say Y here if your system has a Cadence USBSSP dual-role controller.
+	  It supports: dual-role switch, Host-only, and Peripheral-only.
+
 endif
 
 if USB_CDNS3
@@ -91,27 +96,14 @@ config USB_CDNS3_STARFIVE
 	  be dynamically linked and module will be called cdns3-starfive.ko
 endif
 
-if USB_CDNS_SUPPORT
-
-config USB_CDNSP_PCI
-	tristate "Cadence CDNSP Dual-Role Controller"
-	depends on USB_CDNS_SUPPORT && USB_PCI && ACPI
-	help
-	  Say Y here if your system has a Cadence CDNSP dual-role controller.
-	  It supports: dual-role switch Host-only, and Peripheral-only.
-
-	  If you choose to build this driver is a dynamically linked
-	  module, the module will be called cdnsp.ko.
-endif
-
-if USB_CDNSP_PCI
+if USB_CDNSP
 
 config USB_CDNSP_GADGET
-	bool "Cadence CDNSP device controller"
-	depends on USB_GADGET=y || USB_GADGET=USB_CDNSP_PCI
+	bool "Cadence USBSSP device controller"
+	depends on USB_GADGET=y || USB_GADGET=USB_CDNSP
 	help
 	  Say Y here to enable device controller functionality of the
-	  Cadence CDNSP-DEV driver.
+	  Cadence USBSSP-DEV driver.
 
 	  Cadence CDNSP Device Controller in device mode is
 	  very similar to XHCI controller. Therefore some algorithms
@@ -120,8 +112,8 @@ config USB_CDNSP_GADGET
 	  It doesn't support LS.
 
 config USB_CDNSP_HOST
-	bool "Cadence CDNSP host controller"
-	depends on USB=y || USB=USB_CDNSP_PCI
+	bool "Cadence USBSSP host controller"
+	depends on USB=y || USB=USB_CDNSP
 	select USB_CDNS_HOST
 	help
 	  Say Y here to enable host controller functionality of the
@@ -130,4 +122,14 @@ config USB_CDNSP_HOST
 	  Host controller is compliant with XHCI so it uses
 	  standard XHCI driver.
 
+config USB_CDNSP_PCI
+	tristate "Cadence USBSSP support on PCIe-based platforms"
+	depends on USB_PCI && ACPI
+	help
+	  If you're using the USBSSP Core IP with a PCIe, please say
+	  'Y' or 'M' here.
+
+	  If you choose to build this driver as module it will
+	  be dynamically linked and module will be called cdnsp-pci.ko
+
 endif
diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
index 48dfae75b5aa..0e9b20e799d0 100644
--- a/drivers/usb/cdns3/Makefile
+++ b/drivers/usb/cdns3/Makefile
@@ -3,42 +3,32 @@
 CFLAGS_cdns3-trace.o				:= -I$(src)
 CFLAGS_cdnsp-trace.o				:= -I$(src)
 
-cdns-usb-common-y				:= core.o drd.o
-cdns3-y						:= cdns3-plat.o
+cdns-usb-common-y				:= core.o drd.o cdns3-plat.o
 
 ifeq ($(CONFIG_USB),m)
 obj-m						+= cdns-usb-common.o
-obj-m						+= cdns3.o
 else
 obj-$(CONFIG_USB_CDNS_SUPPORT)			+= cdns-usb-common.o
-obj-$(CONFIG_USB_CDNS3)				+= cdns3.o
 endif
 
 cdns-usb-common-$(CONFIG_USB_CDNS_HOST) 	+= host.o
-cdns3-$(CONFIG_USB_CDNS3_GADGET)		+= cdns3-gadget.o cdns3-ep0.o
 
+# For CDNS3 gadget
 ifneq ($(CONFIG_USB_CDNS3_GADGET),)
+cdns3-y						:= cdns3-gadget.o cdns3-ep0.o
 cdns3-$(CONFIG_TRACING)				+= cdns3-trace.o
+obj-$(CONFIG_USB_CDNS3)				+= cdns3.o
 endif
-
 obj-$(CONFIG_USB_CDNS3_PCI_WRAP)		+= cdns3-pci-wrap.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
 
-cdnsp-udc-pci-y					:= cdnsp-pci.o
-
-ifdef CONFIG_USB_CDNSP_PCI
-ifeq ($(CONFIG_USB),m)
-obj-m						+= cdnsp-udc-pci.o
-else
-obj-$(CONFIG_USB_CDNSP_PCI) 			+= cdnsp-udc-pci.o
-endif
-endif
-
-cdnsp-udc-pci-$(CONFIG_USB_CDNSP_GADGET)	+= cdnsp-ring.o cdnsp-gadget.o \
-						   cdnsp-mem.o cdnsp-ep0.o
-
+# For CDNSP gadget
 ifneq ($(CONFIG_USB_CDNSP_GADGET),)
-cdnsp-udc-pci-$(CONFIG_TRACING)			+= cdnsp-trace.o
+cdnsp-y						:= cdnsp-ring.o cdnsp-gadget.o \
+						   cdnsp-mem.o cdnsp-ep0.o
+cdnsp-$(CONFIG_TRACING)				+= cdnsp-trace.o
+obj-$(CONFIG_USB_CDNSP)				+= cdnsp.o
 endif
+obj-$(CONFIG_USB_CDNSP_PCI)			+= cdnsp-pci.o
diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
index d59a60a16ec7..b800bd1bedd4 100644
--- a/drivers/usb/cdns3/cdns3-gadget.c
+++ b/drivers/usb/cdns3/cdns3-gadget.c
@@ -3508,3 +3508,7 @@ int cdns3_gadget_init(struct cdns *cdns)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(cdns3_gadget_init);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cadence USBSS DRD Driver - gadget");
diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
index 735df88774e4..156404bd6a43 100644
--- a/drivers/usb/cdns3/cdns3-plat.c
+++ b/drivers/usb/cdns3/cdns3-plat.c
@@ -23,6 +23,9 @@
 #include "gadget-export.h"
 #include "drd.h"
 
+static const unsigned long cdns3_plat;
+static const unsigned long cdnsp_plat;
+
 static int set_phy_power_on(struct cdns *cdns)
 {
 	int ret;
@@ -64,6 +67,8 @@ static int cdns3_plat_probe(struct platform_device *pdev)
 
 	cdns->dev = dev;
 	cdns->pdata = dev_get_platdata(dev);
+	if (cdns->pdata && cdns->pdata->override_apb_timeout)
+		cdns->override_apb_timeout = cdns->pdata->override_apb_timeout;
 
 	platform_set_drvdata(pdev, cdns);
 
@@ -143,7 +148,10 @@ static int cdns3_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_phy_power_on;
 
-	cdns->gadget_init = cdns3_gadget_init;
+	if (device_get_match_data(dev) == &cdnsp_plat)
+		cdns->gadget_init = cdnsp_gadget_init;
+	else
+		cdns->gadget_init = cdns3_gadget_init;
 
 	ret = cdns_init(cdns);
 	if (ret)
@@ -317,7 +325,8 @@ static const struct dev_pm_ops cdns3_pm_ops = {
 
 #ifdef CONFIG_OF
 static const struct of_device_id of_cdns3_match[] = {
-	{ .compatible = "cdns,usb3" },
+	{ .compatible = "cdns,usb3", .data = &cdns3_plat },
+	{ .compatible = "cdns,usbssp", .data = &cdnsp_plat },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, of_cdns3_match);
@@ -336,6 +345,7 @@ static struct platform_driver cdns3_driver = {
 module_platform_driver(cdns3_driver);
 
 MODULE_ALIAS("platform:cdns3");
+MODULE_ALIAS("platform:cdnsp");
 MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Cadence USB3 DRD Controller Driver");
diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
index 6b3815f8a6e5..8db7eee528a1 100644
--- a/drivers/usb/cdns3/cdnsp-gadget.c
+++ b/drivers/usb/cdns3/cdnsp-gadget.c
@@ -2075,3 +2075,7 @@ int cdnsp_gadget_init(struct cdns *cdns)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(cdnsp_gadget_init);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cadence CDNSP DRD Driver - gadget");
diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
index 566d94e49102..7deb25816548 100644
--- a/drivers/usb/cdns3/cdnsp-pci.c
+++ b/drivers/usb/cdns3/cdnsp-pci.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Cadence PCI Glue driver.
+ * Cadence USBSSP PCI Glue driver.
  *
  * Copyright (C) 2019 Cadence.
  *
@@ -8,15 +8,27 @@
  *
  */
 
-#include <linux/platform_device.h>
-#include <linux/dma-mapping.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/slab.h>
 #include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/slab.h>
 
 #include "core.h"
-#include "gadget-export.h"
+
+struct cdnsp_wrap {
+	struct platform_device *plat_dev;
+	struct resource dev_res[6];
+	int devfn;
+};
+
+#define RES_IRQ_HOST_ID		0
+#define RES_IRQ_PERIPHERAL_ID	1
+#define RES_IRQ_OTG_ID		2
+#define RES_HOST_ID		3
+#define RES_DEV_ID		4
+#define RES_DRD_ID		5
 
 #define PCI_BAR_HOST		0
 #define PCI_BAR_OTG		0
@@ -28,14 +40,14 @@
 #define PCI_DRIVER_NAME		"cdns-pci-usbssp"
 #define PLAT_DRIVER_NAME	"cdns-usbssp"
 
-#define CHICKEN_APB_TIMEOUT_VALUE       0x1C20
+#define CHICKEN_APB_TIMEOUT_VALUE	0x1C20
 
 static struct pci_dev *cdnsp_get_second_fun(struct pci_dev *pdev)
 {
 	/*
 	 * Gets the second function.
-	 * Platform has two function. The fist keeps resources for
-	 * Host/Device while the secon keeps resources for DRD/OTG.
+	 * Platform has two function. The first keeps resources for
+	 * Host/Device while the second keeps resources for DRD/OTG.
 	 */
 	if (pdev->device == PCI_DEVICE_ID_CDNS_USBSSP)
 		return pci_get_device(pdev->vendor, PCI_DEVICE_ID_CDNS_USBSS, NULL);
@@ -48,11 +60,12 @@ static struct pci_dev *cdnsp_get_second_fun(struct pci_dev *pdev)
 static int cdnsp_pci_probe(struct pci_dev *pdev,
 			   const struct pci_device_id *id)
 {
-	struct device *dev = &pdev->dev;
-	struct pci_dev *func;
+	struct platform_device_info plat_info;
+	static struct cdns3_platform_data pdata;
+	struct cdnsp_wrap *wrap;
 	struct resource *res;
-	struct cdns *cdnsp;
-	int ret;
+	struct pci_dev *func;
+	int ret = 0;
 
 	/*
 	 * For GADGET/HOST PCI (devfn) function number is 0,
@@ -79,146 +92,105 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
 	}
 
 	pci_set_master(pdev);
+
 	if (pci_is_enabled(func)) {
-		cdnsp = pci_get_drvdata(func);
+		wrap = pci_get_drvdata(func);
 	} else {
-		cdnsp = kzalloc_obj(*cdnsp);
-		if (!cdnsp) {
+		wrap = kzalloc_obj(*wrap);
+		if (!wrap) {
 			ret = -ENOMEM;
 			goto put_pci;
 		}
 	}
 
-	/* For GADGET device function number is 0. */
-	if (pdev->devfn == 0) {
-		resource_size_t rsrc_start, rsrc_len;
-
-		/* Function 0: host(BAR_0) + device(BAR_1).*/
-		dev_dbg(dev, "Initialize resources\n");
-		rsrc_start = pci_resource_start(pdev, PCI_BAR_DEV);
-		rsrc_len = pci_resource_len(pdev, PCI_BAR_DEV);
-		res = devm_request_mem_region(dev, rsrc_start, rsrc_len, "dev");
-		if (!res) {
-			dev_dbg(dev, "controller already in use\n");
-			ret = -EBUSY;
-			goto free_cdnsp;
-		}
-
-		cdnsp->dev_regs = devm_ioremap(dev, rsrc_start, rsrc_len);
-		if (!cdnsp->dev_regs) {
-			dev_dbg(dev, "error mapping memory\n");
-			ret = -EFAULT;
-			goto free_cdnsp;
-		}
-
-		cdnsp->dev_irq = pdev->irq;
-		dev_dbg(dev, "USBSS-DEV physical base addr: %pa\n",
-			&rsrc_start);
-
-		res = &cdnsp->xhci_res[0];
-		res->start = pci_resource_start(pdev, PCI_BAR_HOST);
-		res->end = pci_resource_end(pdev, PCI_BAR_HOST);
-		res->name = "xhci";
-		res->flags = IORESOURCE_MEM;
-		dev_dbg(dev, "USBSS-XHCI physical base addr: %pa\n",
-			&res->start);
-
-		/* Interrupt for XHCI, */
-		res = &cdnsp->xhci_res[1];
-		res->start = pdev->irq;
-		res->name = "host";
-		res->flags = IORESOURCE_IRQ;
+	res = wrap->dev_res;
+
+	if (pdev->devfn == PCI_DEV_FN_HOST_DEVICE) {
+		/* Function 0: host(BAR_0) + device(BAR_2). */
+		dev_dbg(&pdev->dev, "Initialize Device resources\n");
+		res[RES_DEV_ID].start = pci_resource_start(pdev, PCI_BAR_DEV);
+		res[RES_DEV_ID].end = pci_resource_end(pdev, PCI_BAR_DEV);
+		res[RES_DEV_ID].name = "dev";
+		res[RES_DEV_ID].flags = IORESOURCE_MEM;
+		dev_dbg(&pdev->dev, "USBSSP-DEV physical base addr: %pa\n",
+			&res[RES_DEV_ID].start);
+
+		res[RES_HOST_ID].start = pci_resource_start(pdev, PCI_BAR_HOST);
+		res[RES_HOST_ID].end = pci_resource_end(pdev, PCI_BAR_HOST);
+		res[RES_HOST_ID].name = "xhci";
+		res[RES_HOST_ID].flags = IORESOURCE_MEM;
+		dev_dbg(&pdev->dev, "USBSSP-XHCI physical base addr: %pa\n",
+			&res[RES_HOST_ID].start);
+
+		/* Interrupt for XHCI */
+		wrap->dev_res[RES_IRQ_HOST_ID].start = pdev->irq;
+		wrap->dev_res[RES_IRQ_HOST_ID].name = "host";
+		wrap->dev_res[RES_IRQ_HOST_ID].flags = IORESOURCE_IRQ;
+
+		/* Interrupt for device. It's the same as for HOST. */
+		wrap->dev_res[RES_IRQ_PERIPHERAL_ID].start = pdev->irq;
+		wrap->dev_res[RES_IRQ_PERIPHERAL_ID].name = "peripheral";
+		wrap->dev_res[RES_IRQ_PERIPHERAL_ID].flags = IORESOURCE_IRQ;
 	} else {
-		res = &cdnsp->otg_res;
-		res->start = pci_resource_start(pdev, PCI_BAR_OTG);
-		res->end =   pci_resource_end(pdev, PCI_BAR_OTG);
-		res->name = "otg";
-		res->flags = IORESOURCE_MEM;
-		dev_dbg(dev, "CDNSP-DRD physical base addr: %pa\n",
-			&res->start);
+		res[RES_DRD_ID].start = pci_resource_start(pdev, PCI_BAR_OTG);
+		res[RES_DRD_ID].end = pci_resource_end(pdev, PCI_BAR_OTG);
+		res[RES_DRD_ID].name = "otg";
+		res[RES_DRD_ID].flags = IORESOURCE_MEM;
+		dev_dbg(&pdev->dev, "CDNSP-DRD physical base addr: %pa\n",
+			&res[RES_DRD_ID].start);
 
 		/* Interrupt for OTG/DRD. */
-		cdnsp->otg_irq = pdev->irq;
+		wrap->dev_res[RES_IRQ_OTG_ID].start = pdev->irq;
+		wrap->dev_res[RES_IRQ_OTG_ID].name = "otg";
+		wrap->dev_res[RES_IRQ_OTG_ID].flags = IORESOURCE_IRQ;
 	}
 
-	/*
-	 * Cadence PCI based platform require some longer timeout for APB
-	 * to fixes domain clock synchronization issue after resuming
-	 * controller from L1 state.
-	 */
-	cdnsp->override_apb_timeout = CHICKEN_APB_TIMEOUT_VALUE;
-	pci_set_drvdata(pdev, cdnsp);
-
 	if (pci_is_enabled(func)) {
-		cdnsp->dev = dev;
-		cdnsp->gadget_init = cdnsp_gadget_init;
-
-		ret = cdns_init(cdnsp);
-		if (ret)
-			goto free_cdnsp;
+		/* set up platform device info */
+		pdata.override_apb_timeout = CHICKEN_APB_TIMEOUT_VALUE;
+		memset(&plat_info, 0, sizeof(plat_info));
+		plat_info.parent = &pdev->dev;
+		plat_info.fwnode = pdev->dev.fwnode;
+		plat_info.name = PLAT_DRIVER_NAME;
+		plat_info.id = pdev->devfn;
+		plat_info.res = wrap->dev_res;
+		plat_info.num_res = ARRAY_SIZE(wrap->dev_res);
+		plat_info.dma_mask = pdev->dma_mask;
+		plat_info.data = &pdata;
+		plat_info.size_data = sizeof(pdata);
+		wrap->devfn = pdev->devfn;
+		/* register platform device */
+		wrap->plat_dev = platform_device_register_full(&plat_info);
+		if (IS_ERR(wrap->plat_dev)) {
+			ret = PTR_ERR(wrap->plat_dev);
+			kfree(wrap);
+			goto put_pci;
+		}
 	}
 
-	device_wakeup_enable(&pdev->dev);
-	if (pci_dev_run_wake(pdev))
-		pm_runtime_put_noidle(&pdev->dev);
-
-	return 0;
-
-free_cdnsp:
-	if (!pci_is_enabled(func))
-		kfree(cdnsp);
-
+	pci_set_drvdata(pdev, wrap);
 put_pci:
 	pci_dev_put(func);
-
 	return ret;
 }
 
 static void cdnsp_pci_remove(struct pci_dev *pdev)
 {
-	struct cdns *cdnsp;
+	struct cdnsp_wrap *wrap;
 	struct pci_dev *func;
 
 	func = cdnsp_get_second_fun(pdev);
-	cdnsp = (struct cdns *)pci_get_drvdata(pdev);
+	wrap = pci_get_drvdata(pdev);
 
-	if (pci_dev_run_wake(pdev))
-		pm_runtime_get_noresume(&pdev->dev);
+	if (wrap->devfn == pdev->devfn)
+		platform_device_unregister(wrap->plat_dev);
 
-	if (pci_is_enabled(func)) {
-		cdns_remove(cdnsp);
-	} else {
-		kfree(cdnsp);
-	}
+	if (!pci_is_enabled(func))
+		kfree(wrap);
 
 	pci_dev_put(func);
 }
 
-static int __maybe_unused cdnsp_pci_suspend(struct device *dev)
-{
-	struct cdns *cdns = dev_get_drvdata(dev);
-
-	return cdns_suspend(cdns);
-}
-
-static int __maybe_unused cdnsp_pci_resume(struct device *dev)
-{
-	struct cdns *cdns = dev_get_drvdata(dev);
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&cdns->lock, flags);
-	ret = cdns_resume(cdns);
-	spin_unlock_irqrestore(&cdns->lock, flags);
-	cdns_set_active(cdns, 1);
-
-	return ret;
-}
-
-static const struct dev_pm_ops cdnsp_pci_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(cdnsp_pci_suspend, cdnsp_pci_resume)
-};
-
 static const struct pci_device_id cdnsp_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_USBSSP),
 	  .class = PCI_CLASS_SERIAL_USB_DEVICE },
@@ -230,13 +202,10 @@ static const struct pci_device_id cdnsp_pci_ids[] = {
 };
 
 static struct pci_driver cdnsp_pci_driver = {
-	.name = "cdnsp-pci",
+	.name = PCI_DRIVER_NAME,
 	.id_table = cdnsp_pci_ids,
 	.probe = cdnsp_pci_probe,
 	.remove = cdnsp_pci_remove,
-	.driver = {
-		.pm = &cdnsp_pci_pm_ops,
-	}
 };
 
 module_pci_driver(cdnsp_pci_driver);
@@ -245,4 +214,4 @@ MODULE_DEVICE_TABLE(pci, cdnsp_pci_ids);
 MODULE_ALIAS("pci:cdnsp");
 MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>");
 MODULE_LICENSE("GPL v2");
-MODULE_DESCRIPTION("Cadence CDNSP PCI driver");
+MODULE_DESCRIPTION("Cadence CDNSP PCI wrapper");
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
index 801be9e61340..9149612bb2a8 100644
--- a/drivers/usb/cdns3/core.h
+++ b/drivers/usb/cdns3/core.h
@@ -45,6 +45,7 @@ struct cdns3_platform_data {
 	unsigned long quirks;
 #define CDNS3_DEFAULT_PM_RUNTIME_ALLOW	BIT(0)
 #define CDNS3_DRD_SUSPEND_RESIDENCY_ENABLE	BIT(1)
+	u32 override_apb_timeout;	/* 0 = use default (e.g. for PCI) */
 };
 
 /**
diff --git a/drivers/usb/cdns3/gadget-export.h b/drivers/usb/cdns3/gadget-export.h
index c37b6269b001..0cb600e2b5d2 100644
--- a/drivers/usb/cdns3/gadget-export.h
+++ b/drivers/usb/cdns3/gadget-export.h
@@ -10,7 +10,7 @@
 #ifndef __LINUX_CDNS3_GADGET_EXPORT
 #define __LINUX_CDNS3_GADGET_EXPORT
 
-#if IS_ENABLED(CONFIG_USB_CDNSP_GADGET)
+#if defined(CONFIG_USB_CDNSP_GADGET) && IS_REACHABLE(CONFIG_USB_CDNSP)
 
 int cdnsp_gadget_init(struct cdns *cdns);
 #else
@@ -22,7 +22,7 @@ static inline int cdnsp_gadget_init(struct cdns *cdns)
 
 #endif /* CONFIG_USB_CDNSP_GADGET */
 
-#if IS_ENABLED(CONFIG_USB_CDNS3_GADGET)
+#if defined(CONFIG_USB_CDNS3_GADGET) && IS_REACHABLE(CONFIG_USB_CDNS3)
 
 int cdns3_gadget_init(struct cdns *cdns);
 #else
-- 
2.50.1

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

[Peter Chen]

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.

Signed-off-by: Peter Chen <peter.chen@cixtech.com>
---
 .../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@cadence.com>
 
+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.
+
 properties:
   compatible:
-    const: cdns,usb3
+    enum:
+      - cdns,usb3
+      - cdns,usbssp
 
   reg:
     items:
@@ -49,7 +57,7 @@ properties:
       cdns3 to type C connector.
 
   maximum-speed:
-    enum: [super-speed, high-speed, full-speed]
+    enum: [super-speed-plus, super-speed, high-speed, full-speed]
 
   phys:
     minItems: 1
@@ -90,6 +98,7 @@ unevaluatedProperties: false
 
 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 {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        usb@a0000000 {
+            compatible = "cdns,usbssp";
+            reg = <0x00 0xa0000000 0x00 0x10000>,
+                  <0x00 0xa0010000 0x00 0x10000>,
+                  <0x00 0xa0020000 0x00 0x10000>;
+            reg-names = "otg", "xhci", "dev";
+            interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "host", "peripheral", "otg";
+            maximum-speed = "super-speed-plus";
+            dr_mode = "otg";
+        };
+    };
-- 
2.50.1

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

[Krzysztof Kozlowski]

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.

> 
> Signed-off-by: Peter Chen <peter.chen@cixtech.com>
> ---
>  .../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@cadence.com>
>  
> +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?

> +
>  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?


>  
>    reg:
>      items:
> @@ -49,7 +57,7 @@ properties:
>        cdns3 to type C connector.
>  
>    maximum-speed:
> -    enum: [super-speed, high-speed, full-speed]
> +    enum: [super-speed-plus, super-speed, high-speed, full-speed]
>  
>    phys:
>      minItems: 1
> @@ -90,6 +98,7 @@ unevaluatedProperties: false
>  
>  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.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

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.

So here you explan what you did

> 
> 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

Heh, identical but completely incompatible. If you are using Claude then
at least read its feedback - what does it say when devices are 100%
compatible?

> and the same platform driver (cdns3-plat.c).
> 
> Changes to the binding:

And here. How many times more?

> - compatible: const -> enum with cdns,usb3 and cdns,usbssp
> - maximum-speed: add super-speed-plus
> - Add USBSSP example

Pointless. Explain WHY are you doing this, why do we even want this
generic compatible, not paste here Claude microslop output.

Best regards,
Krzysztof

Re: [PATCH 1/2] usb: cdns3: Add USBSSP platform driver support

[Krzysztof Kozlowski]

On Mon, Mar 02, 2026 at 11:03:33AM +0800, Peter Chen wrote:
> -	cdns->gadget_init = cdns3_gadget_init;
> +	if (device_get_match_data(dev) == &cdnsp_plat)
> +		cdns->gadget_init = cdnsp_gadget_init;
> +	else
> +		cdns->gadget_init = cdns3_gadget_init;
>  
>  	ret = cdns_init(cdns);
>  	if (ret)
> @@ -317,7 +325,8 @@ static const struct dev_pm_ops cdns3_pm_ops = {
>  
>  #ifdef CONFIG_OF
>  static const struct of_device_id of_cdns3_match[] = {
> -	{ .compatible = "cdns,usb3" },
> +	{ .compatible = "cdns,usb3", .data = &cdns3_plat },
> +	{ .compatible = "cdns,usbssp", .data = &cdnsp_plat },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, of_cdns3_match);
> @@ -336,6 +345,7 @@ static struct platform_driver cdns3_driver = {
>  module_platform_driver(cdns3_driver);
>  
>  MODULE_ALIAS("platform:cdns3");
> +MODULE_ALIAS("platform:cdnsp");

You do not need this at all. Fix your DTS or platforms or this
driver.

>  MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>");
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Cadence USB3 DRD Controller Driver");

Best regards,
Krzysztof

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

[Pawel Laszczak]

Hi Peter,

>
>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).
>

Include the change for the wakeup interrupt in the next version.
Unlike the legacy USBSS, the USBSSP IP handles device-mode wakeup events
natively through the xHCI-based Event Ring via TRBs, rather than using a
dedicated sideband interrupt line. This integration aligns the wakeup mechanism
with the standard xHCI event processing, making the separate "wakeup"
interrupt property unnecessary for this variant.

Thanks,
Regards Pawel

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@cixtech.com>
> > ---
> >  .../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@cadence.com>
> >
> > +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

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

[Krzysztof Kozlowski]

On 02/03/2026 10:21, Peter Chen wrote:
> 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@cixtech.com>
>>> ---
>>>  .../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@cadence.com>
>>>
>>> +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"?

> 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)

You just described drivers, so this does not convince me at all.

> 
> 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?

You did not respond to this one. Look how this schema is used.

> 
> 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:

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

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

[Peter Chen]

On 26-03-02 08:29:40, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL
> 
> 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.
> 
> So here you explan what you did
> 
> >
> > 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
> 
> Heh, identical but completely incompatible. If you are using Claude then
> at least read its feedback - what does it say when devices are 100%
> compatible?

You are right, that was a contradictory statement. I have added both
the same and differences at the last reply. I will rewrite the description
to be accurate.

> 
> > and the same platform driver (cdns3-plat.c).
> >
> > Changes to the binding:
> 
> And here. How many times more?
> 
> > - compatible: const -> enum with cdns,usb3 and cdns,usbssp
> > - maximum-speed: add super-speed-plus
> > - Add USBSSP example
> 
> Pointless. Explain WHY are you doing this, why do we even want this
> generic compatible, not paste here Claude microslop output.
> 

Fair point. Will rewrite the commit message for v2 to focus on WHY.
Thanks for the review.

-- 

Best regards,
Peter

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

[Peter Chen]

On 26-03-02 10:27:11, Krzysztof Kozlowski wrote:
> >>>  maintainers:
> >>>    - Pawel Laszczak <pawell@cadence.com>
> >>>
> >>> +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?

Peter

> 
> > 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)
> 
> You just described drivers, so this does not convince me at all.
> 
> >
> > 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?
> 
> You did not respond to this one. Look how this schema is used.
> 
> >
> > 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:
> 
> 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

-- 

Best regards,
Peter

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

[Peter Chen]

On 26-03-02 09:03:25, Pawel Laszczak wrote:
> EXTERNAL EMAIL
> 
> Hi Peter,
> 
> >
> >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).
> >
> 
> Include the change for the wakeup interrupt in the next version.
> Unlike the legacy USBSS, the USBSSP IP handles device-mode wakeup events
> natively through the xHCI-based Event Ring via TRBs, rather than using a
> dedicated sideband interrupt line. This integration aligns the wakeup mechanism
> with the standard xHCI event processing, making the separate "wakeup"
> interrupt property unnecessary for this variant.
> 

Thanks for your suggestion. This "wakeup" interrupt is not related to
controller, it is the glue layer logic that detecting dp/dm/id/vbus or
rx changes.

-- 

Best regards,
Peter

Re: [PATCH 1/2] usb: cdns3: Add USBSSP platform driver support

[Peter Chen]

On 26-03-02 08:31:57, Krzysztof Kozlowski wrote:
> On Mon, Mar 02, 2026 at 11:03:33AM +0800, Peter Chen wrote:
> > -     cdns->gadget_init = cdns3_gadget_init;
> > +     if (device_get_match_data(dev) == &cdnsp_plat)
> > +             cdns->gadget_init = cdnsp_gadget_init;
> > +     else
> > +             cdns->gadget_init = cdns3_gadget_init;
> >
> >       ret = cdns_init(cdns);
> >       if (ret)
> > @@ -317,7 +325,8 @@ static const struct dev_pm_ops cdns3_pm_ops = {
> >
> >  #ifdef CONFIG_OF
> >  static const struct of_device_id of_cdns3_match[] = {
> > -     { .compatible = "cdns,usb3" },
> > +     { .compatible = "cdns,usb3", .data = &cdns3_plat },
> > +     { .compatible = "cdns,usbssp", .data = &cdnsp_plat },
> >       { },
> >  };
> >  MODULE_DEVICE_TABLE(of, of_cdns3_match);
> > @@ -336,6 +345,7 @@ static struct platform_driver cdns3_driver = {
> >  module_platform_driver(cdns3_driver);
> >
> >  MODULE_ALIAS("platform:cdns3");
> > +MODULE_ALIAS("platform:cdnsp");
> 
> You do not need this at all. Fix your DTS or platforms or this
> driver.

Yes, will fix it at v2.

-- 

Best regards,
Peter

RE: [PATCH 0/2] usb: cdns3: USBSSP platform driver support

[Pawel Laszczak]

>
>Hi Pawel,
>
>The Cadence USBSSP (CDNSP) controller was previously only accessible
>through PCI, coupling the gadget driver with the PCI glue layer into a single
>monolithic module (cdnsp-udc-pci). This prevented using the CDNSP IP on
>SoC/platform designs that expose the controller through device tree, eg, CIX
>Sky1 SoC.
>
>In this series, it restructures the driver to decouple the CDNSP gadget from
>PCI, and refactor cdnsp-pci.c into a thin PCI-to-platform wrapper (similar to
>cdns3-pci-wrap.c) that registers a platform device and passes PCI resources
>and platform data to the common platform driver. So, please help test it in
>your platform.
>
>The changes are tested with random configuration combination tests.
>
>============================================================
>ALL TESTS COMPLETE. Summary:
>============================================================
>Starting cdns3 config combination tests...
>Timestamp: 2026年 03月 02日 星期一 09:34:47 CST
>
>PASS: all-builtin (SUPPORT=y CDNS3=y GADGET=y HOST=y CDNSP=y
>CDNSP_G=y CDNSP_H=y)
>PASS: support-y_cdns3-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y
>HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
>PASS: support-y_cdns3-m_gadget-n (SUPPORT=y CDNS3=m GADGET=n
>HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
>PASS: support-y_cdnsp-m_gadget-y (SUPPORT=y CDNS3=y GADGET=y HOST=y
>CDNSP=m CDNSP_G=y CDNSP_H=y)
>PASS: all-module (SUPPORT=m CDNS3=m GADGET=y HOST=y CDNSP=m
>CDNSP_G=y CDNSP_H=y)
>PASS: no-cdns3 (SUPPORT=y CDNS3=n GADGET=n HOST=n CDNSP=y
>CDNSP_G=y CDNSP_H=y)
>PASS: no-gadget (SUPPORT=y CDNS3=y GADGET=n HOST=y CDNSP=y
>CDNSP_G=n CDNSP_H=y)
>PASS: support-y_both-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y HOST=y
>CDNSP=m CDNSP_G=y CDNSP_H=y)
>PASS: minimal-module (SUPPORT=m CDNS3=m GADGET=n HOST=n CDNSP=m
>CDNSP_G=n CDNSP_H=n)
>
>This patch was developed with assistance from Anthropic Claude Opus 4.6.
>

I can compile the kernel but when I try to install modules with
make modules_install I get error:

DEPMOD  /lib/modules/7.0.0-rc1-new-pci-plat-support-next-20260227+
depmod: ERROR: Cycle detected: cdns_usb_common -> cdnsp -> cdns_usb_common
depmod: ERROR: Cycle detected: udc_core
depmod: ERROR: Found 2 modules in dependency cycles!
make[2]: *** [scripts/Makefile.modinst:132: depmod] Error 1

It occurs even with minimal configuration:
CONFIG_USB_CDNS_SUPPORT=m
# CONFIG_USB_CDNS3 is not set
CONFIG_USB_CDNSP=m
CONFIG_USB_CDNSP_GADGET=y

Regards,
Pawel

>Peter Chen (2):
>  usb: cdns3: Add USBSSP platform driver support
>  dt-bindings: usb: cdns,usb3: Add support for USBSSP
>
> .../devicetree/bindings/usb/cdns,usb3.yaml    |  36 ++-
> drivers/usb/cdns3/Kconfig                     |  44 ++--
> drivers/usb/cdns3/Makefile                    |  30 +--
> drivers/usb/cdns3/cdns3-gadget.c              |   4 +
> drivers/usb/cdns3/cdns3-plat.c                |  14 +-
> drivers/usb/cdns3/cdnsp-gadget.c              |   4 +
> drivers/usb/cdns3/cdnsp-pci.c                 | 221 ++++++++----------
> drivers/usb/cdns3/core.h                      |   1 +
> drivers/usb/cdns3/gadget-export.h             |   4 +-
> 9 files changed, 184 insertions(+), 174 deletions(-)
>
>--
>2.50.1

Re: [PATCH 0/2] usb: cdns3: USBSSP platform driver support

[Peter Chen]

On 26-03-03 07:34:54, Pawel Laszczak wrote:
> [Some people who received this message don't often get email from pawell@cadence.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL
> 
> >
> >Hi Pawel,
> >
> >The Cadence USBSSP (CDNSP) controller was previously only accessible
> >through PCI, coupling the gadget driver with the PCI glue layer into a single
> >monolithic module (cdnsp-udc-pci). This prevented using the CDNSP IP on
> >SoC/platform designs that expose the controller through device tree, eg, CIX
> >Sky1 SoC.
> >
> >In this series, it restructures the driver to decouple the CDNSP gadget from
> >PCI, and refactor cdnsp-pci.c into a thin PCI-to-platform wrapper (similar to
> >cdns3-pci-wrap.c) that registers a platform device and passes PCI resources
> >and platform data to the common platform driver. So, please help test it in
> >your platform.
> >
> >The changes are tested with random configuration combination tests.
> >
> >============================================================
> >ALL TESTS COMPLETE. Summary:
> >============================================================
> >Starting cdns3 config combination tests...
> >Timestamp: 2026年 03月 02日 星期一 09:34:47 CST
> >
> >PASS: all-builtin (SUPPORT=y CDNS3=y GADGET=y HOST=y CDNSP=y
> >CDNSP_G=y CDNSP_H=y)
> >PASS: support-y_cdns3-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y
> >HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
> >PASS: support-y_cdns3-m_gadget-n (SUPPORT=y CDNS3=m GADGET=n
> >HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
> >PASS: support-y_cdnsp-m_gadget-y (SUPPORT=y CDNS3=y GADGET=y HOST=y
> >CDNSP=m CDNSP_G=y CDNSP_H=y)
> >PASS: all-module (SUPPORT=m CDNS3=m GADGET=y HOST=y CDNSP=m
> >CDNSP_G=y CDNSP_H=y)
> >PASS: no-cdns3 (SUPPORT=y CDNS3=n GADGET=n HOST=n CDNSP=y
> >CDNSP_G=y CDNSP_H=y)
> >PASS: no-gadget (SUPPORT=y CDNS3=y GADGET=n HOST=y CDNSP=y
> >CDNSP_G=n CDNSP_H=y)
> >PASS: support-y_both-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y HOST=y
> >CDNSP=m CDNSP_G=y CDNSP_H=y)
> >PASS: minimal-module (SUPPORT=m CDNS3=m GADGET=n HOST=n CDNSP=m
> >CDNSP_G=n CDNSP_H=n)
> >
> >This patch was developed with assistance from Anthropic Claude Opus 4.6.
> >
> 
> I can compile the kernel but when I try to install modules with
> make modules_install I get error:
> 
> DEPMOD  /lib/modules/7.0.0-rc1-new-pci-plat-support-next-20260227+
> depmod: ERROR: Cycle detected: cdns_usb_common -> cdnsp -> cdns_usb_common
> depmod: ERROR: Cycle detected: udc_core
> depmod: ERROR: Found 2 modules in dependency cycles!
> make[2]: *** [scripts/Makefile.modinst:132: depmod] Error 1
> 
> It occurs even with minimal configuration:
> CONFIG_USB_CDNS_SUPPORT=m
> # CONFIG_USB_CDNS3 is not set
> CONFIG_USB_CDNSP=m
> CONFIG_USB_CDNSP_GADGET=y
> 

Thanks for testing. It needs to let cdns3(p) platform driver as
standalone module to fix it. Would you please try below fix on top
of my patch set:

diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
index 0e9b20e799d0..63484f145bb9 100644
--- a/drivers/usb/cdns3/Makefile
+++ b/drivers/usb/cdns3/Makefile
@@ -3,12 +3,14 @@
 CFLAGS_cdns3-trace.o				:= -I$(src)
 CFLAGS_cdnsp-trace.o				:= -I$(src)
 
-cdns-usb-common-y				:= core.o drd.o cdns3-plat.o
+cdns-usb-common-y				:= core.o drd.o
 
 ifeq ($(CONFIG_USB),m)
 obj-m						+= cdns-usb-common.o
+obj-m						+= cdns3-plat.o
 else
 obj-$(CONFIG_USB_CDNS_SUPPORT)			+= cdns-usb-common.o
+obj-$(CONFIG_USB_CDNS_SUPPORT)			+= cdns3-plat.o
 endif
 
 cdns-usb-common-$(CONFIG_USB_CDNS_HOST) 	+= host.o

Besides, would you please comment [1], the key point is could we
keep one platform driver with one compatible string, and cdns3 or cdnsp
platform is decided at runtime?

[1] https://www.spinics.net/lists/kernel/msg6072480.html

-- 

Best regards,
Peter

RE: [PATCH 0/2] usb: cdns3: USBSSP platform driver support

[Pawel Laszczak]

>
>On 26-03-03 07:34:54, Pawel Laszczak wrote:
>> [Some people who received this message don't often get email from
>pawell@cadence.com. Learn why this is important at
>https://urldefense.com/v3/__https://aka.ms/LearnAboutSenderIdentification__;!
>!EHscmS1ygiU1lA!AMXCokQopzhe7zk6kKPFeLLp-
>yuh1is6xaU_qe56iXLYV9j5bEV_jUOV23iU07J2VMvUVf89LhR60AKPHeqB$  ]
>>
>> EXTERNAL EMAIL
>>
>> >
>> >Hi Pawel,
>> >
>> >The Cadence USBSSP (CDNSP) controller was previously only accessible
>> >through PCI, coupling the gadget driver with the PCI glue layer into a single
>> >monolithic module (cdnsp-udc-pci). This prevented using the CDNSP IP on
>> >SoC/platform designs that expose the controller through device tree, eg, CIX
>> >Sky1 SoC.
>> >
>> >In this series, it restructures the driver to decouple the CDNSP gadget from
>> >PCI, and refactor cdnsp-pci.c into a thin PCI-to-platform wrapper (similar to
>> >cdns3-pci-wrap.c) that registers a platform device and passes PCI resources
>> >and platform data to the common platform driver. So, please help test it in
>> >your platform.
>> >
>> >The changes are tested with random configuration combination tests.
>> >
>> >============================================================
>> >ALL TESTS COMPLETE. Summary:
>> >============================================================
>> >Starting cdns3 config combination tests...
>> >Timestamp: 2026年 03月 02日 星期一 09:34:47 CST
>> >
>> >PASS: all-builtin (SUPPORT=y CDNS3=y GADGET=y HOST=y CDNSP=y
>> >CDNSP_G=y CDNSP_H=y)
>> >PASS: support-y_cdns3-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y
>> >HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
>> >PASS: support-y_cdns3-m_gadget-n (SUPPORT=y CDNS3=m GADGET=n
>> >HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
>> >PASS: support-y_cdnsp-m_gadget-y (SUPPORT=y CDNS3=y GADGET=y HOST=y
>> >CDNSP=m CDNSP_G=y CDNSP_H=y)
>> >PASS: all-module (SUPPORT=m CDNS3=m GADGET=y HOST=y CDNSP=m
>> >CDNSP_G=y CDNSP_H=y)
>> >PASS: no-cdns3 (SUPPORT=y CDNS3=n GADGET=n HOST=n CDNSP=y
>> >CDNSP_G=y CDNSP_H=y)
>> >PASS: no-gadget (SUPPORT=y CDNS3=y GADGET=n HOST=y CDNSP=y
>> >CDNSP_G=n CDNSP_H=y)
>> >PASS: support-y_both-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y HOST=y
>> >CDNSP=m CDNSP_G=y CDNSP_H=y)
>> >PASS: minimal-module (SUPPORT=m CDNS3=m GADGET=n HOST=n CDNSP=m
>> >CDNSP_G=n CDNSP_H=n)
>> >
>> >This patch was developed with assistance from Anthropic Claude Opus 4.6.
>> >
>>
>> I can compile the kernel but when I try to install modules with
>> make modules_install I get error:
>>
>> DEPMOD  /lib/modules/7.0.0-rc1-new-pci-plat-support-next-20260227+
>> depmod: ERROR: Cycle detected: cdns_usb_common -> cdnsp ->
>cdns_usb_common
>> depmod: ERROR: Cycle detected: udc_core
>> depmod: ERROR: Found 2 modules in dependency cycles!
>> make[2]: *** [scripts/Makefile.modinst:132: depmod] Error 1
>>
>> It occurs even with minimal configuration:
>> CONFIG_USB_CDNS_SUPPORT=m
>> # CONFIG_USB_CDNS3 is not set
>> CONFIG_USB_CDNSP=m
>> CONFIG_USB_CDNSP_GADGET=y
>>
>
>Thanks for testing. It needs to let cdns3(p) platform driver as
>standalone module to fix it. Would you please try below fix on top
>of my patch set:
>
>diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>index 0e9b20e799d0..63484f145bb9 100644
>--- a/drivers/usb/cdns3/Makefile
>+++ b/drivers/usb/cdns3/Makefile
>@@ -3,12 +3,14 @@
> CFLAGS_cdns3-trace.o				:= -I$(src)
> CFLAGS_cdnsp-trace.o				:= -I$(src)
>
>-cdns-usb-common-y				:= core.o drd.o cdns3-plat.o
>+cdns-usb-common-y				:= core.o drd.o
>
> ifeq ($(CONFIG_USB),m)
> obj-m						+= cdns-usb-common.o
>+obj-m						+= cdns3-plat.o
> else
> obj-$(CONFIG_USB_CDNS_SUPPORT)			+= cdns-usb-common.o
>+obj-$(CONFIG_USB_CDNS_SUPPORT)			+= cdns3-plat.o
> endif
>
> cdns-usb-common-$(CONFIG_USB_CDNS_HOST) 	+= host.o
>

With this fix, the modules installation works correctly.

I have an additional comment regarding the grouping of USBSSP and CDNS3
options. Currently, they are not grouped together in menuconfig,
which looks a bit untidy. 
It might be better to move some of the Kconfig configuration higher up
to ensure they are properly grouped in the menu.

Regards
Pawel

>Besides, would you please comment [1], the key point is could we
>keep one platform driver with one compatible string, and cdns3 or cdnsp
>platform is decided at runtime?
>
>[1]
>https://urldefense.com/v3/__https://www.spinics.net/lists/kernel/msg6072480.
>html__;!!EHscmS1ygiU1lA!AMXCokQopzhe7zk6kKPFeLLp-
>yuh1is6xaU_qe56iXLYV9j5bEV_jUOV23iU07J2VMvUVf89LhR60DCMbBxz$
>
>--
>
>Best regards,
>Peter

Re: [PATCH 0/2] usb: cdns3: USBSSP platform driver support

[Peter Chen]

On 26-03-04 08:22:25, Pawel Laszczak wrote:
> [Some people who received this message don't often get email from pawell@cadence.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL
> 
> >
> >On 26-03-03 07:34:54, Pawel Laszczak wrote:
> >> [Some people who received this message don't often get email from
> >pawell@cadence.com. Learn why this is important at
> >https://urldefense.com/v3/__https://aka.ms/LearnAboutSenderIdentification__;!
> >!EHscmS1ygiU1lA!AMXCokQopzhe7zk6kKPFeLLp-
> >yuh1is6xaU_qe56iXLYV9j5bEV_jUOV23iU07J2VMvUVf89LhR60AKPHeqB$  ]
> >>
> >> EXTERNAL EMAIL
> >>
> >> >
> >> >Hi Pawel,
> >> >
> >> >The Cadence USBSSP (CDNSP) controller was previously only accessible
> >> >through PCI, coupling the gadget driver with the PCI glue layer into a single
> >> >monolithic module (cdnsp-udc-pci). This prevented using the CDNSP IP on
> >> >SoC/platform designs that expose the controller through device tree, eg, CIX
> >> >Sky1 SoC.
> >> >
> >> >In this series, it restructures the driver to decouple the CDNSP gadget from
> >> >PCI, and refactor cdnsp-pci.c into a thin PCI-to-platform wrapper (similar to
> >> >cdns3-pci-wrap.c) that registers a platform device and passes PCI resources
> >> >and platform data to the common platform driver. So, please help test it in
> >> >your platform.
> >> >
> >> >The changes are tested with random configuration combination tests.
> >> >
> >> >============================================================
> >> >ALL TESTS COMPLETE. Summary:
> >> >============================================================
> >> >Starting cdns3 config combination tests...
> >> >Timestamp: 2026年 03月 02日 星期一 09:34:47 CST
> >> >
> >> >PASS: all-builtin (SUPPORT=y CDNS3=y GADGET=y HOST=y CDNSP=y
> >> >CDNSP_G=y CDNSP_H=y)
> >> >PASS: support-y_cdns3-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y
> >> >HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
> >> >PASS: support-y_cdns3-m_gadget-n (SUPPORT=y CDNS3=m GADGET=n
> >> >HOST=y CDNSP=y CDNSP_G=y CDNSP_H=y)
> >> >PASS: support-y_cdnsp-m_gadget-y (SUPPORT=y CDNS3=y GADGET=y HOST=y
> >> >CDNSP=m CDNSP_G=y CDNSP_H=y)
> >> >PASS: all-module (SUPPORT=m CDNS3=m GADGET=y HOST=y CDNSP=m
> >> >CDNSP_G=y CDNSP_H=y)
> >> >PASS: no-cdns3 (SUPPORT=y CDNS3=n GADGET=n HOST=n CDNSP=y
> >> >CDNSP_G=y CDNSP_H=y)
> >> >PASS: no-gadget (SUPPORT=y CDNS3=y GADGET=n HOST=y CDNSP=y
> >> >CDNSP_G=n CDNSP_H=y)
> >> >PASS: support-y_both-m_gadget-y (SUPPORT=y CDNS3=m GADGET=y HOST=y
> >> >CDNSP=m CDNSP_G=y CDNSP_H=y)
> >> >PASS: minimal-module (SUPPORT=m CDNS3=m GADGET=n HOST=n CDNSP=m
> >> >CDNSP_G=n CDNSP_H=n)
> >> >
> >> >This patch was developed with assistance from Anthropic Claude Opus 4.6.
> >> >
> >>
> >> I can compile the kernel but when I try to install modules with
> >> make modules_install I get error:
> >>
> >> DEPMOD  /lib/modules/7.0.0-rc1-new-pci-plat-support-next-20260227+
> >> depmod: ERROR: Cycle detected: cdns_usb_common -> cdnsp ->
> >cdns_usb_common
> >> depmod: ERROR: Cycle detected: udc_core
> >> depmod: ERROR: Found 2 modules in dependency cycles!
> >> make[2]: *** [scripts/Makefile.modinst:132: depmod] Error 1
> >>
> >> It occurs even with minimal configuration:
> >> CONFIG_USB_CDNS_SUPPORT=m
> >> # CONFIG_USB_CDNS3 is not set
> >> CONFIG_USB_CDNSP=m
> >> CONFIG_USB_CDNSP_GADGET=y
> >>
> >
> >Thanks for testing. It needs to let cdns3(p) platform driver as
> >standalone module to fix it. Would you please try below fix on top
> >of my patch set:
> >
> >diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
> >index 0e9b20e799d0..63484f145bb9 100644
> >--- a/drivers/usb/cdns3/Makefile
> >+++ b/drivers/usb/cdns3/Makefile
> >@@ -3,12 +3,14 @@
> > CFLAGS_cdns3-trace.o                          := -I$(src)
> > CFLAGS_cdnsp-trace.o                          := -I$(src)
> >
> >-cdns-usb-common-y                             := core.o drd.o cdns3-plat.o
> >+cdns-usb-common-y                             := core.o drd.o
> >
> > ifeq ($(CONFIG_USB),m)
> > obj-m                                         += cdns-usb-common.o
> >+obj-m                                         += cdns3-plat.o
> > else
> > obj-$(CONFIG_USB_CDNS_SUPPORT)                        += cdns-usb-common.o
> >+obj-$(CONFIG_USB_CDNS_SUPPORT)                        += cdns3-plat.o
> > endif
> >
> > cdns-usb-common-$(CONFIG_USB_CDNS_HOST)       += host.o
> >
> 
> With this fix, the modules installation works correctly.
> 
> I have an additional comment regarding the grouping of USBSSP and CDNS3
> options. Currently, they are not grouped together in menuconfig,
> which looks a bit untidy.
> It might be better to move some of the Kconfig configuration higher up
> to ensure they are properly grouped in the menu.
> 

Thanks for testing. Would you please comment below question, it depends
on how I do the v2 patch?

> Regards
> Pawel
> 
> >Besides, would you please comment [1], the key point is could we
> >keep one platform driver with one compatible string, and cdns3 or cdnsp
> >platform is decided at runtime?
> >
> >[1]
> >https://urldefense.com/v3/__https://www.spinics.net/lists/kernel/msg6072480.
> >html__;!!EHscmS1ygiU1lA!AMXCokQopzhe7zk6kKPFeLLp-
> >yuh1is6xaU_qe56iXLYV9j5bEV_jUOV23iU07J2VMvUVf89LhR60DCMbBxz$
> >
> >--
> >
> >Best regards,
> >Peter

-- 

Best regards,
Peter

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

[Pawel Laszczak]

>
>On 26-03-02 10:27:11, Krzysztof Kozlowski wrote:
>> >>>  maintainers:
>> >>>    - Pawel Laszczak <pawell@cadence.com>
>> >>>
>> >>> +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?
>
>Peter
>
>>
>> > 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)
>>
>> You just described drivers, so this does not convince me at all.
>>
>> >
>> > 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?
>>
>> You did not respond to this one. Look how this schema is used.
>>
>> >
>> > 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:
>>
>> 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?

Krzysztof, what do you think about this approach?

>
>--
>
>Best regards,
>Peter

Re: [PATCH 1/2] usb: cdns3: Add USBSSP platform driver support

[kernel test robot]

Hi Peter,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v7.0-rc2 next-20260304]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Peter-Chen/usb-cdns3-Add-USBSSP-platform-driver-support/20260302-110634
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20260302030339.324196-2-peter.chen%40cixtech.com
patch subject: [PATCH 1/2] usb: cdns3: Add USBSSP platform driver support
config: x86_64-buildonly-randconfig-001-20260305 (https://download.01.org/0day-ci/archive/20260305/202603050731.SDNJ7lx5-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260305/202603050731.SDNJ7lx5-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603050731.SDNJ7lx5-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/usb/cdns3/cdns3-plat.c:26:28: warning: 'cdns3_plat' defined but not used [-Wunused-const-variable=]
      26 | static const unsigned long cdns3_plat;
         |                            ^~~~~~~~~~


vim +/cdns3_plat +26 drivers/usb/cdns3/cdns3-plat.c

    25	
  > 26	static const unsigned long cdns3_plat;
    27	static const unsigned long cdnsp_plat;
    28	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] usb: cdns3: Add USBSSP platform driver support

[kernel test robot]

Hi Peter,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v7.0-rc2 next-20260304]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Peter-Chen/usb-cdns3-Add-USBSSP-platform-driver-support/20260302-110634
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20260302030339.324196-2-peter.chen%40cixtech.com
patch subject: [PATCH 1/2] usb: cdns3: Add USBSSP platform driver support
config: hexagon-randconfig-001-20260305 (https://download.01.org/0day-ci/archive/20260305/202603050737.OrrASrnZ-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 9a109fbb6e184ec9bcce10615949f598f4c974a9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260305/202603050737.OrrASrnZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603050737.OrrASrnZ-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/usb/cdns3/cdns3-plat.c:26:28: warning: unused variable 'cdns3_plat' [-Wunused-const-variable]
      26 | static const unsigned long cdns3_plat;
         |                            ^~~~~~~~~~
   1 warning generated.


vim +/cdns3_plat +26 drivers/usb/cdns3/cdns3-plat.c

    25	
  > 26	static const unsigned long cdns3_plat;
    27	static const unsigned long cdnsp_plat;
    28	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] usb: cdns3: Add USBSSP platform driver support

[kernel test robot]

Hi Peter,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v7.0-rc2 next-20260306]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Peter-Chen/usb-cdns3-Add-USBSSP-platform-driver-support/20260302-110634
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20260302030339.324196-2-peter.chen%40cixtech.com
patch subject: [PATCH 1/2] usb: cdns3: Add USBSSP platform driver support
config: i386-randconfig-007-20250530 (https://download.01.org/0day-ci/archive/20260308/202603081901.b3stSZC8-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260308/202603081901.b3stSZC8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603081901.b3stSZC8-lkp@intel.com/

All errors (new ones prefixed by >>):

>> depmod: ERROR: Cycle detected: cdns_usb_common -> cdns3 -> cdns_usb_common
   depmod: ERROR: Found 2 modules in dependency cycles!
   make[3]: *** [scripts/Makefile.modinst:132: depmod] Error 1
   make[3]: Target '__modinst' not remade because of errors.
   make[2]: *** [Makefile:2007: modules_install] Error 2
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'modules_install' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'modules_install' not remade because of errors.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

RE: [PATCH 1/2] usb: cdns3: Add USBSSP platform driver support

[Pawel Laszczak]

>
>The Cadence USBSSP (CDNSP) controller was previously only accessible
>through PCI, coupling the gadget driver with the PCI glue layer into a
>single monolithic module (cdnsp-udc-pci). This prevented using the
>CDNSP IP on SoC/platform designs that expose the controller through
>device tree.
>
>Restructure the driver to decouple the CDNSP gadget from PCI:
>
>- Introduce CONFIG_USB_CDNSP as a standalone tristate (analogous to
>  CONFIG_USB_CDNS3), with USB_CDNSP_GADGET and USB_CDNSP_HOST as
>  bool sub-options. The gadget code builds as a separate cdnsp.ko
>  module.
>
>- Refactor cdnsp-pci.c into a thin PCI-to-platform wrapper (similar
>  to cdns3-pci-wrap.c) that registers a platform device and passes
>  PCI resources and platform data to the common platform driver.
>
>- Add "cdns,usbssp" compatible string to the shared platform driver
>  (cdns3-plat.c). The probe function uses of_device_id match data to
>  select cdnsp_gadget_init vs cdns3_gadget_init.
>
>- Move cdns3-plat.o into cdns-usb-common, since it now serves as the
>  shared platform driver entry point for both CDNS3 and CDNSP.
>
>- Fix gadget-export.h to use IS_REACHABLE() keyed on the tristate
>  module config (CONFIG_USB_CDNS3/CONFIG_USB_CDNSP) instead of
>  IS_ENABLED() on the bool gadget config. The bool configs are always
>  'y' when enabled, causing IS_ENABLED/IS_REACHABLE to always return
>  true and resulting in link errors when cdns-usb-common is built-in
>  but the gadget module is loadable.
>
>- Add missing MODULE_LICENSE()/MODULE_DESCRIPTION() and
>  EXPORT_SYMBOL_GPL() to the cdns3 and cdnsp gadget modules, required
>  by modpost.
>
>- Pass override_apb_timeout through cdns3_platform_data so the PCI
>  wrapper can communicate PCI-specific APB timeout values to the
>  common driver.
>
>This patch was developed with assistance from Anthropic Claude Opus 4.6.
>
>Signed-off-by: Peter Chen <peter.chen@cixtech.com>
>---
> drivers/usb/cdns3/Kconfig         |  44 +++---
> drivers/usb/cdns3/Makefile        |  30 ++--
> drivers/usb/cdns3/cdns3-gadget.c  |   4 +
> drivers/usb/cdns3/cdns3-plat.c    |  14 +-
> drivers/usb/cdns3/cdnsp-gadget.c  |   4 +
> drivers/usb/cdns3/cdnsp-pci.c     | 221 +++++++++++++-----------------
> drivers/usb/cdns3/core.h          |   1 +
> drivers/usb/cdns3/gadget-export.h |   4 +-
> 8 files changed, 151 insertions(+), 171 deletions(-)
>
>diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
>index 0a514b591527..80c740f5c755 100644
>--- a/drivers/usb/cdns3/Kconfig
>+++ b/drivers/usb/cdns3/Kconfig
>@@ -20,8 +20,13 @@ config USB_CDNS3
> 	  Say Y here if your system has a Cadence USB3 dual-role controller.
> 	  It supports: dual-role switch, Host-only, and Peripheral-only.
>
>-	  If you choose to build this driver is a dynamically linked
>-	  as module, the module will be called cdns3.ko.
>+config USB_CDNSP
>+	tristate "Cadence USBSSP Dual-Role Controller"
>+	depends on USB_CDNS_SUPPORT
>+	help
>+	  Say Y here if your system has a Cadence USBSSP dual-role controller.
>+	  It supports: dual-role switch, Host-only, and Peripheral-only.
>+
> endif
>
> if USB_CDNS3
>@@ -91,27 +96,14 @@ config USB_CDNS3_STARFIVE
> 	  be dynamically linked and module will be called cdns3-starfive.ko
> endif
>
>-if USB_CDNS_SUPPORT
>-
>-config USB_CDNSP_PCI
>-	tristate "Cadence CDNSP Dual-Role Controller"
>-	depends on USB_CDNS_SUPPORT && USB_PCI && ACPI
>-	help
>-	  Say Y here if your system has a Cadence CDNSP dual-role controller.
>-	  It supports: dual-role switch Host-only, and Peripheral-only.
>-
>-	  If you choose to build this driver is a dynamically linked
>-	  module, the module will be called cdnsp.ko.
>-endif
>-
>-if USB_CDNSP_PCI
>+if USB_CDNSP
>
> config USB_CDNSP_GADGET
>-	bool "Cadence CDNSP device controller"
>-	depends on USB_GADGET=y || USB_GADGET=USB_CDNSP_PCI
>+	bool "Cadence USBSSP device controller"
>+	depends on USB_GADGET=y || USB_GADGET=USB_CDNSP
> 	help
> 	  Say Y here to enable device controller functionality of the
>-	  Cadence CDNSP-DEV driver.
>+	  Cadence USBSSP-DEV driver.
>
> 	  Cadence CDNSP Device Controller in device mode is
> 	  very similar to XHCI controller. Therefore some algorithms
>@@ -120,8 +112,8 @@ config USB_CDNSP_GADGET
> 	  It doesn't support LS.
>
> config USB_CDNSP_HOST
>-	bool "Cadence CDNSP host controller"
>-	depends on USB=y || USB=USB_CDNSP_PCI
>+	bool "Cadence USBSSP host controller"
>+	depends on USB=y || USB=USB_CDNSP
> 	select USB_CDNS_HOST
> 	help
> 	  Say Y here to enable host controller functionality of the
>@@ -130,4 +122,14 @@ config USB_CDNSP_HOST
> 	  Host controller is compliant with XHCI so it uses
> 	  standard XHCI driver.
>
>+config USB_CDNSP_PCI
>+	tristate "Cadence USBSSP support on PCIe-based platforms"
>+	depends on USB_PCI && ACPI
>+	help
>+	  If you're using the USBSSP Core IP with a PCIe, please say
>+	  'Y' or 'M' here.
>+
>+	  If you choose to build this driver as module it will
>+	  be dynamically linked and module will be called cdnsp-pci.ko
>+
> endif
>diff --git a/drivers/usb/cdns3/Makefile b/drivers/usb/cdns3/Makefile
>index 48dfae75b5aa..0e9b20e799d0 100644
>--- a/drivers/usb/cdns3/Makefile
>+++ b/drivers/usb/cdns3/Makefile
>@@ -3,42 +3,32 @@
> CFLAGS_cdns3-trace.o				:= -I$(src)
> CFLAGS_cdnsp-trace.o				:= -I$(src)
>
>-cdns-usb-common-y				:= core.o drd.o
>-cdns3-y						:= cdns3-plat.o
>+cdns-usb-common-y				:= core.o drd.o cdns3-plat.o
>
> ifeq ($(CONFIG_USB),m)
> obj-m						+= cdns-usb-common.o
>-obj-m						+= cdns3.o
> else
> obj-$(CONFIG_USB_CDNS_SUPPORT)			+= cdns-usb-
>common.o
>-obj-$(CONFIG_USB_CDNS3)				+= cdns3.o
> endif
>
> cdns-usb-common-$(CONFIG_USB_CDNS_HOST) 	+= host.o
>-cdns3-$(CONFIG_USB_CDNS3_GADGET)		+= cdns3-gadget.o
>cdns3-ep0.o
>
>+# For CDNS3 gadget
> ifneq ($(CONFIG_USB_CDNS3_GADGET),)
>+cdns3-y						:= cdns3-gadget.o
>cdns3-ep0.o
> cdns3-$(CONFIG_TRACING)				+= cdns3-trace.o
>+obj-$(CONFIG_USB_CDNS3)				+= cdns3.o
> endif
>-
> obj-$(CONFIG_USB_CDNS3_PCI_WRAP)		+= cdns3-pci-wrap.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
>
>-cdnsp-udc-pci-y					:= cdnsp-pci.o
>-
>-ifdef CONFIG_USB_CDNSP_PCI
>-ifeq ($(CONFIG_USB),m)
>-obj-m						+= cdnsp-udc-pci.o
>-else
>-obj-$(CONFIG_USB_CDNSP_PCI) 			+= cdnsp-udc-pci.o
>-endif
>-endif
>-
>-cdnsp-udc-pci-$(CONFIG_USB_CDNSP_GADGET)	+= cdnsp-ring.o cdnsp-
>gadget.o \
>-						   cdnsp-mem.o cdnsp-ep0.o
>-
>+# For CDNSP gadget
> ifneq ($(CONFIG_USB_CDNSP_GADGET),)
>-cdnsp-udc-pci-$(CONFIG_TRACING)			+= cdnsp-trace.o
>+cdnsp-y						:= cdnsp-ring.o cdnsp-
>gadget.o \
>+						   cdnsp-mem.o cdnsp-ep0.o
>+cdnsp-$(CONFIG_TRACING)				+= cdnsp-trace.o
>+obj-$(CONFIG_USB_CDNSP)				+= cdnsp.o
> endif
>+obj-$(CONFIG_USB_CDNSP_PCI)			+= cdnsp-pci.o
>diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-
>gadget.c
>index d59a60a16ec7..b800bd1bedd4 100644
>--- a/drivers/usb/cdns3/cdns3-gadget.c
>+++ b/drivers/usb/cdns3/cdns3-gadget.c
>@@ -3508,3 +3508,7 @@ int cdns3_gadget_init(struct cdns *cdns)
>
> 	return 0;
> }
>+EXPORT_SYMBOL_GPL(cdns3_gadget_init);
>+
>+MODULE_LICENSE("GPL");
>+MODULE_DESCRIPTION("Cadence USBSS DRD Driver - gadget");
>diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
>index 735df88774e4..156404bd6a43 100644
>--- a/drivers/usb/cdns3/cdns3-plat.c
>+++ b/drivers/usb/cdns3/cdns3-plat.c
>@@ -23,6 +23,9 @@
> #include "gadget-export.h"
> #include "drd.h"
>
>+static const unsigned long cdns3_plat;
>+static const unsigned long cdnsp_plat;
>+
> static int set_phy_power_on(struct cdns *cdns)
> {
> 	int ret;
>@@ -64,6 +67,8 @@ static int cdns3_plat_probe(struct platform_device
>*pdev)
>
> 	cdns->dev = dev;
> 	cdns->pdata = dev_get_platdata(dev);
>+	if (cdns->pdata && cdns->pdata->override_apb_timeout)
>+		cdns->override_apb_timeout = cdns->pdata-
>>override_apb_timeout;
>
> 	platform_set_drvdata(pdev, cdns);
>
>@@ -143,7 +148,10 @@ static int cdns3_plat_probe(struct platform_device
>*pdev)
> 	if (ret)
> 		goto err_phy_power_on;
>
>-	cdns->gadget_init = cdns3_gadget_init;
>+	if (device_get_match_data(dev) == &cdnsp_plat)

To support USBSSP pci glue layer it should looks:
	if (device_get_match_data(dev) == &cdnsp_plat ||  cdns->pdata)

>+		cdns->gadget_init = cdnsp_gadget_init;
>+	else
>+		cdns->gadget_init = cdns3_gadget_init;
>
> 	ret = cdns_init(cdns);
> 	if (ret)
>@@ -317,7 +325,8 @@ static const struct dev_pm_ops cdns3_pm_ops = {
>
> #ifdef CONFIG_OF
> static const struct of_device_id of_cdns3_match[] = {
>-	{ .compatible = "cdns,usb3" },
>+	{ .compatible = "cdns,usb3", .data = &cdns3_plat },
>+	{ .compatible = "cdns,usbssp", .data = &cdnsp_plat },
> 	{ },
> };
> MODULE_DEVICE_TABLE(of, of_cdns3_match);
>@@ -336,6 +345,7 @@ static struct platform_driver cdns3_driver = {
> module_platform_driver(cdns3_driver);
>
> MODULE_ALIAS("platform:cdns3");
>+MODULE_ALIAS("platform:cdnsp");
> MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>");
> MODULE_LICENSE("GPL v2");
> MODULE_DESCRIPTION("Cadence USB3 DRD Controller Driver");
>diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-
>gadget.c
>index 6b3815f8a6e5..8db7eee528a1 100644
>--- a/drivers/usb/cdns3/cdnsp-gadget.c
>+++ b/drivers/usb/cdns3/cdnsp-gadget.c
>@@ -2075,3 +2075,7 @@ int cdnsp_gadget_init(struct cdns *cdns)
>
> 	return 0;
> }
>+EXPORT_SYMBOL_GPL(cdnsp_gadget_init);
>+
>+MODULE_LICENSE("GPL");
>+MODULE_DESCRIPTION("Cadence CDNSP DRD Driver - gadget");
>diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
>index 566d94e49102..7deb25816548 100644
>--- a/drivers/usb/cdns3/cdnsp-pci.c
>+++ b/drivers/usb/cdns3/cdnsp-pci.c
>@@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
>- * Cadence PCI Glue driver.
>+ * Cadence USBSSP PCI Glue driver.
>  *
>  * Copyright (C) 2019 Cadence.
>  *
>@@ -8,15 +8,27 @@
>  *
>  */
>
>-#include <linux/platform_device.h>
>-#include <linux/dma-mapping.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
>-#include <linux/slab.h>
> #include <linux/pci.h>
>+#include <linux/platform_device.h>
>+#include <linux/dma-mapping.h>
>+#include <linux/slab.h>
>
> #include "core.h"
>-#include "gadget-export.h"
>+
>+struct cdnsp_wrap {
>+	struct platform_device *plat_dev;
>+	struct resource dev_res[6];
>+	int devfn;
>+};
>+
>+#define RES_IRQ_HOST_ID		0
>+#define RES_IRQ_PERIPHERAL_ID	1
>+#define RES_IRQ_OTG_ID		2
>+#define RES_HOST_ID		3
>+#define RES_DEV_ID		4
>+#define RES_DRD_ID		5
>
> #define PCI_BAR_HOST		0
> #define PCI_BAR_OTG		0
>@@ -28,14 +40,14 @@
> #define PCI_DRIVER_NAME		"cdns-pci-usbssp"
> #define PLAT_DRIVER_NAME	"cdns-usbssp"

Platform driver name is "cdns-usb3"

Regards,
Pawel

>
>-#define CHICKEN_APB_TIMEOUT_VALUE       0x1C20
>+#define CHICKEN_APB_TIMEOUT_VALUE	0x1C20
>
> static struct pci_dev *cdnsp_get_second_fun(struct pci_dev *pdev)
> {
> 	/*
> 	 * Gets the second function.
>-	 * Platform has two function. The fist keeps resources for
>-	 * Host/Device while the secon keeps resources for DRD/OTG.
>+	 * Platform has two function. The first keeps resources for
>+	 * Host/Device while the second keeps resources for DRD/OTG.
> 	 */
> 	if (pdev->device == PCI_DEVICE_ID_CDNS_USBSSP)
> 		return pci_get_device(pdev->vendor,
>PCI_DEVICE_ID_CDNS_USBSS, NULL);
>@@ -48,11 +60,12 @@ static struct pci_dev *cdnsp_get_second_fun(struct
>pci_dev *pdev)
> static int cdnsp_pci_probe(struct pci_dev *pdev,
> 			   const struct pci_device_id *id)
> {
>-	struct device *dev = &pdev->dev;
>-	struct pci_dev *func;
>+	struct platform_device_info plat_info;
>+	static struct cdns3_platform_data pdata;
>+	struct cdnsp_wrap *wrap;
> 	struct resource *res;
>-	struct cdns *cdnsp;
>-	int ret;
>+	struct pci_dev *func;
>+	int ret = 0;
>
> 	/*
> 	 * For GADGET/HOST PCI (devfn) function number is 0,
>@@ -79,146 +92,105 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
> 	}
>
> 	pci_set_master(pdev);
>+
> 	if (pci_is_enabled(func)) {
>-		cdnsp = pci_get_drvdata(func);
>+		wrap = pci_get_drvdata(func);
> 	} else {
>-		cdnsp = kzalloc_obj(*cdnsp);
>-		if (!cdnsp) {
>+		wrap = kzalloc_obj(*wrap);
>+		if (!wrap) {
> 			ret = -ENOMEM;
> 			goto put_pci;
> 		}
> 	}
>
>-	/* For GADGET device function number is 0. */
>-	if (pdev->devfn == 0) {
>-		resource_size_t rsrc_start, rsrc_len;
>-
>-		/* Function 0: host(BAR_0) + device(BAR_1).*/
>-		dev_dbg(dev, "Initialize resources\n");
>-		rsrc_start = pci_resource_start(pdev, PCI_BAR_DEV);
>-		rsrc_len = pci_resource_len(pdev, PCI_BAR_DEV);
>-		res = devm_request_mem_region(dev, rsrc_start, rsrc_len,
>"dev");
>-		if (!res) {
>-			dev_dbg(dev, "controller already in use\n");
>-			ret = -EBUSY;
>-			goto free_cdnsp;
>-		}
>-
>-		cdnsp->dev_regs = devm_ioremap(dev, rsrc_start, rsrc_len);
>-		if (!cdnsp->dev_regs) {
>-			dev_dbg(dev, "error mapping memory\n");
>-			ret = -EFAULT;
>-			goto free_cdnsp;
>-		}
>-
>-		cdnsp->dev_irq = pdev->irq;
>-		dev_dbg(dev, "USBSS-DEV physical base addr: %pa\n",
>-			&rsrc_start);
>-
>-		res = &cdnsp->xhci_res[0];
>-		res->start = pci_resource_start(pdev, PCI_BAR_HOST);
>-		res->end = pci_resource_end(pdev, PCI_BAR_HOST);
>-		res->name = "xhci";
>-		res->flags = IORESOURCE_MEM;
>-		dev_dbg(dev, "USBSS-XHCI physical base addr: %pa\n",
>-			&res->start);
>-
>-		/* Interrupt for XHCI, */
>-		res = &cdnsp->xhci_res[1];
>-		res->start = pdev->irq;
>-		res->name = "host";
>-		res->flags = IORESOURCE_IRQ;
>+	res = wrap->dev_res;
>+
>+	if (pdev->devfn == PCI_DEV_FN_HOST_DEVICE) {
>+		/* Function 0: host(BAR_0) + device(BAR_2). */
>+		dev_dbg(&pdev->dev, "Initialize Device resources\n");
>+		res[RES_DEV_ID].start = pci_resource_start(pdev,
>PCI_BAR_DEV);
>+		res[RES_DEV_ID].end = pci_resource_end(pdev,
>PCI_BAR_DEV);
>+		res[RES_DEV_ID].name = "dev";
>+		res[RES_DEV_ID].flags = IORESOURCE_MEM;
>+		dev_dbg(&pdev->dev, "USBSSP-DEV physical base addr:
>%pa\n",
>+			&res[RES_DEV_ID].start);
>+
>+		res[RES_HOST_ID].start = pci_resource_start(pdev,
>PCI_BAR_HOST);
>+		res[RES_HOST_ID].end = pci_resource_end(pdev,
>PCI_BAR_HOST);
>+		res[RES_HOST_ID].name = "xhci";
>+		res[RES_HOST_ID].flags = IORESOURCE_MEM;
>+		dev_dbg(&pdev->dev, "USBSSP-XHCI physical base addr:
>%pa\n",
>+			&res[RES_HOST_ID].start);
>+
>+		/* Interrupt for XHCI */
>+		wrap->dev_res[RES_IRQ_HOST_ID].start = pdev->irq;
>+		wrap->dev_res[RES_IRQ_HOST_ID].name = "host";
>+		wrap->dev_res[RES_IRQ_HOST_ID].flags = IORESOURCE_IRQ;
>+
>+		/* Interrupt for device. It's the same as for HOST. */
>+		wrap->dev_res[RES_IRQ_PERIPHERAL_ID].start = pdev->irq;
>+		wrap->dev_res[RES_IRQ_PERIPHERAL_ID].name =
>"peripheral";
>+		wrap->dev_res[RES_IRQ_PERIPHERAL_ID].flags =
>IORESOURCE_IRQ;
> 	} else {
>-		res = &cdnsp->otg_res;
>-		res->start = pci_resource_start(pdev, PCI_BAR_OTG);
>-		res->end =   pci_resource_end(pdev, PCI_BAR_OTG);
>-		res->name = "otg";
>-		res->flags = IORESOURCE_MEM;
>-		dev_dbg(dev, "CDNSP-DRD physical base addr: %pa\n",
>-			&res->start);
>+		res[RES_DRD_ID].start = pci_resource_start(pdev,
>PCI_BAR_OTG);
>+		res[RES_DRD_ID].end = pci_resource_end(pdev,
>PCI_BAR_OTG);
>+		res[RES_DRD_ID].name = "otg";
>+		res[RES_DRD_ID].flags = IORESOURCE_MEM;
>+		dev_dbg(&pdev->dev, "CDNSP-DRD physical base addr:
>%pa\n",
>+			&res[RES_DRD_ID].start);
>
> 		/* Interrupt for OTG/DRD. */
>-		cdnsp->otg_irq = pdev->irq;
>+		wrap->dev_res[RES_IRQ_OTG_ID].start = pdev->irq;
>+		wrap->dev_res[RES_IRQ_OTG_ID].name = "otg";
>+		wrap->dev_res[RES_IRQ_OTG_ID].flags = IORESOURCE_IRQ;
> 	}
>
>-	/*
>-	 * Cadence PCI based platform require some longer timeout for APB
>-	 * to fixes domain clock synchronization issue after resuming
>-	 * controller from L1 state.
>-	 */
>-	cdnsp->override_apb_timeout = CHICKEN_APB_TIMEOUT_VALUE;
>-	pci_set_drvdata(pdev, cdnsp);
>-
> 	if (pci_is_enabled(func)) {
>-		cdnsp->dev = dev;
>-		cdnsp->gadget_init = cdnsp_gadget_init;
>-
>-		ret = cdns_init(cdnsp);
>-		if (ret)
>-			goto free_cdnsp;
>+		/* set up platform device info */
>+		pdata.override_apb_timeout =
>CHICKEN_APB_TIMEOUT_VALUE;
>+		memset(&plat_info, 0, sizeof(plat_info));
>+		plat_info.parent = &pdev->dev;
>+		plat_info.fwnode = pdev->dev.fwnode;
>+		plat_info.name = PLAT_DRIVER_NAME;
>+		plat_info.id = pdev->devfn;
>+		plat_info.res = wrap->dev_res;
>+		plat_info.num_res = ARRAY_SIZE(wrap->dev_res);
>+		plat_info.dma_mask = pdev->dma_mask;
>+		plat_info.data = &pdata;
>+		plat_info.size_data = sizeof(pdata);
>+		wrap->devfn = pdev->devfn;
>+		/* register platform device */
>+		wrap->plat_dev = platform_device_register_full(&plat_info);
>+		if (IS_ERR(wrap->plat_dev)) {
>+			ret = PTR_ERR(wrap->plat_dev);
>+			kfree(wrap);
>+			goto put_pci;
>+		}
> 	}
>
>-	device_wakeup_enable(&pdev->dev);
>-	if (pci_dev_run_wake(pdev))
>-		pm_runtime_put_noidle(&pdev->dev);
>-
>-	return 0;
>-
>-free_cdnsp:
>-	if (!pci_is_enabled(func))
>-		kfree(cdnsp);
>-
>+	pci_set_drvdata(pdev, wrap);
> put_pci:
> 	pci_dev_put(func);
>-
> 	return ret;
> }
>
> static void cdnsp_pci_remove(struct pci_dev *pdev)
> {
>-	struct cdns *cdnsp;
>+	struct cdnsp_wrap *wrap;
> 	struct pci_dev *func;
>
> 	func = cdnsp_get_second_fun(pdev);
>-	cdnsp = (struct cdns *)pci_get_drvdata(pdev);
>+	wrap = pci_get_drvdata(pdev);
>
>-	if (pci_dev_run_wake(pdev))
>-		pm_runtime_get_noresume(&pdev->dev);
>+	if (wrap->devfn == pdev->devfn)
>+		platform_device_unregister(wrap->plat_dev);
>
>-	if (pci_is_enabled(func)) {
>-		cdns_remove(cdnsp);
>-	} else {
>-		kfree(cdnsp);
>-	}
>+	if (!pci_is_enabled(func))
>+		kfree(wrap);
>
> 	pci_dev_put(func);
> }
>
>-static int __maybe_unused cdnsp_pci_suspend(struct device *dev)
>-{
>-	struct cdns *cdns = dev_get_drvdata(dev);
>-
>-	return cdns_suspend(cdns);
>-}
>-
>-static int __maybe_unused cdnsp_pci_resume(struct device *dev)
>-{
>-	struct cdns *cdns = dev_get_drvdata(dev);
>-	unsigned long flags;
>-	int ret;
>-
>-	spin_lock_irqsave(&cdns->lock, flags);
>-	ret = cdns_resume(cdns);
>-	spin_unlock_irqrestore(&cdns->lock, flags);
>-	cdns_set_active(cdns, 1);
>-
>-	return ret;
>-}
>-
>-static const struct dev_pm_ops cdnsp_pci_pm_ops = {
>-	SET_SYSTEM_SLEEP_PM_OPS(cdnsp_pci_suspend,
>cdnsp_pci_resume)
>-};
>-
> static const struct pci_device_id cdnsp_pci_ids[] = {
> 	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_USBSSP),
> 	  .class = PCI_CLASS_SERIAL_USB_DEVICE },
>@@ -230,13 +202,10 @@ static const struct pci_device_id cdnsp_pci_ids[] = {
> };
>
> static struct pci_driver cdnsp_pci_driver = {
>-	.name = "cdnsp-pci",
>+	.name = PCI_DRIVER_NAME,
> 	.id_table = cdnsp_pci_ids,
> 	.probe = cdnsp_pci_probe,
> 	.remove = cdnsp_pci_remove,
>-	.driver = {
>-		.pm = &cdnsp_pci_pm_ops,
>-	}
> };
>
> module_pci_driver(cdnsp_pci_driver);
>@@ -245,4 +214,4 @@ MODULE_DEVICE_TABLE(pci, cdnsp_pci_ids);
> MODULE_ALIAS("pci:cdnsp");
> MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>");
> MODULE_LICENSE("GPL v2");
>-MODULE_DESCRIPTION("Cadence CDNSP PCI driver");
>+MODULE_DESCRIPTION("Cadence CDNSP PCI wrapper");
>diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>index 801be9e61340..9149612bb2a8 100644
>--- a/drivers/usb/cdns3/core.h
>+++ b/drivers/usb/cdns3/core.h
>@@ -45,6 +45,7 @@ struct cdns3_platform_data {
> 	unsigned long quirks;
> #define CDNS3_DEFAULT_PM_RUNTIME_ALLOW	BIT(0)
> #define CDNS3_DRD_SUSPEND_RESIDENCY_ENABLE	BIT(1)
>+	u32 override_apb_timeout;	/* 0 = use default (e.g. for PCI) */
> };
>
> /**
>diff --git a/drivers/usb/cdns3/gadget-export.h b/drivers/usb/cdns3/gadget-
>export.h
>index c37b6269b001..0cb600e2b5d2 100644
>--- a/drivers/usb/cdns3/gadget-export.h
>+++ b/drivers/usb/cdns3/gadget-export.h
>@@ -10,7 +10,7 @@
> #ifndef __LINUX_CDNS3_GADGET_EXPORT
> #define __LINUX_CDNS3_GADGET_EXPORT
>
>-#if IS_ENABLED(CONFIG_USB_CDNSP_GADGET)
>+#if defined(CONFIG_USB_CDNSP_GADGET) &&
>IS_REACHABLE(CONFIG_USB_CDNSP)
>
> int cdnsp_gadget_init(struct cdns *cdns);
> #else
>@@ -22,7 +22,7 @@ static inline int cdnsp_gadget_init(struct cdns *cdns)
>
> #endif /* CONFIG_USB_CDNSP_GADGET */
>
>-#if IS_ENABLED(CONFIG_USB_CDNS3_GADGET)
>+#if defined(CONFIG_USB_CDNS3_GADGET) &&
>IS_REACHABLE(CONFIG_USB_CDNS3)
>
> int cdns3_gadget_init(struct cdns *cdns);
> #else
>--
>2.50.1

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

[Peter Chen]

On 26-03-04 10:02:07, Pawel Laszczak wrote:
> [Some people who received this message don't often get email from pawell@cadence.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> >> >>>
> >> >>> +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


-- 

Best regards,
Peter

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@cadence.com. 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