[RFT PATCH v4 0/3] usb: dwc2: Fix core reset and force mode delays

[RFT PATCH v4 0/3] usb: dwc2: Fix core reset and force mode delays

[John Youn]

This series accounts for the delay from the IDDIG debounce filter when
switching modes. This delay is a function of the PHY clock speed and
can range from 5-50 ms. This delay must be taken into account on core
reset and force modes. A full explanation is provided in the patch
commit log and code comments.

This revision of the series increases the IDDIG delay to 100 ms. Some
rockchip platforms seem to timeout even with 50 ms so I have doubled
this.

Appreciate any testing on RK3188 and RPi platforms.

v4:
* Increased the IDDIG delay to 110ms.
* Removed tested-by for patch 2 since I have changed the delays.

v3:
* Added tested-bys for patch 1-2
* Fixed an issue where a function was not returning a value
* Dropped patch 4

v2:
* Broke up the last patch of the original series

Regards,
John

John Youn (3):
  usb: dwc2: gadget: Only initialize device if in device mode
  usb: dwc2: Add delay to core soft reset
  usb: dwc2: Properly account for the force mode delays

 drivers/usb/dwc2/core.c   | 126 +++++++++++++++++++++++++++++++++++++++-------
 drivers/usb/dwc2/core.h   |   1 +
 drivers/usb/dwc2/gadget.c |   7 ++-
 drivers/usb/dwc2/hw.h     |   1 +
 4 files changed, 116 insertions(+), 19 deletions(-)

-- 
2.9.0

[RFT PATCH v4 1/3] usb: dwc2: gadget: Only initialize device if in device mode

[John Youn]

In dwc2_hsotg_udc_start(), don't initialize the controller for device
mode unless we are actually in device mode.

Tested-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: John Youn <johnyoun@synopsys.com>
---
 drivers/usb/dwc2/gadget.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 94bd19a..4cd6403 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3466,8 +3466,11 @@ static int dwc2_hsotg_udc_start(struct usb_gadget *gadget,
 		otg_set_peripheral(hsotg->uphy->otg, &hsotg->gadget);
 
 	spin_lock_irqsave(&hsotg->lock, flags);
-	dwc2_hsotg_init(hsotg);
-	dwc2_hsotg_core_init_disconnected(hsotg, false);
+	if (dwc2_hw_is_device(hsotg)) {
+		dwc2_hsotg_init(hsotg);
+		dwc2_hsotg_core_init_disconnected(hsotg, false);
+	}
+
 	hsotg->enabled = 0;
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-- 
2.9.0

[RFT PATCH v4 2/3] usb: dwc2: Add delay to core soft reset

[John Youn]

Add a delay to the core soft reset function to account for the IDDIG
debounce filter.

If the current mode is host, either due to the force mode bit being
set (which persists after core reset) or the connector id pin, a core
soft reset will temporarily reset the mode to device and a delay from
the IDDIG debounce filter will occur before going back to host mode.

Signed-off-by: John Youn <johnyoun@synopsys.com>
---
 drivers/usb/dwc2/core.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/dwc2/core.h |  1 +
 drivers/usb/dwc2/hw.h   |  1 +
 3 files changed, 97 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 4135a5f..a3068e0 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -238,6 +238,77 @@ int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg)
 	return ret;
 }
 
+/**
+ * dwc2_wait_for_mode() - Waits for the controller mode.
+ * @hsotg:	Programming view of the DWC_otg controller.
+ * @host_mode:	If true, waits for host mode, otherwise device mode.
+ */
+static void dwc2_wait_for_mode(struct dwc2_hsotg *hsotg,
+			       bool host_mode)
+{
+	ktime_t start;
+	ktime_t end;
+	unsigned int timeout = 110;
+
+	dev_vdbg(hsotg->dev, "Waiting for %s mode\n",
+		 host_mode ? "host" : "device");
+
+	start = ktime_get();
+
+	while (1) {
+		s64 ms;
+
+		if (dwc2_is_host_mode(hsotg) == host_mode) {
+			dev_vdbg(hsotg->dev, "%s mode set\n",
+				 host_mode ? "Host" : "Device");
+			break;
+		}
+
+		end = ktime_get();
+		ms = ktime_to_ms(ktime_sub(end, start));
+
+		if (ms >= (s64)timeout) {
+			dev_warn(hsotg->dev, "%s: Couldn't set %s mode\n",
+				 __func__, host_mode ? "host" : "device");
+			break;
+		}
+
+		usleep_range(1000, 2000);
+	}
+}
+
+/**
+ * dwc2_iddig_filter_enabled() - Returns true if the IDDIG debounce
+ * filter is enabled.
+ */
+static bool dwc2_iddig_filter_enabled(struct dwc2_hsotg *hsotg)
+{
+	u32 gsnpsid;
+	u32 ghwcfg4;
+
+	if (!dwc2_hw_is_otg(hsotg))
+		return false;
+
+	/* Check if core configuration includes the IDDIG filter. */
+	ghwcfg4 = dwc2_readl(hsotg->regs + GHWCFG4);
+	if (!(ghwcfg4 & GHWCFG4_IDDIG_FILT_EN))
+		return false;
+
+	/*
+	 * Check if the IDDIG debounce filter is bypassed. Available
+	 * in core version >= 3.10a.
+	 */
+	gsnpsid = dwc2_readl(hsotg->regs + GSNPSID);
+	if (gsnpsid >= DWC2_CORE_REV_3_10a) {
+		u32 gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
+
+		if (gotgctl & GOTGCTL_DBNCE_FLTR_BYPASS)
+			return false;
+	}
+
+	return true;
+}
+
 /*
  * Do core a soft reset of the core.  Be careful with this because it
  * resets all the internal state machines of the core.
@@ -246,9 +317,30 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 {
 	u32 greset;
 	int count = 0;
+	bool wait_for_host_mode = false;
 
 	dev_vdbg(hsotg->dev, "%s()\n", __func__);
 
+	/*
+	 * If the current mode is host, either due to the force mode
+	 * bit being set (which persists after core reset) or the
+	 * connector id pin, a core soft reset will temporarily reset
+	 * the mode to device. A delay from the IDDIG debounce filter
+	 * will occur before going back to host mode.
+	 *
+	 * Determine whether we will go back into host mode after a
+	 * reset and account for this delay after the reset.
+	 */
+	if (dwc2_iddig_filter_enabled(hsotg)) {
+		u32 gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
+		u32 gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
+
+		if (!(gotgctl & GOTGCTL_CONID_B) ||
+		    (gusbcfg & GUSBCFG_FORCEHOSTMODE)) {
+			wait_for_host_mode = true;
+		}
+	}
+
 	/* Core Soft Reset */
 	greset = dwc2_readl(hsotg->regs + GRSTCTL);
 	greset |= GRSTCTL_CSFTRST;
@@ -277,6 +369,9 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 		}
 	} while (!(greset & GRSTCTL_AHBIDLE));
 
