usb: dwc3: re-enable runtime PM after failed resume
Subsystems (3)
PATCH 1/1 usb: dwc3: re-enable runtime PM after failed resume
When dwc3_resume_common() returns an error, runtime pm is left in disabled state in dwc3_resume(). The next dwc3_suspend_common() should be skipped as the device is already in suspended state but it's not because power.disable_depth is non-zero. Ensures runtime PM is always re-enabled even after failed resume attempts.
--- drivers/usb/dwc3/core.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/usb/dwc3/core.c +2 -3
@@ -2537,7 +2537,7 @@ static int dwc3_suspend(struct device *dev)static int dwc3_resume (struct device *dev) {struct dwc3 *dwc =dev_get_drvdata (dev); -int ret; +int ret =0 ;pinctrl_pm_select_default_state (dev);
@@ -2547,12 +2547,11 @@ static int dwc3_resume(struct device *dev) ret =dwc3_resume_common (dwc,PMSG_RESUME );if (ret) {pm_runtime_set_suspended (dev); -return ret; }pm_runtime_enable (dev); -return 0 ; +return ret; }static void dwc3_complete (struct device *dev)
base-commit: ad618736883b8970f66af799e34007475fe33a68
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
Thinh Nguyen
maintainerQuoted 2 lines · Roy Luo
When
dwc3_resume_common()returns an error, runtime pm is left in disabled state indwc3_resume(). The nextdwc3_suspend_common()
What issue did you see when dwc3_suspend_common is not skipped?
BR,
Thinh
Quoted 44 lines
should be skipped as the device is already in suspended state but it's not because power.disable_depth is non-zero. Ensures runtime PM is always re-enabled even after failed resume attempts.
Fixes 68c26fe58182 ("usb: dwc3: set pm runtime active before resume common")Signed-off-by Roy Luo <[email protected]>--- drivers/usb/dwc3/core.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)drivers/usb/dwc3/core.c +2
-3@@ -2537,7 +2537,7 @@ static int dwc3_suspend(struct device *dev)static int dwc3_resume (struct device *dev) {struct dwc3 *dwc =dev_get_drvdata (dev); -int ret; +int ret =0 ;pinctrl_pm_select_default_state (dev);@@ -2547,12 +2547,11 @@ static int dwc3_resume(struct device *dev) ret =dwc3_resume_common (dwc,PMSG_RESUME );if (ret) {pm_runtime_set_suspended (dev); -return ret; }pm_runtime_enable (dev); -return 0 ; +return ret; }static void dwc3_complete (struct device *dev)base-commit:
ad618736883b8970f66af799e34007475fe33a68Signature · 1 line
2.46.0.469.g59c65b2a67-goog
Quoted 6 lines · Thinh Nguyen
Quoted 2 lines · Roy Luo
When
dwc3_resume_common()returns an error, runtime pm is left in disabled state indwc3_resume(). The nextdwc3_suspend_common()What issue did you see when
dwc3_suspend_commonis not skipped?
Apologies for the delayed response.
To answer your question, if dwc3_suspend_common() isn't skipped, it can lead to issues because dwc->dev is already in a suspended state. This could mean its parent devices (like the power domain or glue driver) are also suspended and may have released resources that dwc requires.
Consequently, calling dwc3_suspend_common() in this situation could result in attempts to access unclocked or unpowered registers.
Regards,
Roy Luo
Thinh Nguyen
maintainerQuoted 10 lines · Roy Luo
Quoted 6 lines · Thinh Nguyen
Quoted 2 lines · Roy Luo
When
dwc3_resume_common()returns an error, runtime pm is left in disabled state indwc3_resume(). The nextdwc3_suspend_common()What issue did you see when
dwc3_suspend_commonis not skipped?Apologies for the delayed response.
Quoted 8 lines
To answer your question, if
dwc3_suspend_common()isn't skipped, it can lead to issues becausedwc->devis already in a suspended state. This could mean its parent devices (like the power domain or glue driver) are also suspended and may have released resources that dwc requires.
Consequently, callingdwc3_suspend_common()in this situation could result in attempts to access unclocked or unpowered registers.
Can you include this info in the commit message?
And while at it, can you also update minor style change to remove the brackets for single line if statement to this:
ret = dwc3_resume_common(dwc, PMSG_RESUME); if (ret)
pm_runtime_set_suspended(dev);
Thanks,
Thinh
Quoted 10 lines · Thinh Nguyen
Can you include this info in the commit message?
And while at it, can you also update minor style change to remove the brackets for single line if statement to this:
ret = dwc3_resume_common(dwc, PMSG_RESUME); if (ret) pm_runtime_set_suspended(dev);
Sure, sent out v2 for review.
Regards,
Roy Luo