Hi Roger,
On Tue, Oct 08, 2024, Roger Quadros wrote:
Hi Thinh,
On 05/10/2024 04:04, Thinh Nguyen wrote:
Hi,
On Tue, Oct 01, 2024, Roger Quadros wrote:
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.
Cc: stable@vger.kernel.org # v6.9+ Fixes: 6d735722063a ("usb: dwc3: core: Prevent phy suspend during init") Link: https://urldefense.com/v3/__https://lore.kernel.org/all/1519dbe7-73b6-4afc-b... Signed-off-by: Roger Quadros rogerq@kernel.org
drivers/usb/dwc3/core.c | 16 ++++++++++++++++ drivers/usb/dwc3/core.h | 2 ++ 2 files changed, 18 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 9eb085f359ce..1233922d4d54 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -2336,6 +2336,9 @@ 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);
- switch (dwc->current_dr_role) { case DWC3_GCTL_PRTCAP_DEVICE: if (pm_runtime_suspended(dwc->dev))
@@ -2387,6 +2390,11 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg) break; }
- if (!PMSG_IS_AUTO(msg)) {
if (!dwc->susphy_state)
dwc3_enable_susphy(dwc, true);
- }
- return 0;
} @@ -2454,6 +2462,14 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg) break; }
- if (!PMSG_IS_AUTO(msg)) {
/* dwc3_core_init_for_resume() disables SUSPHY so just handle
* the enable case
*/
Can we note that this is a particular behavior needed for AM62 here? And can we use this comment style:
Looking at this again, this fix is not specific to AM62 but for all platforms. e.g. if Host Role was already started when going to system suspend, SUSPHY bits were enabled, then after system resume SUSPHY bits are cleared at dwc3_core_init_for_resume().
Host stop/start was not called so SUSPHY bits remain cleared. So here we deal with enabling SUSPHY.
It's true that we have a bug where the SUSPHY bits remain disabled after suspend. However, the SUSPHY bits needing to be set during suspend is unique to AM62. Let's add this note in the dwc3_suspend_common() check.
Thanks, Thinh