usb: xhci: remove '0' write to write-1-to-clear register
authorNiklas Neronin <niklas.neronin@linux.intel.com>
Thu, 15 May 2025 13:56:14 +0000 (16:56 +0300)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 21 May 2025 10:35:33 +0000 (12:35 +0200)
xHCI specification 1.2, section 5.5.2.1.
Interrupt Pending bit is RW1C (Write-1-to-clear), which means that
writing '0' to is has no effect and is removed.

The Interrupt Pending (IP) bit is cleared at the start of interrupt
handling; xhci_clear_interrupt_pending(). This could theoretically
cause a new interrupt to be issued before the xhci driver reaches
the interrupter disable functions.
To address this, the IP bit is read after Interrupt Enable is
disabled, and a debug message is issued if the IP bit is still set.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20250515135621.335595-18-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/host/xhci-hub.c
drivers/usb/host/xhci-ring.c
drivers/usb/host/xhci.c
drivers/usb/host/xhci.h

index 486347776cb29a2fef4cd12cb11127883480dd6d..92bb84f8132a971757edb250144c0b296d01a5cb 100644 (file)
@@ -1907,7 +1907,7 @@ int xhci_bus_resume(struct usb_hcd *hcd)
                         * prevent port event interrupts from interfering
                         * with usb2 port resume process
                         */
-                       xhci_disable_interrupter(xhci->interrupters[0]);
+                       xhci_disable_interrupter(xhci, xhci->interrupters[0]);
                        disabled_irq = true;
                }
        }
index 4a4410f7978f35fe24d349049959c3663688d979..1cae4ec6c7e9d47a88b72ce840c1e5f2316a9f9c 100644 (file)
@@ -3166,7 +3166,7 @@ void xhci_skip_sec_intr_events(struct xhci_hcd *xhci,
        dma_addr_t deq;
 
        /* disable irq, ack pending interrupt and ack all pending events */
-       xhci_disable_interrupter(ir);
+       xhci_disable_interrupter(xhci, ir);
 
        /* last acked event trb is in erdp reg  */
        erdp_reg = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
index 8cdb1a01a3edf5f67141c714694258d68c9276e3..6c4bbabc3a70e215223e040a37221a8225a86e90 100644 (file)
@@ -331,7 +331,6 @@ int xhci_enable_interrupter(struct xhci_interrupter *ir)
                return -EINVAL;
 
        iman = readl(&ir->ir_set->irq_pending);
-       iman &= ~IMAN_IP;
        iman |= IMAN_IE;
        writel(iman, &ir->ir_set->irq_pending);
 
@@ -340,7 +339,7 @@ int xhci_enable_interrupter(struct xhci_interrupter *ir)
        return 0;
 }
 
-int xhci_disable_interrupter(struct xhci_interrupter *ir)
+int xhci_disable_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
 {
        u32 iman;
 
@@ -348,11 +347,13 @@ int xhci_disable_interrupter(struct xhci_interrupter *ir)
                return -EINVAL;
 
        iman = readl(&ir->ir_set->irq_pending);
-       iman &= ~IMAN_IP;
        iman &= ~IMAN_IE;
        writel(iman, &ir->ir_set->irq_pending);
 
-       readl(&ir->ir_set->irq_pending);
+       iman = readl(&ir->ir_set->irq_pending);
+       if (iman & IMAN_IP)
+               xhci_dbg(xhci, "%s: Interrupt pending\n", __func__);
+
        return 0;
 }
 
@@ -754,7 +755,7 @@ void xhci_stop(struct usb_hcd *hcd)
                        "// Disabling event ring interrupts");
        temp = readl(&xhci->op_regs->status);
        writel((temp & ~0x1fff) | STS_EINT, &xhci->op_regs->status);
-       xhci_disable_interrupter(ir);
+       xhci_disable_interrupter(xhci, ir);
 
        xhci_dbg_trace(xhci, trace_xhci_dbg_init, "cleaning up memory");
        xhci_mem_cleanup(xhci);
@@ -1189,7 +1190,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
                xhci_dbg(xhci, "// Disabling event ring interrupts\n");
                temp = readl(&xhci->op_regs->status);
                writel((temp & ~0x1fff) | STS_EINT, &xhci->op_regs->status);
-               xhci_disable_interrupter(xhci->interrupters[0]);
+               xhci_disable_interrupter(xhci, xhci->interrupters[0]);
 
                xhci_dbg(xhci, "cleaning up memory\n");
                xhci_mem_cleanup(xhci);
index 28c4ad7534c189278a48ed6fa3eaff6134f368cc..fc6b97add7fad0fbc25369074d799a1854182d3f 100644 (file)
@@ -1900,7 +1900,7 @@ int xhci_alloc_tt_info(struct xhci_hcd *xhci,
 int xhci_set_interrupter_moderation(struct xhci_interrupter *ir,
                                    u32 imod_interval);
 int xhci_enable_interrupter(struct xhci_interrupter *ir);
-int xhci_disable_interrupter(struct xhci_interrupter *ir);
+int xhci_disable_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir);
 
 /* xHCI ring, segment, TRB, and TD functions */
 dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg, union xhci_trb *trb);