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
-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
