[PATCH 0/5] usb: dwc2: fix dwc2_get_hwparams() + optimize probe time a bit

[PATCH 0/5] usb: dwc2: fix dwc2_get_hwparams() + optimize probe time a bit

[Douglas Anderson]

This is a collection of patches, some by Yunzhi Li at Rockchip and some
by me, that fix dwc2_get_hwparams() on some boards (depending on how the
bootloader left things) and then attempt to optimize DWC2's probe
time (since fixing dwc2_get_hwparams() made probe even slower than it
was).  Note that even after these changes probe time is still not super
fast (we still call dwc2_core_reset() twice per probe and each call
still takes a minimum of 160ms right now), but this at least picks the
low hanging fruit.

Note that this picks patch "v2" of "usb: dwc2: reset dwc2 core before
dwc2_get_hwparams()" from Yunzhi Li rather than v3 because I think it's
a better way to go.

These patches are all rebased atop linuxnext.  Since linuxnext wasn't
booting on my board, they were tested against Heiko Stuebner's "somewhat
stable" github tree (based on v4.3-rc1) with a few linuxnext dwc2
patches pulled in to avoid conflicts.

These patches have either landed or are planned to land on the
chromeos-3.14 branch for use in several Chromebooks that use rk3288.


Douglas Anderson (3):
  usb: dwc2: Restore GUSBCFG in dwc2_get_hwparams()
  CHROMIUM: usb: dwc2: Avoid double-reset at boot time
  usb: dwc2: Speed dwc2_get_hwparams() on some host-only ports

Yunzhi Li (2):
  usb: dwc2: reset dwc2 core before dwc2_get_hwparams()
  usb: dwc2: reduce dwc2 driver probe time

 drivers/usb/dwc2/core.c     | 57 ++++++++++++++++++++++++++-------------------
 drivers/usb/dwc2/core.h     |  3 ++-
 drivers/usb/dwc2/hcd.c      |  6 ++---
 drivers/usb/dwc2/platform.c |  6 +++++
 4 files changed, 44 insertions(+), 28 deletions(-)

-- 
2.6.0.rc2.230.g3dd15c0

[PATCH 1/5] usb: dwc2: Restore GUSBCFG in dwc2_get_hwparams()

[Douglas Anderson]

Previously dwc2_get_hwparams() was changing GUSBCFG and not putting it
back the way it was (specifically it set and cleared FORCEHOSTMODE).
Since we want to move dwc2_core_reset() _before_ dwc2_get_hwparams() we
should make sure dwc2_get_hwparams() isn't messing with things in a
permanent way.

Since we're now looking at GUSBCFG, it's obvious that we shouldn't need
all the extra delays if FORCEHOSTMODE was already set.  This will avoid
some delays for any ports that have forced host mode.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/usb/dwc2/core.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index bf5e951..43c8bf4 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -3097,18 +3097,20 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 
 	/* Force host mode to get HPTXFSIZ / GNPTXFSIZ exact power on value */
 	gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
-	gusbcfg |= GUSBCFG_FORCEHOSTMODE;
-	dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
-	usleep_range(100000, 150000);
+	if (!(gusbcfg & GUSBCFG_FORCEHOSTMODE)) {
+		dwc2_writel(gusbcfg | GUSBCFG_FORCEHOSTMODE,
+			    hsotg->regs + GUSBCFG);
+		usleep_range(100000, 150000);
+	}
 
 	gnptxfsiz = dwc2_readl(hsotg->regs + GNPTXFSIZ);
 	hptxfsiz = dwc2_readl(hsotg->regs + HPTXFSIZ);
 	dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz);
 	dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz);
-	gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
-	gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
-	dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
-	usleep_range(100000, 150000);
+	if (!(gusbcfg & GUSBCFG_FORCEHOSTMODE)) {
+		dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
+		usleep_range(100000, 150000);
+	}
 
 	/* hwcfg2 */
 	hw->op_mode = (hwcfg2 & GHWCFG2_OP_MODE_MASK) >>
-- 
2.6.0.rc2.230.g3dd15c0

[PATCH 2/5] usb: dwc2: reset dwc2 core before dwc2_get_hwparams()

[Douglas Anderson]

From: Yunzhi Li <lyz@rock-chips.com>

We initiate dwc2 usb controller in BIOS, dwc2_core_reset() should
be called before dwc2_get_hwparams() to reset core registers to
default value. Without this the FIFO setting might be incorrect
because calculating FIFO size need power-on value of
GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers.

This patch could avoid warnning massage like in rk3288 platform:
[    2.074764] dwc2 ff580000.usb: 256 invalid for
host_perio_tx_fifo_size. Check HW configuration.

