[PATCH 00/24] xhci features for usb-next

[PATCH 00/24] xhci features for usb-next

[Mathias Nyman]

Hi Greg

A series of new xhci features and refactoring for usb next incuding
  - Debugfs support for showing available port bandwidth
  - xhci parts of eUSB2 double isoc bandwidth support
  - refactoring to decouple allocation and initialzation 
  - other misc cleanups and refactoring

-Mathias

Amardeep Rai (1):
  xhci: Add host support for eUSB2 double isochronous bandwidth devices

Michal Pecio (1):
  usb: xhci: Don't log transfer ring segment list on errors

Niklas Neronin (21):
  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()
  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'
  usb: xhci: add warning message specifying which Set TR Deq failed

Xu Rao (1):
  usb: xhci: Add debugfs support for xHCI port bandwidth

 drivers/usb/host/xhci-caps.h    |   6 +-
 drivers/usb/host/xhci-debugfs.c | 108 ++++++++++++
 drivers/usb/host/xhci-hub.c     |   2 +-
 drivers/usb/host/xhci-mem.c     | 282 ++++++++++++++++----------------
 drivers/usb/host/xhci-ring.c    |  40 +++--
 drivers/usb/host/xhci.c         | 222 ++++++++++++++++++++-----
 drivers/usb/host/xhci.h         | 119 +++++++++-----
 7 files changed, 543 insertions(+), 236 deletions(-)

-- 
2.43.0

[PATCH 01/24] usb: xhci: Don't log transfer ring segment list on errors

[Mathias Nyman]

From: Michal Pecio <michal.pecio@gmail.com>

The error message above used to span two lines, rarely more. A recent
cleanup concentrated useful information from it in one line, but then
it added printing the list of all ring segments, which is even longer
than before. It provides no new information in usual cases and little
in unusual ones, but adds noise to the log. Drop it.

Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d4e157e66473..e70bf7a27556 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2978,9 +2978,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 		 (unsigned long long)xhci_trb_virt_to_dma(td->start_seg, td->start_trb),
 		 (unsigned long long)xhci_trb_virt_to_dma(td->end_seg, td->end_trb));
 
-	xhci_for_each_ring_seg(ep_ring->first_seg, ep_seg)
-		xhci_warn(xhci, "Ring seg %u dma %pad\n", ep_seg->num, &ep_seg->dma);
-
 	return -ESHUTDOWN;
 
 err_out:
-- 
2.43.0

[PATCH 02/24] usb: xhci: Add debugfs support for xHCI port bandwidth

[Mathias Nyman]

From: Xu Rao <raoxu@uniontech.com>

In many projects, you need to obtain the available bandwidth of the
xhci roothub port. Refer to xhci rev1_2 and use the TRB_GET_BW
command to obtain it.

hardware tested:
03:00.3 USB controller: Advanced Micro Devices, Inc. [AMD] Raven USB 3.1
(prog-if 30 [XHCI])
Subsystem: Huawei Technologies Co., Ltd. Raven USB 3.1
Flags: bus master, fast devsel, latency 0, IRQ 30
Memory at c0300000 (64-bit, non-prefetchable) [size=1M]
Capabilities: [48] Vendor Specific Information: Len=08 <?>
Capabilities: [50] Power Management version 3
Capabilities: [64] Express Endpoint, MSI 00
Capabilities: [a0] MSI: Enable- Count=1/8 Maskable- 64bit+
Capabilities: [c0] MSI-X: Enable+ Count=8 Masked-
Kernel driver in use: xhci_hcd

test progress:
1. cd /sys/kernel/debug/usb/xhci/0000:03:00.3/port_bandwidth# ls
FS_BW  HS_BW  SS_BW
2. test fs speed  device
cat FS_BW
port[1] available bw: 90%.
port[2] available bw: 90%.
port[3] available bw: 90%.
port[4] available bw: 90%.
port[5] available bw: 0%.
port[6] available bw: 0%.
port[7] available bw: 0%.
port[8] available bw: 0%.
plug in fs usb audio ID 0d8c:013c
cat FS_BW
port[1] available bw: 76%.
port[2] available bw: 76%.
port[3] available bw: 76%.
port[4] available bw: 76%.
port[5] available bw: 0%.
port[6] available bw: 0%.
port[7] available bw: 0%.
port[8] available bw: 0%.
3. test hs speed device
cat HS_BW
port[1] available bw: 79%.
port[2] available bw: 79%.
port[3] available bw: 79%.
port[4] available bw: 79%.
port[5] available bw: 0%.
port[6] available bw: 0%.
port[7] available bw: 0%.
port[8] available bw: 0%.
plug in hs usb video ID 0408:1040
cat HS_BW
port[1] available bw: 39%.
port[2] available bw: 39%.
port[3] available bw: 39%.
port[4] available bw: 39%.
port[5] available bw: 0%.
port[6] available bw: 0%.
port[7] available bw: 0%.
port[8] available bw: 0%.
4.cat SS_BW
port[1] available bw: 0%.
port[2] available bw: 0%.
port[3] available bw: 0%.
port[4] available bw: 0%.
port[5] available bw: 90%.
port[6] available bw: 90%.
port[7] available bw: 90%.
port[8] available bw: 90%.

Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-debugfs.c | 108 ++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci-mem.c     |  45 ++++++++++++-
 drivers/usb/host/xhci-ring.c    |  13 ++++
 drivers/usb/host/xhci.c         |  36 +++++++++++
 drivers/usb/host/xhci.h         |  14 +++++
 5 files changed, 215 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 1f5ef174abea..c6d44977193f 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -631,6 +631,112 @@ static void xhci_debugfs_create_ports(struct xhci_hcd *xhci,
 	}
 }
 
+static int xhci_port_bw_show(struct xhci_hcd *xhci, u8 dev_speed,
+				struct seq_file *s)
+{
+	unsigned int			num_ports;
+	unsigned int			i;
+	int				ret;
+	struct xhci_container_ctx	*ctx;
+	struct usb_hcd			*hcd = xhci_to_hcd(xhci);
+	struct device			*dev = hcd->self.controller;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		return ret;
+
+	num_ports = HCS_MAX_PORTS(xhci->hcs_params1);
+
+	ctx = xhci_alloc_port_bw_ctx(xhci, 0);
+	if (!ctx) {
+		pm_runtime_put_sync(dev);
+		return -ENOMEM;
+	}
+
+	/* get roothub port bandwidth */
+	ret = xhci_get_port_bandwidth(xhci, ctx, dev_speed);
+	if (ret)
+		goto err_out;
+
+	/* print all roothub ports available bandwidth
+	 * refer to xhci rev1_2 protocol 6.2.6 , byte 0 is reserved
+	 */
+	for (i = 1; i < num_ports+1; i++)
+		seq_printf(s, "port[%d] available bw: %d%%.\n", i,
+				ctx->bytes[i]);
+err_out:
+	pm_runtime_put_sync(dev);
+	xhci_free_port_bw_ctx(xhci, ctx);
+	return ret;
+}
+
+static int xhci_ss_bw_show(struct seq_file *s, void *unused)
+{
+	int ret;
+	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
+
+	ret = xhci_port_bw_show(xhci, USB_SPEED_SUPER, s);
+	return ret;
+}
+
+static int xhci_hs_bw_show(struct seq_file *s, void *unused)
+{
+	int ret;
+	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
+
+	ret = xhci_port_bw_show(xhci, USB_SPEED_HIGH, s);
+	return ret;
+}
+
+static int xhci_fs_bw_show(struct seq_file *s, void *unused)
+{
+	int ret;
+	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
+
+	ret = xhci_port_bw_show(xhci, USB_SPEED_FULL, s);
+	return ret;
+}
+
+static struct xhci_file_map bw_context_files[] = {
+	{"SS_BW",	xhci_ss_bw_show, },
+	{"HS_BW",	xhci_hs_bw_show, },
+	{"FS_BW",	xhci_fs_bw_show, },
+};
+
+static int bw_context_open(struct inode *inode, struct file *file)
+{
+	int			i;
+	struct xhci_file_map	*f_map;
+	const char		*file_name = file_dentry(file)->d_iname;
+
+	for (i = 0; i < ARRAY_SIZE(bw_context_files); i++) {
+		f_map = &bw_context_files[i];
+
+		if (strcmp(f_map->name, file_name) == 0)
+			break;
+	}
+
+	return single_open(file, f_map->show, inode->i_private);
+}
+
+static const struct file_operations bw_fops = {
+	.open			= bw_context_open,
+	.read			= seq_read,
+	.llseek			= seq_lseek,
+	.release		= single_release,
+};
+
+static void xhci_debugfs_create_bandwidth(struct xhci_hcd *xhci,
+					struct dentry *parent)
+{
+	parent = debugfs_create_dir("port_bandwidth", parent);
+
+	xhci_debugfs_create_files(xhci, bw_context_files,
+			  ARRAY_SIZE(bw_context_files),
+			  xhci,
+			  parent, &bw_fops);
+}
+
 void xhci_debugfs_init(struct xhci_hcd *xhci)
 {
 	struct device		*dev = xhci_to_hcd(xhci)->self.controller;
@@ -681,6 +787,8 @@ void xhci_debugfs_init(struct xhci_hcd *xhci)
 	xhci->debugfs_slots = debugfs_create_dir("devices", xhci->debugfs_root);
 
 	xhci_debugfs_create_ports(xhci, xhci->debugfs_root);
+
+	xhci_debugfs_create_bandwidth(xhci, xhci->debugfs_root);
 }
 
 void xhci_debugfs_exit(struct xhci_hcd *xhci)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index ed36df46b140..7ff6a47d3198 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -484,6 +484,35 @@ void xhci_free_container_ctx(struct xhci_hcd *xhci,
 	kfree(ctx);
 }
 
