All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: John Youn <John.Youn@synopsys.com>,
	balbi@ti.com, kever.yang@rock-chips.com
Cc: "Heiko Stübner" <heiko@sntech.de>,
	linux-rockchip@lists.infradead.org,
	"Julius Werner" <jwerner@chromium.org>,
	gregory.herrero@intel.com, yousaf.kaukab@intel.com,
	dinguyen@opensource.altera.com, stern@rowland.harvard.edu,
	ming.lei@canonical.com,
	"Douglas Anderson" <dianders@chromium.org>,
	"Yunzhi Li" <lyz@rock-chips.com>,
	johnyoun@synopsys.com, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v4 07/21] usb: dwc2: hcd: fix split transfer schedule sequence
Date: Wed, 20 Jan 2016 21:04:18 -0800	[thread overview]
Message-ID: <1453352672-27890-8-git-send-email-dianders@chromium.org> (raw)
In-Reply-To: <1453352672-27890-1-git-send-email-dianders@chromium.org>

We're supposed to keep outstanding splits in order.  Keep track of a
list of the order of splits and process channel interrupts in that
order.

Without this change and the following setup:
* Rockchip rk3288 Chromebook, using port ff540000
  -> Pluggable 7-port Hub with Charging (powered)
     -> Microsoft Wireless Keyboard 2000 in port 1.
     -> Das Keyboard in port 2.

...I find that I get dropped keys on the Microsoft keyboard (I'm sure
there are other combinations that fail, but this documents my test).
Specifically I've been typing "hahahahahahaha" on the keyboard and often
see keys dropped or repeated.

