Skip to contentSkip to conversation

usb: dwc3: Trigger a GCTL soft reset when switching modes in DRD

PATCH Superseded John Stultz 1 participant 2 messages Oct 22, 2020

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

PATCH v3 1/2 usb: dwc3: Trigger a GCTL soft reset when switching modes in DRD

From: Yu Chen <[email protected]>

With the current dwc3 code on the HiKey960 we often see the COREIDLE flag get stuck off in __dwc3_gadget_start(), which seems to prevent the reset irq and causes the USB gadget to fail to initialize.

We had seen occasional initialization failures with older kernels but with recent 5.x era kernels it seemed to be becoming much more common, so I dug back through some older trees and realized I dropped this quirk from Yu Chen during upstreaming as I couldn't provide a proper rational for it and it didn't seem to be necessary. I now realize I was wrong.

After resubmitting the quirk, Thinh Nguyen pointed out that it shouldn't be a quirk at all and it is actually mentioned in the programming guide that it should be done when switching modes in DRD.

So, to avoid these !COREIDLE lockups seen on HiKey960, this patch issues GCTL soft reset when switching modes if the controller is in DRD mode.

Cc Felipe Balbi <[email protected]>
Cc Tejas Joglekar <[email protected]>
Cc Yang Fei <[email protected]>
Cc YongQin Liu <[email protected]>
Cc Andrzej Pietrasiewicz <[email protected]>
Cc Thinh Nguyen <[email protected]>
Cc Jun Li <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>

---
v2:
* Rework to always call the GCTL soft reset in DRD mode, rather then using a quirk as suggested by Thinh Nguyen

v3:
* Move GCTL soft reset under the spinlock as suggested by Thinh Nguyen

---
 drivers/usb/dwc3/core.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
drivers/usb/dwc3/core.c +19 -0
index bdf0925da6b6..a2a88284a95b 100644
@@ -114,10 +114,24 @@ void dwc3_set_prtcap(struct dwc3 *dwc, u32 mode)
 	dwc->current_dr_role = mode;
 }
 
+static void dwc3_gctl_core_soft_reset(struct dwc3 *dwc)
+{
+	int reg;
+
+	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+	reg |= (DWC3_GCTL_CORESOFTRESET);
+	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+
+	reg = dwc3_readl(dwc->regs, DWC3_GCTL);
+	reg &= ~(DWC3_GCTL_CORESOFTRESET);
+	dwc3_writel(dwc->regs, DWC3_GCTL, reg);
+}
+
 static void __dwc3_set_mode(struct work_struct *work)
 {
 	struct dwc3 *dwc = work_to_dwc(work);
 	unsigned long flags;
+	int hw_mode;
 	int ret;
 	u32 reg;
 
@@ -156,6 +170,11 @@ static void __dwc3_set_mode(struct work_struct *work)
 
 	spin_lock_irqsave(&dwc->lock, flags);
 
+	/* Execute a GCTL Core Soft Reset when switch mode in DRD*/
+	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD)
+		dwc3_gctl_core_soft_reset(dwc);
+
 	dwc3_set_prtcap(dwc, dwc->desired_dr_role);
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
Signature · 1 line
2.17.1
#1 lore raw

PATCH v3 2/2 usb: dwc3: Fix DRD mode change sequence following programming guide

In reviewing the previous patch, Thinh Nguyen pointed out that the DRD mode change sequence should be like the following when switching from host -> device according to the programming guide (for all DRD IPs):
1. Reset controller with GCTL.CoreSoftReset
2. Set GCTL.PrtCapDir(device)
3. Soft reset with DCTL.CSftRst
4. Then follow up with the initializing registers sequence

The current code does:
a. Soft reset with DCTL.CSftRst on driver probe
b. Reset controller with GCTL.CoreSoftReset (added in previous
patch)
c. Set GCTL.PrtCapDir(device)
d. < missing DCTL.CSftRst >
e. Then follow up with initializing registers sequence

So this patch adds the DCTL.CSftRst soft reset that was currently missing from the dwc3 mode switching.

Cc Felipe Balbi <[email protected]>
Cc Tejas Joglekar <[email protected]>
Cc Yang Fei <[email protected]>
Cc YongQin Liu <[email protected]>
Cc Andrzej Pietrasiewicz <[email protected]>
Cc Thinh Nguyen <[email protected]>
Cc Jun Li <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>

---
Feedback would be appreciated. I'm a little worried I should be conditionalizing the DCTL.CSftRst on DRD mode controllers, but I'm really not sure what the right thing to do is for non-DRD mode controllers.

---
 drivers/usb/dwc3/core.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/dwc3/core.c +3 -0
index a2a88284a95b..c87d8add19e4 100644
@@ -40,6 +40,8 @@
 
 #define DWC3_DEFAULT_AUTOSUSPEND_DELAY	5000 /* ms */
 
+static int dwc3_core_soft_reset(struct dwc3 *dwc);
+
 /**
  * dwc3_get_dr_mode - Validates and sets dr_mode
  * @dwc: pointer to our context structure
@@ -177,6 +179,7 @@ static void __dwc3_set_mode(struct work_struct *work)
 
 	dwc3_set_prtcap(dwc, dwc->desired_dr_role);
 
+	dwc3_core_soft_reset(dwc);
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	switch (dwc->desired_dr_role) {
Signature · 1 line
2.17.1