* [PATCH v9 0/2] usb: cdns3: support configurations without DRD block
@ 2026-05-21 8:16 Pawel Laszczak via B4 Relay
2026-05-21 8:16 ` [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string Pawel Laszczak via B4 Relay
2026-05-21 8:16 ` [PATCH v9 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay
0 siblings, 2 replies; 7+ messages in thread
From: Pawel Laszczak via B4 Relay @ 2026-05-21 8:16 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' 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 v10 as soon as Peter's changes land in
linux-next if required.
---
v9:
- Dropped the "cdns,cdnsp-no-drd" string to avoid generic property-like
naming as requested by Conor.
- Introduced the clean generic fallback "cdns,cdnsp"
- Updated the patch description and commit message to reflect the new naming.
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 compatible string
usb: cdnsp: Add support for device-only configuration
.../devicetree/bindings/usb/cdns,usb3.yaml | 63 ++++++++++++++++++----
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, 159 insertions(+), 34 deletions(-)
---
base-commit: 80dd246accce631c328ea43294e53b2b2dd2aa32
change-id: 20260519-no_drd_config_v9-b915baa43eb0
Best regards,
--
Pawel Laszczak <pawell@cadence.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string 2026-05-21 8:16 [PATCH v9 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay @ 2026-05-21 8:16 ` Pawel Laszczak via B4 Relay 2026-05-21 8:16 ` sashiko-bot 2026-05-21 19:59 ` Conor Dooley 2026-05-21 8:16 ` [PATCH v9 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay 1 sibling, 2 replies; 7+ messages in thread From: Pawel Laszczak via B4 Relay @ 2026-05-21 8:16 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 generic fallback compatible string 'cdns,cdnsp' for Cadence USBSSP controllers to support hardware configurations where the Dual-Role Device (DRD) register block is missing or inaccessible. Following the maintainer's feedback, avoid generic property-like naming (such as "-no-drd") and use a clean generic fallback. To keep the schema resource-driven and strictly validated, define a two-string compatible matrix using an empty schema ({}) wildcard. This allows future vendor SoC compatibles to be prepended while safely falling back to the 2-resource USBSSP configuration. When 'cdns,cdnsp' is matched: - 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> --- v9: - Dropped the "cdns,cdnsp-no-drd" string to avoid generic property-like naming as requested by Conor. - Introduced the clean generic fallback "cdns,cdnsp". 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 | 63 ++++++++++++++++++---- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml index 2d95fb7321af..e8082c5c05a2 100644 --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml @@ -17,22 +17,24 @@ description: properties: compatible: - const: cdns,usb3 + oneOf: + - const: cdns,usb3 + - items: + - {} + - const: cdns,cdnsp 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 +43,7 @@ properties: cleared by xhci core, this interrupt is optional interrupt-names: - minItems: 3 + minItems: 2 items: - const: host - const: peripheral @@ -93,6 +95,47 @@ allOf: - $ref: usb-drd.yaml# - $ref: usb-xhci.yaml# + - if: + properties: + compatible: + contains: + const: cdns,cdnsp + 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] 7+ messages in thread
* Re: [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string 2026-05-21 8:16 ` [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string Pawel Laszczak via B4 Relay @ 2026-05-21 8:16 ` sashiko-bot 2026-05-21 20:03 ` Conor Dooley 2026-05-21 19:59 ` Conor Dooley 1 sibling, 1 reply; 7+ messages in thread From: sashiko-bot @ 2026-05-21 8:16 UTC (permalink / raw) To: Pawel Laszczak via B4 Relay; +Cc: conor+dt, devicetree, robh Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] The schema does not enforce the presence of the `dr_mode` property when `cdns,cdnsp` is matched, despite the commit message explicitly requiring it. - [Medium] Array bounds (`minItems` and `maxItems`) for `reg`, `reg-names`, and `interrupt-names` are not explicitly constrained inside the conditionals. - [Medium] Use of the empty schema `{}` as a wildcard for compatible strings breaks dt-schema validation and explicit documentation rules. -- commit 1f787c873a0b8a21200340bba7dffd95f1916193 Author: Pawel Laszczak <pawell@cadence.com> dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string Introduce a new generic fallback compatible string cdns,cdnsp for Cadence USBSSP controllers without the Dual-Role Device (DRD) register block. It restricts the resources to host and device configurations, requiring dr_mode to be explicitly set. > 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 > @@ -17,22 +17,24 @@ description: > > properties: > compatible: > - const: cdns,usb3 > + oneOf: > + - const: cdns,usb3 > + - items: > + - {} [Severity: Medium] The commit message mentions using an empty schema wildcard {} to allow future vendor SoC compatibles to be prepended. Does this break dt-schema meta-validation during dt_binding_check? The tooling usually expects concrete compatible strings to be explicitly documented via an enum rather than using a wildcard. > + - const: cdns,cdnsp [ ... ] > + - if: > + properties: > + compatible: > + contains: > + const: cdns,cdnsp > + then: > + properties: > + reg: > + items: > + - description: XHCI Host controller registers > + - description: DEVICE controller registers > + reg-names: > + items: > + - const: xhci > + - const: dev [Severity: Medium] In dt-schema, specifying an items list inside conditional branches does not automatically override the top-level minItems and maxItems length constraints. Since the top-level schema allows 2 to 3 items, would omitting maxItems: 2 for reg and reg-names in this 'then' branch allow a 3-item configuration to incorrectly pass validation? Similarly, does omitting minItems: 3 in the 'else' branch allow a 2-item configuration to pass for cdns,usb3? > + interrupts: > + maxItems: 2 > + interrupt-names: > + items: > + - const: host > + - const: peripheral > + dr_mode: > + enum: [host, peripheral] [Severity: Medium] The commit message states that dr_mode must be explicitly set to either 'host' or 'peripheral' when this compatible is matched. Should dr_mode be added to a required list in this block? Without it, the property remains optional, and device trees missing dr_mode could silently pass validation. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260521-no_drd_config_v9-v9-0-2512cef10104@cadence.com?part=1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string 2026-05-21 8:16 ` sashiko-bot @ 2026-05-21 20:03 ` Conor Dooley 0 siblings, 0 replies; 7+ messages in thread From: Conor Dooley @ 2026-05-21 20:03 UTC (permalink / raw) To: sashiko-reviews; +Cc: Pawel Laszczak via B4 Relay, conor+dt, devicetree, robh [-- Attachment #1: Type: text/plain, Size: 3630 bytes --] On Thu, May 21, 2026 at 08:16:51AM +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: > - [Medium] The schema does not enforce the presence of the `dr_mode` property when `cdns,cdnsp` is matched, despite the commit message explicitly requiring it. > - [Medium] Array bounds (`minItems` and `maxItems`) for `reg`, `reg-names`, and `interrupt-names` are not explicitly constrained inside the conditionals. > - [Medium] Use of the empty schema `{}` as a wildcard for compatible strings breaks dt-schema validation and explicit documentation rules. > -- > > commit 1f787c873a0b8a21200340bba7dffd95f1916193 > Author: Pawel Laszczak <pawell@cadence.com> > > dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string > > Introduce a new generic fallback compatible string cdns,cdnsp for > Cadence USBSSP controllers without the Dual-Role Device (DRD) register > block. It restricts the resources to host and device configurations, > requiring dr_mode to be explicitly set. > > > 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 > > @@ -17,22 +17,24 @@ description: > > > > properties: > > compatible: > > - const: cdns,usb3 > > + oneOf: > > + - const: cdns,usb3 > > + - items: > > + - {} > > [Severity: Medium] > The commit message mentions using an empty schema wildcard {} to allow > future vendor SoC compatibles to be prepended. Does this break dt-schema > meta-validation during dt_binding_check? The tooling usually expects > concrete compatible strings to be explicitly documented via an enum rather > than using a wildcard. Obviously this is intentional. > > + - const: cdns,cdnsp > > [ ... ] > > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: cdns,cdnsp > > + then: > > + properties: > > + reg: > > + items: > > + - description: XHCI Host controller registers > > + - description: DEVICE controller registers > > + reg-names: > > + items: > > + - const: xhci > > + - const: dev > > [Severity: Medium] > In dt-schema, specifying an items list inside conditional branches does not > automatically override the top-level minItems and maxItems length constraints. > > Since the top-level schema allows 2 to 3 items, would omitting maxItems: 2 > for reg and reg-names in this 'then' branch allow a 3-item configuration > to incorrectly pass validation? Similarly, does omitting minItems: 3 in the > 'else' branch allow a 2-item configuration to pass for cdns,usb3? This looks valid, easily fixed by adding the correct fooItems properties. > > > + interrupts: > > + maxItems: 2 > > + interrupt-names: > > + items: > > + - const: host > > + - const: peripheral > > + dr_mode: > > + enum: [host, peripheral] > > [Severity: Medium] > The commit message states that dr_mode must be explicitly set to either > 'host' or 'peripheral' when this compatible is matched. > > Should dr_mode be added to a required list in this block? Without it, the > property remains optional, and device trees missing dr_mode could silently > pass validation. This is a good point, because the default dr_mode is otg. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string 2026-05-21 8:16 ` [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string Pawel Laszczak via B4 Relay 2026-05-21 8:16 ` sashiko-bot @ 2026-05-21 19:59 ` Conor Dooley 1 sibling, 0 replies; 7+ messages in thread From: Conor Dooley @ 2026-05-21 19:59 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: 3824 bytes --] On Thu, May 21, 2026 at 10:16:23AM +0200, Pawel Laszczak via B4 Relay wrote: > From: Pawel Laszczak <pawell@cadence.com> > > Introduce a new generic fallback compatible string 'cdns,cdnsp' for > Cadence USBSSP controllers to support hardware configurations where > the Dual-Role Device (DRD) register block is missing or inaccessible. > > Following the maintainer's feedback, avoid generic property-like naming > (such as "-no-drd") and use a clean generic fallback. To keep the schema > resource-driven and strictly validated, define a two-string compatible > matrix using an empty schema ({}) wildcard. This allows future vendor > SoC compatibles to be prepended while safely falling back to the 2-resource > USBSSP configuration. > > When 'cdns,cdnsp' is matched: > - 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> > --- > v9: > - Dropped the "cdns,cdnsp-no-drd" string to avoid generic property-like > naming as requested by Conor. > - Introduced the clean generic fallback "cdns,cdnsp". > > 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 | 63 ++++++++++++++++++---- > 1 file changed, 53 insertions(+), 10 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml > index 2d95fb7321af..e8082c5c05a2 100644 > --- a/Documentation/devicetree/bindings/usb/cdns,usb3.yaml > +++ b/Documentation/devicetree/bindings/usb/cdns,usb3.yaml > @@ -17,22 +17,24 @@ description: > > properties: > compatible: > - const: cdns,usb3 > + oneOf: > + - const: cdns,usb3 > + - items: > + - {} > + - const: cdns,cdnsp I think this is reasonable now. If you respin again, could you add a comment here to the cdnsp case to explain what it represents, unless "cdnsp" will be really obvious to anyone who has this IP in their device? Otherwise, Acked-by: Conor Dooley <conor.dooley@microchip.com> pw-bot: not-applicable Cheers, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v9 2/2] usb: cdnsp: Add support for device-only configuration 2026-05-21 8:16 [PATCH v9 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay 2026-05-21 8:16 ` [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string Pawel Laszczak via B4 Relay @ 2026-05-21 8:16 ` Pawel Laszczak via B4 Relay 2026-05-21 9:04 ` sashiko-bot 1 sibling, 1 reply; 7+ messages in thread From: Pawel Laszczak via B4 Relay @ 2026-05-21 8:16 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/ --- v9: - Switch to the clean generic fallback "cdns,cdnsp" in driver and PCI glue. 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..0d29c3cf58f1 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")) { + 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" }, { }, }; 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..c38a3dc7111a 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"); + 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] 7+ messages in thread
* Re: [PATCH v9 2/2] usb: cdnsp: Add support for device-only configuration 2026-05-21 8:16 ` [PATCH v9 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay @ 2026-05-21 9:04 ` sashiko-bot 0 siblings, 0 replies; 7+ messages in thread From: sashiko-bot @ 2026-05-21 9:04 UTC (permalink / raw) To: Pawel Laszczak via B4 Relay; +Cc: robh, devicetree, conor+dt Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider: Pre-existing issues: - [High] The error path in `cdnsp_pci_probe` incorrectly frees the shared `wrap` structure, leading to a Use-After-Free and Double-Free when the sibling PCI function is subsequently removed. - [High] Concurrent probing of the two PCI functions can cause a race condition leading to a NULL pointer dereference of the `wrap` object. - [High] Missing cleanup of the registered USB role switch and hardware state in the `cdns3_plat_probe` error path. - [High] Failure to call `phy_exit()` for `usb2_phy` when `usb3_phy` acquisition fails in `cdns3_plat_probe()`. - [High] TOCTOU race in `cdns_role_start()` and `cdns_role_stop()` allows multiple concurrent starts or stops of a USB role. - [High] Unbinding the first PCI function disables its MMIO resources while the registered platform device remains active, causing system crashes on subsequent accesses. -- commit a6e4224829252614819c7a92327bc797dbce1637 Author: Pawel Laszczak <pawell@cadence.com> Subject: usb: cdnsp: Add support for device-only configuration This patch adds support for the Cadence USBSSP (cdnsp) controller in hardware configurations where the Dual-Role Device (DRD) register block is missing. It allows the driver to operate exclusively in a fixed peripheral or host mode without relying on DRD logic. It also adjusts BAR indexing for 32-bit addressing used in this layout. Link: Closes: https://lore.kernel.org/oe-kbuild-all/202605141023.18vWXyw3-lkp@intel.com/ > diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c > index 3fe3109a3688a..0d29c3cf58f11 100644 > --- a/drivers/usb/cdns3/cdns3-plat.c > +++ b/drivers/usb/cdns3/cdns3-plat.c [ ... ] > @@ -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; [Severity: High] This is a pre-existing issue, but further down in cdns3_plat_probe, when acquiring usb3_phy, if devm_phy_optional_get() fails, it returns directly via dev_err_probe(). Does this leak the initialized state of the usb2_phy since it bypasses the err_phy3_init label where phy_exit() is called? [Severity: High] This is a pre-existing issue, but also further down in cdns3_plat_probe, if cdns_core_init_role(cdns) fails, the error path jumps to err_cdns_init. Does this error path miss unregistering the USB role switch? If cdns_init() allocates and registers a non-devres managed role switch, bypassing the cleanup could leave dangling sysfs nodes and cause issues on reprobe. > diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c > index 432007cfe6954..c38a3dc7111a7 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); [Severity: High] This is a pre-existing issue, but could this race if both PCI functions are probed concurrently? If both call pcim_enable_device(pdev), both would increment their enable counts. Both would then see pci_is_enabled(func) as true, attempt to get the wrap structure via pci_get_drvdata(func), and receive a NULL pointer since pci_set_drvdata() hasn't been called yet. Could this lead to a NULL pointer dereference when wrap->dev_res is accessed? [ ... ] > @@ -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"); > + 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); [Severity: High] This is a pre-existing issue, but is it safe to unconditionally call kfree(wrap) in the error path of platform_device_register_full()? If the sibling function has already stored wrap in its drvdata, this might leave a dangling pointer attached to the sibling function. When the sibling is later unbound, could its remove routine retrieve the dangling pointer and double-free it? > @@ -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); > } [Severity: High] This is a pre-existing issue, but could this lead to a crash if the user unbinds the first PCI function? If wrap->devfn points to the second function, unbinding the first function would skip unregistering the platform device. The PCI core would then disable the first function and unmap its BARs. Since the platform device remains active, would subsequent MMIO accesses to the unmapped regions trigger a PCI master abort or page fault? > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > index 6a8d1fefbc0d0..504bdf13ea804 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); > } [Severity: High] This is a pre-existing issue, but does cdns_role_stop() have a race condition if multiple threads attempt to start or stop a role concurrently? Since cdns_role_stop() checks cdns->roles[role]->state outside of the cdns->mutex, two threads could observe the state as active, acquire the lock sequentially, and invoke the stop() callback twice. A similar risk exists in cdns_role_start(). -- Sashiko AI review · https://sashiko.dev/#/patchset/20260521-no_drd_config_v9-v9-0-2512cef10104@cadence.com?part=2 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-21 20:03 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-21 8:16 [PATCH v9 0/2] usb: cdns3: support configurations without DRD block Pawel Laszczak via B4 Relay 2026-05-21 8:16 ` [PATCH v9 1/2] dt-bindings: usb: cdns3: Add cdns,cdnsp compatible string Pawel Laszczak via B4 Relay 2026-05-21 8:16 ` sashiko-bot 2026-05-21 20:03 ` Conor Dooley 2026-05-21 19:59 ` Conor Dooley 2026-05-21 8:16 ` [PATCH v9 2/2] usb: cdnsp: Add support for device-only configuration Pawel Laszczak via B4 Relay 2026-05-21 9:04 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox