* [PATCH v5 0/2] usb: cdns3: support configurations without DRD block
@ 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay
0 siblings, 0 replies; 9+ messages in thread
From: Pawel Laszczak @ 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] 9+ messages in thread* [PATCH v5 0/2] usb: cdns3: support configurations without DRD block @ 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay 0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay @ 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay -1 siblings, 0 replies; 9+ messages in thread From: Pawel Laszczak @ 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 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] 9+ messages in thread
* [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property @ 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay 0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay (?) @ 2026-05-13 9:36 ` Rob Herring (Arm) -1 siblings, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: usb: cdns3: Add no_drd property 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay (?) (?) @ 2026-05-14 2:57 ` sashiko-bot -1 siblings, 0 replies; 9+ 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] 9+ messages in thread
* [PATCH v5 2/2] usb: cdnsp: Add support for device-only configuration 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay @ 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay -1 siblings, 0 replies; 9+ messages in thread From: Pawel Laszczak @ 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 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] 9+ messages in thread
* [PATCH v5 2/2] usb: cdnsp: Add support for device-only configuration @ 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay 0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH v5 2/2] usb: cdnsp: Add support for device-only configuration 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay (?) @ 2026-05-14 3:27 ` sashiko-bot -1 siblings, 0 replies; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2026-05-14 3:27 UTC | newest] Thread overview: 9+ 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 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 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 2026-05-13 8:41 ` Pawel Laszczak via B4 Relay 2026-05-14 3:27 ` sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.