[PATCH v2 00/11] usb: xhci: decouple allocation and initialization

[PATCH v2 00/11] usb: xhci: decouple allocation and initialization

[Niklas Neronin]

Currently, after hibernation (S4 state), the xhci driver frees all its
memory and reallocates it from scratch, which is inefficient. Instead, much
of the memory can be simply re-initialized.

The proposed changes begin the process by decoupling initialization code
from memory allocation code. Specifically, the initialization code is moved
from xhci_mem_init() into separate functions, which are then called from
xhci_init().

By implementing these changes, future patches will be able to call the
initialization functions directly after hibernation, thereby improving
efficiency and enhancing code structure.

v2:
 * Add "Array" to "Set Device Context Base Address pointer" comment.
 * Rebase on top of the recently accepted sideband series to resolve some
   conflicts. The rebase impacts commits 8 and 9.
   There will be duplicate code in xhci_create_secondary_interrupter().
   Since I have another patch set pending that slightly impacts this
   function, I have decided to hold off on reworking or cleaning up the
   function.

Niklas Neronin (11):
  usb: xhci: relocate pre-allocation initialization
  usb: xhci: move device slot enabling register write
  usb: xhci: move command ring pointer write
  usb: xhci: refactor xhci_set_cmd_ring_deq()
  usb: xhci: move DCBAA pointer write
  usb: xhci: move doorbell array pointer assignment
  usb: xhci: move enabling of USB 3 device notifications
  usb: xhci: remove error handling from xhci_add_interrupter()
  usb: xhci: move initialization of the primary interrupter
  usb: xhci: add individual allocation checks in xhci_mem_init()
  usb: xhci: cleanup xhci_mem_init()

 drivers/usb/host/xhci-caps.h |   4 +-
 drivers/usb/host/xhci-mem.c  | 168 ++++++++++-------------------------
 drivers/usb/host/xhci.c      | 126 +++++++++++++++++++++-----
 drivers/usb/host/xhci.h      |  12 +--
 4 files changed, 159 insertions(+), 151 deletions(-)

-- 
2.47.2

[PATCH v2 01/11] usb: xhci: relocate pre-allocation initialization

[Niklas Neronin]

Move pre-allocation initialization from xhci_mem_init() to xhci_init().
This change is part of an ongoing effort to separate initialization from
allocation within the xhci driver. By doing so, it will enable future
patches to re-initialize xhci driver memory without the necessity of fully
recreating it.

Additionally, compliance mode recovery initialization has been adjusted to
only occur after successful memory allocation.

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

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index ed36df46b140..d309e8e9d508 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2380,22 +2380,6 @@ xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
 }
 EXPORT_SYMBOL_GPL(xhci_create_secondary_interrupter);
 
-static void xhci_hcd_page_size(struct xhci_hcd *xhci)
-{
-	u32 page_size;
-
-	page_size = readl(&xhci->op_regs->page_size) & XHCI_PAGE_SIZE_MASK;
-	if (!is_power_of_2(page_size)) {
-		xhci_warn(xhci, "Invalid page size register = 0x%x\n", page_size);
-		/* Fallback to 4K page size, since that's common */
-		page_size = 1;
-	}
-
-	xhci->page_size = page_size << 12;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "HCD page size set to %iK",
-		       xhci->page_size >> 10);
-}
-
 int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 {
 	struct xhci_interrupter *ir;
@@ -2404,15 +2388,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	unsigned int	val, val2;
 	u64		val_64;
 	u32		temp;
-	int		i;
-
-	INIT_LIST_HEAD(&xhci->cmd_list);
-
-	/* init command timeout work */
-	INIT_DELAYED_WORK(&xhci->cmd_timer, xhci_handle_command_timeout);
-	init_completion(&xhci->cmd_ring_stop_completion);
-
-	xhci_hcd_page_size(xhci);
 
 	/*
 	 * Program the Number of Device Slots Enabled field in the CONFIG
@@ -2524,9 +2499,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 
 	ir->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
 
-	for (i = 0; i < MAX_HC_SLOTS; i++)
-		xhci->devs[i] = NULL;
-
 	if (scratchpad_alloc(xhci, flags))
 		goto fail;
 	if (xhci_setup_port_arrays(xhci, flags))
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index cfda35004754..ec2e4a2d8af0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -461,6 +461,21 @@ static int xhci_all_ports_seen_u0(struct xhci_hcd *xhci)
 	return (xhci->port_status_u0 == ((1 << xhci->usb3_rhub.num_ports) - 1));
 }
 
+static void xhci_hcd_page_size(struct xhci_hcd *xhci)
+{
+	u32 page_size;
+
+	page_size = readl(&xhci->op_regs->page_size) & XHCI_PAGE_SIZE_MASK;
+	if (!is_power_of_2(page_size)) {
+		xhci_warn(xhci, "Invalid page size register = 0x%x\n", page_size);
+		/* Fallback to 4K page size, since that's common */
+		page_size = 1;
+	}
+
+	xhci->page_size = page_size << 12;
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "HCD page size set to %iK",
+		       xhci->page_size >> 10);
+}
 
 /*
  * Initialize memory for HCD and xHC (one-time init).
@@ -474,11 +489,18 @@ static int xhci_init(struct usb_hcd *hcd)
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 	int retval;
 
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "xhci_init");
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Starting %s", __func__);
 	spin_lock_init(&xhci->lock);
 
+	INIT_LIST_HEAD(&xhci->cmd_list);
+	INIT_DELAYED_WORK(&xhci->cmd_timer, xhci_handle_command_timeout);
+	init_completion(&xhci->cmd_ring_stop_completion);
+	xhci_hcd_page_size(xhci);
+	memset(xhci->devs, 0, MAX_HC_SLOTS * sizeof(*xhci->devs));
+
 	retval = xhci_mem_init(xhci, GFP_KERNEL);
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Finished xhci_init");
+	if (retval)
+		return retval;
 
 	/* Initializing Compliance Mode Recovery Data If Needed */
 	if (xhci_compliance_mode_recovery_timer_quirk_check()) {
@@ -486,7 +508,8 @@ static int xhci_init(struct usb_hcd *hcd)
 		compliance_mode_recovery_timer_init(xhci);
 	}
 
