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.chen@xxxxxxx>
---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
