Devicetree
 help / color / mirror / Atom feed
* [PATCH v5 0/2] usb: cdns3: support configurations without DRD block
@ 2026-05-13  8:41 Pawel Laszczak via B4 Relay
  2026-05-13  8:41 ` [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property Pawel Laszczak via B4 Relay
  2026-05-13  8:41 ` [PATCH v5 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay
  0 siblings, 2 replies; 6+ messages in thread
From: Pawel Laszczak via B4 Relay @ 2026-05-13  8:41 UTC (permalink / raw)
  To: Peter Chen, Roger Quadros, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-usb, devicetree, linux-kernel, Pawel Laszczak,
	Bjorn Helgaas

usb: cdns3: support configurations without DRD block

This series adds support for Cadence USBSSP controllers in hardware
layouts where the Dual-Role Device (DRD) register block is either
missing or inaccessible.

In such configurations, the controller is hardwired to a single role
(either host or device) and the driver must skip all OTG/DRD register
accesses to avoid bus errors or incorrect role detection.

The solution introduces a new 'no_drd' property that can be passed
via DT or software nodes. When set, the driver:
1. Skips DRD register mapping and IRQ requests.
2. Uses a different BAR indexing logic for PCI-based configurations
   (32-bit addressing layout).
3. Hardwires the role based on 'dr_mode'.

Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
Note: This series is based on current linux-next. I am aware of Peter
Chen's recent refactoring series ("usb: cdns3: plat: Expose platform
core driver as library"). Although there is a minor conflict in
cdns3-plat.c, Peter has already provided an Acked-by for this version.
I am happy to provide a rebased v6 as soon as Peter's changes land in
linux-next if required.
---
v5:
- Implemented strict conditional validation using if-then-else logic.
- Enforced 2 register/interrupt items and required 'dr_mode'
  (host or peripheral) when 'no_drd' is present.
- Enforced the standard 3 register/interrupt items (otg, host, dev)
  when 'no_drd' is absent to ensure backward compatibility.
- Updated 'reg-names' and 'interrupt-names' to use enums in the main
  properties section to support flexible resource ordering during
  validation.

v4:
  - Added DT binding documentation for the 'no_drd' property.
  - Relaxed 'reg' and 'interrupts' requirements in the DT schema (minItems 2)
    to allow configurations where the OTG/DRD register block is missing.
  - Moved PCI_DEVICE_ID_CDNS_UDC_USBSSP from pci_ids.h to cdnsp-pci.c
    to keep the global PCI ID list clean.

v3:
  - Improved descriptions and comments for better clarity.
  - Introduced the 'no_drd' property to indicate missing DRD register block.
  - Added support for fixed host-only and device-only configurations.
  - Ensured cdns_otg_disable_irq is called only when no_drd is false.
  - Updated cdns_drd_gadget_on/off to ensure PHY mode is correctly
    handled even if DRD is disabled.

v2:
  - Changed otg_irq to be optional.
  - Added cdns->no_drd check in cdns_power_is_lost.
  - Added cdns->no_drd check in cdns_get_id.

---

---
Pawel Laszczak (2):
      dt-bindings: usb: cdns3: Add no_drd property
      usb: cdnsp: Add support for device-only configuration

 .../devicetree/bindings/usb/cdns,usb3.yaml         | 67 +++++++++++++++++++---
 drivers/usb/cdns3/cdns3-plat.c                     | 26 +++++----
 drivers/usb/cdns3/cdnsp-pci.c                      | 47 ++++++++++++---
 drivers/usb/cdns3/core.c                           |  3 +-
 drivers/usb/cdns3/core.h                           |  4 ++
 drivers/usb/cdns3/drd.c                            | 44 +++++++++++++-
 6 files changed, 159 insertions(+), 32 deletions(-)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260512-b4-no_drd_config-f530f1f16d8a

Best regards,
--  
Pawel Laszczak <pawell@cadence.com>



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

* [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property
  2026-05-13  8:41 [PATCH v5 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay
@ 2026-05-13  8:41 ` Pawel Laszczak via B4 Relay
  2026-05-13  9:36   ` Rob Herring (Arm)
  2026-05-14  2:57   ` sashiko-bot
  2026-05-13  8:41 ` [PATCH v5 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay
  1 sibling, 2 replies; 6+ messages in thread
From: Pawel Laszczak via B4 Relay @ 2026-05-13  8:41 UTC (permalink / raw)
  To: Peter Chen, Roger Quadros, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-usb, devicetree, linux-kernel, Pawel Laszczak

From: Pawel Laszczak <pawell@cadence.com>

Introduce a new boolean property 'no_drd' for Cadence USBSS/USBSSP
controllers to support hardware configurations where the Dual-Role
Device (DRD) register block is missing or inaccessible.

When 'no_drd' is present:
- The 'otg' register and interrupt resources are not required.
- The 'reg' and 'interrupts' properties are restricted to 2 items
  (host and device).
- 'dr_mode' must be explicitly set to either 'host' or 'peripheral'.

When 'no_drd' is absent, the binding maintains backward compatibility
by requiring all 3 resource sets (otg, host, dev).

To achieve this, the schema is updated with an if-then-else logic
and 'reg-names'/'interrupt-names' use enums to allow flexible
ordering during validation.

Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
v5:
- Implemented strict conditional validation using if-then-else logic.
- Enforced 2 register/interrupt items and required 'dr_mode'
  (host or peripheral) when 'no_drd' is present.
- Enforced the standard 3 register/interrupt items (otg, host, dev)
  when 'no_drd' is absent to ensure backward compatibility.
- Updated 'reg-names' and 'interrupt-names' to use enums in the main
  properties section to support flexible resource ordering during
  validation.
---
---
 .../devicetree/bindings/usb/cdns,usb3.yaml         | 67 +++++++++++++++++++---
 1 file changed, 58 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
index 2d95fb7321af..5d9bea62729c 100644
--- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
+++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
@@ -20,19 +20,21 @@ properties:
     const: cdns,usb3
 
   reg:
+    minItems: 2
     items:
       - description: OTG controller registers
       - description: XHCI Host controller registers
       - description: DEVICE controller registers
 
   reg-names:
+    minItems: 2
     items:
-      - const: otg
-      - const: xhci
-      - const: dev
+      - enum: [ otg, xhci, dev ]
+      - enum: [ otg, xhci, dev ]
+      - enum: [ otg, xhci, dev ]
 
   interrupts:
-    minItems: 3
+    minItems: 2
     items:
       - description: XHCI host controller interrupt
       - description: Device controller interrupt
@@ -41,12 +43,12 @@ properties:
                      cleared by xhci core, this interrupt is optional
 
   interrupt-names:
-    minItems: 3
+    minItems: 2
     items:
-      - const: host
-      - const: peripheral
-      - const: otg
-      - const: wakeup
+      - enum: [ host, peripheral, otg, wakeup ]
+      - enum: [ host, peripheral, otg, wakeup ]
+      - enum: [ host, peripheral, otg, wakeup ]
+      - enum: [ host, peripheral, otg, wakeup ]
 
   port:
     $ref: /schemas/graph.yaml#/properties/port
@@ -79,6 +81,13 @@ properties:
     description: Enable resetting of PHY if Rx fail is detected
     type: boolean
 
+  no_drd:
+    description:
+      Indicates that the Dual-Role Device (DRD) register block is not
+      implemented or is inaccessible. In this case, the controller
+      must operate in a fixed peripheral or host mode.
+    type: boolean
+
 dependencies:
   port: [ usb-role-switch ]
 
@@ -93,6 +102,46 @@ allOf:
   - $ref: usb-drd.yaml#
   - $ref: usb-xhci.yaml#
 
+  - if:
+      properties:
+        no_drd: true
+      required:
+        - no_drd
+    then:
+      required:
+        - dr_mode
+      properties:
+        reg:
+          maxItems: 2
+        reg-names:
+          items:
+            - const: xhci
+            - const: dev
+        interrupts:
+          maxItems: 2
+        interrupt-names:
+          items:
+            - const: host
+            - const: peripheral
+        dr_mode:
+          enum: [host, peripheral]
+    else:
+      properties:
+        reg:
+          minItems: 3
+        reg-names:
+          items:
+            - const: otg
+            - const: xhci
+            - const: dev
+        interrupts:
+          minItems: 3
+        interrupt-names:
+          items:
+            - const: host
+            - const: peripheral
+            - const: otg
+
 unevaluatedProperties: false
 
 examples:

-- 
2.43.0



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

* [PATCH v5 2/2] usb: cdnsp: Add support for device-only configuration
  2026-05-13  8:41 [PATCH v5 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay
  2026-05-13  8:41 ` [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property Pawel Laszczak via B4 Relay
@ 2026-05-13  8:41 ` Pawel Laszczak via B4 Relay
  2026-05-14  3:27   ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Pawel Laszczak via B4 Relay @ 2026-05-13  8:41 UTC (permalink / raw)
  To: Peter Chen, Roger Quadros, Greg Kroah-Hartman, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-usb, devicetree, linux-kernel, Pawel Laszczak,
	Bjorn Helgaas

From: Pawel Laszczak <pawell@cadence.com>

This patch introduces support for the Cadence USBSSP (cdnsp)
controller in hardware configurations where the Dual-Role Device (DRD)
register block is not implemented or is inaccessible.

In such cases, the driver cannot rely on the DRD logic to manage roles
and must operate exclusively in a fixed peripheral/host mode.

The change in BAR indexing (from BAR 2 to BAR 1) is a direct
consequence of the 32-bit addressing used in this specific
DRD-disabled hardware layout, compared to the 64-bit addressing
used in DRD-enabled configurations.

Tested on a PCI platform with a hardware configuration that lacks
DRD support. Platform-side changes are included to support the PCI
glue layer's property injection to handle this specific layout.

Acked-by: Peter Chen <peter.chen@kernel.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
 drivers/usb/cdns3/cdns3-plat.c | 26 +++++++++++++----------
 drivers/usb/cdns3/cdnsp-pci.c  | 47 ++++++++++++++++++++++++++++++++++--------
 drivers/usb/cdns3/core.c       |  3 ++-
 drivers/usb/cdns3/core.h       |  4 ++++
 drivers/usb/cdns3/drd.c        | 44 +++++++++++++++++++++++++++++++++++++--
 5 files changed, 101 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
index 3fe3109a3688..86c963a072db 100644
--- a/drivers/usb/cdns3/cdns3-plat.c
+++ b/drivers/usb/cdns3/cdns3-plat.c
@@ -81,6 +81,7 @@ static int cdns3_plat_probe(struct platform_device *pdev)
 	if (cdns->pdata && cdns->pdata->override_apb_timeout)
 		cdns->override_apb_timeout = cdns->pdata->override_apb_timeout;
 
+	cdns->no_drd = device_property_read_bool(dev, "no_drd");
 	platform_set_drvdata(pdev, cdns);
 
 	ret = platform_get_irq_byname(pdev, "host");
@@ -113,21 +114,24 @@ static int cdns3_plat_probe(struct platform_device *pdev)
 
 	cdns->dev_regs	= regs;
 
-	cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
-	if (cdns->otg_irq < 0)
-		return dev_err_probe(dev, cdns->otg_irq,
-				     "Failed to get otg IRQ\n");
-
-	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
-	if (!res) {
-		dev_err(dev, "couldn't get otg resource\n");
-		return -ENXIO;
+	if (!cdns->no_drd) {
+		cdns->otg_irq = platform_get_irq_byname(pdev, "otg");
+		if (cdns->otg_irq < 0)
+			return dev_err_probe(dev, cdns->otg_irq,
+					     "Failed to get otg IRQ\n");
+
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg");
+		if (!res) {
+			dev_err(dev, "couldn't get otg resource\n");
+			return -ENXIO;
+		}
+		cdns->otg_res = *res;
+	} else {
+		dev_dbg(dev, "No DRD support\n");
 	}
 
 	cdns->phyrst_a_enable = device_property_read_bool(dev, "cdns,phyrst-a-enable");
 
-	cdns->otg_res = *res;
-
 	cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");
 	if (cdns->wakeup_irq == -EPROBE_DEFER)
 		return cdns->wakeup_irq;
diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
index 432007cfe695..feb916229870 100644
--- a/drivers/usb/cdns3/cdnsp-pci.c
+++ b/drivers/usb/cdns3/cdnsp-pci.c
@@ -19,6 +19,7 @@
 
 struct cdnsp_wrap {
 	struct platform_device *plat_dev;
+	struct property_entry prop[3];
 	struct resource dev_res[6];
 	int devfn;
 };
@@ -29,10 +30,15 @@ struct cdnsp_wrap {
 #define RES_HOST_ID		3
 #define RES_DEV_ID		4
 #define RES_DRD_ID		5
-
+/* DRD PCI configuration - 64-bit addressing */
+/* First PCI function */
 #define PCI_BAR_HOST		0
-#define PCI_BAR_OTG		0
 #define PCI_BAR_DEV		2
+/* Second PCI function */
+#define PCI_BAR_OTG		0
+/* Device only PCI configuration - 32-bit addressing */
+/* First PCI function */
+#define PCI_BAR_ONLY_DEV	1
 
 #define PCI_DEV_FN_HOST_DEVICE	0
 #define PCI_DEV_FN_OTG		1
@@ -40,6 +46,7 @@ struct cdnsp_wrap {
 #define PCI_DRIVER_NAME		"cdns-pci-usbssp"
 #define PLAT_DRIVER_NAME	"cdns-usb3"
 
+#define PCI_DEVICE_ID_CDNS_UDC_USBSSP   0x0400
 #define CHICKEN_APB_TIMEOUT_VALUE	0x1C20
 
 static struct pci_dev *cdnsp_get_second_fun(struct pci_dev *pdev)
@@ -65,6 +72,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
 	struct cdnsp_wrap *wrap;
 	struct resource *res;
 	struct pci_dev *func;
+	bool no_drd = false;
 	int ret = 0;
 
 	/*
@@ -75,11 +83,14 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
 		    pdev->devfn != PCI_DEV_FN_OTG))
 		return -EINVAL;
 
+	if (pdev->device == PCI_DEVICE_ID_CDNS_UDC_USBSSP)
+		no_drd = true;
+
 	func = cdnsp_get_second_fun(pdev);
-	if (!func)
+	if (!func && !no_drd)
 		return -EINVAL;
 
-	if (func->class == PCI_CLASS_SERIAL_USB_XHCI ||
+	if ((func && func->class == PCI_CLASS_SERIAL_USB_XHCI) ||
 	    pdev->class == PCI_CLASS_SERIAL_USB_XHCI) {
 		ret = -EINVAL;
 		goto put_pci;
@@ -93,7 +104,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	if (pci_is_enabled(func)) {
+	if (func && pci_is_enabled(func)) {
 		wrap = pci_get_drvdata(func);
 	} else {
 		wrap = kzalloc_obj(*wrap);
@@ -106,10 +117,12 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
 	res = wrap->dev_res;
 
 	if (pdev->devfn == PCI_DEV_FN_HOST_DEVICE) {
+		int bar_dev = no_drd ? PCI_BAR_ONLY_DEV : PCI_BAR_DEV;
+
 		/* 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].start = pci_resource_start(pdev, bar_dev);
+		res[RES_DEV_ID].end = pci_resource_end(pdev, 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",
@@ -145,9 +158,20 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
 		wrap->dev_res[RES_IRQ_OTG_ID].flags = IORESOURCE_IRQ;
 	}
 
-	if (pci_is_enabled(func)) {
+	if (no_drd || pci_is_enabled(func)) {
+		u8 idx = 0;
+
 		/* set up platform device info */
 		pdata.override_apb_timeout = CHICKEN_APB_TIMEOUT_VALUE;
+
+		if (no_drd) {
+			wrap->prop[idx++] = PROPERTY_ENTRY_STRING("dr_mode", "peripheral");
+			wrap->prop[idx++] = PROPERTY_ENTRY_BOOL("no_drd");
+		} else {
+			wrap->prop[idx++] = PROPERTY_ENTRY_STRING("dr_mode", "otg");
+			wrap->prop[idx++] = PROPERTY_ENTRY_BOOL("usb-role-switch");
+		}
+
 		memset(&plat_info, 0, sizeof(plat_info));
 		plat_info.parent = &pdev->dev;
 		plat_info.fwnode = pdev->dev.fwnode;
@@ -158,6 +182,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
 		plat_info.dma_mask = pdev->dma_mask;
 		plat_info.data = &pdata;
 		plat_info.size_data = sizeof(pdata);
+		plat_info.properties = wrap->prop;
 		wrap->devfn = pdev->devfn;
 		/* register platform device */
 		wrap->plat_dev = platform_device_register_full(&plat_info);
@@ -185,13 +210,17 @@ static void cdnsp_pci_remove(struct pci_dev *pdev)
 	if (wrap->devfn == pdev->devfn)
 		platform_device_unregister(wrap->plat_dev);
 
-	if (!pci_is_enabled(func))
+	if (!func || !pci_is_enabled(func))
 		kfree(wrap);
 
 	pci_dev_put(func);
 }
 
 static const struct pci_device_id cdnsp_pci_ids[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_UDC_USBSSP),
+	  .class = PCI_CLASS_SERIAL_USB_DEVICE },
+	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_UDC_USBSSP),
+	  .class = PCI_CLASS_SERIAL_USB_CDNS },
 	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_USBSSP),
 	  .class = PCI_CLASS_SERIAL_USB_DEVICE },
 	{ PCI_DEVICE(PCI_VENDOR_ID_CDNS, PCI_DEVICE_ID_CDNS_USBSSP),
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index 6a8d1fefbc0d..504bdf13ea80 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -70,7 +70,8 @@ static void cdns_role_stop(struct cdns *cdns)
 static void cdns_exit_roles(struct cdns *cdns)
 {
 	cdns_role_stop(cdns);
-	cdns_drd_exit(cdns);
+	if (!cdns->no_drd)
+		cdns_drd_exit(cdns);
 }
 
 /**
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
index bca973b999a4..8c492fda924c 100644
--- a/drivers/usb/cdns3/core.h
+++ b/drivers/usb/cdns3/core.h
@@ -84,6 +84,9 @@ struct cdns3_platform_data {
  *                        value in CHICKEN_BITS_3 will be preserved.
  * @gadget_init: pointer to gadget initialization function
  * @host_init: pointer to host initialization function
+ * @no_drd: DRD register block is inaccessible. The controller is hardwired to
+ *          single role (host or device) or the logic for role switching is
+ *          missing.
  */
 struct cdns {
 	struct device			*dev;
@@ -124,6 +127,7 @@ struct cdns {
 	u32                             override_apb_timeout;
 	int (*gadget_init)(struct cdns *cdns);
 	int (*host_init)(struct cdns *cdns);
+	bool				no_drd;
 };
 
 int cdns_hw_role_switch(struct cdns *cdns);
diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
index 84fb38a5723a..f87cf85cb97a 100644
--- a/drivers/usb/cdns3/drd.c
+++ b/drivers/usb/cdns3/drd.c
@@ -87,6 +87,9 @@ int cdns_get_id(struct cdns *cdns)
 {
 	int id;
 
+	if (cdns->no_drd)
+		return 0;
+
 	id = readl(&cdns->otg_regs->sts) & OTGSTS_ID_VALUE;
 	dev_dbg(cdns->dev, "OTG ID: %d", id);
 
@@ -107,7 +110,7 @@ void cdns_clear_vbus(struct cdns *cdns)
 {
 	u32 reg;
 
-	if (cdns->version != CDNSP_CONTROLLER_V2)
+	if (cdns->version != CDNSP_CONTROLLER_V2 || cdns->no_drd)
 		return;
 
 	reg = readl(&cdns->otg_cdnsp_regs->override);
@@ -120,7 +123,7 @@ void cdns_set_vbus(struct cdns *cdns)
 {
 	u32 reg;
 
-	if (cdns->version != CDNSP_CONTROLLER_V2)
+	if (cdns->version != CDNSP_CONTROLLER_V2 || cdns->no_drd)
 		return;
 
 	reg = readl(&cdns->otg_cdnsp_regs->override);
@@ -181,6 +184,9 @@ int cdns_drd_host_on(struct cdns *cdns)
 	u32 val, ready_bit;
 	int ret;
 
+	if (cdns->no_drd)
+		goto phy_set;
+
 	/* Enable host mode. */
 	writel(OTGCMD_HOST_BUS_REQ | OTGCMD_OTG_DIS,
 	       &cdns->otg_regs->cmd);
@@ -197,6 +203,7 @@ int cdns_drd_host_on(struct cdns *cdns)
 	if (ret)
 		dev_err(cdns->dev, "timeout waiting for xhci_ready\n");
 
+phy_set:
 	phy_set_mode(cdns->usb2_phy, PHY_MODE_USB_HOST);
 	phy_set_mode(cdns->usb3_phy, PHY_MODE_USB_HOST);
 	return ret;
@@ -210,6 +217,9 @@ void cdns_drd_host_off(struct cdns *cdns)
 {
 	u32 val;
 
+	if (cdns->no_drd)
+		goto phy_set;
+
 	writel(OTGCMD_HOST_BUS_DROP | OTGCMD_DEV_BUS_DROP |
 	       OTGCMD_DEV_POWER_OFF | OTGCMD_HOST_POWER_OFF,
 	       &cdns->otg_regs->cmd);
@@ -218,6 +228,8 @@ void cdns_drd_host_off(struct cdns *cdns)
 	readl_poll_timeout_atomic(&cdns->otg_regs->state, val,
 				  !(val & OTGSTATE_HOST_STATE_MASK),
 				  1, 2000000);
+
+phy_set:
 	phy_set_mode(cdns->usb2_phy, PHY_MODE_INVALID);
 	phy_set_mode(cdns->usb3_phy, PHY_MODE_INVALID);
 }
@@ -234,6 +246,9 @@ int cdns_drd_gadget_on(struct cdns *cdns)
 	u32 ready_bit;
 	int ret, val;
 
+	if (cdns->no_drd)
+		goto phy_set;
+
 	/* switch OTG core */
 	writel(OTGCMD_DEV_BUS_REQ | reg, &cdns->otg_regs->cmd);
 
@@ -251,6 +266,7 @@ int cdns_drd_gadget_on(struct cdns *cdns)
 		return ret;
 	}
 
+phy_set:
 	phy_set_mode(cdns->usb2_phy, PHY_MODE_USB_DEVICE);
 	phy_set_mode(cdns->usb3_phy, PHY_MODE_USB_DEVICE);
 	return 0;
@@ -265,6 +281,9 @@ void cdns_drd_gadget_off(struct cdns *cdns)
 {
 	u32 val;
 
+	if (cdns->no_drd)
+		goto phy_set;
+
 	/*
 	 * Driver should wait at least 10us after disabling Device
 	 * before turning-off Device (DEV_BUS_DROP).
@@ -277,6 +296,8 @@ void cdns_drd_gadget_off(struct cdns *cdns)
 	readl_poll_timeout_atomic(&cdns->otg_regs->state, val,
 				  !(val & OTGSTATE_DEV_STATE_MASK),
 				  1, 2000000);
+
+phy_set:
 	phy_set_mode(cdns->usb2_phy, PHY_MODE_INVALID);
 	phy_set_mode(cdns->usb3_phy, PHY_MODE_INVALID);
 }
@@ -392,6 +413,19 @@ int cdns_drd_init(struct cdns *cdns)
 	u32 state, reg;
 	int ret;
 
+	if (cdns->no_drd) {
+		cdns->dr_mode = usb_get_dr_mode(cdns->dev);
+		cdns->version = CDNSP_CONTROLLER_V2;
+
+		if (cdns->dr_mode != USB_DR_MODE_HOST &&
+		    cdns->dr_mode != USB_DR_MODE_PERIPHERAL) {
+			dev_err(cdns->dev, "Incorrect dr_mode\n");
+			return -EINVAL;
+		}
+
+		return 0;
+	}
+
 	regs = devm_ioremap_resource(cdns->dev, &cdns->otg_res);
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
@@ -492,6 +526,9 @@ int cdns_drd_init(struct cdns *cdns)
 
 int cdns_drd_exit(struct cdns *cdns)
 {
+	if (cdns->no_drd)
+		return 0;
+
 	cdns_otg_disable_irq(cdns);
 
 	return 0;
@@ -500,6 +537,9 @@ int cdns_drd_exit(struct cdns *cdns)
 /* Indicate the cdns3 core was power lost before */
 bool cdns_power_is_lost(struct cdns *cdns)
 {
+	if (cdns->no_drd)
+		return false;
+
 	if (cdns->version == CDNS3_CONTROLLER_V0) {
 		if (!(readl(&cdns->otg_v0_regs->simulate) & BIT(0)))
 			return true;

-- 
2.43.0



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

* Re: [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property
  2026-05-13  8:41 ` [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property Pawel Laszczak via B4 Relay
@ 2026-05-13  9:36   ` Rob Herring (Arm)
  2026-05-14  2:57   ` sashiko-bot
  1 sibling, 0 replies; 6+ messages in thread
From: Rob Herring (Arm) @ 2026-05-13  9:36 UTC (permalink / raw)
  To: Pawel Laszczak
  Cc: Conor Dooley, Greg Kroah-Hartman, Roger Quadros, devicetree,
	linux-kernel, Peter Chen, linux-usb, Krzysztof Kozlowski


On Wed, 13 May 2026 10:41:21 +0200, Pawel Laszczak wrote:
> Introduce a new boolean property 'no_drd' for Cadence USBSS/USBSSP
> controllers to support hardware configurations where the Dual-Role
> Device (DRD) register block is missing or inaccessible.
> 
> When 'no_drd' is present:
> - The 'otg' register and interrupt resources are not required.
> - The 'reg' and 'interrupts' properties are restricted to 2 items
>   (host and device).
> - 'dr_mode' must be explicitly set to either 'host' or 'peripheral'.
> 
> When 'no_drd' is absent, the binding maintains backward compatibility
> by requiring all 3 resource sets (otg, host, dev).
> 
> To achieve this, the schema is updated with an if-then-else logic
> and 'reg-names'/'interrupt-names' use enums to allow flexible
> ordering during validation.
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
> v5:
> - Implemented strict conditional validation using if-then-else logic.
> - Enforced 2 register/interrupt items and required 'dr_mode'
>   (host or peripheral) when 'no_drd' is present.
> - Enforced the standard 3 register/interrupt items (otg, host, dev)
>   when 'no_drd' is absent to ensure backward compatibility.
> - Updated 'reg-names' and 'interrupt-names' to use enums in the main
>   properties section to support flexible resource ordering during
>   validation.
> ---
> ---
>  .../devicetree/bindings/usb/cdns,usb3.yaml         | 67 +++++++++++++++++++---
>  1 file changed, 58 insertions(+), 9 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.example.dtb: usb@5b110000 (fsl,imx8qm-usb3): usb@5b120000:interrupt-names: ['host', 'peripheral', 'otg', 'wakeup'] is too long
	from schema $id: http://devicetree.org/schemas/usb/fsl,imx8qm-cdns3.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/fsl,imx8qm-cdns3.example.dtb: usb@5b120000 (cdns,usb3): interrupt-names: ['host', 'peripheral', 'otg', 'wakeup'] is too long
	from schema $id: http://devicetree.org/schemas/usb/cdns,usb3.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260513-b4-no_drd_config-v5-1-1c633a4b9a29@cadence.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


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

* Re: [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property
  2026-05-13  8:41 ` [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property Pawel Laszczak via B4 Relay
  2026-05-13  9:36   ` Rob Herring (Arm)
@ 2026-05-14  2:57   ` sashiko-bot
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-05-14  2:57 UTC (permalink / raw)
  To: Pawel Laszczak via B4 Relay; +Cc: conor+dt, krzk+dt, robh, devicetree

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Lost functionality: The optional 'wakeup' interrupt is incorrectly forbidden when 'no_drd' is true.
- [Medium] Devicetree naming convention violation and architectural anti-pattern for 'no_drd'.
- [Medium] Logic contradiction: Schema modifications enforce strict ordering, completely preventing the claimed 'flexible ordering'.
- [Medium] Schema documentation and indexing mismatch for 'reg' and 'interrupts' descriptions.
- [Medium] Validation weakening: The fallback 'else' block inadvertently drops the strict constraint for the 4th optional 'wakeup' interrupt.
--

commit d5f1668911e2cd9ebdc79647d53b3d88019eb9ca
Author: Pawel Laszczak <pawell@cadence.com>

dt-bindings: usb: cdns3: Add no_drd property

Introduce a new boolean property 'no_drd' for Cadence USBSS/USBSSP
controllers to support hardware configurations where the Dual-Role Device (DRD)
register block is missing or inaccessible. The schema is updated with an
if-then-else logic and enums for register and interrupt names.

>    When 'no_drd' is absent, the binding maintains backward compatibility
>    by requiring all 3 resource sets (otg, host, dev).
>    
>    To achieve this, the schema is updated with an if-then-else logic
>    and 'reg-names'/'interrupt-names' use enums to allow flexible
>    ordering during validation.

Does the new if-then-else block actually allow flexible ordering? The
added logic redefines the 'items' arrays as exact sequences (e.g., using
const for each index), which enforces strict tuple validation per index.
This seems to require elements to exactly match the defined order, preventing
the flexibility described here.

> diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> index 2d95fb7321aff..5d9bea62729c1 100644
> --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> @@ -20,19 +20,21 @@ properties:
>      const: cdns,usb3
>  
>    reg:
> +    minItems: 2
>      items:
>        - description: OTG controller registers
>        - description: XHCI Host controller registers
>        - description: DEVICE controller registers

Since this base schema strictly defines index 0 of 'reg' as OTG controller
registers, will this cause issues when 'no_drd' is true? In that case, the
OTG block is omitted, forcing 'reg-names' index 0 to be 'xhci' and index 1
to be 'dev'.

Could this cause validation tooling and generated documentation to incorrectly
document and label the 'xhci' register as 'OTG controller registers'?

[ ... ]

> @@ -79,6 +81,13 @@ properties:
>      description: Enable resetting of PHY if Rx fail is detected
>      type: boolean
>  
> +  no_drd:
> +    description:
> +      Indicates that the Dual-Role Device (DRD) register block is not
> +      implemented or is inaccessible. In this case, the controller
> +      must operate in a fixed peripheral or host mode.
> +    type: boolean

Standard devicetree property names normally use hyphens instead of underscores.
Should this be named 'cdns,no-drd' to include the vendor prefix and prevent
namespace collisions?

Additionally, is using a negative boolean property to denote missing hardware
the best approach here? Devicetree typically describes present hardware, so
missing blocks are often inferred from property lengths or described via a
distinct compatible string.

[ ... ]

> @@ -93,6 +102,46 @@ allOf:
>    - $ref: usb-drd.yaml#
>    - $ref: usb-xhci.yaml#
>  
> +  - if:
> +      properties:
> +        no_drd: true
> +      required:
> +        - no_drd
> +    then:
> +      required:
> +        - dr_mode
> +      properties:
> +        reg:
> +          maxItems: 2
> +        reg-names:
> +          items:
> +            - const: xhci
> +            - const: dev
> +        interrupts:
> +          maxItems: 2
> +        interrupt-names:
> +          items:
> +            - const: host
> +            - const: peripheral

The original binding allows an optional 4th interrupt named 'wakeup', which
is cleared by the xhci core.

Since this conditional block explicitly restricts 'interrupts' to maxItems: 2,
will hardware configurations that utilize the 'wakeup' line fail schema
validation when operating without the DRD block?

> +        dr_mode:
> +          enum: [host, peripheral]
> +    else:
> +      properties:
> +        reg:
> +          minItems: 3
> +        reg-names:
> +          items:
> +            - const: otg
> +            - const: xhci
> +            - const: dev
> +        interrupts:
> +          minItems: 3
> +        interrupt-names:
> +          items:
> +            - const: host
> +            - const: peripheral
> +            - const: otg

The base schema now allows the interrupt-names elements to be any of the four
enums. Because this 'else' block only overrides the first three items with
'const' constraints and omits the 4th item entirely, does the 4th interrupt
fall back to the generic enum?

Could this inadvertently allow duplicate invalid configurations (like
[host, peripheral, otg, host]) instead of strictly requiring 'wakeup' as the
original schema did?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-b4-no_drd_config-v5-0-1c633a4b9a29@cadence.com?part=1

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

* Re: [PATCH v5 2/2] usb: cdnsp: Add support for device-only configuration
  2026-05-13  8:41 ` [PATCH v5 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay
@ 2026-05-14  3:27   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-05-14  3:27 UTC (permalink / raw)
  To: Pawel Laszczak via B4 Relay; +Cc: conor+dt, robh, devicetree, krzk+dt

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized variable `ret` is returned in `cdns_drd_host_on()` when `cdns->no_drd` is true.
- [High] In `cdnsp_pci_probe()`, a failed `platform_device_register_full()` leads to a Use-After-Free and potential Double Free via a dangling pointer in the sibling PCI device's `drvdata`.
--

commit 3bccaf516e61eb4462bbd76ec7e121917dbcc00b
Author: Pawel Laszczak <pawell@cadence.com>
usb: cdnsp: Add support for device-only configuration

This patch introduces support for the Cadence USBSSP controller in hardware
configurations where the Dual-Role Device (DRD) register block is not
implemented. In these configurations, the driver operates exclusively in a
fixed mode and uses 32-bit addressing instead of 64-bit addressing.

> diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
> --- a/drivers/usb/cdns3/cdnsp-pci.c
> +++ b/drivers/usb/cdns3/cdnsp-pci.c
[ ... ]
> @@ -158,6 +182,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
>  		plat_info.dma_mask = pdev->dma_mask;
>  		plat_info.data = &pdata;
>  		plat_info.size_data = sizeof(pdata);
> +		plat_info.properties = wrap->prop;
>  		wrap->devfn = pdev->devfn;
>  		/* register platform device */
>  		wrap->plat_dev = platform_device_register_full(&plat_info);

This is an existing issue in the code modified here, but if
platform_device_register_full() fails just below this hunk:

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

When the second PCI function is probed and pci_is_enabled(func) is true,
wrap is retrieved from the first function's drvdata. Does calling kfree(wrap)
in this error path leave a dangling pointer in the first device's drvdata?
If so, could this lead to a use-after-free and a double free later when
cdnsp_pci_remove() is called?

> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> --- a/drivers/usb/cdns3/drd.c
> +++ b/drivers/usb/cdns3/drd.c
[ ... ]
> @@ -181,6 +184,9 @@ int cdns_drd_host_on(struct cdns *cdns)
>  	u32 val, ready_bit;
>  	int ret;
>  
> +	if (cdns->no_drd)
> +		goto phy_set;
> +

When cdns->no_drd is true, this jumps to phy_set, bypassing the assignment
to ret. Since ret is declared without initialization, could returning it at
the end of the function propagate a non-zero garbage value? This might cause
spurious host mode initialization failures.

>  	/* Enable host mode. */
>  	writel(OTGCMD_HOST_BUS_REQ | OTGCMD_OTG_DIS,
>  	       &cdns->otg_regs->cmd);
[ ... ]
> @@ -197,6 +203,7 @@ int cdns_drd_host_on(struct cdns *cdns)
>  	if (ret)
>  		dev_err(cdns->dev, "timeout waiting for xhci_ready\n");
>  
> +phy_set:
>  	phy_set_mode(cdns->usb2_phy, PHY_MODE_USB_HOST);
>  	phy_set_mode(cdns->usb3_phy, PHY_MODE_USB_HOST);
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-b4-no_drd_config-v5-0-1c633a4b9a29@cadence.com?part=2

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

end of thread, other threads:[~2026-05-14  3:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13  8:41 [PATCH v5 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay
2026-05-13  8:41 ` [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property Pawel Laszczak via B4 Relay
2026-05-13  9:36   ` Rob Herring (Arm)
2026-05-14  2:57   ` sashiko-bot
2026-05-13  8:41 ` [PATCH v5 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay
2026-05-14  3:27   ` sashiko-bot

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