* [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus
2026-10-13 10:29 [PATCH 0/3] " Qianhui Xu
@ 2026-10-13 10:29 ` Qianhui Xu
2026-10-13 11:02 ` sashiko-bot
2026-10-14 20:19 ` kernel test robot
0 siblings, 2 replies; 10+ messages in thread
From: Qianhui Xu @ 2026-10-13 10:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thinh Nguyen, linux-usb
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
linux-arm-msm, linux-kernel, Xu Qianhui
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);
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus
2026-10-13 10:29 ` [PATCH 3/3] " Qianhui Xu
@ 2026-10-13 11:02 ` sashiko-bot
2026-10-14 20:19 ` kernel test robot
1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-10-13 11:02 UTC (permalink / raw)
To: Qianhui Xu, linux-usb; +Cc: devicetree, robh, conor+dt
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus
2026-10-13 10:29 ` [PATCH 3/3] " Qianhui Xu
2026-10-13 11:02 ` sashiko-bot
@ 2026-10-14 20:19 ` kernel test robot
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2026-10-14 20:19 UTC (permalink / raw)
To: Qianhui Xu, Greg Kroah-Hartman, Thinh Nguyen, linux-usb
Cc: llvm, oe-kbuild-all, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-arm-msm, linux-kernel, Xu Qianhui
Hi Qianhui,
kernel test robot noticed the following build warnings:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v7.3-rc7 next-20261012]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Qianhui-Xu/dt-bindings-usb-snps-dwc3-document-usb-psy-name/20261013-182925
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20261013102927.1141696-4-xiaqinghuan%40mail.ustc.edu.cn
patch subject: [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus
config: arm-defconfig (https://download.01.org/0day-ci/archive/20261015/202610150418.XE8EkA7P-lkp@intel.com/config)
compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project 6719b9e31d6a93099612c474eec76d9f41d0275a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261015/202610150418.XE8EkA7P-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610150418.XE8EkA7P-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/usb/dwc3/core.c:171:16: warning: variable 'mux_role' set but not used [-Wunused-but-set-variable]
171 | enum usb_role mux_role = USB_ROLE_NONE;
| ^
1 warning generated.
vim +/mux_role +171 drivers/usb/dwc3/core.c
165
166 static void __dwc3_set_mode(struct work_struct *work)
167 {
168 struct dwc3 *dwc = work_to_dwc(work);
169 unsigned long flags;
170 struct dwc3_role_mux *mux = dwc->role_mux;
> 171 enum usb_role mux_role = USB_ROLE_NONE;
172 int mux_port = 0;
173 int ret;
174 u32 reg;
175 u32 desired_dr_role;
176 int i;
177
178 mutex_lock(&dwc->mutex);
179 spin_lock_irqsave(&dwc->lock, flags);
180 desired_dr_role = dwc->desired_dr_role;
181 if (mux) {
182 mux_port = mux->desired_port;
183 mux_role = mux->desired_role;
184 }
185 spin_unlock_irqrestore(&dwc->lock, flags);
186
187 if (mux && READ_ONCE(mux->error))
188 goto out;
189
190 pm_runtime_get_sync(dwc->dev);
191
192 if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG)
193 dwc3_otg_update(dwc, 0);
194
195 if (!desired_dr_role)
196 goto out;
197
198 if (desired_dr_role == dwc->current_dr_role &&
199 (!mux || mux_port == mux->active_port))
200 goto out;
201
202 if (desired_dr_role == DWC3_GCTL_PRTCAP_OTG && dwc->edev)
203 goto out;
204
205 switch (dwc->current_dr_role) {
206 case DWC3_GCTL_PRTCAP_HOST:
207 dwc3_host_exit(dwc);
208 break;
209 case DWC3_GCTL_PRTCAP_DEVICE:
210 dwc3_gadget_exit(dwc);
211 dwc3_event_buffers_cleanup(dwc);
212 break;
213 case DWC3_GCTL_PRTCAP_OTG:
214 dwc3_otg_exit(dwc);
215 spin_lock_irqsave(&dwc->lock, flags);
216 dwc->desired_otg_role = DWC3_OTG_ROLE_IDLE;
217 spin_unlock_irqrestore(&dwc->lock, flags);
218 dwc3_otg_update(dwc, 1);
219 break;
220 default:
221 break;
222 }
223
224 /*
225 * When current_dr_role is not set, there's no role switching.
226 * Only perform GCTL.CoreSoftReset when there's DRD role switching.
227 */
228 if (dwc->current_dr_role && ((DWC3_IP_IS(DWC3) ||
229 DWC3_VER_IS_PRIOR(DWC31, 190A)) &&
230 desired_dr_role != DWC3_GCTL_PRTCAP_OTG)) {
231 reg = dwc3_readl(dwc, DWC3_GCTL);
232 reg |= DWC3_GCTL_CORESOFTRESET;
233 dwc3_writel(dwc, DWC3_GCTL, reg);
234
235 /*
236 * Wait for internal clocks to synchronized. DWC_usb31 and
237 * DWC_usb32 may need at least 50ms (less for DWC_usb3). To
238 * keep it consistent across different IPs, let's wait up to
239 * 100ms before clearing GCTL.CORESOFTRESET.
240 */
241 msleep(100);
242
243 reg = dwc3_readl(dwc, DWC3_GCTL);
244 reg &= ~DWC3_GCTL_CORESOFTRESET;
245 dwc3_writel(dwc, DWC3_GCTL, reg);
246 }
247
248 /*
249 * The previous host/gadget has been torn down above, so it is safe to
250 * move the shared HS data path to the requesting connector.
251 */
252 if (mux && mux_port != mux->active_port) {
253 ret = gpiod_direction_output(mux->select, mux_port);
254 if (ret) {
255 WRITE_ONCE(mux->error, ret);
256 goto out;
257 }
258
259 spin_lock_irqsave(&dwc->lock, flags);
260 mux->active_port = mux_port;
261 spin_unlock_irqrestore(&dwc->lock, flags);
262 }
263
264 spin_lock_irqsave(&dwc->lock, flags);
265
266 dwc3_set_prtcap(dwc, desired_dr_role, false);
267
268 spin_unlock_irqrestore(&dwc->lock, flags);
269
270 switch (desired_dr_role) {
271 case DWC3_GCTL_PRTCAP_HOST:
272 ret = dwc3_host_init(dwc);
273 if (ret) {
274 dev_err(dwc->dev, "failed to initialize host\n");
275 } else {
276 if (dwc->usb2_phy)
277 otg_set_vbus(dwc->usb2_phy->otg, true);
278
279 for (i = 0; i < dwc->num_usb2_ports; i++)
280 phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
281 for (i = 0; i < dwc->num_usb3_ports; i++)
282 phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
283
284 if (dwc->dis_split_quirk) {
285 reg = dwc3_readl(dwc, DWC3_GUCTL3);
286 reg |= DWC3_GUCTL3_SPLITDISABLE;
287 dwc3_writel(dwc, DWC3_GUCTL3, reg);
288 }
289 }
290 break;
291 case DWC3_GCTL_PRTCAP_DEVICE:
292 dwc3_core_soft_reset(dwc);
293
294 dwc3_event_buffers_setup(dwc);
295
296 if (dwc->usb2_phy)
297 otg_set_vbus(dwc->usb2_phy->otg, false);
298 phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
299 phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
300
301 ret = dwc3_gadget_init(dwc);
302 if (ret)
303 dev_err(dwc->dev, "failed to initialize peripheral\n");
304 break;
305 case DWC3_GCTL_PRTCAP_OTG:
306 dwc3_otg_init(dwc);
307 dwc3_otg_update(dwc, 0);
308 break;
309 default:
310 break;
311 }
312
313 out:
314 pm_runtime_put_autosuspend(dwc->dev);
315 mutex_unlock(&dwc->mutex);
316 }
317
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 0/3] usb: dwc3: two Type-C connectors sharing one HS data bus
@ 2026-11-04 3:27 Qianhui Xu
2026-11-04 3:27 ` [PATCH 1/3] dt-bindings: usb: snps,dwc3: describe two connectors sharing one HS bus Qianhui Xu
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Qianhui Xu @ 2026-11-04 3:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thinh Nguyen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-usb, devicetree, linux-arm-msm, linux-kernel
*** AI-assistance disclosure ***
This cover letter and the patches below were prepared with the help of an
LLM coding assistant. The author reviewed every line, ran every gate
quoted in this letter himself, and takes full responsibility for the
content.
This is one complete, self-contained series of three patches: the binding,
the role-switch lookup it needs, and the DWC3 arbitration between the two
connectors. Nothing is to be applied on top of it, and no patch of it is
posted separately. It supersedes the v4 series
(<20261026123211.243224-1-xiaqinghuan@mail.ustc.edu.cn>) and the v5
fixups (<20261031150154.1106455-1-xiaqinghuan@mail.ustc.edu.cn>) in
full: their content is folded into the three patches below.
Two connectors on one DWC3 controller
=====================================
The board (Lenovo Legion Y700 4th gen, SM8750) wires the Super Speed pairs
of two USB-C receptacles to separate PHYs, while the High Speed D+/D- pair
is shared through a 2:1 mux controlled by a single GPIO. Both connectors
are data-role dual and hand their role to the SoC through pmic-glink, so
the controller has to arbitrate between two independent role requests and
move the shared data path before it is reinitialised.
usb@a600000 {
port-select-gpios = <&tlmm 29 GPIO_ACTIVE_HIGH>;
ports {
port@0 { /* HS path, connector 0 */
endpoint {
remote-endpoint = <&usb_0_dwc3_hs>;
usb-role-switch;
mux-state = <0>;
};
};
port@1 { /* Super Speed, QMP PHY */
};
port@2 { /* HS path, connector 1 */
endpoint {
remote-endpoint = <&usb_1_dwc3_hs>;
usb-role-switch;
mux-state = <1>;
};
};
};
};
Series layout
=============
1/3 documents that wiring: "port-select-gpios", one High Speed port per
connector (port@0 and port@2, port@1 being the Super Speed lane), and,
on the endpoint of each of those ports, "mux-state" (the GPIO level
that reaches that connector) and "usb-role-switch" (this connector has
its own role-switch input).
2/3 adds usb_role_switch_at_endpoint(), so a role switch registered
against an OF graph endpoint can be found by the consumer that owns
the other end of that link. Without it the endpoint registration in
1/3 would have no lookup path.
3/3 implements the arbitration in DWC3: one role switch per connector, the
oldest pending request wins, the GPIO is flipped inside
__dwc3_set_mode() after the previous host or gadget instance is torn
down, and a connector reads back the role it requested.
Reply to the two procedural comments
====================================
Krishna Kurapati wrote, on the v5 posting: "If v5 is being sent, it must
not to be applied on top of v4. It must include (v4 + any additional
changes/improvements)." Agreed, and that is what this series is. v4
1/3..3/3 and the v5 fixups were squashed per patch instead of stacked:
1/3 = v4 1/3 + v5 1/2, 2/3 = v4 2/3 + the usb_role_switch_at_endpoint()
hunk of v5 2/2, 3/3 = v4 3/3 + the remaining hunks of v5 2/2. Applying
this series to a v7.3-rc7 tree gives the same three source files as
"v4 + v5", and there is no "1/2 on top of 3/3" left anywhere.
Krzysztof Kozlowski wrote: "Anyway, this is misthreaded - missing rest of
patchset. Please read submitting patches document and switch to b4", and
separately dropped v4 1/3 ("Unanswered Sashiko comments plus some
confusion with duplicated posting of v5 without the rest, dropping").
Both points are taken. Posting v5 as a two-patch fixup and then re-sending
only its 1/2 on 11-02 was wrong: a series has to be complete and threaded.
This posting is the complete series as one thread -- cover plus 1/3..3/3,
"base-commit" at the bottom, and In-Reply-To pointing at this cover only,
so it applies with `b4 shazam` (or `git am`) as-is. Nothing is re-sent
piecemeal; the 11-02 1/2 has no successor and should be ignored. On the
"git log" remark: the per-revision account now lives only here, the commit
messages describe the code as it finally is.
Dropping v4 1/3 is fine: this 1/3 replaces it, together with the schema
holes that were reported against it. No action is needed to "un-drop" it.
Response to the review comments folded into these patches
=========================================================
Sashiko on the binding (three Medium findings):
- "hs-port patternProperties does not match what the driver expects /
escapes unevaluatedProperties": fixed, the endpoint name pattern is the
one graph.yaml uses, so an unnumbered endpoint or endpoint@2+ is
validated too (1/3).
- "usb-role-switch should not be required only indirectly": fixed, the
controller node must carry "usb-role-switch" whenever port-select-gpios
is present, and dr_mode is required to be "otg". This is a driver
fact, not schema cosmetics: without it dwc3_get_dr_mode() forces dr_mode
back to peripheral, dwc3_get_extcon() skips the role-switch branch and
dwc3_drd_init() never sets the mux up, so a DT that passed the old
schema did not describe a DT the driver could act on (1/3).
- "mux-state complementarity is not expressed": fixed, the pair is one
oneOf over the two assignments, so two connectors both claiming
mux-state = <1> are rejected (1/3).
- standing question, "model the board mux with the standard
mux-controls/mux-states properties": kept the graph-endpoint shape. The
arbitration is "earliest request wins" between two 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 has no per-connector
identity to hang that on, and drivers/usb/ has no mux_control consumer
to follow as precedent. Happy to reshape if the DT maintainers prefer
the standard property.
Sashiko on usb: roles (one High finding):
- "-EPROBE_DEFER is swallowed by !IS_ERR_OR_NULL(sw)": fixed, the error is
passed up, as suggested (2/3). This was also the root cause we hit on
real hardware: the UCSI glue kept a NULL switch and the connectors never
bound.
Sashiko on usb: dwc3 (five High and three Medium findings):
- "dwc3_set_mode() under dwc->lock self-deadlocks": fixed, the request
decision and its issue are paired by mux->lock, and dwc->lock is only
held while individual fields are updated (3/3).
- "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 (3/3).
- "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 (3/3).
- "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 (3/3).
- "fwnode references leak on error paths": fixed, dwc->role_mux is
published before the error paths that used to leak them, so
dwc3_role_mux_exit() can unwind (3/3).
- "sticky mux->error is reported to the next caller after it already
committed its own request": fixed, the sticky value is gone; a failed
data-path switch is logged and counted per connector (3/3).
- found while testing on the board, same area: dwc3_mux_role_get()
returned USB_ROLE_NONE for a connector that did not own the data path,
which hid its pending request; the connector driver then never sent the
set_role(USB_ROLE_NONE) that clears it, and the stale request kept
winning arbitration after unplug. get() now reports the requested role
(3/3).
- "add dwc3_pre_set_role()" (raised against v2): this is exactly what the
folded code does now, both in the single-connector and the mux path.
Changes relative to the previous postings
=========================================
- v4 1/3..3/3 -> this series: 5 files changed, 143 insertions(+),
80 deletions(-) (binding 1/3 and core.c/core.h/drd.c/class.c), which
is the folded v5 fixup plus one documentation fix: after the reshape to
port@0 + port@2 the "port-select-gpios" description still said
"a numbered endpoint of port@0"; it now names port@0/port@2 and their
endpoint. Description only, no schema change.
- v5 1/2..2/2 -> this series: 3 lines of that description, nothing else;
v5's code changes are bit-identical inside 1/3, 2/3 and 3/3.
Testing
=======
Mechanical, all re-run on this exact series:
- checkpatch.pl --strict --patch - (mbox on stdin): 0 errors, 0 warnings,
0 checks for each of the three patches (138 / 63 / 441 lines).
- dt-doc-validate on snps,dwc3-common.yaml: clean, exit 0.
- make W=1 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- for
drivers/usb/dwc3/ and drivers/usb/roles/ with all 19 dwc3 glue drivers
built as modules: 0 warnings, 0 errors.
- git am of 1/3..3/3 on 24a19b953e: applies cleanly patch by patch, and the
resulting tree is identical to the series tip.
- scripts/check-series-selfcontained.sh on the series directory against
the pristine base: PASS, 3/3 patches replayed in order.
- git send-email --dry-run of the produced mbox set: 1 cover + 3 patches,
threaded with In-Reply-To on the cover only.
Board behaviour: the same driver source runs on this board's daily kernel
and registers both per-connector switches
(a600000.usb-port0-role-switch, a600000.usb-port1-role-switch). With
function_graph tracing, 12 consecutive plug/unplug cycles each went
usb_role_switch_set_role -> dwc3_mux_role_set -> glue notifier ->
dwc3_set_mode with zero WARNING/BUG/lockdep splats; USB data and charging
were stable throughout, and /sys/class/typec shows port0, port1 and a
bound partner.
Two honest limitations: (a) the arbitration detail -- which connector wins
a simultaneous request -- is verified by code reading and by the stale
request test above, not by a complete per-port role-value trace; (b) the
behaviour evidence comes from that daily kernel, not from a v7.3-rc7
kernel with this series applied, because pristine-lineage kernels do not
bring this board up yet for reasons unrelated to USB (no display/platform
support). I can re-run the plug sequence on request.
Links
=====
v4: https://lore.kernel.org/linux-usb/20261026123211.243224-1-xiaqinghuan@mail.ustc.edu.cn/
v5: https://lore.kernel.org/linux-usb/20261031150154.1106455-1-xiaqinghuan@mail.ustc.edu.cn/
Krishna's reply: <ba544091-1da4-4616-b1d5-fe23f32679d9@oss.qualcomm.com>
Krzysztof's reply: <20261102-armored-arcane-serval-43faf4@quoll>
v4 1/3 dropped: <20261102-screeching-utopian-avocet-492500@quoll>
Qianhui Xu (3):
dt-bindings: usb: snps,dwc3: describe two connectors sharing one HS
bus
usb: roles: look up role switches attached to a graph endpoint
usb: dwc3: support two connectors sharing one HS data bus
.../bindings/usb/snps,dwc3-common.yaml | 116 +++++++-
drivers/usb/dwc3/core.c | 47 +++-
drivers/usb/dwc3/core.h | 39 +++
drivers/usb/dwc3/debugfs.c | 4 +
drivers/usb/dwc3/drd.c | 254 +++++++++++++++++-
drivers/usb/roles/class.c | 45 ++++
6 files changed, 501 insertions(+), 4 deletions(-)
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] dt-bindings: usb: snps,dwc3: describe two connectors sharing one HS bus
2026-11-04 3:27 [PATCH 0/3] usb: dwc3: two Type-C connectors sharing one HS data bus Qianhui Xu
@ 2026-11-04 3:27 ` Qianhui Xu
2026-11-04 3:44 ` sashiko-bot
2026-11-04 3:27 ` [PATCH 2/3] usb: roles: look up role switches attached to a graph endpoint Qianhui Xu
2026-11-04 3:27 ` [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus Qianhui Xu
2 siblings, 1 reply; 10+ messages in thread
From: Qianhui Xu @ 2026-11-04 3:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thinh Nguyen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-usb, devicetree, linux-arm-msm, linux-kernel
Two connectors can share the High Speed D+/D- pair of one DWC3 controller,
with a GPIO selecting which of them is connected, while their Super Speed
lanes stay separate.
Describe that in snps,dwc3-common.yaml:
- "port-select-gpios" selects which connector drives the shared bus;
- each connector is a port of the controller's "ports" node, "port@0"
and "port@2", with one endpoint per port, so that "ports" keeps
describing the controller's own buses as the graph bindings expect
(port@1 is the Super Speed lane of the PHY, described by
snps,dwc3.yaml);
- the endpoint of a connector carries "mux-state", the level of the
port-select GPIO that reaches it, and "usb-role-switch", declaring
that this connector has its own role-switch input.
The controller node must carry "usb-role-switch" as well, and dr_mode is
required to be "otg": dwc3_get_dr_mode() otherwise forces dr_mode back to
peripheral, dwc3_get_extcon() skips the role-switch branch and
dwc3_drd_init() never sets the mux up, so a device tree that passed the
schema would not describe a device tree the driver can act on.
The two connectors must claim opposite levels of the GPIO, which is
expressed as one choice between the two assignments, and endpoint names
use the pattern of graph.yaml so that an unnumbered or third endpoint is
rejected instead of escaping "unevaluatedProperties: false". When
port-select-gpios is absent the endpoint properties are rejected, so
existing single-connector descriptions are unaffected.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
.../bindings/usb/snps,dwc3-common.yaml | 116 +++++++++++++++++-
1 file changed, 114 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
index 6c0b8b6538..0be2a6e83e 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
@@ -13,6 +13,40 @@ description:
Defines the properties of the DWC3 core as being embedded in either an
vendor-specific implementation or as a standalone component.
+$defs:
+ hs-endpoint:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ unevaluatedProperties: false
+
+ properties:
+ usb-role-switch:
+ type: boolean
+ description:
+ This connector endpoint has its own USB role switch. The switch is
+ registered against this endpoint and looked up with
+ usb_role_switch_at_endpoint().
+
+ mux-state:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1]
+ description:
+ Logical level of the port-select GPIO which connects this
+ 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]
+ mux-state: [usb-role-switch]
+
+ hs-port:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+
+ patternProperties:
+ '^endpoint(@[0-9a-f]+)?$':
+ $ref: '#/$defs/hs-endpoint'
+
allOf:
- $ref: usb-drd.yaml#
- if:
@@ -26,8 +60,87 @@ allOf:
$ref: usb.yaml#
else:
$ref: usb-xhci.yaml#
+ - if:
+ required: [port-select-gpios]
+ then:
+ # 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:
+ # 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:
+ $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:
+ port@0:
+ $ref: '#/$defs/hs-port'
+ properties:
+ endpoint:
+ properties:
+ mux-state:
+ const: 0
+ port@2:
+ $ref: '#/$defs/hs-port'
+ properties:
+ endpoint:
+ properties:
+ mux-state:
+ const: 1
+ - required: [port@0, port@2]
+ properties:
+ port@0:
+ $ref: '#/$defs/hs-port'
+ properties:
+ endpoint:
+ properties:
+ mux-state:
+ const: 1
+ port@2:
+ $ref: '#/$defs/hs-port'
+ properties:
+ endpoint:
+ properties:
+ mux-state:
+ const: 0
+ else:
+ properties:
+ ports:
+ properties:
+ port@0:
+ patternProperties:
+ '^endpoint(@[0-9a-f]+)?$':
+ properties:
+ usb-role-switch: false
+ mux-state: false
properties:
+ port-select-gpios:
+ maxItems: 1
+ description:
+ Selects which of the two connectors drives the controller's shared High
+ Speed data bus. Each connector is described by "port@0" or "port@2",
+ and the endpoint of that port carries the connector's own role-switch
+ input and the "mux-state" level selecting it.
+
extcon:
maxItems: 1
deprecated: true
@@ -411,7 +524,7 @@ properties:
properties:
port@0:
- $ref: /schemas/graph.yaml#/properties/port
+ $ref: '#/$defs/hs-port'
description: High Speed (HS) data bus.
port@1:
@@ -429,4 +542,3 @@ required:
additionalProperties: true
...
-
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] usb: roles: look up role switches attached to a graph endpoint
2026-11-04 3:27 [PATCH 0/3] usb: dwc3: two Type-C connectors sharing one HS data bus Qianhui Xu
2026-11-04 3:27 ` [PATCH 1/3] dt-bindings: usb: snps,dwc3: describe two connectors sharing one HS bus Qianhui Xu
@ 2026-11-04 3:27 ` Qianhui Xu
2026-11-04 3:34 ` sashiko-bot
2026-11-04 3:27 ` [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus Qianhui Xu
2 siblings, 1 reply; 10+ messages in thread
From: Qianhui Xu @ 2026-11-04 3:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thinh Nguyen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-usb, devicetree, linux-arm-msm, linux-kernel
usb_role_switch_get() and fwnode_usb_role_switch_get() look for a switch
on the caller's parent device node or through device_connection. That does
not work when one controller exposes a role switch per connector endpoint:
the switch is registered against the remote graph endpoint and the
consumer, the connector, has no phandle to it.
Add usb_role_switch_at_endpoint(), which walks the caller's OF graph
endpoints and matches a switch registered on the remote endpoint, and try
it after the existing parent lookup. A switch that is described by the
graph but not registered yet comes back as -EPROBE_DEFER, as the rest of
the role-switch API does, so the caller retries instead of concluding that
the connector has no role switch at all. There is no behaviour change for
existing users: if nothing is registered against an endpoint the lookup
falls through to the previous paths.
This is needed by DWC3 designs where two Type-C receptacles share one High
Speed data bus.
Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
drivers/usb/roles/class.c | 45 +++++++++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index edec139b68..ee186d542a 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -157,6 +157,47 @@ usb_role_switch_is_parent(struct fwnode_handle *fwnode)
return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
}
+/*
+ * Look up a role switch registered against the remote endpoint of one of the
+ * caller's OF graph endpoints. This is how a connector endpoint that shares a
+ * USB data bus with another connector (for example two Type-C receptacles on a
+ * single DWC3 controller, selected by a GPIO mux) finds its own role switch.
+ */
+static struct usb_role_switch *
+usb_role_switch_at_endpoint(struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *ep, *remote, *parent;
+ struct usb_role_switch *sw;
+ bool available;
+
+ fwnode_graph_for_each_endpoint(fwnode, ep) {
+ parent = fwnode_graph_get_remote_port_parent(ep);
+ if (!parent)
+ continue;
+ available = fwnode_device_is_available(parent);
+ fwnode_handle_put(parent);
+ if (!available)
+ continue;
+
+ remote = fwnode_graph_get_remote_endpoint(ep);
+ if (!remote)
+ continue;
+ sw = usb_role_switch_match(remote, "usb-role-switch", NULL);
+ fwnode_handle_put(remote);
+ /*
+ * 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;
+ }
+ }
+
+ return NULL;
+}
+
/**
* usb_role_switch_get - Find USB role switch linked with the caller
* @dev: The caller device
@@ -169,6 +210,8 @@ struct usb_role_switch *usb_role_switch_get(struct device *dev)
struct usb_role_switch *sw;
sw = usb_role_switch_is_parent(dev_fwnode(dev));
+ if (!sw)
+ sw = usb_role_switch_at_endpoint(dev_fwnode(dev));
if (!sw)
sw = device_connection_find_match(dev, "usb-role-switch", NULL,
usb_role_switch_match);
@@ -192,6 +235,8 @@ struct usb_role_switch *fwnode_usb_role_switch_get(struct fwnode_handle *fwnode)
struct usb_role_switch *sw;
sw = usb_role_switch_is_parent(fwnode);
+ if (!sw)
+ sw = usb_role_switch_at_endpoint(fwnode);
if (!sw)
sw = fwnode_connection_find_match(fwnode, "usb-role-switch",
NULL, usb_role_switch_match);
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus
2026-11-04 3:27 [PATCH 0/3] usb: dwc3: two Type-C connectors sharing one HS data bus Qianhui Xu
2026-11-04 3:27 ` [PATCH 1/3] dt-bindings: usb: snps,dwc3: describe two connectors sharing one HS bus Qianhui Xu
2026-11-04 3:27 ` [PATCH 2/3] usb: roles: look up role switches attached to a graph endpoint Qianhui Xu
@ 2026-11-04 3:27 ` Qianhui Xu
2026-11-04 3:44 ` sashiko-bot
2 siblings, 1 reply; 10+ messages in thread
From: Qianhui Xu @ 2026-11-04 3:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Thinh Nguyen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-usb, devicetree, linux-arm-msm, linux-kernel
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. Such a controller has to service
two independent role-switch inputs, and the shared data path has to be
moved before the controller is reinitialised for the requested role.
Register one usb_role_switch per connector endpoint described by
"port-select-gpios" and the per-endpoint "mux-state" property. Requests
from the two switches are serialised by the mux lock, the oldest pending
request wins, and 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 same-role request that moves the data path to
the other connector reinitialises the controller as well and starts it
from the same SUSPHY state as a role change.
A connector reads back the role it requested, not the role currently on the
bus: hiding a pending request behind the active port would leave the
connector driver unable to release it, and the stale request would keep
winning the arbitration after unplug. The data-path selection and the mode
request it implies are decided under one lock, and the glue layer is
notified with dwc3_pre_set_role() before dwc3_set_mode(), outside
spinlocks, as the single-connector path does.
A failed port-select write is logged and counted per connector; it cannot
leave a torn-down role behind, because __dwc3_set_mode() is the only place
that programs hardware and it never aborts half-way.
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: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
---
drivers/usb/dwc3/core.c | 47 ++++++-
drivers/usb/dwc3/core.h | 39 ++++++
drivers/usb/dwc3/debugfs.c | 4 +
drivers/usb/dwc3/drd.c | 254 ++++++++++++++++++++++++++++++++++++-
4 files changed, 342 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index fd5c2cd36c..da609a011d 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>
@@ -165,7 +166,10 @@ EXPORT_SYMBOL_GPL(dwc3_set_prtcap);
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;
u32 desired_dr_role;
@@ -174,6 +178,8 @@ 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;
spin_unlock_irqrestore(&dwc->lock, flags);
pm_runtime_get_sync(dwc->dev);
@@ -184,7 +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)
+ /*
+ * 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)
@@ -233,8 +246,40 @@ static void __dwc3_set_mode(struct work_struct *work)
dwc3_writel(dwc, DWC3_GCTL, reg);
}
+ /*
+ * The previous host or gadget instance has been torn down above, so the
+ * shared High Speed data path can now be moved to the requesting
+ * connector. A failure here must not abort the mode change half-way:
+ * 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 (port_switch) {
+ ret = gpiod_direction_output(mux->select, mux_port);
+ if (ret)
+ dev_err_ratelimited(dwc->dev,
+ "failed to select USB port %d: %d\n",
+ mux_port, ret);
+
+ 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_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 f3dabb37f8..eae321e2d7 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1008,6 +1008,43 @@ struct dwc3_glue_ops {
void (*pre_run_stop)(struct dwc3 *dwc, bool is_on);
};
+/* Per-connector state of the optional shared-HS-data-bus mux. */
+struct dwc3_role_port {
+ struct dwc3 *dwc;
+ struct usb_role_switch *sw;
+ struct fwnode_handle *fwnode;
+ enum usb_role role;
+ unsigned int state;
+ /* request order, see dwc3_role_mux.seq */
+ u64 seq;
+ /* data-path switch failures, for diagnostics only */
+ unsigned int errors;
+};
+
+/**
+ * struct dwc3_role_mux - optional data-path mux between two connectors
+ * @select: GPIO selecting which connector drives the shared HS data lines
+ * @lock: serialises role requests coming from both connectors
+ * @ports: per-connector state, indexed by the endpoint's "mux-state"
+ * @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
+ *
+ * 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
+ * role-switch input, and the shared data path has to be moved before the
+ * controller is reinitialised for the requested role.
+ */
+struct dwc3_role_mux {
+ struct gpio_desc *select;
+ /* serialises role requests coming from both connectors */
+ struct mutex lock;
+ struct dwc3_role_port ports[2];
+ int desired_port;
+ int active_port;
+ u64 seq;
+};
+
/**
* struct dwc3 - representation of our controller
* @drd_work: workqueue used for role swapping
@@ -1064,6 +1101,7 @@ struct dwc3_glue_ops {
* - USBPHY_INTERFACE_MODE_UTMI
* - USBPHY_INTERFACE_MODE_UTMIW
* @role_sw: usb_role_switch handle
+ * @role_mux: optional shared data-path mux plus 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 +1303,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..0d42f10988 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 the role and the 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..53106bed3e 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,254 @@ static enum usb_role dwc3_usb_role_switch_get(struct usb_role_switch *sw)
return role;
}
+/*
+ * Index of the connector that should own the shared data path: the oldest
+ * pending request wins, and the last selection is kept when neither connector
+ * requests a role. Requests are ordered by a monotonic counter rather than by
+ * a timestamp, so a wrapped counter cannot be mistaken for an old request.
+ */
+static int dwc3_role_mux_pick(struct dwc3_role_mux *mux)
+{
+ int sel = mux->desired_port;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(mux->ports); i++) {
+ if (mux->ports[i].role == USB_ROLE_NONE)
+ continue;
+ if (mux->ports[sel].role == USB_ROLE_NONE ||
+ mux->ports[i].seq < mux->ports[sel].seq)
+ sel = i;
+ }
+
+ return sel;
+}
+
+static u32 dwc3_role_mux_mode(struct dwc3 *dwc, enum usb_role role)
+{
+ if (role == USB_ROLE_HOST)
+ return DWC3_GCTL_PRTCAP_HOST;
+ if (role == USB_ROLE_DEVICE)
+ return DWC3_GCTL_PRTCAP_DEVICE;
+ if (dwc->role_switch_default_mode == USB_DR_MODE_HOST)
+ return DWC3_GCTL_PRTCAP_HOST;
+
+ return DWC3_GCTL_PRTCAP_DEVICE;
+}
+
+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;
+ unsigned long flags;
+ enum usb_role selected_role;
+ int selected;
+
+ if (role > USB_ROLE_DEVICE)
+ return -EINVAL;
+
+ /*
+ * The request order and the mode request it implies are decided under one
+ * lock, so a second connector cannot change desired_port between the
+ * selection and the dwc3_set_mode() call that consumes it.
+ */
+ mutex_lock(&mux->lock);
+ spin_lock_irqsave(&dwc->lock, flags);
+ /*
+ * A role change between two non-NONE roles of the same connector must not
+ * give that connector a new request ticket: it would otherwise steal the
+ * data path from the connector that asked first.
+ */
+ if (port->role == USB_ROLE_NONE && role != USB_ROLE_NONE)
+ port->seq = ++mux->seq;
+ port->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_pre_set_role(dwc, selected_role);
+ dwc3_set_mode(dwc, dwc3_role_mux_mode(dwc, selected_role));
+
+ mutex_unlock(&mux->lock);
+
+ /*
+ * 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;
+ unsigned long flags;
+ enum usb_role role;
+
+ /*
+ * 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);
+ role = port->role;
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
+ return role;
+}
+
+static void dwc3_role_mux_exit(struct dwc3 *dwc)
+{
+ struct dwc3_role_mux *mux = dwc->role_mux;
+ unsigned int i;
+
+ if (!mux)
+ return;
+
+ for (i = 0; i < ARRAY_SIZE(mux->ports); i++) {
+ if (mux->ports[i].sw)
+ 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 1 when the optional two-connector mux has been set up. */
+static int dwc3_role_mux_init(struct dwc3 *dwc, u32 mode)
+{
+ struct usb_role_switch_desc desc = {};
+ struct dwc3_role_mux *mux;
+ struct fwnode_handle *ep;
+ 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;
+
+ mux = devm_kzalloc(dwc->dev, sizeof(*mux), GFP_KERNEL);
+ if (!mux)
+ return -ENOMEM;
+
+ ret = gpiod_get_value_cansleep(select);
+ if (ret < 0)
+ return ret;
+
+ mux->select = select;
+ mux->desired_port = ret;
+ 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;
+
+ ret = fwnode_property_read_u32(ep, "mux-state", &state);
+ if (ret || state >= ARRAY_SIZE(mux->ports)) {
+ ret = -EINVAL;
+ goto err_put_ep;
+ }
+ if (mux->ports[state].fwnode) {
+ ret = -EINVAL;
+ goto err_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;
+ }
+ }
+
+ /*
+ * 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);
+ 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);
+ mux->ports[i].sw = NULL;
+ mutex_unlock(&mux->lock);
+ goto err;
+ }
+ }
+
+ /*
+ * The controller is only started in its default role once both switches
+ * are registered: before that a connector driver cannot request a role,
+ * so nothing can race with this initial mode request.
+ */
+ dwc3_set_mode(dwc, mode);
+
+ mutex_unlock(&mux->lock);
+
+ return 1;
+
+err:
+ /* 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);
+ goto err;
+}
+
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 +758,11 @@ 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_role_mux_init(dwc, mode);
+ if (ret)
+ return ret < 0 ? ret : 0;
+
dwc3_set_mode(dwc, mode);
dwc3_role_switch.fwnode = dev_fwnode(dwc->dev);
@@ -526,7 +776,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 +791,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_exit(struct dwc3 *dwc) {}
#endif
int dwc3_drd_init(struct dwc3 *dwc)
@@ -597,6 +848,7 @@ void dwc3_drd_exit(struct dwc3 *dwc)
{
unsigned long flags;
+ dwc3_role_mux_exit(dwc);
if (dwc->role_sw)
usb_role_switch_unregister(dwc->role_sw);
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: roles: look up role switches attached to a graph endpoint
2026-11-04 3:27 ` [PATCH 2/3] usb: roles: look up role switches attached to a graph endpoint Qianhui Xu
@ 2026-11-04 3:34 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-04 3:34 UTC (permalink / raw)
To: Qianhui Xu; +Cc: linux-usb
> usb_role_switch_get() and fwnode_usb_role_switch_get() look for a switch
> on the caller's parent device node or through device_connection. That does
> not work when one controller exposes a role switch per connector endpoint:
> the switch is registered against the remote graph endpoint and the
> consumer, the connector, has no phandle to it.
>
> Add usb_role_switch_at_endpoint(), which walks the caller's OF graph
> endpoints and matches a switch registered on the remote endpoint, and try
> it after the existing parent lookup. A switch that is described by the
> graph but not registered yet comes back as -EPROBE_DEFER, as the rest of
> the role-switch API does, so the caller retries instead of concluding that
> the connector has no role switch at all. There is no behaviour change for
> existing users: if nothing is registered against an endpoint the lookup
> falls through to the previous paths.
>
> This is needed by DWC3 designs where two Type-C receptacles share one High
> Speed data bus.
>
> Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104032706.210042-1-xiaqinghuan@mail.ustc.edu.cn?part=2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus
2026-11-04 3:27 ` [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus Qianhui Xu
@ 2026-11-04 3:44 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-04 3:44 UTC (permalink / raw)
To: Qianhui Xu; +Cc: linux-usb, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] usb: dwc3: race and reversed teardown order in dwc3_role_mux_init()
- [High] usb: dwc3: torn port and role state in dwc3_mux_role_set()
--
commit cac2c305bab06e293cb082a796ad2c865c932a9d
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
usb: dwc3: support two connectors sharing one HS data bus
This commit adds support for DWC3 controllers where two USB connectors share
a single High Speed data bus via a GPIO-controlled mux while using separate
Super Speed PHYs. It registers a usb_role_switch per connector endpoint and
arbitrates role requests and GPIO port selection between the two ports.
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index f3e37d383627a..53106bed3eb95 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
[ ... ]
> @@ -501,10 +502,254 @@ static enum usb_role dwc3_usb_role_switch_get(struct usb_role_switch *sw)
[ ... ]
> +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;
> + unsigned long flags;
> + enum usb_role selected_role;
> + int selected;
> +
> + if (role > USB_ROLE_DEVICE)
> + return -EINVAL;
> +
> + /*
> + * The request order and the mode request it implies are decided under one
> + * lock, so a second connector cannot change desired_port between the
> + * selection and the dwc3_set_mode() call that consumes it.
> + */
> + mutex_lock(&mux->lock);
> + spin_lock_irqsave(&dwc->lock, flags);
> + /*
> + * A role change between two non-NONE roles of the same connector must not
> + * give that connector a new request ticket: it would otherwise steal the
> + * data path from the connector that asked first.
> + */
> + if (port->role == USB_ROLE_NONE && role != USB_ROLE_NONE)
> + port->seq = ++mux->seq;
> + port->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_pre_set_role(dwc, selected_role);
> + dwc3_set_mode(dwc, dwc3_role_mux_mode(dwc, selected_role));
[Severity: High]
Can __dwc3_set_mode() observe a torn update between mux->desired_port and
dwc->desired_dr_role here?
In dwc3_mux_role_set(), mux->desired_port is updated under dwc->lock, and
then dwc->lock is dropped across dwc3_pre_set_role() (which may sleep) before
dwc3_set_mode() re-acquires dwc->lock to update dwc->desired_dr_role.
Meanwhile, __dwc3_set_mode() in drivers/usb/dwc3/core.c does not acquire
mux->lock when snapshotting those two fields:
mutex_lock(&dwc->mutex);
spin_lock_irqsave(&dwc->lock, flags);
desired_dr_role = dwc->desired_dr_role;
if (mux)
mux_port = mux->desired_port;
spin_unlock_irqrestore(&dwc->lock, flags);
If dwc->drd_work is already pending on system_freezable_wq from an earlier
dwc3_set_mode() call and starts executing __dwc3_set_mode() while
dwc3_mux_role_set() is sleeping in dwc3_pre_set_role(), wouldn't
__dwc3_set_mode() read the new mux->desired_port paired with the previous
dwc->desired_dr_role, switching the port-select GPIO to the new connector
while initializing the controller in the old role before dwc3_pre_set_role()
has completed?
> +
> + mutex_unlock(&mux->lock);
> +
> + /*
> + * 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 void dwc3_role_mux_exit(struct dwc3 *dwc)
> +{
> + struct dwc3_role_mux *mux = dwc->role_mux;
> + unsigned int i;
> +
> + if (!mux)
> + return;
> +
> + for (i = 0; i < ARRAY_SIZE(mux->ports); i++) {
> + if (mux->ports[i].sw)
> + 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 1 when the optional two-connector mux has been set up. */
> +static int dwc3_role_mux_init(struct dwc3 *dwc, u32 mode)
> +{
[ ... ]
> + 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);
> + 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);
> + mux->ports[i].sw = NULL;
> + mutex_unlock(&mux->lock);
> + goto err;
> + }
> + }
> +
> + /*
> + * The controller is only started in its default role once both switches
> + * are registered: before that a connector driver cannot request a role,
> + * so nothing can race with this initial mode request.
> + */
> + dwc3_set_mode(dwc, mode);
> +
> + mutex_unlock(&mux->lock);
> +
> + return 1;
> +
> +err:
> + /* 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;
[Severity: High]
What happens if usb_role_switch_register() succeeds for port 0 and fails for
port 1 in dwc3_role_mux_init()?
Once mux->ports[0].sw is registered, port 0's connector driver can immediately
look it up and call usb_role_switch_set_role() -> dwc3_mux_role_set(). While
dwc3_role_mux_init() holds mux->lock, that call blocks on
mutex_lock(&mux->lock).
When registration of port 1 fails, dwc3_role_mux_init() unlocks mux->lock
while mux->ports[0].sw is still registered, and at the err label calls
cancel_work_sync(&dwc->drd_work) before dwc3_role_mux_exit(dwc) unregisters
mux->ports[0].sw.
Doesn't that allow a blocked or concurrent dwc3_mux_role_set() on port 0 to:
1. Queue dwc->drd_work via dwc3_set_mode() after cancel_work_sync() has
already returned, leaving __dwc3_set_mode() queued or running on devm-managed
struct dwc3 and struct dwc3_role_mux after probe fails and frees them?
2. Enter dwc3_mux_role_set() just before usb_role_switch_unregister() in
dwc3_role_mux_exit() (which does not synchronize with mux->lock) and read
dwc->role_mux after err sets dwc->role_mux = NULL, dereferencing a NULL mux
pointer in mutex_lock(&mux->lock)?
3. Run __dwc3_set_mode() before cancel_work_sync() finishes and initialize
host or gadget mode (dwc3_host_init() / dwc3_gadget_init()), which is never
cleaned up because dwc3_core_probe() -> dwc3_core_init_mode() jumps to
err_exit_debugfs on failure without calling dwc3_core_exit_mode() or
dwc3_drd_exit()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104032706.210042-1-xiaqinghuan@mail.ustc.edu.cn?part=3
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] dt-bindings: usb: snps,dwc3: describe two connectors sharing one HS bus
2026-11-04 3:27 ` [PATCH 1/3] dt-bindings: usb: snps,dwc3: describe two connectors sharing one HS bus Qianhui Xu
@ 2026-11-04 3:44 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-04 3:44 UTC (permalink / raw)
To: Qianhui Xu; +Cc: linux-usb, devicetree, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: usb: snps,dwc3: missing endpoint/mux-state required in oneOf
- [Medium] dt-bindings: usb: snps,dwc3: port@2 missing from top-level ports schema
- [Medium] dt-bindings: usb: snps,dwc3: Linux-specific mux binding and port@2 split
--
commit f3f5f5bb87ea01fcf8b40062734fa5217caa615a
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>
dt-bindings: usb: snps,dwc3: describe two connectors sharing one HS bus
This commit extends snps,dwc3-common.yaml to describe two USB connectors
sharing a single High-Speed bus via a port-select-gpios multiplexer, using
port@0 and port@2 with per-endpoint mux-state and usb-role-switch
properties.
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
> index 6c0b8b6538246..0be2a6e83ee48 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml
> @@ -13,6 +13,40 @@ description:
> Defines the properties of the DWC3 core as being embedded in either an
> vendor-specific implementation or as a standalone component.
>
> +$defs:
> + hs-endpoint:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + unevaluatedProperties: false
> +
> + properties:
> + usb-role-switch:
> + type: boolean
> + description:
> + This connector endpoint has its own USB role switch. The switch is
> + registered against this endpoint and looked up with
> + usb_role_switch_at_endpoint().
> +
> + mux-state:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1]
> + description:
> + Logical level of the port-select GPIO which connects this
> + 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]
> + mux-state: [usb-role-switch]
> +
> + hs-port:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> +
> + patternProperties:
> + '^endpoint(@[0-9a-f]+)?$':
> + $ref: '#/$defs/hs-endpoint'
> +
> allOf:
> - $ref: usb-drd.yaml#
> - if:
> @@ -26,8 +60,87 @@ allOf:
> $ref: usb.yaml#
> else:
> $ref: usb-xhci.yaml#
> + - if:
> + required: [port-select-gpios]
> + then:
> + # 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:
> + # 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:
> + $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:
> + port@0:
> + $ref: '#/$defs/hs-port'
> + properties:
> + endpoint:
> + properties:
> + mux-state:
> + const: 0
> + port@2:
> + $ref: '#/$defs/hs-port'
> + properties:
> + endpoint:
> + properties:
> + mux-state:
> + const: 1
> + - required: [port@0, port@2]
> + properties:
> + port@0:
> + $ref: '#/$defs/hs-port'
> + properties:
> + endpoint:
> + properties:
> + mux-state:
> + const: 1
> + port@2:
> + $ref: '#/$defs/hs-port'
> + properties:
> + endpoint:
> + properties:
> + mux-state:
> + const: 0
[Severity: Medium]
Does the oneOf block under allOf allow incomplete or duplicate mux-state
configurations to pass schema validation while rejecting numbered
endpoint@0 nodes?
In $defs/hs-endpoint, usb-role-switch and mux-state are only constrained
with dependentRequired, so an endpoint omitting both properties satisfies
hs-endpoint. In addition, neither port@0 nor port@2 in the allOf then
branch specifies required: [endpoint], and the endpoint subschemas inside
the two oneOf branches only specify properties: { mux-state: { const: ... } }
without required: [mux-state].
In JSON Schema, the properties keyword evaluates to true when the target
property is absent. If port@0 has an endpoint with mux-state = <0> and
usb-role-switch, while port@2 omits endpoint (or its endpoint omits both
mux-state and usb-role-switch), the first branch of oneOf evaluates to true
and the second branch evaluates to false, allowing an incomplete device
tree that fails at runtime in dwc3_role_mux_init() to pass validation.
Furthermore, $defs/hs-port defines patternProperties matching
^endpoint(@[0-9a-f]+)?$, whereas the oneOf branches only check the literal
unnumbered property name endpoint:
1. If port@0 uses endpoint with mux-state = <0> and port@2 uses endpoint@0
with mux-state = <0>, port@2 has no unnumbered endpoint property and
vacuously matches const: 1 in the first oneOf branch, allowing duplicate
mux-state levels to pass.
2. If both port@0 and port@2 use endpoint@0 with opposite mux-state values
0 and 1, neither port has an unnumbered endpoint property, so both
branches of oneOf evaluate to true and oneOf fails.
3. If a port defines both endpoint and endpoint@1, hs-port accepts both and
oneOf ignores endpoint@1, even though the commit message states that a
third endpoint is rejected.
Should port@0 and port@2 require a single unnumbered endpoint (or constrain
endpoint count and naming consistently between hs-port and oneOf) and
require mux-state and usb-role-switch on each endpoint when
port-select-gpios is present?
> + else:
> + properties:
> + ports:
> + properties:
> + port@0:
> + patternProperties:
> + '^endpoint(@[0-9a-f]+)?$':
> + properties:
> + usb-role-switch: false
> + mux-state: false
>
> properties:
> + port-select-gpios:
> + maxItems: 1
> + description:
> + Selects which of the two connectors drives the controller's shared High
> + Speed data bus. Each connector is described by "port@0" or "port@2",
> + and the endpoint of that port carries the connector's own role-switch
> + input and the "mux-state" level selecting it.
[Severity: Medium]
Is defining custom port-select-gpios and mux-state properties on the DWC3
controller and splitting the shared High-Speed bus across port@0 and port@2
the right hardware representation for this board topology?
There are three concerns with this binding design in
Documentation/devicetree/bindings/usb/snps,dwc3-common.yaml:
1. The DWC3 controller itself has a single High-Speed D+/D- pair routed
through an external GPIO-controlled multiplexer to two connectors.
Instead of introducing custom port-select-gpios and endpoint mux-state
properties on the DWC3 binding, could this use the existing multiplexer
bindings in Documentation/devicetree/bindings/mux/gpio-mux.yaml and
Documentation/devicetree/bindings/mux/mux-consumer.yaml (such as
mux-controls or mux-states)?
2. Under ports in snps,dwc3-common.yaml, port@0 represents the controller's
High-Speed data bus and port@1 represents the SuperSpeed data bus. Since
the controller still has only one physical High-Speed bus, why is the
second connector modeled as a separate controller port (port@2) rather
than two endpoints (endpoint@0 and endpoint@1) under port@0 or via an
intermediate mux node?
3. Documentation/devicetree/bindings/writing-bindings.rst states that
bindings should describe hardware rather than Linux driver
implementation details. Here, $defs/hs-endpoint and the allOf conditional
for port-select-gpios reference Linux kernel functions
(usb_role_switch_at_endpoint(), dwc3_get_dr_mode(), dwc3_get_extcon(),
dwc3_drd_init(), and dwc3_role_mux_init()) and force dr_mode and
controller-level usb-role-switch into required to satisfy current driver
control flow, even though usb-drd.yaml already defines dr_mode as
optional with a default of otg.
> @@ -411,7 +524,7 @@ properties:
>
> properties:
> port@0:
> - $ref: /schemas/graph.yaml#/properties/port
> + $ref: '#/$defs/hs-port'
> description: High Speed (HS) data bus.
>
> port@1:
[Severity: Medium]
Will dtschema reject every device tree that sets port-select-gpios and
defines port@2 because port@2 is missing from the top-level
properties/ports block in snps,dwc3-common.yaml?
In snps,dwc3-common.yaml, the top-level properties/ports schema references
/schemas/graph.yaml#/properties/ports and only lists port@0 and port@1:
ports:
$ref: /schemas/graph.yaml#/properties/ports
...
properties:
port@0:
$ref: '#/$defs/hs-port'
description: High Speed (HS) data bus.
port@1:
$ref: /schemas/graph.yaml#/properties/port
description: Super Speed (SS) data bus.
(Note also that the comment in allOf and the commit message state that
port@1 is described by snps,dwc3.yaml, whereas port@1 is defined here in
snps,dwc3-common.yaml.)
When dtschema processes a node referencing graph.yaml#/properties/ports
that defines properties, it automatically injects unevaluatedProperties:
false into that properties/ports subschema. In JSON Schema,
unevaluatedProperties: false on the top-level properties/ports block does
not see port@2 evaluated inside the separate allOf then branch, so any
device tree containing port@2 is rejected as having an unevaluated property.
Conversely, in the allOf else branch when port-select-gpios is absent, only
port@0 is restricted and port@2 is not disallowed with port@2: false.
Should port@2 be declared under the top-level properties/ports block and
disallowed with port@2: false in the else branch when port-select-gpios is
absent?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104032706.210042-1-xiaqinghuan@mail.ustc.edu.cn?part=1
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-11-04 3:44 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04 3:27 [PATCH 0/3] usb: dwc3: two Type-C connectors sharing one HS data bus Qianhui Xu
2026-11-04 3:27 ` [PATCH 1/3] dt-bindings: usb: snps,dwc3: describe two connectors sharing one HS bus Qianhui Xu
2026-11-04 3:44 ` sashiko-bot
2026-11-04 3:27 ` [PATCH 2/3] usb: roles: look up role switches attached to a graph endpoint Qianhui Xu
2026-11-04 3:34 ` sashiko-bot
2026-11-04 3:27 ` [PATCH 3/3] usb: dwc3: support two connectors sharing one HS data bus Qianhui Xu
2026-11-04 3:44 ` sashiko-bot
-- strict thread matches above, loose matches on Subject: below --
2026-10-13 10:29 [PATCH 0/3] " Qianhui Xu
2026-10-13 10:29 ` [PATCH 3/3] " Qianhui Xu
2026-10-13 11:02 ` sashiko-bot
2026-10-14 20:19 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox