[PATCH 0/3] usb: dwc3: support two connectors sharing one HS data bus

[PATCH 0/3] usb: dwc3: support two connectors sharing one HS data bus

[Qianhui Xu]

From: Xu Qianhui <xiaqinghuan@mail.ustc.edu.cn>

Some boards wire the High Speed data lines of a single DWC3 controller to
two receptacles and select between them with a GPIO, while the SuperSpeed
lanes stay per connector.  This series makes such a design usable:

  - patch 1 documents "usb-psy-name", a property the dwc3 glue drivers
    already parse but which the bindings never described;

  - patch 2 describes the two-connector HS data path: "port-select-gpios"
    plus a "ports" node whose connector endpoints carry "mux-state" (the
    logical port-select level of that connector) and "usb-role-switch";

  - patch 3 adds the driver side: one role switch per connector, mutual
    arbitration between them, and a data-path switch performed after the
    previous host/gadget instance has been torn down.

Single-connector designs are unaffected: the mux path is only taken when
"port-select-gpios" is present.

This work was developed on the Qualcomm SM8750 (Lenovo TB322FC) tablet;
the board device tree is not upstream yet, so no in-tree user is switched
over by this series.

Xu Qianhui (3):
  dt-bindings: usb: snps,dwc3: document usb-psy-name
  dt-bindings: usb: snps,dwc3: describe the two-connector HS data path
  usb: dwc3: support two connectors sharing one HS data bus

 .../bindings/usb/snps,dwc3-common.yaml        |  65 ++++++-
 drivers/usb/dwc3/core.c                       |  30 ++-
 drivers/usb/dwc3/core.h                       |  22 +++
 drivers/usb/dwc3/debugfs.c                    |   4 +
 drivers/usb/dwc3/drd.c                        | 177 +++++++++++++++++-
 5 files changed, 294 insertions(+), 4 deletions(-)

[PATCH 1/3] dt-bindings: usb: snps,dwc3: document usb-psy-name

[Qianhui Xu]

From: Xu Qianhui <xiaqinghuan@mail.ustc.edu.cn>

The dwc3 driver looks up "usb-psy-name" to find the power supply which
accepts the input current limit requested by the gadget after USB
configuration, and while the bus is suspended or resumed
(dwc3_vbus_draw_work()), but the property is not described by any
binding:

  $ git grep -n usb-psy-name -- Documentation/
  (no match)

As a result, any DTS using it fails dtbs_check.  The property is read
from the DWC3 device node, which for the single-node flavours (e.g.
qcom,snps-dwc3.yaml) is the node that $refs this file, so document it
here.

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

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
index 6c0b8b6538..6609a709c1 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
@@ -51,6 +51,13 @@ properties:
       - items:
           pattern: "^usb(2-([0-9]|1[0-4])|3-[0-3])$"
 
+  usb-psy-name:
+    description:
+      Name of the USB power supply accepting the input current limit
+      requested by the gadget after USB configuration, and during bus
+      suspend and resume.
+    $ref: /schemas/types.yaml#/definitions/string
+
   snps,usb2-lpm-disable:
     description: Indicate if we don't want to enable USB2 HW LPM for host
       mode.

[PATCH 2/3] dt-bindings: usb: snps,dwc3: describe the two-connector HS data path

[Qianhui Xu]

From: Xu Qianhui <xiaqinghuan@mail.ustc.edu.cn>

Some designs wire the High Speed data lines of a single DWC3 controller to
two receptacles and select between them with a GPIO, while the Super Speed
lanes stay per connector:

                         +-----------+  HS  +----------+
  DWC3 ---- port@0 -----| port-select|-----| connector|
        |                |   GPIO    |     +----------+
        +-- port@2 --------------------...
                         +-----------+

Describe the hardware in snps,dwc3-common.yaml:

  - port-select-gpios selects which connector drives the shared HS bus;
  - each endpoint of "ports" carries "mux-state", the logical port-select
    level of that connector, and marks itself as a role-switch input with
    "usb-role-switch".

The connector endpoints are restricted to the port-base/endpoint-base
schemas so that endpoint subnodes cannot carry unrelated properties.

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

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
index 6609a709c1..a18bb08afb 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
@@ -13,6 +13,28 @@ description:
   Defines the properties of the DWC3 core as being embedded in either an
   vendor-specific implementation or as a standalone component.
 
+$defs:
+  hs-port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    unevaluatedProperties: false
+    properties:
+      endpoint:
+        $ref: /schemas/graph.yaml#/$defs/endpoint-base
+        unevaluatedProperties: false
+        properties:
+          usb-role-switch:
+            type: boolean
+            description: Separate role-switch input for this connector.
+          mux-state:
+            $ref: /schemas/types.yaml#/definitions/uint32
+            enum: [0, 1]
+            description:
+              Logical port-select GPIO level for this data path. State 0
+              takes precedence when both connectors request USB data.
+        dependentRequired:
+          usb-role-switch: [mux-state]
+          mux-state: [usb-role-switch]
+
 allOf:
   - $ref: usb-drd.yaml#
   - if:
@@ -26,8 +48,44 @@ allOf:
       $ref: usb.yaml#
     else:
       $ref: usb-xhci.yaml#
+  - if:
+      required: [port-select-gpios]
+    then:
+      required: [usb-role-switch, ports, dr_mode]
+      properties:
+        dr_mode:
+          const: otg
+        ports:
+          required: [port@0, port@2]
+          properties:
+            port@0:
+              required: [endpoint]
+              properties:
+                endpoint:
+                  required: [usb-role-switch, mux-state]
+            port@2:
+              required: [endpoint]
+              properties:
+                endpoint:
+                  required: [usb-role-switch, mux-state]
 
 properties:
+  usb-psy-name:
+    description:
+      Name of the USB power supply accepting the input current limit
+      requested by the gadget after USB configuration, and during bus
+      suspend and resume.
+    $ref: /schemas/types.yaml#/definitions/string
+
+  port-select-gpios:
+    maxItems: 1
+    description:
+      Selects between two external connectors sharing the controller's HS
+      data bus. The port@0 and port@2 endpoints specify distinct mux-state
+      values and provide independent role-switch inputs. The controller
+      stops the previous role before changing the data path. With neither
+      connector requesting data, the last selection is retained.
+
   extcon:
     maxItems: 1
     deprecated: true
@@ -51,13 +109,6 @@ properties:
       - items:
           pattern: "^usb(2-([0-9]|1[0-4])|3-[0-3])$"
 
-  usb-psy-name:
-    description:
-      Name of the USB power supply accepting the input current limit
-      requested by the gadget after USB configuration, and during bus
-      suspend and resume.
-    $ref: /schemas/types.yaml#/definitions/string
-
   snps,usb2-lpm-disable:
     description: Indicate if we don't want to enable USB2 HW LPM for host
       mode.
@@ -418,13 +469,17 @@ properties:
 
     properties:
       port@0:
-        $ref: /schemas/graph.yaml#/properties/port
+        $ref: '#/$defs/hs-port'
         description: High Speed (HS) data bus.
 
       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description: Super Speed (SS) data bus.
 
+      port@2:
+        $ref: '#/$defs/hs-port'
+        description: Alternate connector on the multiplexed HS data bus.
+
   wakeup-source:
     $ref: /schemas/types.yaml#/definitions/flag
     description:
@@ -436,4 +491,3 @@ required:
 
 additionalProperties: true
 ...
-

[PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus

[Qianhui Xu]

From: Xu Qianhui <xiaqinghuan@mail.ustc.edu.cn>

On some boards a single DWC3 controller drives two receptacles: the Super
Speed pairs are wired to separate PHYs, while the High Speed data lines are
muxed to whichever connector is in use.  The controller therefore has to
service two independent role-switch inputs and move the shared data path
before it reinitialises the controller for the requested role.

Register one usb_role_switch per connector endpoint described in
"ports"/"mux-state" when "port-select-gpios" is present.  Requests from the
two switches are serialised by a mutex, the oldest connector holding a role
wins, and the level of the port-select GPIO is only changed once the
previous host or gadget instance has been torn down, which is where
__dwc3_set_mode() runs.  A connector only reports its own role while its
data path is the active one, so the connector drivers see a consistent view
of a shared controller.

While the two-connector mux owns role selection, the debugfs "mode" write
used to force a specific role is refused instead of racing with it.

Signed-off-by: Xu Qianhui <xiaqinghuan@mail.ustc.edu.cn>
---
 drivers/usb/dwc3/core.c    |  30 ++++++-
 drivers/usb/dwc3/core.h    |  22 +++++
 drivers/usb/dwc3/debugfs.c |   4 +
 drivers/usb/dwc3/drd.c     | 177 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 231 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index fd5c2cd36c..76e9e5d180 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/list.h>
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/dma-mapping.h>
 #include <linux/of.h>
 #include <linux/of_graph.h>
@@ -166,6 +167,9 @@ static void __dwc3_set_mode(struct work_struct *work)
 {
 	struct dwc3 *dwc = work_to_dwc(work);
 	unsigned long flags;
+	struct dwc3_role_mux *mux = dwc->role_mux;
+	enum usb_role mux_role = USB_ROLE_NONE;
+	int mux_port = 0;
 	int ret;
 	u32 reg;
 	u32 desired_dr_role;
@@ -174,8 +178,15 @@ static void __dwc3_set_mode(struct work_struct *work)
 	mutex_lock(&dwc->mutex);
 	spin_lock_irqsave(&dwc->lock, flags);
 	desired_dr_role = dwc->desired_dr_role;
+	if (mux) {
+		mux_port = mux->desired_port;
+		mux_role = mux->desired_role;
+	}
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
+	if (mux && READ_ONCE(mux->error))
+		goto out;
+
 	pm_runtime_get_sync(dwc->dev);
 
 	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG)
@@ -184,7 +195,8 @@ static void __dwc3_set_mode(struct work_struct *work)
 	if (!desired_dr_role)
 		goto out;
 
-	if (desired_dr_role == dwc->current_dr_role)
+	if (desired_dr_role == dwc->current_dr_role &&
+	    (!mux || mux_port == mux->active_port))
 		goto out;
 
 	if (desired_dr_role == DWC3_GCTL_PRTCAP_OTG && dwc->edev)
@@ -233,6 +245,22 @@ static void __dwc3_set_mode(struct work_struct *work)
 		dwc3_writel(dwc, DWC3_GCTL, reg);
 	}
 
+	/*
+	 * The previous host/gadget has been torn down above, so it is safe to
+	 * move the shared HS data path to the requesting connector.
+	 */
+	if (mux && mux_port != mux->active_port) {
+		ret = gpiod_direction_output(mux->select, mux_port);
+		if (ret) {
+			WRITE_ONCE(mux->error, ret);
+			goto out;
+		}
+
+		spin_lock_irqsave(&dwc->lock, flags);
+		mux->active_port = mux_port;
+		spin_unlock_irqrestore(&dwc->lock, flags);
+	}
+
 	spin_lock_irqsave(&dwc->lock, flags);
 
 	dwc3_set_prtcap(dwc, desired_dr_role, false);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index f3dabb37f8..ffd07092da 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1008,6 +1008,26 @@ struct dwc3_glue_ops {
 	void	(*pre_run_stop)(struct dwc3 *dwc, bool is_on);
 };
 
