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

Groups > linux.kernel > #1313879 > unrolled thread

[PATCH v4 0/21] usb: dwc2: host: Fix and speed up all the stuff, especially with splits

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/21] usb: dwc2: host: Fix and speed up all the stuff, especially with splits Douglas Anderson <dianders@chromium.org> - 2016-01-21 06:10 +0100
    [PATCH v4 11/21] usb: dwc2: host: Use periodic interrupt even with DMA Douglas Anderson <dianders@chromium.org> - 2016-01-21 06:10 +0100
    [PATCH v4 15/21] usb: dwc2: host: Add scheduler logging for missed SOFs Douglas Anderson <dianders@chromium.org> - 2016-01-21 06:10 +0100
    [PATCH v4 04/21] usb: dwc2: host: Set host_perio_tx_fifo_size to 304 for rk3066 Douglas Anderson <dianders@chromium.org> - 2016-01-21 06:20 +0100
    [PATCH v4 07/21] usb: dwc2: hcd: fix split transfer schedule sequence Douglas Anderson <dianders@chromium.org> - 2016-01-21 06:20 +0100
      Re: [PATCH v4 07/21] usb: dwc2: hcd: fix split transfer schedule  sequence kbuild test robot <lkp@intel.com> - 2016-01-21 06:30 +0100
        Re: [PATCH v4 07/21] usb: dwc2: hcd: fix split transfer schedule sequence Doug Anderson <dianders@chromium.org> - 2016-01-22 01:30 +0100
    [PATCH v4 09/21] usb: dwc2: host: Add a delay before releasing periodic bandwidth Douglas Anderson <dianders@chromium.org> - 2016-01-21 06:20 +0100
    [PATCH v4 03/21] usb: dwc2: host: Set host_rx_fifo_size to 528 for rk3066 Douglas Anderson <dianders@chromium.org> - 2016-01-21 06:20 +0100
    [PATCH v4 02/21] usb: dwc2: host: Get aligned DMA in a more supported way Douglas Anderson <dianders@chromium.org> - 2016-01-21 06:20 +0100
    [PATCH v4 08/21] usb: dwc2: host: Add scheduler tracing Douglas Anderson <dianders@chromium.org> - 2016-01-21 06:20 +0100

#1313879 — [PATCH v4 0/21] usb: dwc2: host: Fix and speed up all the stuff, especially with splits

This is a bit of catchall series for all the bug fix and performance
patches I've been working on over the last few months.  Note that for
dwc2 we need to do LOTS in software and need super low interrupt
latency, so most performance improvements actually fix real bugs.

Patches are structured to start with no-brainer stuff that could be
applied ASAP, especially things I've already gotten Acks for.  Things
get slightly more RFC / RFT like as we get farther down the series.
Anything that can be landed sooner rather than later (especially those
Acked long ago) would help in re-posts (I'm not biased, of course).

It's been a few months since my last post of this series.  In the
meantime I've added a bunch of small bugfixes to the start of it and
also TOTALLY REWROTE the microframe scheduler.  I'll say up front: I
know nothing about USB.  I haven't read the whole spec.  I'm not
terribly familiar with the OHCI, EHCI, and XHCI drivers in the kernel.
...and I'm pretty clueless overall.  Nevertheless, I've attempted to
write up a fancy scheduler based on the portion of the spec talking
about microframe scheduling requirements.  This rewritten scheduler does
seem to help when I start jamming lots of USB things into a hub, so
presumably the code is a reasonably starting point.  Given my current
understanding of USB the old code was fairly insane, so presumably even
if my new patch isn't perfect it's better than what we had.

Anyway, on to the patches:

1. usb: dwc2: rockchip: Make the max_transfer_size automatic

   No brainer.  Can land any time.

2. usb: dwc2: host: Get aligned DMA in a more supported way

   Although this touches a lot of code, it's mostly just deleting
   stuff.  The way this is working is nearly the same as tegra.  Biggest
   objection I expect is that it has too much duplication with tegra and
   musb.  I'd personally prefer to land it now and remove duplication
   later, but up to others.  Speeding up interrupt handler helps with
   SOF scheduling, so this is not just a dumb optimization.

3. usb: dwc2: host: Set host_rx_fifo_size to 528 for rk3066
4. usb: dwc2: host: Set host_perio_tx_fifo_size to 304 for rk3066

   Seems like a good idea and small impact, but if someone hates it or
   it breaks on some Rockchip SoC, just drop it.  I've only tested on
   rk3288 so it would be nice if someone with access to more Rockchip
   SoCs can give a tested by.

5. usb: dwc2: host: Avoid use of chan->qh after qh freed

   Simple bugfix.  Unrelated to the series but thrown in here.

6. usb: dwc2: host: Always add to the tail of queues

   Big functionality improvement.  Small patch.  Suggest applying ASAP.

7. usb: dwc2: hcd: fix split transfer schedule sequence

   Unless I'm misunderstanding, this should be a no-brainer to fix.
   Could be some bikeshedding on how to fix this.  Let me know if/how
   you want me to spin.  Otherwise I'd say land it and it will fix a
   bunch of stuff.

8. usb: dwc2: host: Add scheduler tracing

   Shouldn't hurt anything.  If you have bikesheds, let me know.

9.  usb: dwc2: host: Add a delay before releasing periodic bandwidth
10. usb: dwc2: host: Giveback URB in tasklet context

   I think we should take these.  They improve things a bunch and I have
   found no regressions due to them.  Additional testing appreciated, of
   course.

11. usb: dwc2: host: Use periodic interrupt even with DMA

   Just came up with this one recently so it's had slightly less
   testing.  ...but it certainly fixed a bunch of stuff.  Could probably
   be moved around in the series to be pretty much anywhere.  I don't
   think this has a huge impact until we fix the scheduler (below) but
   at the same time I'm pretty sure it's something that's been wrong for
   a long time.

12. usb: dwc2: host: Rename some fields in struct dwc2_qh
13. usb: dwc2: host: Reorder things in hcd_queue.c
14. usb: dwc2: host: Split code out to make dwc2_do_reserve()

   Cleanups to make future patches easier to understand.  Bikeshed away.
   All no-op changes.

15. usb: dwc2: host: Add scheduler logging for missed SOFs

   I found this to be quite helpful.  If you hate it, drop it from the
   series.

16. usb: dwc2: host: Manage frame nums better in scheduler

   Doesn't totally make sense on its own, but a good halfway point to
   the microframe scheduler.  ...and shouldn't regress anything.  Allows
   us to do the "Properly set even/odd frame" patch below which
   definitely improves things.

17. usb: dwc2: host: Schedule periodic right away if it's time

   Yet another small change to make scheduling tighter.

18. usb: dwc2: host: Add dwc2_hcd_get_future_frame_number() call

   Prep for ("usb: dwc2: host: Properly set even/odd frame")

19. usb: dwc2: host: Properly set even/odd frame

   Helps quite a bit.  Helps even more after the redone microframe
   scheduler.  Feel free to tidy up if you see easy ways to do this.
   Maybe someone has a better way to estimate time on the wire?

20. usb: dwc2: host: Totally redo the microframe scheduler

   Eyeballs please!  I think I've stared at this too much and now my
   eyes are glazing over.  This definitely helps but also probably needs
   a few more spins?  Of course, if nobody wants to review it, IMHO
   checking it in as-is is WAAAAY better than what we had before.

21. usb: dwc2: host: If using uframe scheduler, end splits better

   Low confidence in this one.  Worry that it will end something too
   soon, but haven't seen it yet.

===

Below is discussion of some of the speedup stuff (mostly relevant to the
first few patches).

===

The dwc2 interrupt handler is quite slow.  On rk3288 with a few things
plugged into the ports and with cpufreq locked at 696MHz (to simulate
real world idle system), I can easily observe dwc2_handle_hcd_intr()
taking > 120 us, sometimes > 150 us.  Note that SOF interrupts come
every 125 us with high speed USB, so taking > 120 us in the interrupt
handler is a big deal.

The patches here will speed up the interrupt controller significantly.
After this series, I have a hard time seeing the interrupt controller
taking > 20 us and I don't ever see it taking > 30 us ever in my tests
unless I bring the cpufreq back down.  With the cpufreq at 126 MHz I can
still see the interrupt handler take > 50 us, so I'm sure we could
improve this further.  ...but hey, it's a start.

This series also shows big speed improvements when testing with a USB
Gigabit Ethernet adapter.  Previously the tested adapter would top out
at about 15MB/s.  After these changes it gets about 23MB/s.

In addition to the speedup, this series also has the advantage of
simplifying dwc2 and making it more like everyone else (introducing the
possibility of future simplifications).  Picking this series up will
help your diffstat and likely win you friends.  ;)

===

Steps for gathering data with ftrace (for some reason I have to run
twice):

cd /sys/devices/system/cpu/cpu0/cpufreq/
echo userspace > scaling_governor
echo 696000 > scaling_setspeed

