Hi Dong,
The patch looks good overall. I'll let you know how my testing goes.
I sent an email asking if any other hosts support MSI, because I think
some of the code and error handling should be in the USB core in that
case. If it turns out xHCI is the only host that supports MSI and the
testing goes well, I'll accept this patch.
Thanks for submitting this. :)
Sarah Sharp
On Fri, Jun 04, 2010 at 10:24:44AM -0700, Dong Nguyen wrote:
> >From 2bee389a863f874ef4e5f71630624cff4a37ce0c Mon Sep 17 00:00:00 2001
> From: Dong Nguyen <Dong.Nguyen@xxxxxxx>
> Date: Tue, 25 May 2010 18:24:07 -0700
> Subject: [PATCH] xHCI: Supporting msi/msi-x
>
> Enable MSI/MSI-X supporting in xhci driver.
>
> Provide the mechanism to fall back using MSI and Legacy IRQs
> if MSI-X IRQs register failed.
>
> Signed-off-by: Dong Nguyen <Dong.Nguyen@xxxxxxx>
> ---
> drivers/usb/core/hcd.c | 1 +
> drivers/usb/host/xhci.c | 167 +++++++++++++++++++++++++++++++++++-----------
> drivers/usb/host/xhci.h | 2 +-
> 3 files changed, 129 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 51d95d3..ad3a7a0 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2042,6 +2042,7 @@ irqreturn_t usb_hcd_irq (int irq, void *__hcd)
> local_irq_restore(flags);
> return rc;
> }
> +EXPORT_SYMBOL_GPL(usb_hcd_irq);
>
> /*-------------------------------------------------------------------------*/
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 40e0a0c..8e08643 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -20,6 +20,7 @@
> * Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> */
>
> +#include <linux/pci.h>
> #include <linux/irq.h>
> #include <linux/log2.h>
> #include <linux/module.h>
> @@ -133,22 +134,96 @@ int xhci_reset(struct xhci_hcd *xhci)
> return handshake(xhci, &xhci->op_regs->command, CMD_RESET, 0, 250 * 1000);
> }
>
> +static irqreturn_t xhci_msi_irq(int irq, struct usb_hcd *hcd)
> +{
> + irqreturn_t ret;
>
> -#if 0
> -/* Set up MSI-X table for entry 0 (may claim other entries later) */
> -static int xhci_setup_msix(struct xhci_hcd *xhci)
> + set_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
> +
> + ret = xhci_irq(hcd);
> +
> + return ret;
> +}
> +
> +/*
> + * Free IRQs
> + * free all IRQs request
> + */
> +static void xhci_free_irq(struct xhci_hcd *xhci)
> +{
> + int i;
> + struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> +
> + /* return if using legacy interrupt */
> + if (xhci_to_hcd(xhci)->irq >= 0)
> + return;
> +
> + if (xhci->msix_entries) {
> + for (i = 0; i < xhci->msix_count; i++)
> + if (xhci->msix_entries[i].vector)
> + free_irq(xhci->msix_entries[i].vector,
> + xhci_to_hcd(xhci));
> + } else if (pdev->irq >= 0)
> + free_irq(pdev->irq, xhci_to_hcd(xhci));
> +
> + return;
> +}
> +
> +/*
> + * Set up MSI
> + */
> +static int xhci_setup_msi(struct xhci_hcd *xhci)
> {
> int ret;
> + struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> +
> + ret = pci_enable_msi(pdev);
> + if (ret) {
> + xhci_err(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");
> + pci_disable_msi(pdev);
> + }
> +
> + return ret;
> +}
> +
> +
> +/*
> + * Set up MSI-X
> + */
> +static int xhci_setup_msix(struct xhci_hcd *xhci)
> +{
> + int i, ret = 0;
> struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
>
> - xhci->msix_count = 0;
> - /* XXX: did I do this right? ixgbe does kcalloc for more than one */
> - xhci->msix_entries = kmalloc(sizeof(struct msix_entry), GFP_KERNEL);
> + /*
> + * calculate number of msi-x vectors supported.
> + * - HCS_MAX_INTRS: the max number of interrupts the host can handle,
> + * with max number of interrupters based on the xhci HCSPARAMS1.
> + * - num_online_cpus: maximum msi-x vectors per CPUs core.
> + * Add additional 1 vector to ensure always available interrupt.
> + */
> + xhci->msix_count = min(num_online_cpus() + 1,
> + HCS_MAX_INTRS(xhci->hcs_params1));
> +
> + xhci->msix_entries =
> + kmalloc((sizeof(struct msix_entry))*xhci->msix_count,
> + GFP_KERNEL);
> if (!xhci->msix_entries) {
> xhci_err(xhci, "Failed to allocate MSI-X entries\n");
> return -ENOMEM;
> }
> - xhci->msix_entries[0].entry = 0;
> +
> + for (i = 0; i < xhci->msix_count; i++) {
> + xhci->msix_entries[i].entry = i;
> + xhci->msix_entries[i].vector = 0;
> + }
>
> ret = pci_enable_msix(pdev, xhci->msix_entries, xhci->msix_count);
> if (ret) {
> @@ -156,20 +231,19 @@ static int xhci_setup_msix(struct xhci_hcd *xhci)
> goto free_entries;
> }
>
> - /*
> - * Pass the xhci pointer value as the request_irq "cookie".
> - * If more irqs are added, this will need to be unique for each one.
> - */
> - ret = request_irq(xhci->msix_entries[0].vector, &xhci_irq, 0,
> - "xHCI", xhci_to_hcd(xhci));
> - if (ret) {
> - xhci_err(xhci, "Failed to allocate MSI-X interrupt\n");
> - goto disable_msix;
> + for (i = 0; i < xhci->msix_count; i++) {
> + ret = request_irq(xhci->msix_entries[i].vector,
> + (irq_handler_t)xhci_msi_irq,
> + 0, "xhci_hcd", xhci_to_hcd(xhci));
> + if (ret)
> + goto disable_msix;
> }
> - xhci_dbg(xhci, "Finished setting up MSI-X\n");
> - return 0;
> +
> + return ret;
>
> disable_msix:
> + xhci_err(xhci, "disable MSI-X interrupt\n");
> + xhci_free_irq(xhci);
> pci_disable_msix(pdev);
> free_entries:
> kfree(xhci->msix_entries);
> @@ -177,21 +251,23 @@ free_entries:
> return ret;
> }
>
> -/* XXX: code duplication; can xhci_setup_msix call this? */
> /* Free any IRQs and disable MSI-X */
> static void xhci_cleanup_msix(struct xhci_hcd *xhci)
> {
> struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> - if (!xhci->msix_entries)
> - return;
>
> - free_irq(xhci->msix_entries[0].vector, xhci);
> - pci_disable_msix(pdev);
> - kfree(xhci->msix_entries);
> - xhci->msix_entries = NULL;
> - xhci_dbg(xhci, "Finished cleaning up MSI-X\n");
> + xhci_free_irq(xhci);
> +
> + if (xhci->msix_entries) {
> + pci_disable_msix(pdev);
> + kfree(xhci->msix_entries);
> + xhci->msix_entries = NULL;
> + } else {
> + pci_disable_msi(pdev);
> + }
> +
> + return;
> }
> -#endif
>
> /*
> * Initialize memory for HCD and xHC (one-time init).
> @@ -385,21 +461,37 @@ int xhci_run(struct usb_hcd *hcd)
> {
> u32 temp;
> u64 temp_64;
> + u32 ret;
> struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + struct pci_dev *pdev = to_pci_dev(xhci_to_hcd(xhci)->self.controller);
> void (*doorbell)(struct xhci_hcd *) = NULL;
>
> hcd->uses_new_polling = 1;
> hcd->poll_rh = 0;
>
> xhci_dbg(xhci, "xhci_run\n");
> -#if 0 /* FIXME: MSI not setup yet */
> - /* Do this at the very last minute */
> + /* unregister the legacy interrupt */
> + if (hcd->irq)
> + free_irq(hcd->irq, hcd);
> + hcd->irq = -1;
> +
> ret = xhci_setup_msix(xhci);
> - if (!ret)
> - return ret;
> + if (ret)
> + /* fall back to msi*/
> + ret = xhci_setup_msi(xhci);
> +
> + if (ret) {
> + /* fall back to legacy interrupt*/
> + ret = request_irq(pdev->irq, &usb_hcd_irq, IRQF_SHARED,
> + hcd->irq_descr, hcd);
> + if (ret) {
> + xhci_err(xhci, "request interrupt %d failed\n",
> + pdev->irq);
> + return ret;
> + }
> + hcd->irq = pdev->irq;
> + }
>
> - return -ENOSYS;
> -#endif
> #ifdef CONFIG_USB_XHCI_HCD_DEBUGGING
> init_timer(&xhci->event_ring_timer);
> xhci->event_ring_timer.data = (unsigned long) xhci;
> @@ -481,11 +573,9 @@ void xhci_stop(struct usb_hcd *hcd)
> spin_lock_irq(&xhci->lock);
> xhci_halt(xhci);
> xhci_reset(xhci);
> + xhci_cleanup_msix(xhci);
> spin_unlock_irq(&xhci->lock);
>
> -#if 0 /* No MSI yet */
> - xhci_cleanup_msix(xhci);
> -#endif
> #ifdef CONFIG_USB_XHCI_HCD_DEBUGGING
> /* Tell the event ring poll function not to reschedule */
> xhci->zombie = 1;
> @@ -519,11 +609,8 @@ void xhci_shutdown(struct usb_hcd *hcd)
>
> spin_lock_irq(&xhci->lock);
> xhci_halt(xhci);
> - spin_unlock_irq(&xhci->lock);
> -
> -#if 0
> xhci_cleanup_msix(xhci);
> -#endif
> + spin_unlock_irq(&xhci->lock);
>
> xhci_dbg(xhci, "xhci_shutdown completed - status = %x\n",
> xhci_readl(xhci, &xhci->op_regs->status));
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index fb3440b..a32f644 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1121,7 +1121,7 @@ struct xhci_hcd {
> int page_size;
> /* Valid values are 12 to 20, inclusive */
> int page_shift;
> - /* only one MSI vector for now, but might need more later */
> + /* msi-x vectors */
> int msix_count;
> struct msix_entry *msix_entries;
> /* data structures */
> --
> 1.6.3.3
>
>
>
>
--
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