Signed-off-by: Yunzhi Li <lyz@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/usb/dwc2/core.c     | 2 +-
 drivers/usb/dwc2/core.h     | 1 +
 drivers/usb/dwc2/platform.c | 6 ++++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 43c8bf4..f2002d2 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -481,7 +481,7 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg)
  * Do core a soft reset of the core.  Be careful with this because it
  * resets all the internal state machines of the core.
  */
-static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
+int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 {
 	u32 greset;
 	int count = 0;
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index ebf2504..c258d19 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -862,6 +862,7 @@ enum dwc2_halt_status {
  * The following functions support initialization of the core driver component
  * and the DWC_otg controller
  */
+extern int dwc2_core_reset(struct dwc2_hsotg *hsotg);
 extern void dwc2_core_host_init(struct dwc2_hsotg *hsotg);
 extern int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg);
 extern int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, bool restore);
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index b920e43..d27521c 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -254,6 +254,12 @@ static int dwc2_driver_probe(struct platform_device *dev)
 	spin_lock_init(&hsotg->lock);
 	mutex_init(&hsotg->init_mutex);
 
+	/*
+	 * Reset before dwc2_get_hwparams() then it could get power-on real
+	 * reset value form registers.
+	 */
+	dwc2_core_reset(hsotg);
+
 	/* Detect config values from hardware */
 	retval = dwc2_get_hwparams(hsotg);
 	if (retval)
-- 
2.6.0.rc2.230.g3dd15c0

[PATCH 3/5] CHROMIUM: usb: dwc2: Avoid double-reset at boot time

[Douglas Anderson]

In (usb: dwc2: reset dwc2 core before dwc2_get_hwparams()) we added an
extra reset to the probe path for the dwc2 USB controllers.  This
allowed proper detection of parameters even if the firmware had already
used the USB part.

Unfortunately, this extra reset is quite slow and is affecting boot
speed.  We can avoid the double-reset by skipping the extra reset that
would happen just after the one we added.  Logic that explains why this
is safe:

* As of the CL mentioned above, we now always call dwc2_core_reset() in
  dwc2_driver_probe() before dwc2_hcd_init().

* The only caller of dwc2_hcd_init() is dwc2_driver_probe(), so we're
  guaranteed that dwc2_core_reset() was called before dwc2_hdc_init().

* dwc2_hdc_init() is the only caller that passes an irq other than -1 to
  dwc2_core_init().  Thus if dwc2_core_init() is called with an irq
  other than -1 we're guaranteed that dwc2_core_reset was called before
  dwc2_core_init().

...this allows us to remove the dwc2_core_reset() in dwc2_core_init() if
irq is not < 0.

Note that since "irq" wasn't used in the function dwc2_core_init()
anyway and since select_phy was always set at exactly the same times we
could avoid the reset, we remove "irq" and rename "select_phy" to
"initial_setup" and adjust the callers accordingly.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/usb/dwc2/core.c | 29 ++++++++++++++++++-----------
 drivers/usb/dwc2/core.h |  2 +-
 drivers/usb/dwc2/hcd.c  |  6 +++---
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index f2002d2..9f1c438 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -765,11 +765,10 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
  * dwc2_core_init() - Initializes the DWC_otg controller registers and
  * prepares the core for device mode or host mode operation
  *
- * @hsotg:      Programming view of the DWC_otg controller
- * @select_phy: If true then also set the Phy type
- * @irq:        If >= 0, the irq to register
+ * @hsotg:         Programming view of the DWC_otg controller
+ * @initial_setup: If true then this is the first init for this instance.
  */
-int dwc2_core_init(struct dwc2_hsotg *hsotg, bool select_phy, int irq)
+int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup)
 {
 	u32 usbcfg, otgctl;
 	int retval;
@@ -791,18 +790,26 @@ int dwc2_core_init(struct dwc2_hsotg *hsotg, bool select_phy, int irq)
 
 	dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);
 
-	/* Reset the Controller */
-	retval = dwc2_core_reset(hsotg);
-	if (retval) {
-		dev_err(hsotg->dev, "%s(): Reset failed, aborting\n",
-				__func__);
-		return retval;
+	/*
+	 * Reset the Controller
+	 *
+	 * We only need to reset the controller if this is a re-init.
+	 * For the first init we know for sure that earlier code reset us (it
+	 * needed to in order to properly detect various parameters).
+	 */
+	if (!initial_setup) {
+		retval = dwc2_core_reset(hsotg);
+		if (retval) {
+			dev_err(hsotg->dev, "%s(): Reset failed, aborting\n",
+					__func__);
+			return retval;
+		}
 	}
 
 	/*
 	 * This needs to happen in FS mode before any other programming occurs
 	 */
-	retval = dwc2_phy_init(hsotg, select_phy);
+	retval = dwc2_phy_init(hsotg, initial_setup);
 	if (retval)
 		return retval;
 
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index c258d19..d707a7b 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -900,7 +900,7 @@ extern void dwc2_read_packet(struct dwc2_hsotg *hsotg, u8 *dest, u16 bytes);
 extern void dwc2_flush_tx_fifo(struct dwc2_hsotg *hsotg, const int num);
 extern void dwc2_flush_rx_fifo(struct dwc2_hsotg *hsotg);
 
-extern int dwc2_core_init(struct dwc2_hsotg *hsotg, bool select_phy, int irq);
+extern int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup);
 extern void dwc2_enable_global_interrupts(struct dwc2_hsotg *hcd);
 extern void dwc2_disable_global_interrupts(struct dwc2_hsotg *hcd);
 
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index af4e4a2..e2286cf 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1381,7 +1381,7 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
 			dev_err(hsotg->dev,
 				"Connection id status change timed out\n");
 		hsotg->op_state = OTG_STATE_B_PERIPHERAL;