+struct xhci_container_ctx *xhci_alloc_port_bw_ctx(struct xhci_hcd *xhci,
+						  gfp_t flags)
+{
+	struct xhci_container_ctx *ctx;
+	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
+
+	ctx = kzalloc_node(sizeof(*ctx), flags, dev_to_node(dev));
+	if (!ctx)
+		return NULL;
+
+	ctx->size = GET_PORT_BW_ARRAY_SIZE;
+
+	ctx->bytes = dma_pool_zalloc(xhci->port_bw_pool, flags, &ctx->dma);
+	if (!ctx->bytes) {
+		kfree(ctx);
+		return NULL;
+	}
+	return ctx;
+}
+
+void xhci_free_port_bw_ctx(struct xhci_hcd *xhci,
+			     struct xhci_container_ctx *ctx)
+{
+	if (!ctx)
+		return;
+	dma_pool_free(xhci->port_bw_pool, ctx->bytes, ctx->dma);
+	kfree(ctx);
+}
+
 struct xhci_input_control_ctx *xhci_get_input_control_ctx(
 					      struct xhci_container_ctx *ctx)
 {
@@ -1912,6 +1941,11 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
 			"Freed small stream array pool");
 
+	dma_pool_destroy(xhci->port_bw_pool);
+	xhci->port_bw_pool = NULL;
+	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
+			"Freed xhci port bw array pool");
+
 	dma_pool_destroy(xhci->medium_streams_pool);
 	xhci->medium_streams_pool = NULL;
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init,
@@ -2475,7 +2509,16 @@ 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)
+	/* refer to xhci rev1_2 protocol 5.3.3 max ports is 255.
+	 * refer to xhci rev1_2 protocol 6.4.3.14 port bandwidth buffer need
+	 * to be 16-byte aligned.
+	 */
+	xhci->port_bw_pool =
+		dma_pool_create("xHCI 256 port bw ctx arrays",
+			dev, GET_PORT_BW_ARRAY_SIZE, 16, 0);
+
+	if (!xhci->small_streams_pool || !xhci->medium_streams_pool ||
+		!xhci->port_bw_pool)
 		goto fail;
 
 	/* Set up the command ring to have one segments for now. */
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e70bf7a27556..9607f75b8d2a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1899,6 +1899,8 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
 	case TRB_NEC_GET_FW:
 		xhci_handle_cmd_nec_get_fw(xhci, event);
 		break;
+	case TRB_GET_BW:
+		break;
 	default:
 		/* Skip over unknown commands on the event ring */
 		xhci_info(xhci, "INFO unknown command type %d\n", cmd_type);
@@ -4445,6 +4447,17 @@ int xhci_queue_configure_endpoint(struct xhci_hcd *xhci,
 			command_must_succeed);
 }
 
+/* Queue a get root hub port bandwidth command TRB */
+int xhci_queue_get_port_bw(struct xhci_hcd *xhci,
+		struct xhci_command *cmd, dma_addr_t in_ctx_ptr,
+		u8 dev_speed, bool command_must_succeed)
+{
+	return queue_command(xhci, cmd, lower_32_bits(in_ctx_ptr),
+		upper_32_bits(in_ctx_ptr), 0,
+		TRB_TYPE(TRB_GET_BW) | DEV_SPEED_FOR_TRB(dev_speed),
+		command_must_succeed);
+}
+
 /* Queue an evaluate context command TRB */
 int xhci_queue_evaluate_context(struct xhci_hcd *xhci, struct xhci_command *cmd,
 		dma_addr_t in_ctx_ptr, u32 slot_id, bool command_must_succeed)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b6267fc37b08..fd9d41fe3224 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3090,6 +3090,42 @@ void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
 }
 EXPORT_SYMBOL_GPL(xhci_reset_bandwidth);
 
