[PATCH 0/9] usb: xhci: enhancements to Interrupt Register Set code

[PATCH 0/9] usb: xhci: enhancements to Interrupt Register Set code

[Niklas Neronin]

These changes were inspired by reading the xHCI specification and
searching for the corresponding code within the xhci driver.

Key improvements include:
 * Adjusting the names of Interrupt Register Set values to better match
   their counterparts in the xHCI specification, facilitating easier
   cross-referencing.
 * Simplify bit masks to ensure they only cover their relevant bits,
   reducing complexity and potential issues.
 * Reword and add comments.

Niklas Neronin (9):
  usb: xhci: set requested IMODI to the closest supported value
  usb: xhci: improve Interrupt Management register macros
  usb: xhci: guarantee that IMAN register is flushed
  usb: xhci: remove '0' write to write-1-to-clear register
  usb: xhci: rework Event Ring Segment Table Size mask
  usb: xhci: rework Event Ring Segment Table Address mask
  usb: xhci: cleanup IMOD register comments
  usb: xhci: rename 'irq_pending' to 'iman'
  usb: xhci: rename 'irq_control' to 'imod'

 drivers/usb/host/xhci-mem.c  | 13 +++----
 drivers/usb/host/xhci-ring.c | 11 +++---
 drivers/usb/host/xhci.c      | 43 +++++++++++++---------
 drivers/usb/host/xhci.h      | 70 ++++++++++++++++++------------------
 4 files changed, 73 insertions(+), 64 deletions(-)

-- 
2.47.2

[PATCH 1/9] usb: xhci: set requested IMODI to the closest supported value

[Niklas Neronin]

The function configures the Interrupt Moderation Interval (IMODI) via bits
15:0 in the Interrupt Moderation Register. The IMODI value is specified in
increments of 250 nanoseconds. For instance, an IMODI register value of 16
corresponds to 4000 nanoseconds, resulting in an interrupt every ~1ms.

Currently, the function fails when a requested IMODI value is too large,
only logging a warning message for secondary interrupters. Prevent this by
automatically adjusting the IMODI value to the nearest supported value.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 5 +----
 drivers/usb/host/xhci.c     | 7 +++++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d698095fc88d..ebbf5f039902 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2359,10 +2359,7 @@ xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
 		return NULL;
 	}
 
-	err = xhci_set_interrupter_moderation(ir, imod_interval);
-	if (err)
-		xhci_warn(xhci, "Failed to set interrupter %d moderation to %uns\n",
-			  i, imod_interval);
+	xhci_set_interrupter_moderation(ir, imod_interval);
 
 	xhci_dbg(xhci, "Add secondary interrupter %d, max interrupters %d\n",
 		 i, xhci->max_interrupters);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 0452b8d65832..7a8c545b78b7 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -354,12 +354,15 @@ int xhci_set_interrupter_moderation(struct xhci_interrupter *ir,
 {
 	u32 imod;
 
-	if (!ir || !ir->ir_set || imod_interval > U16_MAX * 250)
+	if (!ir || !ir->ir_set)
 		return -EINVAL;
 
+	/* IMODI value in IMOD register is in 250ns increments */
+	imod_interval = umin(imod_interval / 250, ER_IRQ_INTERVAL_MASK);
+
 	imod = readl(&ir->ir_set->irq_control);
 	imod &= ~ER_IRQ_INTERVAL_MASK;
-	imod |= (imod_interval / 250) & ER_IRQ_INTERVAL_MASK;
+	imod |= imod_interval;
 	writel(imod, &ir->ir_set->irq_control);
 
 	return 0;
-- 
2.47.2

[PATCH 2/9] usb: xhci: improve Interrupt Management register macros

[Niklas Neronin]

The Interrupt Management register (IMAN), contains three fields:
 - Bit 0:	Interrupt Pending (IP)
 - Bit 1:	Interrupt Enable (IE)
 - Bits 31:2:	RsvdP (Reserved and Preserved)

Currently, there are multiple macros for both the IP and IE fields.
Consolidates them into single mask macros for better clarity and
maintainability.

Comment "THIS IS BUGGY - FIXME - IP IS WRITE 1 TO CLEAR" refers to the
fact that both macros 'ER_IRQ_ENABLE' and 'ER_IRQ_DISABLE' clear the IP bit
by writing '0' before modifying the IE bit. However, the IP bit is actually
cleared by writing '1'. To prevent any regression, this behavior has not
been altered. Instead, when the IE bit is modified, the IP macro is used
explicitly to highlight this "quirk".

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci.c |  8 ++++++--
 drivers/usb/host/xhci.h | 14 ++++----------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 7a8c545b78b7..5cf9908188cf 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -330,7 +330,9 @@ static int xhci_enable_interrupter(struct xhci_interrupter *ir)
 		return -EINVAL;
 
 	iman = readl(&ir->ir_set->irq_pending);
-	writel(ER_IRQ_ENABLE(iman), &ir->ir_set->irq_pending);
+	iman &= ~IMAN_IP;
+	iman |= IMAN_IE;
+	writel(iman, &ir->ir_set->irq_pending);
 
 	return 0;
 }
@@ -343,7 +345,9 @@ static int xhci_disable_interrupter(struct xhci_interrupter *ir)
 		return -EINVAL;
 
 	iman = readl(&ir->ir_set->irq_pending);
-	writel(ER_IRQ_DISABLE(iman), &ir->ir_set->irq_pending);
+	iman &= ~IMAN_IP;
+	iman &= ~IMAN_IE;
+	writel(iman, &ir->ir_set->irq_pending);
 
 	return 0;
 }
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 37860f1e3aba..20e5b6ebbab1 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -152,10 +152,6 @@ struct xhci_op_regs {
 #define XHCI_RESET_LONG_USEC		(10 * 1000 * 1000)
 #define XHCI_RESET_SHORT_USEC		(250 * 1000)
 
-/* IMAN - Interrupt Management Register */
-#define IMAN_IE		(1 << 1)
-#define IMAN_IP		(1 << 0)
-
 /* USBSTS - USB status - status bitmasks */
 /* HC not running - set to 1 when run/stop bit is cleared. */
 #define STS_HALT	XHCI_STS_HALT
@@ -241,12 +237,10 @@ struct xhci_intr_reg {
 };
 
 /* irq_pending bitmasks */
-#define	ER_IRQ_PENDING(p)	((p) & 0x1)
-/* bits 2:31 need to be preserved */
-/* THIS IS BUGGY - FIXME - IP IS WRITE 1 TO CLEAR */
-#define	ER_IRQ_CLEAR(p)		((p) & 0xfffffffe)
-#define	ER_IRQ_ENABLE(p)	((ER_IRQ_CLEAR(p)) | 0x2)
-#define	ER_IRQ_DISABLE(p)	((ER_IRQ_CLEAR(p)) & ~(0x2))
+/* bit 0 - Interrupt Pending (IP), whether there is an interrupt pending. Write-1-to-clear. */
+#define	IMAN_IP			(1 << 0)
+/* bit 1 - Interrupt Enable (IE), whether the interrupter is capable of generating an interrupt */
+#define	IMAN_IE			(1 << 1)
 
 /* irq_control bitmasks */
 /* Minimum interval between interrupts (in 250ns intervals).  The interval
-- 
2.47.2

[PATCH 3/9] usb: xhci: guarantee that IMAN register is flushed

[Niklas Neronin]

Add read call to guarantee that the write to the IMAN register has
been flushed.

xHCI specification 1.2, section 5.5.2.1, Note:
"Most systems have write buffers that minimize overhead, but this may
 require a read operation to guarantee that the write has been flushed
 from the posted buffer."

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 3 +++
 drivers/usb/host/xhci.c      | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5d64c297721c..412eb90c29fb 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3092,6 +3092,9 @@ static void xhci_clear_interrupt_pending(struct xhci_interrupter *ir)
 		irq_pending = readl(&ir->ir_set->irq_pending);
 		irq_pending |= IMAN_IP;
 		writel(irq_pending, &ir->ir_set->irq_pending);
+
+		/* Read operation to guarantee the write has been flushed from posted buffers */
+		readl(&ir->ir_set->irq_pending);
 	}
 }
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5cf9908188cf..93daaac102f7 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -334,6 +334,8 @@ static int xhci_enable_interrupter(struct xhci_interrupter *ir)
 	iman |= IMAN_IE;
 	writel(iman, &ir->ir_set->irq_pending);
 
+	/* Read operation to guarantee the write has been flushed from posted buffers */
+	readl(&ir->ir_set->irq_pending);
 	return 0;
 }
 
@@ -349,6 +351,7 @@ static int xhci_disable_interrupter(struct xhci_interrupter *ir)
 	iman &= ~IMAN_IE;
 	writel(iman, &ir->ir_set->irq_pending);
 
+	readl(&ir->ir_set->irq_pending);
 	return 0;
 }
 
-- 
2.47.2

[PATCH 4/9] usb: xhci: remove '0' write to write-1-to-clear register

[Niklas Neronin]

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>
---
 drivers/usb/host/xhci.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 93daaac102f7..a41337360535 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -330,7 +330,6 @@ static 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);
 
@@ -339,7 +338,7 @@ static int xhci_enable_interrupter(struct xhci_interrupter *ir)
 	return 0;
 }
 
-static int xhci_disable_interrupter(struct xhci_interrupter *ir)
+static int xhci_disable_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
 {
 	u32 iman;
 
@@ -347,11 +346,13 @@ static 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;
 }
 
@@ -650,7 +651,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);
@@ -1102,7 +1103,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);
-- 
2.47.2

[PATCH 5/9] usb: xhci: rework Event Ring Segment Table Size mask

[Niklas Neronin]

Event Ring Segment Table Size Register contain two fields:
 - Bits 15:0:	Event Ring Segment Table Size
 - Bits 31:16:	RsvdZ (Reserved and Zero)