cd /sys/kernel/debug/tracing
echo 0 > tracing_on
echo "" > trace
echo nop > current_tracer
echo function_graph > current_tracer
echo dwc2_handle_hcd_intr > set_graph_function
echo dwc2_handle_common_intr >> set_graph_function
echo dwc2_handle_hcd_intr > set_ftrace_filter
echo dwc2_handle_common_intr >> set_ftrace_filter
echo funcgraph-abstime > trace_options
echo 70 > tracing_thresh
echo 1 > /sys/kernel/debug/tracing/tracing_on

sleep 2
cat trace

Changes in v4:
- Add John's Acks from <https://patchwork.kernel.org/patch/7631551>
- Set host_rx_fifo_size to 528 for rk3066 new for v4.
- Set host_perio_tx_fifo_size to 304 for rk3066 new for v4.
- Avoid use of chan->qh after qh freed new for v4.
- Always add to the tail of queues new for v4.
- fix split transfer schedule sequence new for v4.
- Retooled scheduler tracing a bit, so left off John's Ack from v3.
- Moved periodic bandwidth release delay patch earlier again.
- A bit earlier in the list of patches than in v3.
- Use periodic interrupt even with DMA new for v4.
- Rename some fields in struct dwc2_qh new for v4.
- Reorder things in hcd_queue.c new for v4.
- Split code out to make dwc2_do_reserve() new for v4.
- Add scheduler logging for missed SOFs new for v4.
- Manage frame nums better in scheduler new for v4.
- Schedule periodic right away if it's time new for v4.
- Add dwc2_hcd_get_future_frame_number() call new for v4.
- Properly set even/odd frame new for v4.
- Figured out what the microframe scheduler was supposed to do.
- Microframe rewrite is totally different from v3, hopefully more right.
- Microframe rewrite is later in the series now.
- If using uframe scheduler, end splits better new for v4.

Changes in v3:
- Moved periodic bandwidth release delay patch later in the series.
- The uframe scheduler patch is folded into optimization series.
- Optimize uframe scheduler "single uframe" case a little.
- uframe scheduler now atop logging patches.
- uframe scheduler now before delayed bandwidth release patches.
- Add defines like EARLY_FRAME_USEC
- Reorder dwc2_deschedule_periodic() in prep for future patches.
- uframe scheduler now shows real usefulness w/ future patches!
- Assuming single_tt is new for v3; not terribly well tested (yet).
- Keep track and use our uframe new for v3.

Changes in v2:
- Add a warn if setup_dma is not aligned (Julius Werner).
- Periodic bandwidth release delay new for V2
- Commit message now says that URB giveback change needs delay change.
- Totally rewrote uframe scheduler again after writing test code.
- uframe scheduler atop delayed bandwidth release patches.

Douglas Anderson (21):
  usb: dwc2: rockchip: Make the max_transfer_size automatic
  usb: dwc2: host: Get aligned DMA in a more supported way
  usb: dwc2: host: Set host_rx_fifo_size to 528 for rk3066
  usb: dwc2: host: Set host_perio_tx_fifo_size to 304 for rk3066
  usb: dwc2: host: Avoid use of chan->qh after qh freed
  usb: dwc2: host: Always add to the tail of queues
  usb: dwc2: hcd: fix split transfer schedule sequence
  usb: dwc2: host: Add scheduler tracing
  usb: dwc2: host: Add a delay before releasing periodic bandwidth
  usb: dwc2: host: Giveback URB in tasklet context
  usb: dwc2: host: Use periodic interrupt even with DMA
  usb: dwc2: host: Rename some fields in struct dwc2_qh
  usb: dwc2: host: Reorder things in hcd_queue.c
  usb: dwc2: host: Split code out to make dwc2_do_reserve()
  usb: dwc2: host: Add scheduler logging for missed SOFs
  usb: dwc2: host: Manage frame nums better in scheduler
  usb: dwc2: host: Schedule periodic right away if it's time
  usb: dwc2: host: Add dwc2_hcd_get_future_frame_number() call
  usb: dwc2: host: Properly set even/odd frame
  usb: dwc2: host: Totally redo the microframe scheduler
  usb: dwc2: host: If using uframe scheduler, end splits better

 drivers/usb/dwc2/core.c      |  119 ++-
 drivers/usb/dwc2/core.h      |  115 ++-
 drivers/usb/dwc2/hcd.c       |  385 +++++---
 drivers/usb/dwc2/hcd.h       |  126 ++-
 drivers/usb/dwc2/hcd_ddma.c  |   41 +-
 drivers/usb/dwc2/hcd_intr.c  |  164 ++--
 drivers/usb/dwc2/hcd_queue.c | 1995 ++++++++++++++++++++++++++++++++++--------
 drivers/usb/dwc2/platform.c  |    6 +-
 8 files changed, 2279 insertions(+), 672 deletions(-)

-- 
2.7.0.rc3.207.g0ac5344

[toc] | [next] | [standalone]

#1313880 — [PATCH v4 11/21] usb: dwc2: host: Use periodic interrupt even with DMA

The old code in dwc2_process_periodic_channels() would only enable the
"periodic empty" interrupt if we weren't using DMA.  That wasn't right
since we can still get into cases where we have small FIFOs even on
systems that have DMA (the rk3288 is a prime example).

Let's always enable/disable the "periodic empty" when appropriate.  As
part of this:

* Always call dwc2_process_periodic_channels() even if there's nothing
  in periodic_sched_assigned (we move the queue empty check so we still
  avoid the extra work).  That will make extra certain that we will
  properly disable the "periodic empty" interrupt even if there's
  nothing queued up.

* Move the enable of "periodic empty" due to non-empty
  periodic_sched_assigned to be for slave mode (non-DMA mode) only.
  Presumably this was the original intention of the check for DMA since
  it seems to match the comments above where in slave mode we leave
  things on the assigned queue.

Note that even before this change slave mode didn't work for me, so I
can't say for sure that my understanding of slave mode is correct.
However, this shouldn't change anything for slave mode so if slave mode
worked for someone in the past it ought to still work.

With this change, I no longer get constant misses reported by my other
debugging code (and with future patches) when I've got:
* 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.
     -> Jabra Speaker in port 3
     -> Logitech, Inc. Webcam C600 in port 4
     -> Microsoft Sidewinder X6 Keyboard in port 5

...and I'm playing music on the USB speaker and capturing video from the
webcam.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- Use periodic interrupt even with DMA new for v4.

Changes in v3: None
Changes in v2: None

 drivers/usb/dwc2/hcd.c | 71 +++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index fa684c68b8f6..350453999bad 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -1104,10 +1104,14 @@ static void dwc2_process_periodic_channels(struct dwc2_hsotg *hsotg)
 	u32 fspcavail;
 	u32 gintmsk;
 	int status;
-	int no_queue_space = 0;
-	int no_fifo_space = 0;
+	bool no_queue_space = false;
+	bool no_fifo_space = false;
 	u32 qspcavail;
 
+	/* If empty list then just adjust interrupt enables */
+	if (list_empty(&hsotg->periodic_sched_assigned))
+		goto exit;
+
 	if (dbg_perio())
 		dev_vdbg(hsotg->dev, "Queue periodic transactions\n");
 
@@ -1185,42 +1189,32 @@ static void dwc2_process_periodic_channels(struct dwc2_hsotg *hsotg)
 		}
 	}
 
-	if (hsotg->core_params->dma_enable <= 0) {
-		tx_status = dwc2_readl(hsotg->regs + HPTXSTS);
-		qspcavail = (tx_status & TXSTS_QSPCAVAIL_MASK) >>
-			    TXSTS_QSPCAVAIL_SHIFT;
-		fspcavail = (tx_status & TXSTS_FSPCAVAIL_MASK) >>
-			    TXSTS_FSPCAVAIL_SHIFT;
-		if (dbg_perio()) {
-			dev_vdbg(hsotg->dev,
-				 "  P Tx Req Queue Space Avail (after queue): %d\n",
-				 qspcavail);
-			dev_vdbg(hsotg->dev,
-				 "  P Tx FIFO Space Avail (after queue): %d\n",
-				 fspcavail);
-		}
-
-		if (!list_empty(&hsotg->periodic_sched_assigned) ||
-		    no_queue_space || no_fifo_space) {
-			/*
-			 * May need to queue more transactions as the request
-			 * queue or Tx FIFO empties. Enable the periodic Tx
-			 * FIFO empty interrupt. (Always use the half-empty
-			 * level to ensure that new requests are loaded as
-			 * soon as possible.)
-			 */
-			gintmsk = dwc2_readl(hsotg->regs + GINTMSK);
+exit:
+	if (no_queue_space || no_fifo_space ||
+	    (hsotg->core_params->dma_enable <= 0 &&
+	     !list_empty(&hsotg->periodic_sched_assigned))) {
+		/*
+		 * May need to queue more transactions as the request
+		 * queue or Tx FIFO empties. Enable the periodic Tx
+		 * FIFO empty interrupt. (Always use the half-empty
+		 * level to ensure that new requests are loaded as
+		 * soon as possible.)
+		 */
+		gintmsk = dwc2_readl(hsotg->regs + GINTMSK);
+		if (!(gintmsk & GINTSTS_PTXFEMP)) {
 			gintmsk |= GINTSTS_PTXFEMP;
 			dwc2_writel(gintmsk, hsotg->regs + GINTMSK);
-		} else {
-			/*
-			 * Disable the Tx FIFO empty interrupt since there are
-			 * no more transactions that need to be queued right
-			 * now. This function is called from interrupt
-			 * handlers to queue more transactions as transfer
-			 * states change.
-			 */
-			gintmsk = dwc2_readl(hsotg->regs + GINTMSK);
+		}
+	} else {
+		/*
+		 * Disable the Tx FIFO empty interrupt since there are
+		 * no more transactions that need to be queued right
+		 * now. This function is called from interrupt
+		 * handlers to queue more transactions as transfer
+		 * states change.
+		*/
+		gintmsk = dwc2_readl(hsotg->regs + GINTMSK);
+		if (gintmsk & GINTSTS_PTXFEMP) {
 			gintmsk &= ~GINTSTS_PTXFEMP;
 			dwc2_writel(gintmsk, hsotg->regs + GINTMSK);
 		}
@@ -1367,9 +1361,8 @@ void dwc2_hcd_queue_transactions(struct dwc2_hsotg *hsotg,
 	dev_vdbg(hsotg->dev, "Queue Transactions\n");
 #endif
 	/* Process host channels associated with periodic transfers */
-	if ((tr_type == DWC2_TRANSACTION_PERIODIC ||
-	     tr_type == DWC2_TRANSACTION_ALL) &&
-	    !list_empty(&hsotg->periodic_sched_assigned))
+	if (tr_type == DWC2_TRANSACTION_PERIODIC ||
+	    tr_type == DWC2_TRANSACTION_ALL)
 		dwc2_process_periodic_channels(hsotg);
 
 	/* Process host channels associated with non-periodic transfers */
-- 
2.7.0.rc3.207.g0ac5344

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

#1313881 — [PATCH v4 15/21] usb: dwc2: host: Add scheduler logging for missed SOFs

We'll use the new "scheduler verbose debugging" macro to log missed
SOFs.  This is fast enough (assuming you configure it to use the ftrace
buffer) that we can do it without worrying about the speed hit.  The
overhead hit if the scheduler tracing is set to "no_printk" should be
near zero.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- Add scheduler logging for missed SOFs new for v4.

Changes in v3: None
Changes in v2: None

 drivers/usb/dwc2/core.h     |  3 ++-
 drivers/usb/dwc2/hcd.c      |  2 +-
 drivers/usb/dwc2/hcd_intr.c | 12 ++++++++----
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 18f9e4045643..64d45a2053bb 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -809,9 +809,10 @@ struct dwc2_hsotg {
 	bool bus_suspended;
 	bool new_connection;
 
+	u16 last_frame_num;
+
 #ifdef CONFIG_USB_DWC2_TRACK_MISSED_SOFS
 #define FRAME_NUM_ARRAY_SIZE 1000
-	u16 last_frame_num;
 	u16 *frame_num_array;
 	u16 *last_frame_num_array;
 	int frame_num_idx;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 350453999bad..32c160e3e13e 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3077,8 +3077,8 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 			FRAME_NUM_ARRAY_SIZE, GFP_KERNEL);
 	if (!hsotg->last_frame_num_array)
 		goto error1;
-	hsotg->last_frame_num = HFNUM_MAX_FRNUM;
 #endif
+	hsotg->last_frame_num = HFNUM_MAX_FRNUM;
 
 	/* Check if the bus driver or platform code has setup a dma_mask */
 	if (hsotg->core_params->dma_enable > 0 &&
diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index b54a0c41d34f..1875e4590a3c 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -55,12 +55,16 @@
 /* This function is for debug only */
 static void dwc2_track_missed_sofs(struct dwc2_hsotg *hsotg)
 {
-#ifdef CONFIG_USB_DWC2_TRACK_MISSED_SOFS
 	u16 curr_frame_number = hsotg->frame_number;
+	u16 expected = dwc2_frame_num_inc(hsotg->last_frame_num, 1);
+
+	if (expected != curr_frame_number)
+		dwc2_sch_vdbg(hsotg, "MISSED SOF %04x != %04x\n",
+			expected, curr_frame_number);
 
+#ifdef CONFIG_USB_DWC2_TRACK_MISSED_SOFS
 	if (hsotg->frame_num_idx < FRAME_NUM_ARRAY_SIZE) {
-		if (((hsotg->last_frame_num + 1) & HFNUM_MAX_FRNUM) !=
-		    curr_frame_number) {
+		if (expected != curr_frame_number) {
 			hsotg->frame_num_array[hsotg->frame_num_idx] =
 					curr_frame_number;
 			hsotg->last_frame_num_array[hsotg->frame_num_idx] =
@@ -79,8 +83,8 @@ static void dwc2_track_missed_sofs(struct dwc2_hsotg *hsotg)
 		}
 		hsotg->dumped_frame_num_array = 1;
 	}
-	hsotg->last_frame_num = curr_frame_number;
 #endif
+	hsotg->last_frame_num = curr_frame_number;
 }
 
 static void dwc2_hc_handle_tt_clear(struct dwc2_hsotg *hsotg,
-- 
2.7.0.rc3.207.g0ac5344

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

#1313883 — [PATCH v4 04/21] usb: dwc2: host: Set host_perio_tx_fifo_size to 304 for rk3066

Looking at rk3288, there are two dwc2 ports.  One has 960
total_fifo_size and the other 972.

We're currently assigning 528 + 128 + 256 = 912.  That means we're
wasting 48 words on one port and 60 words on the other.  Since we have
one settings block for both ports, let's just eat the 48 words that we
know we have and assign them to what's probably the most
latency-senstive part of the system: periodic transfers.

Presumably other rk3288 parts using the rk3066 settings also have these
extra 48 words as well.

Note: ideally we'd adjust dwc2_calculate_dynamic_fifo() to handle this
more dynamically, but until that's done hardcoding things here seems OK.

NOTE: no known use cases are improved by this patch, but in my stressful
USB tests I certainly see my periodic FIFO filling up both before and
after this change.  Presumably there are some USB configurations where
the periodic FIFO would have been filled before where it no longer is.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- Set host_perio_tx_fifo_size to 304 for rk3066 new for v4.

Changes in v3: None
Changes in v2: None

 drivers/usb/dwc2/platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index b6d7666e715c..afdcdeee266d 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -128,7 +128,7 @@ static const struct dwc2_core_params params_rk3066 = {
 	.en_multiple_tx_fifo		= -1,
 	.host_rx_fifo_size		= 528,	/* 528 DWORDs */
 	.host_nperio_tx_fifo_size	= 128,	/* 128 DWORDs */
-	.host_perio_tx_fifo_size	= 256,	/* 256 DWORDs */
+	.host_perio_tx_fifo_size	= 304,	/* 304 DWORDs */
 	.max_transfer_size		= -1,
 	.max_packet_count		= -1,
 	.host_channels			= -1,
-- 
2.7.0.rc3.207.g0ac5344

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

#1313885 — [PATCH v4 07/21] usb: dwc2: hcd: fix split transfer schedule sequence

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

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

#1313901 — Re: [PATCH v4 07/21] usb: dwc2: hcd: fix split transfer schedule sequence

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

Hi Douglas,

[auto build test ERROR on next-20160120]
[cannot apply to v4.4-rc8 v4.4-rc7 v4.4-rc6 v4.4]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Douglas-Anderson/usb-dwc2-host-Fix-and-speed-up-all-the-stuff-especially-with-splits/20160121-131414
config: x86_64-randconfig-x019-01201142 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/usb/dwc2/core.c: In function 'dwc2_hc_start_transfer':
>> drivers/usb/dwc2/core.c:1876:17: error: 'struct dwc2_hsotg' has no member named 'split_order'
              &hsotg->split_order);
                    ^
--
>> /bin/bash: line 0: [: -ge: unary operator expected

vim +1876 drivers/usb/dwc2/core.c

  1870				ec_mc = 3;
  1871			else
  1872				ec_mc = 1;
  1873	
  1874			/* Put ourselves on the list to keep order straight */
  1875			list_move_tail(&chan->split_order_list_entry,
> 1876				       &hsotg->split_order);
  1877		} else {
  1878			if (dbg_hc(chan))
  1879				dev_vdbg(hsotg->dev, "no split\n");

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

#1314659 — Re: [PATCH v4 07/21] usb: dwc2: hcd: fix split transfer schedule sequence

Hi,

On Wed, Jan 20, 2016 at 9:24 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Douglas,
>
> [auto build test ERROR on next-20160120]
> [cannot apply to v4.4-rc8 v4.4-rc7 v4.4-rc6 v4.4]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url:    https://github.com/0day-ci/linux/commits/Douglas-Anderson/usb-dwc2-host-Fix-and-speed-up-all-the-stuff-especially-with-splits/20160121-131414
> config: x86_64-randconfig-x019-01201142 (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
>    drivers/usb/dwc2/core.c: In function 'dwc2_hc_start_transfer':
>>> drivers/usb/dwc2/core.c:1876:17: error: 'struct dwc2_hsotg' has no member named 'split_order'
>               &hsotg->split_order);
>                     ^
> --
>>> /bin/bash: line 0: [: -ge: unary operator expected
>
> vim +1876 drivers/usb/dwc2/core.c
>
>   1870                          ec_mc = 3;
>   1871                  else
>   1872                          ec_mc = 1;
>   1873
>   1874                  /* Put ourselves on the list to keep order straight */
>   1875                  list_move_tail(&chan->split_order_list_entry,
>> 1876                                 &hsotg->split_order);
>   1877          } else {
>   1878                  if (dbg_hc(chan))
>   1879                          dev_vdbg(hsotg->dev, "no split\n");
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

I've got a fix this and the other error reported by the bot.  I'll
plan to fix both tomorrow and send up a v5 unless someone gives me
some other feedback on the series that takes me longer than a day to
deal with.

-Doug

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

#1313888 — [PATCH v4 09/21] usb: dwc2: host: Add a delay before releasing periodic bandwidth

We'd like to be able to use HCD_BH in order to speed up the dwc2 host
interrupt handler quite a bit.  However, according to the kernel doc for
usb_submit_urb() (specifically the part about "Reserved Bandwidth
Transfers"), we need to keep a reservation active as long as a device
driver keeps submitting.  That was easy to do when we gave back the URB
in the interrupt context: we just looked at when our queue was empty and
released the reserved bandwidth then.  ...but now we need a little more
complexity.

We'll follow EHCI's lead in commit 9118f9eb4f1e ("USB: EHCI: improve
interrupt qh unlink") and add a 5ms delay.  Since we don't have a whole
timer infrastructure in dwc2, we'll just add a timer per QH.  The
overhead for this is very small.

Note that the dwc2 scheduler is pretty broken (see future patches to fix
it).  This patch attempts to replicate all old behavior and just add the
proper delay.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- Moved periodic bandwidth release delay patch earlier again.

Changes in v3:
- Moved periodic bandwidth release delay patch later in the series.

Changes in v2:
- Periodic bandwidth release delay new for V2

 drivers/usb/dwc2/hcd.h       |   6 ++
 drivers/usb/dwc2/hcd_queue.c | 237 +++++++++++++++++++++++++++++++++----------
 2 files changed, 187 insertions(+), 56 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index 809bc4ff9116..79473ea35bd6 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -215,6 +215,7 @@ enum dwc2_transaction_type {
 /**
  * struct dwc2_qh - Software queue head structure
  *
+ * @hsotg:              The HCD state structure for the DWC OTG controller
  * @ep_type:            Endpoint type. One of the following values:
  *                       - USB_ENDPOINT_XFER_CONTROL
  *                       - USB_ENDPOINT_XFER_BULK
@@ -252,13 +253,16 @@ enum dwc2_transaction_type {
  * @n_bytes:            Xfer Bytes array. Each element corresponds to a transfer
  *                      descriptor and indicates original XferSize value for the
  *                      descriptor
+ * @unreserve_timer:    Timer for releasing periodic reservation.
  * @tt_buffer_dirty     True if clear_tt_buffer_complete is pending
+ * @unreserve_pending:  True if we planned to unreserve but haven't yet.
  *
  * A Queue Head (QH) holds the static characteristics of an endpoint and
  * maintains a list of transfers (QTDs) for that endpoint. A QH structure may
  * be entered in either the non-periodic or periodic schedule.
  */
 struct dwc2_qh {
+	struct dwc2_hsotg *hsotg;
 	u8 ep_type;
 	u8 ep_is_in;
 	u16 maxp;
@@ -281,7 +285,9 @@ struct dwc2_qh {
 	dma_addr_t desc_list_dma;
 	u32 desc_list_sz;
 	u32 *n_bytes;
+	struct timer_list unreserve_timer;
 	unsigned tt_buffer_dirty:1;
+	unsigned unreserve_pending:1;
 };
 
 /**
diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c
index 0e9faa75593c..b9e4867e1afd 100644
--- a/drivers/usb/dwc2/hcd_queue.c
+++ b/drivers/usb/dwc2/hcd_queue.c
@@ -53,6 +53,94 @@
 #include "core.h"
 #include "hcd.h"
 
+/* Wait this long before releasing periodic reservation */
+#define DWC2_UNRESERVE_DELAY (msecs_to_jiffies(5))
+
+/**
+ * dwc2_do_unreserve() - Actually release the periodic reservation
+ *
+ * This function actually releases the periodic bandwidth that was reserved
+ * by the given qh.
+ *
+ * @hsotg: The HCD state structure for the DWC OTG controller
+ * @qh:    QH for the periodic transfer.
+ */
+static void dwc2_do_unreserve(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
+{
+	assert_spin_locked(&hsotg->lock);
+
+	WARN_ON(!qh->unreserve_pending);
+
+	/* No more unreserve pending--we're doing it */
+	qh->unreserve_pending = false;
+
+	if (WARN_ON(!list_empty(&qh->qh_list_entry)))
+		list_del_init(&qh->qh_list_entry);
+
+	/* Update claimed usecs per (micro)frame */
+	hsotg->periodic_usecs -= qh->usecs;
+
+	if (hsotg->core_params->uframe_sched > 0) {
+		int i;
+
+		for (i = 0; i < 8; i++) {
+			hsotg->frame_usecs[i] += qh->frame_usecs[i];
+			qh->frame_usecs[i] = 0;
+		}
+	} else {
+		/* Release periodic channel reservation */
+		hsotg->periodic_channels--;
+	}
+}
+
+/**
+ * dwc2_unreserve_timer_fn() - Timer function to release periodic reservation
+ *
+ * According to the kernel doc for usb_submit_urb() (specifically the part about
+ * "Reserved Bandwidth Transfers"), we need to keep a reservation active as
+ * long as a device driver keeps submitting.  Since we're using HCD_BH to give
+ * back the URB we need to give the driver a little bit of time before we
+ * release the reservation.  This worker is called after the appropriate
+ * delay.
+ *
+ * @work: Pointer to a qh unreserve_work.
+ */
+static void dwc2_unreserve_timer_fn(unsigned long data)
+{
+	struct dwc2_qh *qh = (struct dwc2_qh *)data;
+	struct dwc2_hsotg *hsotg = qh->hsotg;
+	unsigned long flags;
+
+	/*
+	 * Wait for the lock, or for us to be scheduled again.  We
+	 * could be scheduled again if:
+	 * - We started executing but didn't get the lock yet.
+	 * - A new reservation came in, but cancel didn't take effect
+	 *   because we already started executing.
+	 * - The timer has been kicked again.
+	 * In that case cancel and wait for the next call.
+	 */
+	while (!spin_trylock_irqsave(&hsotg->lock, flags)) {
+		if (timer_pending(&qh->unreserve_timer))
+			return;
+	}
+
+	/*
+	 * Might be no more unreserve pending if:
+	 * - We started executing but didn't get the lock yet.
+	 * - A new reservation came in, but cancel didn't take effect
+	 *   because we already started executing.
+	 *
+	 * We can't put this in the loop above because unreserve_pending needs
+	 * to be accessed under lock, so we can only check it once we got the
+	 * lock.
+	 */
+	if (qh->unreserve_pending)
+		dwc2_do_unreserve(hsotg, qh);
+
+	spin_unlock_irqrestore(&hsotg->lock, flags);
+}
+
 /**
  * dwc2_qh_init() - Initializes a QH structure
  *
@@ -71,6 +159,9 @@ static void dwc2_qh_init(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,
 	dev_vdbg(hsotg->dev, "%s()\n", __func__);
 
 	/* Initialize QH */
+	qh->hsotg = hsotg;
+	setup_timer(&qh->unreserve_timer, dwc2_unreserve_timer_fn,
+		    (unsigned long)qh);
 	qh->ep_type = dwc2_hcd_get_pipe_type(&urb->pipe_info);
 	qh->ep_is_in = dwc2_hcd_is_pipe_in(&urb->pipe_info) ? 1 : 0;
 
@@ -240,6 +331,15 @@ struct dwc2_qh *dwc2_hcd_qh_create(struct dwc2_hsotg *hsotg,
  */
 void dwc2_hcd_qh_free(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 {
+	/* Make sure any unreserve work is finished. */
+	if (del_timer_sync(&qh->unreserve_timer)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&hsotg->lock, flags);
+		dwc2_do_unreserve(hsotg, qh);
+		spin_unlock_irqrestore(&hsotg->lock, flags);
+	}
+
 	if (qh->desc_list)
 		dwc2_hcd_qh_free_ddma(hsotg, qh);
 	kfree(qh);
@@ -477,51 +577,74 @@ static int dwc2_schedule_periodic(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 {
 	int status;
 
-	if (hsotg->core_params->uframe_sched > 0) {
-		int frame = -1;
-
-		status = dwc2_find_uframe(hsotg, qh);
-		if (status == 0)
-			frame = 7;
-		else if (status > 0)
-			frame = status - 1;
-
-		/* Set the new frame up */
-		if (frame >= 0) {
-			qh->sched_frame &= ~0x7;
-			qh->sched_frame |= (frame & 7);
-			dwc2_sch_dbg(hsotg, "QH=%p sched_p sch=%04x, uf=%d\n",
-				     qh, qh->sched_frame, frame);
+	status = dwc2_check_max_xfer_size(hsotg, qh);
+	if (status) {
+		dev_dbg(hsotg->dev,
+			"%s: Channel max transfer size too small for periodic transfer\n",
+			__func__);
+		return status;
+	}
+
+	/* Cancel pending unreserve; if canceled OK, unreserve was pending */
+	if (del_timer(&qh->unreserve_timer))
+		WARN_ON(!qh->unreserve_pending);
+
+	/*
+	 * Only need to reserve if there's not an unreserve pending, since if an
+	 * unreserve is pending then by definition our old reservation is still
+	 * valid.  Unreserve might still be pending even if we didn't cancel if
+	 * dwc2_unreserve_timer_fn() already started.  Code in the timer handles
+	 * that case.
+	 */
+	if (!qh->unreserve_pending) {
+		if (hsotg->core_params->uframe_sched > 0) {
+			int frame = -1;
+
+			status = dwc2_find_uframe(hsotg, qh);
+			if (status == 0)
+				frame = 7;
+			else if (status > 0)
+				frame = status - 1;
+
+			/* Set the new frame up */
+			if (frame >= 0) {
+				qh->sched_frame &= ~0x7;
+				qh->sched_frame |= (frame & 7);
+				dwc2_sch_dbg(hsotg,
+					     "QH=%p sched_p sch=%04x, uf=%d\n",
+					     qh, qh->sched_frame, frame);
+			}
+
+			if (status > 0)
+				status = 0;
+		} else {
+			status = dwc2_periodic_channel_available(hsotg);
+			if (status) {
+				dev_info(hsotg->dev,
+					"%s: No host channel available for periodic transfer\n",
+					__func__);
+				return status;
+			}
+
+			status = dwc2_check_periodic_bandwidth(hsotg, qh);
 		}
 
-		if (status > 0)
-			status = 0;
-	} else {
-		status = dwc2_periodic_channel_available(hsotg);
 		if (status) {
-			dev_info(hsotg->dev,
-				 "%s: No host channel available for periodic transfer\n",
-				 __func__);
+			dev_dbg(hsotg->dev,
+				"%s: Insufficient periodic bandwidth for periodic transfer\n",
+				__func__);
 			return status;
 		}
 
-		status = dwc2_check_periodic_bandwidth(hsotg, qh);
-	}
+		if (hsotg->core_params->uframe_sched <= 0)
+			/* Reserve periodic channel */
+			hsotg->periodic_channels++;
 
-	if (status) {
-		dev_dbg(hsotg->dev,
-			"%s: Insufficient periodic bandwidth for periodic transfer\n",
-			__func__);
-		return status;
+		/* Update claimed usecs per (micro)frame */
+		hsotg->periodic_usecs += qh->usecs;
 	}
 
-	status = dwc2_check_max_xfer_size(hsotg, qh);
-	if (status) {
-		dev_dbg(hsotg->dev,
-			"%s: Channel max transfer size too small for periodic transfer\n",
-			__func__);
-		return status;
-	}
+	qh->unreserve_pending = 0;
 
 	if (hsotg->core_params->dma_desc_enable > 0)
 		/* Don't rely on SOF and start in ready schedule */
@@ -531,13 +654,6 @@ static int dwc2_schedule_periodic(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 		list_add_tail(&qh->qh_list_entry,
 			      &hsotg->periodic_sched_inactive);
 
-	if (hsotg->core_params->uframe_sched <= 0)
-		/* Reserve periodic channel */
-		hsotg->periodic_channels++;
-
-	/* Update claimed usecs per (micro)frame */
-	hsotg->periodic_usecs += qh->usecs;
-
 	return status;
 }
 
@@ -551,22 +667,31 @@ static int dwc2_schedule_periodic(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 static void dwc2_deschedule_periodic(struct dwc2_hsotg *hsotg,
 				     struct dwc2_qh *qh)
 {
-	int i;
+	bool did_modify;
 
-	list_del_init(&qh->qh_list_entry);
+	assert_spin_locked(&hsotg->lock);
 
-	/* Update claimed usecs per (micro)frame */
-	hsotg->periodic_usecs -= qh->usecs;
+	/*
+	 * Schedule the unreserve to happen in a little bit.  Cases here:
+	 * - Unreserve worker might be sitting there waiting to grab the lock.
+	 *   In this case it will notice it's been schedule again and will
+	 *   quit.
+	 * - Unreserve worker might not be scheduled.
+	 *
+	 * We should never already be scheduled since dwc2_schedule_periodic()
+	 * should have canceled the scheduled unreserve timer (hence the
+	 * warning on did_modify).
+	 *
+	 * We add + 1 to the timer to guarantee that at least 1 jiffy has
+	 * passed (otherwise if the jiffy counter might tick right after we
+	 * read it and we'll get no delay).
+	 */
+	did_modify = mod_timer(&qh->unreserve_timer,
+			       jiffies + DWC2_UNRESERVE_DELAY + 1);
+	WARN_ON(did_modify);
+	qh->unreserve_pending = 1;
 
-	if (hsotg->core_params->uframe_sched > 0) {
-		for (i = 0; i < 8; i++) {
-			hsotg->frame_usecs[i] += qh->frame_usecs[i];
-			qh->frame_usecs[i] = 0;
-		}
-	} else {
-		/* Release periodic channel reservation */
-		hsotg->periodic_channels--;
-	}
+	list_del_init(&qh->qh_list_entry);
 }
 
 /**
-- 
2.7.0.rc3.207.g0ac5344

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

#1313889 — [PATCH v4 03/21] usb: dwc2: host: Set host_rx_fifo_size to 528 for rk3066

As documented in dwc2_calculate_dynamic_fifo(), host_rx_fifo_size should
really be:
 2 * ((Largest Packet size / 4) + 1 + 1) + n
 with n = number of host channel.

We have 9 host channels, so
 2 * ((1024/4) + 2) + 9 = 516 + 9 = 525

We've got 960 / 972 total_fifo_size on rk3288 (and presumably on
rk3066) and 525 + 128 + 256 = 909 so we're still under on both ports
even when we increment by 5.

Since we have space, Kever Yang suggests bumping by 8.  He says this
will meet INCR16 access and next fifo type can start with a aligned
address.

...so let's bump up by 8.  In the future, it would be nice if
dwc2_calculate_dynamic_fifo() could handle the "too small" FIFO case and
come up with something more dynamically.  When we do that we can figure
out how to allocate the extra 48 / 60 bytes of FIFO that we're currently
wasting.

NOTE: no known bugs are fixed by this patch, but it seems like a simple
fix and ought to fix someone.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- Set host_rx_fifo_size to 528 for rk3066 new for v4.

Changes in v3: None
Changes in v2: None

 drivers/usb/dwc2/platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 5008a467ce06..b6d7666e715c 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -126,7 +126,7 @@ static const struct dwc2_core_params params_rk3066 = {
 	.speed				= -1,
 	.enable_dynamic_fifo		= 1,
 	.en_multiple_tx_fifo		= -1,
-	.host_rx_fifo_size		= 520,	/* 520 DWORDs */
+	.host_rx_fifo_size		= 528,	/* 528 DWORDs */
 	.host_nperio_tx_fifo_size	= 128,	/* 128 DWORDs */
 	.host_perio_tx_fifo_size	= 256,	/* 256 DWORDs */
 	.max_transfer_size		= -1,
-- 
2.7.0.rc3.207.g0ac5344

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

#1313891 — [PATCH v4 02/21] usb: dwc2: host: Get aligned DMA in a more supported way

All other host controllers who want aligned buffers for DMA do it a
certain way.  Let's do that too instead of working behind the USB core's
back.  This makes our interrupt handler not take forever and also rips
out a lot of code, simplifying things a bunch.

This also has the side effect of removing the 65535 max transfer size
limit.

NOTE: The actual code to allocate the aligned buffers is ripped almost
completely from the tegra EHCI driver.  At some point in the future we
may want to add this functionality to the USB core to share more code
everywhere.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: John Youn <johnyoun@synopsys.com>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: John Youn <johnyoun@synopsys.com>
---
Changes in v4:
- Add John's Acks from <https://patchwork.kernel.org/patch/7631551>

Changes in v3: None
Changes in v2:
- Add a warn if setup_dma is not aligned (Julius Werner).

 drivers/usb/dwc2/core.c      |  21 +-----
 drivers/usb/dwc2/hcd.c       | 170 +++++++++++++++++++++----------------------
 drivers/usb/dwc2/hcd.h       |  10 ---
 drivers/usb/dwc2/hcd_intr.c  |  65 -----------------
 drivers/usb/dwc2/hcd_queue.c |   7 +-
 5 files changed, 87 insertions(+), 186 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 39a0fa8a4c0a..73f2771b7740 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -1958,19 +1958,11 @@ void dwc2_hc_start_transfer(struct dwc2_hsotg *hsotg,
 	}
 
 	if (hsotg->core_params->dma_enable > 0) {
-		dma_addr_t dma_addr;
-
-		if (chan->align_buf) {
-			if (dbg_hc(chan))
-				dev_vdbg(hsotg->dev, "align_buf\n");
-			dma_addr = chan->align_buf;
-		} else {
-			dma_addr = chan->xfer_dma;
-		}
-		dwc2_writel((u32)dma_addr, hsotg->regs + HCDMA(chan->hc_num));
+		dwc2_writel((u32)chan->xfer_dma,
+			    hsotg->regs + HCDMA(chan->hc_num));
 		if (dbg_hc(chan))
 			dev_vdbg(hsotg->dev, "Wrote %08lx to HCDMA(%d)\n",
-				 (unsigned long)dma_addr, chan->hc_num);
+				 (unsigned long)chan->xfer_dma, chan->hc_num);
 	}
 
 	/* Start the split */
@@ -3363,13 +3355,6 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 	width = (hwcfg3 & GHWCFG3_XFER_SIZE_CNTR_WIDTH_MASK) >>
 		GHWCFG3_XFER_SIZE_CNTR_WIDTH_SHIFT;
 	hw->max_transfer_size = (1 << (width + 11)) - 1;
-	/*
-	 * Clip max_transfer_size to 65535. dwc2_hc_setup_align_buf() allocates
-	 * coherent buffers with this size, and if it's too large we can
-	 * exhaust the coherent DMA pool.
-	 */
-	if (hw->max_transfer_size > 65535)
-		hw->max_transfer_size = 65535;
 	width = (hwcfg3 & GHWCFG3_PACKET_SIZE_CNTR_WIDTH_MASK) >>
 		GHWCFG3_PACKET_SIZE_CNTR_WIDTH_SHIFT;
 	hw->max_packet_count = (1 << (width + 4)) - 1;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 8847c72e55f6..bc4bdbc1534e 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -635,9 +635,9 @@ static void dwc2_hc_init_split(struct dwc2_hsotg *hsotg,
 	chan->hub_port = (u8)hub_port;
 }
 
-static void *dwc2_hc_init_xfer(struct dwc2_hsotg *hsotg,
-			       struct dwc2_host_chan *chan,
-			       struct dwc2_qtd *qtd, void *bufptr)
+static void dwc2_hc_init_xfer(struct dwc2_hsotg *hsotg,
+			      struct dwc2_host_chan *chan,
+			      struct dwc2_qtd *qtd)
 {
 	struct dwc2_hcd_urb *urb = qtd->urb;
 	struct dwc2_hcd_iso_packet_desc *frame_desc;
@@ -657,7 +657,6 @@ static void *dwc2_hc_init_xfer(struct dwc2_hsotg *hsotg,
 			else
 				chan->xfer_buf = urb->setup_packet;
 			chan->xfer_len = 8;
-			bufptr = NULL;
 			break;
 
 		case DWC2_CONTROL_DATA:
@@ -684,7 +683,6 @@ static void *dwc2_hc_init_xfer(struct dwc2_hsotg *hsotg,
 				chan->xfer_dma = hsotg->status_buf_dma;
 			else
 				chan->xfer_buf = hsotg->status_buf;
-			bufptr = NULL;
 			break;
 		}
 		break;
@@ -717,14 +715,6 @@ static void *dwc2_hc_init_xfer(struct dwc2_hsotg *hsotg,
 
 		chan->xfer_len = frame_desc->length - qtd->isoc_split_offset;
 
-		/* For non-dword aligned buffers */
-		if (hsotg->core_params->dma_enable > 0 &&
-		    (chan->xfer_dma & 0x3))
-			bufptr = (u8 *)urb->buf + frame_desc->offset +
-					qtd->isoc_split_offset;
-		else
-			bufptr = NULL;
-
 		if (chan->xact_pos == DWC2_HCSPLT_XACTPOS_ALL) {
 			if (chan->xfer_len <= 188)
 				chan->xact_pos = DWC2_HCSPLT_XACTPOS_ALL;
@@ -733,63 +723,93 @@ static void *dwc2_hc_init_xfer(struct dwc2_hsotg *hsotg,
 		}
 		break;
 	}
+}
+
+#define DWC2_USB_DMA_ALIGN 4
+
+struct dma_aligned_buffer {
+	void *kmalloc_ptr;
+	void *old_xfer_buffer;
+	u8 data[0];
+};
+
+static void dwc2_free_dma_aligned_buffer(struct urb *urb)
+{
+	struct dma_aligned_buffer *temp;
+
+	if (!(urb->transfer_flags & URB_ALIGNED_TEMP_BUFFER))
+		return;
+
+	temp = container_of(urb->transfer_buffer,
+		struct dma_aligned_buffer, data);
 
-	return bufptr;
+	if (usb_urb_dir_in(urb))
+		memcpy(temp->old_xfer_buffer, temp->data,
+		       urb->transfer_buffer_length);
+	urb->transfer_buffer = temp->old_xfer_buffer;
+	kfree(temp->kmalloc_ptr);
+
+	urb->transfer_flags &= ~URB_ALIGNED_TEMP_BUFFER;
 }
 
-static int dwc2_hc_setup_align_buf(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,
-				   struct dwc2_host_chan *chan,
-				   struct dwc2_hcd_urb *urb, void *bufptr)
+static int dwc2_alloc_dma_aligned_buffer(struct urb *urb, gfp_t mem_flags)
 {
-	u32 buf_size;
-	struct urb *usb_urb;
-	struct usb_hcd *hcd;
+	struct dma_aligned_buffer *temp, *kmalloc_ptr;
+	size_t kmalloc_size;
 
-	if (!qh->dw_align_buf) {
-		if (chan->ep_type != USB_ENDPOINT_XFER_ISOC)
-			buf_size = hsotg->core_params->max_transfer_size;
-		else
-			/* 3072 = 3 max-size Isoc packets */
-			buf_size = 3072;
+	if (urb->num_sgs || urb->sg ||
+	    urb->transfer_buffer_length == 0 ||
+	    !((uintptr_t)urb->transfer_buffer & (DWC2_USB_DMA_ALIGN - 1)))
+		return 0;
 
-		qh->dw_align_buf = kmalloc(buf_size, GFP_ATOMIC | GFP_DMA);
-		if (!qh->dw_align_buf)
-			return -ENOMEM;
-		qh->dw_align_buf_size = buf_size;
-	}
+	/* Allocate a buffer with enough padding for alignment */
+	kmalloc_size = urb->transfer_buffer_length +
+		sizeof(struct dma_aligned_buffer) + DWC2_USB_DMA_ALIGN - 1;
 
-	if (chan->xfer_len) {
-		dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
-		usb_urb = urb->priv;
+	kmalloc_ptr = kmalloc(kmalloc_size, mem_flags);
+	if (!kmalloc_ptr)
+		return -ENOMEM;
 
-		if (usb_urb) {
-			if (usb_urb->transfer_flags &
-			    (URB_SETUP_MAP_SINGLE | URB_DMA_MAP_SG |
-			     URB_DMA_MAP_PAGE | URB_DMA_MAP_SINGLE)) {
-				hcd = dwc2_hsotg_to_hcd(hsotg);
-				usb_hcd_unmap_urb_for_dma(hcd, usb_urb);
-			}
-			if (!chan->ep_is_in)
-				memcpy(qh->dw_align_buf, bufptr,
-				       chan->xfer_len);
-		} else {
-			dev_warn(hsotg->dev, "no URB in dwc2_urb\n");
-		}
-	}
+	/* Position our struct dma_aligned_buffer such that data is aligned */
+	temp = PTR_ALIGN(kmalloc_ptr + 1, DWC2_USB_DMA_ALIGN) - 1;
+	temp->kmalloc_ptr = kmalloc_ptr;
+	temp->old_xfer_buffer = urb->transfer_buffer;
+	if (usb_urb_dir_out(urb))
+		memcpy(temp->data, urb->transfer_buffer,
+		       urb->transfer_buffer_length);
+	urb->transfer_buffer = temp->data;
 
-	qh->dw_align_buf_dma = dma_map_single(hsotg->dev,
-			qh->dw_align_buf, qh->dw_align_buf_size,
-			chan->ep_is_in ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
-	if (dma_mapping_error(hsotg->dev, qh->dw_align_buf_dma)) {
-		dev_err(hsotg->dev, "can't map align_buf\n");
-		chan->align_buf = 0;
-		return -EINVAL;
-	}
+	urb->transfer_flags |= URB_ALIGNED_TEMP_BUFFER;
 
-	chan->align_buf = qh->dw_align_buf_dma;
 	return 0;
 }
 
+static int dwc2_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+				      gfp_t mem_flags)
+{
+	int ret;
+
+	/* We assume setup_dma is always aligned; warn if not */
+	WARN_ON_ONCE(urb->setup_dma &&
+		     (urb->setup_dma & (DWC2_USB_DMA_ALIGN - 1)));
+
+	ret = dwc2_alloc_dma_aligned_buffer(urb, mem_flags);
+	if (ret)
+		return ret;
+
+	ret = usb_hcd_map_urb_for_dma(hcd, urb, mem_flags);
+	if (ret)
+		dwc2_free_dma_aligned_buffer(urb);
+
+	return ret;
+}
+
+static void dwc2_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
+{
+	usb_hcd_unmap_urb_for_dma(hcd, urb);
+	dwc2_free_dma_aligned_buffer(urb);
+}
+
 /**
  * dwc2_assign_and_init_hc() - Assigns transactions from a QTD to a free host
  * channel and initializes the host channel to perform the transactions. The
@@ -804,7 +824,6 @@ static int dwc2_assign_and_init_hc(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 	struct dwc2_host_chan *chan;
 	struct dwc2_hcd_urb *urb;
 	struct dwc2_qtd *qtd;
-	void *bufptr = NULL;
 
 	if (dbg_qh(qh))
 		dev_vdbg(hsotg->dev, "%s(%p,%p)\n", __func__, hsotg, qh);
@@ -866,16 +885,10 @@ static int dwc2_assign_and_init_hc(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 		!dwc2_hcd_is_pipe_in(&urb->pipe_info))
 		urb->actual_length = urb->length;
 
-	if (hsotg->core_params->dma_enable > 0) {
+	if (hsotg->core_params->dma_enable > 0)
 		chan->xfer_dma = urb->dma + urb->actual_length;
-
-		/* For non-dword aligned case */
-		if (hsotg->core_params->dma_desc_enable <= 0 &&
-		    (chan->xfer_dma & 0x3))
-			bufptr = (u8 *)urb->buf + urb->actual_length;
-	} else {
+	else
 		chan->xfer_buf = (u8 *)urb->buf + urb->actual_length;
-	}
 
 	chan->xfer_len = urb->length - urb->actual_length;
 	chan->xfer_count = 0;
@@ -887,27 +900,7 @@ static int dwc2_assign_and_init_hc(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 		chan->do_split = 0;
 
 	/* Set the transfer attributes */
-	bufptr = dwc2_hc_init_xfer(hsotg, chan, qtd, bufptr);
-
-	/* Non DWORD-aligned buffer case */
-	if (bufptr) {
-		dev_vdbg(hsotg->dev, "Non-aligned buffer\n");
-		if (dwc2_hc_setup_align_buf(hsotg, qh, chan, urb, bufptr)) {
-			dev_err(hsotg->dev,
-				"%s: Failed to allocate memory to handle non-dword aligned buffer\n",
-				__func__);
-			/* Add channel back to free list */
-			chan->align_buf = 0;
-			chan->multi_count = 0;
-			list_add_tail(&chan->hc_list_entry,
-				      &hsotg->free_hc_list);
-			qtd->in_process = 0;
-			qh->channel = NULL;
-			return -ENOMEM;
-		}
-	} else {
-		chan->align_buf = 0;
-	}
+	dwc2_hc_init_xfer(hsotg, chan, qtd);
 
 	if (chan->ep_type == USB_ENDPOINT_XFER_INT ||
 	    chan->ep_type == USB_ENDPOINT_XFER_ISOC)
@@ -2971,6 +2964,9 @@ static struct hc_driver dwc2_hc_driver = {
 
 	.bus_suspend = _dwc2_hcd_suspend,
 	.bus_resume = _dwc2_hcd_resume,
+
+	.map_urb_for_dma	= dwc2_map_urb_for_dma,
+	.unmap_urb_for_dma	= dwc2_unmap_urb_for_dma,
 };
 
 /*
diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index 8f0a29cefdf7..42f2e4e233da 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -75,8 +75,6 @@ struct dwc2_qh;
  *                      (micro)frame
  * @xfer_buf:           Pointer to current transfer buffer position
  * @xfer_dma:           DMA address of xfer_buf
- * @align_buf:          In Buffer DMA mode this will be used if xfer_buf is not
- *                      DWORD aligned
  * @xfer_len:           Total number of bytes to transfer
  * @xfer_count:         Number of bytes transferred so far
  * @start_pkt_count:    Packet count at start of transfer
@@ -133,7 +131,6 @@ struct dwc2_host_chan {
 
 	u8 *xfer_buf;
 	dma_addr_t xfer_dma;
-	dma_addr_t align_buf;
 	u32 xfer_len;
 	u32 xfer_count;
 	u16 start_pkt_count;
@@ -243,10 +240,6 @@ enum dwc2_transaction_type {
  * @frame_usecs:        Internal variable used by the microframe scheduler
  * @start_split_frame:  (Micro)frame at which last start split was initialized
  * @ntd:                Actual number of transfer descriptors in a list
- * @dw_align_buf:       Used instead of original buffer if its physical address
- *                      is not dword-aligned
- * @dw_align_buf_size:  Size of dw_align_buf
- * @dw_align_buf_dma:   DMA address for dw_align_buf
  * @qtd_list:           List of QTDs for this QH
  * @channel:            Host channel currently processing transfers for this QH
  * @qh_list_entry:      Entry for QH in either the periodic or non-periodic
@@ -279,9 +272,6 @@ struct dwc2_qh {
 	u16 frame_usecs[8];
 	u16 start_split_frame;
 	u16 ntd;
-	u8 *dw_align_buf;
-	int dw_align_buf_size;
-	dma_addr_t dw_align_buf_dma;
 	struct list_head qtd_list;
 	struct dwc2_host_chan *channel;
 	struct list_head qh_list_entry;
diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index f8253803a050..352c98364317 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -472,18 +472,6 @@ static int dwc2_update_urb_state(struct dwc2_hsotg *hsotg,
 		xfer_length = urb->length - urb->actual_length;
 	}
 
-	/* Non DWORD-aligned buffer case handling */
-	if (chan->align_buf && xfer_length) {
-		dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
-		dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
-				chan->qh->dw_align_buf_size,
-				chan->ep_is_in ?
-				DMA_FROM_DEVICE : DMA_TO_DEVICE);
-		if (chan->ep_is_in)
-			memcpy(urb->buf + urb->actual_length,
-					chan->qh->dw_align_buf, xfer_length);
-	}
-
 	dev_vdbg(hsotg->dev, "urb->actual_length=%d xfer_length=%d\n",
 		 urb->actual_length, xfer_length);
 	urb->actual_length += xfer_length;
@@ -565,21 +553,6 @@ static enum dwc2_halt_status dwc2_update_isoc_urb_state(
 		frame_desc->status = 0;
 		frame_desc->actual_length = dwc2_get_actual_xfer_length(hsotg,
 					chan, chnum, qtd, halt_status, NULL);
-
-		/* Non DWORD-aligned buffer case handling */
-		if (chan->align_buf && frame_desc->actual_length) {
-			dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n",
-				 __func__);
-			dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
-					chan->qh->dw_align_buf_size,
-					chan->ep_is_in ?
-					DMA_FROM_DEVICE : DMA_TO_DEVICE);
-			if (chan->ep_is_in)
-				memcpy(urb->buf + frame_desc->offset +
-					qtd->isoc_split_offset,
-					chan->qh->dw_align_buf,
-					frame_desc->actual_length);
-		}
 		break;
 	case DWC2_HC_XFER_FRAME_OVERRUN:
 		urb->error_count++;
@@ -600,21 +573,6 @@ static enum dwc2_halt_status dwc2_update_isoc_urb_state(
 		frame_desc->actual_length = dwc2_get_actual_xfer_length(hsotg,
 					chan, chnum, qtd, halt_status, NULL);
 
-		/* Non DWORD-aligned buffer case handling */
-		if (chan->align_buf && frame_desc->actual_length) {
-			dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n",
-				 __func__);
-			dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
-					chan->qh->dw_align_buf_size,
-					chan->ep_is_in ?
-					DMA_FROM_DEVICE : DMA_TO_DEVICE);
-			if (chan->ep_is_in)
-				memcpy(urb->buf + frame_desc->offset +
-					qtd->isoc_split_offset,
-					chan->qh->dw_align_buf,
-					frame_desc->actual_length);
-		}
-
 		/* Skip whole frame */
 		if (chan->qh->do_split &&
 		    chan->ep_type == USB_ENDPOINT_XFER_ISOC && chan->ep_is_in &&
@@ -680,8 +638,6 @@ static void dwc2_deactivate_qh(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,
 	}
 
 no_qtd:
-	if (qh->channel)
-		qh->channel->align_buf = 0;
 	qh->channel = NULL;
 	dwc2_hcd_qh_deactivate(hsotg, qh, continue_split);
 }
@@ -946,14 +902,6 @@ static int dwc2_xfercomp_isoc_split_in(struct dwc2_hsotg *hsotg,
 
 	frame_desc->actual_length += len;
 
-	if (chan->align_buf) {
-		dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
-		dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
-				chan->qh->dw_align_buf_size, DMA_FROM_DEVICE);
-		memcpy(qtd->urb->buf + frame_desc->offset +
-		       qtd->isoc_split_offset, chan->qh->dw_align_buf, len);
-	}
-
 	qtd->isoc_split_offset += len;
 
 	if (frame_desc->actual_length >= frame_desc->length) {
@@ -1176,19 +1124,6 @@ static void dwc2_update_urb_state_abn(struct dwc2_hsotg *hsotg,
 		xfer_length = urb->length - urb->actual_length;
 	}
 
-	/* Non DWORD-aligned buffer case handling */
-	if (chan->align_buf && xfer_length && chan->ep_is_in) {
-		dev_vdbg(hsotg->dev, "%s(): non-aligned buffer\n", __func__);
-		dma_unmap_single(hsotg->dev, chan->qh->dw_align_buf_dma,
-				chan->qh->dw_align_buf_size,
-				chan->ep_is_in ?
-				DMA_FROM_DEVICE : DMA_TO_DEVICE);
-		if (chan->ep_is_in)
-			memcpy(urb->buf + urb->actual_length,
-					chan->qh->dw_align_buf,
-					xfer_length);
-	}
-
 	urb->actual_length += xfer_length;
 
 	hctsiz = dwc2_readl(hsotg->regs + HCTSIZ(chnum));
diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c
index 27d402f680a3..e0933a9dfad7 100644
--- a/drivers/usb/dwc2/hcd_queue.c
+++ b/drivers/usb/dwc2/hcd_queue.c
@@ -232,13 +232,8 @@ struct dwc2_qh *dwc2_hcd_qh_create(struct dwc2_hsotg *hsotg,
  */
 void dwc2_hcd_qh_free(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 {
-	if (qh->desc_list) {
+	if (qh->desc_list)
 		dwc2_hcd_qh_free_ddma(hsotg, qh);
-	} else {
-		/* kfree(NULL) is safe */
-		kfree(qh->dw_align_buf);
-		qh->dw_align_buf_dma = (dma_addr_t)0;
-	}
 	kfree(qh);
 }
 
-- 
2.7.0.rc3.207.g0ac5344

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

#1313895 — [PATCH v4 08/21] usb: dwc2: host: Add scheduler tracing

In preparation for future changes to the scheduler let's add some
tracing that makes it easy for us to see what's happening.  By default
this tracing will be off.

By changing "core.h" you can easily trace to ftrace, the console, or
nowhere.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
Changes in v4:
- Retooled scheduler tracing a bit, so left off John's Ack from v3.

Changes in v3: None
Changes in v2: None

 drivers/usb/dwc2/core.h      | 20 ++++++++++++++++++++
 drivers/usb/dwc2/hcd.h       |  5 +++++
 drivers/usb/dwc2/hcd_intr.c  |  6 +++++-
 drivers/usb/dwc2/hcd_queue.c | 24 +++++++++++++++++++++++-
 4 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 538cf38af0e4..18f9e4045643 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -44,6 +44,26 @@
 #include <linux/usb/phy.h>
 #include "hw.h"
 
+/*
+ * Suggested defines for tracers:
+ * - no_printk:    Disable tracing
+ * - pr_info:      Print this info to the console
+ * - trace_printk: Print this info to trace buffer (good for verbose logging)
+ */
+
+#define DWC2_TRACE_SCHEDULER		no_printk
+#define DWC2_TRACE_SCHEDULER_VB		no_printk
+
+/* Detailed scheduler tracing, but won't overwhelm console */
+#define dwc2_sch_dbg(hsotg, fmt, ...)					\
+	DWC2_TRACE_SCHEDULER(pr_fmt("%s: SCH: " fmt),			\
+			     dev_name(hsotg->dev), ##__VA_ARGS__)
+
+/* Verbose scheduler tracing */
+#define dwc2_sch_vdbg(hsotg, fmt, ...)					\
+	DWC2_TRACE_SCHEDULER_VB(pr_fmt("%s: SCH: " fmt),		\
+				dev_name(hsotg->dev), ##__VA_ARGS__)
+
 static inline u32 dwc2_readl(const void __iomem *addr)
 {
 	u32 value = __raw_readl(addr);
diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index 1b46e2e617cc..809bc4ff9116 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -563,6 +563,11 @@ static inline u16 dwc2_frame_num_inc(u16 frame, u16 inc)
 	return (frame + inc) & HFNUM_MAX_FRNUM;
 }
 
+static inline u16 dwc2_frame_num_dec(u16 frame, u16 dec)
+{
+	return (frame + HFNUM_MAX_FRNUM + 1 - dec) & HFNUM_MAX_FRNUM;
+}
+
 static inline u16 dwc2_full_frame_num(u16 frame)
 {
 	return (frame & HFNUM_MAX_FRNUM) >> 3;
diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index 577c91096a51..5d25a5ec9736 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -138,13 +138,17 @@ static void dwc2_sof_intr(struct dwc2_hsotg *hsotg)
 	while (qh_entry != &hsotg->periodic_sched_inactive) {
 		qh = list_entry(qh_entry, struct dwc2_qh, qh_list_entry);
 		qh_entry = qh_entry->next;
-		if (dwc2_frame_num_le(qh->sched_frame, hsotg->frame_number))
+		if (dwc2_frame_num_le(qh->sched_frame, hsotg->frame_number)) {
+			dwc2_sch_vdbg(hsotg, "QH=%p ready fn=%04x, sch=%04x\n",
+				      qh, hsotg->frame_number, qh->sched_frame);
+
 			/*
 			 * Move QH to the ready list to be executed next
 			 * (micro)frame
 			 */
 			list_move_tail(&qh->qh_list_entry,
 				  &hsotg->periodic_sched_ready);
+		}
 	}
 	tr_type = dwc2_hcd_select_transactions(hsotg);
 	if (tr_type != DWC2_TRANSACTION_NONE)
diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c
index bc632a72f611..0e9faa75593c 100644
--- a/drivers/usb/dwc2/hcd_queue.c
+++ b/drivers/usb/dwc2/hcd_queue.c
@@ -113,6 +113,9 @@ static void dwc2_qh_init(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,
 		qh->sched_frame = dwc2_frame_num_inc(hsotg->frame_number,
 						     SCHEDULE_SLOP);
 		qh->interval = urb->interval;
+		dwc2_sch_dbg(hsotg, "QH=%p init sch=%04x, fn=%04x, int=%#x\n",
+			     qh, qh->sched_frame, hsotg->frame_number,
+			     qh->interval);
 #if 0
 		/* Increase interrupt polling rate for debugging */
 		if (qh->ep_type == USB_ENDPOINT_XFER_INT)
@@ -126,6 +129,11 @@ static void dwc2_qh_init(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,
 			qh->interval *= 8;
 			qh->sched_frame |= 0x7;
 			qh->start_split_frame = qh->sched_frame;
+			dwc2_sch_dbg(hsotg,
+				     "QH=%p init*8 sch=%04x, fn=%04x, int=%#x\n",
+				     qh, qh->sched_frame, hsotg->frame_number,
+				     qh->interval);
+
 		}
 		dev_dbg(hsotg->dev, "interval=%d\n", qh->interval);
 	}
@@ -482,6 +490,8 @@ static int dwc2_schedule_periodic(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 		if (frame >= 0) {
 			qh->sched_frame &= ~0x7;
 			qh->sched_frame |= (frame & 7);
+			dwc2_sch_dbg(hsotg, "QH=%p sched_p sch=%04x, uf=%d\n",
+				     qh, qh->sched_frame, frame);
 		}
 
 		if (status > 0)
@@ -583,10 +593,16 @@ int dwc2_hcd_qh_add(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 
 	if (!dwc2_frame_num_le(qh->sched_frame, hsotg->frame_number) &&
 			!hsotg->frame_number) {
+		u16 new_frame;
+
 		dev_dbg(hsotg->dev,
 				"reset frame number counter\n");
-		qh->sched_frame = dwc2_frame_num_inc(hsotg->frame_number,
+		new_frame = dwc2_frame_num_inc(hsotg->frame_number,
 				SCHEDULE_SLOP);
+
+		dwc2_sch_vdbg(hsotg, "QH=%p reset sch=%04x=>%04x\n",
+			      qh, qh->sched_frame, new_frame);
+		qh->sched_frame = new_frame;
 	}
 
 	/* Add the new QH to the appropriate schedule */
@@ -652,6 +668,7 @@ static void dwc2_sched_periodic_split(struct dwc2_hsotg *hsotg,
 				      int sched_next_periodic_split)
 {
 	u16 incr;
+	u16 old_frame = qh->sched_frame;
 
 	if (sched_next_periodic_split) {
 		qh->sched_frame = frame_number;
@@ -677,6 +694,11 @@ static void dwc2_sched_periodic_split(struct dwc2_hsotg *hsotg,
 		qh->sched_frame |= 0x7;
 		qh->start_split_frame = qh->sched_frame;
 	}
+
+	dwc2_sch_vdbg(hsotg, "QH=%p next(%d) fn=%04x, sch=%04x=>%04x (%+d)\n",
+		      qh, sched_next_periodic_split, frame_number, old_frame,
+		      qh->sched_frame,
+		      dwc2_frame_num_dec(qh->sched_frame, old_frame));
 }
 
 /*
-- 
2.7.0.rc3.207.g0ac5344

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web