[PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board (rebased to synopsys-usb/next)

[PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board (rebased to synopsys-usb/next)

[John Stultz]

Just wanted to resend my patches for dwc2 controller on the
HiKey board for consideration for the 4.11 merge window.

This patchset is the same as v2, only rebased against
John's synopsys-usb/next branch.

This does still exclude my patchset[1] to add extcon support to
dwc2, which John Youn suspects a pending rework of the dwc2 fifo
init logic might make unnecssary.

thanks
-john

[1] https://lkml.org/lkml/2016/12/6/69

v2:
* Reworked goto logic in patch #2, as suggested by Vardan
v3:
* Rebased to synopsys-usb/next


Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Vardan Mikayelyan <mvardan@synopsys.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org

Chen Yu (2):
  usb: dwc2: Force port resume on switching to device mode
  usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

John Stultz (2):
  usb: dwc2: Avoid sleeping while holding hsotg->lock
  usb: dwc2: Workaround case where GOTGCTL state is wrong

 drivers/usb/dwc2/core.c   |  6 ++--
 drivers/usb/dwc2/core.h   |  9 +++++-
 drivers/usb/dwc2/gadget.c |  2 +-
 drivers/usb/dwc2/hcd.c    | 76 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/dwc2/params.c |  1 +
 5 files changed, 89 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock

[John Stultz]

Basically when plugging in various cables in different orders, I'm
occasionally seeing the following BUG splat:

[   86.215403] BUG: scheduling while atomic: kworker/u16:2/53/0x00000002
[   86.219164] usb 1-1: USB disconnect, device number 9
[   86.226845] Preemption disabled at:[   86.230218]
[<ffffff8008673558>] dwc2_conn_id_status_change+0x120/0x250
[   86.236894] CPU: 0 PID: 53 Comm: kworker/u16:2 Tainted: G        W
     4.9.0-rc8-00051-gd5a7979-dirty #1702
[   86.246836] Hardware name: HiKey Development Board (DT)
[   86.252100] Workqueue: dwc2 dwc2_conn_id_status_change
[   86.257279] Call trace:
[   86.259771] [<ffffff8008087c28>] dump_backtrace+0x0/0x1a0
[   86.265210] [<ffffff8008087ddc>] show_stack+0x14/0x20
[   86.270308] [<ffffff80084343f0>] dump_stack+0x90/0xb0
[   86.275401] [<ffffff80080d8d94>] __schedule_bug+0x6c/0xb8
[   86.280841] [<ffffff8008a07220>] __schedule+0x4f8/0x5b0
[   86.286099] [<ffffff8008a073e8>] schedule+0x38/0xa0
[   86.291017] [<ffffff8008a0a6cc>] schedule_hrtimeout_range_clock+0x8c/0xf0
[   86.297846] [<ffffff8008a0a740>] schedule_hrtimeout_range+0x10/0x18
[   86.304150] [<ffffff8008a0a4a0>] usleep_range+0x50/0x58
[   86.309418] [<ffffff800866d8dc>] dwc2_wait_for_mode.isra.4+0x54/0xd0
[   86.315815] [<ffffff800866f058>] dwc2_core_reset+0xe0/0x168
[   86.321431] [<ffffff800867e364>] dwc2_hsotg_core_init_disconnected+0x2c/0x310
[   86.328602] [<ffffff8008673568>] dwc2_conn_id_status_change+0x130/0x250
[   86.335254] [<ffffff80080ccd48>] process_one_work+0x118/0x370
[   86.341035] [<ffffff80080ccfe8>] worker_thread+0x48/0x498
[   86.346473] [<ffffff80080d2eb0>] kthread+0xd0/0xe8
[   86.351299] [<ffffff8008082e80>] ret_from_fork+0x10/0x50

This seems to be caused by the dwc2_wait_for_mode() calling
usleep_range() while the hstog->lock spinlock is held, since
we take that before calling dwc2_hsotg_core_init_disconnected().

This patch avoids the issue by adding an extra argument to
dwc2_core_reset(), as suggested by John Youn, which allows us to
skip the waiting, which should be unnecessary when calling from
dwc2_hsotg_core_init_disconnected().

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Vardan Mikayelyan <mvardan@synopsys.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v3:
* Rebased to synopsys-usb/next branch

 drivers/usb/dwc2/core.c   | 6 +++---
 drivers/usb/dwc2/core.h   | 2 +-
 drivers/usb/dwc2/gadget.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index c987547..7195366 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -313,7 +313,7 @@ static bool dwc2_iddig_filter_enabled(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.
  */
-int dwc2_core_reset(struct dwc2_hsotg *hsotg)
+int dwc2_core_reset(struct dwc2_hsotg *hsotg, bool skip_wait)
 {
 	u32 greset;
 	int count = 0;
@@ -369,7 +369,7 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
 		}
 	} while (!(greset & GRSTCTL_AHBIDLE));
 
-	if (wait_for_host_mode)
+	if (wait_for_host_mode && !skip_wait)
 		dwc2_wait_for_mode(hsotg, true);
 
 	return 0;
@@ -500,7 +500,7 @@ int dwc2_core_reset_and_force_dr_mode(struct dwc2_hsotg *hsotg)
 {
 	int retval;
 
-	retval = dwc2_core_reset(hsotg);
+	retval = dwc2_core_reset(hsotg, false);
 	if (retval)
 		return retval;
 
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 013feb9..05ce52c 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1089,7 +1089,7 @@ static inline bool dwc2_is_hs_iot(struct dwc2_hsotg *hsotg)
  * The following functions support initialization of the core driver component
  * and the DWC_otg controller
  */
-int dwc2_core_reset(struct dwc2_hsotg *hsotg);
+int dwc2_core_reset(struct dwc2_hsotg *hsotg, bool skip_wait);
 int dwc2_core_reset_and_force_dr_mode(struct dwc2_hsotg *hsotg);
 int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg);
 int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, bool restore);
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 242930e..e9cf162 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3161,7 +3161,7 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg,
 	kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
 
 	if (!is_usb_reset)
-		if (dwc2_core_reset(hsotg))
+		if (dwc2_core_reset(hsotg, true))
 			return;
 
 	/*
-- 
2.7.4

[PATCH 2/4 v3] usb: dwc2: Workaround case where GOTGCTL state is wrong

[John Stultz]

When removing a USB-A to USB-otg adapter cable, we get a change
status irq, and then in dwc2_conn_id_status_change, we
erroniously see the GOTGCTL_CONID_B flag set. This causes us to
get  stuck in the "while (!dwc2_is_device_mode(hsotg))" loop,
spitting out "Waiting for Peripheral Mode, Mode=Host" warnings
until it fails out many seconds later.

This patch works around the issue by re-reading the GOTGCTL
state to check if the GOTGCTL_CONID_B is still set and if not
restarting the change status logic.

I suspect this isn't the best solution, but it seems to work
well for me.

Feedback would be greatly appreciated!

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Vardan Mikayelyan <mvardan@synopsys.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Acked-by: John Youn <johnyoun@synopsys.com>
Reviewed-by: Vardan Mikayelyan <mvardan@synopsys.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2: Rework goto logic suggested by Vardan, and add a comment
v3:
* Rebased to synopsys-usb/next branch
---
 drivers/usb/dwc2/hcd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index b63b14f..888923c 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3238,6 +3238,14 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
 				 dwc2_is_host_mode(hsotg) ? "Host" :
 				 "Peripheral");
 			usleep_range(20000, 40000);
+			/*
+			 * Sometimes the initial GOTGCTRL read is wrong, so
+			 * check it again and jump to host mode if that was
+			 * the case.
+			 */
+			gotgctl = dwc2_readl(hsotg->regs + GOTGCTL);
+			if (!(gotgctl & GOTGCTL_CONID_B))
+				goto host;
 			if (++count > 250)
 				break;
 		}