+struct dwc3_role_port {
+	struct dwc3 *dwc;
+	struct usb_role_switch *sw;
+	struct fwnode_handle *fwnode;
+	enum usb_role role;
+	int state;
+};
+
+struct dwc3_role_mux {
+	struct gpio_desc *select;
+	/* Serialize requests from the two connector role switches. */
+	struct mutex lock;
+	struct dwc3_role_port ports[2];
+	enum usb_role desired_role;
+	int desired_port;
+	int active_port;
+	int error;
+	bool stopping;
+};
+
 /**
  * struct dwc3 - representation of our controller
  * @drd_work: workqueue used for role swapping
@@ -1064,6 +1084,7 @@ struct dwc3_glue_ops {
  *		- USBPHY_INTERFACE_MODE_UTMI
  *		- USBPHY_INTERFACE_MODE_UTMIW
  * @role_sw: usb_role_switch handle
+ * @role_mux: optional shared data-port mux and per-connector role switches
  * @role_switch_default_mode: default operation mode of controller while
  *			usb role is USB_ROLE_NONE.
  * @usb_psy: pointer to power supply interface.
@@ -1265,6 +1286,7 @@ struct dwc3 {
 	struct notifier_block	edev_nb;
 	enum usb_phy_interface	hsphy_mode;
 	struct usb_role_switch	*role_sw;
+	struct dwc3_role_mux	*role_mux;
 	enum usb_dr_mode	role_switch_default_mode;
 
 	struct power_supply	*usb_psy;
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 65d909e4b7..e8616dea6b 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -438,6 +438,10 @@ static ssize_t dwc3_mode_write(struct file *file,
 	u32			mode = 0;
 	char			buf[32];
 
+	/* The connector providers own both role and data-path selection. */
+	if (dwc->role_mux)
+		return -EBUSY;
+
 	if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
 		return -EFAULT;
 
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index f3e37d3836..4bf0497e90 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/extcon.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
@@ -501,10 +502,178 @@ static enum usb_role dwc3_usb_role_switch_get(struct usb_role_switch *sw)
 	return role;
 }
 
