Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1557029 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board (rebased to synopsys-usb/next) John Stultz <john.stultz@linaro.org> - 2017-01-12 01:30 +0100
    [PATCH 4/4 v3] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220 John Stultz <john.stultz@linaro.org> - 2017-01-12 01:30 +0100
    [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock John Stultz <john.stultz@linaro.org> - 2017-01-12 01:30 +0100
      Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock Felipe Balbi <felipe.balbi@linux.intel.com> - 2017-01-16 11:40 +0100
        Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock John Stultz <john.stultz@linaro.org> - 2017-01-16 21:40 +0100
          Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding  hsotg->lock John Youn <John.Youn@synopsys.com> - 2017-01-16 22:00 +0100
            Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock John Stultz <john.stultz@linaro.org> - 2017-01-17 00:40 +0100
    Re: [PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board  (rebased to synopsys-usb/next) John Youn <John.Youn@synopsys.com> - 2017-01-12 03:10 +0100
      Re: [PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board (rebased to synopsys-usb/next) Felipe Balbi <felipe.balbi@linux.intel.com> - 2017-01-12 09:10 +0100
        Re: [PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board  (rebased to synopsys-usb/next) John Youn <John.Youn@synopsys.com> - 2017-01-12 20:30 +0100
        Re: [PATCH 0/4 v3] Fixes and workarounds for dwc2 on HiKey board  (rebased to synopsys-usb/next) John Stultz <john.stultz@linaro.org> - 2017-01-12 20:40 +0100

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

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

[toc] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

> 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

[toc] | [prev] | [next] | [standalone]

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

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/0xd
0
>>>> [   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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1557167

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web