[PATCH 0/1] usb: dwc3: qcom: Keep wakeup option default off

[PATCH 0/1] usb: dwc3: qcom: Keep wakeup option default off

[Peter Chen]

Current policy
--------------
When the controller node has the "wakeup-source" property, dwc3-qcom
calls device_init_wakeup(dev, true). USB is then a system wakeup
source by default. Users who do NOT want the system woken up by USB
have to disable it:

  echo disabled > /sys/devices/.../<dwc3 device>/power/wakeup

Proposed policy
---------------
"wakeup-source" only describes that the controller is capable of
waking up the system (see
Documentation/devicetree/bindings/power/wakeup-source.txt), so with
this patch the driver only marks the device as wakeup capable, and
power/wakeup starts out "disabled". Users who NEED USB as a wakeup
source have to enable it:

  echo enabled > /sys/devices/.../<dwc3 device>/power/wakeup

The motivation is:

- Users do not expect USB activity (connect/disconnect or USB line status
  changes) to wake up the system unless they asked for it.

- Power consumption. With wakeup enabled, host mode system suspend
  keeps PHYs powered (dwc3_core_exit() is skipped) and necessary logic
  for detecting wakeup must be on, so that the wakeup interrupts can
  fire. Today this extra power is spent by default, even on systems
  where USB is never used as a wakeup source and the controller could
  be fully powered off during suspend.

Comparison
----------
                       Current (default on)       Proposed (default off)
                       --------------------       ----------------------
Initial power/wakeup   enabled                    disabled
User action            write "disabled" if USB    write "enabled" if USB
                       wakeup is NOT wanted       wakeup IS wanted
Unexpected wakeups     possible (connect,         none unless the user
                       disconnect, noise)         opted in
Suspend power          higher, even if USB        lower, USB fully powered
                       wakeup is never used       off
Attached devices on    stay suspended, resume     reset-resumed or
resume                 quickly                    re-enumerated

Possible regression
-------------------
This is a user-visible change for existing platforms. Many Qualcomm
arm64 DTs set "wakeup-source" on the USB controller. With this patch:

- USB wakeup stops working until userspace (or a udev rule) writes
  "enabled" to the controller's power/wakeup.

- In host mode, because device_may_wakeup() is now false by default,
  system suspend takes the full power down path in
  dwc3_suspend_common() (dwc3_core_exit()). Platforms where this
  power down path has not been tested may fail to resume.

Peter Chen (1):
  usb: dwc3: qcom: Keep wakeup option default off

 drivers/usb/dwc3/dwc3-qcom.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/1] usb: dwc3: qcom: Keep wakeup option default off

[Peter Chen]

The "wakeup-source" property, as described in
Documentation/devicetree/bindings/power/wakeup-source.txt, only says
that a device is capable of waking up the system. Whether wakeup is
actually enabled is a policy decision, and drivers use
device_may_wakeup() to decide whether to arm the hardware wakeup logic.

Currently dwc3-qcom calls device_init_wakeup(dev, true) when the
property is present, so USB wakeup is enabled by default. Users who do
not want USB activity (e.g. connect/disconnect) to wake up the system
have to disable it through power/wakeup, otherwise unexpected wakeups
may happen.

Having wakeup enabled by default also costs power. In host mode, system
suspend skips dwc3_core_exit() and keeps the PHYs and the wakeup
detection logic powered so that the wakeup interrupts can fire, even on
systems where USB is never used as a wakeup source and the controller
could be fully powered off.

Keep the wakeup option default off and only mark the device as wakeup
capable, so that the user decides which USB controller is used as a
system wakeup source. Users who need it can enable it with:

  echo enabled > /sys/devices/.../<dwc3 device>/power/wakeup

While at it, drop the local wakeup_source variable and use
device_property_read_bool() like the other property reads in probe.

Signed-off-by: Peter Chen <peter.chen@oss.qualcomm.com>
---
 drivers/usb/dwc3/dwc3-qcom.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index ac68b4218b56..fb535e9a4a64 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -616,7 +616,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	struct resource		*r;
 	int			ret;
 	bool			ignore_pipe_clk;
-	bool			wakeup_source;
 
 	qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
 	if (!qcom)
@@ -715,8 +714,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ret)
 		goto remove_core;
 
-	wakeup_source = of_property_read_bool(dev->of_node, "wakeup-source");
-	device_init_wakeup(&pdev->dev, wakeup_source);
+	if (device_property_read_bool(dev, "wakeup-source"))
+		device_set_wakeup_capable(dev, true);
 
 	qcom->is_suspended = false;
 
-- 
2.43.0

Re: [PATCH 1/1] usb: dwc3: qcom: Keep wakeup option default off

[sashiko-bot]

> The "wakeup-source" property, as described in
> Documentation/devicetree/bindings/power/wakeup-source.txt, only says
> that a device is capable of waking up the system. Whether wakeup is
> actually enabled is a policy decision, and drivers use
> device_may_wakeup() to decide whether to arm the hardware wakeup logic.
> 
> Currently dwc3-qcom calls device_init_wakeup(dev, true) when the
> property is present, so USB wakeup is enabled by default. Users who do
> not want USB activity (e.g. connect/disconnect) to wake up the system
> have to disable it through power/wakeup, otherwise unexpected wakeups
> may happen.
> 
> Having wakeup enabled by default also costs power. In host mode, system
> suspend skips dwc3_core_exit() and keeps the PHYs and the wakeup
> detection logic powered so that the wakeup interrupts can fire, even on
> systems where USB is never used as a wakeup source and the controller
> could be fully powered off.
> 
> Keep the wakeup option default off and only mark the device as wakeup
> capable, so that the user decides which USB controller is used as a
> system wakeup source. Users who need it can enable it with:
> 
>   echo enabled > /sys/devices/.../<dwc3 device>/power/wakeup
> 
> While at it, drop the local wakeup_source variable and use
> device_property_read_bool() like the other property reads in probe.
> 
> Signed-off-by: Peter Chen <peter.chen@oss.qualcomm.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110092157.2989350-1-peter.chen@oss.qualcomm.com?part=1

Re: [PATCH 0/1] usb: dwc3: qcom: Keep wakeup option default off

[Thinh Nguyen]

Hi,

On Tue, Nov 10, 2026, Peter Chen wrote:
> Current policy
> --------------
> When the controller node has the "wakeup-source" property, dwc3-qcom
> calls device_init_wakeup(dev, true). USB is then a system wakeup
> source by default. Users who do NOT want the system woken up by USB
> have to disable it:
> 
>   echo disabled > /sys/devices/.../<dwc3 device>/power/wakeup
> 
> Proposed policy
> ---------------
> "wakeup-source" only describes that the controller is capable of
> waking up the system (see
> Documentation/devicetree/bindings/power/wakeup-source.txt), so with
> this patch the driver only marks the device as wakeup capable, and
> power/wakeup starts out "disabled". Users who NEED USB as a wakeup
> source have to enable it:
> 
>   echo enabled > /sys/devices/.../<dwc3 device>/power/wakeup

That won't work in host mode. dwc->sys_wakeup is read only once in
dwc3_core_probe(), so enabling wakeup later from sysfs won't reach the
xhci-plat device, and xhci_suspend() clears the port wake bits. Moving
the wakeup setup before dwc3_core_probe() won't work either, since
wakeup is off at that point. The setting needs to be passed to xhci at
suspend time (perhaps in dwc3_pm_prepare()?).

> 
> The motivation is:
> 
> - Users do not expect USB activity (connect/disconnect or USB line status
>   changes) to wake up the system unless they asked for it.
> 
> - Power consumption. With wakeup enabled, host mode system suspend
>   keeps PHYs powered (dwc3_core_exit() is skipped) and necessary logic
>   for detecting wakeup must be on, so that the wakeup interrupts can
>   fire. Today this extra power is spent by default, even on systems
>   where USB is never used as a wakeup source and the controller could
>   be fully powered off during suspend.

Sounds good, but there are some concerns.

> 
> Comparison
> ----------
>                        Current (default on)       Proposed (default off)
>                        --------------------       ----------------------
> Initial power/wakeup   enabled                    disabled
> User action            write "disabled" if USB    write "enabled" if USB
>                        wakeup is NOT wanted       wakeup IS wanted
> Unexpected wakeups     possible (connect,         none unless the user
>                        disconnect, noise)         opted in
> Suspend power          higher, even if USB        lower, USB fully powered
>                        wakeup is never used       off
> Attached devices on    stay suspended, resume     reset-resumed or
> resume                 quickly                    re-enumerated
> 
> Possible regression
> -------------------

This rings my alarm bells for me any time we mention a user behavior
change or possible regression. Perhaps you can add this to the commit
message.

> This is a user-visible change for existing platforms. Many Qualcomm
> arm64 DTs set "wakeup-source" on the USB controller. With this patch:
> 
> - USB wakeup stops working until userspace (or a udev rule) writes
>   "enabled" to the controller's power/wakeup.
> 
> - In host mode, because device_may_wakeup() is now false by default,