-	return retval;
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Finished %s", __func__);
+	return 0;
 }
 
 /*-------------------------------------------------------------------------*/
-- 
2.47.2

[PATCH v2 02/11] usb: xhci: move device slot enabling register write

[Niklas Neronin]

Refactor the setting of the Number of Device Slots Enabled field into a
separate function, relocating it to xhci_init().

The xHCI driver consistently sets the number of enabled device slots to the
maximum value. The new function is named to reflect this behavior.

Remove the "// " prefix from trace messages, as it is unnecessary and
distracting.

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

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d309e8e9d508..dbf0187086db 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2385,23 +2385,10 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	struct xhci_interrupter *ir;
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
 	dma_addr_t	dma;
-	unsigned int	val, val2;
+	unsigned int	val;
 	u64		val_64;
 	u32		temp;
 
-	/*
-	 * Program the Number of Device Slots Enabled field in the CONFIG
-	 * register with the max value of slots the HC can handle.
-	 */
-	val = HCS_MAX_SLOTS(readl(&xhci->cap_regs->hcs_params1));
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"// xHC can handle at most %d device slots.", val);
-	val2 = readl(&xhci->op_regs->config_reg);
-	val |= (val2 & ~HCS_SLOTS_MASK);
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"// Setting Max device slots reg = 0x%x.", val);
-	writel(val, &xhci->op_regs->config_reg);
-
 	/*
 	 * xHCI section 5.4.6 - Device Context array must be
 	 * "physically contiguous and 64-byte (cache line) aligned".
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec2e4a2d8af0..4de51ba851d8 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -477,6 +477,24 @@ static void xhci_hcd_page_size(struct xhci_hcd *xhci)
 		       xhci->page_size >> 10);
 }
 
+static void xhci_enable_max_dev_slots(struct xhci_hcd *xhci)
+{
+	u32 config_reg;
+	u32 max_slots;
+
+	max_slots = HCS_MAX_SLOTS(xhci->hcs_params1);
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "xHC can handle at most %d device slots",
+		       max_slots);
+
+	config_reg = readl(&xhci->op_regs->config_reg);
+	config_reg &= ~HCS_SLOTS_MASK;
+	config_reg |= max_slots;
+
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Setting Max device slots reg = 0x%x",
+		       config_reg);
+	writel(config_reg, &xhci->op_regs->config_reg);
+}
+
 /*
  * Initialize memory for HCD and xHC (one-time init).
  *
@@ -502,6 +520,9 @@ static int xhci_init(struct usb_hcd *hcd)
 	if (retval)
 		return retval;
 
+	/* Set the Number of Device Slots Enabled to the maximum supported value */
+	xhci_enable_max_dev_slots(xhci);
+
 	/* Initializing Compliance Mode Recovery Data If Needed */
 	if (xhci_compliance_mode_recovery_timer_quirk_check()) {
 		xhci->quirks |= XHCI_COMP_MODE_QUIRK;
-- 
2.47.2

[PATCH v2 03/11] usb: xhci: move command ring pointer write

[Niklas Neronin]

Move command ring pointer write from xhci_mem_init() to xhci_init(),
and utilize the xhci_set_cmd_ring_deq() function.

The xhci_set_cmd_ring_deq() function is nearly identical to the Command
Ring Control register code in xhci_mem_init(). The only notable change is
the use of:
  xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg, xhci->cmd_ring->dequeue)
instead of:
  xhci->cmd_ring->first_seg->dma
but they are effectively the same in this context. The former represents
the exact position of the dequeue pointer, while the latter is the first
DMA in the first segment. Before use, the dequeue pointer is at the first
DMA in the first segment.

The xhci_set_cmd_ring_deq() function is moved without modification, except
for (long unsigned long) -> (unsigned long long) due to checkpatch.pl.

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

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index dbf0187086db..46156e99bbbe 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2386,7 +2386,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
 	dma_addr_t	dma;
 	unsigned int	val;
