Skip to contentSkip to conversation

usb: dwc3: don't reset device side if dwc3 was configured as host-only

PATCH In review Jisheng Zhang 3 participants 3 messages Jun 27, 2023 – Sep 30, 2023

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

PATCH 1/1 usb: dwc3: don't reset device side if dwc3 was configured as host-only

Commit c4a5153e87fd ("usb: dwc3: core: Power-off core/PHYs on system_suspend in host mode") replaces check for HOST only dr_mode with current_dr_role. But during booting, the current_dr_role isn't initialized, thus the device side reset is always issued even if dwc3 was configured as host-only. What's more, on some platforms with host only dwc3, aways issuing device side reset by accessing device register block can cause kernel panic.

Fixes c4a5153e87fd ("usb: dwc3: core: Power-off core/PHYs on system_suspend in host mode")
---
 drivers/usb/dwc3/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/dwc3/core.c +2 -2
index 7b2ce013cc5b..16d7a1d1cbfa 100644
@@ -277,9 +277,9 @@ int dwc3_core_soft_reset(struct dwc3 *dwc)
 	/*
 	 * We're resetting only the device side because, if we're in host mode,
 	 * XHCI driver will reset the host block. If dwc3 was configured for
-	 * host-only mode, then we can return early.
+	 * host-only mode or current role is host, then we can return early.
 	 */
-	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST)
+	if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST)
 		return 0;
 
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
Signature · 1 line
2.40.1
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 8 lines · Jisheng Zhang

Commit c4a5153e87fd ("usb: dwc3: core: Power-off core/PHYs on system_suspend in host mode") replaces check for HOST only dr_mode with current_dr_role. But during booting, the current_dr_role isn't initialized, thus the device side reset is always issued even if dwc3 was configured as host-only. What's more, on some platforms with host only dwc3, aways issuing device side reset by accessing device register block can cause kernel panic.

Ah... good catch!

This should be backported to stable.

So perhaps you or Greg can add Cc: [email protected]

Quoted 25 lines
Fixes c4a5153e87fd ("usb: dwc3: core: Power-off core/PHYs on system_suspend in host mode")
---
 drivers/usb/dwc3/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/dwc3/core.c +2 -2
index 7b2ce013cc5b..16d7a1d1cbfa 100644
@@ -277,9 +277,9 @@ int dwc3_core_soft_reset(struct dwc3 *dwc)
 	/*
 	 * We're resetting only the device side because, if we're in host mode,
 	 * XHCI driver will reset the host block. If dwc3 was configured for
-	 * host-only mode, then we can return early.
+	 * host-only mode or current role is host, then we can return early.
 	 */
-	if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST)
+	if (dwc->dr_mode == USB_DR_MODE_HOST || dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST)
 		return 0;
 
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
Signature · 1 line
2.40.1

Thanks,
Thinh

#2 lore raw
In reply to Jisheng Zhang

It seems that this patch causes driver failed to initialize on rockchip rk3399 devices when DWC3 controller is configured as dual role.

[    2.827119] dwc3 fe900000.usb: error -ETIMEDOUT: failed to initialize core
[    2.827881] dwc3: probe of fe900000.usb failed with error -110

After some tests I am preliminarily certain that this patch caused the failure.