From: Peter Chen <peter.chen@kernel.org>
To: Frank Li <frank.li@nxp.com>
Cc: "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"mathias.nyman@linux.intel.com" <mathias.nyman@linux.intel.com>,
Jun Li <jun.li@nxp.com>, Zhi Li <lznuaa@gmail.com>
Subject: Re: xhci crash at xhci_disable_hub_port_wake when system suspend.
Date: Mon, 10 Jan 2022 20:33:50 +0800 [thread overview]
Message-ID: <20220110123350.GA4302@Peter> (raw)
In-Reply-To: <PAXPR04MB91861A98BE9D240FFEC1D560884D9@PAXPR04MB9186.eurprd04.prod.outlook.com>
On 22-01-07 15:58:26, Frank Li wrote:
> Mathias Nyman
>
> Recently we found a crash at xhci_disable_hub_port_wake when system suspend if enable remote wake up.
>
> Basial flow is.
>
> 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 pm.
>
> Why find this issue now, that is because previous power domain driver will call run time resume before it. But the below commit remove it.
>
> c1df456d0f06eb9275c1cd4c66548fc5738ea428
> Author: Ulf Hansson ulf.hansson@linaro.org
> Date: Thu Mar 4 20:28:43 2021 +0100
>
> PM: domains: Don't runtime resume devices at genpd_prepare()
>
>
>
> According to HCD_FLAG_HW_ACCESSIBLE logic, xhci should not access hardware when second time call xhci_suspend without call xhci_resume.
>
> xhci_disable_hub_port_wake(xhci, &xhci->usb3_rhub, do_wakeup);
> xhci_disable_hub_port_wake(xhci, &xhci->usb2_rhub, do_wakeup);
>
> if (!HCD_HW_ACCESSIBLE(hcd))
> return 0;
>
> .....
> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
>
> I am not sure if it is safe to move xhci_disable_hub_port_wake after HCD_HW_ACCESSIBLE check, Or need add additional run_time_resume before it.
Frank, I prefer adding runtime resume at xhci-plat.c like below, let's see what
Mathias says.
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index c1edcc9b13ce..47a5a10381a7 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -440,6 +440,9 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev)
ret = xhci_priv_suspend_quirk(hcd);
if (ret)
return ret;
+
+ if (pm_runtime_suspended(dev))
+ pm_runtime_resume(dev);
/*
* xhci_suspend() needs `do_wakeup` to know whether host is allowed
* to do wakeup during suspend.
--
Thanks,
Peter Chen
next prev parent reply other threads:[~2022-01-10 12:34 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-07 15:58 xhci crash at xhci_disable_hub_port_wake when system suspend Frank Li
2022-01-10 12:33 ` Peter Chen [this message]
2022-01-10 21:07 ` Mathias Nyman
2022-01-10 21:37 ` Zhi Li
2022-01-12 12:48 ` Peter Chen
2022-01-12 13:36 ` Jun Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220110123350.GA4302@Peter \
--to=peter.chen@kernel.org \
--cc=frank.li@nxp.com \
--cc=jun.li@nxp.com \
--cc=linux-usb@vger.kernel.org \
--cc=lznuaa@gmail.com \
--cc=mathias.nyman@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).