-	u64		val_64;
 	u32		temp;
 
 	/*
@@ -2449,15 +2448,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "First segment DMA is 0x%pad",
 			&xhci->cmd_ring->first_seg->dma);
 
-	/* Set the address in the Command Ring Control register */
-	val_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
-	val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
-		(xhci->cmd_ring->first_seg->dma & (u64) ~CMD_RING_RSVD_BITS) |
-		xhci->cmd_ring->cycle_state;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"// Setting command ring address to 0x%016llx", val_64);
-	xhci_write_64(xhci, val_64, &xhci->op_regs->cmd_ring);
-
 	/* Reserve one command ring TRB for disabling LPM.
 	 * Since the USB core grabs the shared usb_bus bandwidth mutex before
 	 * disabling LPM, we only need to reserve one TRB for all devices.
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4de51ba851d8..92c32a80acae 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -495,6 +495,23 @@ static void xhci_enable_max_dev_slots(struct xhci_hcd *xhci)
 	writel(config_reg, &xhci->op_regs->config_reg);
 }
 
+static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
+{
+	u64	val_64;
+
+	/* step 2: initialize command ring buffer */
+	val_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
+	val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
+		(xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
+					xhci->cmd_ring->dequeue) &
+			(u64) ~CMD_RING_RSVD_BITS) |
+		xhci->cmd_ring->cycle_state;
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+			"// Setting command ring address to 0x%llx",
+			(unsigned long long) val_64);
+	xhci_write_64(xhci, val_64, &xhci->op_regs->cmd_ring);
+}
+
 /*
  * Initialize memory for HCD and xHC (one-time init).
  *
@@ -523,6 +540,9 @@ static int xhci_init(struct usb_hcd *hcd)
 	/* Set the Number of Device Slots Enabled to the maximum supported value */
 	xhci_enable_max_dev_slots(xhci);
 
+	/* Set the address in the Command Ring Control register */
+	xhci_set_cmd_ring_deq(xhci);
+
 	/* Initializing Compliance Mode Recovery Data If Needed */
 	if (xhci_compliance_mode_recovery_timer_quirk_check()) {
 		xhci->quirks |= XHCI_COMP_MODE_QUIRK;
@@ -793,23 +813,6 @@ static void xhci_restore_registers(struct xhci_hcd *xhci)
 	}
 }
 
-static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
-{
-	u64	val_64;
-
-	/* step 2: initialize command ring buffer */
-	val_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
-	val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
-		(xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
-				      xhci->cmd_ring->dequeue) &
-		 (u64) ~CMD_RING_RSVD_BITS) |
-		xhci->cmd_ring->cycle_state;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"// Setting command ring address to 0x%llx",
-			(long unsigned long) val_64);
-	xhci_write_64(xhci, val_64, &xhci->op_regs->cmd_ring);
-}
-
 /*
  * The whole command ring must be cleared to zero when we suspend the host.
  *
-- 
2.47.2

Re: [PATCH v2 03/11] usb: xhci: move command ring pointer write

[Sergey Shtylyov]

On 4/16/25 4:45 PM, Niklas Neronin wrote:

> Move command ring pointer write from xhci_mem_init() to xhci_init(),
> and utilize the xhci_set_cmd_ring_deq() function.
> 
> The xhci_set_cmd_ring_deq() function is nearly identical to the Command
> Ring Control register code in xhci_mem_init(). The only notable change is
> the use of:
>   xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg, xhci->cmd_ring->dequeue)
> instead of:
>   xhci->cmd_ring->first_seg->dma
> but they are effectively the same in this context. The former represents
> the exact position of the dequeue pointer, while the latter is the first
> DMA in the first segment. Before use, the dequeue pointer is at the first
> DMA in the first segment.
> 
> The xhci_set_cmd_ring_deq() function is moved without modification, except
> for (long unsigned long) -> (unsigned long long) due to checkpatch.pl.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>  drivers/usb/host/xhci-mem.c | 10 ----------
>  drivers/usb/host/xhci.c     | 37 ++++++++++++++++++++-----------------
>  2 files changed, 20 insertions(+), 27 deletions(-)

[...]
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 4de51ba851d8..92c32a80acae 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -495,6 +495,23 @@ static void xhci_enable_max_dev_slots(struct xhci_hcd *xhci)
>  	writel(config_reg, &xhci->op_regs->config_reg);
>  }
>  
> +static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
> +{
> +	u64	val_64;
> +
> +	/* step 2: initialize command ring buffer */
> +	val_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
> +	val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
> +		(xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
> +					xhci->cmd_ring->dequeue) &
> +			(u64) ~CMD_RING_RSVD_BITS) |
> +		xhci->cmd_ring->cycle_state;

   Very strange alignment scheme here...

> +	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
> +			"// Setting command ring address to 0x%llx",

   I thought you're dropping those //?

> +			(unsigned long long) val_64);
> +	xhci_write_64(xhci, val_64, &xhci->op_regs->cmd_ring);
> +}
> +
>  /*
>   * Initialize memory for HCD and xHC (one-time init).
>   *
[...]
MBR, Sergey

[PATCH v2 04/11] usb: xhci: refactor xhci_set_cmd_ring_deq()

[Niklas Neronin]

Refactor xhci_set_cmd_ring_deq() making the code more understandable by
using more descriptive constants and separating operations logically.

- Remove 'CMD_RING_RSVD_BITS' the macro is misleading, the reserved bits
  are 5:4, yet the mask is for bits 5:0.
- Introduce masks 'CMD_RING_PTR_MASK' and 'CMD_RING_CYCLE' to clearly
  define the bits for the Command Ring pointer and Command Ring Cycle.
- Simplifying the process of setting the command ring address by separating
  the DMA address calculation and the Command Ring Control register (crcr)
  updates.
- Remove the "// " prefix from trace messages, as it is unnecessary and
  distracting.

Note: In the current implementation, the cycle bit is not cleared before
applying the OR operation. Although this hasn't caused issues so far
because the bit is '0' before reaching this function, the bit is now
cleared before being set to prevent potential future problems and simplify
the process.

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

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 92c32a80acae..d4f3da2c074b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -497,19 +497,21 @@ static void xhci_enable_max_dev_slots(struct xhci_hcd *xhci)
 
 static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
 {
-	u64	val_64;
+	dma_addr_t deq_dma;
+	u64 crcr;
 
-	/* step 2: initialize command ring buffer */
-	val_64 = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
-	val_64 = (val_64 & (u64) CMD_RING_RSVD_BITS) |
-		(xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
-					xhci->cmd_ring->dequeue) &
-			(u64) ~CMD_RING_RSVD_BITS) |
-		xhci->cmd_ring->cycle_state;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"// Setting command ring address to 0x%llx",
-			(unsigned long long) val_64);
-	xhci_write_64(xhci, val_64, &xhci->op_regs->cmd_ring);
+	deq_dma = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg, xhci->cmd_ring->dequeue);
+	deq_dma &= CMD_RING_PTR_MASK;
+
+	crcr = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
+	crcr &= ~CMD_RING_PTR_MASK;
+	crcr |= deq_dma;
+
+	crcr &= ~CMD_RING_CYCLE;
+	crcr |= xhci->cmd_ring->cycle_state;
+
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Setting command ring address to 0x%llx", crcr);
+	xhci_write_64(xhci, crcr, &xhci->op_regs->cmd_ring);
 }
 
 /*
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 7eaabe4f6c87..bfb02dbd9fc7 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -191,16 +191,16 @@ struct xhci_op_regs {
 #define	DEV_NOTE_FWAKE		ENABLE_DEV_NOTE(1)
 
 /* CRCR - Command Ring Control Register - cmd_ring bitmasks */
-/* bit 0 is the command ring cycle state */
+/* bit 0 - Cycle bit indicates the ownership of the command ring */
+#define CMD_RING_CYCLE		(1 << 0)
 /* stop ring operation after completion of the currently executing command */
 #define CMD_RING_PAUSE		(1 << 1)
 /* stop ring immediately - abort the currently executing command */
 #define CMD_RING_ABORT		(1 << 2)
 /* true: command ring is running */
 #define CMD_RING_RUNNING	(1 << 3)
-/* bits 4:5 reserved and should be preserved */
-/* Command Ring pointer - bit mask for the lower 32 bits. */
-#define CMD_RING_RSVD_BITS	(0x3f)
+/* bits 63:6 - Command Ring pointer */
+#define CMD_RING_PTR_MASK	GENMASK_ULL(63, 6)
 
 /* CONFIG - Configure Register - config_reg bitmasks */
 /* bits 0:7 - maximum number of device slots enabled (NumSlotsEn) */
-- 
2.47.2

[PATCH v2 05/11] usb: xhci: move DCBAA pointer write

[Niklas Neronin]

Move the Device Context Base Address Array (DCBAA) pointer write from
xhci_mem_init() to xhci_init(). This is part of the ongoing effort to
separate allocation and initialization.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
v2:
 * Add "Array" to "Set Device Context Base Address pointer" comment.

 drivers/usb/host/xhci-mem.c | 1 -
 drivers/usb/host/xhci.c     | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 46156e99bbbe..19f756a57cb7 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2400,7 +2400,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 			"// Device context base array address = 0x%pad (DMA), %p (virt)",
 			&xhci->dcbaa->dma, xhci->dcbaa);
-	xhci_write_64(xhci, dma, &xhci->op_regs->dcbaa_ptr);
 
 	/*
 	 * Initialize the ring segment pool.  The ring must be a contiguous
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d4f3da2c074b..ef93c1ae5215 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -545,6 +545,9 @@ static int xhci_init(struct usb_hcd *hcd)
 	/* Set the address in the Command Ring Control register */
 	xhci_set_cmd_ring_deq(xhci);
 
+	/* Set Device Context Base Address Array pointer */
+	xhci_write_64(xhci, xhci->dcbaa->dma, &xhci->op_regs->dcbaa_ptr);
+
 	/* Initializing Compliance Mode Recovery Data If Needed */
 	if (xhci_compliance_mode_recovery_timer_quirk_check()) {
 		xhci->quirks |= XHCI_COMP_MODE_QUIRK;
-- 
2.47.2

[PATCH v2 06/11] usb: xhci: move doorbell array pointer assignment

[Niklas Neronin]

Move the assignment of the doorbell array pointer from xhci_mem_init()
to xhci_init(). The assignment now utilizes the newly introduced
xhci_set_doorbell_ptr() function.

Doorbell Array Offset mask (DBOFF_MASK) is updated to directly specify its
bit range as 31:2, rather than using inverted reserved bits 1:0.
This change simplifies the mask representation, making it more intuitive
and easier to understand.

Remove the "// " prefix from trace messages, as it is unnecessary and
distracting.

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

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index f6b9a00a0ab9..4b8ff4815644 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -62,8 +62,8 @@
 
 #define CTX_SIZE(_hcc)		(HCC_64BYTE_CONTEXT(_hcc) ? 64 : 32)
 
-/* db_off bitmask - bits 0:1 reserved */
-#define	DBOFF_MASK	(~0x3)
+/* db_off bitmask - bits 31:2 Doorbell Array Offset */
+#define	DBOFF_MASK	(0xfffffffc)
 
 /* run_regs_off bitmask - bits 0:4 reserved */
 #define	RTSOFF_MASK	(~0x1f)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 19f756a57cb7..391dc2282360 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2385,7 +2385,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	struct xhci_interrupter *ir;
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
 	dma_addr_t	dma;
-	unsigned int	val;
 	u32		temp;
 
 	/*
@@ -2453,13 +2452,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	 */
 	xhci->cmd_ring_reserved_trbs++;
 
-	val = readl(&xhci->cap_regs->db_off);
-	val &= DBOFF_MASK;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-		       "// Doorbell array is located at offset 0x%x from cap regs base addr",
-		       val);
-	xhci->dba = (void __iomem *) xhci->cap_regs + val;
-
 	/* Allocate and set up primary interrupter 0 with an event ring. */
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 		       "Allocating primary event ring");
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ef93c1ae5215..00aae0619ccd 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -514,6 +514,16 @@ static void xhci_set_cmd_ring_deq(struct xhci_hcd *xhci)
 	xhci_write_64(xhci, crcr, &xhci->op_regs->cmd_ring);
 }
 
+static void xhci_set_doorbell_ptr(struct xhci_hcd *xhci)
+{
+	u32 offset;
+
+	offset = readl(&xhci->cap_regs->db_off) & DBOFF_MASK;
+	xhci->dba = (void __iomem *)xhci->cap_regs + offset;
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+		       "Doorbell array is located at offset 0x%x from cap regs base addr", offset);
+}
+
 /*
  * Initialize memory for HCD and xHC (one-time init).
  *
@@ -548,6 +558,9 @@ static int xhci_init(struct usb_hcd *hcd)
 	/* Set Device Context Base Address Array pointer */
 	xhci_write_64(xhci, xhci->dcbaa->dma, &xhci->op_regs->dcbaa_ptr);
 
+	/* Set Doorbell array pointer */
+	xhci_set_doorbell_ptr(xhci);
+
 	/* Initializing Compliance Mode Recovery Data If Needed */
 	if (xhci_compliance_mode_recovery_timer_quirk_check()) {
 		xhci->quirks |= XHCI_COMP_MODE_QUIRK;
-- 
2.47.2

Re: [PATCH v2 06/11] usb: xhci: move doorbell array pointer assignment

[Sergey Shtylyov]

On 4/16/25 4:45 PM, Niklas Neronin wrote:

> Move the assignment of the doorbell array pointer from xhci_mem_init()
> to xhci_init(). The assignment now utilizes the newly introduced
> xhci_set_doorbell_ptr() function.
> 
> Doorbell Array Offset mask (DBOFF_MASK) is updated to directly specify its
> bit range as 31:2, rather than using inverted reserved bits 1:0.
> This change simplifies the mask representation, making it more intuitive
> and easier to understand.
> 
> Remove the "// " prefix from trace messages, as it is unnecessary and
> distracting.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>  drivers/usb/host/xhci-caps.h |  4 ++--
>  drivers/usb/host/xhci-mem.c  |  8 --------
>  drivers/usb/host/xhci.c      | 13 +++++++++++++
>  3 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
> index f6b9a00a0ab9..4b8ff4815644 100644
> --- a/drivers/usb/host/xhci-caps.h
> +++ b/drivers/usb/host/xhci-caps.h
> @@ -62,8 +62,8 @@
>  
>  #define CTX_SIZE(_hcc)		(HCC_64BYTE_CONTEXT(_hcc) ? 64 : 32)
>  
> -/* db_off bitmask - bits 0:1 reserved */
> -#define	DBOFF_MASK	(~0x3)
> +/* db_off bitmask - bits 31:2 Doorbell Array Offset */
> +#define	DBOFF_MASK	(0xfffffffc)

   I don't think parens here are really needed...

[...]

MBR, Sergey

[PATCH v2 07/11] usb: xhci: move enabling of USB 3 device notifications

[Niklas Neronin]

Relocated the enabling of USB 3.0 device notifications from xhci_mem_init()
to xhci_init(). Introduced xhci_set_dev_notifications() function to handle
the notification settings.

Simplify 'DEV_NOTE_FWAKE' masks by directly using the 'ENABLE_DEV_NOTE'
value (1 << 1) instead of using the 'ENABLE_DEV_NOTE' macro.
Macro 'ENABLE_DEV_NOTE' is removed.

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

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 391dc2282360..226013ee443a 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2385,7 +2385,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	struct xhci_interrupter *ir;
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
 	dma_addr_t	dma;
-	u32		temp;
 
 	/*
 	 * xHCI section 5.4.6 - Device Context array must be
@@ -2472,15 +2471,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	if (xhci_setup_port_arrays(xhci, flags))
 		goto fail;
 
-	/* Enable USB 3.0 device notifications for function remote wake, which
-	 * is necessary for allowing USB 3.0 devices to do remote wakeup from
-	 * U3 (device suspend).
-	 */
-	temp = readl(&xhci->op_regs->dev_notification);
-	temp &= ~DEV_NOTE_MASK;
-	temp |= DEV_NOTE_FWAKE;
-	writel(temp, &xhci->op_regs->dev_notification);
-
 	return 0;
 
 fail:
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 00aae0619ccd..18d8637773cb 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -524,6 +524,20 @@ static void xhci_set_doorbell_ptr(struct xhci_hcd *xhci)
 		       "Doorbell array is located at offset 0x%x from cap regs base addr", offset);
 }
 
+/*
+ * Enable USB 3.0 device notifications for function remote wake, which is necessary
+ * for allowing USB 3.0 devices to do remote wakeup from U3 (device suspend).
+ */
+static void xhci_set_dev_notifications(struct xhci_hcd *xhci)
+{
+	u32 dev_notf;
+
+	dev_notf = readl(&xhci->op_regs->dev_notification);
+	dev_notf &= ~DEV_NOTE_MASK;
+	dev_notf |= DEV_NOTE_FWAKE;
+	writel(dev_notf, &xhci->op_regs->dev_notification);
+}
+
 /*
  * Initialize memory for HCD and xHC (one-time init).
  *
@@ -561,6 +575,9 @@ static int xhci_init(struct usb_hcd *hcd)
 	/* Set Doorbell array pointer */
 	xhci_set_doorbell_ptr(xhci);
 
+	/* Set USB 3.0 device notifications for function remote wake */
+	xhci_set_dev_notifications(xhci);
+
 	/* Initializing Compliance Mode Recovery Data If Needed */
 	if (xhci_compliance_mode_recovery_timer_quirk_check()) {
 		xhci->quirks |= XHCI_COMP_MODE_QUIRK;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index bfb02dbd9fc7..c05c3eab005b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -184,11 +184,10 @@ struct xhci_op_regs {
  * notification type that matches a bit set in this bit field.
  */
 #define	DEV_NOTE_MASK		(0xffff)
-#define ENABLE_DEV_NOTE(x)	(1 << (x))
 /* Most of the device notification types should only be used for debug.
  * SW does need to pay attention to function wake notifications.
  */
-#define	DEV_NOTE_FWAKE		ENABLE_DEV_NOTE(1)
+#define	DEV_NOTE_FWAKE		(1 << 1)
 
 /* CRCR - Command Ring Control Register - cmd_ring bitmasks */
 /* bit 0 - Cycle bit indicates the ownership of the command ring */
-- 
2.47.2

[PATCH v2 08/11] usb: xhci: remove error handling from xhci_add_interrupter()

[Niklas Neronin]

Remove redundant error handling from xhci_add_interrupter() instead of
trying to accommodate them in future changes.

======== Reasoning for the removal ========

Function xhci_add_interrupter() is invoked in two scenarios:

Primary Interrupter Setup (ID 0):
 The maximum number of interrupters is always greater than zero, and the
 primary interrupter is always allocated as part of the driver's
 initialization process. In case of failure, the xHCI driver errors and
 exits.

Secondary Interrupter Creation (ID >= 1):
 The interrupter is pre-allocated, and an empty slot is identified before
 invoking xhci_add_interrupter().

In both cases, the existing error handling within xhci_add_interrupter() is
redundant and unnecessary.

Upcoming Changes:
 In the subsequent commit, interrupter initialization will move from
 xhci_mem_init() to xhci_init(). This change is necessary to facilitate
 the ability to restart the xHCI driver without re-allocating memory.
 As a result, the allocated interrupter must be stored in the interrupters
 pointer array before initialization.

 Consequently, xhci_create_secondary_interrupter() would need to handle
 pointer removal for allocated 'interrupters' array upon failure, although
 xhci_add_interrupter() will never fail.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
v2:
 * Set 'err' to '0' in case of available interrupt index. Due to rebase.

 drivers/usb/host/xhci-mem.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 226013ee443a..75ad30d52507 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2287,24 +2287,13 @@ xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int segs, gfp_t flags)
 	return ir;
 }
 
-static int
+static void
 xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
 		     unsigned int intr_num)
 {
 	u64 erst_base;
 	u32 erst_size;
 
-	if (intr_num >= xhci->max_interrupters) {
-		xhci_warn(xhci, "Can't add interrupter %d, max interrupters %d\n",
-			  intr_num, xhci->max_interrupters);
-		return -EINVAL;
-	}
-
-	if (xhci->interrupters[intr_num]) {
-		xhci_warn(xhci, "Interrupter %d\n already set up", intr_num);
-		return -EINVAL;
-	}
-
 	xhci->interrupters[intr_num] = ir;
 	ir->intr_num = intr_num;
 	ir->ir_set = &xhci->run_regs->ir_set[intr_num];
@@ -2325,8 +2314,6 @@ xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
 
 	/* Set the event ring dequeue address of this interrupter */
 	xhci_set_hc_event_deq(xhci, ir);
-
-	return 0;
 }
 
 struct xhci_interrupter *
@@ -2351,13 +2338,16 @@ xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
 		/* Find available secondary interrupter, interrupter 0 is reserved for primary */
 		for (i = 1; i < xhci->max_interrupters; i++) {
 			if (!xhci->interrupters[i]) {
-				err = xhci_add_interrupter(xhci, ir, i);
+				xhci_add_interrupter(xhci, ir, i);
+				err = 0;
 				break;
 			}
 		}
 	} else {
-		if (!xhci->interrupters[intr_num])
-			err = xhci_add_interrupter(xhci, ir, intr_num);
+		if (!xhci->interrupters[intr_num]) {
+			xhci_add_interrupter(xhci, ir, intr_num);
+			err = 0;
+		}
 	}
 	spin_unlock_irq(&xhci->lock);
 
@@ -2461,8 +2451,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	if (!ir)
 		goto fail;
 
-	if (xhci_add_interrupter(xhci, ir, 0))
-		goto fail;
+	xhci_add_interrupter(xhci, ir, 0);
 
 	ir->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
 
-- 
2.47.2

[PATCH v2 09/11] usb: xhci: move initialization of the primary interrupter

[Niklas Neronin]

Move the primary interrupter (0) initialization from xhci_mem_init() to
xhci_init(). This change requires us to save the allocated interrupter
somewhere before initialization. Therefore, store it in the 'interrupters'
array and rework xhci_add_interrupter() to retrieve the interrupter from
the array.

This is part of the ongoing effort to separate allocation and
initialization.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
v2:
 * Add the same logic to specific 'intr_num'. Due to rebase.

 drivers/usb/host/xhci-mem.c | 22 +++++++++-------------
 drivers/usb/host/xhci.c     |  4 ++++
 drivers/usb/host/xhci.h     |  1 +
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 75ad30d52507..007536879c24 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2287,14 +2287,13 @@ xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int segs, gfp_t flags)
 	return ir;
 }
 
-static void
-xhci_add_interrupter(struct xhci_hcd *xhci, struct xhci_interrupter *ir,
-		     unsigned int intr_num)
+void xhci_add_interrupter(struct xhci_hcd *xhci, unsigned int intr_num)
 {
+	struct xhci_interrupter *ir;
 	u64 erst_base;
 	u32 erst_size;
 
-	xhci->interrupters[intr_num] = ir;
+	ir = xhci->interrupters[intr_num];
 	ir->intr_num = intr_num;
 	ir->ir_set = &xhci->run_regs->ir_set[intr_num];
 
@@ -2338,14 +2337,16 @@ xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
 		/* Find available secondary interrupter, interrupter 0 is reserved for primary */
 		for (i = 1; i < xhci->max_interrupters; i++) {
 			if (!xhci->interrupters[i]) {
-				xhci_add_interrupter(xhci, ir, i);
+				xhci->interrupters[i] = ir;
+				xhci_add_interrupter(xhci, i);
 				err = 0;
 				break;
 			}
 		}
 	} else {
 		if (!xhci->interrupters[intr_num]) {
-			xhci_add_interrupter(xhci, ir, intr_num);
+			xhci->interrupters[intr_num] = ir;
+			xhci_add_interrupter(xhci, intr_num);
 			err = 0;
 		}
 	}
@@ -2372,7 +2373,6 @@ EXPORT_SYMBOL_GPL(xhci_create_secondary_interrupter);
 
 int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 {
-	struct xhci_interrupter *ir;
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
 	dma_addr_t	dma;
 
@@ -2447,14 +2447,10 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters),
 					  flags, dev_to_node(dev));
 
-	ir = xhci_alloc_interrupter(xhci, 0, flags);
-	if (!ir)
+	xhci->interrupters[0] = xhci_alloc_interrupter(xhci, 0, flags);
+	if (!xhci->interrupters[0])
 		goto fail;
 
-	xhci_add_interrupter(xhci, ir, 0);
-
-	ir->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
-
 	if (scratchpad_alloc(xhci, flags))
 		goto fail;
 	if (xhci_setup_port_arrays(xhci, flags))
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 18d8637773cb..1cc58d10776d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -578,6 +578,10 @@ static int xhci_init(struct usb_hcd *hcd)
 	/* Set USB 3.0 device notifications for function remote wake */
 	xhci_set_dev_notifications(xhci);
 
+	/* Initialize the Primary interrupter */
+	xhci_add_interrupter(xhci, 0);
+	xhci->interrupters[0]->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
+
 	/* Initializing Compliance Mode Recovery Data If Needed */
 	if (xhci_compliance_mode_recovery_timer_quirk_check()) {
 		xhci->quirks |= XHCI_COMP_MODE_QUIRK;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index c05c3eab005b..97da9f2853f3 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1945,6 +1945,7 @@ void xhci_process_cancelled_tds(struct xhci_virt_ep *ep);
 void xhci_update_erst_dequeue(struct xhci_hcd *xhci,
 			      struct xhci_interrupter *ir,
 			      bool clear_ehb);
+void xhci_add_interrupter(struct xhci_hcd *xhci, unsigned int intr_num);
 
 /* xHCI roothub code */
 void xhci_set_link_state(struct xhci_hcd *xhci, struct xhci_port *port,
-- 
2.47.2

[PATCH v2 10/11] usb: xhci: add individual allocation checks in xhci_mem_init()

[Niklas Neronin]

Break up the existing multi-allocation checks into individual checks.
Add missing allocation check for 'xhci->interrupters'.

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

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 007536879c24..3edb231af199 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2403,11 +2403,13 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	else
 		xhci->segment_pool = dma_pool_create("xHCI ring segments", dev,
 				TRB_SEGMENT_SIZE, TRB_SEGMENT_SIZE, xhci->page_size);
+	if (!xhci->segment_pool)
+		goto fail;
 
 	/* See Table 46 and Note on Figure 55 */
 	xhci->device_pool = dma_pool_create("xHCI input/output contexts", dev,
 			2112, 64, xhci->page_size);
-	if (!xhci->segment_pool || !xhci->device_pool)
+	if (!xhci->device_pool)
 		goto fail;
 
 	/* Linear stream context arrays don't have any boundary restrictions,
@@ -2416,6 +2418,9 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	xhci->small_streams_pool =
 		dma_pool_create("xHCI 256 byte stream ctx arrays",
 			dev, SMALL_STREAM_ARRAY_SIZE, 16, 0);
+	if (!xhci->small_streams_pool)
+		goto fail;
+
 	xhci->medium_streams_pool =
 		dma_pool_create("xHCI 1KB stream ctx arrays",
 			dev, MEDIUM_STREAM_ARRAY_SIZE, 16, 0);
@@ -2423,7 +2428,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	 * will be allocated with dma_alloc_coherent()
 	 */
 
-	if (!xhci->small_streams_pool || !xhci->medium_streams_pool)
+	if (!xhci->medium_streams_pool)
 		goto fail;
 
 	/* Set up the command ring to have one segments for now. */
@@ -2446,6 +2451,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 		       "Allocating primary event ring");
 	xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters),
 					  flags, dev_to_node(dev));