+static int dwc3_mux_role_set(struct usb_role_switch *sw, enum usb_role role)
+{
+	struct dwc3_role_port *port = usb_role_switch_get_drvdata(sw);
+	struct dwc3 *dwc = port->dwc;
+	struct dwc3_role_mux *mux = dwc->role_mux;
+	enum usb_role selected_role;
+	unsigned long flags;
+	int selected, ret;
+	u32 mode;
+
+	if (role < USB_ROLE_NONE || role > USB_ROLE_DEVICE)
+		return -EINVAL;
+
+	mutex_lock(&mux->lock);
+	if (mux->stopping) {
+		ret = -ESHUTDOWN;
+		goto out;
+	}
+	if (READ_ONCE(mux->error)) {
+		ret = READ_ONCE(mux->error);
+		goto out;
+	}
+
+	port->role = role;
+	if (mux->ports[0].role != USB_ROLE_NONE)
+		selected = 0;
+	else if (mux->ports[1].role != USB_ROLE_NONE)
+		selected = 1;
+	else
+		selected = mux->desired_port;
+	selected_role = mux->ports[selected].role;
+	if (selected == mux->desired_port && selected_role == mux->desired_role) {
+		ret = 0;
+		goto out;
+	}
+
+	if (selected_role == USB_ROLE_HOST ||
+	    (selected_role == USB_ROLE_NONE &&
+	     dwc->role_switch_default_mode == USB_DR_MODE_HOST))
+		mode = DWC3_GCTL_PRTCAP_HOST;
+	else
+		mode = DWC3_GCTL_PRTCAP_DEVICE;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	mux->desired_port = selected;
+	mux->desired_role = selected_role;
+	dwc->desired_dr_role = mode;
+	spin_unlock_irqrestore(&dwc->lock, flags);
+	queue_work(system_freezable_wq, &dwc->drd_work);
+	flush_work(&dwc->drd_work);
+	ret = READ_ONCE(mux->error);
+out:
+	mutex_unlock(&mux->lock);
+	return ret;
+}
+
+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_role_mux *mux = port->dwc->role_mux;
+	unsigned long flags;
+	enum usb_role role = USB_ROLE_NONE;
+
+	mutex_lock(&mux->lock);
+	spin_lock_irqsave(&port->dwc->lock, flags);
+	if (!READ_ONCE(mux->error) && mux->active_port == port->state)
+		role = port->role;
+	spin_unlock_irqrestore(&port->dwc->lock, flags);
+	mutex_unlock(&mux->lock);
+	return role;
+}
+
+static void dwc3_role_mux_unregister(struct dwc3 *dwc)
+{
+	struct dwc3_role_mux *mux = dwc->role_mux;
+	unsigned int i;
+
+	if (!mux)
+		return;
+
+	mutex_lock(&mux->lock);
+	mux->stopping = true;
+	mutex_unlock(&mux->lock);
+	for (i = 0; i < ARRAY_SIZE(mux->ports); i++) {
+		usb_role_switch_unregister(mux->ports[i].sw);
+		mux->ports[i].sw = NULL;
+		fwnode_handle_put(mux->ports[i].fwnode);
+		mux->ports[i].fwnode = NULL;
+	}
+}
+
+/* Returns one when the optional two-connector mux is configured. */
+static int dwc3_setup_role_mux(struct dwc3 *dwc, u32 mode)
+{
+	struct usb_role_switch_desc desc = {};
+	struct fwnode_handle *ep;
+	struct dwc3_role_mux *mux;
+	struct gpio_desc *select;
+	char name[64];
+	u32 state;
+	int ret, i;
+
+	select = devm_gpiod_get_optional(dwc->dev, "port-select", GPIOD_ASIS);
+	if (IS_ERR(select))
+		return PTR_ERR(select);
+	if (!select)
+		return 0;
+	ret = gpiod_get_value_cansleep(select);
+	if (ret < 0)
+		return ret;
+
+	mux = devm_kzalloc(dwc->dev, sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return -ENOMEM;
+	mux->select = select;
+	mux->desired_port = ret;
+	mux->active_port = -1;
+	mutex_init(&mux->lock);
+	dwc->role_mux = mux;
+
+	fwnode_graph_for_each_endpoint(dev_fwnode(dwc->dev), ep) {
+		if (!fwnode_property_present(ep, "usb-role-switch"))
+			continue;
+		ret = fwnode_property_read_u32(ep, "mux-state", &state);
+		if (ret || state >= ARRAY_SIZE(mux->ports)) {
+			ret = -EINVAL;
+			goto put_ep;
+		}
+		if (mux->ports[state].fwnode) {
+			ret = -EINVAL;
+			goto put_ep;
+		}
+		mux->ports[state].fwnode = fwnode_handle_get(ep);
+		mux->ports[state].state = state;
+		mux->ports[state].dwc = dwc;
+	}
+	for (i = 0; i < ARRAY_SIZE(mux->ports); i++) {
+		if (!mux->ports[i].fwnode) {
+			ret = -EINVAL;
+			goto err;
+		}
+	}
+
+	/* All state is initialized before either role provider becomes visible. */
+	dwc3_set_mode(dwc, mode);
+	for (i = 0; i < ARRAY_SIZE(mux->ports); i++) {
+		snprintf(name, sizeof(name), "%s-port%d", dev_name(dwc->dev), i);
+		desc.name = name;
+		desc.fwnode = mux->ports[i].fwnode;
+		desc.set = dwc3_mux_role_set;
+		desc.get = dwc3_mux_role_get;
+		desc.driver_data = &mux->ports[i];
+		mux->ports[i].sw = usb_role_switch_register(dwc->dev, &desc);
+		if (IS_ERR(mux->ports[i].sw)) {
+			ret = PTR_ERR(mux->ports[i].sw);
+			goto err;
+		}
+	}
+	return 1;
+
+put_ep:
+	fwnode_handle_put(ep);
+err:
+	dwc3_drd_exit(dwc);
+	return ret;
+}
+
 static int dwc3_setup_role_switch(struct dwc3 *dwc)
 {
 	struct usb_role_switch_desc dwc3_role_switch = {NULL};
 	u32 mode;
+	int ret;
 
 	dwc->role_switch_default_mode = usb_get_role_switch_default_mode(dwc->dev);
 	if (dwc->role_switch_default_mode == USB_DR_MODE_HOST) {
@@ -513,6 +682,10 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc)
 		dwc->role_switch_default_mode = USB_DR_MODE_PERIPHERAL;
 		mode = DWC3_GCTL_PRTCAP_DEVICE;
 	}
+	ret = dwc3_setup_role_mux(dwc, mode);
+	if (ret)
+		return ret < 0 ? ret : 0;
+
 	dwc3_set_mode(dwc, mode);
 
 	dwc3_role_switch.fwnode = dev_fwnode(dwc->dev);
@@ -526,7 +699,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc)
 
 	if (dwc->dev->of_node) {
 		/* populate connector entry */
-		int ret = devm_of_platform_populate(dwc->dev);
+		ret = devm_of_platform_populate(dwc->dev);
 
 		if (ret) {
 			usb_role_switch_unregister(dwc->role_sw);
@@ -541,6 +714,7 @@ static int dwc3_setup_role_switch(struct dwc3 *dwc)
 #else
 #define ROLE_SWITCH 0
 #define dwc3_setup_role_switch(x) 0
+static inline void dwc3_role_mux_unregister(struct dwc3 *dwc) {}
 #endif
 
 int dwc3_drd_init(struct dwc3 *dwc)
@@ -597,6 +771,7 @@ void dwc3_drd_exit(struct dwc3 *dwc)
 {
 	unsigned long flags;
 
+	dwc3_role_mux_unregister(dwc);
 	if (dwc->role_sw)
 		usb_role_switch_unregister(dwc->role_sw);
 

Re: [PATCH 1/3] dt-bindings: usb: snps,dwc3: document usb-psy-name

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: formalizing OS-specific string property instead of hardware phandle
--

commit 9d9ee3558a17b54512300202ecbcaedb7bd4d866
Author: Xu Qianhui <xiaqinghuan@mail.ustc.edu.cn>

dt-bindings: usb: snps,dwc3: document usb-psy-name

This commit adds the usb-psy-name property to the snps,dwc3-common.yaml
binding schema. It aims to resolve dtbs_check failures by documenting a
property currently read by the driver to find the power supply.

> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
> index 6c0b8b6538246..6609a709c1d7b 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
[ ... ]
> @@ -51,6 +51,13 @@ properties:
>        - items:
>            pattern: "^usb(2-([0-9]|1[0-4])|3-[0-3])$"
>  
> +  usb-psy-name:
> +    description:
> +      Name of the USB power supply accepting the input current limit
> +      requested by the gadget after USB configuration, and during bus
> +      suspend and resume.
> +    $ref: /schemas/types.yaml#/definitions/string

[Severity: Medium]
Does formalizing this OS-specific string property instead of using a
hardware phandle violate Device Tree UABI guidelines?

Adding usb-psy-name to the official schema codifies an OS-specific internal
identifier into the Device Tree UABI. Hardware relationships are typically
expected to be described using standard phandle references, such as:
power-supplies = <&phandle>;

Could the driver be updated to support standard phandle properties and
treat the string-based lookup as a deprecated legacy fallback, rather than
promoting it to the official UABI schema?

> +
>    snps,usb2-lpm-disable:
>      description: Indicate if we don't want to enable USB2 HW LPM for host
>        mode.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013102927.1141696-1-xiaqinghuan@mail.ustc.edu.cn?part=1

Re: [PATCH 2/3] dt-bindings: usb: snps,dwc3: describe the two-connector HS data path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: usb: abstraction violation placing usb-role-switch in endpoint
--

commit 91efaa7a371d20cd91b98bff5337b76e3b332667
Author: Xu Qianhui <xiaqinghuan@mail.ustc.edu.cn>

dt-bindings: usb: snps,dwc3: describe the two-connector HS data path

Some designs wire the High Speed data lines of a single DWC3 controller to
two receptacles and select between them with a GPIO, while the Super Speed
lanes stay per connector. This commit describes the hardware in
snps,dwc3-common.yaml, adding port-select-gpios to select the connector
and placing mux-state and usb-role-switch inside each endpoint.

> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
> index 6609a709c1d7b..a18bb08afbae1 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
> @@ -13,6 +13,28 @@ description:
>    Defines the properties of the DWC3 core as being embedded in either an
>    vendor-specific implementation or as a standalone component.
>  
> +$defs:
> +  hs-port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    unevaluatedProperties: false
> +    properties:
> +      endpoint:
> +        $ref: /schemas/graph.yaml#/$defs/endpoint-base
> +        unevaluatedProperties: false
> +        properties:
> +          usb-role-switch:
> +            type: boolean
> +            description: Separate role-switch input for this connector.
> +          mux-state:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            enum: [0, 1]
> +            description:
> +              Logical port-select GPIO level for this data path. State 0
> +              takes precedence when both connectors request USB data.

[Severity: Medium]
Does placing usb-role-switch and mux-state inside an OF graph endpoint node
violate Devicetree abstractions?

Endpoints are strictly meant to describe connection links (like
remote-endpoint), while functional device capabilities typically belong to the
device or port node. The generic USB role switch framework also expects this
property on the device node rather than the endpoint. Could this placement
create an ABI breakage risk for the device tree?

> +        dependentRequired:
> +          usb-role-switch: [mux-state]
> +          mux-state: [usb-role-switch]
> +

[ ... ]

>  properties:
> +  usb-psy-name:
> +    description:
> +      Name of the USB power supply accepting the input current limit
> +      requested by the gadget after USB configuration, and during bus
> +      suspend and resume.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +  port-select-gpios:
> +    maxItems: 1
> +    description:
> +      Selects between two external connectors sharing the controller's HS
> +      data bus. The port@0 and port@2 endpoints specify distinct mux-state
> +      values and provide independent role-switch inputs. The controller
> +      stops the previous role before changing the data path. With neither
> +      connector requesting data, the last selection is retained.
> +

[Severity: Medium]
Does embedding external board-level hardware mux control directly into the
DWC3 IP block binding bypass the standard generic mux subsystem?

By introducing port-select-gpios directly instead of using a standard mux
node (such as gpio-mux), this creates a non-standard DT ABI that doesn't
integrate natively with the mux framework. Would it be better to model this
using the standard mux bindings?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013102927.1141696-1-xiaqinghuan@mail.ustc.edu.cn?part=2

Re: [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] pm: Unbalanced pm_runtime_put_autosuspend on error path in __dwc3_set_mode
- [High] usb: dwc3: Double-free/UAF of gadget/host resources on GPIO failure in __dwc3_set_mode
- [High] usb: dwc3: deadlock during suspend due to flush_work() on freezable wq
- [Medium] usb: dwc3: Role selection logic implements static priority, contradicting "oldest wins" design
--

commit e9f771770a6e130d4f1859d1ec3e53526b8f1f97
Author: Xu Qianhui <xiaqinghuan@mail.ustc.edu.cn>
Subject: usb: dwc3: support two connectors sharing one HS data bus

This commit introduces support for a single DWC3 controller to drive two
receptacles by muxing the High Speed data lines. It registers one USB role
switch per connector and serializes requests using a mutex, intending to
grant the role to the oldest connector.

> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index fd5c2cd36c595..76e9e5d1808f0 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -174,8 +178,15 @@ static void __dwc3_set_mode(struct work_struct *work)
>  	mutex_lock(&dwc->mutex);
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	desired_dr_role = dwc->desired_dr_role;
> +	if (mux) {
> +		mux_port = mux->desired_port;
> +		mux_role = mux->desired_role;
> +	}
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> +	if (mux && READ_ONCE(mux->error))
> +		goto out;

[Severity: High]
If this early error check evaluates to true, it jumps to the out label
before pm_runtime_get_sync() is called. The out label then unconditionally
calls pm_runtime_put_autosuspend(). 

Does this drop a Runtime PM reference that was never acquired, potentially
underflowing the usage count and powering down hardware that is still
actively in use?

> +
>  	pm_runtime_get_sync(dwc->dev);
>  
>  	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG)

[ ... ]

> @@ -233,6 +245,22 @@ static void __dwc3_set_mode(struct work_struct *work)
>  		dwc3_writel(dwc, DWC3_GCTL, reg);
>  	}
>  
> +	/*
> +	 * The previous host/gadget has been torn down above, so it is safe to
> +	 * move the shared HS data path to the requesting connector.
> +	 */
> +	if (mux && mux_port != mux->active_port) {
> +		ret = gpiod_direction_output(mux->select, mux_port);
> +		if (ret) {
> +			WRITE_ONCE(mux->error, ret);
> +			goto out;

[Severity: High]
When gpiod_direction_output() fails, this jumps to the out label after
tearing down the current role, but before dwc3_set_prtcap() is called to
update dwc->current_dr_role.

During subsequent role switches or driver removal, will dwc->current_dr_role
still evaluate to the old role? If so, this could cause dwc3_gadget_exit()
or dwc3_host_exit() to execute a second time, resulting in a double-free of
setup buffers or a use-after-free on device structures.

> +		}
> +
> +		spin_lock_irqsave(&dwc->lock, flags);

[ ... ]

> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index f3e37d383627a..4bf0497e902a7 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -501,10 +502,178 @@ static enum usb_role dwc3_usb_role_switch_get(struct usb_role_switch *sw)
>  	return role;
>  }
>  
> +static int dwc3_mux_role_set(struct usb_role_switch *sw, enum usb_role role)
> +{
> +	struct dwc3_role_port *port = usb_role_switch_get_drvdata(sw);
> +	struct dwc3 *dwc = port->dwc;
> +	struct dwc3_role_mux *mux = dwc->role_mux;
> +	enum usb_role selected_role;
> +	unsigned long flags;
> +	int selected, ret;
> +	u32 mode;
> +
> +	if (role < USB_ROLE_NONE || role > USB_ROLE_DEVICE)
> +		return -EINVAL;
> +
> +	mutex_lock(&mux->lock);
> +	if (mux->stopping) {
> +		ret = -ESHUTDOWN;
> +		goto out;
> +	}
> +	if (READ_ONCE(mux->error)) {
> +		ret = READ_ONCE(mux->error);
> +		goto out;
> +	}
> +
> +	port->role = role;
> +	if (mux->ports[0].role != USB_ROLE_NONE)
> +		selected = 0;
> +	else if (mux->ports[1].role != USB_ROLE_NONE)
> +		selected = 1;

[Severity: Medium]
The commit message states that "requests from the two switches are
serialised by a mutex, the oldest connector holding a role wins".

However, this static conditional structure seems to prioritize port 0.
If port 1 is the oldest connector actively holding a role, and port 0
subsequently requests a role, won't this check evaluate port 0 first?
Setting selected = 0 here appears to abruptly preempt port 1 and give
the role to the newer connector, breaking the intended arbitration logic.

> +	else
> +		selected = mux->desired_port;
> +	selected_role = mux->ports[selected].role;
> +	if (selected == mux->desired_port && selected_role == mux->desired_role) {
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	if (selected_role == USB_ROLE_HOST ||
> +	    (selected_role == USB_ROLE_NONE &&
> +	     dwc->role_switch_default_mode == USB_DR_MODE_HOST))
> +		mode = DWC3_GCTL_PRTCAP_HOST;
> +	else
> +		mode = DWC3_GCTL_PRTCAP_DEVICE;
> +
> +	spin_lock_irqsave(&dwc->lock, flags);
> +	mux->desired_port = selected;
> +	mux->desired_role = selected_role;
> +	dwc->desired_dr_role = mode;
> +	spin_unlock_irqrestore(&dwc->lock, flags);
> +	queue_work(system_freezable_wq, &dwc->drd_work);
> +	flush_work(&dwc->drd_work);

[Severity: High]
This code queues drd_work on system_freezable_wq and synchronously blocks
on it using flush_work().

Since role switch events can be triggered from non-freezable contexts
(such as ucsi_handle_connector_change running on a non-freezable workqueue),
what happens if a role switch occurs during system suspend after freezable
workqueues have been frozen?

Could flush_work() block indefinitely since drd_work cannot run, leading to
a complete system suspend deadlock?

> +	ret = READ_ONCE(mux->error);
> +out:
> +	mutex_unlock(&mux->lock);
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013102927.1141696-1-xiaqinghuan@mail.ustc.edu.cn?part=3