From 158886cd2cf4599e04f9b7e10cb767f5f39b14f1 Mon Sep 17 00:00:00 2001 From: Andiry Xu Date: Wed, 30 Nov 2011 16:37:41 +0800 Subject: xHCI: fix bug in xhci_clear_command_ring() When system enters suspend, xHCI driver clears command ring by writing zero to all the TRBs. However, this also writes zero to the Link TRB, and the ring is mangled. This may cause driver accesses wrong memory address and the result is unpredicted. When clear the command ring, keep the last Link TRB intact, only clear its cycle bit. This should fix the "command ring full" issue reported by Oliver Neukum. This should be backported to stable kernels as old as 2.6.37, since the commit 89821320 "xhci: Fix command ring replay after resume" is merged. Signed-off-by: Andiry Xu Signed-off-by: Sarah Sharp Reported-by: Oliver Neukum --- drivers/usb/host/xhci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/usb/host/xhci.c') diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index aa94c0195791..a1afb7c39f7e 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -711,7 +711,10 @@ static void xhci_clear_command_ring(struct xhci_hcd *xhci) ring = xhci->cmd_ring; seg = ring->deq_seg; do { - memset(seg->trbs, 0, SEGMENT_SIZE); + memset(seg->trbs, 0, + sizeof(union xhci_trb) * (TRBS_PER_SEGMENT - 1)); + seg->trbs[TRBS_PER_SEGMENT - 1].link.control &= + cpu_to_le32(~TRB_CYCLE); seg = seg->next; } while (seg != ring->deq_seg); -- cgit v1.2.3 From 3b9783b277e66731891ab42eeaacebbdcdd6e629 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Thu, 22 Dec 2011 15:02:13 -0800 Subject: xhci: Remove warnings about MSI and MSI-X capabilities. xHCI host controllers may not be capable of MSI, but they should be able to be used in legacy PCI interrupt mode. Similarly, some xHCI host controllers will have MSI support but not MSI-X support. Lower the dmesg log level from an error to debug. The message won't appear unless CONFIG_USB_XHCI_HCD_DEBUGGING is turned on. If we need to find out whether the device can support MSI or MSI-X and it's not being enabled by the driver, it's easy to ask the user to run lspci. Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/usb/host/xhci.c') diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index a1afb7c39f7e..0968b856ef0a 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -200,14 +200,14 @@ static int xhci_setup_msi(struct xhci_hcd *xhci) ret = pci_enable_msi(pdev); if (ret) { - xhci_err(xhci, "failed to allocate MSI entry\n"); + xhci_dbg(xhci, "failed to allocate MSI entry\n"); return ret; } ret = request_irq(pdev->irq, (irq_handler_t)xhci_msi_irq, 0, "xhci_hcd", xhci_to_hcd(xhci)); if (ret) { - xhci_err(xhci, "disable MSI interrupt\n"); + xhci_dbg(xhci, "disable MSI interrupt\n"); pci_disable_msi(pdev); } @@ -270,7 +270,7 @@ static int xhci_setup_msix(struct xhci_hcd *xhci) ret = pci_enable_msix(pdev, xhci->msix_entries, xhci->msix_count); if (ret) { - xhci_err(xhci, "Failed to enable MSI-X\n"); + xhci_dbg(xhci, "Failed to enable MSI-X\n"); goto free_entries; } @@ -286,7 +286,7 @@ static int xhci_setup_msix(struct xhci_hcd *xhci) return ret; disable_msix: - xhci_err(xhci, "disable MSI-X interrupt\n"); + xhci_dbg(xhci, "disable MSI-X interrupt\n"); xhci_free_irq(xhci); pci_disable_msix(pdev); free_entries: -- cgit v1.2.3 From 79688acfb5e124fcf586add00af32a2a1a532c64 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Mon, 19 Dec 2011 16:56:04 -0800 Subject: xhci: Be less verbose during URB cancellation. With devices that can need up to 128 segments (with 64 TRBs per segment), we can't afford to print out the entire endpoint ring every time an URB is canceled. Instead, print the offset of the TRB, along with device pathname and endpoint number. Only print DMA addresses, since virtual addresses of internal structures are not useful. Change the cancellation code to be more clear about what steps of the cancellation it is in the process of doing (queueing the request, handling the stop endpoint command, turning the TDs into no-ops, or moving the dequeue pointers). Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci-ring.c | 15 ++++++--------- drivers/usb/host/xhci.c | 19 +++++++++++-------- 2 files changed, 17 insertions(+), 17 deletions(-) (limited to 'drivers/usb/host/xhci.c') diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 1fdb7e1cca3f..f34a268c6ccd 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -552,12 +552,9 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, cpu_to_le32(TRB_CYCLE); cur_trb->generic.field[3] |= cpu_to_le32( TRB_TYPE(TRB_TR_NOOP)); - xhci_dbg(xhci, "Cancel TRB %p (0x%llx dma) " - "in seg %p (0x%llx dma)\n", - cur_trb, - (unsigned long long)xhci_trb_virt_to_dma(cur_seg, cur_trb), - cur_seg, - (unsigned long long)cur_seg->dma); + xhci_dbg(xhci, "TRB to noop at offset 0x%llx\n", + (unsigned long long) + xhci_trb_virt_to_dma(cur_seg, cur_trb)); } if (cur_trb == cur_td->last_trb) break; @@ -697,9 +694,9 @@ static void handle_stopped_endpoint(struct xhci_hcd *xhci, */ list_for_each(entry, &ep->cancelled_td_list) { cur_td = list_entry(entry, struct xhci_td, cancelled_td_list); - xhci_dbg(xhci, "Cancelling TD starting at %p, 0x%llx (dma).\n", - cur_td->first_trb, - (unsigned long long)xhci_trb_virt_to_dma(cur_td->start_seg, cur_td->first_trb)); + xhci_dbg(xhci, "Removing canceled TD starting at 0x%llx (dma).\n", + (unsigned long long)xhci_trb_virt_to_dma( + cur_td->start_seg, cur_td->first_trb)); ep_ring = xhci_urb_to_transfer_ring(xhci, cur_td->urb); if (!ep_ring) { /* This shouldn't happen unless a driver is mucking diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 0968b856ef0a..f3d0b8d96440 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1333,9 +1333,6 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) goto done; } - xhci_dbg(xhci, "Cancel URB %p\n", urb); - xhci_dbg(xhci, "Event ring:\n"); - xhci_debug_ring(xhci, xhci->event_ring); ep_index = xhci_get_endpoint_index(&urb->ep->desc); ep = &xhci->devs[urb->dev->slot_id]->eps[ep_index]; ep_ring = xhci_urb_to_transfer_ring(xhci, urb); @@ -1344,12 +1341,18 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) goto done; } - xhci_dbg(xhci, "Endpoint ring:\n"); - xhci_debug_ring(xhci, ep_ring); - urb_priv = urb->hcpriv; - - for (i = urb_priv->td_cnt; i < urb_priv->length; i++) { + i = urb_priv->td_cnt; + if (i < urb_priv->length) + xhci_dbg(xhci, "Cancel URB %p, dev %s, ep 0x%x, " + "starting at offset 0x%llx\n", + urb, urb->dev->devpath, + urb->ep->desc.bEndpointAddress, + (unsigned long long) xhci_trb_virt_to_dma( + urb_priv->td[i]->start_seg, + urb_priv->td[i]->first_trb)); + + for (; i < urb_priv->length; i++) { td = urb_priv->td[i]; list_add_tail(&td->cancelled_td_list, &ep->cancelled_td_list); } -- cgit v1.2.3 From 71d85724bdd947a3b42a88d08af79f290a1a767b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 4 Jan 2012 23:29:18 +0100 Subject: xhci: Properly handle COMP_2ND_BW_ERR I encountered a result of COMP_2ND_BW_ERR while improving how the pwc webcam driver handles not having the full usb1 bandwidth available to itself. I created the following test setup, a NEC xhci controller with a single TT USB 2 hub plugged into it, with a usb keyboard and a pwc webcam plugged into the usb2 hub. This caused the following to show up in dmesg when trying to stream from the pwc camera at its highest alt setting: xhci_hcd 0000:01:00.0: ERROR: unexpected command completion code 0x23. usb 6-2.1: Not enough bandwidth for altsetting 9 And usb_set_interface returned -EINVAL, which caused my pwc code to not do the right thing as it expected -ENOSPC. This patch makes the xhci driver properly handle COMP_2ND_BW_ERR and makes usb_set_interface return -ENOSPC as expected. This should be backported to stable kernels as old as 2.6.32. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp Cc: stable@vger.kernel.org --- drivers/usb/host/xhci.c | 1 + drivers/usb/host/xhci.h | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/host/xhci.c') diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index f3d0b8d96440..6693de7ce03f 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1623,6 +1623,7 @@ static int xhci_configure_endpoint_result(struct xhci_hcd *xhci, /* FIXME: can we allocate more resources for the HC? */ break; case COMP_BW_ERR: + case COMP_2ND_BW_ERR: dev_warn(&udev->dev, "Not enough bandwidth " "for new device state.\n"); ret = -ENOSPC; diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index ecd2ad5d226d..fb99c8379142 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1033,7 +1033,6 @@ struct xhci_transfer_event { /* Invalid Stream ID Error */ #define COMP_STRID_ERR 34 /* Secondary Bandwidth Error - may be returned by a Configure Endpoint cmd */ -/* FIXME - check for this */ #define COMP_2ND_BW_ERR 35 /* Split Transaction Error */ #define COMP_SPLIT_ERR 36 -- cgit v1.2.3 From 18b7ede5f7ee2092aedcb578d3ac30bd5d4fc23c Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Mon, 2 Jan 2012 13:35:41 +0200 Subject: usb: ch9: fix up MaxStreams helper According to USB 3.0 Specification Table 9-22, if bmAttributes [4:0] are set to zero, it means "no streams supported", but the way this helper was defined on Linux, we will *always* have one stream which might cause several problems. For example on DWC3, we would tell the controller endpoint has streams enabled and yet start transfers with Stream ID set to 0, which would goof up the host side. While doing that, convert the macro to an inline function due to the different checks we now need. Signed-off-by: Felipe Balbi Signed-off-by: Sarah Sharp Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/dwc3/gadget.c | 3 +-- drivers/usb/host/xhci.c | 3 +-- include/linux/usb/ch9.h | 20 +++++++++++++++++++- 3 files changed, 21 insertions(+), 5 deletions(-) (limited to 'drivers/usb/host/xhci.c') diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 4c6bedad51fd..a696bde53222 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -297,8 +297,7 @@ static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep, params.param1 = DWC3_DEPCFG_XFER_COMPLETE_EN | DWC3_DEPCFG_XFER_NOT_READY_EN; - if (comp_desc && USB_SS_MAX_STREAMS(comp_desc->bmAttributes) - && usb_endpoint_xfer_bulk(desc)) { + if (usb_ss_max_streams(comp_desc) && usb_endpoint_xfer_bulk(desc)) { params.param1 |= DWC3_DEPCFG_STREAM_CAPABLE | DWC3_DEPCFG_STREAM_EVENT_EN; dep->stream_capable = true; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index f3d0b8d96440..dda84756c465 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -2799,8 +2799,7 @@ static int xhci_calculate_streams_and_bitmask(struct xhci_hcd *xhci, if (ret < 0) return ret; - max_streams = USB_SS_MAX_STREAMS( - eps[i]->ss_ep_comp.bmAttributes); + max_streams = usb_ss_max_streams(&eps[i]->ss_ep_comp); if (max_streams < (*num_streams - 1)) { xhci_dbg(xhci, "Ep 0x%x only supports %u stream IDs.\n", eps[i]->desc.bEndpointAddress, diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h index d5da6c68c250..61b29057b054 100644 --- a/include/linux/usb/ch9.h +++ b/include/linux/usb/ch9.h @@ -605,8 +605,26 @@ struct usb_ss_ep_comp_descriptor { } __attribute__ ((packed)); #define USB_DT_SS_EP_COMP_SIZE 6 + /* Bits 4:0 of bmAttributes if this is a bulk endpoint */ -#define USB_SS_MAX_STREAMS(p) (1 << ((p) & 0x1f)) +static inline int +usb_ss_max_streams(const struct usb_ss_ep_comp_descriptor *comp) +{ + int max_streams; + + if (!comp) + return 0; + + max_streams = comp->bmAttributes & 0x1f; + + if (!max_streams) + return 0; + + max_streams = 1 << max_streams; + + return max_streams; +} + /* Bits 1:0 of bmAttributes if this is an isoc endpoint */ #define USB_SS_MULT(p) (1 + ((p) & 0x3)) -- cgit v1.2.3 From 68d07f64b8a11a852d48d1b05b724c3e20c0d94b Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Mon, 13 Feb 2012 16:25:57 -0800 Subject: USB: Don't fail USB3 probe on missing legacy PCI IRQ. Intel has a PCI USB xhci host controller on a new platform. It doesn't have a line IRQ definition in BIOS. The Linux driver refuses to initialize this controller, but Windows works well because it only depends on MSI. Actually, Linux also can work for MSI. This patch avoids the line IRQ checking for USB3 HCDs in usb core PCI probe. It allows the xHCI driver to try to enable MSI or MSI-X first. It will fail the probe if MSI enabling failed and there's no legacy PCI IRQ. This patch should be backported to kernels as old as 2.6.32. Signed-off-by: Alex Shi Signed-off-by: Sarah Sharp Cc: stable@vger.kernel.org --- drivers/usb/core/hcd-pci.c | 5 ++++- drivers/usb/core/hcd.c | 6 ++++-- drivers/usb/host/xhci.c | 5 +++++ 3 files changed, 13 insertions(+), 3 deletions(-) (limited to 'drivers/usb/host/xhci.c') diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c index d136b8f4c8a7..81e2c0d9c17d 100644 --- a/drivers/usb/core/hcd-pci.c +++ b/drivers/usb/core/hcd-pci.c @@ -187,7 +187,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) return -ENODEV; dev->current_state = PCI_D0; - if (!dev->irq) { + /* The xHCI driver supports MSI and MSI-X, + * so don't fail if the BIOS doesn't provide a legacy IRQ. + */ + if (!dev->irq && (driver->flags & HCD_MASK) != HCD_USB3) { dev_err(&dev->dev, "Found HC with no IRQ. Check BIOS/PCI %s setup!\n", pci_name(dev)); diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index eb19cba34ac9..e1282328fc27 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2447,8 +2447,10 @@ int usb_add_hcd(struct usb_hcd *hcd, && device_can_wakeup(&hcd->self.root_hub->dev)) dev_dbg(hcd->self.controller, "supports USB remote wakeup\n"); - /* enable irqs just before we start the controller */ - if (usb_hcd_is_primary_hcd(hcd)) { + /* enable irqs just before we start the controller, + * if the BIOS provides legacy PCI irqs. + */ + if (usb_hcd_is_primary_hcd(hcd) && irqnum) { retval = usb_hcd_request_irqs(hcd, irqnum, irqflags); if (retval) goto err_request_irq; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6bbe3c3a7111..c939f5fdef9e 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -352,6 +352,11 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd) /* hcd->irq is -1, we have MSI */ return 0; + if (!pdev->irq) { + xhci_err(xhci, "No msi-x/msi found and no IRQ in BIOS\n"); + return -EINVAL; + } + /* fall back to legacy interrupt*/ ret = request_irq(pdev->irq, &usb_hcd_irq, IRQF_SHARED, hcd->irq_descr, hcd); -- cgit v1.2.3