Skip to contentSkip to conversation

usb: dwc3: core: Fix system suspend on TI AM62 platforms

PATCH In review Roger Quadros 4 participants 8 messages Oct 11, 2024 – Nov 4, 2024

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 usb: dwc3: core: Fix system suspend on TI AM62 platforms

Since commit 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init"), system suspend is broken on AM62 TI platforms.

Before that commit, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY bits (hence forth called 2 SUSPHY bits) were being set during core initialization and even during core re-initialization after a system suspend/resume.

These bits are required to be set for system suspend/resume to work correctly on AM62 platforms.

Since that commit, the 2 SUSPHY bits are not set for DEVICE/OTG mode if gadget driver is not loaded and started.
For Host mode, the 2 SUSPHY bits are set before the first system suspend but get cleared at system resume during core re-init and are never set again.

This patch resovles these two issues by ensuring the 2 SUSPHY bits are set before system suspend and restored to the original state during system resume.

Fixes 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init")

---
Changes in v3:
- Fix single line comment style
- add DWC3_GUSB3PIPECTL_SUSPHY to documentation of susphy_state
- Added Acked-by tag
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Fix comment style
- Use both USB3 and USB2 SUSPHY bits to determine susphy_state during system suspend/resume.
- Restore SUSPHY bits at system resume regardless if it was set or cleared before system suspend.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
 drivers/usb/dwc3/core.c | 19 +++++++++++++++++++
 drivers/usb/dwc3/core.h |  3 +++
 2 files changed, 22 insertions(+)
drivers/usb/dwc3/core.c +19 -0
index 9eb085f359ce..ca77f0b186c4 100644
@@ -2336,6 +2336,11 @@ 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);
+
 	switch (dwc->current_dr_role) {
 	case DWC3_GCTL_PRTCAP_DEVICE:
 		if (pm_runtime_suspended(dwc->dev))
@@ -2387,6 +2392,15 @@ 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;
 }
 
@@ -2454,6 +2468,11 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
 		break;
 	}
 
+	if (!PMSG_IS_AUTO(msg)) {
+		/* restore SUSPHY state to that before system suspend. */
+		dwc3_enable_susphy(dwc, dwc->susphy_state);
+	}
+
 	return 0;
 }
 
drivers/usb/dwc3/core.h +3 -0
index c71240e8f7c7..31de4b57ae7c 100644
@@ -1150,6 +1150,8 @@ struct dwc3_scratchpad_array {
  * @sys_wakeup: set if the device may do system wakeup.
  * @wakeup_configured: set if the device is configured for remote wakeup.
  * @suspended: set to track suspend event due to U3/L2.
+ * @susphy_state: state of DWC3_GUSB2PHYCFG_SUSPHY + DWC3_GUSB3PIPECTL_SUSPHY
+ *		  before PM suspend.
  * @imod_interval: set the interrupt moderation interval in 250ns
  *			increments or 0 to disable.
  * @max_cfg_eps: current max number of IN eps used across all USB configs.
@@ -1382,6 +1384,7 @@ struct dwc3 {
 	unsigned		sys_wakeup:1;
 	unsigned		wakeup_configured:1;
 	unsigned		suspended:1;
+	unsigned		susphy_state:1;
 
 	u16			imod_interval;
 

---
base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc
change-id: 20240923-am62-lpm-usb-f420917bd707

Best regards,

Signature · 1 line
Roger Quadros <[email protected]>
Tested-by on v3 1/1
#1 lore raw
Quoted 16 lines · Roger Quadros

Since commit 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init"), system suspend is broken on AM62 TI platforms.

Before that commit, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY bits (hence forth called 2 SUSPHY bits) were being set during core initialization and even during core re-initialization after a system suspend/resume.

These bits are required to be set for system suspend/resume to work correctly on AM62 platforms.

Since that commit, the 2 SUSPHY bits are not set for DEVICE/OTG mode if gadget driver is not loaded and started.
For Host mode, the 2 SUSPHY bits are set before the first system suspend but get cleared at system resume during core re-init and are never set again.

Quoted 8 lines

This patch resovles these two issues by ensuring the 2 SUSPHY bits are set before system suspend and restored to the original state during system resume.

Fixes 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init")

Thanks for fixing!

Best
Markus

Reviewed-by on v3 1/1
#2 lore raw
In reply to Roger Quadros
Quoted 11 lines · Roger Quadros

Since commit 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init"), system suspend is broken on AM62 TI platforms.

Before that commit, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY bits (hence forth called 2 SUSPHY bits) were being set during core initialization and even during core re-initialization after a system suspend/resume.

These bits are required to be set for system suspend/resume to work correctly on AM62 platforms.

Quoted 7 lines

Since that commit, the 2 SUSPHY bits are not set for DEVICE/OTG mode if gadget driver is not loaded and started.
For Host mode, the 2 SUSPHY bits are set before the first system suspend but get cleared at system resume during core re-init and are never set again.

This patch resovles these two issues by ensuring the 2 SUSPHY bits are set before system suspend and restored to the original state during system resume.

Thanks for the fix Roger,

Quoted 8 lines
Fixes 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init")

---
Changes in v3:

[...]

Signature · 3 lines
Best regards,
Dhruva Gole
Texas Instruments Incorporated
#3 lore raw
In reply to Roger Quadros

[email protected]

Hi Roger,

Quoted 46 lines · Roger Quadros

Since commit 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init"), system suspend is broken on AM62 TI platforms.

Before that commit, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY bits (hence forth called 2 SUSPHY bits) were being set during core initialization and even during core re-initialization after a system suspend/resume.

These bits are required to be set for system suspend/resume to work correctly on AM62 platforms.

Since that commit, the 2 SUSPHY bits are not set for DEVICE/OTG mode if gadget driver is not loaded and started.
For Host mode, the 2 SUSPHY bits are set before the first system suspend but get cleared at system resume during core re-init and are never set again.

This patch resovles these two issues by ensuring the 2 SUSPHY bits are set before system suspend and restored to the original state during system resume.

Fixes 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init")

---
Changes in v3:
- Fix single line comment style
- add DWC3_GUSB3PIPECTL_SUSPHY to documentation of susphy_state
- Added Acked-by tag
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Fix comment style
- Use both USB3 and USB2 SUSPHY bits to determine susphy_state during system suspend/resume.
- Restore SUSPHY bits at system resume regardless if it was set or cleared before system suspend.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
 drivers/usb/dwc3/core.c | 19 +++++++++++++++++++
 drivers/usb/dwc3/core.h |  3 +++
 2 files changed, 22 insertions(+)
drivers/usb/dwc3/core.c +0 -0
index 9eb085f359ce..ca77f0b186c4 100644

drivers/usb/dwc3/core.c static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg) · line 2343

Quoted 8 lines
drivers/usb/dwc3/core.c +5 -0
@@ -2336,6 +2336,11 @@ 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);
+