The current mask 'ERST_SIZE_MASK' refers to the RsvdZ bits (31:16).
Change the mask to refer to bits 15:0, which are the Event Ring Segment
Table Size bits.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 4 ++--
 drivers/usb/host/xhci.h     | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index ebbf5f039902..b6a2b0c01eb2 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1802,7 +1802,7 @@ xhci_remove_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir)
 	 */
 	if (ir->ir_set) {
 		tmp = readl(&ir->ir_set->erst_size);
-		tmp &= ERST_SIZE_MASK;
+		tmp &= ~ERST_SIZE_MASK;
 		writel(tmp, &ir->ir_set->erst_size);
 
 		xhci_write_64(xhci, ERST_EHB, &ir->ir_set->erst_dequeue);
@@ -2306,7 +2306,7 @@ xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
 
 	/* set ERST count with the number of entries in the segment table */
 	erst_size = readl(&ir->ir_set->erst_size);
-	erst_size &= ERST_SIZE_MASK;
+	erst_size &= ~ERST_SIZE_MASK;
 	erst_size |= ir->event_ring->num_segs;
 	writel(erst_size, &ir->ir_set->erst_size);
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 20e5b6ebbab1..5e5b71958745 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -252,8 +252,8 @@ struct xhci_intr_reg {
 #define ER_IRQ_COUNTER_MASK	(0xffff << 16)
 
 /* erst_size bitmasks */
-/* Preserve bits 16:31 of erst_size */
-#define	ERST_SIZE_MASK		(0xffff << 16)
+/* bits 15:0 - Event Ring Segment Table Size, number of ERST entries */
+#define	ERST_SIZE_MASK		(0xffff)
 
 /* erst_base bitmasks */
 #define ERST_BASE_RSVDP		(GENMASK_ULL(5, 0))
-- 
2.47.2

[PATCH 6/9] usb: xhci: rework Event Ring Segment Table Address mask

[Niklas Neronin]

Event Ring Segment Table Base Address Register contain two fields:
 - Bits 5:0:	RsvdP (Reserved and Preserved)
 - Bits 63:6:	Event Ring Segment Table Base Address

Currently, an inverted RsvdP mask (ERST_BASE_RSVDP) is used to extract
bits 63:6. Replaces the inverted mask with a non-inverted mask,
'ERST_BASE_ADDRESS_MASK', which makes the code easier to read.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 4 ++--
 drivers/usb/host/xhci.h     | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b6a2b0c01eb2..5ef38f97f7ea 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2311,8 +2311,8 @@ xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
 	writel(erst_size, &ir->ir_set->erst_size);
 
 	erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
-	erst_base &= ERST_BASE_RSVDP;
-	erst_base |= ir->erst.erst_dma_addr & ~ERST_BASE_RSVDP;
+	erst_base &= ~ERST_BASE_ADDRESS_MASK;
+	erst_base |= ir->erst.erst_dma_addr & ERST_BASE_ADDRESS_MASK;
 	if (xhci->quirks & XHCI_WRITE_64_HI_LO)
 		hi_lo_writeq(erst_base, &ir->ir_set->erst_base);
 	else
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 5e5b71958745..2a4beb6a6695 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -256,7 +256,8 @@ struct xhci_intr_reg {
 #define	ERST_SIZE_MASK		(0xffff)
 
 /* erst_base bitmasks */
-#define ERST_BASE_RSVDP		(GENMASK_ULL(5, 0))
+/* bits 63:6 - Event Ring Segment Table Base Address Register */
+#define ERST_BASE_ADDRESS_MASK	GENMASK_ULL(63, 6)
 
 /* erst_dequeue bitmasks */
 /* Dequeue ERST Segment Index (DESI) - Segment number (or alias)
-- 
2.47.2

[PATCH 7/9] usb: xhci: cleanup IMOD register comments

[Niklas Neronin]

Patch does not contain any functional changes.

Add missing macro descriptions with specific bit definitions for each data
field and reordered them accordingly.

Remove "HW use only" from Interrupt Moderation Counter. xHCI Specification
1.2, section 5.5.2.2, states "This counter may be directly written by
software at any time to alter the interrupt rate."

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci.h | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2a4beb6a6695..715b860995f3 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -211,14 +211,13 @@ struct xhci_op_regs {
 #define XHCI_PAGE_SIZE_MASK     0xffff
 
 /**
- * struct xhci_intr_reg - Interrupt Register Set
- * @irq_pending:	IMAN - Interrupt Management Register.  Used to enable
+ * struct xhci_intr_reg - Interrupt Register Set, v1.2 section 5.5.2.
+ * @irq_pending:	IMAN - Interrupt Management Register. Used to enable
  *			interrupts and check for pending interrupts.
- * @irq_control:	IMOD - Interrupt Moderation Register.
- * 			Used to throttle interrupts.
- * @erst_size:		Number of segments in the Event Ring Segment Table (ERST).
- * @erst_base:		ERST base address.
- * @erst_dequeue:	Event ring dequeue pointer.
+ * @irq_control:	IMOD - Interrupt Moderation Register. Used to throttle interrupts.
+ * @erst_size:		ERSTSZ - Number of segments in the Event Ring Segment Table (ERST).
+ * @erst_base:		ERSTBA - Event ring segment table base address.
+ * @erst_dequeue:	ERDP - Event ring dequeue pointer.
  *
  * Each interrupter (defined by a MSI-X vector) has an event ring and an Event
  * Ring Segment Table (ERST) associated with it.  The event ring is comprised of
@@ -243,12 +242,13 @@ struct xhci_intr_reg {
 #define	IMAN_IE			(1 << 1)
 
 /* irq_control bitmasks */
-/* Minimum interval between interrupts (in 250ns intervals).  The interval
- * between interrupts will be longer if there are no events on the event ring.
- * Default is 4000 (1 ms).
+/*
+ * bits 15:0 - Interrupt Moderation Interval, the minimum interval between interrupts
+ * (in 250ns intervals). The interval between interrupts will be longer if there are no
+ * events on the event ring. Default is 4000 (1 ms).
  */
 #define ER_IRQ_INTERVAL_MASK	(0xffff)
-/* Counter used to count down the time to the next interrupt - HW use only */
+/* bits 31:16 - Interrupt Moderation Counter, used to count down the time to the next interrupt */
 #define ER_IRQ_COUNTER_MASK	(0xffff << 16)
 
 /* erst_size bitmasks */
@@ -260,15 +260,18 @@ struct xhci_intr_reg {
 #define ERST_BASE_ADDRESS_MASK	GENMASK_ULL(63, 6)
 
 /* erst_dequeue bitmasks */
-/* Dequeue ERST Segment Index (DESI) - Segment number (or alias)
- * where the current dequeue pointer lies.  This is an optional HW hint.
+/*
+ * bits 2:0 - Dequeue ERST Segment Index (DESI), is the segment number (or alias) where the
+ * current dequeue pointer lies. This is an optional HW hint.
  */
 #define ERST_DESI_MASK		(0x7)
-/* Event Handler Busy (EHB) - is the event ring scheduled to be serviced by
+/*
+ * bit 3 - Event Handler Busy (EHB), whether the event ring is scheduled to be serviced by
  * a work queue (or delayed service routine)?
  */
 #define ERST_EHB		(1 << 3)
-#define ERST_PTR_MASK		(GENMASK_ULL(63, 4))
+/* bits 63:4 - Event Ring Dequeue Pointer */
+#define ERST_PTR_MASK		GENMASK_ULL(63, 4)
 
 /**
  * struct xhci_run_regs
-- 
2.47.2

[PATCH 8/9] usb: xhci: rename 'irq_pending' to 'iman'

[Niklas Neronin]

The Interrupt Register Set contains Interrupt Management register (IMAN).
The IMAN register contains the following fields:
 - Bit 0:	Interrupt Pending (IP)
 - Bit 1:	Interrupt Enable (IE)
 - Bits 31:2:	RsvdP (Reserved and Preserved)

Tn the xhci driver, the pointer currently named 'irq_pending' refers to the
IMAN register. However, the name "irq_pending" only describes one of the
fields within the IMAN register, rather than the entire register itself.
To improve clarity and better align with the xHCI specification,
the pointer is renamed to 'iman'.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 10 +++++-----
 drivers/usb/host/xhci.c      | 16 ++++++++--------
 drivers/usb/host/xhci.h      |  8 ++++----
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 412eb90c29fb..aa04a910c855 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3087,14 +3087,14 @@ static void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
 static void xhci_clear_interrupt_pending(struct xhci_interrupter *ir)
 {
 	if (!ir->ip_autoclear) {
-		u32 irq_pending;
+		u32 iman;
 
-		irq_pending = readl(&ir->ir_set->irq_pending);
-		irq_pending |= IMAN_IP;
-		writel(irq_pending, &ir->ir_set->irq_pending);
+		iman = readl(&ir->ir_set->iman);
+		iman |= IMAN_IP;
+		writel(iman, &ir->ir_set->iman);
 
 		/* Read operation to guarantee the write has been flushed from posted buffers */
-		readl(&ir->ir_set->irq_pending);
+		readl(&ir->ir_set->iman);
 	}
 }
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a41337360535..2b4dd527c64d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -329,12 +329,12 @@ static int xhci_enable_interrupter(struct xhci_interrupter *ir)
 	if (!ir || !ir->ir_set)
 		return -EINVAL;
 
-	iman = readl(&ir->ir_set->irq_pending);
+	iman = readl(&ir->ir_set->iman);
 	iman |= IMAN_IE;
-	writel(iman, &ir->ir_set->irq_pending);
+	writel(iman, &ir->ir_set->iman);
 
 	/* Read operation to guarantee the write has been flushed from posted buffers */
-	readl(&ir->ir_set->irq_pending);
+	readl(&ir->ir_set->iman);
 	return 0;
 }
 
@@ -345,11 +345,11 @@ static int xhci_disable_interrupter(struct xhci_hcd *xhci, struct xhci_interrupt
 	if (!ir || !ir->ir_set)
 		return -EINVAL;
 
-	iman = readl(&ir->ir_set->irq_pending);
+	iman = readl(&ir->ir_set->iman);
 	iman &= ~IMAN_IE;
-	writel(iman, &ir->ir_set->irq_pending);
+	writel(iman, &ir->ir_set->iman);
 
-	iman = readl(&ir->ir_set->irq_pending);
+	iman = readl(&ir->ir_set->iman);
 	if (iman & IMAN_IP)
 		xhci_dbg(xhci, "%s: Interrupt pending\n", __func__);
 
@@ -730,7 +730,7 @@ static void xhci_save_registers(struct xhci_hcd *xhci)
 		ir->s3_erst_size = readl(&ir->ir_set->erst_size);
 		ir->s3_erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
 		ir->s3_erst_dequeue = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
-		ir->s3_irq_pending = readl(&ir->ir_set->irq_pending);
+		ir->s3_iman = readl(&ir->ir_set->iman);
 		ir->s3_irq_control = readl(&ir->ir_set->irq_control);
 	}
 }
@@ -754,7 +754,7 @@ static void xhci_restore_registers(struct xhci_hcd *xhci)
 		writel(ir->s3_erst_size, &ir->ir_set->erst_size);
 		xhci_write_64(xhci, ir->s3_erst_base, &ir->ir_set->erst_base);
 		xhci_write_64(xhci, ir->s3_erst_dequeue, &ir->ir_set->erst_dequeue);
-		writel(ir->s3_irq_pending, &ir->ir_set->irq_pending);
+		writel(ir->s3_iman, &ir->ir_set->iman);
 		writel(ir->s3_irq_control, &ir->ir_set->irq_control);
 	}
 }
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 715b860995f3..05e35f1303c3 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -212,7 +212,7 @@ struct xhci_op_regs {
 
 /**
  * struct xhci_intr_reg - Interrupt Register Set, v1.2 section 5.5.2.
- * @irq_pending:	IMAN - Interrupt Management Register. Used to enable
+ * @iman:		IMAN - Interrupt Management Register. Used to enable
  *			interrupts and check for pending interrupts.
  * @irq_control:	IMOD - Interrupt Moderation Register. Used to throttle interrupts.
  * @erst_size:		ERSTSZ - Number of segments in the Event Ring Segment Table (ERST).
@@ -227,7 +227,7 @@ struct xhci_op_regs {
  * updates the dequeue pointer.
  */
 struct xhci_intr_reg {
-	__le32	irq_pending;
+	__le32	iman;
 	__le32	irq_control;
 	__le32	erst_size;
 	__le32	rsvd;
@@ -235,7 +235,7 @@ struct xhci_intr_reg {
 	__le64	erst_dequeue;
 };
 
-/* irq_pending bitmasks */
+/* iman bitmasks */
 /* bit 0 - Interrupt Pending (IP), whether there is an interrupt pending. Write-1-to-clear. */
 #define	IMAN_IP			(1 << 0)
 /* bit 1 - Interrupt Enable (IE), whether the interrupter is capable of generating an interrupt */
@@ -1446,7 +1446,7 @@ struct xhci_interrupter {
 	bool			ip_autoclear;
 	u32			isoc_bei_interval;
 	/* For interrupter registers save and restore over suspend/resume */
-	u32	s3_irq_pending;
+	u32	s3_iman;
 	u32	s3_irq_control;
 	u32	s3_erst_size;
 	u64	s3_erst_base;
-- 
2.47.2

[PATCH 9/9] usb: xhci: rename 'irq_control' to 'imod'

[Niklas Neronin]

The Interrupt Register Set contains Interrupt Moderation register (IMOD).
The IMOD register contains the following fields:
 - Bits 15:0:	Interrupt Moderation Interval (IMODI)
 - Bits 31:16:	Interrupt Moderation Counter (IMODC)

In the xHCI driver, the pointer currently named 'irq_control' refers to the
IMOD register. However, the name 'irq_control' does not accurately describe
the register or its contents, and the xHCI specification does not use the
term "irq control" or "interrupt control" for this register. To improve
clarity and better align with the xHCI specification, the pointer is
renamed to 'imod'.

Additionally, the IMOD register fields IMODI & IMODC have their own masks,
which are also renamed for consistency:
 * 'ER_IRQ_INTERVAL_MASK' -> 'IMODI_MASK'
 * 'ER_IRQ_COUNTER_MASK' -> 'IMODC_MASK'

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci.c | 12 ++++++------
 drivers/usb/host/xhci.h | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2b4dd527c64d..ec88d2cacb13 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -366,12 +366,12 @@ int xhci_set_interrupter_moderation(struct xhci_interrupter *ir,
 		return -EINVAL;
 
 	/* IMODI value in IMOD register is in 250ns increments */
-	imod_interval = umin(imod_interval / 250, ER_IRQ_INTERVAL_MASK);
+	imod_interval = umin(imod_interval / 250, IMODI_MASK);
 
-	imod = readl(&ir->ir_set->irq_control);
-	imod &= ~ER_IRQ_INTERVAL_MASK;
+	imod = readl(&ir->ir_set->imod);
+	imod &= ~IMODI_MASK;
 	imod |= imod_interval;
-	writel(imod, &ir->ir_set->irq_control);
+	writel(imod, &ir->ir_set->imod);
 
 	return 0;
 }
@@ -731,7 +731,7 @@ static void xhci_save_registers(struct xhci_hcd *xhci)
 		ir->s3_erst_base = xhci_read_64(xhci, &ir->ir_set->erst_base);
 		ir->s3_erst_dequeue = xhci_read_64(xhci, &ir->ir_set->erst_dequeue);
 		ir->s3_iman = readl(&ir->ir_set->iman);
-		ir->s3_irq_control = readl(&ir->ir_set->irq_control);
+		ir->s3_imod = readl(&ir->ir_set->imod);
 	}
 }
 
@@ -755,7 +755,7 @@ static void xhci_restore_registers(struct xhci_hcd *xhci)
 		xhci_write_64(xhci, ir->s3_erst_base, &ir->ir_set->erst_base);
 		xhci_write_64(xhci, ir->s3_erst_dequeue, &ir->ir_set->erst_dequeue);
 		writel(ir->s3_iman, &ir->ir_set->iman);
-		writel(ir->s3_irq_control, &ir->ir_set->irq_control);
+		writel(ir->s3_imod, &ir->ir_set->imod);
 	}
 }
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 05e35f1303c3..5eb01d1a3e7e 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -214,7 +214,7 @@ struct xhci_op_regs {
  * struct xhci_intr_reg - Interrupt Register Set, v1.2 section 5.5.2.
  * @iman:		IMAN - Interrupt Management Register. Used to enable
  *			interrupts and check for pending interrupts.
- * @irq_control:	IMOD - Interrupt Moderation Register. Used to throttle interrupts.
+ * @imod:		IMOD - Interrupt Moderation Register. Used to throttle interrupts.
  * @erst_size:		ERSTSZ - Number of segments in the Event Ring Segment Table (ERST).
  * @erst_base:		ERSTBA - Event ring segment table base address.
  * @erst_dequeue:	ERDP - Event ring dequeue pointer.
@@ -228,7 +228,7 @@ struct xhci_op_regs {
  */
 struct xhci_intr_reg {
 	__le32	iman;
-	__le32	irq_control;
+	__le32	imod;
 	__le32	erst_size;
 	__le32	rsvd;
 	__le64	erst_base;
@@ -241,15 +241,15 @@ struct xhci_intr_reg {
 /* bit 1 - Interrupt Enable (IE), whether the interrupter is capable of generating an interrupt */
 #define	IMAN_IE			(1 << 1)
 
-/* irq_control bitmasks */
+/* imod bitmasks */
 /*
  * bits 15:0 - Interrupt Moderation Interval, the minimum interval between interrupts
  * (in 250ns intervals). The interval between interrupts will be longer if there are no
  * events on the event ring. Default is 4000 (1 ms).
  */
-#define ER_IRQ_INTERVAL_MASK	(0xffff)
+#define IMODI_MASK		(0xffff)
 /* bits 31:16 - Interrupt Moderation Counter, used to count down the time to the next interrupt */
-#define ER_IRQ_COUNTER_MASK	(0xffff << 16)
+#define IMODC_MASK		(0xffff << 16)
 
 /* erst_size bitmasks */
 /* bits 15:0 - Event Ring Segment Table Size, number of ERST entries */
@@ -1447,7 +1447,7 @@ struct xhci_interrupter {
 	u32			isoc_bei_interval;
 	/* For interrupter registers save and restore over suspend/resume */
 	u32	s3_iman;
-	u32	s3_irq_control;
+	u32	s3_imod;
 	u32	s3_erst_size;
 	u64	s3_erst_base;
 	u64	s3_erst_dequeue;
-- 
2.47.2

Re: [PATCH 1/9] usb: xhci: set requested IMODI to the closest supported value

[Mathias Nyman]

On 8.4.2025 14.57, Niklas Neronin wrote:
> The function configures the Interrupt Moderation Interval (IMODI) via bits
> 15:0 in the Interrupt Moderation Register. The IMODI value is specified in
> increments of 250 nanoseconds. For instance, an IMODI register value of 16
> corresponds to 4000 nanoseconds, resulting in an interrupt every ~1ms.
> 
> Currently, the function fails when a requested IMODI value is too large,
> only logging a warning message for secondary interrupters. Prevent this by
> automatically adjusting the IMODI value to the nearest supported value.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>   drivers/usb/host/xhci-mem.c | 5 +----
>   drivers/usb/host/xhci.c     | 7 +++++--
>   2 files changed, 6 insertions(+), 6 deletions(-)
> 

This patch now conflicts with the recently accepted sideband series.
Any chance you could respin this on top of that.

Otherwise whole series looks good.

Thanks
Mathias