From 25cd2882e2fc8bd8ed7acaee0ec979f11feda6d7 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Fri, 17 Jan 2014 14:15:44 -0800 Subject: usb/xhci: Change how we indicate a host supports Link PM. The xHCI driver currently uses a USB core internal field, udev->lpm_capable, to indicate the xHCI driver knows how to calculate the LPM timeout values. If this value is set for the host controller udev, it means Link PM can be enabled for child devices under that host. Change the code so the xHCI driver isn't mucking with USB core internal fields. Instead, indicate the xHCI driver doesn't support Link PM on this host by clearing the U1 and U2 exit latencies in the roothub SuperSpeed Extended Capabilities BOS descriptor. The code to check for the roothub setting U1 and U2 exit latencies to zero will also disable LPM for external devices that do that same. This was already effectively done with commit ae8963adb4ad8c5f2a89ca1d99fb7bb721e7599f "usb: Don't enable LPM if the exit latency is zero." Leave that code in place, so that if a device sets one exit latency value to zero, but the other is set to a valid value, LPM is only enabled for the U1 or U2 state that had the valid value. This is the same behavior the code had before. Also, change messages about missing Link PM information from warning level to info level. Only print a warning about the first device that doesn't support LPM, to avoid log spam. Further, cleanup some unnecessary line breaks to help people to grep for the error messages. Signed-off-by: Sarah Sharp Cc: Alan Stern diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index f6c6b6f..763c313 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -141,19 +141,27 @@ static int usb_device_supports_lpm(struct usb_device *udev) return 0; } - /* All USB 3.0 must support LPM, but we need their max exit latency - * information from the SuperSpeed Extended Capabilities BOS descriptor. + /* + * According to the USB 3.0 spec, all USB 3.0 devices must support LPM. + * However, there are some that don't, and they set the U1/U2 exit + * latencies to zero. */ if (!udev->bos->ss_cap) { - dev_warn(&udev->dev, "No LPM exit latency info found. " - "Power management will be impacted.\n"); + dev_info(&udev->dev, "No LPM exit latency info found, disabling LPM.\n"); return 0; } - if (udev->parent->lpm_capable) - return 1; - dev_warn(&udev->dev, "Parent hub missing LPM exit latency info. " - "Power management will be impacted.\n"); + if (udev->bos->ss_cap->bU1devExitLat == 0 && + udev->bos->ss_cap->bU2DevExitLat == 0) { + if (udev->parent) + dev_info(&udev->dev, "LPM exit latency is zeroed, disabling LPM.\n"); + else + dev_info(&udev->dev, "We don't know the algorithms for LPM for this host, disabling LPM.\n"); + return 0; + } + + if (!udev->parent || udev->parent->lpm_capable) + return 1; return 0; } diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 9992fbf..1ad6bc1 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -732,9 +732,11 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, /* Set the U1 and U2 exit latencies. */ memcpy(buf, &usb_bos_descriptor, USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE); - temp = readl(&xhci->cap_regs->hcs_params3); - buf[12] = HCS_U1_LATENCY(temp); - put_unaligned_le16(HCS_U2_LATENCY(temp), &buf[13]); + if ((xhci->quirks & XHCI_LPM_SUPPORT)) { + temp = readl(&xhci->cap_regs->hcs_params3); + buf[12] = HCS_U1_LATENCY(temp); + put_unaligned_le16(HCS_U2_LATENCY(temp), &buf[13]); + } /* Indicate whether the host has LTM support. */ temp = readl(&xhci->cap_regs->hcc_params); diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 04f986d..6c03584 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -222,12 +222,6 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) goto put_usb3_hcd; /* Roothub already marked as USB 3.0 speed */ - /* We know the LPM timeout algorithms for this host, let the USB core - * enable and disable LPM for devices under the USB 3.0 roothub. - */ - if (xhci->quirks & XHCI_LPM_SUPPORT) - hcd_to_bus(xhci->shared_hcd)->root_hub->lpm_capable = 1; - return 0; put_usb3_hcd: -- cgit v0.10.2 From e587b8b270d3706147c806d42cc4ac78232caac7 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Wed, 8 Jan 2014 17:13:11 +0100 Subject: xhci: make warnings greppable This changes debug messages and warnings in xhci-ring.c to be on a single line so grep can find them. grep must have precedence over the 80 column limit. [Sarah fixed two checkpatch.pl issues with split lines introduced by this commit.] Signed-off-by: Oliver Neukum Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0ed64eb..3ec1d8f 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1082,8 +1082,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, ep_ring = xhci_stream_id_to_ring(dev, ep_index, stream_id); if (!ep_ring) { - xhci_warn(xhci, "WARN Set TR deq ptr command for " - "freed stream ID %u\n", + xhci_warn(xhci, "WARN Set TR deq ptr command for freed stream ID %u\n", stream_id); /* XXX: Harmless??? */ dev->eps[ep_index].ep_state &= ~SET_DEQ_PENDING; @@ -1099,12 +1098,10 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, switch (cmd_comp_code) { case COMP_TRB_ERR: - xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid because " - "of stream ID configuration\n"); + xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid because of stream ID configuration\n"); break; case COMP_CTX_STATE: - xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed due " - "to incorrect slot or ep state.\n"); + xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed due to incorrect slot or ep state.\n"); ep_state = le32_to_cpu(ep_ctx->ep_info); ep_state &= EP_STATE_MASK; slot_state = le32_to_cpu(slot_ctx->dev_state); @@ -1114,13 +1111,12 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, slot_state, ep_state); break; case COMP_EBADSLT: - xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed because " - "slot %u was not enabled.\n", slot_id); + xhci_warn(xhci, "WARN Set TR Deq Ptr cmd failed because slot %u was not enabled.\n", + slot_id); break; default: - xhci_warn(xhci, "WARN Set TR Deq Ptr cmd with unknown " - "completion code of %u.\n", - cmd_comp_code); + xhci_warn(xhci, "WARN Set TR Deq Ptr cmd with unknown completion code of %u.\n", + cmd_comp_code); break; } /* OK what do we do now? The endpoint state is hosed, and we @@ -1142,8 +1138,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, update_ring_for_set_deq_completion(xhci, dev, ep_ring, ep_index); } else { - xhci_warn(xhci, "Mismatch between completed Set TR Deq " - "Ptr command & xHCI internal state.\n"); + xhci_warn(xhci, "Mismatch between completed Set TR Deq Ptr command & xHCI internal state.\n"); xhci_warn(xhci, "ep deq seg = %p, deq ptr = %p\n", dev->eps[ep_index].queued_deq_seg, dev->eps[ep_index].queued_deq_ptr); -- cgit v0.10.2 From 153413032c6ea624fccc6732aba27a57688a7f91 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 4 Oct 2013 00:29:44 +0200 Subject: xhci: fix usb3 streams xhci maintains a radix tree for each stream endpoint because it must be able to map a trb address to the stream ring. Each ring segment must be added to the ring for this to work. Currently xhci sticks only the first segment of each stream ring into the radix tree. Result is that things work initially, but as soon as the first segment is full xhci can't map the trb address from the completion event to the stream ring any more -> BOOM. You'll find this message in the logs: ERROR Transfer event for disabled endpoint or incorrect stream ring This patch adds a helper function to update the radix tree, and a function to remove ring segments from the tree. Both functions loop over the segment list and handles all segments instead of just the first. [Note: Sarah changed this patch to add radix_tree_maybe_preload() and radix_tree_preload_end() calls around the radix tree insert, since we can now insert entries in interrupt context. There are now two helper functions to make the code cleaner, and those functions are moved to make them static.] Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index bce4391..39f9a99 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -149,14 +149,95 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring, } } +/* + * We need a radix tree for mapping physical addresses of TRBs to which stream + * ID they belong to. We need to do this because the host controller won't tell + * us which stream ring the TRB came from. We could store the stream ID in an + * event data TRB, but that doesn't help us for the cancellation case, since the + * endpoint may stop before it reaches that event data TRB. + * + * The radix tree maps the upper portion of the TRB DMA address to a ring + * segment that has the same upper portion of DMA addresses. For example, say I + * have segments of size 1KB, that are always 64-byte aligned. A segment may + * start at 0x10c91000 and end at 0x10c913f0. If I use the upper 10 bits, the + * key to the stream ID is 0x43244. I can use the DMA address of the TRB to + * pass the radix tree a key to get the right stream ID: + * + * 0x10c90fff >> 10 = 0x43243 + * 0x10c912c0 >> 10 = 0x43244 + * 0x10c91400 >> 10 = 0x43245 + * + * Obviously, only those TRBs with DMA addresses that are within the segment + * will make the radix tree return the stream ID for that ring. + * + * Caveats for the radix tree: + * + * The radix tree uses an unsigned long as a key pair. On 32-bit systems, an + * unsigned long will be 32-bits; on a 64-bit system an unsigned long will be + * 64-bits. Since we only request 32-bit DMA addresses, we can use that as the + * key on 32-bit or 64-bit systems (it would also be fine if we asked for 64-bit + * PCI DMA addresses on a 64-bit system). There might be a problem on 32-bit + * extended systems (where the DMA address can be bigger than 32-bits), + * if we allow the PCI dma mask to be bigger than 32-bits. So don't do that. + */ +static int xhci_update_stream_mapping(struct xhci_ring *ring, gfp_t mem_flags) +{ + struct xhci_segment *seg; + unsigned long key; + int ret; + + if (WARN_ON_ONCE(ring->trb_address_map == NULL)) + return 0; + + seg = ring->first_seg; + do { + key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); + /* Skip any segments that were already added. */ + if (radix_tree_lookup(ring->trb_address_map, key)) + continue; + + ret = radix_tree_maybe_preload(mem_flags); + if (ret) + return ret; + ret = radix_tree_insert(ring->trb_address_map, + key, ring); + radix_tree_preload_end(); + if (ret) + return ret; + seg = seg->next; + } while (seg != ring->first_seg); + + return 0; +} + +static void xhci_remove_stream_mapping(struct xhci_ring *ring) +{ + struct xhci_segment *seg; + unsigned long key; + + if (WARN_ON_ONCE(ring->trb_address_map == NULL)) + return; + + seg = ring->first_seg; + do { + key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); + if (radix_tree_lookup(ring->trb_address_map, key)) + radix_tree_delete(ring->trb_address_map, key); + seg = seg->next; + } while (seg != ring->first_seg); +} + /* XXX: Do we need the hcd structure in all these functions? */ void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring) { if (!ring) return; - if (ring->first_seg) + if (ring->first_seg) { + if (ring->type == TYPE_STREAM) + xhci_remove_stream_mapping(ring); xhci_free_segments_for_ring(xhci, ring->first_seg); + } kfree(ring); } @@ -354,6 +435,11 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring, "ring expansion succeed, now has %d segments", ring->num_segs); + if (ring->type == TYPE_STREAM) { + ret = xhci_update_stream_mapping(ring, flags); + WARN_ON(ret); /* FIXME */ + } + return 0; } @@ -510,36 +596,6 @@ struct xhci_ring *xhci_stream_id_to_ring( * The number of stream contexts in the stream context array may be bigger than * the number of streams the driver wants to use. This is because the number of * stream context array entries must be a power of two. - * - * We need a radix tree for mapping physical addresses of TRBs to which stream - * ID they belong to. We need to do this because the host controller won't tell - * us which stream ring the TRB came from. We could store the stream ID in an - * event data TRB, but that doesn't help us for the cancellation case, since the - * endpoint may stop before it reaches that event data TRB. - * - * The radix tree maps the upper portion of the TRB DMA address to a ring - * segment that has the same upper portion of DMA addresses. For example, say I - * have segments of size 1KB, that are always 64-byte aligned. A segment may - * start at 0x10c91000 and end at 0x10c913f0. If I use the upper 10 bits, the - * key to the stream ID is 0x43244. I can use the DMA address of the TRB to - * pass the radix tree a key to get the right stream ID: - * - * 0x10c90fff >> 10 = 0x43243 - * 0x10c912c0 >> 10 = 0x43244 - * 0x10c91400 >> 10 = 0x43245 - * - * Obviously, only those TRBs with DMA addresses that are within the segment - * will make the radix tree return the stream ID for that ring. - * - * Caveats for the radix tree: - * - * The radix tree uses an unsigned long as a key pair. On 32-bit systems, an - * unsigned long will be 32-bits; on a 64-bit system an unsigned long will be - * 64-bits. Since we only request 32-bit DMA addresses, we can use that as the - * key on 32-bit or 64-bit systems (it would also be fine if we asked for 64-bit - * PCI DMA addresses on a 64-bit system). There might be a problem on 32-bit - * extended systems (where the DMA address can be bigger than 32-bits), - * if we allow the PCI dma mask to be bigger than 32-bits. So don't do that. */ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci, unsigned int num_stream_ctxs, @@ -548,7 +604,6 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci, struct xhci_stream_info *stream_info; u32 cur_stream; struct xhci_ring *cur_ring; - unsigned long key; u64 addr; int ret; @@ -603,6 +658,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci, if (!cur_ring) goto cleanup_rings; cur_ring->stream_id = cur_stream; + cur_ring->trb_address_map = &stream_info->trb_address_map; /* Set deq ptr, cycle bit, and stream context type */ addr = cur_ring->first_seg->dma | SCT_FOR_CTX(SCT_PRI_TR) | @@ -612,10 +668,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci, xhci_dbg(xhci, "Setting stream %d ring ptr to 0x%08llx\n", cur_stream, (unsigned long long) addr); - key = (unsigned long) - (cur_ring->first_seg->dma >> TRB_SEGMENT_SHIFT); - ret = radix_tree_insert(&stream_info->trb_address_map, - key, cur_ring); + ret = xhci_update_stream_mapping(cur_ring, mem_flags); if (ret) { xhci_ring_free(xhci, cur_ring); stream_info->stream_rings[cur_stream] = NULL; @@ -635,9 +688,6 @@ cleanup_rings: for (cur_stream = 1; cur_stream < num_streams; cur_stream++) { cur_ring = stream_info->stream_rings[cur_stream]; if (cur_ring) { - addr = cur_ring->first_seg->dma; - radix_tree_delete(&stream_info->trb_address_map, - addr >> TRB_SEGMENT_SHIFT); xhci_ring_free(xhci, cur_ring); stream_info->stream_rings[cur_stream] = NULL; } @@ -698,7 +748,6 @@ void xhci_free_stream_info(struct xhci_hcd *xhci, { int cur_stream; struct xhci_ring *cur_ring; - dma_addr_t addr; if (!stream_info) return; @@ -707,9 +756,6 @@ void xhci_free_stream_info(struct xhci_hcd *xhci, cur_stream++) { cur_ring = stream_info->stream_rings[cur_stream]; if (cur_ring) { - addr = cur_ring->first_seg->dma; - radix_tree_delete(&stream_info->trb_address_map, - addr >> TRB_SEGMENT_SHIFT); xhci_ring_free(xhci, cur_ring); stream_info->stream_rings[cur_stream] = NULL; } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 58ed9d0..a6aa98f 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1341,6 +1341,7 @@ struct xhci_ring { unsigned int num_trbs_free_temp; enum xhci_ring_type type; bool last_td_was_short; + struct radix_tree_root *trb_address_map; }; struct xhci_erst_entry { -- cgit v0.10.2 From df6138347bcde316e4d4b24ae4d9d0296461c79a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 4 Oct 2013 00:29:45 +0200 Subject: xhci: Free streams when they are still allocated on a set_interface call And warn about this, as that would be a driver bug. Like wise drivers should ensure that streams are properly free-ed before a device is reset. So lets warn about that too. This already causes warnings in the form of: [ 96.982398] xhci_hcd 0000:01:00.0: WARN Can't disable streams for endpoint 0x81 , streams are already disabled! [ 96.982400] xhci_hcd 0000:01:00.0: WARN xhci_free_streams() called with non-streams endpoint But it is better to also warn about the actual cause of this later warnings. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6fe577d..c3fa32a 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2678,6 +2678,20 @@ static int xhci_configure_endpoint(struct xhci_hcd *xhci, return ret; } +static void xhci_check_bw_drop_ep_streams(struct xhci_hcd *xhci, + struct xhci_virt_device *vdev, int i) +{ + struct xhci_virt_ep *ep = &vdev->eps[i]; + + if (ep->ep_state & EP_HAS_STREAMS) { + xhci_warn(xhci, "WARN: endpoint 0x%02x has streams on set_interface, freeing streams.\n", + xhci_get_endpoint_address(i)); + xhci_free_stream_info(xhci, ep->stream_info); + ep->stream_info = NULL; + ep->ep_state &= ~EP_HAS_STREAMS; + } +} + /* Called after one or more calls to xhci_add_endpoint() or * xhci_drop_endpoint(). If this call fails, the USB core is expected * to call xhci_reset_bandwidth(). @@ -2742,8 +2756,10 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev) /* Free any rings that were dropped, but not changed. */ for (i = 1; i < 31; ++i) { if ((le32_to_cpu(ctrl_ctx->drop_flags) & (1 << (i + 1))) && - !(le32_to_cpu(ctrl_ctx->add_flags) & (1 << (i + 1)))) + !(le32_to_cpu(ctrl_ctx->add_flags) & (1 << (i + 1)))) { xhci_free_or_cache_endpoint_ring(xhci, virt_dev, i); + xhci_check_bw_drop_ep_streams(xhci, virt_dev, i); + } } xhci_zero_in_ctx(xhci, virt_dev); /* @@ -2759,6 +2775,7 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev) if (virt_dev->eps[i].ring) { xhci_free_or_cache_endpoint_ring(xhci, virt_dev, i); } + xhci_check_bw_drop_ep_streams(xhci, virt_dev, i); virt_dev->eps[i].ring = virt_dev->eps[i].new_ring; virt_dev->eps[i].new_ring = NULL; } @@ -3519,6 +3536,8 @@ int xhci_discover_or_reset_device(struct usb_hcd *hcd, struct usb_device *udev) struct xhci_virt_ep *ep = &virt_dev->eps[i]; if (ep->ep_state & EP_HAS_STREAMS) { + xhci_warn(xhci, "WARN: endpoint 0x%02x has streams on device reset, freeing streams.\n", + xhci_get_endpoint_address(i)); xhci_free_stream_info(xhci, ep->stream_info); ep->stream_info = NULL; ep->ep_state &= ~EP_HAS_STREAMS; -- cgit v0.10.2 From ee4aa54bce2ae27c36998da3d32d9f55cb48ca21 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 4 Oct 2013 00:29:46 +0200 Subject: xhci: Check size rather then number of streams when allocating stream ctxs Before this a device needing ie 32 stream ctxs would end up with an entry from the small_streams_pool which has 256 bytes entries, where as 32 stream ctxs need 512 bytes. Things actually keep running for a surprisingly long time before crashing because of this. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 39f9a99..be7b7b6 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -520,12 +520,12 @@ static void xhci_free_stream_ctx(struct xhci_hcd *xhci, struct xhci_stream_ctx *stream_ctx, dma_addr_t dma) { struct device *dev = xhci_to_hcd(xhci)->self.controller; + size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs; - if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE) - dma_free_coherent(dev, - sizeof(struct xhci_stream_ctx)*num_stream_ctxs, + if (size > MEDIUM_STREAM_ARRAY_SIZE) + dma_free_coherent(dev, size, stream_ctx, dma); - else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE) + else if (size <= SMALL_STREAM_ARRAY_SIZE) return dma_pool_free(xhci->small_streams_pool, stream_ctx, dma); else @@ -548,12 +548,12 @@ static struct xhci_stream_ctx *xhci_alloc_stream_ctx(struct xhci_hcd *xhci, gfp_t mem_flags) { struct device *dev = xhci_to_hcd(xhci)->self.controller; + size_t size = sizeof(struct xhci_stream_ctx) * num_stream_ctxs; - if (num_stream_ctxs > MEDIUM_STREAM_ARRAY_SIZE) - return dma_alloc_coherent(dev, - sizeof(struct xhci_stream_ctx)*num_stream_ctxs, + if (size > MEDIUM_STREAM_ARRAY_SIZE) + return dma_alloc_coherent(dev, size, dma, mem_flags); - else if (num_stream_ctxs <= SMALL_STREAM_ARRAY_SIZE) + else if (size <= SMALL_STREAM_ARRAY_SIZE) return dma_pool_alloc(xhci->small_streams_pool, mem_flags, dma); else -- cgit v0.10.2 From c4bedb77ec4cb42f37cae4cbfddda8283161f7c8 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 4 Oct 2013 00:29:47 +0200 Subject: xhci: For streams the css flag most be read from the stream-ctx on ep stop Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 3ec1d8f..51c5109 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -546,9 +546,9 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, struct xhci_dequeue_state *state) { struct xhci_virt_device *dev = xhci->devs[slot_id]; + struct xhci_virt_ep *ep = &dev->eps[ep_index]; struct xhci_ring *ep_ring; struct xhci_generic_trb *trb; - struct xhci_ep_ctx *ep_ctx; dma_addr_t addr; ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id, @@ -573,8 +573,16 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci, /* Dig out the cycle state saved by the xHC during the stop ep cmd */ xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Finding endpoint context"); - ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index); - state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq); + /* 4.6.9 the css flag is written to the stream context for streams */ + if (ep->ep_state & EP_HAS_STREAMS) { + struct xhci_stream_ctx *ctx = + &ep->stream_info->stream_ctx_array[stream_id]; + state->new_cycle_state = 0x1 & le64_to_cpu(ctx->stream_ring); + } else { + struct xhci_ep_ctx *ep_ctx + = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index); + state->new_cycle_state = 0x1 & le64_to_cpu(ep_ctx->deq); + } state->new_deq_ptr = cur_td->last_trb; xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, -- cgit v0.10.2 From 95241dbdf8281ece1355b8673b882d6a182f3c7d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 4 Oct 2013 00:29:48 +0200 Subject: xhci: Set SCT field for Set TR dequeue on streams Nec XHCI controllers don't seem to care, but without this Intel XHCI controllers reject Set TR dequeue commands with a COMP_TRB_ERR, leading to the following warning: WARN Set TR Deq Ptr cmd invalid because of stream ID configuration And very shortly after this the system completely freezes. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 51c5109..f17e5c8 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -4073,6 +4073,7 @@ static int queue_set_tr_deq(struct xhci_hcd *xhci, int slot_id, u32 trb_slot_id = SLOT_ID_FOR_TRB(slot_id); u32 trb_ep_index = EP_ID_FOR_TRB(ep_index); u32 trb_stream_id = STREAM_ID_FOR_TRB(stream_id); + u32 trb_sct = 0; u32 type = TRB_TYPE(TRB_SET_DEQ); struct xhci_virt_ep *ep; @@ -4091,7 +4092,9 @@ static int queue_set_tr_deq(struct xhci_hcd *xhci, int slot_id, } ep->queued_deq_seg = deq_seg; ep->queued_deq_ptr = deq_ptr; - return queue_command(xhci, lower_32_bits(addr) | cycle_state, + if (stream_id) + trb_sct = SCT_FOR_TRB(SCT_PRI_TR); + return queue_command(xhci, lower_32_bits(addr) | trb_sct | cycle_state, upper_32_bits(addr), trb_stream_id, trb_slot_id | trb_ep_index | type, false); } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index a6aa98f..2c77bf7 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1118,9 +1118,10 @@ enum xhci_setup_dev { #define TRB_TO_SUSPEND_PORT(p) (((p) & (1 << 23)) >> 23) #define LAST_EP_INDEX 30 -/* Set TR Dequeue Pointer command TRB fields */ +/* Set TR Dequeue Pointer command TRB fields, 6.4.3.9 */ #define TRB_TO_STREAM_ID(p) ((((p) & (0xffff << 16)) >> 16)) #define STREAM_ID_FOR_TRB(p) ((((p)) & 0xffff) << 16) +#define SCT_FOR_TRB(p) (((p) << 1) & 0x7) /* Port Status Change Event TRB fields */ -- cgit v0.10.2 From 9aad95e292f58d00aa0f2e30c7f7dafd7fc7491c Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 4 Oct 2013 00:29:49 +0200 Subject: xhci: For streams the dequeue ptr must be read from the stream ctx This fixes TR dequeue validation failing on Intel XHCI controllers with the following warning: Mismatch between completed Set TR Deq Ptr command & xHCI internal state. Interestingly enough reading the deq ptr from the ep ctx after a TR Deq Ptr command does work on a Nec XHCI controller, it seems the Nec writes the ptr to both the ep and stream contexts when streams are used. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index f17e5c8..b3d27e6 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1081,12 +1081,14 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, unsigned int stream_id; struct xhci_ring *ep_ring; struct xhci_virt_device *dev; + struct xhci_virt_ep *ep; struct xhci_ep_ctx *ep_ctx; struct xhci_slot_ctx *slot_ctx; ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3])); stream_id = TRB_TO_STREAM_ID(le32_to_cpu(trb->generic.field[2])); dev = xhci->devs[slot_id]; + ep = &dev->eps[ep_index]; ep_ring = xhci_stream_id_to_ring(dev, ep_index, stream_id); if (!ep_ring) { @@ -1134,12 +1136,19 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, * cancelling URBs, which might not be an error... */ } else { + u64 deq; + /* 4.6.10 deq ptr is written to the stream ctx for streams */ + if (ep->ep_state & EP_HAS_STREAMS) { + struct xhci_stream_ctx *ctx = + &ep->stream_info->stream_ctx_array[stream_id]; + deq = le64_to_cpu(ctx->stream_ring) & SCTX_DEQ_MASK; + } else { + deq = le64_to_cpu(ep_ctx->deq) & ~EP_CTX_CYCLE_MASK; + } xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, - "Successful Set TR Deq Ptr cmd, deq = @%08llx", - le64_to_cpu(ep_ctx->deq)); - if (xhci_trb_virt_to_dma(dev->eps[ep_index].queued_deq_seg, - dev->eps[ep_index].queued_deq_ptr) == - (le64_to_cpu(ep_ctx->deq) & ~(EP_CTX_CYCLE_MASK))) { + "Successful Set TR Deq Ptr cmd, deq = @%08llx", deq); + if (xhci_trb_virt_to_dma(ep->queued_deq_seg, + ep->queued_deq_ptr) == deq) { /* Update the ring's dequeue segment and dequeue pointer * to reflect the new position. */ @@ -1148,8 +1157,7 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id, } else { xhci_warn(xhci, "Mismatch between completed Set TR Deq Ptr command & xHCI internal state.\n"); xhci_warn(xhci, "ep deq seg = %p, deq ptr = %p\n", - dev->eps[ep_index].queued_deq_seg, - dev->eps[ep_index].queued_deq_ptr); + ep->queued_deq_seg, ep->queued_deq_ptr); } } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 2c77bf7..d280e92 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -703,6 +703,7 @@ struct xhci_ep_ctx { /* deq bitmasks */ #define EP_CTX_CYCLE_MASK (1 << 0) +#define SCTX_DEQ_MASK (~0xfL) /** -- cgit v0.10.2 From a3901538611f8d5180df44f5b61293e70903958f Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 4 Oct 2013 17:05:55 +0200 Subject: xhci: use usb_ss_max_streams in xhci_check_streams_endpoint The ss_ep_comp bmAttributes filed can contain more info then just the streams, use usb_ss_max_streams to properly get max streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index c3fa32a..6cb9c22 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2971,7 +2971,7 @@ static int xhci_check_streams_endpoint(struct xhci_hcd *xhci, ret = xhci_check_args(xhci_to_hcd(xhci), udev, ep, 1, true, __func__); if (ret <= 0) return -EINVAL; - if (ep->ss_ep_comp.bmAttributes == 0) { + if (usb_ss_max_streams(&ep->ss_ep_comp) == 0) { xhci_warn(xhci, "WARN: SuperSpeed Endpoint Companion" " descriptor for ep 0x%x does not support streams\n", ep->desc.bEndpointAddress); -- cgit v0.10.2 From d57342232db459bf820b90e39496190965d7a775 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Thu, 17 Oct 2013 12:44:58 -0700 Subject: xhci: Remove segments from radix tree on failed insert. If we're expanding a stream ring, we want to make sure we can add those ring segments to the radix tree that maps segments to ring pointers. Try the radix tree insert after the new ring segments have been allocated (the last segment in the new ring chunk will point to the first newly allocated segment), but before the new ring segments are linked into the old ring. If insert fails on any one segment, remove each segment from the radix tree, deallocate the new segments, and return. Otherwise, link the new segments into the tree. HdG: Add a check to only update stream mappings in xhci_ring_expansion when the ring is a stream ring. Signed-off-by: Sarah Sharp Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index be7b7b6..edfb31a 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -180,53 +180,98 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring, * extended systems (where the DMA address can be bigger than 32-bits), * if we allow the PCI dma mask to be bigger than 32-bits. So don't do that. */ -static int xhci_update_stream_mapping(struct xhci_ring *ring, gfp_t mem_flags) +static int xhci_insert_segment_mapping(struct radix_tree_root *trb_address_map, + struct xhci_ring *ring, + struct xhci_segment *seg, + gfp_t mem_flags) { - struct xhci_segment *seg; unsigned long key; int ret; - if (WARN_ON_ONCE(ring->trb_address_map == NULL)) + key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); + /* Skip any segments that were already added. */ + if (radix_tree_lookup(trb_address_map, key)) return 0; - seg = ring->first_seg; - do { - key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); - /* Skip any segments that were already added. */ - if (radix_tree_lookup(ring->trb_address_map, key)) - continue; + ret = radix_tree_maybe_preload(mem_flags); + if (ret) + return ret; + ret = radix_tree_insert(trb_address_map, + key, ring); + radix_tree_preload_end(); + return ret; +} - ret = radix_tree_maybe_preload(mem_flags); - if (ret) - return ret; - ret = radix_tree_insert(ring->trb_address_map, - key, ring); - radix_tree_preload_end(); +static void xhci_remove_segment_mapping(struct radix_tree_root *trb_address_map, + struct xhci_segment *seg) +{ + unsigned long key; + + key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); + if (radix_tree_lookup(trb_address_map, key)) + radix_tree_delete(trb_address_map, key); +} + +static int xhci_update_stream_segment_mapping( + struct radix_tree_root *trb_address_map, + struct xhci_ring *ring, + struct xhci_segment *first_seg, + struct xhci_segment *last_seg, + gfp_t mem_flags) +{ + struct xhci_segment *seg; + struct xhci_segment *failed_seg; + int ret; + + if (WARN_ON_ONCE(trb_address_map == NULL)) + return 0; + + seg = first_seg; + do { + ret = xhci_insert_segment_mapping(trb_address_map, + ring, seg, mem_flags); if (ret) - return ret; + goto remove_streams; + if (seg == last_seg) + return 0; seg = seg->next; - } while (seg != ring->first_seg); + } while (seg != first_seg); return 0; + +remove_streams: + failed_seg = seg; + seg = first_seg; + do { + xhci_remove_segment_mapping(trb_address_map, seg); + if (seg == failed_seg) + return ret; + seg = seg->next; + } while (seg != first_seg); + + return ret; } static void xhci_remove_stream_mapping(struct xhci_ring *ring) { struct xhci_segment *seg; - unsigned long key; if (WARN_ON_ONCE(ring->trb_address_map == NULL)) return; seg = ring->first_seg; do { - key = (unsigned long)(seg->dma >> TRB_SEGMENT_SHIFT); - if (radix_tree_lookup(ring->trb_address_map, key)) - radix_tree_delete(ring->trb_address_map, key); + xhci_remove_segment_mapping(ring->trb_address_map, seg); seg = seg->next; } while (seg != ring->first_seg); } +static int xhci_update_stream_mapping(struct xhci_ring *ring, gfp_t mem_flags) +{ + return xhci_update_stream_segment_mapping(ring->trb_address_map, ring, + ring->first_seg, ring->last_seg, mem_flags); +} + /* XXX: Do we need the hcd structure in all these functions? */ void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring) { @@ -430,16 +475,26 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring, if (ret) return -ENOMEM; + if (ring->type == TYPE_STREAM) + ret = xhci_update_stream_segment_mapping(ring->trb_address_map, + ring, first, last, flags); + if (ret) { + struct xhci_segment *next; + do { + next = first->next; + xhci_segment_free(xhci, first); + if (first == last) + break; + first = next; + } while (true); + return ret; + } + xhci_link_rings(xhci, ring, first, last, num_segs); xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion, "ring expansion succeed, now has %d segments", ring->num_segs); - if (ring->type == TYPE_STREAM) { - ret = xhci_update_stream_mapping(ring, flags); - WARN_ON(ret); /* FIXME */ - } - return 0; } -- cgit v0.10.2 From 12d4bbcea727710bbd04de3e1de05957a0675e60 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:23 +0200 Subject: usb-core: Fix usb_free_streams return value documentation Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 2518c32..9b445f4 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2078,8 +2078,7 @@ EXPORT_SYMBOL_GPL(usb_alloc_streams); * Reverts a group of bulk endpoints back to not using stream IDs. * Can fail if we are given bad arguments, or HCD is broken. * - * Return: On success, the number of allocated streams. On failure, a negative - * error code. + * Return: 0 on success. On failure, a negative error code. */ int usb_free_streams(struct usb_interface *interface, struct usb_host_endpoint **eps, unsigned int num_eps, -- cgit v0.10.2 From 8f5d35441ff26b31e3812556ce468c76f1eb216b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:24 +0200 Subject: usb-core: Move USB_MAXENDPOINTS definitions to usb.h So that it can be used in other places too. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c index 8d72f0c..14ba398 100644 --- a/drivers/usb/core/config.c +++ b/drivers/usb/core/config.c @@ -10,7 +10,6 @@ #define USB_MAXALTSETTING 128 /* Hard limit */ -#define USB_MAXENDPOINTS 30 /* Hard limit */ #define USB_MAXCONFIG 8 /* Arbitrary limit */ diff --git a/include/linux/usb.h b/include/linux/usb.h index 7f6eb85..9b73dd7 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -202,6 +202,8 @@ static inline void usb_set_intfdata(struct usb_interface *intf, void *data) struct usb_interface *usb_get_intf(struct usb_interface *intf); void usb_put_intf(struct usb_interface *intf); +/* Hard limit */ +#define USB_MAXENDPOINTS 30 /* this maximum is arbitrary */ #define USB_MAXINTERFACES 32 #define USB_MAXIADS (USB_MAXINTERFACES/2) -- cgit v0.10.2 From 8d4f70b2fa52ca80f74faebc2471f74ee374cf61 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:25 +0200 Subject: usb-core: Track if an endpoint has streams This is a preparation patch for adding support for bulk streams to usbfs. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 9b445f4..9c4e292 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2049,7 +2049,7 @@ int usb_alloc_streams(struct usb_interface *interface, { struct usb_hcd *hcd; struct usb_device *dev; - int i; + int i, ret; dev = interface_to_usbdev(interface); hcd = bus_to_hcd(dev->bus); @@ -2058,13 +2058,24 @@ int usb_alloc_streams(struct usb_interface *interface, if (dev->speed != USB_SPEED_SUPER) return -EINVAL; - /* Streams only apply to bulk endpoints. */ - for (i = 0; i < num_eps; i++) + for (i = 0; i < num_eps; i++) { + /* Streams only apply to bulk endpoints. */ if (!usb_endpoint_xfer_bulk(&eps[i]->desc)) return -EINVAL; + /* Re-alloc is not allowed */ + if (eps[i]->streams) + return -EINVAL; + } - return hcd->driver->alloc_streams(hcd, dev, eps, num_eps, + ret = hcd->driver->alloc_streams(hcd, dev, eps, num_eps, num_streams, mem_flags); + if (ret < 0) + return ret; + + for (i = 0; i < num_eps; i++) + eps[i]->streams = ret; + + return ret; } EXPORT_SYMBOL_GPL(usb_alloc_streams); @@ -2086,19 +2097,26 @@ int usb_free_streams(struct usb_interface *interface, { struct usb_hcd *hcd; struct usb_device *dev; - int i; + int i, ret; dev = interface_to_usbdev(interface); hcd = bus_to_hcd(dev->bus); if (dev->speed != USB_SPEED_SUPER) return -EINVAL; - /* Streams only apply to bulk endpoints. */ + /* Double-free is not allowed */ for (i = 0; i < num_eps; i++) - if (!eps[i] || !usb_endpoint_xfer_bulk(&eps[i]->desc)) + if (!eps[i] || !eps[i]->streams) return -EINVAL; - return hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags); + ret = hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags); + if (ret < 0) + return ret; + + for (i = 0; i < num_eps; i++) + eps[i]->streams = 0; + + return ret; } EXPORT_SYMBOL_GPL(usb_free_streams); diff --git a/include/linux/usb.h b/include/linux/usb.h index 9b73dd7..f1015ce 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -57,6 +57,7 @@ struct ep_device; * @extra: descriptors following this endpoint in the configuration * @extralen: how many bytes of "extra" are valid * @enabled: URBs may be submitted to this endpoint + * @streams: number of USB-3 streams allocated on the endpoint * * USB requests are always queued to a given endpoint, identified by a * descriptor within an active interface in a given USB configuration. @@ -71,6 +72,7 @@ struct usb_host_endpoint { unsigned char *extra; /* Extra descriptors */ int extralen; int enabled; + int streams; }; /* host-side wrapper for one interface setting's parsed descriptors */ -- cgit v0.10.2 From 6343e8bf09de16ab4dcae2c6ca1a0e8dbd4dd770 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:26 +0200 Subject: usb-core: Free bulk streams on interface release Documentation/usb/bulk-streams.txt says: All stream IDs will be deallocated when the driver releases the interface, to ensure that drivers that don't support streams will be able to use the endpoint This commit actually implements this. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 85e0450..08283d4 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -400,8 +400,9 @@ static int usb_unbind_interface(struct device *dev) { struct usb_driver *driver = to_usb_driver(dev->driver); struct usb_interface *intf = to_usb_interface(dev); + struct usb_host_endpoint *ep, **eps = NULL; struct usb_device *udev; - int error, r, lpm_disable_error; + int i, j, error, r, lpm_disable_error; intf->condition = USB_INTERFACE_UNBINDING; @@ -425,6 +426,26 @@ static int usb_unbind_interface(struct device *dev) driver->disconnect(intf); usb_cancel_queued_reset(intf); + /* Free streams */ + for (i = 0, j = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) { + ep = &intf->cur_altsetting->endpoint[i]; + if (ep->streams == 0) + continue; + if (j == 0) { + eps = kmalloc(USB_MAXENDPOINTS * sizeof(void *), + GFP_KERNEL); + if (!eps) { + dev_warn(dev, "oom, leaking streams\n"); + break; + } + } + eps[j++] = ep; + } + if (j) { + usb_free_streams(intf, eps, j, GFP_KERNEL); + kfree(eps); + } + /* Reset other interface state. * We cannot do a Set-Interface if the device is suspended or * if it is prepared for a system sleep (since installing a new -- cgit v0.10.2 From 5ec9c1771ce83a1e2b7ec96ed9f29a9f1b25e71e Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:27 +0200 Subject: usbfs: Kill urbs on interface before doing a set_interface The usb_set_interface documentation says: * Also, drivers must not change altsettings while urbs are scheduled for * endpoints in that interface; all such urbs must first be completed * (perhaps forced by unlinking). For in kernel drivers we trust the drivers to get this right, but we cannot trust userspace to get this right, so enforce it by killing any urbs still pending on the interface. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index f3ba2e0..2a95e4e 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1143,6 +1143,9 @@ static int proc_setintf(struct dev_state *ps, void __user *arg) return -EFAULT; if ((ret = checkintf(ps, setintf.interface))) return ret; + + destroy_async_on_interface(ps, setintf.interface); + return usb_set_interface(ps->dev, setintf.interface, setintf.altsetting); } -- cgit v0.10.2 From b2d03eb56e66620a9b27f1a0c2795722087effc9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:28 +0200 Subject: usbfs: proc_do_submiturb use a local variable for number_of_packets This is a preparation patch for adding support for bulk streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 2a95e4e..c88d8bf 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1208,6 +1208,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, struct usb_ctrlrequest *dr = NULL; unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; + int number_of_packets = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1261,7 +1262,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, le16_to_cpup(&dr->wIndex)); if (ret) goto error; - uurb->number_of_packets = 0; uurb->buffer_length = le16_to_cpup(&dr->wLength); uurb->buffer += 8; if ((dr->bRequestType & USB_DIR_IN) && uurb->buffer_length) { @@ -1291,7 +1291,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, uurb->type = USBDEVFS_URB_TYPE_INTERRUPT; goto interrupt_urb; } - uurb->number_of_packets = 0; num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; @@ -1301,7 +1300,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, if (!usb_endpoint_xfer_int(&ep->desc)) return -EINVAL; interrupt_urb: - uurb->number_of_packets = 0; break; case USBDEVFS_URB_TYPE_ISO: @@ -1311,15 +1309,16 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, return -EINVAL; if (!usb_endpoint_xfer_isoc(&ep->desc)) return -EINVAL; + number_of_packets = uurb->number_of_packets; isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) * - uurb->number_of_packets; + number_of_packets; if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL))) return -ENOMEM; if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) { ret = -EFAULT; goto error; } - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { /* * arbitrary limit need for USB 3.0 * bMaxBurst (0~15 allowed, 1~16 packets) @@ -1350,7 +1349,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ret = -EFAULT; goto error; } - as = alloc_async(uurb->number_of_packets); + as = alloc_async(number_of_packets); if (!as) { ret = -ENOMEM; goto error; @@ -1444,7 +1443,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->setup_packet = (unsigned char *)dr; dr = NULL; as->urb->start_frame = uurb->start_frame; - as->urb->number_of_packets = uurb->number_of_packets; + as->urb->number_of_packets = number_of_packets; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); @@ -1452,7 +1451,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->interval = ep->desc.bInterval; as->urb->context = as; as->urb->complete = async_completed; - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { as->urb->iso_frame_desc[u].offset = totlen; as->urb->iso_frame_desc[u].length = isopkt[u].length; totlen += isopkt[u].length; -- cgit v0.10.2 From 948cd8c18c466fdcbe707bb2a42a148796bfccdd Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:29 +0200 Subject: usbfs: Add support for bulk stream ids This patch makes it possible to specify a bulk stream id when submitting an urb using the async usbfs API. It overloads the number_of_packets usbdevfs_urb field for this. This is not pretty, but given other constraints it is the best we can do. The reasoning leading to this goes as follows: 1) We want to support bulk streams in the usbfs API 2) We do not want to extend the usbdevfs_urb struct with a new member, as that would mean defining new ioctl numbers for all async API ioctls + adding compat versions for the old ones (times 2 for 32 bit support) 3) 1 + 2 means we need to re-use an existing field 4) number_of_packets is only used for isoc urbs, and streams are bulk only so it is the best (and only) candidate for re-using Note that: 1) This patch only uses number_of_packets as stream_id if the app has actually allocated streams on the ep, so that old apps which may have garbage in there (as it was unused until now in the bulk case), will not break 2) This patch does not add support for allocating / freeing bulk-streams, that is done in a follow up patch Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index c88d8bf..d7571a6 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1209,6 +1209,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; int number_of_packets = 0; + unsigned int stream_id = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1294,6 +1295,8 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; + if (ep->streams) + stream_id = uurb->stream_id; break; case USBDEVFS_URB_TYPE_INTERRUPT: @@ -1444,6 +1447,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, dr = NULL; as->urb->start_frame = uurb->start_frame; as->urb->number_of_packets = number_of_packets; + as->urb->stream_id = stream_id; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h index 0c65e4b..cbf122d 100644 --- a/include/uapi/linux/usbdevice_fs.h +++ b/include/uapi/linux/usbdevice_fs.h @@ -102,7 +102,10 @@ struct usbdevfs_urb { int buffer_length; int actual_length; int start_frame; - int number_of_packets; + union { + int number_of_packets; /* Only used for isoc urbs */ + unsigned int stream_id; /* Only used with bulk streams */ + }; int error_count; unsigned int signr; /* signal to be sent on completion, or 0 if none should be sent. */ -- cgit v0.10.2 From 2fec32b06e374642802f7fb4f5350317cd14732b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:30 +0200 Subject: usbfs: Add ep_to_host_endpoint helper function Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index d7571a6..502974b 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -769,6 +769,15 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, return ret; } +static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, + unsigned char ep) +{ + if (ep & USB_ENDPOINT_DIR_MASK) + return dev->ep_in[ep & USB_ENDPOINT_NUMBER_MASK]; + else + return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -1230,15 +1239,10 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, if (ret) return ret; } - if ((uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0) { - is_in = 1; - ep = ps->dev->ep_in[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } else { - is_in = 0; - ep = ps->dev->ep_out[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } + ep = ep_to_host_endpoint(ps->dev, uurb->endpoint); if (!ep) return -ENOENT; + is_in = (uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0; u = 0; switch(uurb->type) { -- cgit v0.10.2 From bcf7f6e39335af4f03da8c26a98185fd49754fcc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:31 +0200 Subject: usbfs: Add support for allocating / freeing streams This allows userspace to use bulk-streams, just like in kernel drivers, see Documentation/usb/bulk-streams.txt for details on the in kernel API. This is exported pretty much one on one to userspace. To use streams an app must first make a USBDEVFS_ALLOC_STREAMS ioctl, on success this will return the number of streams available (which may be less then requested). If there are n streams the app can then submit usbdevfs_urb-s with their stream_id member set to 1-n to use a specific stream. IE if USBDEVFS_ALLOC_STREAMS returns 4 then stream_id 1-4 can be used. When the app is done using streams it should call USBDEVFS_FREE_STREAMS Note applications are advised to use libusb rather then using the usbdevfs api directly. The latest version of libusb has support for streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 502974b..12401ee4 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -778,6 +778,79 @@ static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; } +static int parse_usbdevfs_streams(struct dev_state *ps, + struct usbdevfs_streams __user *streams, + unsigned int *num_streams_ret, + unsigned int *num_eps_ret, + struct usb_host_endpoint ***eps_ret, + struct usb_interface **intf_ret) +{ + unsigned int i, num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf = NULL; + unsigned char ep; + int ifnum, ret; + + if (get_user(num_streams, &streams->num_streams) || + get_user(num_eps, &streams->num_eps)) + return -EFAULT; + + if (num_eps < 1 || num_eps > USB_MAXENDPOINTS) + return -EINVAL; + + /* The XHCI controller allows max 2 ^ 16 streams */ + if (num_streams_ret && (num_streams < 2 || num_streams > 65536)) + return -EINVAL; + + eps = kmalloc(num_eps * sizeof(*eps), GFP_KERNEL); + if (!eps) + return -ENOMEM; + + for (i = 0; i < num_eps; i++) { + if (get_user(ep, &streams->eps[i])) { + ret = -EFAULT; + goto error; + } + eps[i] = ep_to_host_endpoint(ps->dev, ep); + if (!eps[i]) { + ret = -EINVAL; + goto error; + } + + /* usb_alloc/free_streams operate on an usb_interface */ + ifnum = findintfep(ps->dev, ep); + if (ifnum < 0) { + ret = ifnum; + goto error; + } + + if (i == 0) { + ret = checkintf(ps, ifnum); + if (ret < 0) + goto error; + intf = usb_ifnum_to_if(ps->dev, ifnum); + } else { + /* Verify all eps belong to the same interface */ + if (ifnum != intf->altsetting->desc.bInterfaceNumber) { + ret = -EINVAL; + goto error; + } + } + } + + if (num_streams_ret) + *num_streams_ret = num_streams; + *num_eps_ret = num_eps; + *eps_ret = eps; + *intf_ret = intf; + + return 0; + +error: + kfree(eps); + return ret; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -2009,6 +2082,45 @@ static int proc_disconnect_claim(struct dev_state *ps, void __user *arg) return claimintf(ps, dc.interface); } +static int proc_alloc_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps, arg, &num_streams, &num_eps, + &eps, &intf); + if (r) + return r; + + destroy_async_on_interface(ps, + intf->altsetting[0].desc.bInterfaceNumber); + + r = usb_alloc_streams(intf, eps, num_eps, num_streams, GFP_KERNEL); + kfree(eps); + return r; +} + +static int proc_free_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps, arg, NULL, &num_eps, &eps, &intf); + if (r) + return r; + + destroy_async_on_interface(ps, + intf->altsetting[0].desc.bInterfaceNumber); + + r = usb_free_streams(intf, eps, num_eps, GFP_KERNEL); + kfree(eps); + return r; +} + /* * NOTE: All requests here that have interface numbers as parameters * are assuming that somehow the configuration has been prevented from @@ -2185,6 +2297,12 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, case USBDEVFS_DISCONNECT_CLAIM: ret = proc_disconnect_claim(ps, p); break; + case USBDEVFS_ALLOC_STREAMS: + ret = proc_alloc_streams(ps, p); + break; + case USBDEVFS_FREE_STREAMS: + ret = proc_free_streams(ps, p); + break; } usb_unlock_device(dev); if (ret >= 0) diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h index cbf122d..abe5f4b 100644 --- a/include/uapi/linux/usbdevice_fs.h +++ b/include/uapi/linux/usbdevice_fs.h @@ -147,6 +147,11 @@ struct usbdevfs_disconnect_claim { char driver[USBDEVFS_MAXDRIVERNAME + 1]; }; +struct usbdevfs_streams { + unsigned int num_streams; /* Not used by USBDEVFS_FREE_STREAMS */ + unsigned int num_eps; + unsigned char eps[0]; +}; #define USBDEVFS_CONTROL _IOWR('U', 0, struct usbdevfs_ctrltransfer) #define USBDEVFS_CONTROL32 _IOWR('U', 0, struct usbdevfs_ctrltransfer32) @@ -179,5 +184,7 @@ struct usbdevfs_disconnect_claim { #define USBDEVFS_RELEASE_PORT _IOR('U', 25, unsigned int) #define USBDEVFS_GET_CAPABILITIES _IOR('U', 26, __u32) #define USBDEVFS_DISCONNECT_CLAIM _IOR('U', 27, struct usbdevfs_disconnect_claim) +#define USBDEVFS_ALLOC_STREAMS _IOR('U', 28, struct usbdevfs_streams) +#define USBDEVFS_FREE_STREAMS _IOR('U', 29, struct usbdevfs_streams) #endif /* _UAPI_LINUX_USBDEVICE_FS_H */ -- cgit v0.10.2 From d89bd835326947e6618b97469159d3266016fe0a Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 13 Sep 2013 13:27:11 +0200 Subject: uas: properly reinitialize in uas_eh_bus_reset_handler Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index d966b59..fc08ee9 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -85,6 +85,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo, gfp_t gfp); static void uas_do_work(struct work_struct *work); static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); +static void uas_configure_endpoints(struct uas_dev_info *devinfo); +static void uas_free_streams(struct uas_dev_info *devinfo); static DECLARE_WORK(uas_work, uas_do_work); static DEFINE_SPINLOCK(uas_work_lock); @@ -800,7 +802,10 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); + uas_free_streams(devinfo); err = usb_reset_device(udev); + if (!err) + uas_configure_endpoints(devinfo); devinfo->resetting = 0; if (err) { -- cgit v0.10.2 From 1bf8198e6b2da3e30960e95f8d215f44572515ce Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 13 Sep 2013 13:27:12 +0200 Subject: uas: make work list per-device Simplifies locking, we'll protect the list with the device spin lock. Also plugs races which can happen when two devices operate on the global list. While being at it rename the list head from "list" to "work", preparing for the addition of a second list. Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index fc08ee9..3cf5a5f 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -51,6 +51,8 @@ struct uas_dev_info { unsigned uas_sense_old:1; struct scsi_cmnd *cmnd; spinlock_t lock; + struct work_struct work; + struct list_head work_list; }; enum { @@ -77,7 +79,7 @@ struct uas_cmd_info { struct urb *cmd_urb; struct urb *data_in_urb; struct urb *data_out_urb; - struct list_head list; + struct list_head work; }; /* I hate forward declarations, but I actually have a loop */ @@ -88,10 +90,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); -static DECLARE_WORK(uas_work, uas_do_work); -static DEFINE_SPINLOCK(uas_work_lock); -static LIST_HEAD(uas_work_list); - static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, struct uas_cmd_info *cmdinfo) { @@ -118,75 +116,66 @@ static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, static void uas_do_work(struct work_struct *work) { + struct uas_dev_info *devinfo = + container_of(work, struct uas_dev_info, work); struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; - struct list_head list; unsigned long flags; int err; - spin_lock_irq(&uas_work_lock); - list_replace_init(&uas_work_list, &list); - spin_unlock_irq(&uas_work_lock); - - list_for_each_entry_safe(cmdinfo, temp, &list, list) { + spin_lock_irqsave(&devinfo->lock, flags); + list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { struct scsi_pointer *scp = (void *)cmdinfo; - struct scsi_cmnd *cmnd = container_of(scp, - struct scsi_cmnd, SCp); - struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; - spin_lock_irqsave(&devinfo->lock, flags); + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, + SCp); err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC); - if (!err) + if (!err) { cmdinfo->state &= ~IS_IN_WORK_LIST; - spin_unlock_irqrestore(&devinfo->lock, flags); - if (err) { - list_del(&cmdinfo->list); - spin_lock_irq(&uas_work_lock); - list_add_tail(&cmdinfo->list, &uas_work_list); - spin_unlock_irq(&uas_work_lock); - schedule_work(&uas_work); + list_del(&cmdinfo->work); + } else { + schedule_work(&devinfo->work); } } + spin_unlock_irqrestore(&devinfo->lock, flags); } static void uas_abort_work(struct uas_dev_info *devinfo) { struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; - struct list_head list; unsigned long flags; - spin_lock_irq(&uas_work_lock); - list_replace_init(&uas_work_list, &list); - spin_unlock_irq(&uas_work_lock); - spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &list, list) { + list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { struct scsi_pointer *scp = (void *)cmdinfo; - struct scsi_cmnd *cmnd = container_of(scp, - struct scsi_cmnd, SCp); - struct uas_dev_info *di = (void *)cmnd->device->hostdata; - - if (di == devinfo) { - cmdinfo->state |= COMMAND_ABORTED; - cmdinfo->state &= ~IS_IN_WORK_LIST; - if (devinfo->resetting) { - /* uas_stat_cmplt() will not do that - * when a device reset is in - * progress */ - cmdinfo->state &= ~COMMAND_INFLIGHT; - } - uas_try_complete(cmnd, __func__); - } else { - /* not our uas device, relink into list */ - list_del(&cmdinfo->list); - spin_lock_irq(&uas_work_lock); - list_add_tail(&cmdinfo->list, &uas_work_list); - spin_unlock_irq(&uas_work_lock); + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, + SCp); + cmdinfo->state |= COMMAND_ABORTED; + cmdinfo->state &= ~IS_IN_WORK_LIST; + if (devinfo->resetting) { + /* uas_stat_cmplt() will not do that + * when a device reset is in + * progress */ + cmdinfo->state &= ~COMMAND_INFLIGHT; } + uas_try_complete(cmnd, __func__); + list_del(&cmdinfo->work); } spin_unlock_irqrestore(&devinfo->lock, flags); } +static void uas_add_work(struct uas_cmd_info *cmdinfo) +{ + struct scsi_pointer *scp = (void *)cmdinfo; + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); + struct uas_dev_info *devinfo = cmnd->device->hostdata; + + WARN_ON(!spin_is_locked(&devinfo->lock)); + list_add_tail(&cmdinfo->work, &devinfo->work_list); + cmdinfo->state |= IS_IN_WORK_LIST; + schedule_work(&devinfo->work); +} + static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd) { struct sense_iu *sense_iu = urb->transfer_buffer; @@ -288,11 +277,7 @@ static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd, cmdinfo->state |= direction | SUBMIT_STATUS_URB; err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC); if (err) { - spin_lock(&uas_work_lock); - list_add_tail(&cmdinfo->list, &uas_work_list); - cmdinfo->state |= IS_IN_WORK_LIST; - spin_unlock(&uas_work_lock); - schedule_work(&uas_work); + uas_add_work(cmdinfo); } } @@ -694,11 +679,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, spin_unlock_irqrestore(&devinfo->lock, flags); return SCSI_MLQUEUE_DEVICE_BUSY; } - spin_lock(&uas_work_lock); - list_add_tail(&cmdinfo->list, &uas_work_list); - cmdinfo->state |= IS_IN_WORK_LIST; - spin_unlock(&uas_work_lock); - schedule_work(&uas_work); + uas_add_work(cmdinfo); } spin_unlock_irqrestore(&devinfo->lock, flags); @@ -764,10 +745,8 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) spin_lock_irqsave(&devinfo->lock, flags); cmdinfo->state |= COMMAND_ABORTED; if (cmdinfo->state & IS_IN_WORK_LIST) { - spin_lock(&uas_work_lock); - list_del(&cmdinfo->list); + list_del(&cmdinfo->work); cmdinfo->state &= ~IS_IN_WORK_LIST; - spin_unlock(&uas_work_lock); } if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); @@ -1007,6 +986,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) init_usb_anchor(&devinfo->sense_urbs); init_usb_anchor(&devinfo->data_urbs); spin_lock_init(&devinfo->lock); + INIT_WORK(&devinfo->work, uas_do_work); + INIT_LIST_HEAD(&devinfo->work_list); uas_configure_endpoints(devinfo); result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); @@ -1050,6 +1031,7 @@ static void uas_disconnect(struct usb_interface *intf) struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; devinfo->resetting = 1; + cancel_work_sync(&devinfo->work); uas_abort_work(devinfo); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); -- cgit v0.10.2 From 326349f82461918830ed59913a3ccd8ffa9ac46f Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 13 Sep 2013 13:27:13 +0200 Subject: uas: add dead request list This patch adds a new list where all requests which are canceled are added to, so we don't loose them. Then, after killing all inflight urbs on bus reset (and disconnect) we'll walk over the list and clean them up. Without this we can end up with aborted requests lingering around in case of status pipe transfer errors. Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3cf5a5f..f049038 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -53,6 +53,7 @@ struct uas_dev_info { spinlock_t lock; struct work_struct work; struct list_head work_list; + struct list_head dead_list; }; enum { @@ -80,6 +81,7 @@ struct uas_cmd_info { struct urb *data_in_urb; struct urb *data_out_urb; struct list_head work; + struct list_head dead; }; /* I hate forward declarations, but I actually have a loop */ @@ -89,6 +91,7 @@ static void uas_do_work(struct work_struct *work); static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); +static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller); static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, struct uas_cmd_info *cmdinfo) @@ -150,16 +153,12 @@ static void uas_abort_work(struct uas_dev_info *devinfo) struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); + uas_log_cmd_state(cmnd, __func__); + WARN_ON(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; - if (devinfo->resetting) { - /* uas_stat_cmplt() will not do that - * when a device reset is in - * progress */ - cmdinfo->state &= ~COMMAND_INFLIGHT; - } - uas_try_complete(cmnd, __func__); list_del(&cmdinfo->work); + list_add_tail(&cmdinfo->dead, &devinfo->dead_list); } spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -176,6 +175,28 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) schedule_work(&devinfo->work); } +static void uas_zap_dead(struct uas_dev_info *devinfo) +{ + struct uas_cmd_info *cmdinfo; + struct uas_cmd_info *temp; + unsigned long flags; + + spin_lock_irqsave(&devinfo->lock, flags); + list_for_each_entry_safe(cmdinfo, temp, &devinfo->dead_list, dead) { + struct scsi_pointer *scp = (void *)cmdinfo; + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, + SCp); + uas_log_cmd_state(cmnd, __func__); + WARN_ON(!(cmdinfo->state & COMMAND_ABORTED)); + /* all urbs are killed, clear inflight bits */ + cmdinfo->state &= ~(COMMAND_INFLIGHT | + DATA_IN_URB_INFLIGHT | + DATA_OUT_URB_INFLIGHT); + uas_try_complete(cmnd, __func__); + } + spin_unlock_irqrestore(&devinfo->lock, flags); +} + static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd) { struct sense_iu *sense_iu = urb->transfer_buffer; @@ -263,6 +284,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) if (cmdinfo->state & COMMAND_ABORTED) { scmd_printk(KERN_INFO, cmnd, "abort completed\n"); cmnd->result = DID_ABORT << 16; + list_del(&cmdinfo->dead); } cmnd->scsi_done(cmnd); return 0; @@ -292,7 +314,13 @@ static void uas_stat_cmplt(struct urb *urb) u16 tag; if (urb->status) { - dev_err(&urb->dev->dev, "URB BAD STATUS %d\n", urb->status); + if (urb->status == -ENOENT) { + dev_err(&urb->dev->dev, "stat urb: killed, stream %d\n", + urb->stream_id); + } else { + dev_err(&urb->dev->dev, "stat urb: status %d\n", + urb->status); + } usb_free_urb(urb); return; } @@ -743,7 +771,9 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) uas_log_cmd_state(cmnd, __func__); spin_lock_irqsave(&devinfo->lock, flags); + WARN_ON(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; + list_add_tail(&cmdinfo->dead, &devinfo->dead_list); if (cmdinfo->state & IS_IN_WORK_LIST) { list_del(&cmdinfo->work); cmdinfo->state &= ~IS_IN_WORK_LIST; @@ -776,11 +806,13 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) struct usb_device *udev = devinfo->udev; int err; + shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); devinfo->resetting = 1; uas_abort_work(devinfo); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); + uas_zap_dead(devinfo); uas_free_streams(devinfo); err = usb_reset_device(udev); if (!err) @@ -988,6 +1020,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) spin_lock_init(&devinfo->lock); INIT_WORK(&devinfo->work, uas_do_work); INIT_LIST_HEAD(&devinfo->work_list); + INIT_LIST_HEAD(&devinfo->dead_list); uas_configure_endpoints(devinfo); result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); @@ -1036,6 +1069,7 @@ static void uas_disconnect(struct usb_interface *intf) usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); + uas_zap_dead(devinfo); scsi_remove_host(shost); uas_free_streams(devinfo); kfree(devinfo); -- cgit v0.10.2 From f491ecbb47d5a709d46401da3cd54ff8ee666ca0 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 13 Sep 2013 13:27:14 +0200 Subject: uas: replace BUG_ON() + WARN_ON() with WARN_ON_ONCE() Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index f049038..046eedf 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -154,7 +154,7 @@ static void uas_abort_work(struct uas_dev_info *devinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, __func__); - WARN_ON(cmdinfo->state & COMMAND_ABORTED); + WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; list_del(&cmdinfo->work); @@ -169,7 +169,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); struct uas_dev_info *devinfo = cmnd->device->hostdata; - WARN_ON(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); list_add_tail(&cmdinfo->work, &devinfo->work_list); cmdinfo->state |= IS_IN_WORK_LIST; schedule_work(&devinfo->work); @@ -187,7 +187,7 @@ static void uas_zap_dead(struct uas_dev_info *devinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, __func__); - WARN_ON(!(cmdinfo->state & COMMAND_ABORTED)); + WARN_ON_ONCE(!(cmdinfo->state & COMMAND_ABORTED)); /* all urbs are killed, clear inflight bits */ cmdinfo->state &= ~(COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT | @@ -271,13 +271,13 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; - WARN_ON(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & (COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT | DATA_OUT_URB_INFLIGHT | UNLINK_DATA_URBS)) return -EBUSY; - BUG_ON(cmdinfo->state & COMMAND_COMPLETED); + WARN_ON_ONCE(cmdinfo->state & COMMAND_COMPLETED); cmdinfo->state |= COMMAND_COMPLETED; usb_free_urb(cmdinfo->data_in_urb); usb_free_urb(cmdinfo->data_out_urb); @@ -398,8 +398,9 @@ static void uas_data_cmplt(struct urb *urb) sdb = scsi_out(cmnd); cmdinfo->state &= ~DATA_OUT_URB_INFLIGHT; } - BUG_ON(sdb == NULL); - if (urb->status) { + if (sdb == NULL) { + WARN_ON_ONCE(1); + } else if (urb->status) { /* error: no data transfered */ sdb->resid = sdb->length; } else { @@ -573,7 +574,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; int err; - WARN_ON(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & SUBMIT_STATUS_URB) { err = uas_submit_sense_urb(cmnd->device->host, gfp, cmdinfo->stream); @@ -771,7 +772,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) uas_log_cmd_state(cmnd, __func__); spin_lock_irqsave(&devinfo->lock, flags); - WARN_ON(cmdinfo->state & COMMAND_ABORTED); + WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; list_add_tail(&cmdinfo->dead, &devinfo->dead_list); if (cmdinfo->state & IS_IN_WORK_LIST) { -- cgit v0.10.2 From d5f808d3f88e90b13a4839e07b3dc6e715e2ba88 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 19:30:26 +0200 Subject: uas: Urbs must be anchored before submitting them Otherwise they may complete before they get anchored and thus never get unanchored (as the unanchoring is done by the usb core on completion). This commit also remove the usb_get_urb / usb_put_urb around cmd submission + anchoring, since if done in the proper order this is not necessary. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 046eedf..059ce62 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -531,10 +531,12 @@ static int uas_submit_task_urb(struct scsi_cmnd *cmnd, gfp_t gfp, usb_free_urb, NULL); urb->transfer_flags |= URB_FREE_BUFFER; + usb_anchor_urb(urb, &devinfo->cmd_urbs); err = usb_submit_urb(urb, gfp); - if (err) + if (err) { + usb_unanchor_urb(urb); goto err; - usb_anchor_urb(urb, &devinfo->cmd_urbs); + } return 0; @@ -558,13 +560,14 @@ static int uas_submit_sense_urb(struct Scsi_Host *shost, urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); if (!urb) return SCSI_MLQUEUE_DEVICE_BUSY; + usb_anchor_urb(urb, &devinfo->sense_urbs); if (usb_submit_urb(urb, gfp)) { + usb_unanchor_urb(urb); shost_printk(KERN_INFO, shost, "sense urb submission failure\n"); usb_free_urb(urb); return SCSI_MLQUEUE_DEVICE_BUSY; } - usb_anchor_urb(urb, &devinfo->sense_urbs); return 0; } @@ -594,14 +597,15 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & SUBMIT_DATA_IN_URB) { + usb_anchor_urb(cmdinfo->data_in_urb, &devinfo->data_urbs); if (usb_submit_urb(cmdinfo->data_in_urb, gfp)) { + usb_unanchor_urb(cmdinfo->data_in_urb); scmd_printk(KERN_INFO, cmnd, "data in urb submission failure\n"); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_IN_URB; cmdinfo->state |= DATA_IN_URB_INFLIGHT; - usb_anchor_urb(cmdinfo->data_in_urb, &devinfo->data_urbs); } if (cmdinfo->state & ALLOC_DATA_OUT_URB) { @@ -614,14 +618,15 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & SUBMIT_DATA_OUT_URB) { + usb_anchor_urb(cmdinfo->data_out_urb, &devinfo->data_urbs); if (usb_submit_urb(cmdinfo->data_out_urb, gfp)) { + usb_unanchor_urb(cmdinfo->data_out_urb); scmd_printk(KERN_INFO, cmnd, "data out urb submission failure\n"); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_OUT_URB; cmdinfo->state |= DATA_OUT_URB_INFLIGHT; - usb_anchor_urb(cmdinfo->data_out_urb, &devinfo->data_urbs); } if (cmdinfo->state & ALLOC_CMD_URB) { @@ -633,14 +638,13 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & SUBMIT_CMD_URB) { - usb_get_urb(cmdinfo->cmd_urb); + usb_anchor_urb(cmdinfo->cmd_urb, &devinfo->cmd_urbs); if (usb_submit_urb(cmdinfo->cmd_urb, gfp)) { + usb_unanchor_urb(cmdinfo->cmd_urb); scmd_printk(KERN_INFO, cmnd, "cmd urb submission failure\n"); return SCSI_MLQUEUE_DEVICE_BUSY; } - usb_anchor_urb(cmdinfo->cmd_urb, &devinfo->cmd_urbs); - usb_put_urb(cmdinfo->cmd_urb); cmdinfo->cmd_urb = NULL; cmdinfo->state &= ~SUBMIT_CMD_URB; cmdinfo->state |= COMMAND_INFLIGHT; -- cgit v0.10.2 From 6ce8213b3328ae4a1db34339c282144740ac1ec6 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 19:00:45 +0200 Subject: uas: Properly set interface to altsetting 0 on probe failure - Rename labels to properly reflect this - Don't skip free-ing the streams when scsi_init_shared_tag_map fails Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 059ce62..ec1b22d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -993,8 +993,8 @@ static void uas_free_streams(struct uas_dev_info *devinfo) */ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) { - int result; - struct Scsi_Host *shost; + int result = -ENOMEM; + struct Scsi_Host *shost = NULL; struct uas_dev_info *devinfo; struct usb_device *udev = interface_to_usbdev(intf); @@ -1003,12 +1003,11 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) devinfo = kmalloc(sizeof(struct uas_dev_info), GFP_KERNEL); if (!devinfo) - return -ENOMEM; + goto set_alt0; - result = -ENOMEM; shost = scsi_host_alloc(&uas_host_template, sizeof(void *)); if (!shost) - goto free; + goto set_alt0; shost->max_cmd_len = 16 + 252; shost->max_id = 1; @@ -1030,11 +1029,11 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); if (result) - goto free; + goto free_streams; result = scsi_add_host(shost, &intf->dev); if (result) - goto deconfig_eps; + goto free_streams; shost->hostdata[0] = (unsigned long)devinfo; @@ -1042,9 +1041,10 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) usb_set_intfdata(intf, shost); return result; -deconfig_eps: +free_streams: uas_free_streams(devinfo); - free: +set_alt0: + usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); kfree(devinfo); if (shost) scsi_host_put(shost); -- cgit v0.10.2 From 7e50e0bec45897caeb978e5aecc9184a2dc00df2 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 19:19:04 +0200 Subject: uas: Avoid unnecessary unlock / lock calls around unlink_data_urbs All callers of unlink_data_urbs drop devinfo->lock before calling it, and then immediately take it again after the call. And the first thing unlink_data_urbs does is take the lock again, and the last thing it does is drop it. This commit removes all the unnecessary lock dropping and taking. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index ec1b22d..dcaf6119 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -93,28 +93,26 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller); +/* Must be called with devinfo->lock held, will temporary unlock the lock */ static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, - struct uas_cmd_info *cmdinfo) + struct uas_cmd_info *cmdinfo, + unsigned long *lock_flags) { - unsigned long flags; - /* * The UNLINK_DATA_URBS flag makes sure uas_try_complete * (called by urb completion) doesn't release cmdinfo * underneath us. */ - spin_lock_irqsave(&devinfo->lock, flags); cmdinfo->state |= UNLINK_DATA_URBS; - spin_unlock_irqrestore(&devinfo->lock, flags); + spin_unlock_irqrestore(&devinfo->lock, *lock_flags); if (cmdinfo->data_in_urb) usb_unlink_urb(cmdinfo->data_in_urb); if (cmdinfo->data_out_urb) usb_unlink_urb(cmdinfo->data_out_urb); - spin_lock_irqsave(&devinfo->lock, flags); + spin_lock_irqsave(&devinfo->lock, *lock_flags); cmdinfo->state &= ~UNLINK_DATA_URBS; - spin_unlock_irqrestore(&devinfo->lock, flags); } static void uas_do_work(struct work_struct *work) @@ -361,9 +359,7 @@ static void uas_stat_cmplt(struct urb *urb) uas_sense(urb, cmnd); if (cmnd->result != 0) { /* cancel data transfers on error */ - spin_unlock_irqrestore(&devinfo->lock, flags); - uas_unlink_data_urbs(devinfo, cmdinfo); - spin_lock_irqsave(&devinfo->lock, flags); + uas_unlink_data_urbs(devinfo, cmdinfo, &flags); } cmdinfo->state &= ~COMMAND_INFLIGHT; uas_try_complete(cmnd, __func__); @@ -787,9 +783,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) spin_unlock_irqrestore(&devinfo->lock, flags); ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); } else { - spin_unlock_irqrestore(&devinfo->lock, flags); - uas_unlink_data_urbs(devinfo, cmdinfo); - spin_lock_irqsave(&devinfo->lock, flags); + uas_unlink_data_urbs(devinfo, cmdinfo, &flags); uas_try_complete(cmnd, __func__); spin_unlock_irqrestore(&devinfo->lock, flags); ret = SUCCESS; -- cgit v0.10.2 From a887cd366347c38607f0d9c28ca2baed40cac8fc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 19:47:28 +0200 Subject: uas: uas_alloc_cmd_urb: drop unused stream_id parameter The cmd endpoint never has streams, so the stream_id parameter is unused. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index dcaf6119..5eacb80 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -454,7 +454,7 @@ static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp, } static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp, - struct scsi_cmnd *cmnd, u16 stream_id) + struct scsi_cmnd *cmnd) { struct usb_device *udev = devinfo->udev; struct scsi_device *sdev = cmnd->device; @@ -626,8 +626,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & ALLOC_CMD_URB) { - cmdinfo->cmd_urb = uas_alloc_cmd_urb(devinfo, gfp, cmnd, - cmdinfo->stream); + cmdinfo->cmd_urb = uas_alloc_cmd_urb(devinfo, gfp, cmnd); if (!cmdinfo->cmd_urb) return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~ALLOC_CMD_URB; -- cgit v0.10.2 From 6c2334e9019039d7952190e239e6a8f0d10101fe Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 22:20:54 +0200 Subject: uas: Fix uas not working when plugged into an ehci port I thought it would be a good idea to also test uas with usb-2, and it turns out it was, as it did not work. The problem is that the uas driver was passing the bEndpointAddress' direction bit to usb_rcvbulkpipe, the xhci code seems to not care about this, but with the ehci code this causes usb_submit_urb failure. With this fixed the uas code works nicely with an uas device plugged into an ehci port. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 5eacb80..6ad5de9 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -948,13 +948,13 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) eps[3] = usb_pipe_endpoint(udev, devinfo->data_out_pipe); } else { devinfo->cmd_pipe = usb_sndbulkpipe(udev, - eps[0]->desc.bEndpointAddress); + usb_endpoint_num(&eps[0]->desc)); devinfo->status_pipe = usb_rcvbulkpipe(udev, - eps[1]->desc.bEndpointAddress); + usb_endpoint_num(&eps[1]->desc)); devinfo->data_in_pipe = usb_rcvbulkpipe(udev, - eps[2]->desc.bEndpointAddress); + usb_endpoint_num(&eps[2]->desc)); devinfo->data_out_pipe = usb_sndbulkpipe(udev, - eps[3]->desc.bEndpointAddress); + usb_endpoint_num(&eps[3]->desc)); } devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, 3, 256, -- cgit v0.10.2 From be326f4c9bdfdff8a85145fb89b0a44c4d20ebc6 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 22 Sep 2013 16:27:02 +0200 Subject: uas: Fix reset locking Fix the uas_eh_bus_reset_handler not properly taking the usbdev lock before calling usb_device_reset, the usb-core expects this lock to be taken when usb_device_reset is called. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6ad5de9..36ef82a 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -804,6 +804,13 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) struct usb_device *udev = devinfo->udev; int err; + err = usb_lock_device_for_reset(udev, devinfo->intf); + if (err) { + shost_printk(KERN_ERR, sdev->host, + "%s FAILED to get lock err %d\n", __func__, err); + return FAILED; + } + shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); devinfo->resetting = 1; uas_abort_work(devinfo); @@ -817,6 +824,8 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) uas_configure_endpoints(devinfo); devinfo->resetting = 0; + usb_unlock_device(udev); + if (err) { shost_printk(KERN_INFO, sdev->host, "%s FAILED\n", __func__); return FAILED; -- cgit v0.10.2 From 4de7a3735bdc4219cf57a0d44f92c06d7127a211 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 22 Oct 2013 16:10:44 +0100 Subject: uas: Fix reset handling for externally triggered reset Handle usb-device resets not triggered from uas_eh_bus_reset_handler(), when this happens, disable cmd queuing during the reset, and wait for existing requests to finish in pre_reset. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 36ef82a..0ee5a05 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -818,10 +819,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); uas_zap_dead(devinfo); - uas_free_streams(devinfo); err = usb_reset_device(udev); - if (!err) - uas_configure_endpoints(devinfo); devinfo->resetting = 0; usb_unlock_device(udev); @@ -1055,13 +1053,41 @@ set_alt0: static int uas_pre_reset(struct usb_interface *intf) { -/* XXX: Need to return 1 if it's not our device in error handling */ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + unsigned long flags; + + /* Block new requests */ + spin_lock_irqsave(shost->host_lock, flags); + scsi_block_requests(shost); + spin_unlock_irqrestore(shost->host_lock, flags); + + /* Wait for any pending requests to complete */ + flush_work(&devinfo->work); + if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) == 0) { + shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); + return 1; + } + + uas_free_streams(devinfo); + return 0; } static int uas_post_reset(struct usb_interface *intf) { -/* XXX: Need to return 1 if it's not our device in error handling */ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + unsigned long flags; + + uas_configure_endpoints(devinfo); + + spin_lock_irqsave(shost->host_lock, flags); + scsi_report_bus_reset(shost, 0); + spin_unlock_irqrestore(shost->host_lock, flags); + + scsi_unblock_requests(shost); + return 0; } -- cgit v0.10.2 From e52e031498cb51aff4f80a19a56700a127cf2a9a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 23 Oct 2013 14:27:09 +0100 Subject: uas: s/response_ui/response_iu/ Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 0ee5a05..33f9dcd 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -46,7 +46,7 @@ struct uas_dev_info { struct usb_anchor sense_urbs; struct usb_anchor data_urbs; int qdepth, resetting; - struct response_ui response; + struct response_iu response; unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe; unsigned use_streams:1; unsigned uas_sense_old:1; diff --git a/include/linux/usb/uas.h b/include/linux/usb/uas.h index 5499ab5..1404178 100644 --- a/include/linux/usb/uas.h +++ b/include/linux/usb/uas.h @@ -79,7 +79,7 @@ struct sense_iu { __u8 sense[SCSI_SENSE_BUFFERSIZE]; }; -struct response_ui { +struct response_iu { __u8 iu_id; __u8 rsvd1; __be16 tag; -- cgit v0.10.2 From 00d202cc12127fe9a9fa477a78cb37e32d7f4360 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 31 Oct 2013 09:59:12 +0100 Subject: uas: Fix response iu struct definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The response iu struct before this patch has a size of 7 bytes (discounting padding), which is weird since all other iu-s are explictly padded to a multiple of 4 bytes. More over submitting a 7 byte bulk transfer to the status endpoint when expecting a response iu results in an USB babble error, as the device actually sends 8 bytes. Up on closer reading of the UAS spec: http://www.t10.org/cgi-bin/ac.pl?t=f&f=uas2r00.pdf The reason for this becomes clear, the 2 entries in "Table 17 — RESPONSE IU" are numbered 4 and 6, looking at other iu definitions in the spec, esp. multi-byte fields, this indicates that the ADDITIONAL RESPONSE INFORMATION field is not a 2 byte field as one might assume at a first look, but is a multi-byte field containing 3 bytes. This also aligns with the SCSI Architecture Model 4 spec, which UAS is based on which states in paragraph "7.1 Task management function procedure calls" that the "Additional Response Information" output argument for a Task management function procedure call is 3 bytes. Last but not least I've verified this by sending a logical unit reset task management call with an invalid lun to an actual uasp device, and received back a response-iu with byte 6 being 0, and byte 7 being 9, which is the responce code for an invalid iu, which confirms that the response code is being reported in byte 7 of the response iu rather then in byte 6. Things were working before despite this error in the response iu struct definition because the additional response info field is normally filled with zeros, and 0 is the response code value for success. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/include/linux/usb/uas.h b/include/linux/usb/uas.h index 1404178..772b66b 100644 --- a/include/linux/usb/uas.h +++ b/include/linux/usb/uas.h @@ -83,7 +83,7 @@ struct response_iu { __u8 iu_id; __u8 rsvd1; __be16 tag; - __be16 add_response_info; + __u8 add_response_info[3]; __u8 response_code; }; -- cgit v0.10.2 From d24354bbf78f9194ec21087130a69b84864e50df Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 21 Oct 2013 11:15:11 +0100 Subject: uas: Pack iu struct definitions The iu struct definitions are usb packet definitions, so no alignment should happen. Notice that assuming 32 bit alignment this does not make any difference at all. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/include/linux/usb/uas.h b/include/linux/usb/uas.h index 772b66b..3fc8e8b 100644 --- a/include/linux/usb/uas.h +++ b/include/linux/usb/uas.h @@ -9,7 +9,7 @@ struct iu { __u8 iu_id; __u8 rsvd1; __be16 tag; -}; +} __attribute__((__packed__)); enum { IU_ID_COMMAND = 0x01, @@ -52,7 +52,7 @@ struct command_iu { __u8 rsvd7; struct scsi_lun lun; __u8 cdb[16]; /* XXX: Overflow-checking tools may misunderstand */ -}; +} __attribute__((__packed__)); struct task_mgmt_iu { __u8 iu_id; @@ -62,7 +62,7 @@ struct task_mgmt_iu { __u8 rsvd2; __be16 task_tag; struct scsi_lun lun; -}; +} __attribute__((__packed__)); /* * Also used for the Read Ready and Write Ready IUs since they have the @@ -77,7 +77,7 @@ struct sense_iu { __u8 rsvd7[7]; __be16 len; __u8 sense[SCSI_SENSE_BUFFERSIZE]; -}; +} __attribute__((__packed__)); struct response_iu { __u8 iu_id; @@ -85,7 +85,7 @@ struct response_iu { __be16 tag; __u8 add_response_info[3]; __u8 response_code; -}; +} __attribute__((__packed__)); struct usb_pipe_usage_descriptor { __u8 bLength; -- cgit v0.10.2 From d3f7c1560aee57d0ec293253e0c0e79a84ea3016 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 23 Oct 2013 17:46:17 +0100 Subject: uas: Use all available stream ids If we get ie 16 streams we can use stream-id 1-16, not 1-15. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 33f9dcd..3f021f2 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -722,7 +722,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, { struct Scsi_Host *shost = cmnd->device->host; struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; - u16 tag = devinfo->qdepth - 1; + u16 tag = devinfo->qdepth; unsigned long flags; spin_lock_irqsave(&devinfo->lock, flags); @@ -843,7 +843,7 @@ static int uas_slave_configure(struct scsi_device *sdev) { struct uas_dev_info *devinfo = sdev->hostdata; scsi_set_tag_type(sdev, MSG_ORDERED_TAG); - scsi_activate_tcq(sdev, devinfo->qdepth - 3); + scsi_activate_tcq(sdev, devinfo->qdepth - 2); return 0; } @@ -1027,7 +1027,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) INIT_LIST_HEAD(&devinfo->dead_list); uas_configure_endpoints(devinfo); - result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); + result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 2); if (result) goto free_streams; -- cgit v0.10.2 From e1be067b681054e963dfd01346c0d7fc0f8a63aa Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 21 Oct 2013 08:00:58 +0100 Subject: uas: Add a uas_find_uas_alt_setting helper function This is a preparation patch for teaching usb-storage to not bind to uas devices. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3f021f2..54db365 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -892,10 +892,10 @@ static int uas_isnt_supported(struct usb_device *udev) return -ENODEV; } -static int uas_switch_interface(struct usb_device *udev, - struct usb_interface *intf) +static int uas_find_uas_alt_setting(struct usb_interface *intf) { int i; + struct usb_device *udev = interface_to_usbdev(intf); int sg_supported = udev->bus->sg_tablesize != 0; for (i = 0; i < intf->num_altsetting; i++) { @@ -904,15 +904,26 @@ static int uas_switch_interface(struct usb_device *udev, if (uas_is_interface(alt)) { if (!sg_supported) return uas_isnt_supported(udev); - return usb_set_interface(udev, - alt->desc.bInterfaceNumber, - alt->desc.bAlternateSetting); + return alt->desc.bAlternateSetting; } } return -ENODEV; } +static int uas_switch_interface(struct usb_device *udev, + struct usb_interface *intf) +{ + int alt; + + alt = uas_find_uas_alt_setting(intf); + if (alt < 0) + return alt; + + return usb_set_interface(udev, + intf->altsetting[0].desc.bInterfaceNumber, alt); +} + static void uas_configure_endpoints(struct uas_dev_info *devinfo) { struct usb_host_endpoint *eps[4] = { }; -- cgit v0.10.2 From 82aa0387d52dd0db2173b844ad52d114807c189b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 21 Oct 2013 08:53:31 +0100 Subject: uas: Move uas detect code to uas-detect.h This is a preparation patch for teaching usb-storage to not bind to uas devices. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h new file mode 100644 index 0000000..28101c7 --- /dev/null +++ b/drivers/usb/storage/uas-detect.h @@ -0,0 +1,40 @@ +#include +#include + +static int uas_is_interface(struct usb_host_interface *intf) +{ + return (intf->desc.bInterfaceClass == USB_CLASS_MASS_STORAGE && + intf->desc.bInterfaceSubClass == USB_SC_SCSI && + intf->desc.bInterfaceProtocol == USB_PR_UAS); +} + +static int uas_isnt_supported(struct usb_device *udev) +{ + struct usb_hcd *hcd = bus_to_hcd(udev->bus); + + dev_warn(&udev->dev, "The driver for the USB controller %s does not " + "support scatter-gather which is\n", + hcd->driver->description); + dev_warn(&udev->dev, "required by the UAS driver. Please try an" + "alternative USB controller if you wish to use UAS.\n"); + return -ENODEV; +} + +static int uas_find_uas_alt_setting(struct usb_interface *intf) +{ + int i; + struct usb_device *udev = interface_to_usbdev(intf); + int sg_supported = udev->bus->sg_tablesize != 0; + + for (i = 0; i < intf->num_altsetting; i++) { + struct usb_host_interface *alt = &intf->altsetting[i]; + + if (uas_is_interface(alt)) { + if (!sg_supported) + return uas_isnt_supported(udev); + return alt->desc.bAlternateSetting; + } + } + + return -ENODEV; +} diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 54db365..6ea892f 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -25,6 +25,8 @@ #include #include +#include "uas-detect.h" + /* * The r00-r01c specs define this version of the SENSE IU data structure. * It's still in use by several different firmware releases. @@ -873,44 +875,6 @@ static struct usb_device_id uas_usb_ids[] = { }; MODULE_DEVICE_TABLE(usb, uas_usb_ids); -static int uas_is_interface(struct usb_host_interface *intf) -{ - return (intf->desc.bInterfaceClass == USB_CLASS_MASS_STORAGE && - intf->desc.bInterfaceSubClass == USB_SC_SCSI && - intf->desc.bInterfaceProtocol == USB_PR_UAS); -} - -static int uas_isnt_supported(struct usb_device *udev) -{ - struct usb_hcd *hcd = bus_to_hcd(udev->bus); - - dev_warn(&udev->dev, "The driver for the USB controller %s does not " - "support scatter-gather which is\n", - hcd->driver->description); - dev_warn(&udev->dev, "required by the UAS driver. Please try an" - "alternative USB controller if you wish to use UAS.\n"); - return -ENODEV; -} - -static int uas_find_uas_alt_setting(struct usb_interface *intf) -{ - int i; - struct usb_device *udev = interface_to_usbdev(intf); - int sg_supported = udev->bus->sg_tablesize != 0; - - for (i = 0; i < intf->num_altsetting; i++) { - struct usb_host_interface *alt = &intf->altsetting[i]; - - if (uas_is_interface(alt)) { - if (!sg_supported) - return uas_isnt_supported(udev); - return alt->desc.bAlternateSetting; - } - } - - return -ENODEV; -} - static int uas_switch_interface(struct usb_device *udev, struct usb_interface *intf) { -- cgit v0.10.2 From 127329d76b8534fb58c207db1f172d8468b828ff Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 7 Nov 2013 08:19:45 +0100 Subject: xhci: xhci_mem_cleanup: make sure cmd_ring_reserved_trbs really is 0 cmd_ring_reserved_trbs gets decremented by xhci_free_stream_info(), so set it to 0 after freeing all rings, otherwise it wraps around to a very large value when rings with streams are free-ed. Before this patch the wrap-around could be triggered when xhci_resume calls xhci_mem_cleanup if the controller resume fails. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index edfb31a..60fc672 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1812,7 +1812,6 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) if (xhci->lpm_command) xhci_free_command(xhci, xhci->lpm_command); - xhci->cmd_ring_reserved_trbs = 0; if (xhci->cmd_ring) xhci_ring_free(xhci, xhci->cmd_ring); xhci->cmd_ring = NULL; @@ -1877,6 +1876,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci) } no_bw: + xhci->cmd_ring_reserved_trbs = 0; xhci->num_usb2_ports = 0; xhci->num_usb3_ports = 0; xhci->num_active_eps = 0; -- cgit v0.10.2 From 84c1e40fd794a883431fc7688f653d3faa0265f0 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 5 Nov 2013 15:50:03 +0100 Subject: xhci: The trb_address_map radix tree expects 1KB segment memory aligment If we align segment dma pool memory to 64 bytes, then a segment can be located at 0x10000040 - 0x1000043f, and a segment from another ring at 0x10000440 - 0x1000083f. The last trb in the first segment at 0x10000430 will then translate to the same radix tree key as the first trb of the second segment, while they are in different rings! This patches fixes this by changing the alignment of the dma pool to be 1KB rather then 64 bytes. An alternative fix would be to reduce the shift used to calculate the radix tree keys, but that would (slighlty) grow the radix trees so I believe this is the better fix. Note this patch is mostly theoretical since in practice I've not seen the dma_pool actually return not 1KB aligned memory. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 60fc672..c089668 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -158,7 +158,7 @@ static void xhci_link_rings(struct xhci_hcd *xhci, struct xhci_ring *ring, * * The radix tree maps the upper portion of the TRB DMA address to a ring * segment that has the same upper portion of DMA addresses. For example, say I - * have segments of size 1KB, that are always 64-byte aligned. A segment may + * have segments of size 1KB, that are always 1KB aligned. A segment may * start at 0x10c91000 and end at 0x10c913f0. If I use the upper 10 bits, the * key to the stream ID is 0x43244. I can use the DMA address of the TRB to * pass the radix tree a key to get the right stream ID: @@ -2375,11 +2375,12 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) /* * Initialize the ring segment pool. The ring must be a contiguous * structure comprised of TRBs. The TRBs must be 16 byte aligned, - * however, the command ring segment needs 64-byte aligned segments, - * so we pick the greater alignment need. + * however, the command ring segment needs 64-byte aligned segments + * and our use of dma addresses in the trb_address_map radix tree needs + * TRB_SEGMENT_SIZE alignment, so we pick the greater alignment need. */ xhci->segment_pool = dma_pool_create("xHCI ring segments", dev, - TRB_SEGMENT_SIZE, 64, xhci->page_size); + TRB_SEGMENT_SIZE, TRB_SEGMENT_SIZE, xhci->page_size); /* See Table 46 and Note on Figure 55 */ xhci->device_pool = dma_pool_create("xHCI input/output contexts", dev, -- cgit v0.10.2 From f7920884eb640bc642f3b4e56f5237d30a080eda Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 15 Nov 2013 12:14:38 +0100 Subject: xhci: Handle MaxPSASize == 0 Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6cb9c22..1db3c27 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3138,6 +3138,12 @@ int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev, xhci_dbg(xhci, "Driver wants %u stream IDs (including stream 0).\n", num_streams); + /* MaxPSASize value 0 (2 streams) means streams are not supported */ + if (HCC_MAX_PSA(xhci->hcc_params) < 4) { + xhci_dbg(xhci, "xHCI controller does not support streams.\n"); + return -ENOSYS; + } + config_cmd = xhci_alloc_command(xhci, true, true, mem_flags); if (!config_cmd) { xhci_dbg(xhci, "Could not allocate xHCI command structure.\n"); -- cgit v0.10.2 From 7a7b562d08ad6db98d6c8ec634620a11aaf8921a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 8 Nov 2013 16:37:26 +0100 Subject: usb: Clear host_endpoint->streams when implicitly freeing streams If streams are still allocated on device-reset or set-interface then the hcd code implictly frees the streams. Clear host_endpoint->streams in this case so that if a driver later tries to re-allocate them it won't run afoul of the device already having streams check in usb_alloc_streams(). Note normally streams still being allocated at reset / set-intf would be a driver bug, but this can happen without it being a driver bug on reset-resume. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 763c313..4f7629d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -5131,7 +5131,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev) struct usb_hcd *hcd = bus_to_hcd(udev->bus); struct usb_device_descriptor descriptor = udev->descriptor; struct usb_host_bos *bos; - int i, ret = 0; + int i, j, ret = 0; int port1 = udev->portnum; if (udev->state == USB_STATE_NOTATTACHED || @@ -5257,6 +5257,9 @@ static int usb_reset_and_verify_device(struct usb_device *udev) ret); goto re_enumerate; } + /* Resetting also frees any allocated streams */ + for (j = 0; j < intf->cur_altsetting->desc.bNumEndpoints; j++) + intf->cur_altsetting->endpoint[j].streams = 0; } done: diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index f829a1a..9646957 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1293,8 +1293,7 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) struct usb_interface *iface; struct usb_host_interface *alt; struct usb_hcd *hcd = bus_to_hcd(dev->bus); - int ret; - int manual = 0; + int i, ret, manual = 0; unsigned int epaddr; unsigned int pipe; @@ -1329,6 +1328,10 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) mutex_unlock(hcd->bandwidth_mutex); return -ENOMEM; } + /* Changing alt-setting also frees any allocated streams */ + for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++) + iface->cur_altsetting->endpoint[i].streams = 0; + ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt); if (ret < 0) { dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n", -- cgit v0.10.2 From a82b76f7fa6154e8ab2d8071842a3e38b9c0d0ff Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 8 Nov 2013 13:41:05 +0100 Subject: usb: Reset USB-3 devices on USB-3 link bounce On disconnect USB3 protocol ports transit from U0 to SS.Inactive to Rx.Detect, on a recoverable error, the port stays in SS.Inactive and we recover from it by doing a warm-reset (through usb_device_reset if we have a udev for the port). If this really is a disconnect we may end up trying the warm-reset anyways, since khubd may run before the SS.Inactive to Rx.Detect transition, or it may get skipped if the transition to Rx.Detect happens before khubd gets run. With a loose connector, or in the case which actually led me to debugging this bad ACPI firmware toggling Vbus off and on in quick succession, the port may transition from Rx.Detect to U0 again before khubd gets run. In this case the device state is unknown really, but khubd happily goes into the resuscitate an existing device path, and the device driver never gets notified about the device state being messed up. If the above scenario happens with a streams using device, as soon as an urb is submitted to an endpoint with streams, the following appears in dmesg: ERROR Transfer event for disabled endpoint or incorrect stream ring @0000000036807420 00000000 00000000 04000000 04078000 Notice how the TRB address is all zeros. I've seen this both on Intel Pantherpoint and Nec xhci hosts. Luckily we can detect the U0 to SS.Inactive to Rx.Detect to U0 all having happened before khubd runs case since the C_LINK_STATE bit gets set in the portchange bits on the U0 -> SS.Inactive change. This bit will also be set on suspend / resume, but then it gets cleared by port_hub_init before khubd runs. So if the C_LINK_STATE bit is set and a warm-reset is not needed, iow the port is not still in SS.Inactive, and the port still has a connection, then the device needs to be reset to put it back in a known state. I've verified that doing the device reset also fixes the transfer event with all zeros address issue. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 4f7629d..5da5394 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4758,6 +4758,8 @@ static void hub_events(void) /* deal with port status changes */ for (i = 1; i <= hdev->maxchild; i++) { + struct usb_device *udev = hub->ports[i - 1]->child; + if (test_bit(i, hub->busy_bits)) continue; connect_change = test_bit(i, hub->change_bits); @@ -4856,8 +4858,6 @@ static void hub_events(void) */ if (hub_port_warm_reset_required(hub, portstatus)) { int status; - struct usb_device *udev = - hub->ports[i - 1]->child; dev_dbg(hub_dev, "warm reset port %d\n", i); if (!udev || @@ -4874,6 +4874,24 @@ static void hub_events(void) usb_unlock_device(udev); connect_change = 0; } + /* + * On disconnect USB3 protocol ports transit from U0 to + * SS.Inactive to Rx.Detect. If this happens a warm- + * reset is not needed, but a (re)connect may happen + * before khubd runs and sees the disconnect, and the + * device may be an unknown state. + * + * If the port went through SS.Inactive without khubd + * seeing it the C_LINK_STATE change flag will be set, + * and we reset the dev to put it in a known state. + */ + } else if (udev && hub_is_superspeed(hub->hdev) && + (portchange & USB_PORT_STAT_C_LINK_STATE) && + (portstatus & USB_PORT_STAT_CONNECTION)) { + usb_lock_device(udev); + usb_reset_device(udev); + usb_unlock_device(udev); + connect_change = 0; } if (connect_change) -- cgit v0.10.2 From 79b4c06112f12c31d03cf22b1ed5ce09423fd887 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 25 Oct 2013 17:04:33 +0100 Subject: uas: Add the posibilty to blacklist uas devices from using the uas driver Once we start supporting uas hardware, and as more and more uas devices become available, we will likely start seeing broken devices. This patch prepares for the inevitable need for blacklisting those devices from using the uas driver (they will use usb-storage instead). Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h index 28101c7..02bf5ec 100644 --- a/drivers/usb/storage/uas-detect.h +++ b/drivers/usb/storage/uas-detect.h @@ -38,3 +38,14 @@ static int uas_find_uas_alt_setting(struct usb_interface *intf) return -ENODEV; } + +static int uas_use_uas_driver(struct usb_interface *intf, + const struct usb_device_id *id) +{ + unsigned long flags = id->driver_info; + + if (flags & US_FL_IGNORE_UAS) + return 0; + + return uas_find_uas_alt_setting(intf) >= 0; +} diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6ea892f..e817e72 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -866,7 +867,14 @@ static struct scsi_host_template uas_host_template = { .ordered_tag = 1, }; +#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \ + vendorName, productName, useProtocol, useTransport, \ + initFunction, flags) \ +{ USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \ + .driver_info = (flags) } + static struct usb_device_id uas_usb_ids[] = { +# include "unusual_uas.h" { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_BULK) }, { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_UAS) }, /* 0xaa is a prototype device I happen to have access to */ @@ -875,6 +883,8 @@ static struct usb_device_id uas_usb_ids[] = { }; MODULE_DEVICE_TABLE(usb, uas_usb_ids); +#undef UNUSUAL_DEV + static int uas_switch_interface(struct usb_device *udev, struct usb_interface *intf) { @@ -973,6 +983,9 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) struct uas_dev_info *devinfo; struct usb_device *udev = interface_to_usbdev(intf); + if (!uas_use_uas_driver(intf, id)) + return -ENODEV; + if (uas_switch_interface(udev, intf)) return -ENODEV; @@ -1083,10 +1096,6 @@ static void uas_disconnect(struct usb_interface *intf) kfree(devinfo); } -/* - * XXX: Should this plug into libusual so we can auto-upgrade devices from - * Bulk-Only to UAS? - */ static struct usb_driver uas_driver = { .name = "uas", .probe = uas_probe, diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h new file mode 100644 index 0000000..7244444 --- /dev/null +++ b/drivers/usb/storage/unusual_uas.h @@ -0,0 +1,52 @@ +/* Driver for USB Attached SCSI devices - Unusual Devices File + * + * (c) 2013 Hans de Goede + * + * Based on the same file for the usb-storage driver, which is: + * (c) 2000-2002 Matthew Dharm (mdharm-usb@one-eyed-alien.net) + * (c) 2000 Adam J. Richter (adam@yggdrasil.com), Yggdrasil Computing, Inc. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2, or (at your option) any + * later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +/* + * IMPORTANT NOTE: This file must be included in another file which defines + * a UNUSUAL_DEV macro before this file is included. + */ + +/* + * If you edit this file, please try to keep it sorted first by VendorID, + * then by ProductID. + * + * If you want to add an entry for this file, be sure to include the + * following information: + * - a patch that adds the entry for your device, including your + * email address right above the entry (plus maybe a brief + * explanation of the reason for the entry), + * - lsusb -v output for the device + * Send your submission to Hans de Goede + * and don't forget to CC: the USB development list + */ + +/* + * This is an example entry for the US_FL_IGNORE_UAS flag. Once we have an + * actual entry using US_FL_IGNORE_UAS this entry should be removed. + * + * UNUSUAL_DEV( 0xabcd, 0x1234, 0x0100, 0x0100, + * "Example", + * "Storage with broken UAS", + * USB_SC_DEVICE, USB_PR_DEVICE, NULL, + * US_FL_IGNORE_UAS), + */ diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h index 6303568..1a64b26 100644 --- a/include/linux/usb_usual.h +++ b/include/linux/usb_usual.h @@ -67,8 +67,10 @@ /* Initial READ(10) (and others) must be retried */ \ US_FLAG(WRITE_CACHE, 0x00200000) \ /* Write Cache status is not available */ \ - US_FLAG(NEEDS_CAP16, 0x00400000) - /* cannot handle READ_CAPACITY_10 */ + US_FLAG(NEEDS_CAP16, 0x00400000) \ + /* cannot handle READ_CAPACITY_10 */ \ + US_FLAG(IGNORE_UAS, 0x00800000) \ + /* Device advertises UAS but it is broken */ #define US_FLAG(name, value) US_FL_##name = value , enum { US_DO_ALL_FLAGS }; -- cgit v0.10.2 From 5bfd5b5d8b9cd9e8ba1709f2b9dc35bd4b26c8b1 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 21 Oct 2013 09:40:48 +0100 Subject: usb-storage: Don't bind to uas devices if the uas driver is enabled uas devices have 2 alternative settings on their usb-storage interface, one for usb-storage and one for uas. Using the uas driver is preferred, so if the uas driver is enabled, and the device has an uas alt setting, don't bind. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h index adbeb25..f4a82291 100644 --- a/drivers/usb/storage/unusual_devs.h +++ b/drivers/usb/storage/unusual_devs.h @@ -2086,6 +2086,11 @@ UNUSUAL_DEV( 0xed10, 0x7636, 0x0001, 0x0001, "Digital MP3 Audio Player", USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NOT_LOCKABLE ), +/* Unusual uas devices */ +#if IS_ENABLED(CONFIG_USB_UAS) +#include "unusual_uas.h" +#endif + /* Control/Bulk transport for all SubClass values */ USUAL_DEV(USB_SC_RBC, USB_PR_CB), USUAL_DEV(USB_SC_8020, USB_PR_CB), diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index 1c0b89f..388f567 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -72,6 +72,10 @@ #include "sierra_ms.h" #include "option_ms.h" +#if IS_ENABLED(CONFIG_USB_UAS) +#include "uas-detect.h" +#endif + /* Some informational data */ MODULE_AUTHOR("Matthew Dharm "); MODULE_DESCRIPTION("USB Mass Storage driver for Linux"); @@ -1035,6 +1039,12 @@ static int storage_probe(struct usb_interface *intf, int result; int size; + /* If uas is enabled and this device can do uas then ignore it. */ +#if IS_ENABLED(CONFIG_USB_UAS) + if (uas_use_uas_driver(intf, id)) + return -ENXIO; +#endif + /* * If the device isn't standard (is handled by a subdriver * module) then don't accept it. -- cgit v0.10.2 From d24d481b7d369b08cce734bf80be374eed5a6e58 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 16 Nov 2013 12:09:39 +0100 Subject: usb-storage: Modify and export adjust_quirks so that it can be used by uas Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c index 388f567..f1c9626 100644 --- a/drivers/usb/storage/usb.c +++ b/drivers/usb/storage/usb.c @@ -463,14 +463,14 @@ static int associate_dev(struct us_data *us, struct usb_interface *intf) #define TOLOWER(x) ((x) | 0x20) /* Adjust device flags based on the "quirks=" module parameter */ -static void adjust_quirks(struct us_data *us) +void usb_stor_adjust_quirks(struct usb_device *udev, unsigned long *fflags) { char *p; - u16 vid = le16_to_cpu(us->pusb_dev->descriptor.idVendor); - u16 pid = le16_to_cpu(us->pusb_dev->descriptor.idProduct); + u16 vid = le16_to_cpu(udev->descriptor.idVendor); + u16 pid = le16_to_cpu(udev->descriptor.idProduct); unsigned f = 0; unsigned int mask = (US_FL_SANE_SENSE | US_FL_BAD_SENSE | - US_FL_FIX_CAPACITY | + US_FL_FIX_CAPACITY | US_FL_IGNORE_UAS | US_FL_CAPACITY_HEURISTICS | US_FL_IGNORE_DEVICE | US_FL_NOT_LOCKABLE | US_FL_MAX_SECTORS_64 | US_FL_CAPACITY_OK | US_FL_IGNORE_RESIDUE | @@ -541,14 +541,18 @@ static void adjust_quirks(struct us_data *us) case 's': f |= US_FL_SINGLE_LUN; break; + case 'u': + f |= US_FL_IGNORE_UAS; + break; case 'w': f |= US_FL_NO_WP_DETECT; break; /* Ignore unrecognized flag characters */ } } - us->fflags = (us->fflags & ~mask) | f; + *fflags = (*fflags & ~mask) | f; } +EXPORT_SYMBOL_GPL(usb_stor_adjust_quirks); /* Get the unusual_devs entries and the string descriptors */ static int get_device_info(struct us_data *us, const struct usb_device_id *id, @@ -568,7 +572,7 @@ static int get_device_info(struct us_data *us, const struct usb_device_id *id, idesc->bInterfaceProtocol : unusual_dev->useTransport; us->fflags = id->driver_info; - adjust_quirks(us); + usb_stor_adjust_quirks(us->pusb_dev, &us->fflags); if (us->fflags & US_FL_IGNORE_DEVICE) { dev_info(pdev, "device ignored\n"); diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h index 75f70f0..307e339 100644 --- a/drivers/usb/storage/usb.h +++ b/drivers/usb/storage/usb.h @@ -201,4 +201,7 @@ extern int usb_stor_probe1(struct us_data **pus, extern int usb_stor_probe2(struct us_data *us); extern void usb_stor_disconnect(struct usb_interface *intf); +extern void usb_stor_adjust_quirks(struct usb_device *dev, + unsigned long *fflags); + #endif -- cgit v0.10.2 From 97172a660cfc744996112eb625a77282a4b627b7 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 16 Nov 2013 12:19:36 +0100 Subject: uas: Honor no-uas quirk set in usb-storage's quirks module parameter Falling back from uas to usb-storage requires coordination between uas and usb-storage, so use usb-storage's quirks module parameter, rather then requiring the user to pass a param to 2 different modules. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/Kconfig b/drivers/usb/storage/Kconfig index 1dd0604..666dcb6 100644 --- a/drivers/usb/storage/Kconfig +++ b/drivers/usb/storage/Kconfig @@ -204,7 +204,7 @@ config USB_STORAGE_ENE_UB6250 config USB_UAS tristate "USB Attached SCSI" - depends on SCSI && BROKEN + depends on SCSI && USB_STORAGE && BROKEN help The USB Attached SCSI protocol is supported by some USB storage devices. It permits higher performance by supporting diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h index 02bf5ec..082bde1 100644 --- a/drivers/usb/storage/uas-detect.h +++ b/drivers/usb/storage/uas-detect.h @@ -1,5 +1,6 @@ #include #include +#include "usb.h" static int uas_is_interface(struct usb_host_interface *intf) { @@ -42,8 +43,11 @@ static int uas_find_uas_alt_setting(struct usb_interface *intf) static int uas_use_uas_driver(struct usb_interface *intf, const struct usb_device_id *id) { + struct usb_device *udev = interface_to_usbdev(intf); unsigned long flags = id->driver_info; + usb_stor_adjust_quirks(udev, &flags); + if (flags & US_FL_IGNORE_UAS) return 0; -- cgit v0.10.2 From 34f11e59c3d5d025ad9162d8fd126e0b7ca54f40 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 08:54:48 +0100 Subject: uas: Add uas_find_endpoints() helper function This is a preparation patch for adding better descriptor validation. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index e817e72..1ac66f2 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -898,16 +898,11 @@ static int uas_switch_interface(struct usb_device *udev, intf->altsetting[0].desc.bInterfaceNumber, alt); } -static void uas_configure_endpoints(struct uas_dev_info *devinfo) +static int uas_find_endpoints(struct usb_host_interface *alt, + struct usb_host_endpoint *eps[]) { - struct usb_host_endpoint *eps[4] = { }; - struct usb_interface *intf = devinfo->intf; - struct usb_device *udev = devinfo->udev; - struct usb_host_endpoint *endpoint = intf->cur_altsetting->endpoint; - unsigned i, n_endpoints = intf->cur_altsetting->desc.bNumEndpoints; - - devinfo->uas_sense_old = 0; - devinfo->cmnd = NULL; + struct usb_host_endpoint *endpoint = alt->endpoint; + unsigned i, n_endpoints = alt->desc.bNumEndpoints; for (i = 0; i < n_endpoints; i++) { unsigned char *extra = endpoint[i].extra; @@ -924,12 +919,29 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) } } + if (!eps[0] || !eps[1] || !eps[2] || !eps[3]) + return -ENODEV; + + return 0; +} + +static void uas_configure_endpoints(struct uas_dev_info *devinfo) +{ + struct usb_host_endpoint *eps[4] = { }; + struct usb_device *udev = devinfo->udev; + int r; + + devinfo->uas_sense_old = 0; + devinfo->cmnd = NULL; + + r = uas_find_endpoints(devinfo->intf->cur_altsetting, eps); + /* - * Assume that if we didn't find a control pipe descriptor, we're + * Assume that if we didn't find a proper set of descriptors, we're * using a device with old firmware that happens to be set up like * this. */ - if (!eps[0]) { + if (r != 0) { devinfo->cmd_pipe = usb_sndbulkpipe(udev, 1); devinfo->status_pipe = usb_rcvbulkpipe(udev, 1); devinfo->data_in_pipe = usb_rcvbulkpipe(udev, 2); -- cgit v0.10.2 From d495c1baa1b3ba277bb5ae24adeab0600151cba4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 09:06:54 +0100 Subject: uas: Fix bounds check in uas_find_endpoints The loop uses up to 3 bytes of the endpoint extra data. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 1ac66f2..7662b3e 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -907,7 +907,7 @@ static int uas_find_endpoints(struct usb_host_interface *alt, for (i = 0; i < n_endpoints; i++) { unsigned char *extra = endpoint[i].extra; int len = endpoint[i].extralen; - while (len > 1) { + while (len >= 3) { if (extra[1] == USB_DT_PIPE_USAGE) { unsigned pipe_id = extra[2]; if (pipe_id > 0 && pipe_id < 5) -- cgit v0.10.2 From d77adc0284beea5783c52b2af49217a37c2114cd Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:03:34 +0100 Subject: uas: Move uas_find_endpoints to uas-detect.h No changes, just the move. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h index 082bde1..8de030a 100644 --- a/drivers/usb/storage/uas-detect.h +++ b/drivers/usb/storage/uas-detect.h @@ -40,6 +40,33 @@ static int uas_find_uas_alt_setting(struct usb_interface *intf) return -ENODEV; } +static int uas_find_endpoints(struct usb_host_interface *alt, + struct usb_host_endpoint *eps[]) +{ + struct usb_host_endpoint *endpoint = alt->endpoint; + unsigned i, n_endpoints = alt->desc.bNumEndpoints; + + for (i = 0; i < n_endpoints; i++) { + unsigned char *extra = endpoint[i].extra; + int len = endpoint[i].extralen; + while (len >= 3) { + if (extra[1] == USB_DT_PIPE_USAGE) { + unsigned pipe_id = extra[2]; + if (pipe_id > 0 && pipe_id < 5) + eps[pipe_id - 1] = &endpoint[i]; + break; + } + len -= extra[0]; + extra += extra[0]; + } + } + + if (!eps[0] || !eps[1] || !eps[2] || !eps[3]) + return -ENODEV; + + return 0; +} + static int uas_use_uas_driver(struct usb_interface *intf, const struct usb_device_id *id) { diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7662b3e..5cedc7f 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -898,33 +898,6 @@ static int uas_switch_interface(struct usb_device *udev, intf->altsetting[0].desc.bInterfaceNumber, alt); } -static int uas_find_endpoints(struct usb_host_interface *alt, - struct usb_host_endpoint *eps[]) -{ - struct usb_host_endpoint *endpoint = alt->endpoint; - unsigned i, n_endpoints = alt->desc.bNumEndpoints; - - for (i = 0; i < n_endpoints; i++) { - unsigned char *extra = endpoint[i].extra; - int len = endpoint[i].extralen; - while (len >= 3) { - if (extra[1] == USB_DT_PIPE_USAGE) { - unsigned pipe_id = extra[2]; - if (pipe_id > 0 && pipe_id < 5) - eps[pipe_id - 1] = &endpoint[i]; - break; - } - len -= extra[0]; - extra += extra[0]; - } - } - - if (!eps[0] || !eps[1] || !eps[2] || !eps[3]) - return -ENODEV; - - return 0; -} - static void uas_configure_endpoints(struct uas_dev_info *devinfo) { struct usb_host_endpoint *eps[4] = { }; -- cgit v0.10.2 From 74d71aec619f33ec1ff5b2090792ab96d840bd3b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:10:36 +0100 Subject: uas: Drop fixed endpoint config handling The fixed endpoint config code was only necessary to deal with an early uas prototype which has never been released, so lets drop it and enforce proper uas endpoint descriptors. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 5cedc7f..754468b 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -908,31 +908,17 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) devinfo->cmnd = NULL; r = uas_find_endpoints(devinfo->intf->cur_altsetting, eps); - - /* - * Assume that if we didn't find a proper set of descriptors, we're - * using a device with old firmware that happens to be set up like - * this. - */ - if (r != 0) { - devinfo->cmd_pipe = usb_sndbulkpipe(udev, 1); - devinfo->status_pipe = usb_rcvbulkpipe(udev, 1); - devinfo->data_in_pipe = usb_rcvbulkpipe(udev, 2); - devinfo->data_out_pipe = usb_sndbulkpipe(udev, 2); - - eps[1] = usb_pipe_endpoint(udev, devinfo->status_pipe); - eps[2] = usb_pipe_endpoint(udev, devinfo->data_in_pipe); - eps[3] = usb_pipe_endpoint(udev, devinfo->data_out_pipe); - } else { - devinfo->cmd_pipe = usb_sndbulkpipe(udev, - usb_endpoint_num(&eps[0]->desc)); - devinfo->status_pipe = usb_rcvbulkpipe(udev, - usb_endpoint_num(&eps[1]->desc)); - devinfo->data_in_pipe = usb_rcvbulkpipe(udev, - usb_endpoint_num(&eps[2]->desc)); - devinfo->data_out_pipe = usb_sndbulkpipe(udev, - usb_endpoint_num(&eps[3]->desc)); - } + if (r) + return r; + + devinfo->cmd_pipe = usb_sndbulkpipe(udev, + usb_endpoint_num(&eps[0]->desc)); + devinfo->status_pipe = usb_rcvbulkpipe(udev, + usb_endpoint_num(&eps[1]->desc)); + devinfo->data_in_pipe = usb_rcvbulkpipe(udev, + usb_endpoint_num(&eps[2]->desc)); + devinfo->data_out_pipe = usb_sndbulkpipe(udev, + usb_endpoint_num(&eps[3]->desc)); devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, 3, 256, GFP_KERNEL); -- cgit v0.10.2 From 6134041bef0aeb9cb7c8a8daf045b44513cd8396 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 16 Nov 2013 11:37:41 +0100 Subject: uas: Verify endpoint descriptors from uas_use_uas_driver() Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h index 8de030a..b8a02e1 100644 --- a/drivers/usb/storage/uas-detect.h +++ b/drivers/usb/storage/uas-detect.h @@ -70,13 +70,23 @@ static int uas_find_endpoints(struct usb_host_interface *alt, static int uas_use_uas_driver(struct usb_interface *intf, const struct usb_device_id *id) { + struct usb_host_endpoint *eps[4] = { }; struct usb_device *udev = interface_to_usbdev(intf); unsigned long flags = id->driver_info; + int r, alt; usb_stor_adjust_quirks(udev, &flags); if (flags & US_FL_IGNORE_UAS) return 0; - return uas_find_uas_alt_setting(intf) >= 0; + alt = uas_find_uas_alt_setting(intf); + if (alt < 0) + return 0; + + r = uas_find_endpoints(&intf->altsetting[alt], eps); + if (r < 0) + return 0; + + return 1; } -- cgit v0.10.2 From 58d51444cdd066239e9b660d72319d941c758fc3 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:23:26 +0100 Subject: uas: Not being able to alloc streams when connected through usb-3 is an error Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 754468b..d758fae 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -93,7 +93,6 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo, gfp_t gfp); static void uas_do_work(struct work_struct *work); static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); -static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller); @@ -898,7 +897,7 @@ static int uas_switch_interface(struct usb_device *udev, intf->altsetting[0].desc.bInterfaceNumber, alt); } -static void uas_configure_endpoints(struct uas_dev_info *devinfo) +static int uas_configure_endpoints(struct uas_dev_info *devinfo) { struct usb_host_endpoint *eps[4] = { }; struct usb_device *udev = devinfo->udev; @@ -920,14 +919,18 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) devinfo->data_out_pipe = usb_sndbulkpipe(udev, usb_endpoint_num(&eps[3]->desc)); - devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, 3, 256, - GFP_KERNEL); - if (devinfo->qdepth < 0) { + if (udev->speed != USB_SPEED_SUPER) { devinfo->qdepth = 256; devinfo->use_streams = 0; } else { + devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, + 3, 256, GFP_KERNEL); + if (devinfo->qdepth < 0) + return devinfo->qdepth; devinfo->use_streams = 1; } + + return 0; } static void uas_free_streams(struct uas_dev_info *devinfo) @@ -984,7 +987,10 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) INIT_WORK(&devinfo->work, uas_do_work); INIT_LIST_HEAD(&devinfo->work_list); INIT_LIST_HEAD(&devinfo->dead_list); - uas_configure_endpoints(devinfo); + + result = uas_configure_endpoints(devinfo); + if (result) + goto set_alt0; result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 2); if (result) @@ -1039,7 +1045,11 @@ static int uas_post_reset(struct usb_interface *intf) struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; unsigned long flags; - uas_configure_endpoints(devinfo); + if (uas_configure_endpoints(devinfo) != 0) { + shost_printk(KERN_ERR, shost, + "%s: alloc streams error after reset", __func__); + return 1; + } spin_lock_irqsave(shost->host_lock, flags); scsi_report_bus_reset(shost, 0); -- cgit v0.10.2 From 70cf0fba7625987ef16085f458e3869c6e3043c1 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:37:23 +0100 Subject: uas: task_mgmt: Kill the sense-urb if we fail to submit the cmd urb Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index d758fae..9c6f9f9 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -550,39 +550,38 @@ err: * daft to me. */ -static int uas_submit_sense_urb(struct Scsi_Host *shost, - gfp_t gfp, unsigned int stream) +static struct urb *uas_submit_sense_urb(struct Scsi_Host *shost, + gfp_t gfp, unsigned int stream) { struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; struct urb *urb; urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); if (!urb) - return SCSI_MLQUEUE_DEVICE_BUSY; + return NULL; usb_anchor_urb(urb, &devinfo->sense_urbs); if (usb_submit_urb(urb, gfp)) { usb_unanchor_urb(urb); shost_printk(KERN_INFO, shost, "sense urb submission failure\n"); usb_free_urb(urb); - return SCSI_MLQUEUE_DEVICE_BUSY; + return NULL; } - return 0; + return urb; } static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo, gfp_t gfp) { struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; - int err; + struct urb *urb; WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & SUBMIT_STATUS_URB) { - err = uas_submit_sense_urb(cmnd->device->host, gfp, + urb = uas_submit_sense_urb(cmnd->device->host, gfp, cmdinfo->stream); - if (err) { - return err; - } + if (!urb) + return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~SUBMIT_STATUS_URB; } @@ -726,10 +725,12 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; u16 tag = devinfo->qdepth; unsigned long flags; + struct urb *sense_urb; spin_lock_irqsave(&devinfo->lock, flags); memset(&devinfo->response, 0, sizeof(devinfo->response)); - if (uas_submit_sense_urb(shost, GFP_ATOMIC, tag)) { + sense_urb = uas_submit_sense_urb(shost, GFP_ATOMIC, tag); + if (!sense_urb) { shost_printk(KERN_INFO, shost, "%s: %s: submit sense urb failed\n", __func__, fname); @@ -741,6 +742,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, "%s: %s: submit task mgmt urb failed\n", __func__, fname); spin_unlock_irqrestore(&devinfo->lock, flags); + usb_kill_urb(sense_urb); return FAILED; } spin_unlock_irqrestore(&devinfo->lock, flags); -- cgit v0.10.2 From b83b86a352280cc8cbbf3760096c703986143b81 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:51:00 +0100 Subject: uas: Don't allow more then one task to run at the same time Since we use a fixed tag / stream for tasks we cannot allow more then one to run at the same time. This could happen before this time if a task timed out. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 9c6f9f9..7fc4ad2 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -53,6 +53,7 @@ struct uas_dev_info { unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe; unsigned use_streams:1; unsigned uas_sense_old:1; + unsigned running_task:1; struct scsi_cmnd *cmnd; spinlock_t lock; struct work_struct work; @@ -195,6 +196,7 @@ static void uas_zap_dead(struct uas_dev_info *devinfo) DATA_OUT_URB_INFLIGHT); uas_try_complete(cmnd, __func__); } + devinfo->running_task = 0; spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -340,6 +342,9 @@ static void uas_stat_cmplt(struct urb *urb) if (!cmnd) { if (iu->iu_id == IU_ID_RESPONSE) { + if (!devinfo->running_task) + dev_warn(&urb->dev->dev, + "stat urb: recv unexpected response iu\n"); /* store results for uas_eh_task_mgmt() */ memcpy(&devinfo->response, iu, sizeof(devinfo->response)); } @@ -726,14 +731,26 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, u16 tag = devinfo->qdepth; unsigned long flags; struct urb *sense_urb; + int result = SUCCESS; spin_lock_irqsave(&devinfo->lock, flags); + + if (devinfo->running_task) { + shost_printk(KERN_INFO, shost, + "%s: %s: error already running a task\n", + __func__, fname); + spin_unlock_irqrestore(&devinfo->lock, flags); + return FAILED; + } + + devinfo->running_task = 1; memset(&devinfo->response, 0, sizeof(devinfo->response)); sense_urb = uas_submit_sense_urb(shost, GFP_ATOMIC, tag); if (!sense_urb) { shost_printk(KERN_INFO, shost, "%s: %s: submit sense urb failed\n", __func__, fname); + devinfo->running_task = 0; spin_unlock_irqrestore(&devinfo->lock, flags); return FAILED; } @@ -741,6 +758,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, shost_printk(KERN_INFO, shost, "%s: %s: submit task mgmt urb failed\n", __func__, fname); + devinfo->running_task = 0; spin_unlock_irqrestore(&devinfo->lock, flags); usb_kill_urb(sense_urb); return FAILED; @@ -748,23 +766,33 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, spin_unlock_irqrestore(&devinfo->lock, flags); if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 3000) == 0) { + /* + * Note we deliberately do not clear running_task here. If we + * allow new tasks to be submitted, there is no way to figure + * out if a received response_iu is for the failed task or for + * the new one. A bus-reset will eventually clear running_task. + */ shost_printk(KERN_INFO, shost, "%s: %s timed out\n", __func__, fname); return FAILED; } + + spin_lock_irqsave(&devinfo->lock, flags); + devinfo->running_task = 0; if (be16_to_cpu(devinfo->response.tag) != tag) { shost_printk(KERN_INFO, shost, "%s: %s failed (wrong tag %d/%d)\n", __func__, fname, be16_to_cpu(devinfo->response.tag), tag); - return FAILED; - } - if (devinfo->response.response_code != RC_TMF_COMPLETE) { + result = FAILED; + } else if (devinfo->response.response_code != RC_TMF_COMPLETE) { shost_printk(KERN_INFO, shost, "%s: %s failed (rc 0x%x)\n", __func__, fname, devinfo->response.response_code); - return FAILED; + result = FAILED; } - return SUCCESS; + spin_unlock_irqrestore(&devinfo->lock, flags); + + return result; } static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) @@ -982,6 +1010,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) devinfo->intf = intf; devinfo->udev = udev; devinfo->resetting = 0; + devinfo->running_task = 0; init_usb_anchor(&devinfo->cmd_urbs); init_usb_anchor(&devinfo->sense_urbs); init_usb_anchor(&devinfo->data_urbs); -- cgit v0.10.2 From e36e64930cffd94e1c37fdb82f35989384aa946b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 7 Nov 2013 08:35:55 +0100 Subject: uas: Use GFP_NOIO rather then GFP_ATOMIC where possible We can sleep in our own workqueue (which is the whole reason for having it), and scsi error handlers are also always called from a context which may sleep. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7fc4ad2..8023944 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -133,7 +133,7 @@ static void uas_do_work(struct work_struct *work) struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); - err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC); + err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO); if (!err) { cmdinfo->state &= ~IS_IN_WORK_LIST; list_del(&cmdinfo->work); @@ -745,7 +745,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, devinfo->running_task = 1; memset(&devinfo->response, 0, sizeof(devinfo->response)); - sense_urb = uas_submit_sense_urb(shost, GFP_ATOMIC, tag); + sense_urb = uas_submit_sense_urb(shost, GFP_NOIO, tag); if (!sense_urb) { shost_printk(KERN_INFO, shost, "%s: %s: submit sense urb failed\n", @@ -754,7 +754,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, spin_unlock_irqrestore(&devinfo->lock, flags); return FAILED; } - if (uas_submit_task_urb(cmnd, GFP_ATOMIC, function, tag)) { + if (uas_submit_task_urb(cmnd, GFP_NOIO, function, tag)) { shost_printk(KERN_INFO, shost, "%s: %s: submit task mgmt urb failed\n", __func__, fname); -- cgit v0.10.2 From 0df1f663f32e5dc28cba68375b09bba5eaad103f Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 7 Nov 2013 08:47:05 +0100 Subject: uas: Add suspend/resume support Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 8023944..7a16ed8 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -1091,6 +1091,45 @@ static int uas_post_reset(struct usb_interface *intf) return 0; } +static int uas_suspend(struct usb_interface *intf, pm_message_t message) +{ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + + /* Wait for any pending requests to complete */ + flush_work(&devinfo->work); + if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) == 0) { + shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); + return -ETIME; + } + + return 0; +} + +static int uas_resume(struct usb_interface *intf) +{ + return 0; +} + +static int uas_reset_resume(struct usb_interface *intf) +{ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + unsigned long flags; + + if (uas_configure_endpoints(devinfo) != 0) { + shost_printk(KERN_ERR, shost, + "%s: alloc streams error after reset", __func__); + return -EIO; + } + + spin_lock_irqsave(shost->host_lock, flags); + scsi_report_bus_reset(shost, 0); + spin_unlock_irqrestore(shost->host_lock, flags); + + return 0; +} + static void uas_disconnect(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); @@ -1114,6 +1153,9 @@ static struct usb_driver uas_driver = { .disconnect = uas_disconnect, .pre_reset = uas_pre_reset, .post_reset = uas_post_reset, + .suspend = uas_suspend, + .resume = uas_resume, + .reset_resume = uas_reset_resume, .id_table = uas_usb_ids, }; -- cgit v0.10.2 From da65c2bb99542d05f2d8f67efe6627915f4c5ea4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 11 Nov 2013 11:51:42 +0100 Subject: uas: Reset device on reboot Some BIOS-es will hang on reboot when an uas device is attached and left in uas mode on reboot. This commit adds a shutdown handler which on reboot puts the device back into usb-storage mode, fixing the hang on reboot on these systems. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7a16ed8..019f203 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -54,6 +54,7 @@ struct uas_dev_info { unsigned use_streams:1; unsigned uas_sense_old:1; unsigned running_task:1; + unsigned shutdown:1; struct scsi_cmnd *cmnd; spinlock_t lock; struct work_struct work; @@ -1011,6 +1012,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) devinfo->udev = udev; devinfo->resetting = 0; devinfo->running_task = 0; + devinfo->shutdown = 0; init_usb_anchor(&devinfo->cmd_urbs); init_usb_anchor(&devinfo->sense_urbs); init_usb_anchor(&devinfo->data_urbs); @@ -1053,6 +1055,9 @@ static int uas_pre_reset(struct usb_interface *intf) struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; unsigned long flags; + if (devinfo->shutdown) + return 0; + /* Block new requests */ spin_lock_irqsave(shost->host_lock, flags); scsi_block_requests(shost); @@ -1076,6 +1081,9 @@ static int uas_post_reset(struct usb_interface *intf) struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; unsigned long flags; + if (devinfo->shutdown) + return 0; + if (uas_configure_endpoints(devinfo) != 0) { shost_printk(KERN_ERR, shost, "%s: alloc streams error after reset", __func__); @@ -1147,6 +1155,27 @@ static void uas_disconnect(struct usb_interface *intf) kfree(devinfo); } +/* + * Put the device back in usb-storage mode on shutdown, as some BIOS-es + * hang on reboot when the device is still in uas mode. Note the reset is + * necessary as some devices won't revert to usb-storage mode without it. + */ +static void uas_shutdown(struct device *dev) +{ + struct usb_interface *intf = to_usb_interface(dev); + struct usb_device *udev = interface_to_usbdev(intf); + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + + if (system_state != SYSTEM_RESTART) + return; + + devinfo->shutdown = 1; + uas_free_streams(devinfo); + usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); + usb_reset_device(udev); +} + static struct usb_driver uas_driver = { .name = "uas", .probe = uas_probe, @@ -1156,6 +1185,7 @@ static struct usb_driver uas_driver = { .suspend = uas_suspend, .resume = uas_resume, .reset_resume = uas_reset_resume, + .drvwrap.driver.shutdown = uas_shutdown, .id_table = uas_usb_ids, }; -- cgit v0.10.2 From f323abcda35ea4bae851c9be8f115ee45cc9cf22 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 10:51:33 +0100 Subject: uas: Fix task-management not working when connected over USB-2 For USB-2 connections the stream-id must always be 0. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 019f203..10e580e 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -746,7 +746,8 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, devinfo->running_task = 1; memset(&devinfo->response, 0, sizeof(devinfo->response)); - sense_urb = uas_submit_sense_urb(shost, GFP_NOIO, tag); + sense_urb = uas_submit_sense_urb(shost, GFP_NOIO, + devinfo->use_streams ? tag : 0); if (!sense_urb) { shost_printk(KERN_INFO, shost, "%s: %s: submit sense urb failed\n", -- cgit v0.10.2 From c6d4579d4ba24c494d03daf656cd2ff2a9e683c6 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 10:53:57 +0100 Subject: uas: uas_alloc_data_urb: Remove unnecessary use_streams check uas_alloc_data_urb always gets called with a stream_id value of 0 when not using streams. Removing the check makes it consistent with uas_alloc_sense_urb. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 10e580e..e06505c 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -429,8 +429,7 @@ static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp, goto out; usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length, uas_data_cmplt, cmnd); - if (devinfo->use_streams) - urb->stream_id = stream_id; + urb->stream_id = stream_id; urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0; urb->sg = sdb->table.sgl; out: -- cgit v0.10.2 From 61c09ce510a1eba8595beda6aac194f42571d768 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 13:44:20 +0100 Subject: uas: Properly complete inflight commands on bus-reset or disconnect Before this commit the uas driver would keep track of scsi commands which still need to have some urbs submitted to the device, and complete this with an ABORT result code on bus-reset or disconnect, but in flight scsi commands which have all their urbs submitted, and thus are not part of the work list, would never get their done callback called. The problem is killed sense urbs don't have any tag info, so it is impossible to tell which scsi cmd they belong to, so merely making sure all the urbs have completed one way or the other is not enough. This commit fixes this by changing the work list to an inflight list, which keeps tracks of all inflight scsi cmnds, using the IS_IN_WORK_LIST flag to determine if actual work needs to be done in uas_do_work(), and by moving marking all inflight scsi commands as aborted and moving them to the dead list on bus-reset or disconnect. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index e06505c..1a18839 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -58,7 +58,7 @@ struct uas_dev_info { struct scsi_cmnd *cmnd; spinlock_t lock; struct work_struct work; - struct list_head work_list; + struct list_head inflight_list; struct list_head dead_list; }; @@ -86,7 +86,7 @@ struct uas_cmd_info { struct urb *cmd_urb; struct urb *data_in_urb; struct urb *data_out_urb; - struct list_head work; + struct list_head inflight; struct list_head dead; }; @@ -125,34 +125,36 @@ static void uas_do_work(struct work_struct *work) struct uas_dev_info *devinfo = container_of(work, struct uas_dev_info, work); struct uas_cmd_info *cmdinfo; - struct uas_cmd_info *temp; unsigned long flags; int err; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { + list_for_each_entry(cmdinfo, &devinfo->inflight_list, inflight) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); + + if (!(cmdinfo->state & IS_IN_WORK_LIST)) + continue; + err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO); - if (!err) { + if (!err) cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->work); - } else { + else schedule_work(&devinfo->work); - } } spin_unlock_irqrestore(&devinfo->lock, flags); } -static void uas_abort_work(struct uas_dev_info *devinfo) +static void uas_abort_inflight(struct uas_dev_info *devinfo) { struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; unsigned long flags; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { + list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, + inflight) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); @@ -160,7 +162,7 @@ static void uas_abort_work(struct uas_dev_info *devinfo) WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->work); + list_del(&cmdinfo->inflight); list_add_tail(&cmdinfo->dead, &devinfo->dead_list); } spin_unlock_irqrestore(&devinfo->lock, flags); @@ -173,7 +175,6 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) struct uas_dev_info *devinfo = cmnd->device->hostdata; WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); - list_add_tail(&cmdinfo->work, &devinfo->work_list); cmdinfo->state |= IS_IN_WORK_LIST; schedule_work(&devinfo->work); } @@ -289,7 +290,8 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) scmd_printk(KERN_INFO, cmnd, "abort completed\n"); cmnd->result = DID_ABORT << 16; list_del(&cmdinfo->dead); - } + } else + list_del(&cmdinfo->inflight); cmnd->scsi_done(cmnd); return 0; } @@ -717,6 +719,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, uas_add_work(cmdinfo); } + list_add_tail(&cmdinfo->inflight, &devinfo->inflight_list); spin_unlock_irqrestore(&devinfo->lock, flags); return 0; } @@ -807,11 +810,9 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) spin_lock_irqsave(&devinfo->lock, flags); WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; + cmdinfo->state &= ~IS_IN_WORK_LIST; + list_del(&cmdinfo->inflight); list_add_tail(&cmdinfo->dead, &devinfo->dead_list); - if (cmdinfo->state & IS_IN_WORK_LIST) { - list_del(&cmdinfo->work); - cmdinfo->state &= ~IS_IN_WORK_LIST; - } if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); @@ -847,7 +848,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); devinfo->resetting = 1; - uas_abort_work(devinfo); + uas_abort_inflight(devinfo); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); @@ -1018,7 +1019,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) init_usb_anchor(&devinfo->data_urbs); spin_lock_init(&devinfo->lock); INIT_WORK(&devinfo->work, uas_do_work); - INIT_LIST_HEAD(&devinfo->work_list); + INIT_LIST_HEAD(&devinfo->inflight_list); INIT_LIST_HEAD(&devinfo->dead_list); result = uas_configure_endpoints(devinfo); @@ -1145,7 +1146,7 @@ static void uas_disconnect(struct usb_interface *intf) devinfo->resetting = 1; cancel_work_sync(&devinfo->work); - uas_abort_work(devinfo); + uas_abort_inflight(devinfo); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); -- cgit v0.10.2 From da3033ea08397fb70279f22789002e6001432f3d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 13:57:24 +0100 Subject: uas: add uas_mark_cmd_dead helper function Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 1a18839..7810c13 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -146,6 +146,21 @@ static void uas_do_work(struct work_struct *work) spin_unlock_irqrestore(&devinfo->lock, flags); } +static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, + struct uas_cmd_info *cmdinfo, const char *caller) +{ + struct scsi_pointer *scp = (void *)cmdinfo; + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); + + uas_log_cmd_state(cmnd, caller); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); + cmdinfo->state |= COMMAND_ABORTED; + cmdinfo->state &= ~IS_IN_WORK_LIST; + list_del(&cmdinfo->inflight); + list_add_tail(&cmdinfo->dead, &devinfo->dead_list); +} + static void uas_abort_inflight(struct uas_dev_info *devinfo) { struct uas_cmd_info *cmdinfo; @@ -154,17 +169,8 @@ static void uas_abort_inflight(struct uas_dev_info *devinfo) spin_lock_irqsave(&devinfo->lock, flags); list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, - inflight) { - struct scsi_pointer *scp = (void *)cmdinfo; - struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, - SCp); - uas_log_cmd_state(cmnd, __func__); - WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); - cmdinfo->state |= COMMAND_ABORTED; - cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->inflight); - list_add_tail(&cmdinfo->dead, &devinfo->dead_list); - } + inflight) + uas_mark_cmd_dead(devinfo, cmdinfo, __func__); spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -806,13 +812,8 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) unsigned long flags; int ret; - uas_log_cmd_state(cmnd, __func__); spin_lock_irqsave(&devinfo->lock, flags); - WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); - cmdinfo->state |= COMMAND_ABORTED; - cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->inflight); - list_add_tail(&cmdinfo->dead, &devinfo->dead_list); + uas_mark_cmd_dead(devinfo, cmdinfo, __func__); if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); -- cgit v0.10.2 From 040d1a8f11f390f36a8cd7fc04c0c836639b0b6a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 14:02:12 +0100 Subject: uas: cmdinfo: use only one list head cmds are either on the inflight list or on the dead list, never both, so we only need one list head. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7810c13..cfe0102 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -86,8 +86,7 @@ struct uas_cmd_info { struct urb *cmd_urb; struct urb *data_in_urb; struct urb *data_out_urb; - struct list_head inflight; - struct list_head dead; + struct list_head list; }; /* I hate forward declarations, but I actually have a loop */ @@ -129,7 +128,7 @@ static void uas_do_work(struct work_struct *work) int err; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry(cmdinfo, &devinfo->inflight_list, inflight) { + list_for_each_entry(cmdinfo, &devinfo->inflight_list, list) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); @@ -157,8 +156,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->inflight); - list_add_tail(&cmdinfo->dead, &devinfo->dead_list); + list_move_tail(&cmdinfo->list, &devinfo->dead_list); } static void uas_abort_inflight(struct uas_dev_info *devinfo) @@ -168,8 +166,7 @@ static void uas_abort_inflight(struct uas_dev_info *devinfo) unsigned long flags; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, - inflight) + list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, list) uas_mark_cmd_dead(devinfo, cmdinfo, __func__); spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -192,7 +189,7 @@ static void uas_zap_dead(struct uas_dev_info *devinfo) unsigned long flags; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &devinfo->dead_list, dead) { + list_for_each_entry_safe(cmdinfo, temp, &devinfo->dead_list, list) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); @@ -295,9 +292,8 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) if (cmdinfo->state & COMMAND_ABORTED) { scmd_printk(KERN_INFO, cmnd, "abort completed\n"); cmnd->result = DID_ABORT << 16; - list_del(&cmdinfo->dead); - } else - list_del(&cmdinfo->inflight); + } + list_del(&cmdinfo->list); cmnd->scsi_done(cmnd); return 0; } @@ -725,7 +721,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, uas_add_work(cmdinfo); } - list_add_tail(&cmdinfo->inflight, &devinfo->inflight_list); + list_add_tail(&cmdinfo->list, &devinfo->inflight_list); spin_unlock_irqrestore(&devinfo->lock, flags); return 0; } -- cgit v0.10.2 From c6f63207a3ba689025b2120792ea831cf72f9a81 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 13 Nov 2013 09:24:15 +0100 Subject: uas: Fix command / task mgmt submission racing with disconnect Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index cfe0102..8c68580 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -670,13 +670,15 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, BUILD_BUG_ON(sizeof(struct uas_cmd_info) > sizeof(struct scsi_pointer)); + spin_lock_irqsave(&devinfo->lock, flags); + if (devinfo->resetting) { cmnd->result = DID_ERROR << 16; cmnd->scsi_done(cmnd); + spin_unlock_irqrestore(&devinfo->lock, flags); return 0; } - spin_lock_irqsave(&devinfo->lock, flags); if (devinfo->cmnd) { spin_unlock_irqrestore(&devinfo->lock, flags); return SCSI_MLQUEUE_DEVICE_BUSY; @@ -740,6 +742,11 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, spin_lock_irqsave(&devinfo->lock, flags); + if (devinfo->resetting) { + spin_unlock_irqrestore(&devinfo->lock, flags); + return FAILED; + } + if (devinfo->running_task) { shost_printk(KERN_INFO, shost, "%s: %s: error already running a task\n", @@ -809,6 +816,12 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) int ret; spin_lock_irqsave(&devinfo->lock, flags); + + if (devinfo->resetting) { + spin_unlock_irqrestore(&devinfo->lock, flags); + return FAILED; + } + uas_mark_cmd_dead(devinfo, cmdinfo, __func__); if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); -- cgit v0.10.2 From 21fc05b680f6fba868b41e2713ade3fdea4049f9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 13 Nov 2013 09:32:22 +0100 Subject: uas: Fix memory management The scsi-host structure is refcounted, scsi_remove_host tears down the scsi-host but does not decrement the refcount, so we need to call scsi_put_host on disconnect to get the underlying memory to be freed. After calling scsi_remove_host, the scsi-core may still hold a reference to the scsi-host, iow we may still get called after uas_disconnect, but we do our own life cycle management of uas_devinfo, freeing it on disconnect, and thus may end up using devinfo after it has been freed. Switch to letting scsi_host_alloc allocate and manage the memory for us. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 8c68580..d81d041 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -315,7 +315,7 @@ static void uas_stat_cmplt(struct urb *urb) { struct iu *iu = urb->transfer_buffer; struct Scsi_Host *shost = urb->context; - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; struct scsi_cmnd *cmnd; struct uas_cmd_info *cmdinfo; unsigned long flags; @@ -562,7 +562,7 @@ err: static struct urb *uas_submit_sense_urb(struct Scsi_Host *shost, gfp_t gfp, unsigned int stream) { - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; struct urb *urb; urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); @@ -734,7 +734,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, const char *fname, u8 function) { struct Scsi_Host *shost = cmnd->device->host; - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; u16 tag = devinfo->qdepth; unsigned long flags; struct urb *sense_urb; @@ -879,7 +879,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) static int uas_slave_alloc(struct scsi_device *sdev) { - sdev->hostdata = (void *)sdev->host->hostdata[0]; + sdev->hostdata = (void *)sdev->host->hostdata; return 0; } @@ -1005,11 +1005,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) if (uas_switch_interface(udev, intf)) return -ENODEV; - devinfo = kmalloc(sizeof(struct uas_dev_info), GFP_KERNEL); - if (!devinfo) - goto set_alt0; - - shost = scsi_host_alloc(&uas_host_template, sizeof(void *)); + shost = scsi_host_alloc(&uas_host_template, + sizeof(struct uas_dev_info)); if (!shost) goto set_alt0; @@ -1019,6 +1016,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) shost->max_channel = 0; shost->sg_tablesize = udev->bus->sg_tablesize; + devinfo = (struct uas_dev_info *)shost->hostdata; devinfo->intf = intf; devinfo->udev = udev; devinfo->resetting = 0; @@ -1044,8 +1042,6 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) if (result) goto free_streams; - shost->hostdata[0] = (unsigned long)devinfo; - scsi_scan_host(shost); usb_set_intfdata(intf, shost); return result; @@ -1054,7 +1050,6 @@ free_streams: uas_free_streams(devinfo); set_alt0: usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); - kfree(devinfo); if (shost) scsi_host_put(shost); return result; @@ -1063,7 +1058,7 @@ set_alt0: static int uas_pre_reset(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; unsigned long flags; if (devinfo->shutdown) @@ -1089,7 +1084,7 @@ static int uas_pre_reset(struct usb_interface *intf) static int uas_post_reset(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; unsigned long flags; if (devinfo->shutdown) @@ -1113,7 +1108,7 @@ static int uas_post_reset(struct usb_interface *intf) static int uas_suspend(struct usb_interface *intf, pm_message_t message) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; /* Wait for any pending requests to complete */ flush_work(&devinfo->work); @@ -1133,7 +1128,7 @@ static int uas_resume(struct usb_interface *intf) static int uas_reset_resume(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; unsigned long flags; if (uas_configure_endpoints(devinfo) != 0) { @@ -1152,7 +1147,7 @@ static int uas_reset_resume(struct usb_interface *intf) static void uas_disconnect(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; devinfo->resetting = 1; cancel_work_sync(&devinfo->work); @@ -1163,7 +1158,7 @@ static void uas_disconnect(struct usb_interface *intf) uas_zap_dead(devinfo); scsi_remove_host(shost); uas_free_streams(devinfo); - kfree(devinfo); + scsi_host_put(shost); } /* @@ -1176,7 +1171,7 @@ static void uas_shutdown(struct device *dev) struct usb_interface *intf = to_usb_interface(dev); struct usb_device *udev = interface_to_usbdev(intf); struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; if (system_state != SYSTEM_RESTART) return; -- cgit v0.10.2 From 3a4462e0e2fe8f715f54147d36b5433a7ff5a85a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 14 Nov 2013 11:06:13 +0100 Subject: uas: Clear cmdinfo on command queue-ing The scsi error handling path re-uses previously queued up (and errored-out) cmds. If such a re-used cmd had a data-phase then cmdinfo will have data_in_urb / data_out_urb still set to the free-ed urbs from the errored-out cmd, and they will get free-ed a second time when the error handling cmd completes, corrupting the kernel heap. Clearing cmdinfo on command queue-ing fixes this, and seems like a good idea in general. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index d81d041..fceffcc 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -684,6 +684,8 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, return SCSI_MLQUEUE_DEVICE_BUSY; } + memset(cmdinfo, 0, sizeof(*cmdinfo)); + if (blk_rq_tagged(cmnd->request)) { cmdinfo->stream = cmnd->request->tag + 2; } else { -- cgit v0.10.2 From 673331c87c492898a9152f3754f3174128e1514a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 14 Nov 2013 14:27:27 +0100 Subject: uas: Use the right error codes for different kinds of errors Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index fceffcc..6ec48c2 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -146,7 +146,8 @@ static void uas_do_work(struct work_struct *work) } static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, - struct uas_cmd_info *cmdinfo, const char *caller) + struct uas_cmd_info *cmdinfo, + int result, const char *caller) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); @@ -156,10 +157,12 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; + cmnd->result = result << 16; list_move_tail(&cmdinfo->list, &devinfo->dead_list); } -static void uas_abort_inflight(struct uas_dev_info *devinfo) +static void uas_abort_inflight(struct uas_dev_info *devinfo, int result, + const char *caller) { struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; @@ -167,7 +170,7 @@ static void uas_abort_inflight(struct uas_dev_info *devinfo) spin_lock_irqsave(&devinfo->lock, flags); list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, list) - uas_mark_cmd_dead(devinfo, cmdinfo, __func__); + uas_mark_cmd_dead(devinfo, cmdinfo, result, caller); spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -289,10 +292,8 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) cmdinfo->state |= COMMAND_COMPLETED; usb_free_urb(cmdinfo->data_in_urb); usb_free_urb(cmdinfo->data_out_urb); - if (cmdinfo->state & COMMAND_ABORTED) { + if (cmdinfo->state & COMMAND_ABORTED) scmd_printk(KERN_INFO, cmnd, "abort completed\n"); - cmnd->result = DID_ABORT << 16; - } list_del(&cmdinfo->list); cmnd->scsi_done(cmnd); return 0; @@ -824,7 +825,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) return FAILED; } - uas_mark_cmd_dead(devinfo, cmdinfo, __func__); + uas_mark_cmd_dead(devinfo, cmdinfo, DID_ABORT, __func__); if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); @@ -860,7 +861,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); devinfo->resetting = 1; - uas_abort_inflight(devinfo); + uas_abort_inflight(devinfo, DID_RESET, __func__); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); @@ -1153,7 +1154,7 @@ static void uas_disconnect(struct usb_interface *intf) devinfo->resetting = 1; cancel_work_sync(&devinfo->work); - uas_abort_inflight(devinfo); + uas_abort_inflight(devinfo, DID_NO_CONNECT, __func__); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); -- cgit v0.10.2 From 876285cc9cf418f626375f28bb0fc5d88012f12d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 7 Nov 2013 08:52:42 +0100 Subject: uas: Improve error reporting Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6ec48c2..f09205b 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -411,6 +411,12 @@ static void uas_data_cmplt(struct urb *urb) if (sdb == NULL) { WARN_ON_ONCE(1); } else if (urb->status) { + if (urb->status != -ECONNRESET) { + uas_log_cmd_state(cmnd, __func__); + scmd_printk(KERN_ERR, cmnd, + "data cmplt err %d stream %d\n", + urb->status, urb->stream_id); + } /* error: no data transfered */ sdb->resid = sdb->length; } else { @@ -420,6 +426,17 @@ static void uas_data_cmplt(struct urb *urb) spin_unlock_irqrestore(&devinfo->lock, flags); } +static void uas_cmd_cmplt(struct urb *urb) +{ + struct scsi_cmnd *cmnd = urb->context; + + if (urb->status) { + uas_log_cmd_state(cmnd, __func__); + scmd_printk(KERN_ERR, cmnd, "cmd cmplt err %d\n", urb->status); + } + usb_free_urb(urb); +} + static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp, unsigned int pipe, u16 stream_id, struct scsi_cmnd *cmnd, @@ -497,7 +514,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp, memcpy(iu->cdb, cmnd->cmnd, cmnd->cmd_len); usb_fill_bulk_urb(urb, udev, devinfo->cmd_pipe, iu, sizeof(*iu) + len, - usb_free_urb, NULL); + uas_cmd_cmplt, cmnd); urb->transfer_flags |= URB_FREE_BUFFER; out: return urb; @@ -537,13 +554,15 @@ static int uas_submit_task_urb(struct scsi_cmnd *cmnd, gfp_t gfp, } usb_fill_bulk_urb(urb, udev, devinfo->cmd_pipe, iu, sizeof(*iu), - usb_free_urb, NULL); + uas_cmd_cmplt, cmnd); urb->transfer_flags |= URB_FREE_BUFFER; usb_anchor_urb(urb, &devinfo->cmd_urbs); err = usb_submit_urb(urb, gfp); if (err) { usb_unanchor_urb(urb); + uas_log_cmd_state(cmnd, __func__); + scmd_printk(KERN_ERR, cmnd, "task submission err %d\n", err); goto err; } @@ -560,20 +579,25 @@ err: * daft to me. */ -static struct urb *uas_submit_sense_urb(struct Scsi_Host *shost, +static struct urb *uas_submit_sense_urb(struct scsi_cmnd *cmnd, gfp_t gfp, unsigned int stream) { + struct Scsi_Host *shost = cmnd->device->host; struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; struct urb *urb; + int err; urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); if (!urb) return NULL; usb_anchor_urb(urb, &devinfo->sense_urbs); - if (usb_submit_urb(urb, gfp)) { + err = usb_submit_urb(urb, gfp); + if (err) { usb_unanchor_urb(urb); + uas_log_cmd_state(cmnd, __func__); shost_printk(KERN_INFO, shost, - "sense urb submission failure\n"); + "sense urb submission error %d stream %d\n", + err, stream); usb_free_urb(urb); return NULL; } @@ -585,11 +609,11 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, { struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; struct urb *urb; + int err; WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & SUBMIT_STATUS_URB) { - urb = uas_submit_sense_urb(cmnd->device->host, gfp, - cmdinfo->stream); + urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo->stream); if (!urb) return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~SUBMIT_STATUS_URB; @@ -606,10 +630,13 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (cmdinfo->state & SUBMIT_DATA_IN_URB) { usb_anchor_urb(cmdinfo->data_in_urb, &devinfo->data_urbs); - if (usb_submit_urb(cmdinfo->data_in_urb, gfp)) { + err = usb_submit_urb(cmdinfo->data_in_urb, gfp); + if (err) { usb_unanchor_urb(cmdinfo->data_in_urb); + uas_log_cmd_state(cmnd, __func__); scmd_printk(KERN_INFO, cmnd, - "data in urb submission failure\n"); + "data in urb submission error %d stream %d\n", + err, cmdinfo->data_in_urb->stream_id); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_IN_URB; @@ -627,10 +654,13 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (cmdinfo->state & SUBMIT_DATA_OUT_URB) { usb_anchor_urb(cmdinfo->data_out_urb, &devinfo->data_urbs); - if (usb_submit_urb(cmdinfo->data_out_urb, gfp)) { + err = usb_submit_urb(cmdinfo->data_out_urb, gfp); + if (err) { usb_unanchor_urb(cmdinfo->data_out_urb); + uas_log_cmd_state(cmnd, __func__); scmd_printk(KERN_INFO, cmnd, - "data out urb submission failure\n"); + "data out urb submission error %d stream %d\n", + err, cmdinfo->data_out_urb->stream_id); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_OUT_URB; @@ -646,10 +676,12 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (cmdinfo->state & SUBMIT_CMD_URB) { usb_anchor_urb(cmdinfo->cmd_urb, &devinfo->cmd_urbs); - if (usb_submit_urb(cmdinfo->cmd_urb, gfp)) { + err = usb_submit_urb(cmdinfo->cmd_urb, gfp); + if (err) { usb_unanchor_urb(cmdinfo->cmd_urb); + uas_log_cmd_state(cmnd, __func__); scmd_printk(KERN_INFO, cmnd, - "cmd urb submission failure\n"); + "cmd urb submission error %d\n", err); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->cmd_urb = NULL; @@ -760,7 +792,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, devinfo->running_task = 1; memset(&devinfo->response, 0, sizeof(devinfo->response)); - sense_urb = uas_submit_sense_urb(shost, GFP_NOIO, + sense_urb = uas_submit_sense_urb(cmnd, GFP_NOIO, devinfo->use_streams ? tag : 0); if (!sense_urb) { shost_printk(KERN_INFO, shost, -- cgit v0.10.2 From 8e453155d7f8dfa53863ba6f8da6c68f7c17ece4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 15 Nov 2013 10:04:11 +0100 Subject: uas: Add some data in/out ready iu sanity checks Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index f09205b..6208682 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -379,9 +379,19 @@ static void uas_stat_cmplt(struct urb *urb) uas_try_complete(cmnd, __func__); break; case IU_ID_READ_READY: + if (!cmdinfo->data_in_urb || + (cmdinfo->state & DATA_IN_URB_INFLIGHT)) { + scmd_printk(KERN_ERR, cmnd, "unexpected read rdy\n"); + break; + } uas_xfer_data(urb, cmnd, SUBMIT_DATA_IN_URB); break; case IU_ID_WRITE_READY: + if (!cmdinfo->data_out_urb || + (cmdinfo->state & DATA_OUT_URB_INFLIGHT)) { + scmd_printk(KERN_ERR, cmnd, "unexpected write rdy\n"); + break; + } uas_xfer_data(urb, cmnd, SUBMIT_DATA_OUT_URB); break; default: -- cgit v0.10.2 From 37599f9603bed3d72becdc1a59c164576df9fbfc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 15 Nov 2013 10:04:31 +0100 Subject: uas: Make sure sg elements are properly aligned Copy the sg alignment trick from the usb-storage driver, without this I'm seeing intermittent errors when using uas devices with an ehci controller. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6208682..ad97615 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -925,6 +925,24 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) static int uas_slave_alloc(struct scsi_device *sdev) { sdev->hostdata = (void *)sdev->host->hostdata; + + /* USB has unusual DMA-alignment requirements: Although the + * starting address of each scatter-gather element doesn't matter, + * the length of each element except the last must be divisible + * by the Bulk maxpacket value. There's currently no way to + * express this by block-layer constraints, so we'll cop out + * and simply require addresses to be aligned at 512-byte + * boundaries. This is okay since most block I/O involves + * hardware sectors that are multiples of 512 bytes in length, + * and since host controllers up through USB 2.0 have maxpacket + * values no larger than 512. + * + * But it doesn't suffice for Wireless USB, where Bulk maxpacket + * values can be as large as 2048. To make that work properly + * will require changes to the block layer. + */ + blk_queue_update_dma_alignment(sdev->request_queue, (512 - 1)); + return 0; } -- cgit v0.10.2 From dc88608dba784f902b3127fd68d0c4f92a532cd0 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 13 Sep 2013 13:27:15 +0200 Subject: uas: remove BROKEN xhci streams support is fixed, unblock usb attached scsi. Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/Kconfig b/drivers/usb/storage/Kconfig index 666dcb6..13b5bfb 100644 --- a/drivers/usb/storage/Kconfig +++ b/drivers/usb/storage/Kconfig @@ -204,7 +204,7 @@ config USB_STORAGE_ENE_UB6250 config USB_UAS tristate "USB Attached SCSI" - depends on SCSI && USB_STORAGE && BROKEN + depends on SCSI && USB_STORAGE help The USB Attached SCSI protocol is supported by some USB storage devices. It permits higher performance by supporting -- cgit v0.10.2 From f50a4968deb7bf38c46f5baf62db9431a099531a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 28 Oct 2013 10:48:04 +0000 Subject: uas: Add Hans de Goede as uas maintainer At the kernel-summit Sarah Sharp asked me if I was willing to become the uas maintainer. I said yes, and here is a patch to make this official. Also remove Matthew Wilcox and Sarah Sharp as maintainers at their request. I've also added myself to the module's author tag, so that if people look there rather then in maintainers they will know they should bug me about uas too. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/MAINTAINERS b/MAINTAINERS index c6d0e93..fc536bc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9063,8 +9063,7 @@ S: Maintained F: drivers/net/wireless/ath/ar5523/ USB ATTACHED SCSI -M: Matthew Wilcox -M: Sarah Sharp +M: Hans de Goede M: Gerd Hoffmann L: linux-usb@vger.kernel.org L: linux-scsi@vger.kernel.org diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index ad97615..08e9710 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -2,6 +2,7 @@ * USB Attached SCSI * Note that this is not the same as the USB Mass Storage driver * + * Copyright Hans de Goede for Red Hat, Inc. 2013 * Copyright Matthew Wilcox for Intel Corp, 2010 * Copyright Sarah Sharp for Intel Corp, 2010 * @@ -1261,4 +1262,5 @@ static struct usb_driver uas_driver = { module_usb_driver(uas_driver); MODULE_LICENSE("GPL"); -MODULE_AUTHOR("Matthew Wilcox and Sarah Sharp"); +MODULE_AUTHOR( + "Hans de Goede , Matthew Wilcox and Sarah Sharp"); -- cgit v0.10.2 From 7cace978fba5d0ec6eed50509cda40eea85f8e98 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 11:36:43 +0100 Subject: uas: Remove comment about registering a uas scsi controller for each usb bus Although an interesting concept, I don't think that this is a good idea: -This will result in lots of "virtual" scsi controllers confusing users -If we get a scsi-bus-reset we will now need to do a usb-device-reset of all uas devices on the same usb bus, which is something to avoid if possible Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 08e9710..a7ac97c 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -1050,12 +1050,6 @@ static void uas_free_streams(struct uas_dev_info *devinfo) usb_free_streams(devinfo->intf, eps, 3, GFP_KERNEL); } -/* - * XXX: What I'd like to do here is register a SCSI host for each USB host in - * the system. Follow usb-storage's design of registering a SCSI host for - * each USB device for the moment. Can implement this by walking up the - * USB hierarchy until we find a USB host. - */ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) { int result = -ENOMEM; -- cgit v0.10.2 From 50e8725e7c429701e530439013f9681e1fa36b5d Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Fri, 21 Feb 2014 09:27:30 -0800 Subject: xhci: Refactor command watchdog and fix split string. In preparation for fixing this function for streams endpoints, refactor code in the command watchdog timeout function into two new functions. One kills all URBs on a ring (either stream or endpoint), the other kills all URBs associated with an endpoint. Fix a split string while we're at it. Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index b3d27e6..58cbc06 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -900,6 +900,43 @@ remove_finished_td: /* Return to the event handler with xhci->lock re-acquired */ } +static void xhci_kill_ring_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring) +{ + struct xhci_td *cur_td; + + while (!list_empty(&ring->td_list)) { + cur_td = list_first_entry(&ring->td_list, + struct xhci_td, td_list); + list_del_init(&cur_td->td_list); + if (!list_empty(&cur_td->cancelled_td_list)) + list_del_init(&cur_td->cancelled_td_list); + xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN); + } +} + +static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, + int slot_id, int ep_index) +{ + struct xhci_td *cur_td; + struct xhci_virt_ep *ep; + struct xhci_ring *ring; + + ep = &xhci->devs[slot_id]->eps[ep_index]; + ring = ep->ring; + if (!ring) + return; + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Killing URBs for slot ID %u, ep index %u", + slot_id, ep_index); + xhci_kill_ring_urbs(xhci, ring); + while (!list_empty(&ep->cancelled_td_list)) { + cur_td = list_first_entry(&ep->cancelled_td_list, + struct xhci_td, cancelled_td_list); + list_del_init(&cur_td->cancelled_td_list); + xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN); + } +} + /* Watchdog timer function for when a stop endpoint command fails to complete. * In this case, we assume the host controller is broken or dying or dead. The * host may still be completing some other events, so we have to be careful to @@ -923,9 +960,6 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) { struct xhci_hcd *xhci; struct xhci_virt_ep *ep; - struct xhci_virt_ep *temp_ep; - struct xhci_ring *ring; - struct xhci_td *cur_td; int ret, i, j; unsigned long flags; @@ -982,34 +1016,8 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) for (i = 0; i < MAX_HC_SLOTS; i++) { if (!xhci->devs[i]) continue; - for (j = 0; j < 31; j++) { - temp_ep = &xhci->devs[i]->eps[j]; - ring = temp_ep->ring; - if (!ring) - continue; - xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, - "Killing URBs for slot ID %u, " - "ep index %u", i, j); - while (!list_empty(&ring->td_list)) { - cur_td = list_first_entry(&ring->td_list, - struct xhci_td, - td_list); - list_del_init(&cur_td->td_list); - if (!list_empty(&cur_td->cancelled_td_list)) - list_del_init(&cur_td->cancelled_td_list); - xhci_giveback_urb_in_irq(xhci, cur_td, - -ESHUTDOWN); - } - while (!list_empty(&temp_ep->cancelled_td_list)) { - cur_td = list_first_entry( - &temp_ep->cancelled_td_list, - struct xhci_td, - cancelled_td_list); - list_del_init(&cur_td->cancelled_td_list); - xhci_giveback_urb_in_irq(xhci, cur_td, - -ESHUTDOWN); - } - } + for (j = 0; j < 31; j++) + xhci_kill_endpoint_urbs(xhci, i, j); } spin_unlock_irqrestore(&xhci->lock, flags); xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, -- cgit v0.10.2 From 21d0e51bfb290349adc02fa8fec716d77f53df51 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Fri, 21 Feb 2014 14:29:02 -0800 Subject: xhci: Kill streams URBs when the host dies. If the host controller stops responding to commands, we need to kill all the URBs that were queued to all endpoints. The current code would only kill URBs that had been queued to the endpoint rings. ep->ring is set to NULL if streams has been enabled for the endpoint, which means URBs submitted with a non-zero stream_id would never get killed. Fix this. Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 58cbc06..5f926be 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -922,13 +922,27 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring; ep = &xhci->devs[slot_id]->eps[ep_index]; - ring = ep->ring; - if (!ring) - return; - xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, - "Killing URBs for slot ID %u, ep index %u", - slot_id, ep_index); - xhci_kill_ring_urbs(xhci, ring); + if ((ep->ep_state & EP_HAS_STREAMS) || + (ep->ep_state & EP_GETTING_NO_STREAMS)) { + int stream_id; + + for (stream_id = 0; stream_id < ep->stream_info->num_streams; + stream_id++) { + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Killing URBs for slot ID %u, ep index %u, stream %u", + slot_id, ep_index, stream_id + 1); + xhci_kill_ring_urbs(xhci, + ep->stream_info->stream_rings[stream_id]); + } + } else { + ring = ep->ring; + if (!ring) + return; + xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, + "Killing URBs for slot ID %u, ep index %u", + slot_id, ep_index); + xhci_kill_ring_urbs(xhci, ring); + } while (!list_empty(&ep->cancelled_td_list)) { cur_td = list_first_entry(&ep->cancelled_td_list, struct xhci_td, cancelled_td_list); -- cgit v0.10.2 From 14aec589327a6fc4035f5327d90ac5548f501c4c Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Tue, 11 Feb 2014 20:36:04 +0100 Subject: storage: accept some UAS devices if streams are unavailable On some older XHCIs streams are not supported and the UAS driver will fail at probe time. For those devices storage should try to bind to UAS devices. This patch adds a flag for stream support to HCDs and evaluates it. [Note: Sarah fixed a bug where the USB 2.0 root hub, not USB 3.0 root hub would get marked as being able to support streams.] Signed-off-by: Oliver Neukum Signed-off-by: Sarah Sharp Acked-by: Hans de Goede diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 6c03584..cbf0c5c 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -222,6 +222,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) goto put_usb3_hcd; /* Roothub already marked as USB 3.0 speed */ + if (HCC_MAX_PSA(xhci->hcc_params) >= 4) + xhci->shared_hcd->can_do_streams = 1; + return 0; put_usb3_hcd: diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 8affef9..151901c 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -158,6 +158,9 @@ static int xhci_plat_probe(struct platform_device *pdev) */ *((struct xhci_hcd **) xhci->shared_hcd->hcd_priv) = xhci; + if (HCC_MAX_PSA(xhci->hcc_params) >= 4) + xhci->shared_hcd->can_do_streams = 1; + ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED); if (ret) goto put_usb3_hcd; diff --git a/drivers/usb/storage/uas-detect.h b/drivers/usb/storage/uas-detect.h index b8a02e1..bb05b98 100644 --- a/drivers/usb/storage/uas-detect.h +++ b/drivers/usb/storage/uas-detect.h @@ -72,6 +72,7 @@ static int uas_use_uas_driver(struct usb_interface *intf, { struct usb_host_endpoint *eps[4] = { }; struct usb_device *udev = interface_to_usbdev(intf); + struct usb_hcd *hcd = bus_to_hcd(udev->bus); unsigned long flags = id->driver_info; int r, alt; @@ -80,6 +81,9 @@ static int uas_use_uas_driver(struct usb_interface *intf, if (flags & US_FL_IGNORE_UAS) return 0; + if (udev->speed >= USB_SPEED_SUPER && !hcd->can_do_streams) + return 0; + alt = uas_find_uas_alt_setting(intf); if (alt < 0) return 0; diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index efe8d8a..485cd5e 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -143,6 +143,7 @@ struct usb_hcd { unsigned authorized_default:1; unsigned has_tt:1; /* Integrated TT in root hub */ unsigned amd_resume_bug:1; /* AMD remote wakeup quirk */ + unsigned can_do_streams:1; /* HC supports streams */ unsigned int irq; /* irq allocated */ void __iomem *regs; /* device memory/io */ -- cgit v0.10.2 From bcffae7708eb8352f44dc510b326541fe43a02a4 Mon Sep 17 00:00:00 2001 From: Mathias Nyman Date: Mon, 3 Mar 2014 19:30:17 +0200 Subject: xhci: Prevent runtime pm from autosuspending during initialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready. Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready. xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe. Signed-off-by: Mathias Nyman Acked-by: Dan Williams Acked-by: Alan Stern Signed-off-by: Sarah Sharp Cc: stable@vger.kernel.org diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index cbf0c5c..47390e3 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -190,6 +190,10 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) struct usb_hcd *hcd; driver = (struct hc_driver *)id->driver_data; + + /* Prevent runtime suspending between USB-2 and USB-3 initialization */ + pm_runtime_get_noresume(&dev->dev); + /* Register the USB 2.0 roothub. * FIXME: USB core must know to register the USB 2.0 roothub first. * This is sort of silly, because we could just set the HCD driver flags @@ -199,7 +203,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) retval = usb_hcd_pci_probe(dev, id); if (retval) - return retval; + goto put_runtime_pm; /* USB 2.0 roothub is stored in the PCI device now. */ hcd = dev_get_drvdata(&dev->dev); @@ -225,12 +229,17 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) if (HCC_MAX_PSA(xhci->hcc_params) >= 4) xhci->shared_hcd->can_do_streams = 1; + /* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */ + pm_runtime_put_noidle(&dev->dev); + return 0; put_usb3_hcd: usb_put_hcd(xhci->shared_hcd); dealloc_usb2_hcd: usb_hcd_pci_remove(dev); +put_runtime_pm: + pm_runtime_put_noidle(&dev->dev); return retval; } -- cgit v0.10.2 From 7969943789df1196faa9ba67518d83fd93e4f9f6 Mon Sep 17 00:00:00 2001 From: Adrian Huang Date: Thu, 27 Feb 2014 11:26:03 +0000 Subject: xhci: add the meaningful IRQ description if it is empty When some xHCI host controllers fall back to use the legacy IRQ, the member irq_descr of the usb_hcd structure will be empty. This leads to the empty string of the xHCI host controller in /proc/interrupts. Here is the example (The irq 19 is the xHCI host controller): CPU0 0: 91 IO-APIC-edge timer 8: 1 IO-APIC-edge rtc0 9: 7191 IO-APIC-fasteoi acpi 18: 104 IR-IO-APIC-fasteoi ehci_hcd:usb1, ehci_hcd:usb2 19: 473 IR-IO-APIC-fasteoi After applying the patch, the name of the registered xHCI host controller can be displayed correctly. Here is the example: CPU0 0: 91 IO-APIC-edge timer 8: 1 IO-APIC-edge rtc0 9: 7191 IO-APIC-fasteoi acpi 18: 104 IR-IO-APIC-fasteoi ehci_hcd:usb1, ehci_hcd:usb2 19: 473 IR-IO-APIC-fasteoi xhci_hcd:usb3 Tested on v3.14-rc4. Signed-off-by: Adrian Huang Reviewed-by: Nagananda Chumbalkar Signed-off-by: Sarah Sharp diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 1db3c27..652be21 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -390,6 +390,10 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd) } legacy_irq: + if (!strlen(hcd->irq_descr)) + snprintf(hcd->irq_descr, sizeof(hcd->irq_descr), "%s:usb%d", + hcd->driver->description, hcd->self.busnum); + /* fall back to legacy interrupt*/ ret = request_irq(pdev->irq, &usb_hcd_irq, IRQF_SHARED, hcd->irq_descr, hcd); -- cgit v0.10.2