I'm running into an issue on my Pixel 6 device with this change when the dwc3-exynos device has runtime PM enabled. Basically, after the device boots up and I disconnect USB, the dwc3-exynos device enters runtime suspend followed by system suspend 15 seconds later. On system suspend, the clocks powering these dwc3 registers are off which results in an SError. I have verified that reverting this change fixes the issue.

I noticed that dwc3-qcom.c also supports runtime PM for their dwc3 device and most likely is affected by this as well. It would be great if someone with a Qualcomm device could test out dwc3 suspend as well.

Here is the crash stack:

SError Interrupt on CPU7, code 0x00000000be000011 -- SError

  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  96 irq, BUS_DATA0 group, 0x0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : readl+0x40/0x80
  lr : readl+0x38/0x80
  sp : ffffffc08baa39a0
  x29: ffffffc08baa39a0 x28: ffffffd4dd140000 x27: ffffffd4dd140d70
  x26: ffffffd4dd2b2000 x25: ffffff800cef2410 x24: ffffff800cef24c0
  x23: ffffffd4dd24e000 x22: ffffff887df59440 x21: ffffffc085298100
  x20: ffffffd4db8acf60 x19: ffffffc085298200 x18: ffffffc091b730b0
  x17: 000000002a703c0b x16: 000000002a703c0b x15: 0000000000953000
  x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
  x11: 7f7f7f7f7f7fffff x10: 0000000000000000 x9 : ffffffd4dc0d7d48
  x8 : 0000000000000000 x7 : 0000000000008000 x6 : 0000000000000000
  x5 : 500020737562ffff x4 : 500020737562ffff x3 : ffffffd4db8acf60
  x2 : ffffffd4db8a7bac x1 : ffffffc085298200 x0 : 0000000000000020
  Kernel panic - not syncing: Asynchronous SError Interrupt
  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  Call trace:
   dump_backtrace+0xec/0x128
   show_stack+0x18/0x28
   dump_stack_lvl+0x40/0x88
   dump_stack+0x18/0x24
   panic+0x134/0x45c
   nmi_panic+0x3c/0x88
   arm64_serror_panic+0x64/0x8c
   do_serror+0xc4/0xc8
   el1h_64_error_handler+0x34/0x48
   el1h_64_error+0x68/0x6c
   readl+0x40/0x80
   dwc3_suspend_common+0x34/0x454
   dwc3_suspend+0x20/0x40
   platform_pm_suspend+0x40/0x90
   dpm_run_callback+0x60/0x250
   device_suspend+0x334/0x614
   dpm_suspend+0xc4/0x368
   dpm_suspend_start+0x90/0x100
   suspend_devices_and_enter+0x128/0xad0
   pm_suspend+0x354/0x650
   state_store+0x104/0x144
   kobj_attr_store+0x30/0x48
   sysfs_kf_write+0x54/0x6c
   kernfs_fop_write_iter+0x104/0x1e4
   vfs_write+0x3bc/0x50c
   ksys_write+0x78/0xe8
   __arm64_sys_write+0x1c/0x2c
   invoke_syscall+0x58/0x10c
   el0_svc_common+0xa8/0xdc
   do_el0_svc+0x1c/0x28
   el0_svc+0x38/0x6c
   el0t_64_sync_handler+0x70/0xbc
   el0t_64_sync+0x1a8/0x1ac

Thanks,
Will

<snip>

#4 lore raw

Hi William,

Quoted 67 lines · William McVicker

[email protected]

Hi Roger,

Quoted 46 lines · Roger Quadros

Since commit 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init"), system suspend is broken on AM62 TI platforms.

Before that commit, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY bits (hence forth called 2 SUSPHY bits) were being set during core initialization and even during core re-initialization after a system suspend/resume.

These bits are required to be set for system suspend/resume to work correctly on AM62 platforms.

Since that commit, the 2 SUSPHY bits are not set for DEVICE/OTG mode if gadget driver is not loaded and started.
For Host mode, the 2 SUSPHY bits are set before the first system suspend but get cleared at system resume during core re-init and are never set again.

This patch resovles these two issues by ensuring the 2 SUSPHY bits are set before system suspend and restored to the original state during system resume.

Fixes 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init")

---
Changes in v3:
- Fix single line comment style
- add DWC3_GUSB3PIPECTL_SUSPHY to documentation of susphy_state
- Added Acked-by tag
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Fix comment style
- Use both USB3 and USB2 SUSPHY bits to determine susphy_state during system suspend/resume.
- Restore SUSPHY bits at system resume regardless if it was set or cleared before system suspend.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
 drivers/usb/dwc3/core.c | 19 +++++++++++++++++++
 drivers/usb/dwc3/core.h |  3 +++
 2 files changed, 22 insertions(+)
drivers/usb/dwc3/core.c +0 -0
index 9eb085f359ce..ca77f0b186c4 100644

drivers/usb/dwc3/core.c static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg) · line 2343

Quoted 8 lines
drivers/usb/dwc3/core.c +5 -0
@@ -2336,6 +2336,11 @@ 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);
+

I'm running into an issue on my Pixel 6 device with this change when the dwc3-exynos device has runtime PM enabled. Basically, after the device boots up and I disconnect USB, the dwc3-exynos device enters runtime suspend followed by system suspend 15 seconds later. On system suspend, the clocks powering these dwc3 registers are off which results in an SError. I have verified that reverting this change fixes the issue.

Quoted 3 lines

I noticed that dwc3-qcom.c also supports runtime PM for their dwc3 device and most likely is affected by this as well. It would be great if someone with a Qualcomm device could test out dwc3 suspend as well.

Chris was facing another issue with this patch on Rockchip RK3566 [1]

Looks like we totally missed the runtime suspended case I'll think about a solution and send something by today.

[1] - https://lore.kernel.org/all/[email protected]/

Quoted 66 lines

Here is the crash stack:

SError Interrupt on CPU7, code 0x00000000be000011 -- SError

  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  96 irq, BUS_DATA0 group, 0x0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : readl+0x40/0x80
  lr : readl+0x38/0x80
  sp : ffffffc08baa39a0
  x29: ffffffc08baa39a0 x28: ffffffd4dd140000 x27: ffffffd4dd140d70
  x26: ffffffd4dd2b2000 x25: ffffff800cef2410 x24: ffffff800cef24c0
  x23: ffffffd4dd24e000 x22: ffffff887df59440 x21: ffffffc085298100
  x20: ffffffd4db8acf60 x19: ffffffc085298200 x18: ffffffc091b730b0
  x17: 000000002a703c0b x16: 000000002a703c0b x15: 0000000000953000
  x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
  x11: 7f7f7f7f7f7fffff x10: 0000000000000000 x9 : ffffffd4dc0d7d48
  x8 : 0000000000000000 x7 : 0000000000008000 x6 : 0000000000000000
  x5 : 500020737562ffff x4 : 500020737562ffff x3 : ffffffd4db8acf60
  x2 : ffffffd4db8a7bac x1 : ffffffc085298200 x0 : 0000000000000020
  Kernel panic - not syncing: Asynchronous SError Interrupt
  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  Call trace:
   dump_backtrace+0xec/0x128
   show_stack+0x18/0x28
   dump_stack_lvl+0x40/0x88
   dump_stack+0x18/0x24
   panic+0x134/0x45c
   nmi_panic+0x3c/0x88
   arm64_serror_panic+0x64/0x8c
   do_serror+0xc4/0xc8
   el1h_64_error_handler+0x34/0x48
   el1h_64_error+0x68/0x6c
   readl+0x40/0x80
   dwc3_suspend_common+0x34/0x454
   dwc3_suspend+0x20/0x40
   platform_pm_suspend+0x40/0x90
   dpm_run_callback+0x60/0x250
   device_suspend+0x334/0x614
   dpm_suspend+0xc4/0x368
   dpm_suspend_start+0x90/0x100
   suspend_devices_and_enter+0x128/0xad0
   pm_suspend+0x354/0x650
   state_store+0x104/0x144
   kobj_attr_store+0x30/0x48
   sysfs_kf_write+0x54/0x6c
   kernfs_fop_write_iter+0x104/0x1e4
   vfs_write+0x3bc/0x50c
   ksys_write+0x78/0xe8
   __arm64_sys_write+0x1c/0x2c
   invoke_syscall+0x58/0x10c
   el0_svc_common+0xa8/0xdc
   do_el0_svc+0x1c/0x28
   el0_svc+0x38/0x6c
   el0t_64_sync_handler+0x70/0xbc
   el0t_64_sync+0x1a8/0x1ac

Thanks,
Will

<snip>

Signature · 2 lines
cheers,
-roger
#5 lore raw

Hi William & Chris,

Quoted 74 lines · Roger Quadros

Hi William,

Quoted 67 lines · William McVicker

[email protected]

Hi Roger,

Quoted 46 lines · Roger Quadros

Since commit 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init"), system suspend is broken on AM62 TI platforms.

Before that commit, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY bits (hence forth called 2 SUSPHY bits) were being set during core initialization and even during core re-initialization after a system suspend/resume.

These bits are required to be set for system suspend/resume to work correctly on AM62 platforms.

Since that commit, the 2 SUSPHY bits are not set for DEVICE/OTG mode if gadget driver is not loaded and started.
For Host mode, the 2 SUSPHY bits are set before the first system suspend but get cleared at system resume during core re-init and are never set again.

This patch resovles these two issues by ensuring the 2 SUSPHY bits are set before system suspend and restored to the original state during system resume.

Fixes 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init")

---
Changes in v3:
- Fix single line comment style
- add DWC3_GUSB3PIPECTL_SUSPHY to documentation of susphy_state
- Added Acked-by tag
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Fix comment style
- Use both USB3 and USB2 SUSPHY bits to determine susphy_state during system suspend/resume.
- Restore SUSPHY bits at system resume regardless if it was set or cleared before system suspend.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
 drivers/usb/dwc3/core.c | 19 +++++++++++++++++++
 drivers/usb/dwc3/core.h |  3 +++
 2 files changed, 22 insertions(+)
drivers/usb/dwc3/core.c +0 -0
index 9eb085f359ce..ca77f0b186c4 100644

drivers/usb/dwc3/core.c static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg) · line 2343

Quoted 8 lines
drivers/usb/dwc3/core.c +5 -0
@@ -2336,6 +2336,11 @@ 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);
+

I'm running into an issue on my Pixel 6 device with this change when the dwc3-exynos device has runtime PM enabled. Basically, after the device boots up and I disconnect USB, the dwc3-exynos device enters runtime suspend followed by system suspend 15 seconds later. On system suspend, the clocks powering these dwc3 registers are off which results in an SError. I have verified that reverting this change fixes the issue.

Quoted 3 lines

I noticed that dwc3-qcom.c also supports runtime PM for their dwc3 device and most likely is affected by this as well. It would be great if someone with a Qualcomm device could test out dwc3 suspend as well.

Quoted 68 lines

Chris was facing another issue with this patch on Rockchip RK3566 [1]

Looks like we totally missed the runtime suspended case I'll think about a solution and send something by today.

[1] - https://lore.kernel.org/all/[email protected]/

Quoted 61 lines

Here is the crash stack:

SError Interrupt on CPU7, code 0x00000000be000011 -- SError

  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  96 irq, BUS_DATA0 group, 0x0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : readl+0x40/0x80
  lr : readl+0x38/0x80
  sp : ffffffc08baa39a0
  x29: ffffffc08baa39a0 x28: ffffffd4dd140000 x27: ffffffd4dd140d70
  x26: ffffffd4dd2b2000 x25: ffffff800cef2410 x24: ffffff800cef24c0
  x23: ffffffd4dd24e000 x22: ffffff887df59440 x21: ffffffc085298100
  x20: ffffffd4db8acf60 x19: ffffffc085298200 x18: ffffffc091b730b0
  x17: 000000002a703c0b x16: 000000002a703c0b x15: 0000000000953000
  x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
  x11: 7f7f7f7f7f7fffff x10: 0000000000000000 x9 : ffffffd4dc0d7d48
  x8 : 0000000000000000 x7 : 0000000000008000 x6 : 0000000000000000
  x5 : 500020737562ffff x4 : 500020737562ffff x3 : ffffffd4db8acf60
  x2 : ffffffd4db8a7bac x1 : ffffffc085298200 x0 : 0000000000000020
  Kernel panic - not syncing: Asynchronous SError Interrupt
  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  Call trace:
   dump_backtrace+0xec/0x128
   show_stack+0x18/0x28
   dump_stack_lvl+0x40/0x88
   dump_stack+0x18/0x24
   panic+0x134/0x45c
   nmi_panic+0x3c/0x88
   arm64_serror_panic+0x64/0x8c
   do_serror+0xc4/0xc8
   el1h_64_error_handler+0x34/0x48
   el1h_64_error+0x68/0x6c
   readl+0x40/0x80
   dwc3_suspend_common+0x34/0x454
   dwc3_suspend+0x20/0x40
   platform_pm_suspend+0x40/0x90
   dpm_run_callback+0x60/0x250
   device_suspend+0x334/0x614
   dpm_suspend+0xc4/0x368
   dpm_suspend_start+0x90/0x100
   suspend_devices_and_enter+0x128/0xad0
   pm_suspend+0x354/0x650
   state_store+0x104/0x144
   kobj_attr_store+0x30/0x48
   sysfs_kf_write+0x54/0x6c
   kernfs_fop_write_iter+0x104/0x1e4
   vfs_write+0x3bc/0x50c
   ksys_write+0x78/0xe8
   __arm64_sys_write+0x1c/0x2c
   invoke_syscall+0x58/0x10c
   el0_svc_common+0xa8/0xdc
   do_el0_svc+0x1c/0x28
   el0_svc+0x38/0x6c
   el0t_64_sync_handler+0x70/0xbc
   el0t_64_sync+0x1a8/0x1ac

Does this patch fix it for you?

From ee8b353523556a29a423261af9c15be261941ff8 Mon Sep 17 00:00:00 2001
From: Roger Quadros <[email protected]>
Date: Sat, 2 Nov 2024 14:14:47 +0200
Subject: [PATCH] usb: dwc3: fix fault at system suspend if device was already
runtime suspended

If the device was already runtime suspended then during system suspend we cannot access the device registers else it will crash.

Cc [email protected] # v5.15+
Fixes 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms")
---
 drivers/usb/dwc3/core.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/usb/dwc3/core.c +6 -4
index 427e5660f87c..4933f1b4d9c6 100644
@@ -2342,10 +2342,12 @@ 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)) {
+		dwc->susphy_state = (dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)) &
+				    DWC3_GUSB2PHYCFG_SUSPHY) ||
+				    (dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)) &
+				    DWC3_GUSB3PIPECTL_SUSPHY);
+	}
 
 	switch (dwc->current_dr_role) {
 	case DWC3_GCTL_PRTCAP_DEVICE:

base-commit: 4b57d665bce1306a2a887cb760aa0c0e7efb42ab

Signature · 5 lines
2.34.1

-- 
cheers,
-roger
#6 lore raw

Hi,

Quoted 188 lines · Roger Quadros

Hi William & Chris,

Quoted 74 lines · Roger Quadros

Hi William,

Quoted 67 lines · William McVicker

[email protected]

Hi Roger,

Quoted 46 lines · Roger Quadros

Since commit 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init"), system suspend is broken on AM62 TI platforms.

Before that commit, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY bits (hence forth called 2 SUSPHY bits) were being set during core initialization and even during core re-initialization after a system suspend/resume.

These bits are required to be set for system suspend/resume to work correctly on AM62 platforms.

Since that commit, the 2 SUSPHY bits are not set for DEVICE/OTG mode if gadget driver is not loaded and started.
For Host mode, the 2 SUSPHY bits are set before the first system suspend but get cleared at system resume during core re-init and are never set again.

This patch resovles these two issues by ensuring the 2 SUSPHY bits are set before system suspend and restored to the original state during system resume.

Fixes 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init")

---
Changes in v3:
- Fix single line comment style
- add DWC3_GUSB3PIPECTL_SUSPHY to documentation of susphy_state
- Added Acked-by tag
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Fix comment style
- Use both USB3 and USB2 SUSPHY bits to determine susphy_state during system suspend/resume.
- Restore SUSPHY bits at system resume regardless if it was set or cleared before system suspend.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
 drivers/usb/dwc3/core.c | 19 +++++++++++++++++++
 drivers/usb/dwc3/core.h |  3 +++
 2 files changed, 22 insertions(+)
drivers/usb/dwc3/core.c +0 -0
index 9eb085f359ce..ca77f0b186c4 100644

drivers/usb/dwc3/core.c static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg) · line 2343

Quoted 8 lines
drivers/usb/dwc3/core.c +5 -0
@@ -2336,6 +2336,11 @@ 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);
+

I'm running into an issue on my Pixel 6 device with this change when the dwc3-exynos device has runtime PM enabled. Basically, after the device boots up and I disconnect USB, the dwc3-exynos device enters runtime suspend followed by system suspend 15 seconds later. On system suspend, the clocks powering these dwc3 registers are off which results in an SError. I have verified that reverting this change fixes the issue.

Quoted 3 lines

I noticed that dwc3-qcom.c also supports runtime PM for their dwc3 device and most likely is affected by this as well. It would be great if someone with a Qualcomm device could test out dwc3 suspend as well.

Quoted 68 lines

Chris was facing another issue with this patch on Rockchip RK3566 [1]

Looks like we totally missed the runtime suspended case I'll think about a solution and send something by today.

[1] - https://lore.kernel.org/all/[email protected]/

Quoted 61 lines

Here is the crash stack:

SError Interrupt on CPU7, code 0x00000000be000011 -- SError

  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  96 irq, BUS_DATA0 group, 0x0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : readl+0x40/0x80
  lr : readl+0x38/0x80
  sp : ffffffc08baa39a0
  x29: ffffffc08baa39a0 x28: ffffffd4dd140000 x27: ffffffd4dd140d70
  x26: ffffffd4dd2b2000 x25: ffffff800cef2410 x24: ffffff800cef24c0
  x23: ffffffd4dd24e000 x22: ffffff887df59440 x21: ffffffc085298100
  x20: ffffffd4db8acf60 x19: ffffffc085298200 x18: ffffffc091b730b0
  x17: 000000002a703c0b x16: 000000002a703c0b x15: 0000000000953000
  x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
  x11: 7f7f7f7f7f7fffff x10: 0000000000000000 x9 : ffffffd4dc0d7d48
  x8 : 0000000000000000 x7 : 0000000000008000 x6 : 0000000000000000
  x5 : 500020737562ffff x4 : 500020737562ffff x3 : ffffffd4db8acf60
  x2 : ffffffd4db8a7bac x1 : ffffffc085298200 x0 : 0000000000000020
  Kernel panic - not syncing: Asynchronous SError Interrupt
  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  Call trace:
   dump_backtrace+0xec/0x128
   show_stack+0x18/0x28
   dump_stack_lvl+0x40/0x88
   dump_stack+0x18/0x24
   panic+0x134/0x45c
   nmi_panic+0x3c/0x88
   arm64_serror_panic+0x64/0x8c
   do_serror+0xc4/0xc8
   el1h_64_error_handler+0x34/0x48
   el1h_64_error+0x68/0x6c
   readl+0x40/0x80
   dwc3_suspend_common+0x34/0x454
   dwc3_suspend+0x20/0x40
   platform_pm_suspend+0x40/0x90
   dpm_run_callback+0x60/0x250
   device_suspend+0x334/0x614
   dpm_suspend+0xc4/0x368
   dpm_suspend_start+0x90/0x100
   suspend_devices_and_enter+0x128/0xad0
   pm_suspend+0x354/0x650
   state_store+0x104/0x144
   kobj_attr_store+0x30/0x48
   sysfs_kf_write+0x54/0x6c
   kernfs_fop_write_iter+0x104/0x1e4
   vfs_write+0x3bc/0x50c
   ksys_write+0x78/0xe8
   __arm64_sys_write+0x1c/0x2c
   invoke_syscall+0x58/0x10c
   el0_svc_common+0xa8/0xdc
   do_el0_svc+0x1c/0x28
   el0_svc+0x38/0x6c
   el0t_64_sync_handler+0x70/0xbc
   el0t_64_sync+0x1a8/0x1ac