+/* Get the available bandwidth of the ports under the xhci roothub */
+int xhci_get_port_bandwidth(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
+			    u8 dev_speed)
+{
+	struct xhci_command *cmd;
+	unsigned long flags;
+	int ret;
+
+	if (!ctx || !xhci)
+		return -EINVAL;
+
+	cmd = xhci_alloc_command(xhci, true, GFP_KERNEL);
+	if (!cmd)
+		return -ENOMEM;
+
+	cmd->in_ctx = ctx;
+
+	/* get xhci port bandwidth, refer to xhci rev1_2 protocol 4.6.15 */
+	spin_lock_irqsave(&xhci->lock, flags);
+
+	ret = xhci_queue_get_port_bw(xhci, cmd, ctx->dma, dev_speed, 0);
+	if (ret) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		goto err_out;
+	}
+	xhci_ring_cmd_db(xhci);
+	spin_unlock_irqrestore(&xhci->lock, flags);
+
+	wait_for_completion(cmd->completion);
+err_out:
+	kfree(cmd->completion);
+	kfree(cmd);
+
+	return ret;
+}
+
 static void xhci_setup_input_ctx_for_config_ep(struct xhci_hcd *xhci,
 		struct xhci_container_ctx *in_ctx,
 		struct xhci_container_ctx *out_ctx,
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index df9ed8a74af6..f8198ec02981 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -589,6 +589,7 @@ struct xhci_stream_info {
 
 #define	SMALL_STREAM_ARRAY_SIZE		256
 #define	MEDIUM_STREAM_ARRAY_SIZE	1024
+#define	GET_PORT_BW_ARRAY_SIZE		256
 
 /* Some Intel xHCI host controllers need software to keep track of the bus
  * bandwidth.  Keep track of endpoint info here.  Each root port is allocated
@@ -1006,6 +1007,9 @@ enum xhci_setup_dev {
 /* bits 16:23 are the virtual function ID */
 /* bits 24:31 are the slot ID */
 
+/* bits 19:16 are the dev speed */
+#define DEV_SPEED_FOR_TRB(p)    ((p) << 16)
+
 /* Stop Endpoint TRB - ep_index to endpoint ID for this TRB */
 #define SUSPEND_PORT_FOR_TRB(p)		(((p) & 1) << 23)
 #define TRB_TO_SUSPEND_PORT(p)		(((p) & (1 << 23)) >> 23)
@@ -1558,6 +1562,7 @@ struct xhci_hcd {
 	struct dma_pool	*device_pool;
 	struct dma_pool	*segment_pool;
 	struct dma_pool	*small_streams_pool;
+	struct dma_pool	*port_bw_pool;
 	struct dma_pool	*medium_streams_pool;
 
 	/* Host controller watchdog timer structures */
@@ -1850,6 +1855,10 @@ struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
 		int type, gfp_t flags);
 void xhci_free_container_ctx(struct xhci_hcd *xhci,
 		struct xhci_container_ctx *ctx);
+struct xhci_container_ctx *xhci_alloc_port_bw_ctx(struct xhci_hcd *xhci,
+		gfp_t flags);
+void xhci_free_port_bw_ctx(struct xhci_hcd *xhci,
+		struct xhci_container_ctx *ctx);
 struct xhci_interrupter *
 xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
 				  u32 imod_interval, unsigned int intr_num);
@@ -1923,6 +1932,11 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
 int xhci_queue_configure_endpoint(struct xhci_hcd *xhci,
 		struct xhci_command *cmd, dma_addr_t in_ctx_ptr, u32 slot_id,
 		bool command_must_succeed);
+int xhci_queue_get_port_bw(struct xhci_hcd *xhci,
+		struct xhci_command *cmd, dma_addr_t in_ctx_ptr,
+		u8 dev_speed, bool command_must_succeed);
+int xhci_get_port_bandwidth(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
+		u8 dev_speed);
 int xhci_queue_evaluate_context(struct xhci_hcd *xhci, struct xhci_command *cmd,
 		dma_addr_t in_ctx_ptr, u32 slot_id, bool command_must_succeed);
 int xhci_queue_reset_ep(struct xhci_hcd *xhci, struct xhci_command *cmd,
-- 
2.43.0

[PATCH 03/24] usb: xhci: relocate pre-allocation initialization

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 7ff6a47d3198..a7955e02905c 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2414,22 +2414,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;
@@ -2438,15 +2422,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
@@ -2567,9 +2542,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 fd9d41fe3224..b073e9d91665 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.43.0

[PATCH 04/24] usb: xhci: move device slot enabling register write

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 a7955e02905c..e03109a24a50 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2419,23 +2419,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 b073e9d91665..ec0a2fa7d003 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.43.0

[PATCH 05/24] usb: xhci: move command ring pointer write

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 e03109a24a50..c4b94f7bacfb 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2420,7 +2420,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;
 
 	/*
@@ -2492,15 +2491,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 ec0a2fa7d003..66a9106d8b31 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.43.0

[PATCH 06/24] usb: xhci: refactor xhci_set_cmd_ring_deq()

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 28 +++++++++++++++-------------
 drivers/usb/host/xhci.h |  8 ++++----
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 66a9106d8b31..4c9174c5c7c7 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;
-
-	/* 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);
+	dma_addr_t deq_dma;
+	u64 crcr;
+
+	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 f8198ec02981..6c1758f8fd01 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.43.0

[PATCH 07/24] usb: xhci: move DCBAA pointer write

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 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 c4b94f7bacfb..ac96f0155cab 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2434,7 +2434,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 4c9174c5c7c7..e8c262865188 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.43.0

[PATCH 08/24] usb: xhci: move doorbell array pointer assignment

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 ac96f0155cab..2f4dbb67b1bf 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2419,7 +2419,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;
 
 	/*
@@ -2496,13 +2495,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 e8c262865188..0639d8b7372b 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.43.0

[PATCH 09/24] usb: xhci: move enabling of USB 3 device notifications

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 2f4dbb67b1bf..718354bd7fb2 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2419,7 +2419,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
@@ -2515,15 +2514,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 0639d8b7372b..fa80cc30c3fe 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 6c1758f8fd01..31d945c4ac07 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.43.0

[PATCH 10/24] usb: xhci: remove error handling from xhci_add_interrupter()

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 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 718354bd7fb2..bfb01c432b23 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2321,24 +2321,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];
@@ -2359,8 +2348,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 *
@@ -2385,13 +2372,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);
 
@@ -2504,8 +2494,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.43.0

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

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 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 bfb01c432b23..eb076f5ed1d0 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2321,14 +2321,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];
 
@@ -2372,14 +2371,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;
 		}
 	}
@@ -2406,7 +2407,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;
 
@@ -2490,14 +2490,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 fa80cc30c3fe..c6b517401c94 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 31d945c4ac07..b0b16cd7df91 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1959,6 +1959,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.43.0

[PATCH 12/24] usb: xhci: add individual allocation checks in xhci_mem_init()

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index eb076f5ed1d0..8cadd785ac0e 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2437,11 +2437,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,
@@ -2450,12 +2452,17 @@ 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);
 	/* Any stream context array bigger than MEDIUM_STREAM_ARRAY_SIZE
 	 * will be allocated with dma_alloc_coherent()
 	 */
+	if (!xhci->medium_streams_pool)
+		goto fail;
 
 	/* refer to xhci rev1_2 protocol 5.3.3 max ports is 255.
 	 * refer to xhci rev1_2 protocol 6.4.3.14 port bandwidth buffer need
@@ -2464,9 +2471,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
 	xhci->port_bw_pool =
 		dma_pool_create("xHCI 256 port bw ctx arrays",
 			dev, GET_PORT_BW_ARRAY_SIZE, 16, 0);
-
-	if (!xhci->small_streams_pool || !xhci->medium_streams_pool ||
-		!xhci->port_bw_pool)
+	if (!xhci->port_bw_pool)
 		goto fail;
 
 	/* Set up the command ring to have one segments for now. */
@@ -2489,6 +2494,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.43.0

[PATCH 13/24] usb: xhci: cleanup xhci_mem_init()

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-mem.c | 52 +++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 8cadd785ac0e..08513e5d321a 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2414,14 +2414,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
@@ -2441,36 +2441,37 @@ 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;
 
-	/* refer to xhci rev1_2 protocol 5.3.3 max ports is 255.
+	/*
+	 * refer to xhci rev1_2 protocol 5.3.3 max ports is 255.
 	 * refer to xhci rev1_2 protocol 6.4.3.14 port bandwidth buffer need
 	 * to be 16-byte aligned.
 	 */
-	xhci->port_bw_pool =
-		dma_pool_create("xHCI 256 port bw ctx arrays",
-			dev, GET_PORT_BW_ARRAY_SIZE, 16, 0);
+	xhci->port_bw_pool = dma_pool_create("xHCI 256 port bw ctx arrays",
+					     dev, GET_PORT_BW_ARRAY_SIZE, 16, 0);
 	if (!xhci->port_bw_pool)
 		goto fail;
 
@@ -2478,20 +2479,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, "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);
+		       &xhci->cmd_ring->first_seg->dma);
 
-	/* Reserve one command ring TRB for disabling LPM.
+	/*
+	 * 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)
@@ -2503,6 +2504,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.43.0

[PATCH 14/24] usb: xhci: set requested IMODI to the closest supported value

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 08513e5d321a..dcfe7774e9ed 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2393,10 +2393,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",
 		 ir->intr_num, xhci->max_interrupters);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c6b517401c94..c3a1a67b6563 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -355,12 +355,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.43.0

[PATCH 15/24] usb: xhci: improve Interrupt Management register macros

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 c3a1a67b6563..472589679af3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -331,7 +331,9 @@ 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;
 }
@@ -344,7 +346,9 @@ 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 b0b16cd7df91..28c4ad7534c1 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
@@ -240,12 +236,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.43.0

[PATCH 16/24] usb: xhci: guarantee that IMAN register is flushed

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 9607f75b8d2a..91a9ad687704 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3088,6 +3088,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 472589679af3..8cdb1a01a3ed 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -335,6 +335,8 @@ 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;
 }
 
@@ -350,6 +352,7 @@ 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.43.0

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

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
---
 drivers/usb/host/xhci-hub.c  |  2 +-
 drivers/usb/host/xhci-ring.c |  2 +-
 drivers/usb/host/xhci.c      | 13 +++++++------
 drivers/usb/host/xhci.h      |  2 +-
 4 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 486347776cb2..92bb84f8132a 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -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;
 		}
 	}
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 91a9ad687704..9efa0d1735df 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3167,7 +3167,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);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8cdb1a01a3ed..6c4bbabc3a70 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -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);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 28c4ad7534c1..fc6b97add7fa 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -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);
-- 
2.43.0

[PATCH 18/24] usb: xhci: rework Event Ring Segment Table Size mask

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 dcfe7774e9ed..ec2c4851c689 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1831,7 +1831,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_update_erst_dequeue(xhci, ir, true);
@@ -2333,7 +2333,7 @@ void xhci_add_interrupter(struct xhci_hcd *xhci, unsigned int intr_num)
 
 	/* 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 fc6b97add7fa..19dd47d76140 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -251,8 +251,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.43.0

[PATCH 19/24] usb: xhci: rework Event Ring Segment Table Address mask

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 ec2c4851c689..bd745a0f2f78 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2338,8 +2338,8 @@ void xhci_add_interrupter(struct xhci_hcd *xhci, unsigned int intr_num)
 	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 19dd47d76140..7865e21f0b1f 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -255,7 +255,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.43.0

[PATCH 20/24] usb: xhci: cleanup IMOD register comments

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 7865e21f0b1f..4a4ce6784bf0 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -210,14 +210,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
@@ -242,12 +241,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 */
@@ -259,15 +259,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.43.0

[PATCH 21/24] usb: xhci: rename 'irq_pending' to 'iman'

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 9efa0d1735df..e3c823e1caee 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3083,14 +3083,14 @@ 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 6c4bbabc3a70..3450762fc7bd 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -330,12 +330,12 @@ 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;
 }
 
