2016-03-29 02:36:36

by John Youn

[permalink] [raw]
Subject: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems

Hi,

The following patch series addresses the core reset and force mode
delay problems we have been seeing on dwc2 for some platforms.

I think I have identified the source of the inconsistencies between
platforms and this series attempts to address them.

Basically everything stems from the IDDIG debounce filter delay, which
is a function of the PHY clock speed and can range from 5-50 ms if
enabled. 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.

The first two patches are prerequisites to the force mode fixes,
including one patch that was sent separately by Przemek Rudy. I have
resubmitted it with this series for convenience.

Please help by reviewing and testing on your platforms.

Patches were tested on:
* Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL

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

Przemek Rudy (1):
usb: dwc2: do not override forced dr_mode in gadget setup

drivers/usb/dwc2/core.c | 195 ++++++++++++++++++++++++++++----------------
drivers/usb/dwc2/core.h | 2 +-
drivers/usb/dwc2/gadget.c | 30 +++++--
drivers/usb/dwc2/hcd.c | 6 +-
drivers/usb/dwc2/hw.h | 1 +
drivers/usb/dwc2/platform.c | 9 +-
6 files changed, 161 insertions(+), 82 deletions(-)

--
2.7.4


2016-03-29 02:36:41

by John Youn

[permalink] [raw]
Subject: [RFT PATCH 2/4] usb: dwc2: gadget: Only initialize device if in device mode

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

Signed-off-by: John Youn <[email protected]>
---
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 818f158..181080b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3127,8 +3127,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.7.4

2016-03-29 02:36:52

by John Youn

[permalink] [raw]
Subject: [RFT PATCH 3/4] usb: dwc2: Add delay to core soft reset

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 <[email protected]>
---
drivers/usb/dwc2/core.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/hw.h | 1 +
3 files changed, 99 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 4135a5f..bb903e2 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -238,6 +238,78 @@ 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.
+ * @timeout: Time to wait in ms.
+ */
+static void dwc2_wait_for_mode(struct dwc2_hsotg *hsotg,
+ bool host_mode,
+ unsigned int timeout)
+{
+ ktime_t start;
+ ktime_t end;
+
+ 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 +318,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 +370,10 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
}
} while (!(greset & GRSTCTL_AHBIDLE));

+ /* Wait up to 50 ms for the IDDIG debounce filter. */
+ if (wait_for_host_mode)
+ dwc2_wait_for_mode(hsotg, true, 50);
+
return 0;
}

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 3c58d63..3a4a0d4 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -858,6 +858,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 281b57b..fad08b3 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.7.4

2016-03-29 02:36:48

by John Youn

[permalink] [raw]
Subject: [RFT PATCH 4/4] usb: dwc2: Properly account for the force mode delays

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 50 ms. This is the maximum delay based on the slowest PHY
clock speed.

Forcing the mode is needed during driver probe to get the host and
device specific HW parameters. It's also needed once more during probe
to set the controller in host or device mode if dr_mode is either
USB_DR_MODE_HOST or USB_DR_MODE_PERIPHERAL. And if dr_mode is
USB_DR_MODE_OTG, we clear the force mode bits.

The force mode bits should not be touched at any other time during the
driver lifetime and they should be preserved whenever the GUSBCFG
register is written to. The force mode bit values will presist across
soft resets of the core.

Given the above, we no longer need any other reset delays, force delays,
or any forced modes. So replace all calls to
dwc2_core_reset_and_force_dr_mode() with dwc2_core_reset() and remove
all other unnecessary delays.

Also remove the dwc2_force_mode_if_needed() function since the "if
needed" part is already taken care of by the polling in
dwc2_force_mode().

Finally, remove all other calls to dwc2_clear_force_mode().

Signed-off-by: John Youn <[email protected]>
---
drivers/usb/dwc2/core.c | 98 +++++++++++++--------------------------------
drivers/usb/dwc2/core.h | 1 -
drivers/usb/dwc2/hcd.c | 6 +--
drivers/usb/dwc2/platform.c | 9 ++++-
4 files changed, 39 insertions(+), 75 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index bb903e2..9b83b20 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -377,14 +377,14 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
return 0;
}

-/*
- * Force the mode of the controller.
+/**
+ * dwc2_force_mode() - Force the mode of the controller.
*
* Forcing the mode is needed for two cases:
*
* 1) If the dr_mode is set to either HOST or PERIPHERAL we force the
* controller to stay in a particular mode regardless of ID pin
- * changes. We do this usually after a core reset.
+ * changes. We do this once during probe.
*
* 2) During probe we want to read reset values of the hw
* configuration registers that are only available in either host or
@@ -397,11 +397,11 @@ 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)
+static void dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host)
{
u32 gusbcfg;
u32 set;
@@ -413,17 +413,17 @@ static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host)
* Force mode has no effect if the hardware is not OTG.
*/
if (!dwc2_hw_is_otg(hsotg))
- return false;
+ return;

/*
* If dr_mode is either peripheral or host only, there is no
* need to ever force the mode to the opposite mode.
*/
if (WARN_ON(host && hsotg->dr_mode == USB_DR_MODE_PERIPHERAL))
- return false;
+ return;

if (WARN_ON(!host && hsotg->dr_mode == USB_DR_MODE_HOST))
- return false;
+ return;

gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);

@@ -434,27 +434,34 @@ static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host)
gusbcfg |= set;
dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);

- msleep(25);
- return true;
+ dwc2_wait_for_mode(hsotg, host, 50);
}

-/*
- * Clears the force mode bits.
+/**
+ * dwc2_clear_force_mode() - Clears the force mode bits.
+ *
+ * After clearing the bits, wait up to 50 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)
{
u32 gusbcfg;

+ if (!dwc2_hw_is_otg(hsotg))
+ return;
+
+ dev_dbg(hsotg->dev, "Clearing force mode bits\n");
+
gusbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
gusbcfg &= ~GUSBCFG_FORCEHOSTMODE;
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))
+ msleep(50);
}

/*
@@ -477,31 +484,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);
-}
-
-/*
- * Do core a soft reset of the core. Be careful with this because it
- * resets all the internal state machines of the core.
- *
- * Additionally this will apply force mode as per the hsotg->dr_mode
- * parameter.
- */
-int dwc2_core_reset_and_force_dr_mode(struct dwc2_hsotg *hsotg)
-{
- int retval;
-
- retval = dwc2_core_reset(hsotg);
- if (retval)
- return retval;
-
- dwc2_force_dr_mode(hsotg);
- return 0;
}

/**
@@ -1423,22 +1405,6 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
}

/*
- * Forces either host or device mode if the controller is not
- * currently in that mode.
- *
- * Returns true if the mode was forced.
- */
-static bool dwc2_force_mode_if_needed(struct dwc2_hsotg *hsotg, bool host)
-{
- if (host && dwc2_is_host_mode(hsotg))
- return false;
- else if (!host && dwc2_is_device_mode(hsotg))
- return false;
-
- return dwc2_force_mode(hsotg, host);
-}
-
-/*
* Gets host hardware parameters. Forces host mode if not currently in
* host mode. Should be called immediately after a core soft reset in
* order to get the reset values.
@@ -1448,21 +1414,17 @@ static void dwc2_get_host_hwparams(struct dwc2_hsotg *hsotg)
struct dwc2_hw_params *hw = &hsotg->hw_params;
u32 gnptxfsiz;
u32 hptxfsiz;
- bool forced;

if (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)
return;

- forced = dwc2_force_mode_if_needed(hsotg, true);
+ dwc2_force_mode(hsotg, true);

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);

- if (forced)
- dwc2_clear_force_mode(hsotg);
-
hw->host_nperio_tx_fifo_size = (gnptxfsiz & FIFOSIZE_DEPTH_MASK) >>
FIFOSIZE_DEPTH_SHIFT;
hw->host_perio_tx_fifo_size = (hptxfsiz & FIFOSIZE_DEPTH_MASK) >>
@@ -1477,20 +1439,16 @@ static void dwc2_get_host_hwparams(struct dwc2_hsotg *hsotg)
static void dwc2_get_dev_hwparams(struct dwc2_hsotg *hsotg)
{
struct dwc2_hw_params *hw = &hsotg->hw_params;
- bool forced;
u32 gnptxfsiz;

if (hsotg->dr_mode == USB_DR_MODE_HOST)
return;

- forced = dwc2_force_mode_if_needed(hsotg, false);
+ dwc2_force_mode(hsotg, false);

gnptxfsiz = dwc2_readl(hsotg->regs + GNPTXFSIZ);
dev_dbg(hsotg->dev, "gnptxfsiz=%08x\n", gnptxfsiz);

- if (forced)
- dwc2_clear_force_mode(hsotg);
-
hw->dev_nperio_tx_fifo_size = (gnptxfsiz & FIFOSIZE_DEPTH_MASK) >>
FIFOSIZE_DEPTH_SHIFT;
}
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 3a4a0d4..53d276f 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -989,7 +989,6 @@ enum dwc2_halt_status {
* and the DWC_otg controller
*/
extern int dwc2_core_reset(struct dwc2_hsotg *hsotg);
-extern int dwc2_core_reset_and_force_dr_mode(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/hcd.c b/drivers/usb/dwc2/hcd.c
index 1f62551..9308415 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -134,7 +134,7 @@ static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);

/* Reset after a PHY select */
- retval = dwc2_core_reset_and_force_dr_mode(hsotg);
+ retval = dwc2_core_reset(hsotg);

if (retval) {
dev_err(hsotg->dev,
@@ -214,7 +214,7 @@ static int dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);

/* Reset after setting the PHY parameters */
- retval = dwc2_core_reset_and_force_dr_mode(hsotg);
+ retval = dwc2_core_reset(hsotg);
if (retval) {
dev_err(hsotg->dev,
"%s: Reset failed, aborting", __func__);
@@ -2204,7 +2204,7 @@ static int dwc2_core_init(struct dwc2_hsotg *hsotg, bool initial_setup)
* needed to in order to properly detect various parameters).
*/
if (!initial_setup) {
- retval = dwc2_core_reset_and_force_dr_mode(hsotg);
+ retval = dwc2_core_reset(hsotg);
if (retval) {
dev_err(hsotg->dev, "%s(): Reset failed, aborting\n",
__func__);
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 88629be..e882d3c 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -568,7 +568,9 @@ static int dwc2_driver_probe(struct platform_device *dev)
* Reset before dwc2_get_hwparams() then it could get power-on real
* reset value form registers.
*/
- dwc2_core_reset_and_force_dr_mode(hsotg);
+ retval = dwc2_core_reset(hsotg);
+ if (retval)
+ goto error;

/* Detect config values from hardware */
retval = dwc2_get_hwparams(hsotg);
@@ -578,6 +580,11 @@ static int dwc2_driver_probe(struct platform_device *dev)
/* Validate parameter values */
dwc2_set_parameters(hsotg, params);

+ /*
+ * For OTG cores, set the force mode bits to reflect the value
+ * of dr_mode. Force mode bits should not be touched at any
+ * other time after this.
+ */
dwc2_force_dr_mode(hsotg);

if (hsotg->dr_mode != USB_DR_MODE_HOST) {
--
2.7.4

2016-03-29 02:37:33

by John Youn

[permalink] [raw]
Subject: [RFT PATCH 1/4] usb: dwc2: do not override forced dr_mode in gadget setup

From: Przemek Rudy <[email protected]>

The host/device mode set with dr_mode should be kept all the time,
not being changed to OTG in gadget setup (by overriding CFGUSB_FORCEDEVMODE
and CFGUSB_FORCEHOSTMODE bits).

Signed-off-by: Przemek Rudy <[email protected]>
Signed-off-by: John Youn <[email protected]>
---
drivers/usb/dwc2/gadget.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e9940dd..818f158 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2254,6 +2254,7 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg,
{
u32 intmsk;
u32 val;
+ u32 usbcfg;

/* Kill any ep0 requests as controller will be reinitialized */
kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
@@ -2267,10 +2268,16 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg,
* set configuration.
*/

+ /* keep other bits untouched (so e.g. forced modes are not lost) */
+ usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
+ usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
+ GUSBCFG_HNPCAP);
+
/* set the PLL on, remove the HNP/SRP and set the PHY */
val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
- dwc2_writel(hsotg->phyif | GUSBCFG_TOUTCAL(7) |
- (val << GUSBCFG_USBTRDTIM_SHIFT), hsotg->regs + GUSBCFG);
+ usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
+ (val << GUSBCFG_USBTRDTIM_SHIFT);
+ dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);

dwc2_hsotg_init_fifo(hsotg);

@@ -3031,6 +3038,7 @@ static struct usb_ep_ops dwc2_hsotg_ep_ops = {
static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)
{
u32 trdtim;
+ u32 usbcfg;
/* unmask subset of endpoint interrupts */

dwc2_writel(DIEPMSK_TIMEOUTMSK | DIEPMSK_AHBERRMSK |
@@ -3054,11 +3062,16 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg)

dwc2_hsotg_init_fifo(hsotg);

+ /* keep other bits untouched (so e.g. forced modes are not lost) */
+ usbcfg = dwc2_readl(hsotg->regs + GUSBCFG);
+ usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP |
+ GUSBCFG_HNPCAP);
+
/* set the PLL on, remove the HNP/SRP and set the PHY */
trdtim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5;
- dwc2_writel(hsotg->phyif | GUSBCFG_TOUTCAL(7) |
- (trdtim << GUSBCFG_USBTRDTIM_SHIFT),
- hsotg->regs + GUSBCFG);
+ usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) |
+ (trdtim << GUSBCFG_USBTRDTIM_SHIFT);
+ dwc2_writel(usbcfg, hsotg->regs + GUSBCFG);

if (using_dma(hsotg))
__orr32(hsotg->regs + GAHBCFG, GAHBCFG_DMA_EN);
--
2.7.4

2016-03-31 08:53:28

by Stefan Wahren

[permalink] [raw]
Subject: Re: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems

Hi John,

Am 29.03.2016 um 04:36 schrieb John Youn:
> Hi,
>
> The following patch series addresses the core reset and force mode
> delay problems we have been seeing on dwc2 for some platforms.
>
> I think I have identified the source of the inconsistencies between
> platforms and this series attempts to address them.
>
> Basically everything stems from the IDDIG debounce filter delay, which
> is a function of the PHY clock speed and can range from 5-50 ms if
> enabled. 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.
>
> The first two patches are prerequisites to the force mode fixes,
> including one patch that was sent separately by Przemek Rudy. I have
> resubmitted it with this series for convenience.
>
> Please help by reviewing and testing on your platforms.
>
> Patches were tested on:
> * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL

i applied the complete patch series on 4.6rc-1 and successfully tested
it with my Raspberry Pi B (IP 2.80a, dr_mode=HOST).

Probing and usual USB devices (network, keyboard) still works fine.

Do you need any other specific tests?

Regards
Stefan


>
> 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
>
> Przemek Rudy (1):
> usb: dwc2: do not override forced dr_mode in gadget setup
>
> drivers/usb/dwc2/core.c | 195 ++++++++++++++++++++++++++++----------------
> drivers/usb/dwc2/core.h | 2 +-
> drivers/usb/dwc2/gadget.c | 30 +++++--
> drivers/usb/dwc2/hcd.c | 6 +-
> drivers/usb/dwc2/hw.h | 1 +
> drivers/usb/dwc2/platform.c | 9 +-
> 6 files changed, 161 insertions(+), 82 deletions(-)
>

2016-03-31 20:45:41

by Michael Niewöhner

[permalink] [raw]
Subject: Re: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems

Hi John,

Am 29.03.2016 um 04:36 schrieb John Youn <[email protected]>:

> Hi,
>
> The following patch series addresses the core reset and force mode
> delay problems we have been seeing on dwc2 for some platforms.
>
> I think I have identified the source of the inconsistencies between
> platforms and this series attempts to address them.
>
> Basically everything stems from the IDDIG debounce filter delay, which
> is a function of the PHY clock speed and can range from 5-50 ms if
> enabled. 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.
>
> The first two patches are prerequisites to the force mode fixes,
> including one patch that was sent separately by Przemek Rudy. I have
> resubmitted it with this series for convenience.
>
> Please help by reviewing and testing on your platforms.
>
> Patches were tested on:
> * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL
>
> 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
>
> Przemek Rudy (1):
> usb: dwc2: do not override forced dr_mode in gadget setup
>
> drivers/usb/dwc2/core.c | 195 ++++++++++++++++++++++++++++----------------
> drivers/usb/dwc2/core.h | 2 +-
> drivers/usb/dwc2/gadget.c | 30 +++++--
> drivers/usb/dwc2/hcd.c | 6 +-
> drivers/usb/dwc2/hw.h | 1 +
> drivers/usb/dwc2/platform.c | 9 +-
> 6 files changed, 161 insertions(+), 82 deletions(-)
>
> --
> 2.7.4
>

after applying your patch series on v4.6-rc1 usb keeps being broken on rk3188.
Besides that I get "dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode? repeatedly.

Currently this works for me:
- Revert "usb: dwc2: Fix probe problem on bcm2835?
- Apply "usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"


Best regards
Michael


2016-03-31 21:44:08

by Michael Niewöhner

[permalink] [raw]
Subject: Re: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems

Hi John,

Am 29.03.2016 um 04:36 schrieb John Youn <[email protected]>:

> Hi,
>
> The following patch series addresses the core reset and force mode
> delay problems we have been seeing on dwc2 for some platforms.
>
> I think I have identified the source of the inconsistencies between
> platforms and this series attempts to address them.
>
> Basically everything stems from the IDDIG debounce filter delay, which
> is a function of the PHY clock speed and can range from 5-50 ms if
> enabled. 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.
>
> The first two patches are prerequisites to the force mode fixes,
> including one patch that was sent separately by Przemek Rudy. I have
> resubmitted it with this series for convenience.
>
> Please help by reviewing and testing on your platforms.
>
> Patches were tested on:
> * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL
>
> 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
>
> Przemek Rudy (1):
> usb: dwc2: do not override forced dr_mode in gadget setup
>
> drivers/usb/dwc2/core.c | 195 ++++++++++++++++++++++++++++----------------
> drivers/usb/dwc2/core.h | 2 +-
> drivers/usb/dwc2/gadget.c | 30 +++++--
> drivers/usb/dwc2/hcd.c | 6 +-
> drivers/usb/dwc2/hw.h | 1 +
> drivers/usb/dwc2/platform.c | 9 +-
> 6 files changed, 161 insertions(+), 82 deletions(-)
>
> --
> 2.7.4
>

after applying your patch series on v4.6-rc1 usb keeps being broken on rk3188.
Besides that I get "dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode? repeatedly.

Currently this works for me:
- Revert "usb: dwc2: Fix probe problem on bcm2835?
- Apply "usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"


Best regards
Michael


2016-04-07 18:42:11

by John Youn

[permalink] [raw]
Subject: Re: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems

On 3/31/2016 2:44 PM, Michael Niewoehner wrote:
> Hi John,
>
> Am 29.03.2016 um 04:36 schrieb John Youn <[email protected]>:
>
>> Hi,
>>
>> The following patch series addresses the core reset and force mode
>> delay problems we have been seeing on dwc2 for some platforms.
>>
>> I think I have identified the source of the inconsistencies between
>> platforms and this series attempts to address them.
>>
>> Basically everything stems from the IDDIG debounce filter delay, which
>> is a function of the PHY clock speed and can range from 5-50 ms if
>> enabled. 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.
>>
>> The first two patches are prerequisites to the force mode fixes,
>> including one patch that was sent separately by Przemek Rudy. I have
>> resubmitted it with this series for convenience.
>>
>> Please help by reviewing and testing on your platforms.
>>
>> Patches were tested on:
>> * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL
>>
>> 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
>>
>> Przemek Rudy (1):
>> usb: dwc2: do not override forced dr_mode in gadget setup
>>
>> drivers/usb/dwc2/core.c | 195 ++++++++++++++++++++++++++++----------------
>> drivers/usb/dwc2/core.h | 2 +-
>> drivers/usb/dwc2/gadget.c | 30 +++++--
>> drivers/usb/dwc2/hcd.c | 6 +-
>> drivers/usb/dwc2/hw.h | 1 +
>> drivers/usb/dwc2/platform.c | 9 +-
>> 6 files changed, 161 insertions(+), 82 deletions(-)
>>
>> --
>> 2.7.4
>>
>
> after applying your patch series on v4.6-rc1 usb keeps being broken on rk3188.
> Besides that I get "dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode“ repeatedly.
>
> Currently this works for me:
> - Revert "usb: dwc2: Fix probe problem on bcm2835“
> - Apply "usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"
>
>
> Best regards
> Michael
>

Thanks Michael.

I won't be able to look at this again until next week. In the meantime
could you provide a driver log? In particular I want to see the values
of your GHWCFG registers, and where you are seeing the
dwc2_wait_for_mode() failure.

Regards,
John

2016-04-07 20:36:26

by Michael Niewöhner

[permalink] [raw]
Subject: Re: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems


Am 07.04.2016 um 20:41 schrieb John Youn <[email protected]>:

> On 3/31/2016 2:44 PM, Michael Niewoehner wrote:
>> Hi John,
>>
>> Am 29.03.2016 um 04:36 schrieb John Youn <[email protected]>:
>>
>>> Hi,
>>>
>>> The following patch series addresses the core reset and force mode
>>> delay problems we have been seeing on dwc2 for some platforms.
>>>
>>> I think I have identified the source of the inconsistencies between
>>> platforms and this series attempts to address them.
>>>
>>> Basically everything stems from the IDDIG debounce filter delay, which
>>> is a function of the PHY clock speed and can range from 5-50 ms if
>>> enabled. 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.
>>>
>>> The first two patches are prerequisites to the force mode fixes,
>>> including one patch that was sent separately by Przemek Rudy. I have
>>> resubmitted it with this series for convenience.
>>>
>>> Please help by reviewing and testing on your platforms.
>>>
>>> Patches were tested on:
>>> * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL
>>>
>>> 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
>>>
>>> Przemek Rudy (1):
>>> usb: dwc2: do not override forced dr_mode in gadget setup
>>>
>>> drivers/usb/dwc2/core.c | 195 ++++++++++++++++++++++++++++----------------
>>> drivers/usb/dwc2/core.h | 2 +-
>>> drivers/usb/dwc2/gadget.c | 30 +++++--
>>> drivers/usb/dwc2/hcd.c | 6 +-
>>> drivers/usb/dwc2/hw.h | 1 +
>>> drivers/usb/dwc2/platform.c | 9 +-
>>> 6 files changed, 161 insertions(+), 82 deletions(-)
>>>
>>> --
>>> 2.7.4
>>>
>>
>> after applying your patch series on v4.6-rc1 usb keeps being broken on rk3188.
>> Besides that I get "dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode? repeatedly.
>>
>> Currently this works for me:
>> - Revert "usb: dwc2: Fix probe problem on bcm2835?
>> - Apply "usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"
>>
>>
>> Best regards
>> Michael
>>
>
> Thanks Michael.
>
> I won't be able to look at this again until next week. In the meantime
> could you provide a driver log? In particular I want to see the values
> of your GHWCFG registers, and where you are seeing the
> dwc2_wait_for_mode() failure.
>
> Regards,
> John

Looks like the problem is gone on -rc2? on -rc1 the errors came up shortly after "dwc2 10180000.usb? messages.
USB keeps being broken, though. The USB hub is detected but nothing that is attached to it.

Here are the logs and register values for each test with Doug?s and your patches.

Michael


good usb, Doug's patches

[ 0.420125] usbcore: registered new interface driver usbfs
[ 0.426246] usbcore: registered new interface driver hub
[ 0.432296] usbcore: registered new device driver usb
[...]
[ 0.853769] 10180000.usb supply vusb_d not found, using dummy regulator
[ 0.860560] 10180000.usb supply vusb_a not found, using dummy regulator
[ 0.867365] dwc2 10180000.usb: Configuration mismatch. dr_mode forced to host
[ 0.977737] dwc2 10180000.usb: 128 invalid for host_nperio_tx_fifo_size. Check H.
[ 0.986562] dwc2 10180000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW.
[ 1.047959] dwc2 10180000.usb: DWC OTG Controller
[ 1.052732] dwc2 10180000.usb: new USB bus registered, assigned bus number 1
[ 1.059868] dwc2 10180000.usb: irq 24, io mem 0x00000000
[ 1.065586] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
[ 1.072430] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[ 1.079706] usb usb1: Product: DWC OTG Controller
[ 1.084432] usb usb1: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
[ 1.090390] usb usb1: SerialNumber: 10180000.usb
[ 1.096000] hub 1-0:1.0: USB hub found
[ 1.099884] hub 1-0:1.0: 1 port detected
[ 1.104668] 101c0000.usb supply vusb_d not found, using dummy regulator
[ 1.111428] 101c0000.usb supply vusb_a not found, using dummy regulator
[ 1.247968] dwc2 101c0000.usb: DWC OTG Controller
[ 1.252743] dwc2 101c0000.usb: new USB bus registered, assigned bus number 2
[ 1.259879] dwc2 101c0000.usb: irq 25, io mem 0x00000000
[ 1.265604] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
[ 1.272447] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[ 1.279721] usb usb2: Product: DWC OTG Controller
[ 1.284448] usb usb2: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
[ 1.290404] usb usb2: SerialNumber: 101c0000.usb
[ 1.295951] hub 2-0:1.0: USB hub found
[ 1.299830] hub 2-0:1.0: 1 port detected
[ 1.305694] usbcore: registered new interface driver usb-storage


root@c0rock:~# cat debug/10180000.usb/regdump | grep GHWCFG
[ 1770.718938] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
[ 1770.729974] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
GHWCFG1 = 0x00006664
GHWCFG2 = 0x22aa2450
GHWCFG3 = 0x03cc00e8
GHWCFG4 = 0xdbf04060
root@c0rock:~# cat debug/101c0000.usb/regdump | grep GHWCFG
GHWCFG1 = 0x00000000
GHWCFG2 = 0x22a7c856
GHWCFG3 = 0x03380068
GHWCFG4 = 0x09f04060


==========================


bad usb, your patches

[ 0.420098] usbcore: registered new interface driver usbfs
[ 0.426218] usbcore: registered new interface driver hub
[ 0.432221] usbcore: registered new device driver usb
[...]
[ 0.854505] 10180000.usb supply vusb_d not found, using dummy regulator
[ 0.861295] 10180000.usb supply vusb_a not found, using dummy regulator
[ 0.868126] dwc2 10180000.usb: Configuration mismatch. dr_mode forced to host
[ 0.917648] dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode
[ 0.924472] dwc2 10180000.usb: 128 invalid for host_nperio_tx_fifo_size. Check H.
[ 0.933314] dwc2 10180000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW.
[ 0.977914] dwc2 10180000.usb: DWC OTG Controller
[ 0.982687] dwc2 10180000.usb: new USB bus registered, assigned bus number 1
[ 0.989827] dwc2 10180000.usb: irq 24, io mem 0x00000000
[ 0.995543] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
[ 1.002390] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[ 1.009666] usb usb1: Product: DWC OTG Controller
[ 1.014391] usb usb1: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
[ 1.020347] usb usb1: SerialNumber: 10180000.usb
[ 1.025937] hub 1-0:1.0: USB hub found
[ 1.029819] hub 1-0:1.0: 1 port detected
[ 1.034605] 101c0000.usb supply vusb_d not found, using dummy regulator
[ 1.041367] 101c0000.usb supply vusb_a not found, using dummy regulator
[ 1.048480] dwc2 101c0000.usb: DWC OTG Controller
[ 1.053251] dwc2 101c0000.usb: new USB bus registered, assigned bus number 2
[ 1.060389] dwc2 101c0000.usb: irq 25, io mem 0x00000000
[ 1.066121] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
[ 1.072972] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[ 1.080251] usb usb2: Product: DWC OTG Controller
[ 1.084978] usb usb2: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
[ 1.090935] usb usb2: SerialNumber: 101c0000.usb
[ 1.096504] hub 2-0:1.0: USB hub found
[ 1.100387] hub 2-0:1.0: 1 port detected
[ 1.106271] usbcore: registered new interface driver usb-storage


root@c0rock:~# cat debug/10180000.usb/regdump | grep GHWCFG
[ 224.592330] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
[ 224.603277] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
GHWCFG1 = 0x00006664
GHWCFG2 = 0x22aa2450
GHWCFG3 = 0x03cc00e8
GHWCFG4 = 0xdbf04060
root@c0rock:~# cat debug/101c0000.usb/regdump | grep GHWCFG
GHWCFG1 = 0x00000000
GHWCFG2 = 0x22a7c856
GHWCFG3 = 0x03380068
GHWCFG4 = 0x09f04060

2016-04-07 22:12:39

by John Youn

[permalink] [raw]
Subject: Re: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems

On 4/7/2016 1:36 PM, Michael Niewoehner wrote:
>
> Am 07.04.2016 um 20:41 schrieb John Youn <[email protected]>:
>
>> On 3/31/2016 2:44 PM, Michael Niewoehner wrote:
>>> Hi John,
>>>
>>> Am 29.03.2016 um 04:36 schrieb John Youn <[email protected]>:
>>>
>>>> Hi,
>>>>
>>>> The following patch series addresses the core reset and force mode
>>>> delay problems we have been seeing on dwc2 for some platforms.
>>>>
>>>> I think I have identified the source of the inconsistencies between
>>>> platforms and this series attempts to address them.
>>>>
>>>> Basically everything stems from the IDDIG debounce filter delay, which
>>>> is a function of the PHY clock speed and can range from 5-50 ms if
>>>> enabled. 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.
>>>>
>>>> The first two patches are prerequisites to the force mode fixes,
>>>> including one patch that was sent separately by Przemek Rudy. I have
>>>> resubmitted it with this series for convenience.
>>>>
>>>> Please help by reviewing and testing on your platforms.
>>>>
>>>> Patches were tested on:
>>>> * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL
>>>>
>>>> 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
>>>>
>>>> Przemek Rudy (1):
>>>> usb: dwc2: do not override forced dr_mode in gadget setup
>>>>
>>>> drivers/usb/dwc2/core.c | 195 ++++++++++++++++++++++++++++----------------
>>>> drivers/usb/dwc2/core.h | 2 +-
>>>> drivers/usb/dwc2/gadget.c | 30 +++++--
>>>> drivers/usb/dwc2/hcd.c | 6 +-
>>>> drivers/usb/dwc2/hw.h | 1 +
>>>> drivers/usb/dwc2/platform.c | 9 +-
>>>> 6 files changed, 161 insertions(+), 82 deletions(-)
>>>>
>>>> --
>>>> 2.7.4
>>>>
>>>
>>> after applying your patch series on v4.6-rc1 usb keeps being broken on rk3188.
>>> Besides that I get "dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode“ repeatedly.
>>>
>>> Currently this works for me:
>>> - Revert "usb: dwc2: Fix probe problem on bcm2835“
>>> - Apply "usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"
>>>
>>>
>>> Best regards
>>> Michael
>>>
>>
>> Thanks Michael.
>>
>> I won't be able to look at this again until next week. In the meantime
>> could you provide a driver log? In particular I want to see the values
>> of your GHWCFG registers, and where you are seeing the
>> dwc2_wait_for_mode() failure.
>>
>> Regards,
>> John
>
> Looks like the problem is gone on -rc2… on -rc1 the errors came up shortly after "dwc2 10180000.usb“ messages.
> USB keeps being broken, though. The USB hub is detected but nothing that is attached to it.
>
> Here are the logs and register values for each test with Doug’s and your patches.
>
> Michael
>
>
> good usb, Doug's patches
>
> [ 0.420125] usbcore: registered new interface driver usbfs
> [ 0.426246] usbcore: registered new interface driver hub
> [ 0.432296] usbcore: registered new device driver usb
> [...]
> [ 0.853769] 10180000.usb supply vusb_d not found, using dummy regulator
> [ 0.860560] 10180000.usb supply vusb_a not found, using dummy regulator
> [ 0.867365] dwc2 10180000.usb: Configuration mismatch. dr_mode forced to host
> [ 0.977737] dwc2 10180000.usb: 128 invalid for host_nperio_tx_fifo_size. Check H.
> [ 0.986562] dwc2 10180000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW.
> [ 1.047959] dwc2 10180000.usb: DWC OTG Controller
> [ 1.052732] dwc2 10180000.usb: new USB bus registered, assigned bus number 1
> [ 1.059868] dwc2 10180000.usb: irq 24, io mem 0x00000000
> [ 1.065586] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
> [ 1.072430] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> [ 1.079706] usb usb1: Product: DWC OTG Controller
> [ 1.084432] usb usb1: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
> [ 1.090390] usb usb1: SerialNumber: 10180000.usb
> [ 1.096000] hub 1-0:1.0: USB hub found
> [ 1.099884] hub 1-0:1.0: 1 port detected
> [ 1.104668] 101c0000.usb supply vusb_d not found, using dummy regulator
> [ 1.111428] 101c0000.usb supply vusb_a not found, using dummy regulator
> [ 1.247968] dwc2 101c0000.usb: DWC OTG Controller
> [ 1.252743] dwc2 101c0000.usb: new USB bus registered, assigned bus number 2
> [ 1.259879] dwc2 101c0000.usb: irq 25, io mem 0x00000000
> [ 1.265604] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
> [ 1.272447] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> [ 1.279721] usb usb2: Product: DWC OTG Controller
> [ 1.284448] usb usb2: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
> [ 1.290404] usb usb2: SerialNumber: 101c0000.usb
> [ 1.295951] hub 2-0:1.0: USB hub found
> [ 1.299830] hub 2-0:1.0: 1 port detected
> [ 1.305694] usbcore: registered new interface driver usb-storage
>
>
> root@c0rock:~# cat debug/10180000.usb/regdump | grep GHWCFG
> [ 1770.718938] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
> [ 1770.729974] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
> GHWCFG1 = 0x00006664
> GHWCFG2 = 0x22aa2450
> GHWCFG3 = 0x03cc00e8
> GHWCFG4 = 0xdbf04060
> root@c0rock:~# cat debug/101c0000.usb/regdump | grep GHWCFG
> GHWCFG1 = 0x00000000
> GHWCFG2 = 0x22a7c856
> GHWCFG3 = 0x03380068
> GHWCFG4 = 0x09f04060
>
>
> ==========================
>
>
> bad usb, your patches
>
> [ 0.420098] usbcore: registered new interface driver usbfs
> [ 0.426218] usbcore: registered new interface driver hub
> [ 0.432221] usbcore: registered new device driver usb
> [...]
> [ 0.854505] 10180000.usb supply vusb_d not found, using dummy regulator
> [ 0.861295] 10180000.usb supply vusb_a not found, using dummy regulator
> [ 0.868126] dwc2 10180000.usb: Configuration mismatch. dr_mode forced to host
> [ 0.917648] dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode
> [ 0.924472] dwc2 10180000.usb: 128 invalid for host_nperio_tx_fifo_size. Check H.
> [ 0.933314] dwc2 10180000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW.
> [ 0.977914] dwc2 10180000.usb: DWC OTG Controller
> [ 0.982687] dwc2 10180000.usb: new USB bus registered, assigned bus number 1
> [ 0.989827] dwc2 10180000.usb: irq 24, io mem 0x00000000
> [ 0.995543] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
> [ 1.002390] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> [ 1.009666] usb usb1: Product: DWC OTG Controller
> [ 1.014391] usb usb1: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
> [ 1.020347] usb usb1: SerialNumber: 10180000.usb
> [ 1.025937] hub 1-0:1.0: USB hub found
> [ 1.029819] hub 1-0:1.0: 1 port detected
> [ 1.034605] 101c0000.usb supply vusb_d not found, using dummy regulator
> [ 1.041367] 101c0000.usb supply vusb_a not found, using dummy regulator
> [ 1.048480] dwc2 101c0000.usb: DWC OTG Controller
> [ 1.053251] dwc2 101c0000.usb: new USB bus registered, assigned bus number 2
> [ 1.060389] dwc2 101c0000.usb: irq 25, io mem 0x00000000
> [ 1.066121] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
> [ 1.072972] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> [ 1.080251] usb usb2: Product: DWC OTG Controller
> [ 1.084978] usb usb2: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
> [ 1.090935] usb usb2: SerialNumber: 101c0000.usb
> [ 1.096504] hub 2-0:1.0: USB hub found
> [ 1.100387] hub 2-0:1.0: 1 port detected
> [ 1.106271] usbcore: registered new interface driver usb-storage
>
>
> root@c0rock:~# cat debug/10180000.usb/regdump | grep GHWCFG
> [ 224.592330] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
> [ 224.603277] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
> GHWCFG1 = 0x00006664
> GHWCFG2 = 0x22aa2450
> GHWCFG3 = 0x03cc00e8
> GHWCFG4 = 0xdbf04060

This controller is OTG with the IDDIG filter. So it will default to
device on every reset and there will be a delay before it goes into
host mode.

Could you try to increase the timeout to see if it helps anything?
Patch is below.


> root@c0rock:~# cat debug/101c0000.usb/regdump | grep GHWCFG
> GHWCFG1 = 0x00000000
> GHWCFG2 = 0x22a7c856
> GHWCFG3 = 0x03380068
> GHWCFG4 = 0x09f04060

Looks like this one is a host-only controller so it should work fine,
correct?

Regards,
John


---->8----

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index ab25620..c72f4e5 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -262,15 +262,15 @@ static void dwc2_wait_for_mode(struct dwc2_hsotg *hsotg,
{
ktime_t start;
ktime_t end;
+ s64 ms = 0;

dev_vdbg(hsotg->dev, "Waiting for %s mode\n",
host_mode ? "host" : "device");

start = ktime_get();

+ timeout = 300;
while (1) {
- s64 ms;
-
if (dwc2_is_host_mode(hsotg) == host_mode) {
dev_vdbg(hsotg->dev, "%s mode set\n",
host_mode ? "Host" : "Device");
@@ -288,6 +288,9 @@ static void dwc2_wait_for_mode(struct dwc2_hsotg *hsotg,

usleep_range(1000, 2000);
}
+
+ dev_info(hsotg->dev, "%s: mode=%d, time=%lld ms\n", __func__,
+ dwc2_is_host_mode(hsotg), ms);
}

2016-04-08 10:26:32

by Michael Niewöhner

[permalink] [raw]
Subject: Re: [RFT PATCH 0/4] usb: dwc2: Fix core reset and force mode delay problems


Am 08.04.2016 um 00:12 schrieb John Youn <[email protected]>:

> On 4/7/2016 1:36 PM, Michael Niewoehner wrote:
>>
>> Am 07.04.2016 um 20:41 schrieb John Youn <[email protected]>:
>>
>>> On 3/31/2016 2:44 PM, Michael Niewoehner wrote:
>>>> Hi John,
>>>>
>>>> Am 29.03.2016 um 04:36 schrieb John Youn <[email protected]>:
>>>>
>>>>> Hi,
>>>>>
>>>>> The following patch series addresses the core reset and force mode
>>>>> delay problems we have been seeing on dwc2 for some platforms.
>>>>>
>>>>> I think I have identified the source of the inconsistencies between
>>>>> platforms and this series attempts to address them.
>>>>>
>>>>> Basically everything stems from the IDDIG debounce filter delay, which
>>>>> is a function of the PHY clock speed and can range from 5-50 ms if
>>>>> enabled. 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.
>>>>>
>>>>> The first two patches are prerequisites to the force mode fixes,
>>>>> including one patch that was sent separately by Przemek Rudy. I have
>>>>> resubmitted it with this series for convenience.
>>>>>
>>>>> Please help by reviewing and testing on your platforms.
>>>>>
>>>>> Patches were tested on:
>>>>> * Synopsys HAPS platform IP 3.20a OTG, dr_mode=OTG,HOST,PERIPHERAL
>>>>>
>>>>> 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
>>>>>
>>>>> Przemek Rudy (1):
>>>>> usb: dwc2: do not override forced dr_mode in gadget setup
>>>>>
>>>>> drivers/usb/dwc2/core.c | 195 ++++++++++++++++++++++++++++----------------
>>>>> drivers/usb/dwc2/core.h | 2 +-
>>>>> drivers/usb/dwc2/gadget.c | 30 +++++--
>>>>> drivers/usb/dwc2/hcd.c | 6 +-
>>>>> drivers/usb/dwc2/hw.h | 1 +
>>>>> drivers/usb/dwc2/platform.c | 9 +-
>>>>> 6 files changed, 161 insertions(+), 82 deletions(-)
>>>>>
>>>>> --
>>>>> 2.7.4
>>>>>
>>>>
>>>> after applying your patch series on v4.6-rc1 usb keeps being broken on rk3188.
>>>> Besides that I get "dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode? repeatedly.
>>>>
>>>> Currently this works for me:
>>>> - Revert "usb: dwc2: Fix probe problem on bcm2835?
>>>> - Apply "usb: dwc2: Add a 10 ms delay to dwc2_core_reset()"
>>>>
>>>>
>>>> Best regards
>>>> Michael
>>>>
>>>
>>> Thanks Michael.
>>>
>>> I won't be able to look at this again until next week. In the meantime
>>> could you provide a driver log? In particular I want to see the values
>>> of your GHWCFG registers, and where you are seeing the
>>> dwc2_wait_for_mode() failure.
>>>
>>> Regards,
>>> John
>>
>> Looks like the problem is gone on -rc2? on -rc1 the errors came up shortly after "dwc2 10180000.usb? messages.
>> USB keeps being broken, though. The USB hub is detected but nothing that is attached to it.
>>
>> Here are the logs and register values for each test with Doug?s and your patches.
>>
>> Michael
>>
>>
>> good usb, Doug's patches
>>
>> [ 0.420125] usbcore: registered new interface driver usbfs
>> [ 0.426246] usbcore: registered new interface driver hub
>> [ 0.432296] usbcore: registered new device driver usb
>> [...]
>> [ 0.853769] 10180000.usb supply vusb_d not found, using dummy regulator
>> [ 0.860560] 10180000.usb supply vusb_a not found, using dummy regulator
>> [ 0.867365] dwc2 10180000.usb: Configuration mismatch. dr_mode forced to host
>> [ 0.977737] dwc2 10180000.usb: 128 invalid for host_nperio_tx_fifo_size. Check H.
>> [ 0.986562] dwc2 10180000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW.
>> [ 1.047959] dwc2 10180000.usb: DWC OTG Controller
>> [ 1.052732] dwc2 10180000.usb: new USB bus registered, assigned bus number 1
>> [ 1.059868] dwc2 10180000.usb: irq 24, io mem 0x00000000
>> [ 1.065586] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
>> [ 1.072430] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
>> [ 1.079706] usb usb1: Product: DWC OTG Controller
>> [ 1.084432] usb usb1: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
>> [ 1.090390] usb usb1: SerialNumber: 10180000.usb
>> [ 1.096000] hub 1-0:1.0: USB hub found
>> [ 1.099884] hub 1-0:1.0: 1 port detected
>> [ 1.104668] 101c0000.usb supply vusb_d not found, using dummy regulator
>> [ 1.111428] 101c0000.usb supply vusb_a not found, using dummy regulator
>> [ 1.247968] dwc2 101c0000.usb: DWC OTG Controller
>> [ 1.252743] dwc2 101c0000.usb: new USB bus registered, assigned bus number 2
>> [ 1.259879] dwc2 101c0000.usb: irq 25, io mem 0x00000000
>> [ 1.265604] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
>> [ 1.272447] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
>> [ 1.279721] usb usb2: Product: DWC OTG Controller
>> [ 1.284448] usb usb2: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
>> [ 1.290404] usb usb2: SerialNumber: 101c0000.usb
>> [ 1.295951] hub 2-0:1.0: USB hub found
>> [ 1.299830] hub 2-0:1.0: 1 port detected
>> [ 1.305694] usbcore: registered new interface driver usb-storage
>>
>>
>> root@c0rock:~# cat debug/10180000.usb/regdump | grep GHWCFG
>> [ 1770.718938] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
>> [ 1770.729974] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
>> GHWCFG1 = 0x00006664
>> GHWCFG2 = 0x22aa2450
>> GHWCFG3 = 0x03cc00e8
>> GHWCFG4 = 0xdbf04060
>> root@c0rock:~# cat debug/101c0000.usb/regdump | grep GHWCFG
>> GHWCFG1 = 0x00000000
>> GHWCFG2 = 0x22a7c856
>> GHWCFG3 = 0x03380068
>> GHWCFG4 = 0x09f04060
>>
>>
>> ==========================
>>
>>
>> bad usb, your patches
>>
>> [ 0.420098] usbcore: registered new interface driver usbfs
>> [ 0.426218] usbcore: registered new interface driver hub
>> [ 0.432221] usbcore: registered new device driver usb
>> [...]
>> [ 0.854505] 10180000.usb supply vusb_d not found, using dummy regulator
>> [ 0.861295] 10180000.usb supply vusb_a not found, using dummy regulator
>> [ 0.868126] dwc2 10180000.usb: Configuration mismatch. dr_mode forced to host
>> [ 0.917648] dwc2 10180000.usb: dwc2_wait_for_mode: Couldn't set host mode
>> [ 0.924472] dwc2 10180000.usb: 128 invalid for host_nperio_tx_fifo_size. Check H.
>> [ 0.933314] dwc2 10180000.usb: 256 invalid for host_perio_tx_fifo_size. Check HW.
>> [ 0.977914] dwc2 10180000.usb: DWC OTG Controller
>> [ 0.982687] dwc2 10180000.usb: new USB bus registered, assigned bus number 1
>> [ 0.989827] dwc2 10180000.usb: irq 24, io mem 0x00000000
>> [ 0.995543] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
>> [ 1.002390] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
>> [ 1.009666] usb usb1: Product: DWC OTG Controller
>> [ 1.014391] usb usb1: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
>> [ 1.020347] usb usb1: SerialNumber: 10180000.usb
>> [ 1.025937] hub 1-0:1.0: USB hub found
>> [ 1.029819] hub 1-0:1.0: 1 port detected
>> [ 1.034605] 101c0000.usb supply vusb_d not found, using dummy regulator
>> [ 1.041367] 101c0000.usb supply vusb_a not found, using dummy regulator
>> [ 1.048480] dwc2 101c0000.usb: DWC OTG Controller
>> [ 1.053251] dwc2 101c0000.usb: new USB bus registered, assigned bus number 2
>> [ 1.060389] dwc2 101c0000.usb: irq 25, io mem 0x00000000
>> [ 1.066121] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
>> [ 1.072972] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
>> [ 1.080251] usb usb2: Product: DWC OTG Controller
>> [ 1.084978] usb usb2: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
>> [ 1.090935] usb usb2: SerialNumber: 101c0000.usb
>> [ 1.096504] hub 2-0:1.0: USB hub found
>> [ 1.100387] hub 2-0:1.0: 1 port detected
>> [ 1.106271] usbcore: registered new interface driver usb-storage
>>
>>
>> root@c0rock:~# cat debug/10180000.usb/regdump | grep GHWCFG
>> [ 224.592330] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
>> [ 224.603277] dwc2 10180000.usb: Mode Mismatch Interrupt: currently in Host mode
>> GHWCFG1 = 0x00006664
>> GHWCFG2 = 0x22aa2450
>> GHWCFG3 = 0x03cc00e8
>> GHWCFG4 = 0xdbf04060
>
> This controller is OTG with the IDDIG filter. So it will default to
> device on every reset and there will be a delay before it goes into
> host mode.
>
> Could you try to increase the timeout to see if it helps anything?
> Patch is below.
>
>
>> root@c0rock:~# cat debug/101c0000.usb/regdump | grep GHWCFG
>> GHWCFG1 = 0x00000000
>> GHWCFG2 = 0x22a7c856
>> GHWCFG3 = 0x03380068
>> GHWCFG4 = 0x09f04060
>
> Looks like this one is a host-only controller so it should work fine,
> correct?

I don?t think so.. attached devices are not detected but it looks like the usb hub is detected (see dmesg below).

>
> Regards,
> John
>
>
> ---->8----
>
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index ab25620..c72f4e5 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -262,15 +262,15 @@ static void dwc2_wait_for_mode(struct dwc2_hsotg *hsotg,
> {
> ktime_t start;
> ktime_t end;
> + s64 ms = 0;
>
> dev_vdbg(hsotg->dev, "Waiting for %s mode\n",
> host_mode ? "host" : "device");
>
> start = ktime_get();
>
> + timeout = 300;
> while (1) {
> - s64 ms;
> -
> if (dwc2_is_host_mode(hsotg) == host_mode) {
> dev_vdbg(hsotg->dev, "%s mode set\n",
> host_mode ? "Host" : "Device");
> @@ -288,6 +288,9 @@ static void dwc2_wait_for_mode(struct dwc2_hsotg *hsotg,
>
> usleep_range(1000, 2000);
> }
> +
> + dev_info(hsotg->dev, "%s: mode=%d, time=%lld ms\n", __func__,
> + dwc2_is_host_mode(hsotg), ms);
> }

dmesg output:
[ 0.853521] 10180000.usb supply vusb_d not found, using dummy regulator
[ 0.860310] 10180000.usb supply vusb_a not found, using dummy regulator
[ 0.867130] dwc2 10180000.usb: Configuration mismatch. dr_mode forced to host
[ 0.983984] dwc2 10180000.usb: dwc2_wait_for_mode: mode=1, time=100 ms
[ 0.990560] dwc2 10180000.usb: dwc2_wait_for_mode: mode=1, time=0 ms
[ 0.997218] dwc2 10180000.usb: DWC OTG Controller
[ 1.001987] dwc2 10180000.usb: new USB bus registered, assigned bus number 1
[ 1.009128] dwc2 10180000.usb: irq 24, io mem 0x00000000
[ 1.014946] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
[ 1.021769] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[ 1.029037] usb usb1: Product: DWC OTG Controller
[ 1.033764] usb usb1: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
[ 1.039719] usb usb1: SerialNumber: 10180000.usb
[ 1.045349] hub 1-0:1.0: USB hub found
[ 1.049200] hub 1-0:1.0: 1 port detected
[ 1.054018] 101c0000.usb supply vusb_d not found, using dummy regulator
[ 1.060753] 101c0000.usb supply vusb_a not found, using dummy regulator
[ 1.067832] dwc2 101c0000.usb: DWC OTG Controller
[ 1.072601] dwc2 101c0000.usb: new USB bus registered, assigned bus number 2
[ 1.079734] dwc2 101c0000.usb: irq 25, io mem 0x00000000
[ 1.085480] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
[ 1.092297] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[ 1.099575] usb usb2: Product: DWC OTG Controller
[ 1.104320] usb usb2: Manufacturer: Linux 4.6.0-rc2+ dwc2_hsotg
[ 1.110259] usb usb2: SerialNumber: 101c0000.usb
[ 1.115820] hub 2-0:1.0: USB hub found
[ 1.119670] hub 2-0:1.0: 1 port detected
[ 1.125551] usbcore: registered new interface driver usb-storage


There was one more error I missed in my last mail. It does only show up with your patches
[ 6.684413] usb usb2-port1: connect-debounce failed


Best regards,
Michael