@@ -3252,6 +3260,7 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
 		spin_unlock_irqrestore(&hsotg->lock, flags);
 		dwc2_hsotg_core_connect(hsotg);
 	} else {
+host:
 		/* A-Device connector (Host Mode) */
 		dev_dbg(hsotg->dev, "connId A\n");
 		while (!dwc2_is_host_mode(hsotg)) {
-- 
2.7.4

[PATCH 3/4 v3] usb: dwc2: Force port resume on switching to device mode

[John Stultz]

From: Chen Yu <chenyu56@huawei.com>

We've seen failures when switching between host and gadget mode,
which was diagnosed as being caused due to the bus being
auto-suspended when we switched.

So this patch forces a port resume when switching to device
mode if the bus is suspended.

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Vardan Mikayelyan <mvardan@synopsys.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Chen Yu <chenyu56@huawei.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v3:
* Rebased to synopsys-usb/next branch

 drivers/usb/dwc2/hcd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 888923c..cdf0bac 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -54,6 +54,8 @@
 #include "core.h"
 #include "hcd.h"
 
+static void dwc2_port_resume(struct dwc2_hsotg *hsotg);
+
 /*
  * =========================================================================
  *  Host Core Layer Functions
@@ -3232,6 +3234,11 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
 	if (gotgctl & GOTGCTL_CONID_B) {
 		/* Wait for switch to device mode */
 		dev_dbg(hsotg->dev, "connId B\n");
+		if (hsotg->bus_suspended) {
+			dev_info(hsotg->dev,
+				 "Do port resume before switching to device mode\n");
+			dwc2_port_resume(hsotg);
+		}
 		while (!dwc2_is_device_mode(hsotg)) {
 			dev_info(hsotg->dev,
 				 "Waiting for Peripheral Mode, Mode=%s\n",
-- 
2.7.4

[PATCH 4/4 v3] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220

[John Stultz]

From: Chen Yu <chenyu56@huawei.com>

The Hi6220's usb controller is limited in that it does not
support "Split Transactions", so it does not support communicating
with low-speed and full-speed devices behind a high-speed hub.

Thus it requires a quirk so that we can manually drop the usb
speed when low/full-speed are attached, and bump back to high
speed when they are removed.

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Vardan Mikayelyan <mvardan@synopsys.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Chen Yu <chenyu56@huawei.com>
[jstultz: Reworked to simplify the patch, and made
 commit log to be more specific about the issue]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2:
* Fix build issue reported by kbuildbot
* Rework to avoid using new dts entry suggested by RobH
* Further tweaks from Chen Yu to try to address comments from
  John Youn
* Further simplified logic
* Reworked to adapt around changes that landed in 4.10-rc
v3:
* Rebased to synopsys-usb/next branch
---
 drivers/usb/dwc2/core.h   |  7 ++++++
 drivers/usb/dwc2/hcd.c    | 60 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/dwc2/params.c |  1 +
 3 files changed, 68 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 05ce52c..5370e64 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -444,6 +444,11 @@ enum dwc2_ep0_state {
  *			in DWORDS with possible values from from
  *			16-32768 (default: 256, 256, 256, 256, 768,
  *			768, 768, 768, 0, 0, 0, 0, 0, 0, 0).
+ * @change_speed_quirk: Change speed configuration to DWC2_SPEED_PARAM_FULL
+ *                      while full&low speed device connect. And change speed
+ *                      back to DWC2_SPEED_PARAM_HIGH while device is gone.
+ *			0 - No (default)
+ *			1 - Yes
  *
  * The following parameters may be specified when starting the module. These
  * parameters define how the DWC_otg controller should be configured. A
@@ -501,6 +506,8 @@ struct dwc2_core_params {
 	u32 g_rx_fifo_size;
 	u32 g_np_tx_fifo_size;
 	u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
+
+	bool change_speed_quirk;
 };
 
 /**
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index cdf0bac..c3a0fd8 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -4872,6 +4872,61 @@ static void _dwc2_hcd_clear_tt_buffer_complete(struct usb_hcd *hcd,
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 }
 
+/*
+ * HPRT0_SPD_HIGH_SPEED: high speed
+ * HPRT0_SPD_FULL_SPEED: full speed
+ */
+static void dwc2_change_bus_speed(struct usb_hcd *hcd, int speed)
+{
+	struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+	if (hsotg->params.speed == speed)
+		return;
+
+	hsotg->params.speed = speed;
+	queue_work(hsotg->wq_otg, &hsotg->wf_otg);
+}
+
+static void dwc2_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
+{
+	struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+	if (!hsotg->params.change_speed_quirk)
+		return;
+
+	/*
+	 * On removal, set speed to default high-speed.
+	 */
+	if (udev->parent && udev->parent->speed > USB_SPEED_UNKNOWN &&
+	    udev->parent->speed < USB_SPEED_HIGH) {
+		dev_info(hsotg->dev, "Set speed to default high-speed\n");
+		dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
+	}
+}
+
+static int dwc2_reset_device(struct usb_hcd *hcd, struct usb_device *udev)
+{
+	struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd);
+
+	if (!hsotg->params.change_speed_quirk)
+		return 0;
+
+	if (udev->speed == USB_SPEED_HIGH) {
+		dev_info(hsotg->dev, "Set speed to high-speed\n");
+		dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED);
+	} else if ((udev->speed == USB_SPEED_FULL ||
+				udev->speed == USB_SPEED_LOW)) {
+		/*
+		 * Change speed setting to full-speed if there's
+		 * a full-speed or low-speed device plugged in.
+		 */
+		dev_info(hsotg->dev, "Set speed to full-speed\n");
+		dwc2_change_bus_speed(hcd, HPRT0_SPD_FULL_SPEED);
+	}
+
+	return 0;
+}
+
 static struct hc_driver dwc2_hc_driver = {
 	.description = "dwc2_hsotg",
 	.product_desc = "DWC OTG Controller",
@@ -5027,6 +5082,11 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 			dev_warn(hsotg->dev, "can't set coherent DMA mask\n");
 	}
 
+	if (hsotg->params.change_speed_quirk) {
+		dwc2_hc_driver.free_dev = dwc2_free_dev;
+		dwc2_hc_driver.reset_device = dwc2_reset_device;
+	}
+
 	hcd = usb_create_hcd(&dwc2_hc_driver, hsotg->dev, dev_name(hsotg->dev));
 	if (!hcd)
 		goto error1;
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 620b027..8e5039e 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -68,6 +68,7 @@ static void dwc2_set_his_params(struct dwc2_hsotg *hsotg)
 	p->ahbcfg = GAHBCFG_HBSTLEN_INCR16 <<
 		GAHBCFG_HBSTLEN_SHIFT;
 	p->uframe_sched = false;
+	p->change_speed_quirk = true;
 }
 
 static void dwc2_set_rk_params(struct dwc2_hsotg *hsotg)
-- 
2.7.4

Re: [PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board (rebased to synopsys-usb/next)

[John Youn]

On 1/11/2017 4:22 PM, John Stultz wrote:
> Just wanted to resend my patches for dwc2 controller on the
> HiKey board for consideration for the 4.11 merge window.
> 
> This patchset is the same as v2, only rebased against
> John's synopsys-usb/next branch.
> 
> This does still exclude my patchset[1] to add extcon support to
> dwc2, which John Youn suspects a pending rework of the dwc2 fifo
> init logic might make unnecssary.
> 
> thanks
> -john
> 
> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2016_12_6_69&d=DgIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=6QmpEaovCIrw3iVcLB9lyZ2hPpxb_n6SWfdUDBIINFU&s=JoQtFGWd8y_xhRErt3j8U8uoUw_kq0AStxhqSPwEa3Q&e= 
> 
> v2:
> * Reworked goto logic in patch #2, as suggested by Vardan
> v3:
> * Rebased to synopsys-usb/next
> 

Thanks

For this series:
Acked-by: John Youn <johnyoun@synopsys.com>

Regards,
John

Re: [PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board (rebased to synopsys-usb/next)

[Felipe Balbi]

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


Hi,

John Youn <John.Youn@synopsys.com> writes:
> On 1/11/2017 4:22 PM, John Stultz wrote:
>> Just wanted to resend my patches for dwc2 controller on the
>> HiKey board for consideration for the 4.11 merge window.
>> 
>> This patchset is the same as v2, only rebased against
>> John's synopsys-usb/next branch.
>> 
>> This does still exclude my patchset[1] to add extcon support to
>> dwc2, which John Youn suspects a pending rework of the dwc2 fifo
>> init logic might make unnecssary.
>> 
>> thanks
>> -john
>> 
>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2016_12_6_69&d=DgIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=6QmpEaovCIrw3iVcLB9lyZ2hPpxb_n6SWfdUDBIINFU&s=JoQtFGWd8y_xhRErt3j8U8uoUw_kq0AStxhqSPwEa3Q&e= 
>> 
>> v2:
>> * Reworked goto logic in patch #2, as suggested by Vardan
>> v3:
>> * Rebased to synopsys-usb/next
>> 
>
> Thanks
>
> For this series:
> Acked-by: John Youn <johnyoun@synopsys.com>

Do we need these in the -rc cycle?

-- 
balbi

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

Re: [PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board (rebased to synopsys-usb/next)

[John Youn]

On 1/12/2017 12:07 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <John.Youn@synopsys.com> writes:
>> On 1/11/2017 4:22 PM, John Stultz wrote:
>>> Just wanted to resend my patches for dwc2 controller on the
>>> HiKey board for consideration for the 4.11 merge window.
>>>
>>> This patchset is the same as v2, only rebased against
>>> John's synopsys-usb/next branch.
>>>
>>> This does still exclude my patchset[1] to add extcon support to
>>> dwc2, which John Youn suspects a pending rework of the dwc2 fifo
>>> init logic might make unnecssary.
>>>
>>> thanks
>>> -john
>>>
>>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2016_12_6_69&d=DgIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=6QmpEaovCIrw3iVcLB9lyZ2hPpxb_n6SWfdUDBIINFU&s=JoQtFGWd8y_xhRErt3j8U8uoUw_kq0AStxhqSPwEa3Q&e= 
>>>
>>> v2:
>>> * Reworked goto logic in patch #2, as suggested by Vardan
>>> v3:
>>> * Rebased to synopsys-usb/next
>>>
>>
>> Thanks
>>
>> For this series:
>> Acked-by: John Youn <johnyoun@synopsys.com>
> 
> Do we need these in the -rc cycle?
> 

This series should be applied after my param rework series for next
[1].

If you have any issues with applying I can resend everything together.

Regards,
John

[1] https://www.spinics.net/lists/linux-usb/msg151693.html

Re: [PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board (rebased to synopsys-usb/next)

[John Stultz]

On Thu, Jan 12, 2017 at 12:05 AM, Felipe Balbi
<felipe.balbi@linux.intel.com> wrote:
>
> Hi,
>
> John Youn <John.Youn@synopsys.com> writes:
>> On 1/11/2017 4:22 PM, John Stultz wrote:
>>> Just wanted to resend my patches for dwc2 controller on the
>>> HiKey board for consideration for the 4.11 merge window.
>>>
>>> This patchset is the same as v2, only rebased against
>>> John's synopsys-usb/next branch.
>>>
>>> This does still exclude my patchset[1] to add extcon support to
>>> dwc2, which John Youn suspects a pending rework of the dwc2 fifo
>>> init logic might make unnecssary.
>>>
>>> thanks
>>> -john
>>>
>>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2016_12_6_69&d=DgIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=6QmpEaovCIrw3iVcLB9lyZ2hPpxb_n6SWfdUDBIINFU&s=JoQtFGWd8y_xhRErt3j8U8uoUw_kq0AStxhqSPwEa3Q&e=
>>>
>>> v2:
>>> * Reworked goto logic in patch #2, as suggested by Vardan
>>> v3:
>>> * Rebased to synopsys-usb/next
>>>
>>
>> Thanks
>>
>> For this series:
>> Acked-by: John Youn <johnyoun@synopsys.com>
>
> Do we need these in the -rc cycle?

I'd say no. There is one fix "Avoid sleeping while holding
hsotg->lock", which could be considered, but seems to only get
triggered on HiKey. So I wouldn't consider it a regression since the
other patches in this series are needed for HiKey to function.

thanks
-john

Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock

[Felipe Balbi]

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


Hi,

John Stultz <john.stultz@linaro.org> writes:
> Basically when plugging in various cables in different orders, I'm
> occasionally seeing the following BUG splat:
>
> [   86.215403] BUG: scheduling while atomic: kworker/u16:2/53/0x00000002
> [   86.219164] usb 1-1: USB disconnect, device number 9
> [   86.226845] Preemption disabled at:[   86.230218]
> [<ffffff8008673558>] dwc2_conn_id_status_change+0x120/0x250
> [   86.236894] CPU: 0 PID: 53 Comm: kworker/u16:2 Tainted: G        W
>      4.9.0-rc8-00051-gd5a7979-dirty #1702
> [   86.246836] Hardware name: HiKey Development Board (DT)
> [   86.252100] Workqueue: dwc2 dwc2_conn_id_status_change
> [   86.257279] Call trace:
> [   86.259771] [<ffffff8008087c28>] dump_backtrace+0x0/0x1a0
> [   86.265210] [<ffffff8008087ddc>] show_stack+0x14/0x20
> [   86.270308] [<ffffff80084343f0>] dump_stack+0x90/0xb0
> [   86.275401] [<ffffff80080d8d94>] __schedule_bug+0x6c/0xb8
> [   86.280841] [<ffffff8008a07220>] __schedule+0x4f8/0x5b0
> [   86.286099] [<ffffff8008a073e8>] schedule+0x38/0xa0
> [   86.291017] [<ffffff8008a0a6cc>] schedule_hrtimeout_range_clock+0x8c/0xf0
> [   86.297846] [<ffffff8008a0a740>] schedule_hrtimeout_range+0x10/0x18
> [   86.304150] [<ffffff8008a0a4a0>] usleep_range+0x50/0x58
> [   86.309418] [<ffffff800866d8dc>] dwc2_wait_for_mode.isra.4+0x54/0xd0
> [   86.315815] [<ffffff800866f058>] dwc2_core_reset+0xe0/0x168
> [   86.321431] [<ffffff800867e364>] dwc2_hsotg_core_init_disconnected+0x2c/0x310
> [   86.328602] [<ffffff8008673568>] dwc2_conn_id_status_change+0x130/0x250
> [   86.335254] [<ffffff80080ccd48>] process_one_work+0x118/0x370
> [   86.341035] [<ffffff80080ccfe8>] worker_thread+0x48/0x498
> [   86.346473] [<ffffff80080d2eb0>] kthread+0xd0/0xe8
> [   86.351299] [<ffffff8008082e80>] ret_from_fork+0x10/0x50
>
> This seems to be caused by the dwc2_wait_for_mode() calling
> usleep_range() while the hstog->lock spinlock is held, since
> we take that before calling dwc2_hsotg_core_init_disconnected().
>
> This patch avoids the issue by adding an extra argument to
> dwc2_core_reset(), as suggested by John Youn, which allows us to
> skip the waiting, which should be unnecessary when calling from
> dwc2_hsotg_core_init_disconnected().
>
> Cc: Wei Xu <xuwei5@hisilicon.com>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: Amit Pundir <amit.pundir@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: John Youn <johnyoun@synopsys.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Chen Yu <chenyu56@huawei.com>
> Cc: Vardan Mikayelyan <mvardan@synopsys.com>
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---

doesn't apply to my testing/next. Please rebase

-- 
balbi

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

Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock

[John Stultz]

On Mon, Jan 16, 2017 at 2:36 AM, Felipe Balbi
<felipe.balbi@linux.intel.com> wrote:
>
> Hi,
>
> John Stultz <john.stultz@linaro.org> writes:
>> Basically when plugging in various cables in different orders, I'm
>> occasionally seeing the following BUG splat:
>>
>> [   86.215403] BUG: scheduling while atomic: kworker/u16:2/53/0x00000002
>> [   86.219164] usb 1-1: USB disconnect, device number 9
>> [   86.226845] Preemption disabled at:[   86.230218]
>> [<ffffff8008673558>] dwc2_conn_id_status_change+0x120/0x250
>> [   86.236894] CPU: 0 PID: 53 Comm: kworker/u16:2 Tainted: G        W
>>      4.9.0-rc8-00051-gd5a7979-dirty #1702
>> [   86.246836] Hardware name: HiKey Development Board (DT)
>> [   86.252100] Workqueue: dwc2 dwc2_conn_id_status_change
>> [   86.257279] Call trace:
>> [   86.259771] [<ffffff8008087c28>] dump_backtrace+0x0/0x1a0
>> [   86.265210] [<ffffff8008087ddc>] show_stack+0x14/0x20
>> [   86.270308] [<ffffff80084343f0>] dump_stack+0x90/0xb0
>> [   86.275401] [<ffffff80080d8d94>] __schedule_bug+0x6c/0xb8
>> [   86.280841] [<ffffff8008a07220>] __schedule+0x4f8/0x5b0
>> [   86.286099] [<ffffff8008a073e8>] schedule+0x38/0xa0
>> [   86.291017] [<ffffff8008a0a6cc>] schedule_hrtimeout_range_clock+0x8c/0xf0
>> [   86.297846] [<ffffff8008a0a740>] schedule_hrtimeout_range+0x10/0x18
>> [   86.304150] [<ffffff8008a0a4a0>] usleep_range+0x50/0x58
>> [   86.309418] [<ffffff800866d8dc>] dwc2_wait_for_mode.isra.4+0x54/0xd0
>> [   86.315815] [<ffffff800866f058>] dwc2_core_reset+0xe0/0x168
>> [   86.321431] [<ffffff800867e364>] dwc2_hsotg_core_init_disconnected+0x2c/0x310
>> [   86.328602] [<ffffff8008673568>] dwc2_conn_id_status_change+0x130/0x250
>> [   86.335254] [<ffffff80080ccd48>] process_one_work+0x118/0x370
>> [   86.341035] [<ffffff80080ccfe8>] worker_thread+0x48/0x498
>> [   86.346473] [<ffffff80080d2eb0>] kthread+0xd0/0xe8
>> [   86.351299] [<ffffff8008082e80>] ret_from_fork+0x10/0x50
>>
>> This seems to be caused by the dwc2_wait_for_mode() calling
>> usleep_range() while the hstog->lock spinlock is held, since
>> we take that before calling dwc2_hsotg_core_init_disconnected().
>>
>> This patch avoids the issue by adding an extra argument to
>> dwc2_core_reset(), as suggested by John Youn, which allows us to
>> skip the waiting, which should be unnecessary when calling from
>> dwc2_hsotg_core_init_disconnected().
>>
>> Cc: Wei Xu <xuwei5@hisilicon.com>
>> Cc: Guodong Xu <guodong.xu@linaro.org>
>> Cc: Amit Pundir <amit.pundir@linaro.org>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: John Youn <johnyoun@synopsys.com>
>> Cc: Douglas Anderson <dianders@chromium.org>
>> Cc: Chen Yu <chenyu56@huawei.com>
>> Cc: Vardan Mikayelyan <mvardan@synopsys.com>
>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: linux-usb@vger.kernel.org
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
>
> doesn't apply to my testing/next. Please rebase

So these were rebased onto JohnY's tree here:
  https://github.com/synopsys-usb/linux.git next

And apparently have been merged there. I suspect he's going to submit
his entire tree there to you?

JohnY: Is this right?

thanks
-john

Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock

[John Youn]

> On Jan 16, 2017, at 12:37 PM, John Stultz <john.stultz@linaro.org> wrote:
> 
> On Mon, Jan 16, 2017 at 2:36 AM, Felipe Balbi
> <felipe.balbi@linux.intel.com> wrote:
>> 
>> Hi,
>> 
>> John Stultz <john.stultz@linaro.org> writes:
>>> Basically when plugging in various cables in different orders, I'm
>>> occasionally seeing the following BUG splat:
>>> 
>>> [   86.215403] BUG: scheduling while atomic: kworker/u16:2/53/0x00000002
>>> [   86.219164] usb 1-1: USB disconnect, device number 9
>>> [   86.226845] Preemption disabled at:[   86.230218]
>>> [<ffffff8008673558>] dwc2_conn_id_status_change+0x120/0x250
>>> [   86.236894] CPU: 0 PID: 53 Comm: kworker/u16:2 Tainted: G        W
>>>     4.9.0-rc8-00051-gd5a7979-dirty #1702
>>> [   86.246836] Hardware name: HiKey Development Board (DT)
>>> [   86.252100] Workqueue: dwc2 dwc2_conn_id_status_change
>>> [   86.257279] Call trace:
>>> [   86.259771] [<ffffff8008087c28>] dump_backtrace+0x0/0x1a0
>>> [   86.265210] [<ffffff8008087ddc>] show_stack+0x14/0x20
>>> [   86.270308] [<ffffff80084343f0>] dump_stack+0x90/0xb0
>>> [   86.275401] [<ffffff80080d8d94>] __schedule_bug+0x6c/0xb8
>>> [   86.280841] [<ffffff8008a07220>] __schedule+0x4f8/0x5b0
>>> [   86.286099] [<ffffff8008a073e8>] schedule+0x38/0xa0
>>> [   86.291017] [<ffffff8008a0a6cc>] schedule_hrtimeout_range_clock+0x8c/0xf0
>>> [   86.297846] [<ffffff8008a0a740>] schedule_hrtimeout_range+0x10/0x18
>>> [   86.304150] [<ffffff8008a0a4a0>] usleep_range+0x50/0x58
>>> [   86.309418] [<ffffff800866d8dc>] dwc2_wait_for_mode.isra.4+0x54/0xd0
>>> [   86.315815] [<ffffff800866f058>] dwc2_core_reset+0xe0/0x168
>>> [   86.321431] [<ffffff800867e364>] dwc2_hsotg_core_init_disconnected+0x2c/0x310
>>> [   86.328602] [<ffffff8008673568>] dwc2_conn_id_status_change+0x130/0x250
>>> [   86.335254] [<ffffff80080ccd48>] process_one_work+0x118/0x370
>>> [   86.341035] [<ffffff80080ccfe8>] worker_thread+0x48/0x498
>>> [   86.346473] [<ffffff80080d2eb0>] kthread+0xd0/0xe8
>>> [   86.351299] [<ffffff8008082e80>] ret_from_fork+0x10/0x50
>>> 
>>> This seems to be caused by the dwc2_wait_for_mode() calling
>>> usleep_range() while the hstog->lock spinlock is held, since
>>> we take that before calling dwc2_hsotg_core_init_disconnected().
>>> 
>>> This patch avoids the issue by adding an extra argument to
>>> dwc2_core_reset(), as suggested by John Youn, which allows us to
>>> skip the waiting, which should be unnecessary when calling from
>>> dwc2_hsotg_core_init_disconnected().
>>> 
>>> Cc: Wei Xu <xuwei5@hisilicon.com>
>>> Cc: Guodong Xu <guodong.xu@linaro.org>
>>> Cc: Amit Pundir <amit.pundir@linaro.org>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: John Youn <johnyoun@synopsys.com>
>>> Cc: Douglas Anderson <dianders@chromium.org>
>>> Cc: Chen Yu <chenyu56@huawei.com>
>>> Cc: Vardan Mikayelyan <mvardan@synopsys.com>
>>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>>> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: linux-usb@vger.kernel.org
>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>>> ---
>> 
>> doesn't apply to my testing/next. Please rebase
> 
> So these were rebased onto JohnY's tree here:
>  https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_synopsys-2Dusb_linux.git&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=eBJTBuJyU21iKJvHdy5FxqtxsVARo0iIqJGxMHrlbyQ&s=VIiAT32aG7s04G5NoOOthNdm2JX0eWjJpg62neY_-KI&e=  next
> 
> And apparently have been merged there. I suspect he's going to submit
> his entire tree there to you?
> 
> JohnY: Is this right?


Yeah I'll get these issues sorted out with Felipe. Which may mean resubmitting everything the the proper order.

Thanks,
John

Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock

[John Stultz]

On Mon, Jan 16, 2017 at 12:57 PM, John Youn <John.Youn@synopsys.com> wrote:
>
>
>> On Jan 16, 2017, at 12:37 PM, John Stultz <john.stultz@linaro.org> wrote:
>>
>> On Mon, Jan 16, 2017 at 2:36 AM, Felipe Balbi
>> <felipe.balbi@linux.intel.com> wrote:
>>>
>>> Hi,
>>>
>>> John Stultz <john.stultz@linaro.org> writes:
>>>> Basically when plugging in various cables in different orders, I'm
>>>> occasionally seeing the following BUG splat:
>>>>
>>>> [   86.215403] BUG: scheduling while atomic: kworker/u16:2/53/0x00000002
>>>> [   86.219164] usb 1-1: USB disconnect, device number 9
>>>> [   86.226845] Preemption disabled at:[   86.230218]
>>>> [<ffffff8008673558>] dwc2_conn_id_status_change+0x120/0x250
>>>> [   86.236894] CPU: 0 PID: 53 Comm: kworker/u16:2 Tainted: G        W
>>>>     4.9.0-rc8-00051-gd5a7979-dirty #1702
>>>> [   86.246836] Hardware name: HiKey Development Board (DT)
>>>> [   86.252100] Workqueue: dwc2 dwc2_conn_id_status_change
>>>> [   86.257279] Call trace:
>>>> [   86.259771] [<ffffff8008087c28>] dump_backtrace+0x0/0x1a0
>>>> [   86.265210] [<ffffff8008087ddc>] show_stack+0x14/0x20
>>>> [   86.270308] [<ffffff80084343f0>] dump_stack+0x90/0xb0
>>>> [   86.275401] [<ffffff80080d8d94>] __schedule_bug+0x6c/0xb8
>>>> [   86.280841] [<ffffff8008a07220>] __schedule+0x4f8/0x5b0
>>>> [   86.286099] [<ffffff8008a073e8>] schedule+0x38/0xa0
>>>> [   86.291017] [<ffffff8008a0a6cc>] schedule_hrtimeout_range_clock+0x8c/0xf0
>>>> [   86.297846] [<ffffff8008a0a740>] schedule_hrtimeout_range+0x10/0x18
>>>> [   86.304150] [<ffffff8008a0a4a0>] usleep_range+0x50/0x58
>>>> [   86.309418] [<ffffff800866d8dc>] dwc2_wait_for_mode.isra.4+0x54/0xd0
>>>> [   86.315815] [<ffffff800866f058>] dwc2_core_reset+0xe0/0x168
>>>> [   86.321431] [<ffffff800867e364>] dwc2_hsotg_core_init_disconnected+0x2c/0x310
>>>> [   86.328602] [<ffffff8008673568>] dwc2_conn_id_status_change+0x130/0x250
>>>> [   86.335254] [<ffffff80080ccd48>] process_one_work+0x118/0x370
>>>> [   86.341035] [<ffffff80080ccfe8>] worker_thread+0x48/0x498
>>>> [   86.346473] [<ffffff80080d2eb0>] kthread+0xd0/0xe8
>>>> [   86.351299] [<ffffff8008082e80>] ret_from_fork+0x10/0x50
>>>>
>>>> This seems to be caused by the dwc2_wait_for_mode() calling
>>>> usleep_range() while the hstog->lock spinlock is held, since
>>>> we take that before calling dwc2_hsotg_core_init_disconnected().
>>>>
>>>> This patch avoids the issue by adding an extra argument to
>>>> dwc2_core_reset(), as suggested by John Youn, which allows us to
>>>> skip the waiting, which should be unnecessary when calling from
>>>> dwc2_hsotg_core_init_disconnected().
>>>>
>>>> Cc: Wei Xu <xuwei5@hisilicon.com>
>>>> Cc: Guodong Xu <guodong.xu@linaro.org>
>>>> Cc: Amit Pundir <amit.pundir@linaro.org>
>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>> Cc: John Youn <johnyoun@synopsys.com>
>>>> Cc: Douglas Anderson <dianders@chromium.org>
>>>> Cc: Chen Yu <chenyu56@huawei.com>
>>>> Cc: Vardan Mikayelyan <mvardan@synopsys.com>
>>>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>>>> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: linux-usb@vger.kernel.org
>>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>>>> ---
>>>
>>> doesn't apply to my testing/next. Please rebase
>>
>> So these were rebased onto JohnY's tree here:
>>  https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_synopsys-2Dusb_linux.git&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=eBJTBuJyU21iKJvHdy5FxqtxsVARo0iIqJGxMHrlbyQ&s=VIiAT32aG7s04G5NoOOthNdm2JX0eWjJpg62neY_-KI&e=  next
>>
>> And apparently have been merged there. I suspect he's going to submit
>> his entire tree there to you?
>>
>> JohnY: Is this right?
>
>
> Yeah I'll get these issues sorted out with Felipe. Which may mean resubmitting everything the the proper order.

Ok. Thanks for sorting this out. Please let me know if there's
anything else you need from me!

thanks again!
-john