@@ -346,11 +346,11 @@ int xhci_disable_interrupter(struct xhci_hcd *xhci, 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);
 
-	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__);
 
@@ -834,7 +834,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);
 	}
 }
@@ -858,7 +858,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 4a4ce6784bf0..62d12d23617f 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -211,7 +211,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).
@@ -226,7 +226,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;
@@ -234,7 +234,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 */
@@ -1452,7 +1452,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.43.0

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

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

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>
Signed-off-by: Mathias Nyman <mathias.nyman@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 3450762fc7bd..9769c68b2e9f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -367,12 +367,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;
 }
@@ -835,7 +835,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);
 	}
 }
 
@@ -859,7 +859,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 62d12d23617f..49887a303e43 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -213,7 +213,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.
@@ -227,7 +227,7 @@ struct xhci_op_regs {
  */
 struct xhci_intr_reg {
 	__le32	iman;
-	__le32	irq_control;
+	__le32	imod;
 	__le32	erst_size;
 	__le32	rsvd;
 	__le64	erst_base;
@@ -240,15 +240,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 */
@@ -1453,7 +1453,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.43.0

[PATCH 23/24] usb: xhci: add warning message specifying which Set TR Deq failed

[Mathias Nyman]

From: Niklas Neronin <niklas.neronin@linux.intel.com>

Currently, errors from the Set TR Deq command are not handled.
Add a warning message to specify the slot, endpoint, and TRB address when
a Set TR Deq command fails. This additional information will aid in
debugging such failures.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ring.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e3c823e1caee..eff6b84dc915 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1448,6 +1448,11 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd *xhci, int slot_id,
 		unsigned int ep_state;
 		unsigned int slot_state;
 
+		xhci_warn(xhci, "Set TR Deq error for TRB 0x%llx in slot %d ep %u\n",
+			  (unsigned long long)xhci_trb_virt_to_dma(ep->queued_deq_seg,
+								   ep->queued_deq_ptr),
+			  slot_id, ep_index);
+
 		switch (cmd_comp_code) {
 		case COMP_TRB_ERROR:
 			xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid because of stream ID configuration\n");
-- 
2.43.0

Re: [PATCH 23/24] usb: xhci: add warning message specifying which Set TR Deq failed

[Michał Pecio]

On Thu, 15 May 2025 16:56:20 +0300, Mathias Nyman wrote:
> From: Niklas Neronin <niklas.neronin@linux.intel.com>
> 
> Currently, errors from the Set TR Deq command are not handled.
> Add a warning message to specify the slot, endpoint, and TRB address
> when a Set TR Deq command fails. This additional information will aid
> in debugging such failures.
> 
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci-ring.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-ring.c
> b/drivers/usb/host/xhci-ring.c index e3c823e1caee..eff6b84dc915 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -1448,6 +1448,11 @@ static void xhci_handle_cmd_set_deq(struct
> xhci_hcd *xhci, int slot_id, unsigned int ep_state;
>  		unsigned int slot_state;
>  
> +		xhci_warn(xhci, "Set TR Deq error for TRB 0x%llx in slot %d ep %u\n",
> +			  (unsigned long long)xhci_trb_virt_to_dma(ep->queued_deq_seg,
> +						ep->queued_deq_ptr),

Is printing this pointer actually useful? It doesn't tell why
the error happened. It will not relate to other messages - the
command failed, so dequeue stays at its old position.

> +			  slot_id, ep_index);
> +
>  		switch (cmd_comp_code) {
>  		case COMP_TRB_ERROR:
>  			xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid because of stream ID configuration\n");

This will now become a multi-line log message, even though actual
information it contains could fit in one line.

How about replacing this new line and the whole switch with:

+               ep_state = GET_EP_CTX_STATE(ep_ctx);
+               slot_state = GET_SLOT_STATE(le32_to_cpu(slot_ctx->dev_state));
+
+               xhci_warn(xhci, "Set TR Dequeue %s for slot %d ep %d (%s/%s)\n",
+                               xhci_trb_comp_code_string(cmd_comp_code), slot_id, ep_index,
+                               xhci_slot_state_string(slot_state), xhci_ep_state_string(ep_state));

Example output:
xhci_hcd 0000:02:00.0: Set TR Dequeue TRB Error for slot 1 ep 6 (configured/stopped)

IMO this has the further benefit that "TRB Error" is something I can
search in the spec, while "because of stream ID configuration" is not.
And it isn't even the true treason in every case of TRB Error.

Thanks,
Michal

Re: [PATCH 23/24] usb: xhci: add warning message specifying which Set TR Deq failed

[Neronin, Niklas]

On 16/05/2025 15.43, Michał Pecio wrote:
> On Thu, 15 May 2025 16:56:20 +0300, Mathias Nyman wrote:
>> From: Niklas Neronin <niklas.neronin@linux.intel.com>
>>
>> Currently, errors from the Set TR Deq command are not handled.
>> Add a warning message to specify the slot, endpoint, and TRB address
>> when a Set TR Deq command fails. This additional information will aid
>> in debugging such failures.
>>
>> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>>  drivers/usb/host/xhci-ring.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c
>> b/drivers/usb/host/xhci-ring.c index e3c823e1caee..eff6b84dc915 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -1448,6 +1448,11 @@ static void xhci_handle_cmd_set_deq(struct
>> xhci_hcd *xhci, int slot_id, unsigned int ep_state;
>>  		unsigned int slot_state;
>>  
>> +		xhci_warn(xhci, "Set TR Deq error for TRB 0x%llx in slot %d ep %u\n",
>> +			  (unsigned long long)xhci_trb_virt_to_dma(ep->queued_deq_seg,
>> +						ep->queued_deq_ptr),
> 
> Is printing this pointer actually useful? It doesn't tell why
> the error happened. It will not relate to other messages - the
> command failed, so dequeue stays at its old position.
> 

Apologies, I should have retracted this patch as part of the Set TR Deq series.

I still plan on trying to add the "Set TR Deq Error Handling" series in the future,
with a few tweaks (infinite re-try loop). Then each case will have its own
tailored warning, while this warning will print general error info.

Best Regards,
Niklas

Re: [PATCH 23/24] usb: xhci: add warning message specifying which Set TR Deq failed

[Mathias Nyman]

On 16.5.2025 15.43, Michał Pecio wrote:
> On Thu, 15 May 2025 16:56:20 +0300, Mathias Nyman wrote:
>> From: Niklas Neronin <niklas.neronin@linux.intel.com>
>>
>> Currently, errors from the Set TR Deq command are not handled.
>> Add a warning message to specify the slot, endpoint, and TRB address
>> when a Set TR Deq command fails. This additional information will aid
>> in debugging such failures.
>>
>> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>> ---
>>   drivers/usb/host/xhci-ring.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-ring.c
>> b/drivers/usb/host/xhci-ring.c index e3c823e1caee..eff6b84dc915 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -1448,6 +1448,11 @@ static void xhci_handle_cmd_set_deq(struct
>> xhci_hcd *xhci, int slot_id, unsigned int ep_state;
>>   		unsigned int slot_state;
>>   
>> +		xhci_warn(xhci, "Set TR Deq error for TRB 0x%llx in slot %d ep %u\n",
>> +			  (unsigned long long)xhci_trb_virt_to_dma(ep->queued_deq_seg,
>> +						ep->queued_deq_ptr),
> 
> Is printing this pointer actually useful? It doesn't tell why
> the error happened. It will not relate to other messages - the
> command failed, so dequeue stays at its old position.
  
Printing the DMA address has turned out to be quite useful, quickly shows corner
cases like end or beginning of ring segment, or address missing upper 32 bits.

Comparable with the "trb not in TD" error messages


> 
>> +			  slot_id, ep_index);
>> +
>>   		switch (cmd_comp_code) {
>>   		case COMP_TRB_ERROR:
>>   			xhci_warn(xhci, "WARN Set TR Deq Ptr cmd invalid because of stream ID configuration\n");
> 
> This will now become a multi-line log message, even though actual
> information it contains could fit in one line.
> 
> How about replacing this new line and the whole switch with:
> 
> +               ep_state = GET_EP_CTX_STATE(ep_ctx);
> +               slot_state = GET_SLOT_STATE(le32_to_cpu(slot_ctx->dev_state));
> +
> +               xhci_warn(xhci, "Set TR Dequeue %s for slot %d ep %d (%s/%s)\n",
> +                               xhci_trb_comp_code_string(cmd_comp_code), slot_id, ep_index,
> +                               xhci_slot_state_string(slot_state), xhci_ep_state_string(ep_state));
> 
> Example output:
> xhci_hcd 0000:02:00.0: Set TR Dequeue TRB Error for slot 1 ep 6 (configured/stopped)
> 
> IMO this has the further benefit that "TRB Error" is something I can
> search in the spec, while "because of stream ID configuration" is not.
> And it isn't even the true treason in every case of TRB Error.

This works as well, but tuning this message is something we can do in a later fixup.
I don't think it's worth resending the series due to this

Thanks
Mathias

Re: [PATCH 23/24] usb: xhci: add warning message specifying which Set TR Deq failed

[Greg KH]

On Mon, May 19, 2025 at 05:33:41PM +0300, Mathias Nyman wrote:
> On 16.5.2025 15.43, Michał Pecio wrote:
> > On Thu, 15 May 2025 16:56:20 +0300, Mathias Nyman wrote:
> > > From: Niklas Neronin <niklas.neronin@linux.intel.com>
> > > 
> > > Currently, errors from the Set TR Deq command are not handled.
> > > Add a warning message to specify the slot, endpoint, and TRB address
> > > when a Set TR Deq command fails. This additional information will aid
> > > in debugging such failures.
> > > 
> > > Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> > > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > ---
> > >   drivers/usb/host/xhci-ring.c | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/usb/host/xhci-ring.c
> > > b/drivers/usb/host/xhci-ring.c index e3c823e1caee..eff6b84dc915 100644
> > > --- a/drivers/usb/host/xhci-ring.c
> > > +++ b/drivers/usb/host/xhci-ring.c
> > > @@ -1448,6 +1448,11 @@ static void xhci_handle_cmd_set_deq(struct
> > > xhci_hcd *xhci, int slot_id, unsigned int ep_state;
> > >   		unsigned int slot_state;
> > > +		xhci_warn(xhci, "Set TR Deq error for TRB 0x%llx in slot %d ep %u\n",
> > > +			  (unsigned long long)xhci_trb_virt_to_dma(ep->queued_deq_seg,
> > > +						ep->queued_deq_ptr),
> > 
> > Is printing this pointer actually useful? It doesn't tell why
> > the error happened. It will not relate to other messages - the
> > command failed, so dequeue stays at its old position.
> Printing the DMA address has turned out to be quite useful, quickly shows corner
> cases like end or beginning of ring segment, or address missing upper 32 bits.

Printing kernel addresses is not allowed.  If you really want to do
this, use the proper printk flag for it (%p I think).

> Comparable with the "trb not in TD" error messages

Should that be fixed too?  Again, don't print kernel addresses please,
people consider that an information leak.

thanks,

greg k-h

Re: [PATCH 23/24] usb: xhci: add warning message specifying which Set TR Deq failed

[Mathias Nyman]

>>>
>>> Is printing this pointer actually useful? It doesn't tell why
>>> the error happened. It will not relate to other messages - the
>>> command failed, so dequeue stays at its old position.
>> Printing the DMA address has turned out to be quite useful, quickly shows corner
>> cases like end or beginning of ring segment, or address missing upper 32 bits.
> 
> Printing kernel addresses is not allowed.  If you really want to do
> this, use the proper printk flag for it (%p I think).

These are all DMA bus addresses used by the device.
Wasn't aware those are bad as well

> 
>> Comparable with the "trb not in TD" error messages
> 
> Should that be fixed too?  Again, don't print kernel addresses please,
> people consider that an information leak.

Yes, there are several places that print DMA addresses using  %llx in the xhci driver
We can change those to %pad, or some index

Thanks
Mathias

[PATCH 24/24] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Mathias Nyman]

From: Amardeep Rai <amardeep.rai@intel.com>

Detect eUSB2 double isoc bw capable hosts and devices, and set the proper
xhci endpoint context values such as 'Mult', 'Max Burst Size', and 'Max
ESIT Payload' to enable the double isochronous bandwidth endpoints.

Intel xHC uses the endpoint context 'Mult' field for eUSB2 isoc
endpoints even if hosts supporting Large ESIT Payload Capability should
normally ignore the mult field.

Signed-off-by: Amardeep Rai <amardeep.rai@intel.com>
Co-developed-by: Kannappan R <r.kannappan@intel.com>
Signed-off-by: Kannappan R <r.kannappan@intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-caps.h |  2 ++
 drivers/usb/host/xhci-mem.c  | 62 ++++++++++++++++++++++++++++--------
 drivers/usb/host/xhci-ring.c |  6 ++--
 drivers/usb/host/xhci.c      | 17 +++++++++-
 drivers/usb/host/xhci.h      | 19 +++++++++++
 5 files changed, 89 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index 4b8ff4815644..723a56052439 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -89,3 +89,5 @@
 #define HCC2_GSC(p)             ((p) & (1 << 8))
 /* true: HC support Virtualization Based Trusted I/O Capability */
 #define HCC2_VTC(p)             ((p) & (1 << 9))
+/* true: HC support Double BW on a eUSB2 HS ISOC EP */
+#define HCC2_EUSB2_DIC(p)	((p) & (1 << 11))
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index bd745a0f2f78..494f9eacab84 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1328,18 +1328,36 @@ static unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
 	return interval;
 }
 
-/* The "Mult" field in the endpoint context is only set for SuperSpeed isoc eps.
+/*
+ * xHCs without LEC use the "Mult" field in the endpoint context for SuperSpeed
+ * isoc eps, and High speed isoc eps that support bandwidth doubling. Standard
  * High speed endpoint descriptors can define "the number of additional
  * transaction opportunities per microframe", but that goes in the Max Burst
  * endpoint context field.
  */
-static u32 xhci_get_endpoint_mult(struct usb_device *udev,
-		struct usb_host_endpoint *ep)
+static u32 xhci_get_endpoint_mult(struct xhci_hcd *xhci,
+				  struct usb_device *udev,
+				  struct usb_host_endpoint *ep)
 {
-	if (udev->speed < USB_SPEED_SUPER ||
-			!usb_endpoint_xfer_isoc(&ep->desc))
+	bool lec;
+
+	/* xhci 1.1 with LEC set does not use mult field, except intel eusb2 */
+	lec = xhci->hci_version > 0x100 && HCC2_LEC(xhci->hcc_params2);
+
+	/* eusb2 double isoc bw devices are the only usb2 devices using mult */
+	if (xhci_eusb2_is_isoc_bw_double(udev, ep)) {
+		if (!lec || xhci->quirks & XHCI_INTEL_HOST)
+			return 1;
+	}
+
+	/* Oherwise only isoc transfers on hosts without LEC uses mult field */
+	if (!usb_endpoint_xfer_isoc(&ep->desc) || lec)
 		return 0;
-	return ep->ss_ep_comp.bmAttributes;
+
+	if (udev->speed >= USB_SPEED_SUPER)
+		return ep->ss_ep_comp.bmAttributes;
+
+	return 0;
 }
 
 static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
@@ -1351,8 +1369,18 @@ static u32 xhci_get_endpoint_max_burst(struct usb_device *udev,
 
 	if (udev->speed == USB_SPEED_HIGH &&
 	    (usb_endpoint_xfer_isoc(&ep->desc) ||
-	     usb_endpoint_xfer_int(&ep->desc)))
-		return usb_endpoint_maxp_mult(&ep->desc) - 1;
+	     usb_endpoint_xfer_int(&ep->desc))) {
+		if (xhci_eusb2_is_isoc_bw_double(udev, ep))
+			/*
+			 * eUSB2 double isoc bw endpoints max packet field
+			 * service opportunity bits 12:11 are not valid, so set
+			 * the ctx burst to max service opportunity "2" as these
+			 * eps support transferring over 3072 bytes per interval
+			 */
+			return 2;
+		else
+			return usb_endpoint_maxp_mult(&ep->desc) - 1;
+	}
 
 	return 0;
 }
@@ -1400,6 +1428,10 @@ static u32 xhci_get_max_esit_payload(struct usb_device *udev,
 	if (udev->speed >= USB_SPEED_SUPER)
 		return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
 
+	/* High speed eusb2 double isoc bw with over 3072 bytes per esit */
+	if (xhci_eusb2_is_isoc_bw_double(udev, ep))
+		return le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
+
 	max_packet = usb_endpoint_maxp(&ep->desc);
 	max_burst = usb_endpoint_maxp_mult(&ep->desc);
 	/* A 0 in max burst means 1 transfer per ESIT */
@@ -1437,6 +1469,13 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 
 	ring_type = usb_endpoint_type(&ep->desc);
 
+	/* Ensure host supports double isoc bandwidth for eusb2 devices */
+	if (xhci_eusb2_is_isoc_bw_double(udev, ep) &&
+	    !HCC2_EUSB2_DIC(xhci->hcc_params2))	{
+		dev_dbg(&udev->dev, "Double Isoc Bandwidth not supported by xhci\n");
+		return -EINVAL;
+	}
+
 	/*
 	 * Get values to fill the endpoint context, mostly from ep descriptor.
 	 * The average TRB buffer lengt for bulk endpoints is unclear as we
@@ -1456,8 +1495,8 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 		}
 	}
 
-	mult = xhci_get_endpoint_mult(udev, ep);
-	max_packet = usb_endpoint_maxp(&ep->desc);
+	mult = xhci_get_endpoint_mult(xhci, udev, ep);
+	max_packet = xhci_usb_endpoint_maxp(udev, ep);
 	max_burst = xhci_get_endpoint_max_burst(udev, ep);
 	avg_trb_len = max_esit_payload;
 
@@ -1478,9 +1517,6 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 	/* xHCI 1.0 and 1.1 indicates that ctrl ep avg TRB Length should be 8 */
 	if (usb_endpoint_xfer_control(&ep->desc) && xhci->hci_version >= 0x100)
 		avg_trb_len = 8;
-	/* xhci 1.1 with LEC support doesn't use mult field, use RsvdZ */
-	if ((xhci->hci_version > 0x100) && HCC2_LEC(xhci->hcc_params2))
-		mult = 0;
 
 	/* Set up the endpoint ring */
 	virt_dev->eps[ep_index].new_ring =
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index eff6b84dc915..f94626d8bcce 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3549,7 +3549,7 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int transferred,
 	if ((xhci->quirks & XHCI_MTK_HOST) && (xhci->hci_version < 0x100))
 		trb_buff_len = 0;
 
-	maxp = usb_endpoint_maxp(&urb->ep->desc);
+	maxp = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
 	total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
 
 	/* Queueing functions don't count the current TRB into transferred */
@@ -3566,7 +3566,7 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct urb *urb, u32 enqd_len,
 	u32 new_buff_len;
 	size_t len;
 
-	max_pkt = usb_endpoint_maxp(&urb->ep->desc);
+	max_pkt = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
 	unalign = (enqd_len + *trb_buff_len) % max_pkt;
 
 	/* we got lucky, last normal TRB data on segment is packet aligned */
@@ -4137,7 +4137,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 		addr = start_addr + urb->iso_frame_desc[i].offset;
 		td_len = urb->iso_frame_desc[i].length;
 		td_remain_len = td_len;
-		max_pkt = usb_endpoint_maxp(&urb->ep->desc);
+		max_pkt = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
 		total_pkt_count = DIV_ROUND_UP(td_len, max_pkt);
 
 		/* A zero-length transfer still involves at least one packet. */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 9769c68b2e9f..2ac383929b1c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1353,7 +1353,7 @@ static bool xhci_urb_temp_buffer_required(struct usb_hcd *hcd,
 	struct scatterlist *tail_sg;
 
 	tail_sg = urb->sg;
-	max_pkt = usb_endpoint_maxp(&urb->ep->desc);
+	max_pkt = xhci_usb_endpoint_maxp(urb->dev, urb->ep);
 
 	if (!urb->num_sgs)
 		return ret;
@@ -2940,6 +2940,21 @@ int xhci_stop_endpoint_sync(struct xhci_hcd *xhci, struct xhci_virt_ep *ep, int
 }
 EXPORT_SYMBOL_GPL(xhci_stop_endpoint_sync);
 
+/*
+ * xhci_usb_endpoint_maxp - get endpoint max packet size
+ * @host_ep: USB host endpoint to be checked
+ *
+ * Returns max packet from the correct descriptor
+ */
+
+int xhci_usb_endpoint_maxp(struct usb_device *udev,
+			   struct usb_host_endpoint *host_ep)
+{
+	if (xhci_eusb2_is_isoc_bw_double(udev, host_ep))
+		return le16_to_cpu(host_ep->eusb2_isoc_ep_comp.wMaxPacketSize);
+	return usb_endpoint_maxp(&host_ep->desc);
+}
+
 /* Issue a configure endpoint command or evaluate context command
  * and wait for it to finish.
  */
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 49887a303e43..e0c5238c9327 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1735,6 +1735,23 @@ static inline bool xhci_has_one_roothub(struct xhci_hcd *xhci)
 	       (!xhci->usb2_rhub.num_ports || !xhci->usb3_rhub.num_ports);
 }
 
+/*
+ * USB 2.0 specification, chapter 5.6.4 Isochronous Transfer Bus Access
+ * Constraint. A high speed endpoint can move up to 3072 bytes per microframe
+ * (or 192Mb/s).
+ */
+#define MAX_ISOC_XFER_SIZE_HS  3072
+
+static inline bool xhci_eusb2_is_isoc_bw_double(struct usb_device *udev,
+						struct usb_host_endpoint *ep)
+{
+	return le16_to_cpu(udev->descriptor.bcdUSB) == 0x220 &&
+		usb_endpoint_xfer_isoc(&ep->desc) &&
+		le16_to_cpu(ep->desc.wMaxPacketSize) == 0 &&
+		le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval) >
+		MAX_ISOC_XFER_SIZE_HS;
+}
+
 #define xhci_dbg(xhci, fmt, args...) \
 	dev_dbg(xhci_to_hcd(xhci)->self.controller , fmt , ## args)
 #define xhci_err(xhci, fmt, args...) \
@@ -1958,6 +1975,8 @@ 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);
+int xhci_usb_endpoint_maxp(struct usb_device *udev,
+			   struct usb_host_endpoint *host_ep);
 
 /* xHCI roothub code */
 void xhci_set_link_state(struct xhci_hcd *xhci, struct xhci_port *port,
-- 
2.43.0

Re: [PATCH 24/24] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Thinh Nguyen]

On Thu, May 15, 2025, Mathias Nyman wrote:
> From: Amardeep Rai <amardeep.rai@intel.com>
> 
> Detect eUSB2 double isoc bw capable hosts and devices, and set the proper
> xhci endpoint context values such as 'Mult', 'Max Burst Size', and 'Max
> ESIT Payload' to enable the double isochronous bandwidth endpoints.
> 
> Intel xHC uses the endpoint context 'Mult' field for eUSB2 isoc
> endpoints even if hosts supporting Large ESIT Payload Capability should
> normally ignore the mult field.
> 
> Signed-off-by: Amardeep Rai <amardeep.rai@intel.com>
> Co-developed-by: Kannappan R <r.kannappan@intel.com>
> Signed-off-by: Kannappan R <r.kannappan@intel.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci-caps.h |  2 ++
>  drivers/usb/host/xhci-mem.c  | 62 ++++++++++++++++++++++++++++--------
>  drivers/usb/host/xhci-ring.c |  6 ++--
>  drivers/usb/host/xhci.c      | 17 +++++++++-
>  drivers/usb/host/xhci.h      | 19 +++++++++++
>  5 files changed, 89 insertions(+), 17 deletions(-)
> 


<snip>


> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 49887a303e43..e0c5238c9327 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1735,6 +1735,23 @@ static inline bool xhci_has_one_roothub(struct xhci_hcd *xhci)
>  	       (!xhci->usb2_rhub.num_ports || !xhci->usb3_rhub.num_ports);
>  }
>  
> +/*
> + * USB 2.0 specification, chapter 5.6.4 Isochronous Transfer Bus Access
> + * Constraint. A high speed endpoint can move up to 3072 bytes per microframe
> + * (or 192Mb/s).
> + */
> +#define MAX_ISOC_XFER_SIZE_HS  3072
> +
> +static inline bool xhci_eusb2_is_isoc_bw_double(struct usb_device *udev,
> +						struct usb_host_endpoint *ep)
> +{
> +	return le16_to_cpu(udev->descriptor.bcdUSB) == 0x220 &&
> +		usb_endpoint_xfer_isoc(&ep->desc) &&
> +		le16_to_cpu(ep->desc.wMaxPacketSize) == 0 &&
> +		le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval) >
> +		MAX_ISOC_XFER_SIZE_HS;

Shouldn't this check for isoc IN direction only?

Do we need to check for connected speed?

Also, why are we checking eusb2_isoc_ep_comp.dwBytesPerInterval >
MAX_ISOC_XFER_SIZE_HS to determine if it's double isoc?

In xhci_get_max_esit_payload, you use this check to determine whether to
use dwBytesPerInterval:

	/* High speed eusb2 double isoc bw with over 3072 bytes per esit */
	if (xhci_eusb2_is_isoc_bw_double(udev, ep))
		return le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);

Shouldn't we just use the dwBytesPerInterval if it is valid? Otherwise
there can be a case where dwBytesPerInterval is set below the
MAX_ISOC_XFER_SIZE_HS and break this check.

BR,
Thinh

Re: [PATCH 24/24] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Thinh Nguyen]

On Thu, May 15, 2025, Thinh Nguyen wrote:
> On Thu, May 15, 2025, Mathias Nyman wrote:
> > From: Amardeep Rai <amardeep.rai@intel.com>
> > 
> > Detect eUSB2 double isoc bw capable hosts and devices, and set the proper
> > xhci endpoint context values such as 'Mult', 'Max Burst Size', and 'Max
> > ESIT Payload' to enable the double isochronous bandwidth endpoints.
> > 
> > Intel xHC uses the endpoint context 'Mult' field for eUSB2 isoc
> > endpoints even if hosts supporting Large ESIT Payload Capability should
> > normally ignore the mult field.
> > 
> > Signed-off-by: Amardeep Rai <amardeep.rai@intel.com>
> > Co-developed-by: Kannappan R <r.kannappan@intel.com>
> > Signed-off-by: Kannappan R <r.kannappan@intel.com>
> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > ---
> >  drivers/usb/host/xhci-caps.h |  2 ++
> >  drivers/usb/host/xhci-mem.c  | 62 ++++++++++++++++++++++++++++--------
> >  drivers/usb/host/xhci-ring.c |  6 ++--
> >  drivers/usb/host/xhci.c      | 17 +++++++++-
> >  drivers/usb/host/xhci.h      | 19 +++++++++++
> >  5 files changed, 89 insertions(+), 17 deletions(-)
> > 
> 
> 
> <snip>
> 
> 
> > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > index 49887a303e43..e0c5238c9327 100644
> > --- a/drivers/usb/host/xhci.h
> > +++ b/drivers/usb/host/xhci.h
> > @@ -1735,6 +1735,23 @@ static inline bool xhci_has_one_roothub(struct xhci_hcd *xhci)
> >  	       (!xhci->usb2_rhub.num_ports || !xhci->usb3_rhub.num_ports);
> >  }
> >  
> > +/*
> > + * USB 2.0 specification, chapter 5.6.4 Isochronous Transfer Bus Access
> > + * Constraint. A high speed endpoint can move up to 3072 bytes per microframe
> > + * (or 192Mb/s).
> > + */
> > +#define MAX_ISOC_XFER_SIZE_HS  3072
> > +
> > +static inline bool xhci_eusb2_is_isoc_bw_double(struct usb_device *udev,
> > +						struct usb_host_endpoint *ep)
> > +{
> > +	return le16_to_cpu(udev->descriptor.bcdUSB) == 0x220 &&
> > +		usb_endpoint_xfer_isoc(&ep->desc) &&
> > +		le16_to_cpu(ep->desc.wMaxPacketSize) == 0 &&
> > +		le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval) >
> > +		MAX_ISOC_XFER_SIZE_HS;
> 
> Shouldn't this check for isoc IN direction only?
> 
> Do we need to check for connected speed?
> 
> Also, why are we checking eusb2_isoc_ep_comp.dwBytesPerInterval >
> MAX_ISOC_XFER_SIZE_HS to determine if it's double isoc?
> 
> In xhci_get_max_esit_payload, you use this check to determine whether to
> use dwBytesPerInterval:
> 
> 	/* High speed eusb2 double isoc bw with over 3072 bytes per esit */
> 	if (xhci_eusb2_is_isoc_bw_double(udev, ep))
> 		return le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
> 
> Shouldn't we just use the dwBytesPerInterval if it is valid? Otherwise
> there can be a case where dwBytesPerInterval is set below the
> MAX_ISOC_XFER_SIZE_HS and break this check.
> 

Actually spec indicates the dwBytesPerInterval value must be between
3073 and 6144 for eusb2 double isoc. For eUSB2v2, this value can be
less.

BR,
Thinh

Re: [PATCH 24/24] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Mathias Nyman]

On 16.5.2025 3.40, Thinh Nguyen wrote:
> On Thu, May 15, 2025, Thinh Nguyen wrote:
>> On Thu, May 15, 2025, Mathias Nyman wrote:
>>> From: Amardeep Rai <amardeep.rai@intel.com>
>>>
>>> Detect eUSB2 double isoc bw capable hosts and devices, and set the proper
>>> xhci endpoint context values such as 'Mult', 'Max Burst Size', and 'Max
>>> ESIT Payload' to enable the double isochronous bandwidth endpoints.
>>>
>>> Intel xHC uses the endpoint context 'Mult' field for eUSB2 isoc
>>> endpoints even if hosts supporting Large ESIT Payload Capability should
>>> normally ignore the mult field.
>>>
>>> Signed-off-by: Amardeep Rai <amardeep.rai@intel.com>
>>> Co-developed-by: Kannappan R <r.kannappan@intel.com>
>>> Signed-off-by: Kannappan R <r.kannappan@intel.com>
>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>> ---
>>>   drivers/usb/host/xhci-caps.h |  2 ++
>>>   drivers/usb/host/xhci-mem.c  | 62 ++++++++++++++++++++++++++++--------
>>>   drivers/usb/host/xhci-ring.c |  6 ++--
>>>   drivers/usb/host/xhci.c      | 17 +++++++++-
>>>   drivers/usb/host/xhci.h      | 19 +++++++++++
>>>   5 files changed, 89 insertions(+), 17 deletions(-)
>>>
>>
>>
>> <snip>
>>
>>
>>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>>> index 49887a303e43..e0c5238c9327 100644
>>> --- a/drivers/usb/host/xhci.h
>>> +++ b/drivers/usb/host/xhci.h
>>> @@ -1735,6 +1735,23 @@ static inline bool xhci_has_one_roothub(struct xhci_hcd *xhci)
>>>   	       (!xhci->usb2_rhub.num_ports || !xhci->usb3_rhub.num_ports);
>>>   }
>>>   
>>> +/*
>>> + * USB 2.0 specification, chapter 5.6.4 Isochronous Transfer Bus Access
>>> + * Constraint. A high speed endpoint can move up to 3072 bytes per microframe
>>> + * (or 192Mb/s).
>>> + */
>>> +#define MAX_ISOC_XFER_SIZE_HS  3072
>>> +
>>> +static inline bool xhci_eusb2_is_isoc_bw_double(struct usb_device *udev,
>>> +						struct usb_host_endpoint *ep)
>>> +{
>>> +	return le16_to_cpu(udev->descriptor.bcdUSB) == 0x220 &&
>>> +		usb_endpoint_xfer_isoc(&ep->desc) &&
>>> +		le16_to_cpu(ep->desc.wMaxPacketSize) == 0 &&
>>> +		le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval) >
>>> +		MAX_ISOC_XFER_SIZE_HS;
>>
>> Shouldn't this check for isoc IN direction only?

Probably not needed as only Isoc IN endpoints can have the additional isoc
endpoint  companion descriptor. So only the IN endpoint would fulfill the
desc.wMaxPacketSize == 0 and ep->eusb2_isoc_ep_comp.dwBytesPerInterval > 3072
check.
  
>>
>> Do we need to check for connected speed?

Good question.

The bcdUSB == 0x220 tells it's a "directly connected inbox native eUSB2 device",
so both host and device should support and enumerate at High-speed.

If the device for some reason supports enumerating as Full-speed after reset,
then I doubt it would share config, interface and endpoint descriptors intended
for High-speed and eUSB2 double isoc bw.

But I'm not 100% sure about this, need to double check it.

>>
>> Also, why are we checking eusb2_isoc_ep_comp.dwBytesPerInterval >
>> MAX_ISOC_XFER_SIZE_HS to determine if it's double isoc?
>>
>> In xhci_get_max_esit_payload, you use this check to determine whether to
>> use dwBytesPerInterval:
>>
>> 	/* High speed eusb2 double isoc bw with over 3072 bytes per esit */
>> 	if (xhci_eusb2_is_isoc_bw_double(udev, ep))
>> 		return le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
>>
>> Shouldn't we just use the dwBytesPerInterval if it is valid? Otherwise
>> there can be a case where dwBytesPerInterval is set below the
>> MAX_ISOC_XFER_SIZE_HS and break this check.
>>
> 
> Actually spec indicates the dwBytesPerInterval value must be between
> 3073 and 6144 for eusb2 double isoc. For eUSB2v2, this value can be
> less.

This patch only adds support for double isoc bandwidth eusb2 devices,
i.e. with BytesPerInterval 3073-6144. eUSB2V2 with bcdUSB 0x230 are not
yet supported

Thanks
Mathias

Re: [PATCH 24/24] xhci: Add host support for eUSB2 double isochronous bandwidth devices

[Thinh Nguyen]

On Fri, May 16, 2025, Mathias Nyman wrote:
> On 16.5.2025 3.40, Thinh Nguyen wrote:
> > On Thu, May 15, 2025, Thinh Nguyen wrote:
> > > On Thu, May 15, 2025, Mathias Nyman wrote:
> > > > From: Amardeep Rai <amardeep.rai@intel.com>
> > > > 
> > > > Detect eUSB2 double isoc bw capable hosts and devices, and set the proper
> > > > xhci endpoint context values such as 'Mult', 'Max Burst Size', and 'Max
> > > > ESIT Payload' to enable the double isochronous bandwidth endpoints.
> > > > 
> > > > Intel xHC uses the endpoint context 'Mult' field for eUSB2 isoc
> > > > endpoints even if hosts supporting Large ESIT Payload Capability should
> > > > normally ignore the mult field.
> > > > 
> > > > Signed-off-by: Amardeep Rai <amardeep.rai@intel.com>
> > > > Co-developed-by: Kannappan R <r.kannappan@intel.com>
> > > > Signed-off-by: Kannappan R <r.kannappan@intel.com>
> > > > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > > ---
> > > >   drivers/usb/host/xhci-caps.h |  2 ++
> > > >   drivers/usb/host/xhci-mem.c  | 62 ++++++++++++++++++++++++++++--------
> > > >   drivers/usb/host/xhci-ring.c |  6 ++--
> > > >   drivers/usb/host/xhci.c      | 17 +++++++++-
> > > >   drivers/usb/host/xhci.h      | 19 +++++++++++
> > > >   5 files changed, 89 insertions(+), 17 deletions(-)
> > > > 
> > > 
> > > 
> > > <snip>
> > > 
> > > 
> > > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > > > index 49887a303e43..e0c5238c9327 100644
> > > > --- a/drivers/usb/host/xhci.h
> > > > +++ b/drivers/usb/host/xhci.h
> > > > @@ -1735,6 +1735,23 @@ static inline bool xhci_has_one_roothub(struct xhci_hcd *xhci)
> > > >   	       (!xhci->usb2_rhub.num_ports || !xhci->usb3_rhub.num_ports);
> > > >   }
> > > > +/*
> > > > + * USB 2.0 specification, chapter 5.6.4 Isochronous Transfer Bus Access
> > > > + * Constraint. A high speed endpoint can move up to 3072 bytes per microframe
> > > > + * (or 192Mb/s).
> > > > + */
> > > > +#define MAX_ISOC_XFER_SIZE_HS  3072
> > > > +
> > > > +static inline bool xhci_eusb2_is_isoc_bw_double(struct usb_device *udev,
> > > > +						struct usb_host_endpoint *ep)
> > > > +{
> > > > +	return le16_to_cpu(udev->descriptor.bcdUSB) == 0x220 &&
> > > > +		usb_endpoint_xfer_isoc(&ep->desc) &&
> > > > +		le16_to_cpu(ep->desc.wMaxPacketSize) == 0 &&
> > > > +		le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval) >
> > > > +		MAX_ISOC_XFER_SIZE_HS;
> > > 
> > > Shouldn't this check for isoc IN direction only?
> 
> Probably not needed as only Isoc IN endpoints can have the additional isoc
> endpoint  companion descriptor. So only the IN endpoint would fulfill the
> desc.wMaxPacketSize == 0 and ep->eusb2_isoc_ep_comp.dwBytesPerInterval > 3072
> check.

