* [PATCH] usb: dwc3: fix fault at system suspend if device was already runtime suspended
@ 2024-11-04 14:00 Roger Quadros
2024-11-04 18:04 ` William McVicker
2024-11-04 23:08 ` Thinh Nguyen
0 siblings, 2 replies; 3+ messages in thread
From: Roger Quadros @ 2024-11-04 14:00 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Dhruva Gole, sashal,
William McVicker, Chris Morgan
Cc: Vishal Mahaveer, msp, srk, linux-usb, linux-kernel, stable,
Roger Quadros
If the device was already runtime suspended then during system suspend
we cannot access the device registers else it will crash.
Also we cannot access any registers after dwc3_core_exit() on some
platforms so move the dwc3_enable_susphy() call to the top.
Cc: stable@vger.kernel.org # v5.15+
Reported-by: William McVicker <willmcvicker@google.com>
Closes: https://lore.kernel.org/all/ZyVfcUuPq56R2m1Y@google.com
Fixes: 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms")
Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
drivers/usb/dwc3/core.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 427e5660f87c..98114c2827c0 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2342,10 +2342,18 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
u32 reg;
int i;
- dwc->susphy_state = (dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)) &
- DWC3_GUSB2PHYCFG_SUSPHY) ||
- (dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)) &
- DWC3_GUSB3PIPECTL_SUSPHY);
+ if (!pm_runtime_suspended(dwc->dev) && !PMSG_IS_AUTO(msg)) {
+ dwc->susphy_state = (dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)) &
+ DWC3_GUSB2PHYCFG_SUSPHY) ||
+ (dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)) &
+ DWC3_GUSB3PIPECTL_SUSPHY);
+ /*
+ * TI AM62 platform requires SUSPHY to be
+ * enabled for system suspend to work.
+ */
+ if (!dwc->susphy_state)
+ dwc3_enable_susphy(dwc, true);
+ }
switch (dwc->current_dr_role) {
case DWC3_GCTL_PRTCAP_DEVICE:
@@ -2398,15 +2406,6 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
break;
}
- if (!PMSG_IS_AUTO(msg)) {
- /*
- * TI AM62 platform requires SUSPHY to be
- * enabled for system suspend to work.
- */
- if (!dwc->susphy_state)
- dwc3_enable_susphy(dwc, true);
- }
-
return 0;
}
---
base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
change-id: 20241102-am62-lpm-usb-fix-347dd86135c1
Best regards,
--
Roger Quadros <rogerq@kernel.org>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: dwc3: fix fault at system suspend if device was already runtime suspended
2024-11-04 14:00 [PATCH] usb: dwc3: fix fault at system suspend if device was already runtime suspended Roger Quadros
@ 2024-11-04 18:04 ` William McVicker
2024-11-04 23:08 ` Thinh Nguyen
1 sibling, 0 replies; 3+ messages in thread
From: William McVicker @ 2024-11-04 18:04 UTC (permalink / raw)
To: Roger Quadros
Cc: Thinh Nguyen, Greg Kroah-Hartman, Dhruva Gole, sashal,
Chris Morgan, Vishal Mahaveer, msp, srk, linux-usb, linux-kernel,
stable
Hi Roger,
On 11/04/2024, Roger Quadros wrote:
> If the device was already runtime suspended then during system suspend
> we cannot access the device registers else it will crash.
>
> Also we cannot access any registers after dwc3_core_exit() on some
> platforms so move the dwc3_enable_susphy() call to the top.
>
> Cc: stable@vger.kernel.org # v5.15+
> Reported-by: William McVicker <willmcvicker@google.com>
> Closes: https://lore.kernel.org/all/ZyVfcUuPq56R2m1Y@google.com
> Fixes: 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms")
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
I verified the patch works on my Pixel 6 device with runtime PM enabled. Thanks
for the fix! Feel free to add
Tested-by: Will McVicker <willmcvicker@google.com>
Thanks,
Will
> ---
> drivers/usb/dwc3/core.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 427e5660f87c..98114c2827c0 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -2342,10 +2342,18 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> u32 reg;
> int i;
>
> - dwc->susphy_state = (dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)) &
> - DWC3_GUSB2PHYCFG_SUSPHY) ||
> - (dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)) &
> - DWC3_GUSB3PIPECTL_SUSPHY);
> + if (!pm_runtime_suspended(dwc->dev) && !PMSG_IS_AUTO(msg)) {
> + dwc->susphy_state = (dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)) &
> + DWC3_GUSB2PHYCFG_SUSPHY) ||
> + (dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)) &
> + DWC3_GUSB3PIPECTL_SUSPHY);
> + /*
> + * TI AM62 platform requires SUSPHY to be
> + * enabled for system suspend to work.
> + */
> + if (!dwc->susphy_state)
> + dwc3_enable_susphy(dwc, true);
> + }
>
> switch (dwc->current_dr_role) {
> case DWC3_GCTL_PRTCAP_DEVICE:
> @@ -2398,15 +2406,6 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> break;
> }
>
> - if (!PMSG_IS_AUTO(msg)) {
> - /*
> - * TI AM62 platform requires SUSPHY to be
> - * enabled for system suspend to work.
> - */
> - if (!dwc->susphy_state)
> - dwc3_enable_susphy(dwc, true);
> - }
> -
> return 0;
> }
>
>
> ---
> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> change-id: 20241102-am62-lpm-usb-fix-347dd86135c1
>
> Best regards,
> --
> Roger Quadros <rogerq@kernel.org>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: dwc3: fix fault at system suspend if device was already runtime suspended
2024-11-04 14:00 [PATCH] usb: dwc3: fix fault at system suspend if device was already runtime suspended Roger Quadros
2024-11-04 18:04 ` William McVicker
@ 2024-11-04 23:08 ` Thinh Nguyen
1 sibling, 0 replies; 3+ messages in thread
From: Thinh Nguyen @ 2024-11-04 23:08 UTC (permalink / raw)
To: Roger Quadros
Cc: Thinh Nguyen, Greg Kroah-Hartman, Dhruva Gole, sashal@kernel.org,
William McVicker, Chris Morgan, Vishal Mahaveer, msp@baylibre.com,
srk@ti.com, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Hi Roger,
On Mon, Nov 04, 2024, Roger Quadros wrote:
> If the device was already runtime suspended then during system suspend
> we cannot access the device registers else it will crash.
>
> Also we cannot access any registers after dwc3_core_exit() on some
> platforms so move the dwc3_enable_susphy() call to the top.
>
> Cc: stable@vger.kernel.org # v5.15+
> Reported-by: William McVicker <willmcvicker@google.com>
> Closes: https://urldefense.com/v3/__https://lore.kernel.org/all/ZyVfcUuPq56R2m1Y@google.com__;!!A4F2R9G_pg!a2ySn942v8-FZqAgru6elfn0Auxnl1JyBveK9z2iAeLZpEpVajfl3pbfl5K2hVlGx5YqfRazbhqF8ZJ2t3f_$
> Fixes: 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms")
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
> drivers/usb/dwc3/core.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 427e5660f87c..98114c2827c0 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -2342,10 +2342,18 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> u32 reg;
> int i;
>
> - dwc->susphy_state = (dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)) &
> - DWC3_GUSB2PHYCFG_SUSPHY) ||
> - (dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)) &
> - DWC3_GUSB3PIPECTL_SUSPHY);
> + if (!pm_runtime_suspended(dwc->dev) && !PMSG_IS_AUTO(msg)) {
> + dwc->susphy_state = (dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)) &
> + DWC3_GUSB2PHYCFG_SUSPHY) ||
> + (dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)) &
> + DWC3_GUSB3PIPECTL_SUSPHY);
> + /*
> + * TI AM62 platform requires SUSPHY to be
> + * enabled for system suspend to work.
> + */
> + if (!dwc->susphy_state)
> + dwc3_enable_susphy(dwc, true);
> + }
>
> switch (dwc->current_dr_role) {
> case DWC3_GCTL_PRTCAP_DEVICE:
> @@ -2398,15 +2406,6 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> break;
> }
>
> - if (!PMSG_IS_AUTO(msg)) {
> - /*
> - * TI AM62 platform requires SUSPHY to be
> - * enabled for system suspend to work.
> - */
> - if (!dwc->susphy_state)
> - dwc3_enable_susphy(dwc, true);
> - }
> -
> return 0;
> }
>
>
> ---
> base-commit: 42f7652d3eb527d03665b09edac47f85fb600924
> change-id: 20241102-am62-lpm-usb-fix-347dd86135c1
>
> Best regards,
> --
> Roger Quadros <rogerq@kernel.org>
>
Thanks for the catch and quick turnaround!
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Thanks,
Thinh
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-04 23:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-04 14:00 [PATCH] usb: dwc3: fix fault at system suspend if device was already runtime suspended Roger Quadros
2024-11-04 18:04 ` William McVicker
2024-11-04 23:08 ` Thinh Nguyen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox