[PATCH v4 4/6] usb: dwc2: hcd: fix possible successive vbus supply enables
From: Fabrice Gasnier
Date: Fri Nov 13 2026 - 15:40:38 EST
Relying on HPRT0_PWR value to call dwc2_vbus_supply_init() shows some
corner case resulting in vbus enable count to increase.
As consequence, dwc2_vbus_supply_exit call done when clearing
PWR bit won't disable vbus (for example upon suspend request).
Address two case that has been seen:
- In _dwc2_hcd_start(), external VBUS supply may have already been enabled,
before entering dwc2_hcd_reinit (e.g. the controller is in host mode,
without any device plugged).
- Upon resume from low power in case registers have been lost. In clock
gating mode PM suspend flow doesn't disable vbus while in host mode:
- dwc2_port_suspend() enters clock gating (it sets lx_state to DWC2_L2)
- _dwc2_hcd_suspend() nothing more to do as already in L2.
- _dwc2_hcd_resume() - checks HPRT0_CONNSTS. If it's not set (register
loss) it sets HPRT0_PWR and call dwc2_vbus_supply_init().
Fix this by adding a boolean flag to balance vbus enable/disable calls
instead of relying on HPRT0_PWR value. Clean the existing checks on pwr
bit. A mutex is added as dwc2_vbus_supply_init/_exit calls can run
concurrently from hub_control and dwc2_conn_id_status_change.
Fixes: cd7cd0e6cedf ("usb: dwc2: fix unbalanced use of external vbus-supply")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx>
---
Changes in v4:
- drop previous vbus_was_off boolean. Introduce vbus_supply_enabled flag
instead and vbus_supply_lock.
- remove checks on pwr when possible as now replaced by this flag to keep
balanced calls.
Changes in v2:
- Add a vbus_was_off boolean to check explicitly an off-on transition
on port power, to enable vbus supply, instead of relying an XOR check
that would be symmetric, e.g. wrongly enable it in case it gets switched
off (1 to 0) as reported by Sashiko.
---
drivers/usb/dwc2/core.h | 6 ++++++
drivers/usb/dwc2/hcd.c | 45 ++++++++++++++++++++++++---------------------
drivers/usb/dwc2/platform.c | 1 +
3 files changed, 31 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 32662cb3d3eb..c97c958b19a5 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -10,6 +10,7 @@
#include <linux/acpi.h>
#include <linux/device-id/pci.h>
+#include <linux/mutex.h>
#include <linux/phy/phy.h>
#include <linux/regulator/consumer.h>
#include <linux/usb/gadget.h>
@@ -886,6 +887,9 @@ struct dwc2_hregs_backup {
* removed once all SoCs support usb transceiver.
* @supplies: Definition of USB power supplies
* @vbus_supply: Regulator supplying vbus.
+ * @vbus_supply_lock: Protects vbus_supply state and regulator operations.
+ * @vbus_supply_enabled: True if this driver holds an enable reference to
+ * vbus_supply.
* @usb33d: Optional 3.3v regulator used on some stm32 devices to
* supply ID and VBUS detection hardware.
* @lock: Spinlock that protects all the driver data structures
@@ -1079,6 +1083,8 @@ struct dwc2_hsotg {
struct dwc2_hsotg_plat *plat;
struct regulator_bulk_data supplies[DWC2_NUM_SUPPLIES];
struct regulator *vbus_supply;
+ struct mutex vbus_supply_lock;
+ bool vbus_supply_enabled;
struct regulator *usb33d;
spinlock_t lock;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 7e4202c145f2..053d1234884b 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -133,18 +133,32 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
{
- if (hsotg->vbus_supply)
- return regulator_enable(hsotg->vbus_supply);
+ int ret = 0;
- return 0;
+ scoped_guard(mutex, &hsotg->vbus_supply_lock) {
+ if (hsotg->vbus_supply && !hsotg->vbus_supply_enabled) {
+ ret = regulator_enable(hsotg->vbus_supply);
+ if (!ret)
+ hsotg->vbus_supply_enabled = true;
+ }
+ }
+
+ return ret;
}
static int dwc2_vbus_supply_exit(struct dwc2_hsotg *hsotg)
{
- if (hsotg->vbus_supply)
- return regulator_disable(hsotg->vbus_supply);
+ int ret = 0;
- return 0;
+ scoped_guard(mutex, &hsotg->vbus_supply_lock) {
+ if (hsotg->vbus_supply && hsotg->vbus_supply_enabled) {
+ ret = regulator_disable(hsotg->vbus_supply);
+ if (!ret)
+ hsotg->vbus_supply_enabled = false;
+ }
+ }
+
+ return ret;
}
/**
@@ -3435,7 +3449,6 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq,
u32 port_status;
u32 speed;
u32 pcgctl;
- u32 pwr;
switch (typereq) {
case ClearHubFeature:
@@ -3480,11 +3493,9 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq,
dev_dbg(hsotg->dev,
"ClearPortFeature USB_PORT_FEAT_POWER\n");
hprt0 = dwc2_read_hprt0(hsotg);
- pwr = hprt0 & HPRT0_PWR;
hprt0 &= ~HPRT0_PWR;
dwc2_writel(hsotg, hprt0, HPRT0);
- if (pwr)
- dwc2_vbus_supply_exit(hsotg);
+ dwc2_vbus_supply_exit(hsotg);
break;
case USB_PORT_FEAT_INDICATOR:
@@ -3688,11 +3699,9 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq,
dev_dbg(hsotg->dev,
"SetPortFeature - USB_PORT_FEAT_POWER\n");
hprt0 = dwc2_read_hprt0(hsotg);
- pwr = hprt0 & HPRT0_PWR;
hprt0 |= HPRT0_PWR;
dwc2_writel(hsotg, hprt0, HPRT0);
- if (!pwr)
- dwc2_vbus_supply_init(hsotg);
+ dwc2_vbus_supply_init(hsotg);
break;
case USB_PORT_FEAT_RESET:
@@ -3727,7 +3736,6 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq,
dwc2_writel(hsotg, 0, PCGCTL);
hprt0 = dwc2_read_hprt0(hsotg);
- pwr = hprt0 & HPRT0_PWR;
/* Clear suspend bit if resetting from suspend state */
hprt0 &= ~HPRT0_SUSP;
@@ -3741,8 +3749,7 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq,
dev_dbg(hsotg->dev,
"In host mode, hprt0=%08x\n", hprt0);
dwc2_writel(hsotg, hprt0, HPRT0);
- if (!pwr)
- dwc2_vbus_supply_init(hsotg);
+ dwc2_vbus_supply_init(hsotg);
}
/* Clear reset bit in 10ms (FS/LS) or 50ms (HS) */
@@ -4344,7 +4351,6 @@ static void _dwc2_hcd_stop(struct usb_hcd *hcd)
{
struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
unsigned long flags;
- u32 hprt0;
/* Turn off all host-specific interrupts */
dwc2_disable_host_interrupts(hsotg);
@@ -4353,7 +4359,6 @@ static void _dwc2_hcd_stop(struct usb_hcd *hcd)
synchronize_irq(hcd->irq);
spin_lock_irqsave(&hsotg->lock, flags);
- hprt0 = dwc2_read_hprt0(hsotg);
/* Ensure hcd is disconnected */
dwc2_hcd_disconnect(hsotg, true);
dwc2_hcd_stop(hsotg);
@@ -4362,9 +4367,7 @@ static void _dwc2_hcd_stop(struct usb_hcd *hcd)
clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
spin_unlock_irqrestore(&hsotg->lock, flags);
- /* keep balanced supply init/exit by checking HPRT0_PWR */
- if (hprt0 & HPRT0_PWR)
- dwc2_vbus_supply_exit(hsotg);
+ dwc2_vbus_supply_exit(hsotg);
usleep_range(1000, 3000);
}
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 72214593f431..968b4ba6d814 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -471,6 +471,7 @@ static int dwc2_driver_probe(struct platform_device *dev)
return retval;
spin_lock_init(&hsotg->lock);
+ mutex_init(&hsotg->vbus_supply_lock);
hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
if (IS_ERR(hsotg->vbus_supply)) {
--
2.43.0