Ok.

> > > 
> > > Do we need to check for connected speed?
> 
> Good question.
> 
> The bcdUSB == 0x220 tells it's a "directly connected inbox native eUSB2 device",
> so both host and device should support and enumerate at High-speed.

Do we also need to check if it's connected directly to the roothub here.
Probably there's no real device with this bcdUSB that's not "inbox"
and connected directly to the roothub though.

> 
> If the device for some reason supports enumerating as Full-speed after reset,
> then I doubt it would share config, interface and endpoint descriptors intended
> for High-speed and eUSB2 double isoc bw.

Good point, the device should be able to detect and provide proper
config should there be speed change.

> 
> But I'm not 100% sure about this, need to double check it.
> 
> > > 
> > > Also, why are we checking eusb2_isoc_ep_comp.dwBytesPerInterval >
> > > MAX_ISOC_XFER_SIZE_HS to determine if it's double isoc?
> > > 
> > > In xhci_get_max_esit_payload, you use this check to determine whether to
> > > use dwBytesPerInterval:
> > > 
> > > 	/* High speed eusb2 double isoc bw with over 3072 bytes per esit */
> > > 	if (xhci_eusb2_is_isoc_bw_double(udev, ep))
> > > 		return le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
> > > 
> > > Shouldn't we just use the dwBytesPerInterval if it is valid? Otherwise
> > > there can be a case where dwBytesPerInterval is set below the
> > > MAX_ISOC_XFER_SIZE_HS and break this check.
> > > 
> > 
> > Actually spec indicates the dwBytesPerInterval value must be between
> > 3073 and 6144 for eusb2 double isoc. For eUSB2v2, this value can be
> > less.
> 
> This patch only adds support for double isoc bandwidth eusb2 devices,
> i.e. with BytesPerInterval 3073-6144. eUSB2V2 with bcdUSB 0x230 are not
> yet supported
> 

Thanks,
Thinh

Re: [PATCH 00/24] xhci features for usb-next

[Greg KH]

On Thu, May 15, 2025 at 04:55:57PM +0300, Mathias Nyman wrote:
> Hi Greg
> 
> A series of new xhci features and refactoring for usb next incuding
>   - Debugfs support for showing available port bandwidth
>   - xhci parts of eUSB2 double isoc bandwidth support
>   - refactoring to decouple allocation and initialzation 
>   - other misc cleanups and refactoring

I've applied all but the last 2 patches here.

thanks,

greg k-h

Re: [PATCH 00/24] xhci features for usb-next

[Mathias Nyman]

On 21.5.2025 13.36, Greg KH wrote:
> On Thu, May 15, 2025 at 04:55:57PM +0300, Mathias Nyman wrote:
>> Hi Greg
>>
>> A series of new xhci features and refactoring for usb next incuding
>>    - Debugfs support for showing available port bandwidth
>>    - xhci parts of eUSB2 double isoc bandwidth support
>>    - refactoring to decouple allocation and initialzation
>>    - other misc cleanups and refactoring
> 
> I've applied all but the last 2 patches here.
> 

Thanks

- Mathias