[PATCH v5 0/2] usb: dwc3: fixes for two connectors sharing one HS bus

[PATCH v5 0/2] usb: dwc3: fixes for two connectors sharing one HS bus

[Qianhui Xu]

*** AI-assistance disclosure ***
This cover letter and the accompanying patches were prepared with the help
of an LLM coding assistant; the author reviewed, tested and takes full
responsibility for the content. All measurements cited below come from real
hardware runs and are reproducible with the commands referenced in our
public reports.

Greetings,

This is v5.  It is a fixup series that applies *on top of* v4 [1] -- it
does not replace v4 1/3..3/3, it fixes them in place:

  - 1/2 tightens the binding of v4 1/3 (snps,dwc3-common.yaml),
  - 2/2 fixes the driver of v4 3/3 (usb: dwc3: support two connectors
    sharing one HS data bus) and also corrects
    usb_role_switch_at_endpoint() introduced by v4 2/3.

Base: v4 3/3 (fd69bd54e328, "usb: dwc3: support two connectors sharing
one HS data bus").  Please apply v4 1/3..3/3 from lore [1] before this
series; the `base-commit:` trailer in this posting is informational only,
since it names a commit that is not in any public tree yet.

Hardware: Lenovo Legion Y700 (4th gen) tablet, SM8750.  Both High Speed
connectors share one DWC3 controller; a port-select GPIO picks which
connector owns the bus, and each connector has its own typec port with its
own role switch, described per graph endpoint.

Changes since v4 (all points are replies to the automated review by
Sashiko [2][3][4]; no human maintainer replied in the v4 thread yet):

Binding (Sashiko on v4 1/3, three Medium findings [2]):

  * "hs-port patternProperties does not match what the driver expects /
    escapes unevaluatedProperties".  Fixed: the endpoint name pattern is
    now the same one graph.yaml uses, so an unnumbered endpoint or
    endpoint@2+ is validated too instead of silently escaping the
    unevaluatedProperties: false rule (v5 1/2).

  * "usb-role-switch should not be required only indirectly".  Fixed: the
    controller node must now carry "usb-role-switch" whenever
    port-select-gpios is present.  This is a driver-fact fix, not just
    schema cosmetics: without that property dwc3_get_dr_mode() forces
    dr_mode back to peripheral and dwc3_drd_init() never runs, so a DT
    that passed the old schema did not describe a DT the driver could act
    on (v5 1/2).

  * "mux-state complementarity is not expressed".  Fixed: the two
    independent `if: const: 0` blocks only rejected two zeroes; two
    endpoints both claiming mux-state = <1> passed validation.  The pair
    is now a single oneOf so exactly one assignment is accepted (v5 1/2).

  * On the standing question whether this should use the standard
    mux-controls/mux-states properties instead of per-endpoint role
    switches: kept the graph-endpoint shape for now.  Reason: the
    arbitration is "earliest request wins" between two *typec connectors*
    that must each be a usb role switch consumer, and the endpoint is the
    fwnode the connector is attached to; a mux-control consumer does not
    give us that per-connector identity.  drivers/usb/ has no
    mux_control consumer today to follow as precedent.  Happy to reshape
    if the DT maintainers prefer the standard property.

usb: roles (Sashiko on v4 2/3, one High finding [3]):

  * "-EPROBE_DEFER is swallowed by !IS_ERR_OR_NULL(sw)".  Fixed: pass the
    error up, as suggested (v5 2/2, last hunk).  This was also the root
    cause we hit on real hardware: the UCSI glue kept a NULL switch and
    the connectors never bound.

usb: dwc3 (Sashiko on v4 3/3, five High + three Medium findings [4]):

  * "dwc3_set_mode() under dwc->lock self-deadlocks".  Fixed: request
    decision and issue are now paired by mux->lock; dwc->lock is only held
    while updating the individual fields (v5 2/2).

  * "glue layer is never notified".  Fixed: dwc3_pre_set_role() is called
    with the effective role, outside spinlocks, before dwc3_set_mode(),
    mirroring the single-connector path (v5 2/2).

  * "same-role request that moves the data path to the other connector".
    Fixed: dwc3_set_prtcap() only clears SUSPHY when PRTCAP changes, so
    that case now clears it explicitly (v5 2/2).

  * "registration earlier than the initial mode request races with a
    connector driver".  Fixed: register and request the initial mode under
    mux->lock, tear down in reverse order on failure (v5 2/2).

  * "fwnode references leak on error paths".  Fixed: dwc->role_mux is
    published before the error paths that used to leak the endpoint
    fwnodes, so dwc3_role_mux_exit() can actually unwind (v5 2/2).

  * "sticky mux->error is reported to the next caller after it already
    committed its own request".  Fixed: the sticky value is dropped; a
    failed data-path switch is logged and counted per connector (v5 2/2).

  * (found during on-device testing, same area) dwc3_mux_role_get()
    returned USB_ROLE_NONE for a connector that did not own the data path,
    hiding its pending request; the connector driver then never sent the
    set_role(NONE) that would clear it, and the stale request kept winning
    arbitration after unplug.  get() now reports the requested role
    (v5 2/2).

Test evidence (all on the SM8750 tablet, kernel built from this series):

  - `git am --3way` of the whole series on top of v4 3/3 succeeds;
    checkpatch --strict reports 0 errors / 0 warnings / 0 checks for both
    patches.
  - `dt-doc-validate` on snps,dwc3-common.yaml: clean.
  - `make W=1 drivers/usb/dwc3/ drivers/usb/roles/` (arm64): 0 warnings.
  - On device: after the -EPROBE_DEFER fix, /sys/class/typec shows port0,
    port1 and a bound partner; with function_graph tracing, 12 consecutive
    plug/unplug cycles each went usb_role_switch_set_role ->
    dwc3_mux_role_set [dwc3] -> glue notifier -> dwc3_set_mode with zero
    WARNING/BUG/lockdep splats, USB data and charging both stable
    throughout.
  - The binding topology (two HS connectors on port@0 and port@2 of the
    controller's port node, one endpoint each, mux-state on the endpoint)
    matches what the board DTS uses; port@1 is occupied by the SuperSpeed
    path to the QMP PHY, which is why the two HS connectors are not
    siblings under one port.

Known limitation, stated up front: the role *arbitration* detail (which
connector wins a simultaneous request) is verified only by code reading and
the stale-request test above; a dedicated per-port role-value trace is
instrumented but its data set is not yet complete.  Everything else listed
above has been exercised on hardware.

Thanks,
Qianhui Xu

[1] v4: https://lore.kernel.org/linux-usb/20261026123211.243224-1-xiaqinghuan@mail.ustc.edu.cn/
[2] Sashiko on v4 1/3: <sashiko-outbox-185247@kernel.org>
[3] Sashiko on v4 2/3: <sashiko-outbox-185254@kernel.org>
[4] Sashiko on v4 3/3: <sashiko-outbox-185299@kernel.org>

Qianhui Xu (2):
  dt-bindings: usb: snps,dwc3: tighten the two-connector shared HS bus
  usb: dwc3: fix the two-connector shared HS bus role handling

 .../bindings/usb/snps,dwc3-common.yaml        | 105 +++++++++---------
 drivers/usb/dwc3/core.c                       |  36 ++++--
 drivers/usb/dwc3/core.h                       |   4 +-
 drivers/usb/dwc3/drd.c                        |  65 ++++++++---
 drivers/usb/roles/class.c                     |   7 +-
 5 files changed, 140 insertions(+), 77 deletions(-)


base-commit: fd69bd54e328c407b1853378b25d15803d3453fa
-- 
2.53.0

[PATCH v5 1/2] dt-bindings: usb: snps,dwc3: tighten the two-connector shared HS bus

[Qianhui Xu]

Two connectors behind one controller only take the mux path when the
controller node itself carries "usb-role-switch": dwc3_get_dr_mode()
otherwise forces dr_mode back to peripheral, dwc3_get_extcon() skips the
role-switch branch and dwc3_drd_init() never calls dwc3_role_mux_init().
The binding accepted such a DT, so a DT that passed the schema check did
not describe a DT the driver could act on.  Require the property whenever
port-select-gpios is present.

Two more holes in the endpoint schema are fixed at the same time:

  - the hs-port patternProperties only covered endpoint@0 and endpoint@1,
    so an unnumbered endpoint or endpoint@2+ escaped the
    unevaluatedProperties: false rule of the graph endpoint.  Use the same
    endpoint name pattern as graph.yaml so every endpoint is validated.

  - the mux-state complementarity was expressed with two independent "if
    const: 0" blocks, which only rejected two zeroes; two endpoints both
    claiming mux-state = <1> passed validation.  Express the pair as oneOf
    so exactly one of the two assignments is accepted.

Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
 .../bindings/usb/snps,dwc3-common.yaml        | 105 +++++++++---------
 1 file changed, 55 insertions(+), 50 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
index 6b06b0027b..b23d5d8d74 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
@@ -31,7 +31,9 @@ $defs:
         enum: [0, 1]
         description:
           Logical level of the port-select GPIO which connects this
-          connector to the shared High Speed data bus.
+          connector to the shared High Speed data bus.  The two connectors of
+          one controller must use different levels; the driver rejects a
+          device tree where two connectors claim the same one.
 
     dependentRequired:
       usb-role-switch: [mux-state]
@@ -42,7 +44,7 @@ $defs:
     unevaluatedProperties: false
 
     patternProperties:
-      '^endpoint@[01]$':
+      '^endpoint(@[0-9a-f]+)?$':
         $ref: '#/$defs/hs-endpoint'
 
 allOf:
@@ -61,70 +63,74 @@ allOf:
   - if:
       required: [port-select-gpios]
     then:
-      required: [dr_mode, ports]
+      # The mux path is only taken when the controller itself carries
+      # "usb-role-switch": dwc3_get_dr_mode() forces dr_mode back to
+      # peripheral, dwc3_get_extcon() skips the role-switch branch and
+      # dwc3_drd_init() never calls dwc3_role_mux_init() without it.
+      required: [dr_mode, ports, usb-role-switch]
       properties:
         dr_mode:
           const: otg
         ports:
-          required: [port@0]
+          # Each connector is one port with a single endpoint: the mux-state
+          # property on that endpoint says which level of the port-select GPIO
+          # reaches it, so two connectors are two ports, not two endpoints of
+          # one port.  port@1 is the SuperSpeed lane of the PHY and is already
+          # described by snps,dwc3.yaml, so the second High Speed connector
+          # lands on port@2.
+          required: [port@0, port@2]
           properties:
             port@0:
-              required: ['#address-cells', '#size-cells', endpoint@0, endpoint@1]
+              $ref: '#/$defs/hs-port'
+            port@2:
+              $ref: '#/$defs/hs-port'
+
+          # The two connectors must claim opposite levels of the port-select
+          # GPIO; requiring both ports in every branch is what makes the two
+          # cases mutually exclusive for oneOf.
+          oneOf:
+            - required: [port@0, port@2]
               properties:
-                endpoint@0:
-                  required: [usb-role-switch, mux-state]
-                endpoint@1:
-                  required: [usb-role-switch, mux-state]
-    else:
-      properties:
-        ports:
-          properties:
-            port@0:
-              patternProperties:
-                '^endpoint@[01]$':
+                port@0:
+                  $ref: '#/$defs/hs-port'
                   properties:
-                    usb-role-switch: false
-                    mux-state: false
-  - if:
-      properties:
-        ports:
-          properties:
-            port@0:
-              properties:
-                endpoint@0:
+                    endpoint:
+                      properties:
+                        mux-state:
+                          const: 0
+                port@2:
+                  $ref: '#/$defs/hs-port'
                   properties:
-                    mux-state:
-                      const: 0
-    then:
-      properties:
-        ports:
-          properties:
-            port@0:
+                    endpoint:
+                      properties:
+                        mux-state:
+                          const: 1
+            - required: [port@0, port@2]
               properties:
-                endpoint@1:
+                port@0:
+                  $ref: '#/$defs/hs-port'
                   properties:
-                    mux-state:
-                      const: 1
-  - if:
-      properties:
-        ports:
-          properties:
-            port@0:
-              properties:
-                endpoint@1:
+                    endpoint:
+                      properties:
+                        mux-state:
+                          const: 1
+                port@2:
+                  $ref: '#/$defs/hs-port'
                   properties:
-                    mux-state:
-                      const: 0
-    then:
+                    endpoint:
+                      properties:
+                        mux-state:
+                          const: 0
+    else:
       properties:
         ports:
           properties:
             port@0:
-              properties:
-                endpoint@0:
+              patternProperties:
+                '^endpoint(@[0-9a-f]+)?$':
                   properties:
-                    mux-state:
-                      const: 1
+                    usb-role-switch: false
+                    mux-state: false
 
 properties:
   port-select-gpios:
@@ -536,4 +542,3 @@ required:
 
 additionalProperties: true
 ...
-
base-commit: fd69bd54e328c407b1853378b25d15803d3453fa
-- 
2.53.0

[PATCH v5 2/2] usb: dwc3: fix the two-connector shared HS bus role handling

[Qianhui Xu]

A controller whose High Speed data lines are shared by two connectors
selects one of them with port-select-gpios and gives each connector its
own role switch.  The first version of that code deadlocks on the first
role change and mislays several pieces of state; fix it in place.

  - dwc3_mux_role_set() called dwc3_set_mode() while holding dwc->lock,
    and dwc3_set_mode() takes dwc->lock itself.  Deciding the request and
    issuing it are now paired by mux->lock, and dwc->lock is only held
    while the individual fields are updated.

  - the same function never notified the glue layer.  Call
    dwc3_pre_set_role() with the effective role, outside spinlocks,
    before dwc3_set_mode(), like the single-connector path does.

  - dwc3_mux_role_get() returned USB_ROLE_NONE for a connector that did
    not own the data path, hiding its pending request.  Connector drivers
    therefore never sent the set_role(USB_ROLE_NONE) that clears it, and
    the stale request kept winning dwc3_role_mux_pick() after the
    connector was unplugged.  Report the role the connector requested.

  - a same-role request that moves the data path to the other connector
    reinitialises the controller, but dwc3_set_prtcap() only clears
    SUSPHY when PRTCAP changes.  Clear it explicitly for that case.

  - the endpoint fwnode references were taken before dwc->role_mux was
    assigned, so the error paths leaked them: dwc3_role_mux_exit() is a
    no-op while dwc->role_mux is NULL.  Publish the mux first.

  - the role switches were registered before the initial mode request was
    issued, so a connector driver probing in that window could have its
    request silently overwritten.  Register and request the initial mode
    under mux->lock, and tear down in the reverse order on failure.

  - the sticky mux->error value was reported to whichever connector called
    set() next, after that call had already committed its own request.
    Drop it; a data-path switch failure is logged and counted per
    connector instead.

While at it, usb_role_switch_at_endpoint() swallowed the -EPROBE_DEFER
returned for a graph endpoint whose switch is not registered yet, turning
it into "this connector has no role switch".  Pass it up.

Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
 drivers/usb/dwc3/core.c   | 36 ++++++++++++++++------
 drivers/usb/dwc3/core.h   |  4 +--
 drivers/usb/dwc3/drd.c    | 65 ++++++++++++++++++++++++++++++---------
 drivers/usb/roles/class.c |  7 ++++-
 4 files changed, 85 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 6607e65e84..da609a011d 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -168,6 +168,7 @@ static void __dwc3_set_mode(struct work_struct *work)
 	struct dwc3 *dwc = work_to_dwc(work);
 	struct dwc3_role_mux *mux = dwc->role_mux;
 	unsigned long flags;
+	bool port_switch;
 	int mux_port = 0;
 	int ret;
 	u32 reg;
@@ -189,8 +190,14 @@ static void __dwc3_set_mode(struct work_struct *work)
 	if (!desired_dr_role)
 		goto out;
 
-	if (desired_dr_role == dwc->current_dr_role &&
-	    (!mux || mux_port == mux->active_port))
+	/*
+	 * Moving the shared data path to the other connector is a mode switch even
+	 * when the role does not change: the running host or gadget instance has to
+	 * be torn down while its own data lines are still selected.
+	 */
+	port_switch = mux && mux_port != mux->active_port;
+
+	if (desired_dr_role == dwc->current_dr_role && !port_switch)
 		goto out;
 
 	if (desired_dr_role == DWC3_GCTL_PRTCAP_OTG && dwc->edev)
@@ -246,22 +253,33 @@ static void __dwc3_set_mode(struct work_struct *work)
 	 * the controller would be left with a torn-down role and a stale
 	 * current_dr_role, which the next request would tear down a second time.
 	 */
-	if (mux && mux_port != mux->active_port) {
+	if (port_switch) {
 		ret = gpiod_direction_output(mux->select, mux_port);
-		if (ret) {
-			WRITE_ONCE(mux->error, ret);
+		if (ret)
 			dev_err_ratelimited(dwc->dev,
 					    "failed to select USB port %d: %d\n",
 					    mux_port, ret);
-		} else {
-			spin_lock_irqsave(&dwc->lock, flags);
+
+		spin_lock_irqsave(&dwc->lock, flags);
+		if (ret)
+			mux->ports[mux_port].errors++;
+		else
 			mux->active_port = mux_port;
-			spin_unlock_irqrestore(&dwc->lock, flags);
-		}
+		spin_unlock_irqrestore(&dwc->lock, flags);
 	}
 
 	spin_lock_irqsave(&dwc->lock, flags);
 
+	/*
+	 * dwc3_set_prtcap() only clears GUSB3PIPECTL.SUSPENDENABLE and
+	 * GUSB2PHYCFG.SUSPHY when PRTCAP actually changes, but a same-role port
+	 * switch reinitialises the controller too and must start from the same
+	 * state: the instance created below would otherwise inherit the SUSPHY
+	 * setting of the one torn down above.
+	 */
+	if (port_switch)
+		dwc3_enable_susphy(dwc, false);
+
 	dwc3_set_prtcap(dwc, desired_dr_role, false);
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 1238376749..eae321e2d7 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1017,6 +1017,8 @@ struct dwc3_role_port {
 	unsigned int		state;
 	/* request order, see dwc3_role_mux.seq */
 	u64			seq;
+	/* data-path switch failures, for diagnostics only */
+	unsigned int		errors;
 };
 
 /**
@@ -1027,7 +1029,6 @@ struct dwc3_role_port {
  * @desired_port: index of the connector that currently wins
  * @active_port: index of the connector currently driving the data lines
  * @seq: monotonic request counter used to order pending requests
- * @error: sticky data-path switch error, reported to the role-switch users
  *
  * Two connectors can share the High Speed data lines of one controller while
  * keeping their Super Speed lanes separate.  Each connector then needs its own
@@ -1042,7 +1043,6 @@ struct dwc3_role_mux {
 	int			desired_port;
 	int			active_port;
 	u64			seq;
-	int			error;
 };
 
 /**
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 20821d6505..53106bed3e 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -543,7 +543,7 @@ static int dwc3_mux_role_set(struct usb_role_switch *sw, enum usb_role role)
 	struct dwc3_role_mux *mux = dwc->role_mux;
 	unsigned long flags;
 	enum usb_role selected_role;
-	int selected, ret;
+	int selected;
 
 	if (role > USB_ROLE_DEVICE)
 		return -EINVAL;
@@ -566,35 +566,52 @@ static int dwc3_mux_role_set(struct usb_role_switch *sw, enum usb_role role)
 	selected = dwc3_role_mux_pick(mux);
 	mux->desired_port = selected;
 	selected_role = mux->ports[selected].role;
+	spin_unlock_irqrestore(&dwc->lock, flags);
+
 	/*
+	 * Notify the glue layer before touching the controller, in the same order
+	 * as the single-connector path.  It may sleep (the Qualcomm glue calls
+	 * pm_runtime_resume_and_get()), so it runs outside of dwc->lock -
+	 * dwc3_set_mode() takes that lock itself and a recursive take deadlocks
+	 * the controller on the very first role change.
+	 *
+	 * Pairing with the selection above is provided by mux->lock: a second
+	 * connector blocks in mutex_lock() until this request has been issued.
+	 *
 	 * Same asynchronous semantics as the single-connector role switch: the
 	 * data path and the controller are reprogrammed from __dwc3_set_mode().
-	 * dwc3_set_mode() only queues that work, so the mux lock may be held.
 	 */
+	dwc3_pre_set_role(dwc, selected_role);
 	dwc3_set_mode(dwc, dwc3_role_mux_mode(dwc, selected_role));
-	ret = mux->error;
-	mux->error = 0;
-	spin_unlock_irqrestore(&dwc->lock, flags);
+
 	mutex_unlock(&mux->lock);
 
-	return ret;
+	/*
+	 * A data-path switch failure is reported through the console and counted
+	 * per connector; it is not returned here.  This call has already updated
+	 * the request state, so a stale error from another connector must not be
+	 * reported as the result of this (successful) request.
+	 */
+	return 0;
 }
 
 static enum usb_role dwc3_mux_role_get(struct usb_role_switch *sw)
 {
 	struct dwc3_role_port *port = usb_role_switch_get_drvdata(sw);
 	struct dwc3 *dwc = port->dwc;
-	struct dwc3_role_mux *mux = dwc->role_mux;
 	unsigned long flags;
-	enum usb_role role = USB_ROLE_NONE;
+	enum usb_role role;
 
 	/*
-	 * A connector only reports its own role while it owns the shared data
-	 * path, so the connector drivers never fight over the controller.
+	 * Report the role this connector itself requested, whether or not it owns
+	 * the shared data path right now.  Hiding a pending request behind
+	 * active_port makes connector drivers read back USB_ROLE_NONE, so they
+	 * never send the set_role(USB_ROLE_NONE) that would release it; the stale
+	 * request then keeps winning dwc3_role_mux_pick() after the connector has
+	 * been unplugged and starves the other one.
 	 */
 	spin_lock_irqsave(&dwc->lock, flags);
-	if (mux->active_port == port->state)
-		role = port->role;
+	role = port->role;
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	return role;
@@ -647,6 +664,14 @@ static int dwc3_role_mux_init(struct dwc3 *dwc, u32 mode)
 	mux->active_port = -1;
 	mutex_init(&mux->lock);
 
+	/*
+	 * Publish the mux before the first endpoint reference is taken:
+	 * dwc3_role_mux_exit() is a no-op while dwc->role_mux is NULL, so every
+	 * error path below would otherwise leak the fwnode references already
+	 * acquired.
+	 */
+	dwc->role_mux = mux;
+
 	fwnode_graph_for_each_endpoint(dev_fwnode(dwc->dev), ep) {
 		if (!fwnode_property_present(ep, "usb-role-switch"))
 			continue;
@@ -673,7 +698,14 @@ static int dwc3_role_mux_init(struct dwc3 *dwc, u32 mode)
 		}
 	}
 
-	dwc->role_mux = mux;
+	/*
+	 * Registering the switches makes them reachable from the connector
+	 * drivers, so the initial mode request has to be issued inside the same
+	 * critical section: a set() arriving in between would be silently
+	 * overwritten by it while port->role keeps the requested value.
+	 * dwc3_mux_role_set() serialises on the same lock.
+	 */
+	mutex_lock(&mux->lock);
 
 	for (i = 0; i < ARRAY_SIZE(mux->ports); i++) {
 		snprintf(name, sizeof(name), "%s-port%u", dev_name(dwc->dev), i);
@@ -686,6 +718,7 @@ static int dwc3_role_mux_init(struct dwc3 *dwc, u32 mode)
 		if (IS_ERR(mux->ports[i].sw)) {
 			ret = PTR_ERR(mux->ports[i].sw);
 			mux->ports[i].sw = NULL;
+			mutex_unlock(&mux->lock);
 			goto err;
 		}
 	}
@@ -697,13 +730,15 @@ static int dwc3_role_mux_init(struct dwc3 *dwc, u32 mode)
 	 */
 	dwc3_set_mode(dwc, mode);
 
+	mutex_unlock(&mux->lock);
+
 	return 1;
 
 err:
-	dwc3_role_mux_exit(dwc);
-	dwc->role_mux = NULL;
 	/* A mode request queued before the failure must not run on freed state. */
 	cancel_work_sync(&dwc->drd_work);
+	dwc3_role_mux_exit(dwc);
+	dwc->role_mux = NULL;
 	return ret;
 err_put_ep:
 	fwnode_handle_put(ep);
diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index fc739fd61f..ee186d542a 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -184,7 +184,12 @@ usb_role_switch_at_endpoint(struct fwnode_handle *fwnode)
 			continue;
 		sw = usb_role_switch_match(remote, "usb-role-switch", NULL);
 		fwnode_handle_put(remote);
-		if (!IS_ERR_OR_NULL(sw)) {
+		/*
+		 * ERR_PTR(-EPROBE_DEFER) means the switch is described by the graph
+		 * but not registered yet: pass it up so the caller defers instead of
+		 * concluding that this connector has no role switch at all.
+		 */
+		if (sw) {
 			fwnode_handle_put(ep);
 			return sw;
 		}
base-commit: fd69bd54e328c407b1853378b25d15803d3453fa
-- 
2.53.0

Re: [PATCH v5 0/2] usb: dwc3: fixes for two connectors sharing one HS bus

[Krishna Kurapati]

On 10/31/2026 8:31 PM, Qianhui Xu wrote:
> *** AI-assistance disclosure ***
> This cover letter and the accompanying patches were prepared with the help
> of an LLM coding assistant; the author reviewed, tested and takes full
> responsibility for the content. All measurements cited below come from real
> hardware runs and are reproducible with the commands referenced in our
> public reports.
> 
> Greetings,
> 
> This is v5.  It is a fixup series that applies *on top of* v4 [1] -- it
> does not replace v4 1/3..3/3, it fixes them in place:
> 

If v5 is being sent, it must not to be applied on top of v4. It must 
include (v4 + any additional changes/improvements).

Regards,
Krishna,

>    - 1/2 tightens the binding of v4 1/3 (snps,dwc3-common.yaml),
>    - 2/2 fixes the driver of v4 3/3 (usb: dwc3: support two connectors
>      sharing one HS data bus) and also corrects
>      usb_role_switch_at_endpoint() introduced by v4 2/3.
> 
> Base: v4 3/3 (fd69bd54e328, "usb: dwc3: support two connectors sharing
> one HS data bus").  Please apply v4 1/3..3/3 from lore [1] before this
> series; the `base-commit:` trailer in this posting is informational only,
> since it names a commit that is not in any public tree yet.
> 
> Hardware: Lenovo Legion Y700 (4th gen) tablet, SM8750.  Both High Speed
> connectors share one DWC3 controller; a port-select GPIO picks which
> connector owns the bus, and each connector has its own typec port with its
> own role switch, described per graph endpoint.
> 
> Changes since v4 (all points are replies to the automated review by
> Sashiko [2][3][4]; no human maintainer replied in the v4 thread yet):
> 
> Binding (Sashiko on v4 1/3, three Medium findings [2]):
> 
>    * "hs-port patternProperties does not match what the driver expects /
>      escapes unevaluatedProperties".  Fixed: the endpoint name pattern is
>      now the same one graph.yaml uses, so an unnumbered endpoint or
>      endpoint@2+ is validated too instead of silently escaping the
>      unevaluatedProperties: false rule (v5 1/2).
> 
>    * "usb-role-switch should not be required only indirectly".  Fixed: the
>      controller node must now carry "usb-role-switch" whenever
>      port-select-gpios is present.  This is a driver-fact fix, not just
>      schema cosmetics: without that property dwc3_get_dr_mode() forces
>      dr_mode back to peripheral and dwc3_drd_init() never runs, so a DT
>      that passed the old schema did not describe a DT the driver could act
>      on (v5 1/2).
> 
>    * "mux-state complementarity is not expressed".  Fixed: the two
>      independent `if: const: 0` blocks only rejected two zeroes; two
>      endpoints both claiming mux-state = <1> passed validation.  The pair
>      is now a single oneOf so exactly one assignment is accepted (v5 1/2).
> 
>    * On the standing question whether this should use the standard
>      mux-controls/mux-states properties instead of per-endpoint role
>      switches: kept the graph-endpoint shape for now.  Reason: the
>      arbitration is "earliest request wins" between two *typec connectors*
>      that must each be a usb role switch consumer, and the endpoint is the
>      fwnode the connector is attached to; a mux-control consumer does not
>      give us that per-connector identity.  drivers/usb/ has no
>      mux_control consumer today to follow as precedent.  Happy to reshape
>      if the DT maintainers prefer the standard property.
> 
> usb: roles (Sashiko on v4 2/3, one High finding [3]):
> 
>    * "-EPROBE_DEFER is swallowed by !IS_ERR_OR_NULL(sw)".  Fixed: pass the
>      error up, as suggested (v5 2/2, last hunk).  This was also the root
>      cause we hit on real hardware: the UCSI glue kept a NULL switch and
>      the connectors never bound.
> 
> usb: dwc3 (Sashiko on v4 3/3, five High + three Medium findings [4]):
> 
>    * "dwc3_set_mode() under dwc->lock self-deadlocks".  Fixed: request
>      decision and issue are now paired by mux->lock; dwc->lock is only held
>      while updating the individual fields (v5 2/2).
> 
>    * "glue layer is never notified".  Fixed: dwc3_pre_set_role() is called
>      with the effective role, outside spinlocks, before dwc3_set_mode(),
>      mirroring the single-connector path (v5 2/2).
> 
>    * "same-role request that moves the data path to the other connector".
>      Fixed: dwc3_set_prtcap() only clears SUSPHY when PRTCAP changes, so
>      that case now clears it explicitly (v5 2/2).
> 
>    * "registration earlier than the initial mode request races with a
>      connector driver".  Fixed: register and request the initial mode under
>      mux->lock, tear down in reverse order on failure (v5 2/2).
> 
>    * "fwnode references leak on error paths".  Fixed: dwc->role_mux is
>      published before the error paths that used to leak the endpoint
>      fwnodes, so dwc3_role_mux_exit() can actually unwind (v5 2/2).
> 
>    * "sticky mux->error is reported to the next caller after it already
>      committed its own request".  Fixed: the sticky value is dropped; a
>      failed data-path switch is logged and counted per connector (v5 2/2).
> 
>    * (found during on-device testing, same area) dwc3_mux_role_get()
>      returned USB_ROLE_NONE for a connector that did not own the data path,
>      hiding its pending request; the connector driver then never sent the
>      set_role(NONE) that would clear it, and the stale request kept winning
>      arbitration after unplug.  get() now reports the requested role
>      (v5 2/2).
> 
> Test evidence (all on the SM8750 tablet, kernel built from this series):
> 
>    - `git am --3way` of the whole series on top of v4 3/3 succeeds;
>      checkpatch --strict reports 0 errors / 0 warnings / 0 checks for both
>      patches.
>    - `dt-doc-validate` on snps,dwc3-common.yaml: clean.
>    - `make W=1 drivers/usb/dwc3/ drivers/usb/roles/` (arm64): 0 warnings.
>    - On device: after the -EPROBE_DEFER fix, /sys/class/typec shows port0,
>      port1 and a bound partner; with function_graph tracing, 12 consecutive
>      plug/unplug cycles each went usb_role_switch_set_role ->
>      dwc3_mux_role_set [dwc3] -> glue notifier -> dwc3_set_mode with zero
>      WARNING/BUG/lockdep splats, USB data and charging both stable
>      throughout.
>    - The binding topology (two HS connectors on port@0 and port@2 of the
>      controller's port node, one endpoint each, mux-state on the endpoint)
>      matches what the board DTS uses; port@1 is occupied by the SuperSpeed
>      path to the QMP PHY, which is why the two HS connectors are not
>      siblings under one port.
> 
> Known limitation, stated up front: the role *arbitration* detail (which
> connector wins a simultaneous request) is verified only by code reading and
> the stale-request test above; a dedicated per-port role-value trace is
> instrumented but its data set is not yet complete.  Everything else listed
> above has been exercised on hardware.
> 
> Thanks,
> Qianhui Xu
> 
> [1] v4: https://lore.kernel.org/linux-usb/20261026123211.243224-1-xiaqinghuan@mail.ustc.edu.cn/
> [2] Sashiko on v4 1/3: <sashiko-outbox-185247@kernel.org>
> [3] Sashiko on v4 2/3: <sashiko-outbox-185254@kernel.org>
> [4] Sashiko on v4 3/3: <sashiko-outbox-185299@kernel.org>
> 
> Qianhui Xu (2):
>    dt-bindings: usb: snps,dwc3: tighten the two-connector shared HS bus
>    usb: dwc3: fix the two-connector shared HS bus role handling
> 
>   .../bindings/usb/snps,dwc3-common.yaml        | 105 +++++++++---------
>   drivers/usb/dwc3/core.c                       |  36 ++++--
>   drivers/usb/dwc3/core.h                       |   4 +-
>   drivers/usb/dwc3/drd.c                        |  65 ++++++++---
>   drivers/usb/roles/class.c                     |   7 +-
>   5 files changed, 140 insertions(+), 77 deletions(-)
> 
> 
> base-commit: fd69bd54e328c407b1853378b25d15803d3453fa