-		dwc2_core_init(hsotg, false, -1);
+		dwc2_core_init(hsotg, false);
 		dwc2_enable_global_interrupts(hsotg);
 		spin_lock_irqsave(&hsotg->lock, flags);
 		dwc2_hsotg_core_init_disconnected(hsotg, false);
@@ -1404,7 +1404,7 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
 		hsotg->op_state = OTG_STATE_A_HOST;
 
 		/* Initialize the Core for Host mode */
-		dwc2_core_init(hsotg, false, -1);
+		dwc2_core_init(hsotg, false);
 		dwc2_enable_global_interrupts(hsotg);
 		dwc2_hcd_start(hsotg);
 	}
@@ -3050,7 +3050,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 	dwc2_disable_global_interrupts(hsotg);
 
 	/* Initialize the DWC_otg core, and select the Phy type */
-	retval = dwc2_core_init(hsotg, true, irq);
+	retval = dwc2_core_init(hsotg, true);
 	if (retval)
 		goto error2;
 
-- 
2.6.0.rc2.230.g3dd15c0

Re: [PATCH 3/5] CHROMIUM: usb: dwc2: Avoid double-reset at boot time

[Doug Anderson]

Hi,

On Wed, Oct 7, 2015 at 5:48 PM, Douglas Anderson <dianders@chromium.org> wrote:
> In (usb: dwc2: reset dwc2 core before dwc2_get_hwparams()) we added an
> extra reset to the probe path for the dwc2 USB controllers.  This
> allowed proper detection of parameters even if the firmware had already
> used the USB part.
>
> Unfortunately, this extra reset is quite slow and is affecting boot
> speed.  We can avoid the double-reset by skipping the extra reset that
> would happen just after the one we added.  Logic that explains why this
> is safe:
>
> * As of the CL mentioned above, we now always call dwc2_core_reset() in
>   dwc2_driver_probe() before dwc2_hcd_init().
>
> * The only caller of dwc2_hcd_init() is dwc2_driver_probe(), so we're
>   guaranteed that dwc2_core_reset() was called before dwc2_hdc_init().
>
> * dwc2_hdc_init() is the only caller that passes an irq other than -1 to
>   dwc2_core_init().  Thus if dwc2_core_init() is called with an irq
>   other than -1 we're guaranteed that dwc2_core_reset was called before
>   dwc2_core_init().
>
> ...this allows us to remove the dwc2_core_reset() in dwc2_core_init() if
> irq is not < 0.
>
> Note that since "irq" wasn't used in the function dwc2_core_init()
> anyway and since select_phy was always set at exactly the same times we
> could avoid the reset, we remove "irq" and rename "select_phy" to
> "initial_setup" and adjust the callers accordingly.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>  drivers/usb/dwc2/core.c | 29 ++++++++++++++++++-----------
>  drivers/usb/dwc2/core.h |  2 +-
>  drivers/usb/dwc2/hcd.c  |  6 +++---
>  3 files changed, 22 insertions(+), 15 deletions(-)