After this change the above setup works properly.  This patch is based
on a previous patch proposed by Yunzhi Li ("usb: dwc2: hcd: fix periodic
transfer schedule sequence")

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Yunzhi Li <lyz@rock-chips.com>
---
Changes in v4:
- fix split transfer schedule sequence new for v4.

Changes in v3: None
Changes in v2: None

 drivers/usb/dwc2/core.c     |  6 ++++++
 drivers/usb/dwc2/core.h     |  2 ++
 drivers/usb/dwc2/hcd.c      |  3 +++
 drivers/usb/dwc2/hcd.h      |  2 ++
 drivers/usb/dwc2/hcd_intr.c | 17 +++++++++++++++++
 5 files changed, 30 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 73f2771b7740..29f4bfe5970f 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -1676,6 +1676,8 @@ void dwc2_hc_cleanup(struct dwc2_hsotg *hsotg, struct dwc2_host_chan *chan)
 
 	chan->xfer_started = 0;
 
+	list_del_init(&chan->split_order_list_entry);
+
 	/*
 	 * Clear channel interrupt enables and any unhandled channel interrupt
 	 * conditions
@@ -1868,6 +1870,10 @@ void dwc2_hc_start_transfer(struct dwc2_hsotg *hsotg,
 			ec_mc = 3;
 		else
 			ec_mc = 1;
+
+		/* Put ourselves on the list to keep order straight */
+		list_move_tail(&chan->split_order_list_entry,
+			       &hsotg->split_order);
 	} else {
 		if (dbg_hc(chan))
 			dev_vdbg(hsotg->dev, "no split\n");
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 7fb6434f4639..538cf38af0e4 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -657,6 +657,7 @@ struct dwc2_hregs_backup {
  *                      periodic_sched_ready because it must be rescheduled for
  *                      the next frame. Otherwise, the item moves to
  *                      periodic_sched_inactive.
+ * @split_order:        List keeping track of channels doing splits, in order.
  * @periodic_usecs:     Total bandwidth claimed so far for periodic transfers.
  *                      This value is in microseconds per (micro)frame. The
  *                      assumption is that all periodic transfers may occur in
@@ -780,6 +781,7 @@ struct dwc2_hsotg {
 	struct list_head periodic_sched_ready;
 	struct list_head periodic_sched_assigned;
 	struct list_head periodic_sched_queued;
+	struct list_head split_order;
 	u16 periodic_usecs;
 	u16 frame_usecs[8];
 	u16 frame_number;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index d2daaea88d91..d2356d52ac7b 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3151,6 +3151,8 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 	INIT_LIST_HEAD(&hsotg->periodic_sched_assigned);
 	INIT_LIST_HEAD(&hsotg->periodic_sched_queued);
 
+	INIT_LIST_HEAD(&hsotg->split_order);
+
 	/*
 	 * Create a host channel descriptor for each host channel implemented
 	 * in the controller. Initialize the channel descriptor array.
@@ -3164,6 +3166,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 		if (channel == NULL)
 			goto error3;
 		channel->hc_num = i;
+		INIT_LIST_HEAD(&channel->split_order_list_entry);
 		hsotg->hc_ptr_array[i] = channel;
 	}
 
diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index 42f2e4e233da..1b46e2e617cc 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -106,6 +106,7 @@ struct dwc2_qh;
  * @hc_list_entry:      For linking to list of host channels
  * @desc_list_addr:     Current QH's descriptor list DMA address
  * @desc_list_sz:       Current QH's descriptor list size
+ * @split_order_list_entry: List entry for keeping track of the order of splits
  *
  * This structure represents the state of a single host channel when acting in
  * host mode. It contains the data items needed to transfer packets to an
@@ -158,6 +159,7 @@ struct dwc2_host_chan {
 	struct list_head hc_list_entry;
 	dma_addr_t desc_list_addr;
 	u32 desc_list_sz;
+	struct list_head split_order_list_entry;
 };
 
 struct dwc2_hcd_pipe_info {
diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index 2c521c00e5e0..577c91096a51 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -2067,6 +2067,7 @@ static void dwc2_hc_intr(struct dwc2_hsotg *hsotg)
 {
 	u32 haint;
 	int i;
+	struct dwc2_host_chan *chan, *chan_tmp;
 
 	haint = dwc2_readl(hsotg->regs + HAINT);
 	if (dbg_perio()) {
@@ -2075,6 +2076,22 @@ static void dwc2_hc_intr(struct dwc2_hsotg *hsotg)
 		dev_vdbg(hsotg->dev, "HAINT=%08x\n", haint);
 	}
 
+	/*
+	 * According to USB 2.0 spec section 11.18.8, a host must
+	 * issue complete-split transactions in a microframe for a
+	 * set of full-/low-speed endpoints in the same relative
+	 * order as the start-splits were issued in a microframe for.
+	 */
+	list_for_each_entry_safe(chan, chan_tmp, &hsotg->split_order,
+				 split_order_list_entry) {
+		int hc_num = chan->hc_num;
+
+		if (haint & (1 << hc_num)) {
+			dwc2_hc_n_intr(hsotg, hc_num);
+			haint &= ~(1 << hc_num);
+		}
+	}
+
 	for (i = 0; i < hsotg->core_params->host_channels; i++) {
 		if (haint & (1 << i))
 			dwc2_hc_n_intr(hsotg, i);
-- 
2.7.0.rc3.207.g0ac5344

WARNING: multiple messages have this Message-ID (diff)
From: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: John Youn <John.Youn-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>,
	balbi-l0cyMroinI0@public.gmane.org,
	kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org
Cc: gregory.herrero-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	"Heiko Stübner" <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>,
	johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"Douglas Anderson"
	<dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	yousaf.kaukab-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org,
	"Yunzhi Li" <lyz-TNX95d0MmH7DzftRWevZcw@public.gmane.org>,
	"Julius Werner" <jwerner-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	dinguyen-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org
Subject: [PATCH v4 07/21] usb: dwc2: hcd: fix split transfer schedule sequence
Date: Wed, 20 Jan 2016 21:04:18 -0800	[thread overview]
Message-ID: <1453352672-27890-8-git-send-email-dianders@chromium.org> (raw)
In-Reply-To: <1453352672-27890-1-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

We're supposed to keep outstanding splits in order.  Keep track of a
list of the order of splits and process channel interrupts in that
order.

Without this change and the following setup:
* Rockchip rk3288 Chromebook, using port ff540000
  -> Pluggable 7-port Hub with Charging (powered)
     -> Microsoft Wireless Keyboard 2000 in port 1.
     -> Das Keyboard in port 2.

...I find that I get dropped keys on the Microsoft keyboard (I'm sure
there are other combinations that fail, but this documents my test).
Specifically I've been typing "hahahahahahaha" on the keyboard and often
see keys dropped or repeated.

After this change the above setup works properly.  This patch is based
on a previous patch proposed by Yunzhi Li ("usb: dwc2: hcd: fix periodic
transfer schedule sequence")

Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Yunzhi Li <lyz-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
Changes in v4:
- fix split transfer schedule sequence new for v4.

Changes in v3: None
Changes in v2: None

 drivers/usb/dwc2/core.c     |  6 ++++++
 drivers/usb/dwc2/core.h     |  2 ++
 drivers/usb/dwc2/hcd.c      |  3 +++
 drivers/usb/dwc2/hcd.h      |  2 ++
 drivers/usb/dwc2/hcd_intr.c | 17 +++++++++++++++++
 5 files changed, 30 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 73f2771b7740..29f4bfe5970f 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -1676,6 +1676,8 @@ void dwc2_hc_cleanup(struct dwc2_hsotg *hsotg, struct dwc2_host_chan *chan)
 
 	chan->xfer_started = 0;
 
+	list_del_init(&chan->split_order_list_entry);
+
 	/*
 	 * Clear channel interrupt enables and any unhandled channel interrupt
 	 * conditions
@@ -1868,6 +1870,10 @@ void dwc2_hc_start_transfer(struct dwc2_hsotg *hsotg,
 			ec_mc = 3;
 		else
 			ec_mc = 1;
+
+		/* Put ourselves on the list to keep order straight */
+		list_move_tail(&chan->split_order_list_entry,
+			       &hsotg->split_order);
 	} else {
 		if (dbg_hc(chan))
 			dev_vdbg(hsotg->dev, "no split\n");
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 7fb6434f4639..538cf38af0e4 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -657,6 +657,7 @@ struct dwc2_hregs_backup {
  *                      periodic_sched_ready because it must be rescheduled for
  *                      the next frame. Otherwise, the item moves to
  *                      periodic_sched_inactive.
+ * @split_order:        List keeping track of channels doing splits, in order.
  * @periodic_usecs:     Total bandwidth claimed so far for periodic transfers.
  *                      This value is in microseconds per (micro)frame. The
  *                      assumption is that all periodic transfers may occur in
@@ -780,6 +781,7 @@ struct dwc2_hsotg {
 	struct list_head periodic_sched_ready;
 	struct list_head periodic_sched_assigned;
 	struct list_head periodic_sched_queued;
+	struct list_head split_order;
 	u16 periodic_usecs;
 	u16 frame_usecs[8];
 	u16 frame_number;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index d2daaea88d91..d2356d52ac7b 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3151,6 +3151,8 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 	INIT_LIST_HEAD(&hsotg->periodic_sched_assigned);
 	INIT_LIST_HEAD(&hsotg->periodic_sched_queued);
 
+	INIT_LIST_HEAD(&hsotg->split_order);
+
 	/*
 	 * Create a host channel descriptor for each host channel implemented
 	 * in the controller. Initialize the channel descriptor array.
@@ -3164,6 +3166,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 		if (channel == NULL)
 			goto error3;
 		channel->hc_num = i;
+		INIT_LIST_HEAD(&channel->split_order_list_entry);
 		hsotg->hc_ptr_array[i] = channel;
 	}
 
diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index 42f2e4e233da..1b46e2e617cc 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -106,6 +106,7 @@ struct dwc2_qh;
  * @hc_list_entry:      For linking to list of host channels
  * @desc_list_addr:     Current QH's descriptor list DMA address
  * @desc_list_sz:       Current QH's descriptor list size
+ * @split_order_list_entry: List entry for keeping track of the order of splits
  *
  * This structure represents the state of a single host channel when acting in
  * host mode. It contains the data items needed to transfer packets to an
@@ -158,6 +159,7 @@ struct dwc2_host_chan {
 	struct list_head hc_list_entry;
 	dma_addr_t desc_list_addr;
 	u32 desc_list_sz;
+	struct list_head split_order_list_entry;
 };
 
 struct dwc2_hcd_pipe_info {
diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index 2c521c00e5e0..577c91096a51 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -2067,6 +2067,7 @@ static void dwc2_hc_intr(struct dwc2_hsotg *hsotg)
 {
 	u32 haint;
 	int i;
+	struct dwc2_host_chan *chan, *chan_tmp;
 
 	haint = dwc2_readl(hsotg->regs + HAINT);
 	if (dbg_perio()) {
@@ -2075,6 +2076,22 @@ static void dwc2_hc_intr(struct dwc2_hsotg *hsotg)
 		dev_vdbg(hsotg->dev, "HAINT=%08x\n", haint);
 	}
 
+	/*
+	 * According to USB 2.0 spec section 11.18.8, a host must
+	 * issue complete-split transactions in a microframe for a
+	 * set of full-/low-speed endpoints in the same relative
+	 * order as the start-splits were issued in a microframe for.
+	 */
+	list_for_each_entry_safe(chan, chan_tmp, &hsotg->split_order,
+				 split_order_list_entry) {
+		int hc_num = chan->hc_num;
+
+		if (haint & (1 << hc_num)) {
+			dwc2_hc_n_intr(hsotg, hc_num);
+			haint &= ~(1 << hc_num);
+		}
+	}
+
 	for (i = 0; i < hsotg->core_params->host_channels; i++) {
 		if (haint & (1 << i))
 			dwc2_hc_n_intr(hsotg, i);
-- 
2.7.0.rc3.207.g0ac5344

  parent reply	other threads:[~2016-01-21  5:10 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-21  5:04 [PATCH v4 0/21] usb: dwc2: host: Fix and speed up all the stuff, especially with splits Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 01/21] usb: dwc2: rockchip: Make the max_transfer_size automatic Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 02/21] usb: dwc2: host: Get aligned DMA in a more supported way Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 03/21] usb: dwc2: host: Set host_rx_fifo_size to 528 for rk3066 Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 04/21] usb: dwc2: host: Set host_perio_tx_fifo_size to 304 " Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 05/21] usb: dwc2: host: Avoid use of chan->qh after qh freed Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 06/21] usb: dwc2: host: Always add to the tail of queues Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` Douglas Anderson [this message]
2016-01-21  5:04   ` [PATCH v4 07/21] usb: dwc2: hcd: fix split transfer schedule sequence Douglas Anderson
2016-01-21  5:24   ` kbuild test robot
2016-01-21  5:24     ` kbuild test robot
2016-01-22  0:24     ` Doug Anderson
2016-01-21  5:04 ` [PATCH v4 08/21] usb: dwc2: host: Add scheduler tracing Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 09/21] usb: dwc2: host: Add a delay before releasing periodic bandwidth Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 10/21] usb: dwc2: host: Giveback URB in tasklet context Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 11/21] usb: dwc2: host: Use periodic interrupt even with DMA Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 12/21] usb: dwc2: host: Rename some fields in struct dwc2_qh Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 13/21] usb: dwc2: host: Reorder things in hcd_queue.c Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 14/21] usb: dwc2: host: Split code out to make dwc2_do_reserve() Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 15/21] usb: dwc2: host: Add scheduler logging for missed SOFs Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 16/21] usb: dwc2: host: Manage frame nums better in scheduler Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 17/21] usb: dwc2: host: Schedule periodic right away if it's time Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 18/21] usb: dwc2: host: Add dwc2_hcd_get_future_frame_number() call Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 19/21] usb: dwc2: host: Properly set even/odd frame Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:04 ` [PATCH v4 20/21] usb: dwc2: host: Totally redo the microframe scheduler Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson
2016-01-21  5:49   ` kbuild test robot
2016-01-21  5:49     ` kbuild test robot
2016-01-21  5:04 ` [PATCH v4 21/21] usb: dwc2: host: If using uframe scheduler, end splits better Douglas Anderson
2016-01-21  5:04   ` Douglas Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1453352672-27890-8-git-send-email-dianders@chromium.org \
    --to=dianders@chromium.org \
    --cc=John.Youn@synopsys.com \
    --cc=balbi@ti.com \
    --cc=dinguyen@opensource.altera.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=gregory.herrero@intel.com \
    --cc=heiko@sntech.de \
    --cc=johnyoun@synopsys.com \
    --cc=jwerner@chromium.org \
    --cc=kever.yang@rock-chips.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=lyz@rock-chips.com \
    --cc=ming.lei@canonical.com \
    --cc=stern@rowland.harvard.edu \
    --cc=yousaf.kaukab@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.