This will be the default in host mode for every qcom platform with
"wakeup-source". Which platforms have you tested?

>   system suspend takes the full power down path in
>   dwc3_suspend_common() (dwc3_core_exit()). Platforms where this
>   power down path has not been tested may fail to resume.
> 

BR,
Thinh

Re: [PATCH 0/1] usb: dwc3: qcom: Keep wakeup option default off

[Peter Chen]

On 26-11-12 01:14:44, Thinh Nguyen wrote:
> > Current policy
> > --------------
> > When the controller node has the "wakeup-source" property, dwc3-qcom
> > calls device_init_wakeup(dev, true). USB is then a system wakeup
> > source by default. Users who do NOT want the system woken up by USB
> > have to disable it:
> > 
> >   echo disabled > /sys/devices/.../<dwc3 device>/power/wakeup
> > 
> > Proposed policy
> > ---------------
> > "wakeup-source" only describes that the controller is capable of
> > waking up the system (see
> > Documentation/devicetree/bindings/power/wakeup-source.txt), so with
> > this patch the driver only marks the device as wakeup capable, and
> > power/wakeup starts out "disabled". Users who NEED USB as a wakeup
> > source have to enable it:
> > 
> >   echo enabled > /sys/devices/.../<dwc3 device>/power/wakeup
> 

Thinh, thanks for reviewing it.

> That won't work in host mode.

At some platforms, USB2 wakeup logic works since there are
USB2 wakeup signal detection logic beyond controller and at SoC level.
For wakeup signal detected by controller, you are right, it needs
to enable controller level wakeup option, eg, WKCN/WKDN at PORTSC.

> dwc->sys_wakeup is read only once in
> dwc3_core_probe(), so enabling wakeup later from sysfs won't reach the
> xhci-plat device, and xhci_suspend() clears the port wake bits. Moving
> the wakeup setup before dwc3_core_probe() won't work either, since
> wakeup is off at that point. The setting needs to be passed to xhci at
> suspend time (perhaps in dwc3_pm_prepare()?).
> 

When talking about whole system wakeup chain for USB:
dwc3 -> xhci-plat -> roothub -> downstream device
it usually depends on user program (per user option), but not bind to
kernel driver, considering some situations that USB device are disconnected
and re-connected, its wakeup option may need to re-set, and one typical
solution is enable wakeup options before system suspend.

> 
> > 
> > Comparison
> > ----------
> >                        Current (default on)       Proposed (default off)
> >                        --------------------       ----------------------
> > Initial power/wakeup   enabled                    disabled
> > User action            write "disabled" if USB    write "enabled" if USB
> >                        wakeup is NOT wanted       wakeup IS wanted
> > Unexpected wakeups     possible (connect,         none unless the user
> >                        disconnect, noise)         opted in
> > Suspend power          higher, even if USB        lower, USB fully powered
> >                        wakeup is never used       off
> > Attached devices on    stay suspended, resume     reset-resumed or
> > resume                 quickly                    re-enumerated
> > 
> > Possible regression
> > -------------------
> 
> This rings my alarm bells for me any time we mention a user behavior
> change or possible regression. Perhaps you can add this to the commit
> message.
> 

Sure.

> > This is a user-visible change for existing platforms. Many Qualcomm
> > arm64 DTs set "wakeup-source" on the USB controller. With this patch:
> > 
> > - USB wakeup stops working until userspace (or a udev rule) writes
> >   "enabled" to the controller's power/wakeup.
> > 
> > - In host mode, because device_may_wakeup() is now false by default,
> 
> This will be the default in host mode for every qcom platform with
> "wakeup-source". Which platforms have you tested?
> 

Yes, it affects all Qcom platforms, that's why I sent RFC patch for it
last week, most probably it have "regression" since it may never
be tested under the situation the PHY is powered down at suspend and
power on at resume, but that kinds of "regression" is the bug we need
to fix, right?

I have tested at Glymur CRD, external ports work (USB3/USB2), but
the port with USB2 Footprint IC (USB2) hard-wired does not work
sometimes, maybe the initial sequence between power-on and suspend/resume
are different, it needs to debug more.

> >   system suspend takes the full power down path in
> >   dwc3_suspend_common() (dwc3_core_exit()). Platforms where this
> >   power down path has not been tested may fail to resume.
> > 
> 

-- 

Thanks,
Peter Chen