+	if (wait_for_host_mode)
+		dwc2_wait_for_mode(hsotg, true);
+
 	return 0;
 }
 
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 78526ee..466c220 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -882,6 +882,7 @@ struct dwc2_hsotg {
 #define DWC2_CORE_REV_2_92a	0x4f54292a
 #define DWC2_CORE_REV_2_94a	0x4f54294a
 #define DWC2_CORE_REV_3_00a	0x4f54300a
+#define DWC2_CORE_REV_3_10a	0x4f54310a
 
 #if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
 	union dwc2_hcd_internal_flags {
diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index efc3bcd..9105844 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -48,6 +48,7 @@
 #define GOTGCTL_ASESVLD			(1 << 18)
 #define GOTGCTL_DBNC_SHORT		(1 << 17)
 #define GOTGCTL_CONID_B			(1 << 16)
+#define GOTGCTL_DBNCE_FLTR_BYPASS	(1 << 15)
 #define GOTGCTL_DEVHNPEN		(1 << 11)
 #define GOTGCTL_HSTSETHNPEN		(1 << 10)
 #define GOTGCTL_HNPREQ			(1 << 9)
-- 
2.9.0

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

[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.

Signed-off-by: John Youn <johnyoun@synopsys.com>
---
 drivers/usb/dwc2/core.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index a3068e0..fa9b26b 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -395,9 +395,9 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
  * Checks are done in this function to determine whether doing a force
  * would be valid or not.
  *
- * If a force is done, it requires a 25ms delay to take effect.
- *
- * Returns true if the mode was forced.
+ * If a force is done, it requires a IDDIG debounce filter delay if
+ * the filter is configured and enabled. We poll the current mode of
+ * the controller to account for this delay.
  */
 static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host)
 {
@@ -432,12 +432,18 @@ static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host)
 	gusbcfg |= set;
 	dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
 
-	msleep(25);
+	dwc2_wait_for_mode(hsotg, host);
 	return true;
 }
 
-/*
- * Clears the force mode bits.
+/**
+ * dwc2_clear_force_mode() - Clears the force mode bits.
+ *
+ * After clearing the bits, wait up to 100 ms to account for any
+ * potential IDDIG filter delay. We can't know if we expect this delay
+ * or not because the value of the connector ID status is affected by
+ * the force mode. We only need to call this once during probe if
+ * dr_mode == OTG.
  */
 static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
 {
@@ -448,11 +454,8 @@ static void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg)
 	gusbcfg &= ~GUSBCFG_FORCEDEVMODE;
 	dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
 
-	/*
-	 * NOTE: This long sleep is _very_ important, otherwise the core will
-	 * not stay in host mode after a connector ID change!
-	 */
-	msleep(25);
+	if (dwc2_iddig_filter_enabled(hsotg))
+		usleep_range(100000, 110000);
 }
 
 /*
@@ -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);
 }
 
 /*
-- 
2.9.0

Re: [RFT PATCH v4 0/3] usb: dwc2: Fix core reset and force mode delays

[Stefan Wahren]

Hi John,

> John Youn <johnyoun@synopsys.com> hat am 1. September 2016 um 23:07
> geschrieben:
> 
> 
> This series accounts for the delay from the IDDIG debounce filter when
> switching modes. This delay is a function of the PHY clock speed and
> can range from 5-50 ms. This delay must be taken into account on core
> reset and force modes. A full explanation is provided in the patch
> commit log and code comments.
> 
> This revision of the series increases the IDDIG delay to 100 ms. Some
> rockchip platforms seem to timeout even with 50 ms so I have doubled
> this.
> 
> Appreciate any testing on RK3188 and RPi platforms.

i tested the whole series successful with a Raspberry Pi B in dr_mode "host" and
"otg"

Tested-by: Stefan Wahren <stefan.wahren@i2se.com>

Re: [RFT PATCH v4 0/3] usb: dwc2: Fix core reset and force mode delays

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]


Hi,

Stefan Wahren <stefan.wahren@i2se.com> writes:
> Hi John,
>
>> John Youn <johnyoun@synopsys.com> hat am 1. September 2016 um 23:07
>> geschrieben:
>> 
>> 
>> This series accounts for the delay from the IDDIG debounce filter when
>> switching modes. This delay is a function of the PHY clock speed and
>> can range from 5-50 ms. This delay must be taken into account on core
>> reset and force modes. A full explanation is provided in the patch
>> commit log and code comments.
>> 
>> This revision of the series increases the IDDIG delay to 100 ms. Some
>> rockchip platforms seem to timeout even with 50 ms so I have doubled
>> this.
>> 
>> Appreciate any testing on RK3188 and RPi platforms.
>
> i tested the whole series successful with a Raspberry Pi B in dr_mode "host" and
> "otg"
>
> Tested-by: Stefan Wahren <stefan.wahren@i2se.com>

John, when you're happy with the series, please resend without RFT so I
can apply :-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]