Merge tag 'for-usb-linus-2014-02-11' of git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci into usb-linus

Sarah writes:

xhci: Revert TD fragment hacks.

Hi Greg,

Recently, we found that commit "35773dac5f86 "usb: xhci: Link TRB must not
occur within a USB payload burst" causes a userspace regression.  It will
cause larger transfers submitted through usbfs that would have succeeded
on older kernels to be rejected.

Commit 35773dac5f86 was designed to address an issue where an ASIX USB
ethernet device would get wedged when it was connected to a 1.0 xHCI
host.  Only this particular ethernet device was impacted, because only the
ax88179_178a driver implemented scatter-gather in 3.12.  The xHCI driver
doesn't currently support TD fragment rules, and commit 35773dac5f86 was a
quick hack that partially implemented one of the rules.

This is the third regression this patch has caused.  There's yet another
quick hack to work around the issue, but I really want to support TD
fragments properly, rather than hacking around it.  It will take us a
kernel release or two to get it implemented, since it is a big
architectural change.

This patchset backs out commit 35773dac5f86, and the two bug fix patches
for it.  The first patch limits arbitrarily aligned scatter-gather under
xHCI 1.0 hosts.

As a result of this patchset:

1. usb-storage and uas will still be able to use scatter-gather, since
   they submit max packet sized aligned transfers.
2. usbfs will behave exactly as before, no more userspace regressions.
3. The ax88179_178a driver works fine without scatter-gather (Mark Lord
   confirms this).

Users of the ASIX chipset may still see occasional packet loss on 1.0 xHCI
hosts, if the xHCI driver needs to split a TRB across 64-KB boundaries,
and a link TRB happens to fall between those two TRBs.  I expect this
corner case to be infrequent.

Sarah Sharp

4 files changed, 16 insertions, 60 deletions

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 909b32a4412f..0ed64eb68e48 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2967,58 +2967,8 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
         }
 
         while (1) {
-                if (room_on_ring(xhci, ep_ring, num_trbs)) {
-                        union xhci_trb *trb = ep_ring->enqueue;
-                        unsigned int usable = ep_ring->enq_seg->trbs +
-                                        TRBS_PER_SEGMENT - 1 - trb;
-                        u32 nop_cmd;
-
-                        /*
-                         * Section 4.11.7.1 TD Fragments states that a link
-                         * TRB must only occur at the boundary between
-                         * data bursts (eg 512 bytes for 480M).
-                         * While it is possible to split a large fragment
-                         * we don't know the size yet.
-                         * Simplest solution is to fill the trb before the
-                         * LINK with nop commands.
-                         */
-                        if (num_trbs == 1 || num_trbs <= usable || usable == 0)
-                                break;
-
-                        if (ep_ring->type != TYPE_BULK)
-                                /*
-                                 * While isoc transfers might have a buffer that
-                                 * crosses a 64k boundary it is unlikely.
-                                 * Since we can't add NOPs without generating
-                                 * gaps in the traffic just hope it never
-                                 * happens at the end of the ring.
-                                 * This could be fixed by writing a LINK TRB
-                                 * instead of the first NOP - however the
-                                 * TRB_TYPE_LINK_LE32() calls would all need
-                                 * changing to check the ring length.
-                                 */
-                                break;
-
-                        if (num_trbs >= TRBS_PER_SEGMENT) {
-                                xhci_err(xhci, "Too many fragments %d, max %d\n",
-                                                num_trbs, TRBS_PER_SEGMENT - 1);
-                                return -EINVAL;
-                        }
-
-                        nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
-                                        ep_ring->cycle_state);
-                        ep_ring->num_trbs_free -= usable;
-                        do {
-                                trb->generic.field[0] = 0;
-                                trb->generic.field[1] = 0;
-                                trb->generic.field[2] = 0;
-                                trb->generic.field[3] = nop_cmd;
-                                trb++;
-                        } while (--usable);
-                        ep_ring->enqueue = trb;
-                        if (room_on_ring(xhci, ep_ring, num_trbs))
-                                break;
-                }
+                if (room_on_ring(xhci, ep_ring, num_trbs))
+                        break;
 
                 if (ep_ring == xhci->cmd_ring) {
                         xhci_err(xhci, "Do not support expand command ring\n");
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 3712359d18ba..6fe577d46fa2 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4730,11 +4730,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
         struct device           *dev = hcd->self.controller;
         int                     retval;
 
-        /* Limit the block layer scatter-gather lists to half a segment. */
-        hcd->self.sg_tablesize = TRBS_PER_SEGMENT / 2;
-
-        /* support to build packet from discontinuous buffers */
-        hcd->self.no_sg_constraint = 1;
+        /* Accept arbitrarily long scatter-gather lists */
+        hcd->self.sg_tablesize = ~0;
 
         /* XHCI controllers don't stop the ep queue on short packets :| */
         hcd->self.no_stop_on_short = 1;
@@ -4760,6 +4757,14 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
                 /* xHCI private pointer was set in xhci_pci_probe for the second
                  * registered roothub.
                  */
+                xhci = hcd_to_xhci(hcd);
+                /*
+                 * Support arbitrarily aligned sg-list entries on hosts without
+                 * TD fragment rules (which are currently unsupported).
+                 */
+                if (xhci->hci_version < 0x100)
+                        hcd->self.no_sg_constraint = 1;
+
                 return 0;
         }
 
@@ -4788,6 +4793,9 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
         if (xhci->hci_version > 0x96)
                 xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
 
+        if (xhci->hci_version < 0x100)
+                hcd->self.no_sg_constraint = 1;
+
         /* Make sure the HC is halted. */
         retval = xhci_halt(xhci);
         if (retval)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 9154fd6cf24c..58ed9d088e63 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1268,7 +1268,7 @@ union xhci_trb {
  * since the command ring is 64-byte aligned.
  * It must also be greater than 16.
  */
-#define TRBS_PER_SEGMENT        256
+#define TRBS_PER_SEGMENT        64
 /* Allow two commands + a link TRB, along with any reserved command TRBs */
 #define MAX_RSVD_CMD_TRBS       (TRBS_PER_SEGMENT - 3)
 #define TRB_SEGMENT_SIZE        (TRBS_PER_SEGMENT*16)
diff --git a/include/linux/usb.h b/include/linux/usb.h
index c716da18c668..7f6eb859873e 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1265,8 +1265,6 @@ typedef void (*usb_complete_t)(struct urb *);
  * @sg: scatter gather buffer list, the buffer size of each element in
  *      the list (except the last) must be divisible by the endpoint's
  *      max packet size if no_sg_constraint isn't set in 'struct usb_bus'
- *      (FIXME: scatter-gather under xHCI is broken for periodic transfers.
- *      Do not use urb->sg for interrupt endpoints for now, only bulk.)
  * @num_mapped_sgs: (internal) number of mapped sg entries
  * @num_sgs: number of entries in the sg list
  * @transfer_buffer_length: How big is transfer_buffer.  The transfer may