Messages in this thread | | | Date | Fri, 15 Sep 2023 13:17:43 -0700 | Subject | Re: [PATCH v2] usb: dwc3: core: Avoid resume dwc3 if already suspended in pm resume | From | Wesley Cheng <> |
| |
Hi William,
On 9/10/2023 8:31 PM, William Wu wrote: > If we enable PM runtime auto suspend for dwc3 on rockchip > platforms (e.g. RK3562), it allows the dwc3 controller to > enter runtime suspend if usb cable detached and power off > the power domain of the controller. When system resume, if > the dwc3 already in runtime suspended, it Shouldn't access > the dwc3 registers in dwc3_resume() because its power domain > maybe power off. > > Test on RK3562 tablet, this patch can help to avoid kernel > panic when accessing the dwc3 registers in dwc3_resume() if > the dwc3 is in runtime suspended and it's power domain is > power off. > > Kernel panic - not syncing: Asynchronous SError Interrupt > Hardware name: Rockchip RK3562 RK817 TABLET LP4 Board (DT) > Call trace: > dump_backtrace.cfi_jt+0x0/0x8 > dump_stack_lvl+0xc0/0x13c > panic+0x174/0x468 > arm64_serror_panic+0x1b0/0x200 > do_serror+0x184/0x1e4 > el1_error+0x94/0x118 > el1_abort+0x40/0x68 > el1_sync_handler+0x58/0x88 > el1_sync+0x8c/0x140 > dwc3_readl+0x30/0x1a0 > dwc3_phy_setup+0x38/0x510 > dwc3_core_init+0x68/0xcd4 > dwc3_core_init_for_resume+0x10c/0x25c > dwc3_resume_common+0x44/0x3d0 > dwc3_resume+0x5c/0xb8 > dpm_run_callback+0x70/0x488 > device_resume+0x250/0x2f8 > dpm_resume+0x258/0x9dc > > Signed-off-by: William Wu <william.wu@rock-chips.com> > --- > Changes in v2: > - Remove Change-Id. > > drivers/usb/dwc3/core.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 9c6bf054f15d..8274a44f2d6a 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -2185,9 +2185,11 @@ static int dwc3_resume(struct device *dev) > > pinctrl_pm_select_default_state(dev); > > - ret = dwc3_resume_common(dwc, PMSG_RESUME); > - if (ret) > - return ret; > + if (!pm_runtime_suspended(dwc->dev)) { > + ret = dwc3_resume_common(dwc, PMSG_RESUME); > + if (ret) > + return ret; > + } > > pm_runtime_disable(dev); > pm_runtime_set_active(dev);
In case DWC3 is runtime suspended, then we will avoid the dwc3_resume_common() call, but the current flow would also set the RPM state to RPM_ACTIVE. (from pm_runtime_set_active()) In this case, what happens if there is a pm_runtime_get/resume on the DWC3 device?
I think it would avoid calling rpm_resume() since the RPM state is already active, so we wouldn't be calling dwc3_runtime_resume(). Do you want to also extend the RPM suspended check to cover if we need to force PM runtime state back to active?
Thanks Wesley Cheng
| |