[RFC PATCH 00/20] xhci: implement xhci-v1+ td-fragment rules, test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sending as an RFC primarily to get feedback on the unit testing approach
while there's still time to yell at me in person at LinuxCon.  Including
Rusty in pursuit of comments on how to do mocked interfaces for testing
purposes in-tree.

===

This series updates the xhci driver to honor all the rules specified in
section 4.11.7.1 of the xhci specification (v1.1).  See the changelog
for patch 19 for more background.

Patches 1-18 are cleanups, fixes, and smaller ring geometry changes to
support patches 19 and 20.  If you only review one of these patches
please review patch 20 ("xhci: unit test ring enqueue/dequeue routines")
to see the current passing test cases and please do propose additional
ones.  As it stands Sarah would like, and I agree, that we need a
cancellation unit test before proposing this for inclusion.

The current unit test is passing as well as basic verification with a
SuperSpeed USB Mass Storage device.  Testing with known problematic
devices is requested / required before this is ready for upstream.

Note the comment in v1_inc_deq() in patch 19.

	/*
	 * ep_inc_deq() lets the dequeue-pointer (deq/tail) wrap the
	 * enqueue-pointer (enq/head)!  However, since room_on_ring() looks at
	 * ->num_trbs_free instead of the position of the ring pointers, it
	 * never causes a problem as enq gets back in line with deq at the next
	 * submission.
	 *
	 * In the case of v1+ rings, conditional_expand() is sensitive to this
	 * wrap and prematurely expands the ring.  Prevent that condition by
	 * stopping once deq == enq.  Eventually, ->num_trbs_free should be
	 * deprecated entirely in favor of just comparing the ring pointers.
	 * For now, for legacy compatibility, we leave well enough alone and
	 * limit this to xhci-v1+ implementations.
	 */

I'm wondering if this is the root cause behind the "set ring dequeue
pointer" problem reported by Julius [1]?  I'll be taking a closer look
at that as I revise this series.

[1]: http://marc.info/?l=linux-usb&m=140484618509288&w=2

Finally, note that this conversion should be a nop for xhci hosts prior
to v1.0 as the old enqueue scheme is preserved.  I'm taking this
approach to limit bug hunting diversions on older hosts.  If anyone has
a pre-1.0 host, test reports are appreciated to verify that it indeed
does not break those hosts.

---

For testing convenience these patches are available via git at:

  git://git.kernel.org/pub/scm/linux/kernel/git/djbw/usb td-fragments-v1

Note that this branch may be rebased and/or abandoned (in favor of a
td-fragments-v2+) as the set is revised.

Dan Williams (20):
      xhci: cleanup remaining cycle bit toggles via ternary conditional
      xhci: remove invalid cast of xhci to a usb_device in xhci_log_ctx trace
      xhci: introduce xhci_to_dev
      xhci: increase trb segment size, kill ->segment_pool
      xhci: prepare for mid-segment link-trbs
      xhci: drop 'xhci' argument to last_trb
      xhci: make xhci_segments doubly linked
      xhci: refactor inc_enq(), share a common advance_enq() with prepare_ring()
      xhci: introduce ring ops to handle event vs non-event rings
      xhci: clarify "ring valid" checks
      xhci: provide helpers for retrieving 'enqueue' and 'dequeue' pointers
      xhci: introduce struct xhci_ring_pointer
      xhci: use %pad for printing dma_addr_t
      xhci: power-of-2 ring sizes
      xhci: kill ->num_trbs_free_temp in struct xhci_ring
      xhci: refactor prepare_transfer()
      xhci: combine xhci_queue_bulk_tx() and queue_bulk_sg_tx()
      xhci: add xhci_ring_reap_td() helper
      xhci: v1.0 scatterlist enqueue support (td-fragment rework)
      xhci: unit test ring enqueue/dequeue routines


 drivers/usb/host/Kconfig                    |   13 
 drivers/usb/host/Makefile                   |    1 
 drivers/usb/host/xhci-dbg.c                 |   97 +-
 drivers/usb/host/xhci-hub.c                 |    5 
 drivers/usb/host/xhci-mem.c                 |  580 +++++-----
 drivers/usb/host/xhci-ring.c                | 1638 ++++++++++++++++++---------
 drivers/usb/host/xhci-trace.h               |   13 
 drivers/usb/host/xhci.c                     |   82 +
 drivers/usb/host/xhci.h                     |  230 +++-
 drivers/usb/host/xhcitest/Makefile          |   35 +
 drivers/usb/host/xhcitest/xhci-trace.h      |   96 ++
 drivers/usb/host/xhcitest/xhci-unit-dbg.c   |    1 
 drivers/usb/host/xhcitest/xhci-unit-trace.c |    2 
 drivers/usb/host/xhcitest/xhci-unit.c       |  641 +++++++++++
 14 files changed, 2457 insertions(+), 977 deletions(-)
 create mode 100644 drivers/usb/host/xhcitest/Makefile
 create mode 100644 drivers/usb/host/xhcitest/xhci-trace.h
 create mode 100644 drivers/usb/host/xhcitest/xhci-unit-dbg.c
 create mode 100644 drivers/usb/host/xhcitest/xhci-unit-trace.c
 create mode 100644 drivers/usb/host/xhcitest/xhci-unit.c
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux