On 28.04.2017 04:04, Peter Chen wrote:
On Thu, Apr 27, 2017 at 03:43:28PM +0300, Mathias Nyman wrote:
Hi

On 27.04.2017 12:49, Peter Chen wrote:
On Mon, Apr 17, 2017 at 04:20:43PM +0800, Peter Chen wrote:
According to xHCI spec Figure 30: Interrupt Throttle Flow Diagram

        If PCI Message Signaled Interrupts (MSI or MSI-X) are enabled,
                then the assertion of the Interrupt Pending (IP) flag in Figure 
30
                generates a PCI Dword write. The IP flag is automatically 
cleared
                by the completion of the PCI write.

the MSI enabled HCs don't need to clear interrupt pending bit, but
hcd->irq = 0 doesn't equal to MSI enabled HCD. At some Dual-role
controller software designs, it sets hcd->irq as 0 to avoid HCD
requesting interrupt, and they want to decide when to call usb_hcd_irq
by software.

ping...


This start to look like xhci msi(x) needs a bigger cleanup.
The msi parts should probably be moved to xhci-pci.c, and let
xhci_try_enable_msi() be pci specific.

xhci platform drivers should not need to worry about msi.


Signed-off-by: Peter Chen <peter.c...@nxp.com>
---
Mathias, I am not very sure if the comments I delete is complete
useless, plese help to check.

  drivers/usb/host/xhci-ring.c | 5 +----
  drivers/usb/host/xhci.c      | 5 +++--
  include/linux/usb/hcd.h      | 1 +
  3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d9936c7..f5adbb3 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2642,12 +2642,9 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
         */
        status |= STS_EINT;
        writel(status, &xhci->op_regs->status);
-       /* FIXME when MSI-X is supported and there are multiple vectors */
-       /* Clear the MSI-X event interrupt status */

-       if (hcd->irq) {
+       if (!hcd->msi_enabled) {
                u32 irq_pending;
-               /* Acknowledge the PCI interrupt */
                irq_pending = readl(&xhci->ir_set->irq_pending);
                irq_pending |= IMAN_IP;
                writel(irq_pending, &xhci->ir_set->irq_pending);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 1828695..dc81041 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -401,9 +401,10 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
                /* fall back to msi*/
                ret = xhci_setup_msi(xhci);

-       if (!ret)
-               /* hcd->irq is 0, we have MSI */
+       if (!ret) {
+               hcd->msi_enabled = 1;
                return 0;
+       }

        if (!pdev->irq) {
                xhci_err(xhci, "No msi-x/msi found and no IRQ in BIOS\n");
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index a469999..50398b6 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -148,6 +148,7 @@ struct usb_hcd {
        unsigned                rh_registered:1;/* is root hub registered? */
        unsigned                rh_pollable:1;  /* may we poll the root hub? */
        unsigned                msix_enabled:1; /* driver has MSI-X enabled? */
+       unsigned                msi_enabled:1;  /* driver has MSI enabled? */
        unsigned                remove_phy:1;   /* auto-remove USB phy */

        /* The next flag is a stopgap, to be removed when all the HCDs
--

so after this
hcd->irq > 0      means a "legacy" interrupt is used.
hcd->irq == 0        means anything else than "legacy" is used, might be 
nothing as well
msi_enabled     means msi or msix is in use
msix_enabled    means msix is in use

This probably works, but moving msi to be xhci-pci specific feels like a better 
way
to fix this, but might end up as a more work


Yes, but my platform is not PCI based, if it is a big patch, I am may
not suitable for it, will you do it? This issue blocks my USB3 driver
working.


Ok, lets have this one to get things working. I'll send it forward after rc1

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to