On Wed, Jun 09, 2010 at 11:16:10AM +0200, Clemens Ladisch wrote:
> Sarah Sharp wrote:
> > On Tue, Jun 08, 2010 at 10:07:47AM +0200, Clemens Ladisch wrote:
> > > Sarah Sharp wrote:
> > > > If so, it might be useful to introduce a new HCD driver function to
> > > > enable MSI, which the USB core can call in usb_add_hcd(). The hosts
> > > > could have a flag to indicate whether they are designed to handle
> > > > MSI/MSI-X.
> > >
> > > Instead of a flag in struct hc_driver, we'd need another callback to be
> > > able to do the per-device whitelisting checks.
> >
> > Where do you propose the callback be used? In usb_add_hcd() or
> > somewhere else, like pci-quirks.c?
>
> A whitelist (which would require disabling MSI for all HCDs except
> those in the list) doesn't feel right for pci-quirks.c.
Ok. There are several other quirks checks in that file, so I thought it
would be ok there, but I have no real opinion on the placement of the
whitelist.
> Okay, the callback would look something like below. (Implementation
> of the callback for EHCI is in the second mail.)
>
> The interrupt number and flags for usb_add_hcd() are determined in
> usb_hcd_pci_probe(), so this is where the new code ends up.
>
> MSI is just a minor variation of a normal interrupt, but this design
> cannot handle MSI-X as it isn't known how many interrupts are acceptable,
> or what the dev parameters in the request_irq() calls should be.
Hmm, why not add new API in the same spot to ask the host how many MSI-X
interrupters it can support? If it says zero, then you can see if it
supports MSI with a call to is_pci_msi_allowed().
> It appears that common MSI code is easy, but that implementing MSI-X
> still requires replacing most of the generic hcd interrupt handling code
> (either with a callback or by replacing the interrupt later).
Perhaps the HCD could provide an MSI/MSI-X call back function? That
would fit with Dong's current patch.
There's also issues with mapping IRQ to internal data structures. The
xHCI host is going to need to be able to map the irq number for the
MSI-X vector into a specific event ring that needs to be serviced.
I'm not exactly sure how that's going to work, maybe Dong has an idea?
Sarah Sharp
> ---8<-------------------------------------------------------------->8---
> USB: allow PCI controllers to use message-signaled interrupts
>
> This patch adds the capability to use MSI to the PCI HCD framework.
>
> To avoid problems with broken chips that have never been tested with
> MSI, the HCD driver must explicitly consent before enabling MSI is
> attempted.
>
> NOT-yet-signed-off-by: Clemens Ladisch <clemens@xxxxxxxxxx>
> ---
> include/linux/usb/hcd.h | 3 +++
> drivers/usb/core/hcd-pci.c | 10 +++++++++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -190,6 +190,9 @@ struct hc_driver {
> #define HCD_USB3 0x0040 /* USB 3.0 */
> #define HCD_MASK 0x0070
>
> + /* determines if hcd-pci should try to enable MSI */
> + bool (*is_pci_msi_allowed) (struct usb_hcd *hcd);
> +
> /* called to init HCD and root hub */
> int (*reset) (struct usb_hcd *hcd);
> int (*start) (struct usb_hcd *hcd);
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -176,6 +176,7 @@ int usb_hcd_pci_probe(struct pci_dev *de
> {
> struct hc_driver *driver;
> struct usb_hcd *hcd;
> + unsigned long irqflags;
> int retval;
>
> if (usb_disabled())
> @@ -246,13 +247,19 @@ int usb_hcd_pci_probe(struct pci_dev *de
>
> pci_set_master(dev);
>
> - retval = usb_add_hcd(hcd, dev->irq, IRQF_DISABLED | IRQF_SHARED);
> + irqflags = IRQF_DISABLED | IRQF_SHARED;
> + if (driver->is_pci_msi_allowed && driver->is_pci_msi_allowed(hcd))
> + if (pci_enable_msi(dev) == 0)
> + irqflags &= ~IRQF_SHARED;
> +
> + retval = usb_add_hcd(hcd, dev->irq, irqflags);
> if (retval != 0)
> goto err4;
> set_hs_companion(dev, hcd);
> return retval;
>
> err4:
> + pci_disable_msi(dev);
> if (driver->flags & HCD_MEMORY) {
> iounmap(hcd->regs);
> err3:
> @@ -293,6 +300,7 @@ void usb_hcd_pci_remove(struct pci_dev *
> return;
>
> usb_remove_hcd(hcd);
> + pci_disable_msi(dev);
> if (hcd->driver->flags & HCD_MEMORY) {
> iounmap(hcd->regs);
> release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
--
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
