[PATCH v5 3/3] usb: dwc2: Properly account for the force mode delays

[Heiko Stübner]

Am Dienstag, 13. September 2016, 12:04:12 schrieb John Youn:
> On 9/13/2016 11:39 AM, Heiko Stübner wrote:
> > Am Dienstag, 13. September 2016, 20:07:25 schrieb Stefan Wahren:
> >> Hi Heiko,
> >> 
> >>> Heiko Stuebner <heiko at sntech.de> hat am 12. September 2016 um 13:05
> >>> geschrieben:
> >>> 
> >>> 
> >>> Hi Stefan,
> >>> 
> >>> Am Montag, 12. September 2016, 07:20:44 CEST schrieb Stefan Wahren:
> >>>>> Heiko Stuebner <heiko at sntech.de> hat am 11. September 2016 um 23:19
> >>>>> geschrieben:
> >>>>> 
> >>>>> Am Mittwoch, 7. September 2016, 19:39:43 CEST schrieb John Youn:
> >>>>>> When a force mode bit is set and the IDDIG debounce filter is
> >>>>>> enabled,
> >>>>>> there is a delay for the forced mode to take effect. This delay is
> >>>>>> due
> >>>>>> to the IDDIG debounce filter and is variable depending on the
> >>>>>> platform's
> >>>>>> PHY clock speed. To account for this delay we can poll for the
> >>>>>> expected
> >>>>>> mode.
> >>>>>> 
> >>>>>> On a clear force mode, since we don't know what mode to poll for,
> >>>>>> delay
> >>>>>> for a fixed 100 ms. This is the maximum delay based on the slowest
> >>>>>> PHY
> >>>>>> clock speed.
> >>>>>> 
> >>>>>> Tested-by: Stefan Wahren <stefan.wahren at i2se.com>
> >>>>>> Signed-off-by: John Youn <johnyoun at synopsys.com>
> >>>>>> ---
> >>>>> 
> >>>>> [...]
> >>>>> 
> >>>>>> @@ -475,12 +478,6 @@ void dwc2_force_dr_mode(struct dwc2_hsotg
> >>>>>> *hsotg)
> >>>>>> 
> >>>>>>  			 __func__, hsotg->dr_mode);
> >>>>>>  		
> >>>>>>  		break;
> >>>>>>  	
> >>>>>>  	}
> >>>>>> 
> >>>>>> -
> >>>>>> -	/*
> >>>>>> -	 * NOTE: This is required for some rockchip soc based
> >>>>>> -	 * platforms.
> >>>>>> -	 */
> >>>>>> -	msleep(50);
> >>>>>> 
> >>>>>>  }
> >>>>> 
> >>>>> sorry for not finding the time to test your subsequent versions, but
> >>>>> this
> >>>>> still
> >>>>> acts up on my Rockchip boards, as I'm still running into errors like
> >>>>> 
> >>>>> 	[    4.875570] usb usb2-port1: connect-debounce failed
> >>>> 
> >>>> could you please name the relevant DTS file of the affected boards?
> >>> 
> >>> So far I've been able to see that on
> >>> 
> >>> rk3188-radxarock
> >>> rk3036-kylin
> >>> 
> >>> both on host-only dwc2 controllers.
> >> 
> >> thanks, does patch 1 & 2 already have a negative effect on these
> >> controllers?
> > 
> > nope, patches 1+2 alone are ok and only the msleep(50) going away causes
> > problems. I'm back home now, so I'll hopefully have time to check
> > host-only
> > vs. otg dwc2 controllers tomorrow as well.
> 
> Ok, thanks for testing Heiko.
> 
> Given that, let's try to fix this in the next -rc (or before) because
> I think the Raspberry Pi needs the other parts of that last patch. We
> can just revert the msleep(50) if needed.

I could nicely confirm, that only the host-only controller seems affected by
this. Same kernel image that fails on the host-only one with the "connect-
debounce failed" message seems to work once I switch that over to the  otg
controller and the system comes up nicely again.

So if we don't find the root cause in the short term, maybe we could at least
limit the delay for host-only variants, like

---------------------- 8< -------------------
diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index fa9b26b..4c0fa0b 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -463,9 +463,18 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
  */
 void dwc2_force_dr_mode(struct dwc2_hsotg *hsotg)
 {
+	bool ret;
+
 	switch (hsotg->dr_mode) {
 	case USB_DR_MODE_HOST:
-		dwc2_force_mode(hsotg, true);
+		ret = dwc2_force_mode(hsotg, true);
+		/*
+		 * NOTE: This is required for some rockchip soc based
+		 * platforms on their host-only dwc2.
+		 */
+		if (!ret)
+			msleep(50);
+
 		break;
 	case USB_DR_MODE_PERIPHERAL:
 		dwc2_force_mode(hsotg, false);
---------------------- 8< -------------------

I've been running with that change for some time now on both the
rk3036-kylin and rk3188-radxarock. Plugged and unplugged different
combinations of usb-devices on the usb otg and host ports, so far
seems to work.


Heiko