Does this patch fix it for you?

From ee8b353523556a29a423261af9c15be261941ff8 Mon Sep 17 00:00:00 2001
From: Roger Quadros <[email protected]>
Date: Sat, 2 Nov 2024 14:14:47 +0200
Subject: [PATCH] usb: dwc3: fix fault at system suspend if device was already
runtime suspended

If the device was already runtime suspended then during system suspend we cannot access the device registers else it will crash.

Cc [email protected] # v5.15+
Fixes 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms")
---
 drivers/usb/dwc3/core.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/usb/dwc3/core.c +6 -4
index 427e5660f87c..4933f1b4d9c6 100644
@@ -2342,10 +2342,12 @@ 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)) {
+		dwc->susphy_state = (dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)) &
+				    DWC3_GUSB2PHYCFG_SUSPHY) ||
+				    (dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)) &
+				    DWC3_GUSB3PIPECTL_SUSPHY);
+	}
 
 	switch (dwc->current_dr_role) {
 	case DWC3_GCTL_PRTCAP_DEVICE:
Quoted 1 line

base-commit: 4b57d665bce1306a2a887cb760aa0c0e7efb42ab

I think this is not sufficient to fix the issue as there is still a call to dwc3_enable_susphy() in the end which needs to be avoided if already runtime suspended.

Signature · 2 lines
cheers,
-roger
#7 lore raw

Hi,

Quoted 188 lines · Roger Quadros

Hi,

Quoted 185 lines · Roger Quadros

Hi William & Chris,

Quoted 74 lines · Roger Quadros

Hi William,

Quoted 67 lines · William McVicker

[email protected]

Hi Roger,

Quoted 46 lines · Roger Quadros

Since commit 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init"), system suspend is broken on AM62 TI platforms.

Before that commit, both DWC3_GUSB3PIPECTL_SUSPHY and DWC3_GUSB2PHYCFG_SUSPHY bits (hence forth called 2 SUSPHY bits) were being set during core initialization and even during core re-initialization after a system suspend/resume.

These bits are required to be set for system suspend/resume to work correctly on AM62 platforms.

Since that commit, the 2 SUSPHY bits are not set for DEVICE/OTG mode if gadget driver is not loaded and started.
For Host mode, the 2 SUSPHY bits are set before the first system suspend but get cleared at system resume during core re-init and are never set again.

This patch resovles these two issues by ensuring the 2 SUSPHY bits are set before system suspend and restored to the original state during system resume.

Fixes 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init")

---
Changes in v3:
- Fix single line comment style
- add DWC3_GUSB3PIPECTL_SUSPHY to documentation of susphy_state
- Added Acked-by tag
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Fix comment style
- Use both USB3 and USB2 SUSPHY bits to determine susphy_state during system suspend/resume.
- Restore SUSPHY bits at system resume regardless if it was set or cleared before system suspend.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
 drivers/usb/dwc3/core.c | 19 +++++++++++++++++++
 drivers/usb/dwc3/core.h |  3 +++
 2 files changed, 22 insertions(+)
drivers/usb/dwc3/core.c +0 -0
index 9eb085f359ce..ca77f0b186c4 100644

drivers/usb/dwc3/core.c static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg) · line 2343

Quoted 8 lines
drivers/usb/dwc3/core.c +5 -0
@@ -2336,6 +2336,11 @@ 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);
+

I'm running into an issue on my Pixel 6 device with this change when the dwc3-exynos device has runtime PM enabled. Basically, after the device boots up and I disconnect USB, the dwc3-exynos device enters runtime suspend followed by system suspend 15 seconds later. On system suspend, the clocks powering these dwc3 registers are off which results in an SError. I have verified that reverting this change fixes the issue.

Quoted 3 lines

I noticed that dwc3-qcom.c also supports runtime PM for their dwc3 device and most likely is affected by this as well. It would be great if someone with a Qualcomm device could test out dwc3 suspend as well.

Quoted 68 lines

Chris was facing another issue with this patch on Rockchip RK3566 [1]

Looks like we totally missed the runtime suspended case I'll think about a solution and send something by today.

[1] - https://lore.kernel.org/all/[email protected]/

Quoted 61 lines

Here is the crash stack:

SError Interrupt on CPU7, code 0x00000000be000011 -- SError

  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  96 irq, BUS_DATA0 group, 0x0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : readl+0x40/0x80
  lr : readl+0x38/0x80
  sp : ffffffc08baa39a0
  x29: ffffffc08baa39a0 x28: ffffffd4dd140000 x27: ffffffd4dd140d70
  x26: ffffffd4dd2b2000 x25: ffffff800cef2410 x24: ffffff800cef24c0
  x23: ffffffd4dd24e000 x22: ffffff887df59440 x21: ffffffc085298100
  x20: ffffffd4db8acf60 x19: ffffffc085298200 x18: ffffffc091b730b0
  x17: 000000002a703c0b x16: 000000002a703c0b x15: 0000000000953000
  x14: 0000000000000000 x13: 0000000000000030 x12: 0101010101010101
  x11: 7f7f7f7f7f7fffff x10: 0000000000000000 x9 : ffffffd4dc0d7d48
  x8 : 0000000000000000 x7 : 0000000000008000 x6 : 0000000000000000
  x5 : 500020737562ffff x4 : 500020737562ffff x3 : ffffffd4db8acf60
  x2 : ffffffd4db8a7bac x1 : ffffffc085298200 x0 : 0000000000000020
  Kernel panic - not syncing: Asynchronous SError Interrupt
  CPU: 7 UID: 1000 PID: 5661 Comm: binder:477_1 Tainted: G        W  OE      6.12.0-rc3-android16-0-maybe-dirty-4k #1 0439eacb3cff642033630df7ee2e250e0625f2f0
  Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
  Hardware name: Raven DVT (DT)
  Call trace:
   dump_backtrace+0xec/0x128
   show_stack+0x18/0x28
   dump_stack_lvl+0x40/0x88
   dump_stack+0x18/0x24
   panic+0x134/0x45c
   nmi_panic+0x3c/0x88
   arm64_serror_panic+0x64/0x8c
   do_serror+0xc4/0xc8
   el1h_64_error_handler+0x34/0x48
   el1h_64_error+0x68/0x6c
   readl+0x40/0x80
   dwc3_suspend_common+0x34/0x454
   dwc3_suspend+0x20/0x40
   platform_pm_suspend+0x40/0x90
   dpm_run_callback+0x60/0x250
   device_suspend+0x334/0x614
   dpm_suspend+0xc4/0x368
   dpm_suspend_start+0x90/0x100
   suspend_devices_and_enter+0x128/0xad0
   pm_suspend+0x354/0x650
   state_store+0x104/0x144
   kobj_attr_store+0x30/0x48
   sysfs_kf_write+0x54/0x6c
   kernfs_fop_write_iter+0x104/0x1e4
   vfs_write+0x3bc/0x50c
   ksys_write+0x78/0xe8
   __arm64_sys_write+0x1c/0x2c
   invoke_syscall+0x58/0x10c
   el0_svc_common+0xa8/0xdc
   do_el0_svc+0x1c/0x28
   el0_svc+0x38/0x6c
   el0t_64_sync_handler+0x70/0xbc
   el0t_64_sync+0x1a8/0x1ac

Does this patch fix it for you?

From ee8b353523556a29a423261af9c15be261941ff8 Mon Sep 17 00:00:00 2001
From: Roger Quadros <[email protected]>
Date: Sat, 2 Nov 2024 14:14:47 +0200
Subject: [PATCH] usb: dwc3: fix fault at system suspend if device was already
runtime suspended

If the device was already runtime suspended then during system suspend we cannot access the device registers else it will crash.

Cc [email protected] # v5.15+
Fixes 705e3ce37bcc ("usb: dwc3: core: Fix system suspend on TI AM62 platforms")
---
 drivers/usb/dwc3/core.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/usb/dwc3/core.c +6 -4
index 427e5660f87c..4933f1b4d9c6 100644
@@ -2342,10 +2342,12 @@ 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)) {
+		dwc->susphy_state = (dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0)) &
+				    DWC3_GUSB2PHYCFG_SUSPHY) ||
+				    (dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0)) &
+				    DWC3_GUSB3PIPECTL_SUSPHY);
+	}
 
Quoted 9 lines
Quoted 4 lines
drivers/usb/dwc3/core.c +0 -0
 	switch (dwc->current_dr_role) {
 	case DWC3_GCTL_PRTCAP_DEVICE:

base-commit: 4b57d665bce1306a2a887cb760aa0c0e7efb42ab

I think this is not sufficient to fix the issue as there is still a call to dwc3_enable_susphy() in the end which needs to be avoided if already runtime suspended.

I've sent a new patch [1]. Please test and give your feedback there. Thanks.

[1]- https://lore.kernel.org/all/[email protected]/

Signature · 2 lines
cheers,
-roger