+	if (!xhci->interrupters)
+		goto fail;
 
 	xhci->interrupters[0] = xhci_alloc_interrupter(xhci, 0, flags);
 	if (!xhci->interrupters[0])
-- 
2.47.2

[PATCH v2 11/11] usb: xhci: cleanup xhci_mem_init()

[Niklas Neronin]

Cleanup indentation, spacing and comment formats.

Remove the "// " prefix from trace messages, as it is unnecessary and
distracting.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 46 ++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 3edb231af199..8945329e064d 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2380,14 +2380,14 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	 * xHCI section 5.4.6 - Device Context array must be
 	 * "physically contiguous and 64-byte (cache line) aligned".
 	 */
-	xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa), &dma,
-			flags);
+	xhci->dcbaa = dma_alloc_coherent(dev, sizeof(*xhci->dcbaa), &dma, flags);
 	if (!xhci->dcbaa)
 		goto fail;
+
 	xhci->dcbaa->dma = dma;
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"// Device context base array address = 0x%pad (DMA), %p (virt)",
-			&xhci->dcbaa->dma, xhci->dcbaa);
+		       "Device context base array address = 0x%pad (DMA), %p (virt)",
+		       &xhci->dcbaa->dma, xhci->dcbaa);
 
 	/*
 	 * Initialize the ring segment pool.  The ring must be a contiguous
@@ -2407,27 +2407,26 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 		goto fail;
 
 	/* See Table 46 and Note on Figure 55 */
-	xhci->device_pool = dma_pool_create("xHCI input/output contexts", dev,
-			2112, 64, xhci->page_size);
+	xhci->device_pool = dma_pool_create("xHCI input/output contexts", dev, 2112, 64,
+					    xhci->page_size);
 	if (!xhci->device_pool)
 		goto fail;
 
-	/* Linear stream context arrays don't have any boundary restrictions,
+	/*
+	 * Linear stream context arrays don't have any boundary restrictions,
 	 * and only need to be 16-byte aligned.
 	 */
-	xhci->small_streams_pool =
-		dma_pool_create("xHCI 256 byte stream ctx arrays",
-			dev, SMALL_STREAM_ARRAY_SIZE, 16, 0);
+	xhci->small_streams_pool = dma_pool_create("xHCI 256 byte stream ctx arrays",
+						   dev, SMALL_STREAM_ARRAY_SIZE, 16, 0);
 	if (!xhci->small_streams_pool)
 		goto fail;
 
-	xhci->medium_streams_pool =
-		dma_pool_create("xHCI 1KB stream ctx arrays",
-			dev, MEDIUM_STREAM_ARRAY_SIZE, 16, 0);
-	/* Any stream context array bigger than MEDIUM_STREAM_ARRAY_SIZE
-	 * will be allocated with dma_alloc_coherent()
+	/*
+	 * Any stream context array bigger than MEDIUM_STREAM_ARRAY_SIZE will be
+	 * allocated with dma_alloc_coherent().
 	 */
-
+	xhci->medium_streams_pool = dma_pool_create("xHCI 1KB stream ctx arrays",
+						    dev, MEDIUM_STREAM_ARRAY_SIZE, 16, 0);
 	if (!xhci->medium_streams_pool)
 		goto fail;
 
@@ -2435,20 +2434,20 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	xhci->cmd_ring = xhci_ring_alloc(xhci, 1, TYPE_COMMAND, 0, flags);
 	if (!xhci->cmd_ring)
 		goto fail;
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-			"Allocated command ring at %p", xhci->cmd_ring);
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "First segment DMA is 0x%pad",
-			&xhci->cmd_ring->first_seg->dma);
 
-	/* Reserve one command ring TRB for disabling LPM.
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Allocated command ring at %p", xhci->cmd_ring);
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "First segment DMA is 0x%pad",
+		       &xhci->cmd_ring->first_seg->dma);
+
+	/*
+	 * Reserve one command ring TRB for disabling LPM.
 	 * Since the USB core grabs the shared usb_bus bandwidth mutex before
 	 * disabling LPM, we only need to reserve one TRB for all devices.
 	 */
 	xhci->cmd_ring_reserved_trbs++;
 
 	/* Allocate and set up primary interrupter 0 with an event ring. */
-	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
-		       "Allocating primary event ring");
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Allocating primary event ring");
 	xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters),
 					  flags, dev_to_node(dev));
 	if (!xhci->interrupters)
@@ -2460,6 +2459,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 
 	if (scratchpad_alloc(xhci, flags))
 		goto fail;
+
 	if (xhci_setup_port_arrays(xhci, flags))
 		goto fail;
 
-- 
2.47.2