Re: [PATCH 1/1] usb: xhci-plat: fix crash when suspend if remote wake enable

From: Abel Vesa
Date: Thu Jan 13 2022 - 05:13:19 EST


On 22-01-10 11:27:38, Frank Li wrote:
> Crashed at i.mx8qm platform when suspend if enable remote wakeup
>
> Internal error: synchronous external abort: 96000210 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 2 PID: 244 Comm: kworker/u12:6 Not tainted 5.15.5-dirty #12
> Hardware name: Freescale i.MX8QM MEK (DT)
> Workqueue: events_unbound async_run_entry_fn
> pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : xhci_disable_hub_port_wake.isra.62+0x60/0xf8
> lr : xhci_disable_hub_port_wake.isra.62+0x34/0xf8
> sp : ffff80001394bbf0
> x29: ffff80001394bbf0 x28: 0000000000000000 x27: ffff00081193b578
> x26: ffff00081193b570 x25: 0000000000000000 x24: 0000000000000000
> x23: ffff00081193a29c x22: 0000000000020001 x21: 0000000000000001
> x20: 0000000000000000 x19: ffff800014e90490 x18: 0000000000000000
> x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> x14: 0000000000000000 x13: 0000000000000002 x12: 0000000000000000
> x11: 0000000000000000 x10: 0000000000000960 x9 : ffff80001394baa0
> x8 : ffff0008145d1780 x7 : ffff0008f95b8e80 x6 : 000000001853b453
> x5 : 0000000000000496 x4 : 0000000000000000 x3 : ffff00081193a29c
> x2 : 0000000000000001 x1 : 0000000000000000 x0 : ffff000814591620
> Call trace:
> xhci_disable_hub_port_wake.isra.62+0x60/0xf8
> xhci_suspend+0x58/0x510
> xhci_plat_suspend+0x50/0x78
> platform_pm_suspend+0x2c/0x78
> dpm_run_callback.isra.25+0x50/0xe8
> __device_suspend+0x108/0x3c0
>
> The basic flow:
> 1. run time suspend call xhci_suspend, xhci parent devices gate the clock.
> 2. echo mem >/sys/power/state, system _device_suspend call xhci_suspend
> 3. xhci_suspend call xhci_disable_hub_port_wake, which access register,
> but clock already gated by run time suspend.
>
> This problem was hidden by power domain driver, which call run time resume before it.
>
> But the below commit remove it and make this issue happen.
> commit c1df456d0f06e ("PM: domains: Don't runtime resume devices at genpd_prepare()")
>
> This patch call run time resume before suspend to make sure clock is on
> before access register.
>
> Signed-off-by: Frank Li <Frank.Li@xxxxxxx>

Tested on i.MX8QM.

Testeb-by: Abel Vesa <abel.vesa@xxxxxxx>

> ---
> drivers/usb/host/xhci-plat.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index c6b791a83ad18..7d2f665271310 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -442,6 +442,9 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev)
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> int ret;
>
> + if (pm_runtime_suspended(dev))
> + pm_runtime_resume(dev);
> +
> ret = xhci_priv_suspend_quirk(hcd);
> if (ret)
> return ret;
> --
> 2.24.0.rc1
>