Obviously I stupidly forgot to remove the "CHROMIUM" prefix on this
patch.  :(  Sorry about that.  I can repost if necessary...

-Doug

[PATCH 4/5] usb: dwc2: Speed dwc2_get_hwparams() on some host-only ports

[Douglas Anderson]

On some host-only DWC2 ports (like the one in rk3288) when we set
GUSBCFG_FORCEHOSTMODE in GUSBCFG and then read back, we don't see the
bit set.  Presumably that's because the port is always forced to HOST
mode so there's no reason to implement these status bits.

Since we know dwc2_core_reset() is always called before
dwc2_get_hwparams() and we know dwc2_core_reset() should have set
GUSBCFG_FORCEHOSTMODE whenever hsotg->dr_mode == USB_DR_MODE_HOST, we
can just check hsotg->dr_mode to decide that we can skip the delays in
dwc2_get_hwparams().

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/usb/dwc2/core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 9f1c438..27ade0c 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -3070,7 +3070,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 	unsigned width;
 	u32 hwcfg1, hwcfg2, hwcfg3, hwcfg4;
 	u32 hptxfsiz, grxfsiz, gnptxfsiz;
-	u32 gusbcfg;
+	u32 gusbcfg = 0;
 
 	/*
 	 * Attempt to ensure this device is really a DWC_otg Controller.
@@ -3103,8 +3103,8 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 	dev_dbg(hsotg->dev, "grxfsiz=%08x\n", grxfsiz);
 
 	/* Force host mode to get HPTXFSIZ / GNPTXFSIZ exact power on value */
-	gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
-	if (!(gusbcfg & GUSBCFG_FORCEHOSTMODE)) {
+	if (hsotg->dr_mode != USB_DR_MODE_HOST) {
+		gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
 		dwc2_writel(gusbcfg | GUSBCFG_FORCEHOSTMODE,
 			    hsotg->regs + GUSBCFG);
 		usleep_range(100000, 150000);
@@ -3114,7 +3114,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 	hptxfsiz = dwc2_readl(hsotg->regs + HPTXFSIZ);
 	dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz);
 	dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz);
-	if (!(gusbcfg & GUSBCFG_FORCEHOSTMODE)) {
+	if (hsotg->dr_mode != USB_DR_MODE_HOST) {
 		dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
 		usleep_range(100000, 150000);
 	}
-- 
2.6.0.rc2.230.g3dd15c0

[PATCH 5/5] usb: dwc2: reduce dwc2 driver probe time

[Douglas Anderson]

From: Yunzhi Li <lyz@rock-chips.com>

I found that the probe function of dwc2 driver takes much time
when kernel boot up. There are many long delays in the probe
function these take almost 1 second.

This patch trying to reduce unnecessary delay time.

In dwc2_core_reset() I see it use two at least 20ms delays to
wait AHB idle and core soft reset, but dwc2 data book said that
dwc2 core soft reset and AHB idle just need a few clocks (I think
it refers to AHB clock, and AHB clock run at 150MHz in my RK3288
board), so 20ms is too long, delay 1us for wait AHB idle and soft
reset is enough.

And in dwc2_get_hwparams() it takes 150ms to wait ForceHostMode
and ForceDeviceMode valid but in data book it said software must
wait at least 25ms before the change to take effect, so I reduce
this time to 25ms~50ms. By the way, is there any state bit show
that the force mode take effect ? Could we poll curmod bit for
figuring out if the change take effect ?

It seems that usleep_range() at boot time will pick the longest
value in the range. In dwc2_core_reset() there is a very long
delay takes 200ms, and this function run twice when probe, could
any one tell me is this delay time resonable ?

I have tried this patch in my RK3288-evb board. It works well.

Signed-off-by: Yunzhi Li <lyz@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/usb/dwc2/core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 27ade0c..59fe48f 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -491,7 +491,7 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 
 	/* Wait for AHB master IDLE state */
 	do {
-		usleep_range(20000, 40000);
+		udelay(1);
 		greset = dwc2_readl(hsotg->regs + GRSTCTL);
 		if (++count > 50) {
 			dev_warn(hsotg->dev,
@@ -506,7 +506,7 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 	greset |= GRSTCTL_CSFTRST;
 	dwc2_writel(greset, hsotg->regs + GRSTCTL);
 	do {
-		usleep_range(20000, 40000);
+		udelay(1);
 		greset = dwc2_readl(hsotg->regs + GRSTCTL);
 		if (++count > 50) {
 			dev_warn(hsotg->dev,
@@ -537,7 +537,7 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 	 * NOTE: This long sleep is _very_ important, otherwise the core will
 	 * not stay in host mode after a connector ID change!
 	 */
-	usleep_range(150000, 200000);
+	usleep_range(150000, 160000);
 
 	return 0;
 }
@@ -3107,7 +3107,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 		gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
 		dwc2_writel(gusbcfg | GUSBCFG_FORCEHOSTMODE,
 			    hsotg->regs + GUSBCFG);
-		usleep_range(100000, 150000);
+		usleep_range(25000, 50000);
 	}
 
 	gnptxfsiz = dwc2_readl(hsotg->regs + GNPTXFSIZ);
@@ -3116,7 +3116,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 	dev_dbg(hsotg->dev, "hptxfsiz=%08x\n", hptxfsiz);
 	if (hsotg->dr_mode != USB_DR_MODE_HOST) {
 		dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
-		usleep_range(100000, 150000);
+		usleep_range(25000, 50000);
 	}
 
 	/* hwcfg2 */
-- 
2.6.0.rc2.230.g3dd15c0