Alan,
Please take a look at patch 1 if you have time.
Hi Andiry,
It looks like these patches may have missed Greg's potentially-last pull
request for 2.6.37. I'll send the first (and possibly the 6th patch) to
Greg just in case Linus decides to do another rc.
This patchset is a mixture of urgent bug fixes, new features, and
cleanup. In the future, can you break those up into separate patchsets?
Here's my thoughts on getting these pushed:
1. synchronize irq - fixes some very scary warnings/errors on module
unload for Matthew and Rafael, so they should pushed sooner for 2.6.37.
On the other hand, I'm a bit concerned about this bug fix because
Alan hasn't confirmed if all PCI host controllers set hcd->irq ==
pci_dev->irq. I'm afraid a late fix will break some obscure PCI
host controller in a subtle way that will be hard to detect.
2. AMD hudson quirk - too much code for this late in the release cycle.
It will have to wait until 2.6.38. I will try to get it reviewed
before the merge window opens, because I want to make sure the PCI
space map/unmap is correct. But it would have been better to see
this patch sooner, as it's going to be a stretch if it does get into
2.6.38.
3. fix queue_trb in isoc transfers - does this fix a bug? I think isoc
transfers were working pretty soundly on 2.6.37. If it's a bug-fix by
inspection, then it could probably wait until 2.6.38.
4. remove redundant parameter - definitely should wait for 2.6.38
5. Fix cycle bit. I would be inclined to put this bug off until
2.6.37, except that I'm pretty sure it will fix some issues I've
been seeing when the NEC host controller stops responding to the
UAS device queries. But it's not an urgent crashing-machines bug
fix at this point, so I think I'll put off pushing it until 2.6.38.
6/7 printing optimizations, should be left off until 2.6.38.
I'll queue up patch 1 (along with the other resume fix I was holding
onto) and send them off to Greg, unless Alan has some last minute
objection to it.
Sarah Sharp
On Mon, Dec 20, 2010 at 06:29:23PM +0800, Andiry Xu wrote:
> >From 8668139f7857aad09c5308d2e5e47b3369c83b38 Mon Sep 17 00:00:00 2001
> From: Andiry Xu <andiry.xu@xxxxxxx>
> Date: Mon, 20 Dec 2010 15:17:14 +0800
> Subject: [PATCH 1/7] xHCI: synchronize irq in xhci_suspend()
>
> Synchronize the interrupts instead of free them in xhci_suspend(). This will
> prevent a double free when the host is suspended and then the card removed.
>
> Set hcd->irq to pdev->irq when using MSI and INTx, and add the equal check in
> suspend_common(), so MSI-X synchronization will be handled by xhci_suspend(),
> and MSI/INTx will be synchronized in suspend_common().
>
> Reported-by: Matthew Garrett <mjg@xxxxxxxxxx>
> Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
> ---
> drivers/usb/core/hcd-pci.c | 6 +++++-
> drivers/usb/host/xhci.c | 40 +++++++++++-----------------------------
> 2 files changed, 16 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 3799573..c793443 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -406,7 +406,11 @@ static int suspend_common(struct device *dev, bool do_wakeup)
> return retval;
> }
>
> - synchronize_irq(pci_dev->irq);
> + /* xHCI uses MSI-X by default. Suppose other HCDs will have hcd->irq
> + * equal to pci_dev->irq.
> + */
> + if (hcd->irq == pci_dev->irq)
> + synchronize_irq(pci_dev->irq);
>
> /* Downstream ports from this root hub should already be quiesced, so
> * there will be no DMA activity. Now we can shut down the upstream
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 45e4a31..2947d94 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -202,6 +202,7 @@ static void xhci_free_irq(struct xhci_hcd *xhci)
> static int xhci_setup_msi(struct xhci_hcd *xhci)
> {
> int ret;
> + struct usb_hcd *hcd = xhci_to_hcd(xhci);
> struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
>
> ret = pci_enable_msi(pdev);
> @@ -217,6 +218,7 @@ static int xhci_setup_msi(struct xhci_hcd *xhci)
> pci_disable_msi(pdev);
> }
>
> + hcd->irq = pdev->irq;
> return ret;
> }
>
> @@ -647,6 +649,7 @@ int xhci_suspend(struct xhci_hcd *xhci)
> int rc = 0;
> struct usb_hcd *hcd = xhci_to_hcd(xhci);
> u32 command;
> + int i;
>
> spin_lock_irq(&xhci->lock);
> clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> @@ -677,10 +680,15 @@ int xhci_suspend(struct xhci_hcd *xhci)
> spin_unlock_irq(&xhci->lock);
> return -ETIMEDOUT;
> }
> - /* step 5: remove core well power */
> - xhci_cleanup_msix(xhci);
> spin_unlock_irq(&xhci->lock);
>
> + /* step 5: remove core well power */
> + /* synchronize irq when using MSI-X */
> + if (xhci->msix_entries) {
> + for (i = 0; i < xhci->msix_count; i++)
> + synchronize_irq(xhci->msix_entries[i].vector);
> + }
> +
> return rc;
> }
>
> @@ -694,7 +702,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
> {
> u32 command, temp = 0;
> struct usb_hcd *hcd = xhci_to_hcd(xhci);
> - struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
> int old_state, retval;
>
> old_state = hcd->state;
> @@ -729,9 +736,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
> xhci_dbg(xhci, "Stop HCD\n");
> xhci_halt(xhci);
> xhci_reset(xhci);
> - if (hibernated)
> - xhci_cleanup_msix(xhci);
> spin_unlock_irq(&xhci->lock);
> + xhci_cleanup_msix(xhci);
>
> #ifdef CONFIG_USB_XHCI_HCD_DEBUGGING
> /* Tell the event ring poll function not to reschedule */
> @@ -765,30 +771,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
> return retval;
> }
>
> - spin_unlock_irq(&xhci->lock);
> - /* Re-setup MSI-X */
> - if (hcd->irq)
> - free_irq(hcd->irq, hcd);
> - hcd->irq = -1;
> -
> - retval = xhci_setup_msix(xhci);
> - if (retval)
> - /* fall back to msi*/
> - retval = xhci_setup_msi(xhci);
> -
> - if (retval) {
> - /* fall back to legacy interrupt*/
> - retval = request_irq(pdev->irq, &usb_hcd_irq, IRQF_SHARED,
> - hcd->irq_descr, hcd);
> - if (retval) {
> - xhci_err(xhci, "request interrupt %d failed\n",
> - pdev->irq);
> - return retval;
> - }
> - hcd->irq = pdev->irq;
> - }
> -
> - spin_lock_irq(&xhci->lock);
> /* step 4: set Run/Stop bit */
> command = xhci_readl(xhci, &xhci->op_regs->command);
> command |= CMD_RUN;
> --
> 1.7.1
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
