* Re: [PATCH v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string
2026-05-15 11:24 ` [PATCH v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string Pawel Laszczak via B4 Relay
@ 2026-05-15 11:21 ` sashiko-bot
2026-05-15 17:12 ` Conor Dooley
1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-05-15 11:21 UTC (permalink / raw)
To: Pawel Laszczak via B4 Relay; +Cc: krzk+dt, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The 'dr_mode' property is not explicitly enforced as required for 'cdns,cdnsp-no-drd', contradicting the commit message.
- [Medium] Missing `minItems` and `maxItems` limits in the conditional blocks allow incomplete or over-specified property arrays to silently pass validation.
--
commit 1fd95ddfc77fad408e99901f5ed7387c21b64dfd
Author: Pawel Laszczak <pawell@cadence.com>
dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string
Introduce a new compatible string cdns,cdnsp-no-drd for controllers
missing the DRD register block. This restricts register and interrupt
resources to 2 items and specifies dr_mode requirements. The standard
cdns,usb3 maintains its existing 3-resource requirement.
> diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
[ ... ]
> @@ -93,6 +93,46 @@ allOf:
> - $ref: usb-drd.yaml#
> - $ref: usb-xhci.yaml#
>
> + - if:
> + properties:
> + compatible:
> + const: cdns,cdnsp-no-drd
> + then:
> + properties:
> + reg:
> + items:
> + - description: XHCI Host controller registers
> + - description: DEVICE controller registers
The top-level schema allows up to 3 items for reg. Since providing an items
list doesn't implicitly restrict the overall array length in dt-schema,
could a node with 3 registers incorrectly pass validation here?
Should maxItems: 2 be explicitly added to reg and reg-names in this block?
> + reg-names:
> + items:
> + - const: xhci
> + - const: dev
> + interrupts:
> + maxItems: 2
> + interrupt-names:
> + items:
> + - const: host
> + - const: peripheral
Similarly, could interrupt-names also pass validation with 3 items because
the top-level allows it? Should maxItems: 2 be added here as well?
> + dr_mode:
> + enum: [host, peripheral]
The commit message states that dr_mode must be explicitly set to either host
or peripheral. Does the schema enforce this requirement?
Since dr_mode is only constrained by an enum and not included in a required
list, a device tree omitting the property entirely will still pass
validation. This allows an invalid configuration where the USB core might
fall back to the default otg mode on hardware that lacks DRD capabilities.
Should a required list containing dr_mode be added to this block?
> + else:
> + properties:
> + reg:
> + items:
> + - description: OTG controller registers
> + - description: XHCI Host controller registers
> + - description: DEVICE controller registers
> + reg-names:
> + items:
> + - const: otg
> + - const: xhci
> + - const: dev
The top-level schema was relaxed to allow a minimum of 2 items for reg and
reg-names. Without minItems: 3 specified here, could a malformed legacy
cdns,usb3 node with only 2 registers pass validation?
Should minItems: 3 be added to reg and reg-names in this else block to
maintain backward compatibility?
> + interrupts:
> + minItems: 3
> + maxItems: 4
> + interrupt-names:
> + minItems: 3
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-b4-no_drd_config-v8-0-75cb05317679@cadence.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v8 0/2] usb: cdns3: support configurations without DRD block
@ 2026-05-15 11:24 Pawel Laszczak via B4 Relay
2026-05-15 11:24 ` [PATCH v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string Pawel Laszczak via B4 Relay
2026-05-15 11:24 ` [PATCH v8 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-15 11:24 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, kernel test robot
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 'cdns,cdnsp-no-drd' compatible string
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 v9 as soon as Peter's changes land in
linux-next if required.
---
v8:
- Update commit message to reflect schema changes.
- Removed 'cdns,no-drd' boolean property as per Rob Herring's suggestion.
- Introduced a new compatible string 'cdns,cdnsp-no-drd' for controller
variants that lack the DRD/OTG register block.
- Use 'cdns,cdnsp-no-drd' compatible string to trigger no-DRD logic.
- Update PCI glue layer to inject the new compatible string.
v7:
- Rename 'no_drd' to 'cdns,no-drd' (added vendor prefix).
- Update commit message to reflect property renaming and schema changes.
- Simplify 'reg-names' using a single enum for items.
- Revert 'interrupt-names' to a list of constants for strict ordering.
- Move 'reg' item descriptions to if/else blocks for better accuracy.
- Clean up 'if' logic (remove redundant 'true' check and 'dr_mode'
requirement).
- Add explicit 'items' list for 'interrupt-names' in the 'else' block.
- Fix "uninitialized variable" warning in cdns_drd_host_on() by
initializing 'ret' to 0.
v6:
- Fixed validation error for 'interrupt-names' by correcting
the items definition.
- Adjusted 'minItems'/'maxItems' to properly support the optional
'wakeup' interrupt.
- Fixed 'too long' schema error in examples.
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 cdns,cdnsp-no-drd compatible string
usb: cdnsp: Add support for device-only configuration
.../devicetree/bindings/usb/cdns,usb3.yaml | 60 ++++++++++++++++++----
drivers/usb/cdns3/cdns3-plat.c | 30 +++++++----
drivers/usb/cdns3/cdnsp-pci.c | 48 +++++++++++++----
drivers/usb/cdns3/core.c | 3 +-
drivers/usb/cdns3/core.h | 4 ++
drivers/usb/cdns3/drd.c | 45 ++++++++++++++--
6 files changed, 156 insertions(+), 34 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 v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string
2026-05-15 11:24 [PATCH v8 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay
@ 2026-05-15 11:24 ` Pawel Laszczak via B4 Relay
2026-05-15 11:21 ` sashiko-bot
2026-05-15 17:12 ` Conor Dooley
2026-05-15 11:24 ` [PATCH v8 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-15 11:24 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 compatible string 'cdns,cdnsp-no-drd' for Cadence
USBSS/USBSSP controllers to support hardware configurations where the
Dual-Role Device (DRD) register block is missing or inaccessible.
This change follows the recommendation to imply hardware limitations
directly from the compatible string rather than using a dedicated
boolean property.
When 'cdns,cdnsp-no-drd' is used:
- 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'.
The standard 'cdns,usb3' compatible remains unchanged, maintaining
backward compatibility by requiring all 3 resource sets (otg, host, dev).
Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
v8:
- Update commit message to reflect schema changes.
- Removed 'cdns,no-drd' boolean property as per Rob Herring's suggestion.
- Introduced a new compatible string 'cdns,cdnsp-no-drd' for controller
variants that lack the DRD/OTG register block.
v7:
- Rename 'no_drd' to 'cdns,no-drd'.
- Update commit message to reflect property renaming and schema changes.
- Simplify 'reg-names' using a single enum.
- Revert 'interrupt-names' to a list of constants.
- Move 'reg' item descriptions to if/else blocks for accuracy.
- Clean up 'if/then' logic (remove redundant checks).
- Add explicit 'items' list for 'interrupt-names' in the 'else' block.
v6:
- Fixed validation error for 'interrupt-names' by correcting
the items definition.
- Adjusted 'minItems'/'maxItems' to properly support the optional
'wakeup' interrupt.
- Fixed 'too long' schema error in examples.
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 | 60 ++++++++++++++++++----
1 file changed, 50 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
index 2d95fb7321af..7b0aa9c4a2bd 100644
--- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
+++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
@@ -17,22 +17,22 @@ description:
properties:
compatible:
- const: cdns,usb3
+ enum:
+ - cdns,usb3
+ - cdns,cdnsp-no-drd
reg:
- items:
- - description: OTG controller registers
- - description: XHCI Host controller registers
- - description: DEVICE controller registers
+ minItems: 2
+ maxItems: 3
reg-names:
+ minItems: 2
+ maxItems: 3
items:
- - const: otg
- - const: xhci
- - const: dev
+ enum: [ otg, xhci, dev ]
interrupts:
- minItems: 3
+ minItems: 2
items:
- description: XHCI host controller interrupt
- description: Device controller interrupt
@@ -41,7 +41,7 @@ properties:
cleared by xhci core, this interrupt is optional
interrupt-names:
- minItems: 3
+ minItems: 2
items:
- const: host
- const: peripheral
@@ -93,6 +93,46 @@ allOf:
- $ref: usb-drd.yaml#
- $ref: usb-xhci.yaml#
+ - if:
+ properties:
+ compatible:
+ const: cdns,cdnsp-no-drd
+ then:
+ properties:
+ reg:
+ items:
+ - description: XHCI Host controller registers
+ - description: DEVICE controller registers
+ 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:
+ items:
+ - description: OTG controller registers
+ - description: XHCI Host controller registers
+ - description: DEVICE controller registers
+ reg-names:
+ items:
+ - const: otg
+ - const: xhci
+ - const: dev
+ interrupts:
+ minItems: 3
+ maxItems: 4
+ interrupt-names:
+ minItems: 3
+
unevaluatedProperties: false
examples:
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v8 2/2] usb: cdnsp: Add support for device-only configuration
2026-05-15 11:24 [PATCH v8 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay
2026-05-15 11:24 ` [PATCH v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string Pawel Laszczak via B4 Relay
@ 2026-05-15 11:24 ` Pawel Laszczak via B4 Relay
2026-05-15 11:54 ` sashiko-bot
1 sibling, 1 reply; 6+ messages in thread
From: Pawel Laszczak via B4 Relay @ 2026-05-15 11:24 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, kernel test robot
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>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202605141023.18vWXyw3-lkp@intel.com/
---
v8:
- Use 'cdns,cdnsp-no-drd' compatible string to trigger no-DRD logic.
- Update PCI glue layer to inject the new compatible string.
v7:
- Use 'cdns,no-drd' instead of 'no_drd' to match updated bindings.
- Fix "uninitialized variable" warning in cdns_drd_host_on() by
initializing 'ret' to 0.
---
drivers/usb/cdns3/cdns3-plat.c | 30 ++++++++++++++++----------
drivers/usb/cdns3/cdnsp-pci.c | 48 ++++++++++++++++++++++++++++++++++--------
drivers/usb/cdns3/core.c | 3 ++-
drivers/usb/cdns3/core.h | 4 ++++
drivers/usb/cdns3/drd.c | 45 ++++++++++++++++++++++++++++++++++++---
5 files changed, 106 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
index 3fe3109a3688..303f7c1431dc 100644
--- a/drivers/usb/cdns3/cdns3-plat.c
+++ b/drivers/usb/cdns3/cdns3-plat.c
@@ -81,6 +81,12 @@ 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;
+ if (device_is_compatible(dev, "cdns,cdnsp-no-drd")) {
+ cdns->no_drd = true;
+ cdns->version = CDNSP_CONTROLLER_V2;
+ dev_dbg(dev, "No DRD support\n");
+ }
+
platform_set_drvdata(pdev, cdns);
ret = platform_get_irq_byname(pdev, "host");
@@ -113,21 +119,22 @@ 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;
}
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;
@@ -338,6 +345,7 @@ 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,cdnsp-no-drd" },
{ },
};
MODULE_DEVICE_TABLE(of, of_cdns3_match);
diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
index 432007cfe695..1a71dee5044c 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,21 @@ 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("compatible",
+ "cdns,cdnsp-no-drd");
+ wrap->prop[idx++] = PROPERTY_ENTRY_STRING("dr_mode", "peripheral");
+ } 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 +183,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 +211,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..d2bb682e4552 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);
@@ -179,7 +182,10 @@ static void cdns_otg_enable_irq(struct cdns *cdns)
int cdns_drd_host_on(struct cdns *cdns)
{
u32 val, ready_bit;
- int ret;
+ int ret = 0;
+
+ if (cdns->no_drd)
+ goto phy_set;
/* Enable host mode. */
writel(OTGCMD_HOST_BUS_REQ | OTGCMD_OTG_DIS,
@@ -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,18 @@ 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);
+
+ 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 +525,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 +536,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 v8 2/2] usb: cdnsp: Add support for device-only configuration
2026-05-15 11:24 ` [PATCH v8 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay
@ 2026-05-15 11:54 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-05-15 11:54 UTC (permalink / raw)
To: Pawel Laszczak via B4 Relay; +Cc: krzk+dt, robh, conor+dt, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] In `cdnsp_pci_probe()`, the error path unconditionally frees the shared `wrap` structure if `platform_device_register_full()` fails, causing a use-after-free and double-free vulnerability.
--
commit 4dafbda57ec59e424fb1db39575dac6fe3174dd5
Author: Pawel Laszczak <pawell@cadence.com>
usb: cdnsp: Add support for device-only configuration
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.
Closes: https://lore.kernel.org/oe-kbuild-all/202605141023.18vWXyw3-lkp@intel.com/
> diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
> index 432007cfe6954..1a71dee5044cc 100644
> --- a/drivers/usb/cdns3/cdnsp-pci.c
> +++ b/drivers/usb/cdns3/cdnsp-pci.c
> @@ -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);
[ ... ]
> @@ -158,6 +183,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 might be a pre-existing issue rather than a new regression, but is there
a use-after-free risk in the error handling immediately following this
platform device registration?
If probing a dual-function device, the first PCI function allocates the
shared wrap structure, stores it in its drvdata, and completes its probe.
When the second PCI function probes, it retrieves the shared wrap from the
first function and calls platform_device_register_full().
If that registration fails:
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;
}
Does the unconditional kfree(wrap) here free the memory that the first PCI
function's drvdata still points to?
If so, when the first function is later unbound, cdnsp_pci_remove() might
retrieve the dangling wrap pointer and eventually call kfree(wrap) again,
causing a double-free. Should the error path only free wrap if it was
allocated exclusively during the current probe execution?
> @@ -185,13 +211,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);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-b4-no_drd_config-v8-0-75cb05317679@cadence.com?part=2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string
2026-05-15 11:24 ` [PATCH v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string Pawel Laszczak via B4 Relay
2026-05-15 11:21 ` sashiko-bot
@ 2026-05-15 17:12 ` Conor Dooley
1 sibling, 0 replies; 6+ messages in thread
From: Conor Dooley @ 2026-05-15 17:12 UTC (permalink / raw)
To: pawell
Cc: Peter Chen, Roger Quadros, Greg Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-usb, devicetree,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5372 bytes --]
On Fri, May 15, 2026 at 01:24:38PM +0200, Pawel Laszczak via B4 Relay wrote:
> From: Pawel Laszczak <pawell@cadence.com>
>
> Introduce a new compatible string 'cdns,cdnsp-no-drd' for Cadence
> USBSS/USBSSP controllers to support hardware configurations where the
> Dual-Role Device (DRD) register block is missing or inaccessible.
>
> This change follows the recommendation to imply hardware limitations
> directly from the compatible string rather than using a dedicated
> boolean property.
>
> When 'cdns,cdnsp-no-drd' is used:
> - 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'.
>
> The standard 'cdns,usb3' compatible remains unchanged, maintaining
> backward compatibility by requiring all 3 resource sets (otg, host, dev).
>
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
> v8:
> - Update commit message to reflect schema changes.
> - Removed 'cdns,no-drd' boolean property as per Rob Herring's suggestion.
> - Introduced a new compatible string 'cdns,cdnsp-no-drd' for controller
> variants that lack the DRD/OTG register block.
>
> v7:
> - Rename 'no_drd' to 'cdns,no-drd'.
> - Update commit message to reflect property renaming and schema changes.
> - Simplify 'reg-names' using a single enum.
> - Revert 'interrupt-names' to a list of constants.
> - Move 'reg' item descriptions to if/else blocks for accuracy.
> - Clean up 'if/then' logic (remove redundant checks).
> - Add explicit 'items' list for 'interrupt-names' in the 'else' block.
>
> v6:
> - Fixed validation error for 'interrupt-names' by correcting
> the items definition.
> - Adjusted 'minItems'/'maxItems' to properly support the optional
> 'wakeup' interrupt.
> - Fixed 'too long' schema error in examples.
>
> 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 | 60 ++++++++++++++++++----
> 1 file changed, 50 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> index 2d95fb7321af..7b0aa9c4a2bd 100644
> --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml
> @@ -17,22 +17,22 @@ description:
>
> properties:
> compatible:
> - const: cdns,usb3
> + enum:
> + - cdns,usb3
> + - cdns,cdnsp-no-drd
I doubt this is what Rob meant, he asked for soc-specific compatibles
and this is generic. A soc-specific compatible would be something like
microchip,newsoc-usb3
Thanks,
Conor.
>
> reg:
> - items:
> - - description: OTG controller registers
> - - description: XHCI Host controller registers
> - - description: DEVICE controller registers
> + minItems: 2
> + maxItems: 3
>
> reg-names:
> + minItems: 2
> + maxItems: 3
> items:
> - - const: otg
> - - const: xhci
> - - const: dev
> + enum: [ otg, xhci, dev ]
>
> interrupts:
> - minItems: 3
> + minItems: 2
> items:
> - description: XHCI host controller interrupt
> - description: Device controller interrupt
> @@ -41,7 +41,7 @@ properties:
> cleared by xhci core, this interrupt is optional
>
> interrupt-names:
> - minItems: 3
> + minItems: 2
> items:
> - const: host
> - const: peripheral
> @@ -93,6 +93,46 @@ allOf:
> - $ref: usb-drd.yaml#
> - $ref: usb-xhci.yaml#
>
> + - if:
> + properties:
> + compatible:
> + const: cdns,cdnsp-no-drd
> + then:
> + properties:
> + reg:
> + items:
> + - description: XHCI Host controller registers
> + - description: DEVICE controller registers
> + 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:
> + items:
> + - description: OTG controller registers
> + - description: XHCI Host controller registers
> + - description: DEVICE controller registers
> + reg-names:
> + items:
> + - const: otg
> + - const: xhci
> + - const: dev
> + interrupts:
> + minItems: 3
> + maxItems: 4
> + interrupt-names:
> + minItems: 3
> +
> unevaluatedProperties: false
>
> examples:
>
> --
> 2.43.0
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-15 17:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-15 11:24 [PATCH v8 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay
2026-05-15 11:24 ` [PATCH v8 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp-no-drd compatible string Pawel Laszczak via B4 Relay
2026-05-15 11:21 ` sashiko-bot
2026-05-15 17:12 ` Conor Dooley
2026-05-15 11:24 ` [PATCH v8 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay
2026-05-15 11:54 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox