Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20220110172738.31686-1-Frank.Li@nxp.com> <Yd/7NQLgoEU17TzI@abelvesa>
In-Reply-To: <Yd/7NQLgoEU17TzI@abelvesa>
From:   Zhi Li <lznuaa@gmail.com>
Date:   Mon, 24 Jan 2022 09:49:10 -0600
Message-ID: <CAHrpEqQGDaUzhSpOmxj_J6a8X3ohMCW32jY+PApvmajPe7ckSQ@mail.gmail.com>
Subject: Re: [PATCH 1/1] usb: xhci-plat: fix crash when suspend if remote wake enable
To:     Abel Vesa <abelvesa@kernel.org>
Cc:     Frank Li <Frank.Li@nxp.com>, mathias.nyman@intel.com,
        gregkh@linuxfoundation.org, Peter Chen <peter.chen@kernel.org>,
        linux-usb@vger.kernel.org,
        kernel list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jan 13, 2022 at 4:13 AM Abel Vesa <abelvesa@kernel.org> wrote:
>
> 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@nxp.com>
>
> Tested on i.MX8QM.
>
> Testeb-by: Abel Vesa <abel.vesa@nxp.com>

Friendly ping

>
> > ---
> >  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
> >