[PATCH 00/13] usb: xhci: Host Controller Capability Registers rework

[PATCH 00/13] usb: xhci: Host Controller Capability Registers rework

[Niklas Neronin]

This patch series focuses on improving the definitions and organization of
the Host Controller Capability Registers macros in the xhci driver.
It is the first step in a larger effort to clean up and restructure the
xhci header files for better readability and maintainability.

Because the Structural Parameters 1 register (part of the HC Capability
Registers) relies heavily on xhci-caps.h, a few related patches from
another ongoing rework have been included here as well. These address the
handling of Max Ports and Max Interrupters values.
The Max Slots handling will follow the same pattern in a future series.
That change is more involved and will be submitted separately.

Niklas Neronin (13):
  usb: xhci: remove deprecated TODO comment
  usb: xhci: remove unused trace operation and argument
  usb: xhci: use cached HCSPARAMS1 value
  usb: xhci: simplify handling of Structural Parameters 1 values
  usb: xhci: limit number of ports to 127
  usb: xhci: limit number of interrupts to 128
  usb: xhci: improve xhci-caps.h comments
  usb: xhci: simplify Isochronous Scheduling Threshold handling
  usb: xhci: simplify Max Scratchpad buffer macros
  usb: xhci: drop xhci-caps.h dependence on xhci-ext-caps.h
  usb: xhci: standardize single bit-field macros
  usb: xhci: standardize multi bit-field macros
  usb: xhci: use 64-bit Addressing Capability macro

 drivers/usb/host/xhci-caps.h    | 188 ++++++++++++++++++--------------
 drivers/usb/host/xhci-debugfs.c |  20 ++--
 drivers/usb/host/xhci-histb.c   |   2 +-
 drivers/usb/host/xhci-hub.c     |  12 +-
 drivers/usb/host/xhci-mem.c     |  41 +++----
 drivers/usb/host/xhci-mtk.c     |   4 +-
 drivers/usb/host/xhci-pci.c     |   6 +-
 drivers/usb/host/xhci-plat.c    |   2 +-
 drivers/usb/host/xhci-ring.c    |  40 ++++---
 drivers/usb/host/xhci-tegra.c   |   5 +-
 drivers/usb/host/xhci-trace.h   |  20 +---
 drivers/usb/host/xhci.c         |  57 +++++-----
 drivers/usb/host/xhci.h         |  16 ++-
 13 files changed, 212 insertions(+), 201 deletions(-)

-- 
2.50.1

[PATCH 01/13] usb: xhci: remove deprecated TODO comment

[Niklas Neronin]

The Device Context Base Address Array (DCBAA) contains pointers to device
contexts. These fields are 64-bit registers, capable of holding 64-bit
addresses.

When struct 'xhci_device_context_array' was introduced in commit [1],
the entries were represented as pairs of 'u32', requiring a custom helper
function to set 64-bit addresses. This was later made redundant by
commit [2], which changed the representation to a single 'u64', allowing
direct assignment.

The associated TODO comment referencing the old 32-bit representation is
no longer relevant and is removed.

Link: https://git.kernel.org/torvalds/c/a74588f94655 [1]
Link: https://git.kernel.org/torvalds/c/8e595a5d30a5 [2]
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 58a51f09cceb..8792692ba236 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -800,7 +800,6 @@ struct xhci_device_context_array {
 	/* private xHCD pointers */
 	dma_addr_t	dma;
 };
-/* TODO: write function to set the 64-bit device DMA address */
 /*
  * TODO: change this to be dynamically sized at HC mem init time since the HC
  * might not be able to handle the maximum number of devices possible.
-- 
2.50.1

[PATCH 02/13] usb: xhci: remove unused trace operation and argument

[Niklas Neronin]

Remove endpoint number 'ep_num' argument and memory operation from
xhci_log_ctx() trace function. These changes were added in commit
1d27fabec068 ("xhci: add xhci_address_ctx trace event") on Aug 14, 2013
and have never been used.

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

diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 9abc904f1749..bf13da417f8e 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -71,18 +71,13 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
 );
 
 DECLARE_EVENT_CLASS(xhci_log_ctx,
-	TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
-		 unsigned int ep_num),
-	TP_ARGS(xhci, ctx, ep_num),
+	TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+	TP_ARGS(xhci, ctx),
 	TP_STRUCT__entry(
 		__field(int, ctx_64)
 		__field(unsigned, ctx_type)
 		__field(dma_addr_t, ctx_dma)
 		__field(u8 *, ctx_va)
-		__field(unsigned, ctx_ep_num)
-		__dynamic_array(u32, ctx_data,
-			((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 8) *
-			((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1))
 	),
 	TP_fast_assign(
 
@@ -90,10 +85,6 @@ DECLARE_EVENT_CLASS(xhci_log_ctx,
 		__entry->ctx_type = ctx->type;
 		__entry->ctx_dma = ctx->dma;
 		__entry->ctx_va = ctx->bytes;
-		__entry->ctx_ep_num = ep_num;
-		memcpy(__get_dynamic_array(ctx_data), ctx->bytes,
-			((HCC_64BYTE_CONTEXT(xhci->hcc_params) + 1) * 32) *
-			((ctx->type == XHCI_CTX_TYPE_INPUT) + ep_num + 1));
 	),
 	TP_printk("ctx_64=%d, ctx_type=%u, ctx_dma=@%llx, ctx_va=@%p",
 			__entry->ctx_64, __entry->ctx_type,
@@ -102,9 +93,8 @@ DECLARE_EVENT_CLASS(xhci_log_ctx,
 );
 
 DEFINE_EVENT(xhci_log_ctx, xhci_address_ctx,
-	TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
-		 unsigned int ep_num),
-	TP_ARGS(xhci, ctx, ep_num)
+	TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx),
+	TP_ARGS(xhci, ctx)
 );
 
 DECLARE_EVENT_CLASS(xhci_log_trb,
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 0cb45b95e4f5..42975df517cb 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4366,8 +4366,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 	ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG | EP0_FLAG);
 	ctrl_ctx->drop_flags = 0;
 
-	trace_xhci_address_ctx(xhci, virt_dev->in_ctx,
-				le32_to_cpu(slot_ctx->dev_info) >> 27);
+	trace_xhci_address_ctx(xhci, virt_dev->in_ctx);
 
 	trace_xhci_address_ctrl_ctx(ctrl_ctx);
 	spin_lock_irqsave(&xhci->lock, flags);
@@ -4427,7 +4426,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 		xhci_err(xhci,
 			 "ERROR: unexpected setup %s command completion code 0x%x.\n",
 			 act, command->status);
-		trace_xhci_address_ctx(xhci, virt_dev->out_ctx, 1);
+		trace_xhci_address_ctx(xhci, virt_dev->out_ctx);
 		ret = -EINVAL;
 		break;
 	}
@@ -4445,14 +4444,12 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 	xhci_dbg_trace(xhci, trace_xhci_dbg_address,
 			"Output Context DMA address = %#08llx",
 			(unsigned long long)virt_dev->out_ctx->dma);
-	trace_xhci_address_ctx(xhci, virt_dev->in_ctx,
-				le32_to_cpu(slot_ctx->dev_info) >> 27);
+	trace_xhci_address_ctx(xhci, virt_dev->in_ctx);
 	/*
 	 * USB core uses address 1 for the roothubs, so we add one to the
 	 * address given back to us by the HC.
 	 */
-	trace_xhci_address_ctx(xhci, virt_dev->out_ctx,
-				le32_to_cpu(slot_ctx->dev_info) >> 27);
+	trace_xhci_address_ctx(xhci, virt_dev->out_ctx);
 	/* Zero the input context control for later use */
 	ctrl_ctx->add_flags = 0;
 	ctrl_ctx->drop_flags = 0;
-- 
2.50.1

[PATCH 03/13] usb: xhci: use cached HCSPARAMS1 value

[Niklas Neronin]

The Structural Parameters 1 (HCSPARAMS1) register is read and cached in
'xhci->hcs_params1' during host controller initialization. Since this
register is read-only and its value remains constant for the lifetime of
the controller, re-reading it later is unnecessary.

Replace subsequent register reads with the cached 'xhci->hcs_params1'
value to avoid redundant MMIO access.

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

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 42975df517cb..0630269e5ab7 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4222,8 +4222,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 		xhci_err(xhci, "Error while assigning device slot ID: %s\n",
 			 xhci_trb_comp_code_string(command->status));
 		xhci_err(xhci, "Max number of devices this xHCI host supports is %u.\n",
-				HCS_MAX_SLOTS(
-					readl(&xhci->cap_regs->hcs_params1)));
+			 HCS_MAX_SLOTS(xhci->hcs_params1));
 		xhci_free_command(xhci, command);
 		return 0;
 	}
-- 
2.50.1

[PATCH 04/13] usb: xhci: simplify handling of Structural Parameters 1 values

[Niklas Neronin]

The 32-bit read-only HCSPARAMS1 register contains the following fields:
 Bits  7:0   - Number of Device Slots (MaxSlots)
 Bits 18:8   - Number of Interrupters (MaxIntrs)
 Bits 23:19  - Reserved
 Bits 31:24  - Number of Ports (MaxPorts)

Since the register value is constant for the lifetime of the controller,
it is cached in 'xhci->hcs_params1'. However, platform drivers may
override the number of interrupters through a separate variable,
'xhci->max_interrupters', leaving only the maximum slots and ports values
still derived from the cached register.

To simplify the code and improve readability, replace 'xhci->hcs_params1'
with two dedicated 'u8' fields: 'xhci->max_slots' and 'xhci->max_ports'.
These values are initialized once and used directly instead of calling
'HCS_MAX_SLOTS()' and 'HCS_MAX_PORTS()' macros.

This change reduces code clutter without increasing memory usage.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-debugfs.c | 15 ++++-----------
 drivers/usb/host/xhci-hub.c     |  2 +-
 drivers/usb/host/xhci-mem.c     | 31 +++++++++++++------------------
 drivers/usb/host/xhci-pci.c     |  2 +-
 drivers/usb/host/xhci-ring.c    |  6 ++----
 drivers/usb/host/xhci.c         | 21 ++++++++++-----------
 drivers/usb/host/xhci.h         |  3 ++-
 7 files changed, 33 insertions(+), 47 deletions(-)

diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index c6d44977193f..f0475cf8eef8 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -613,20 +613,16 @@ void xhci_debugfs_remove_slot(struct xhci_hcd *xhci, int slot_id)
 static void xhci_debugfs_create_ports(struct xhci_hcd *xhci,
 				      struct dentry *parent)
 {
-	unsigned int		num_ports;
 	char			port_name[8];
 	struct xhci_port	*port;
 	struct dentry		*dir;
 
-	num_ports = HCS_MAX_PORTS(xhci->hcs_params1);
-
 	parent = debugfs_create_dir("ports", parent);
 
-	while (num_ports--) {
-		scnprintf(port_name, sizeof(port_name), "port%02d",
-			  num_ports + 1);
+	for (int i = 0; i < xhci->max_ports; i++) {
+		scnprintf(port_name, sizeof(port_name), "port%02d", i + 1);
 		dir = debugfs_create_dir(port_name, parent);
-		port = &xhci->hw_ports[num_ports];
+		port = &xhci->hw_ports[i];
 		debugfs_create_file("portsc", 0644, dir, port, &port_fops);
 	}
 }
@@ -634,7 +630,6 @@ 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;
@@ -645,8 +640,6 @@ static int xhci_port_bw_show(struct xhci_hcd *xhci, u8 dev_speed,
 	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);
@@ -661,7 +654,7 @@ static int xhci_port_bw_show(struct xhci_hcd *xhci, u8 dev_speed,
 	/* 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++)
+	for (i = 1; i <= xhci->max_ports; i++)
 		seq_printf(s, "port[%d] available bw: %d%%.\n", i,
 				ctx->bytes[i]);
 err_out:
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index b3a59ce1b3f4..e7c1952cffc5 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -700,7 +700,7 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
 	/* Disable all Device Slots */
 	xhci_dbg(xhci, "Disable all slots\n");
 	spin_unlock_irqrestore(&xhci->lock, *flags);
-	for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
+	for (i = 1; i <= xhci->max_slots; i++) {
 		if (!xhci->devs[i])
 			continue;
 
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 6e5b6057de79..98abf86e0910 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -951,7 +951,7 @@ static void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_i
 		/* is this a hub device that added a tt_info to the tts list */
 		if (tt_info->slot_id == slot_id) {
 			/* are any devices using this tt_info? */
-			for (i = 1; i < HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
+			for (i = 1; i < xhci->max_slots; i++) {
 				vdev = xhci->devs[i];
 				if (vdev && (vdev->tt_info == tt_info))
 					xhci_free_virt_devices_depth_first(
@@ -1899,7 +1899,7 @@ EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
 void xhci_mem_cleanup(struct xhci_hcd *xhci)
 {
 	struct device	*dev = xhci_to_hcd(xhci)->self.sysdev;
-	int i, j, num_ports;
+	int i, j;
 
 	cancel_delayed_work_sync(&xhci->cmd_timer);
 
@@ -1918,8 +1918,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed command ring");
 	xhci_cleanup_command_queue(xhci);
 
-	num_ports = HCS_MAX_PORTS(xhci->hcs_params1);
-	for (i = 0; i < num_ports && xhci->rh_bw; i++) {
+	for (i = 0; i < xhci->max_ports && xhci->rh_bw; i++) {
 		struct xhci_interval_bw_table *bwt = &xhci->rh_bw[i].bw_table;
 		for (j = 0; j < XHCI_MAX_INTERVAL; j++) {
 			struct list_head *ep = &bwt->interval_bw[j].endpoints;
@@ -1928,7 +1927,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
 		}
 	}
 
-	for (i = HCS_MAX_SLOTS(xhci->hcs_params1); i > 0; i--)
+	for (i = xhci->max_slots; i > 0; i--)
 		xhci_free_virt_devices_depth_first(xhci, i);
 
 	dma_pool_destroy(xhci->segment_pool);
@@ -1964,7 +1963,7 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
 	if (!xhci->rh_bw)
 		goto no_bw;
 
-	for (i = 0; i < num_ports; i++) {
+	for (i = 0; i < xhci->max_ports; i++) {
 		struct xhci_tt_bw_info *tt, *n;
 		list_for_each_entry_safe(tt, n, &xhci->rh_bw[i].tts, tt_list) {
 			list_del(&tt->tt_list);
@@ -2165,7 +2164,7 @@ static void xhci_create_rhub_port_array(struct xhci_hcd *xhci,
 	if (!rhub->ports)
 		return;
 
-	for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
+	for (i = 0; i < xhci->max_ports; i++) {
 		if (xhci->hw_ports[i].rhub != rhub ||
 		    xhci->hw_ports[i].hcd_portnum == DUPLICATE_ENTRY)
 			continue;
@@ -2188,19 +2187,17 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
 {
 	void __iomem *base;
 	u32 offset;
-	unsigned int num_ports;
 	int i, j;
 	int cap_count = 0;
 	u32 cap_start;
 	struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
 
-	num_ports = HCS_MAX_PORTS(xhci->hcs_params1);
-	xhci->hw_ports = kcalloc_node(num_ports, sizeof(*xhci->hw_ports),
-				flags, dev_to_node(dev));
+	xhci->hw_ports = kcalloc_node(xhci->max_ports, sizeof(*xhci->hw_ports),
+				      flags, dev_to_node(dev));
 	if (!xhci->hw_ports)
 		return -ENOMEM;
 
-	for (i = 0; i < num_ports; i++) {
+	for (i = 0; i < xhci->max_ports; i++) {
 		xhci->hw_ports[i].addr = &xhci->op_regs->port_status_base +
 			NUM_PORT_REGS * i;
 		xhci->hw_ports[i].hw_portnum = i;
@@ -2209,11 +2206,10 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
 		init_completion(&xhci->hw_ports[i].u3exit_done);
 	}
 
-	xhci->rh_bw = kcalloc_node(num_ports, sizeof(*xhci->rh_bw), flags,
-				   dev_to_node(dev));
+	xhci->rh_bw = kcalloc_node(xhci->max_ports, sizeof(*xhci->rh_bw), flags, dev_to_node(dev));
 	if (!xhci->rh_bw)
 		return -ENOMEM;
-	for (i = 0; i < num_ports; i++) {
+	for (i = 0; i < xhci->max_ports; i++) {
 		struct xhci_interval_bw_table *bw_table;
 
 		INIT_LIST_HEAD(&xhci->rh_bw[i].tts);
@@ -2245,9 +2241,8 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
 	offset = cap_start;
 
 	while (offset) {
-		xhci_add_in_port(xhci, num_ports, base + offset, cap_count);
-		if (xhci->usb2_rhub.num_ports + xhci->usb3_rhub.num_ports ==
-		    num_ports)
+		xhci_add_in_port(xhci, xhci->max_ports, base + offset, cap_count);
+		if (xhci->usb2_rhub.num_ports + xhci->usb3_rhub.num_ports == xhci->max_ports)
 			break;
 		offset = xhci_find_next_ext_cap(base, offset,
 						XHCI_EXT_CAPS_PROTOCOL);
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index f67a4d956204..6a25cbbbc4a4 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -896,7 +896,7 @@ static int xhci_pci_poweroff_late(struct usb_hcd *hcd, bool do_wakeup)
 	if (!(xhci->quirks & XHCI_RESET_TO_DEFAULT))
 		return 0;
 
-	for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
+	for (i = 0; i < xhci->max_ports; i++) {
 		port = &xhci->hw_ports[i];
 		portsc = readl(port->addr);
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 8e209aa33ea7..5f46661c8e6b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1388,7 +1388,7 @@ void xhci_hc_died(struct xhci_hcd *xhci)
 	xhci_cleanup_command_queue(xhci);
 
 	/* return any pending urbs, remove may be waiting for them */
-	for (i = 0; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
+	for (i = 0; i <= xhci->max_slots; i++) {
 		if (!xhci->devs[i])
 			continue;
 		for (j = 0; j < 31; j++)
@@ -1988,7 +1988,6 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 	struct usb_hcd *hcd;
 	u32 port_id;
 	u32 portsc, cmd_reg;
-	int max_ports;
 	unsigned int hcd_portnum;
 	struct xhci_bus_state *bus_state;
 	bool bogus_port_status = false;
@@ -2000,9 +1999,8 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 			  "WARN: xHC returned failed port status event\n");
 
 	port_id = GET_PORT_ID(le32_to_cpu(event->generic.field[0]));
-	max_ports = HCS_MAX_PORTS(xhci->hcs_params1);
 
-	if ((port_id <= 0) || (port_id > max_ports)) {
+	if ((port_id <= 0) || (port_id > xhci->max_ports)) {
 		xhci_warn(xhci, "Port change event with invalid port ID %d\n",
 			  port_id);
 		return;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 0630269e5ab7..d74f47a75c68 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -278,8 +278,7 @@ static void xhci_zero_64b_regs(struct xhci_hcd *xhci)
 	if (upper_32_bits(val))
 		xhci_write_64(xhci, 0, &xhci->op_regs->cmd_ring);
 
-	intrs = min_t(u32, HCS_MAX_INTRS(xhci->hcs_params1),
-		      ARRAY_SIZE(xhci->run_regs->ir_set));
+	intrs = min_t(u32, xhci->max_interrupters, ARRAY_SIZE(xhci->run_regs->ir_set));
 
 	for (i = 0; i < intrs; i++) {
 		struct xhci_intr_reg __iomem *ir;
@@ -471,15 +470,13 @@ static void xhci_hcd_page_size(struct xhci_hcd *xhci)
 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);
+		       xhci->max_slots);
 
 	config_reg = readl(&xhci->op_regs->config_reg);
 	config_reg &= ~HCS_SLOTS_MASK;
-	config_reg |= max_slots;
+	config_reg |= xhci->max_slots;
 
 	xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Setting Max device slots reg = 0x%x",
 		       config_reg);
@@ -4222,7 +4219,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 		xhci_err(xhci, "Error while assigning device slot ID: %s\n",
 			 xhci_trb_comp_code_string(command->status));
 		xhci_err(xhci, "Max number of devices this xHCI host supports is %u.\n",
-			 HCS_MAX_SLOTS(xhci->hcs_params1));
+			 xhci->max_slots);
 		xhci_free_command(xhci, command);
 		return 0;
 	}
@@ -5404,6 +5401,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	 */
 	struct device		*dev = hcd->self.sysdev;
 	int			retval;
+	u32			hcs_params1;
 
 	/* Accept arbitrarily long scatter-gather lists */
 	hcd->self.sg_tablesize = ~0;
@@ -5429,7 +5427,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	xhci->run_regs = hcd->regs +
 		(readl(&xhci->cap_regs->run_regs_off) & RTSOFF_MASK);
 	/* Cache read-only capability registers */
-	xhci->hcs_params1 = readl(&xhci->cap_regs->hcs_params1);
+	hcs_params1 = readl(&xhci->cap_regs->hcs_params1);
 	xhci->hcs_params2 = readl(&xhci->cap_regs->hcs_params2);
 	xhci->hcs_params3 = readl(&xhci->cap_regs->hcs_params3);
 	xhci->hci_version = HC_VERSION(readl(&xhci->cap_regs->hc_capbase));
@@ -5437,10 +5435,11 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	if (xhci->hci_version > 0x100)
 		xhci->hcc_params2 = readl(&xhci->cap_regs->hcc_params2);
 
+	xhci->max_slots = HCS_MAX_SLOTS(hcs_params1);
+	xhci->max_ports = HCS_MAX_PORTS(hcs_params1);
 	/* xhci-plat or xhci-pci might have set max_interrupters already */
-	if ((!xhci->max_interrupters) ||
-	    xhci->max_interrupters > HCS_MAX_INTRS(xhci->hcs_params1))
-		xhci->max_interrupters = HCS_MAX_INTRS(xhci->hcs_params1);
+	if ((!xhci->max_interrupters) || xhci->max_interrupters > HCS_MAX_INTRS(hcs_params1))
+		xhci->max_interrupters = HCS_MAX_INTRS(hcs_params1);
 
 	xhci->quirks |= quirks;
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 8792692ba236..acf52112ab4c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1509,7 +1509,6 @@ struct xhci_hcd {
 	struct xhci_doorbell_array __iomem *dba;
 
 	/* Cached register copies of read-only HC data */
-	__u32		hcs_params1;
 	__u32		hcs_params2;
 	__u32		hcs_params3;
 	__u32		hcc_params;
@@ -1520,6 +1519,8 @@ struct xhci_hcd {
 	/* packed release number */
 	u16		hci_version;
 	u16		max_interrupters;
+	u8		max_slots;
+	u8		max_ports;
 	/* imod_interval in ns (I * 250ns) */
 	u32		imod_interval;
 	u32		page_size;
-- 
2.50.1

[PATCH 05/13] usb: xhci: limit number of ports to 127

[Niklas Neronin]

The xHCI driver allocates various port-related structures based on the
maximum number of ports reported by the controller. The Number of Ports
(MaxPorts) field occupies bits 31:24 of the HCSPARAMS1 register and can
represent values up to 255. However, the 'HCS_MAX_PORTS()' macro currently
reads bits 30:24, effectively limiting the maximum to 127.

Fixing the macro increases the reported port limit to 255, which in turn
increases memory usage regardless of how many ports are actually used.

To maintain compatibility and control memory consumption, set
'xhci->max_ports' to the minimum of the value read from 'HCS_MAX_PORTS()'
and 127 (MAX_HC_PORTS). This preserves the existing limit while making
the restriction explicit and easier to adjust in the future.

Summary:
 * Port allocations are now limited to 127.
 * HC max ports macro now correctly reads the MaxPorts value.
 * Macro 'MAX_HC_PORTS' can be modified to set the port limit.

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

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index 89bc83e4f1eb..8390c969389e 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -12,8 +12,8 @@
 #define HCS_SLOTS_MASK		0xff
 /* bits 8:18, Max Interrupters */
 #define HCS_MAX_INTRS(p)	(((p) >> 8) & 0x7ff)
-/* bits 24:31, Max Ports - max value is 0x7F = 127 ports */
-#define HCS_MAX_PORTS(p)	(((p) >> 24) & 0x7f)
+/* bits 31:24, Max Ports - max value is 255 */
+#define HCS_MAX_PORTS(p)	(((p) >> 24) & 0xff)
 
 /* HCSPARAMS2 - hcs_params2 - bitmasks */
 /* bits 0:3, frames or uframes that SW needs to queue transactions
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d74f47a75c68..5eb3ea0d9e15 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5436,7 +5436,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 		xhci->hcc_params2 = readl(&xhci->cap_regs->hcc_params2);
 
 	xhci->max_slots = HCS_MAX_SLOTS(hcs_params1);
-	xhci->max_ports = HCS_MAX_PORTS(hcs_params1);
+	xhci->max_ports = min(HCS_MAX_PORTS(hcs_params1), MAX_HC_PORTS);
 	/* xhci-plat or xhci-pci might have set max_interrupters already */
 	if ((!xhci->max_interrupters) || xhci->max_interrupters > HCS_MAX_INTRS(hcs_params1))
 		xhci->max_interrupters = HCS_MAX_INTRS(hcs_params1);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index acf52112ab4c..e68bf547f90b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -34,7 +34,10 @@
 
 /* Max number of USB devices for any host controller - limit in section 6.1 */
 #define MAX_HC_SLOTS		256
-/* Section 5.3.3 - MaxPorts */
+/*
+ * Max Number of Ports. xHCI specification section 5.3.3
+ * Valid values are in the range of 1 to 255.
+ */
 #define MAX_HC_PORTS		127
 
 /*
-- 
2.50.1

[PATCH 06/13] usb: xhci: limit number of interrupts to 128

[Niklas Neronin]

The xHCI driver defines only 128 interrupter register slots, yet allows up
to 2047 interrupters. According to the xHCI specification, the maximum
valid number of interrupters is 1024. These mismatches can lead to
out-of-range accesses and excessive memory use.

The Number of Interrupters (MaxIntrs) field occupies bits 18:8 of the
HCSPARAMS1 register, which can yield a value up to 2047, although the
specification limits it to 1024. Cap the value using the 'MAX_HC_INTRS'
macro.

Set 'xhci->max_intrs' to the minimum of the value reported by the
HCSPARAMS1 register and 'MAX_HC_INTRS'. The interrupter register slot
array is defined for 1024 entries, serving only as a structural template
and not increasing memory usage.

Although the xHCI specification allows up to 1024 interrupters, raising
'MAX_HC_INTRS' above 128 provides no practical benefit. The driver only
uses the primary interrupter (0), and secondary interrupters (1+) are
rarely, if ever, used in practice. No reports exist of usage beyond 128.
Therefore, I have limited it to 128.

Summary:
 * Interrupter allocations are now limited to 128 from 2047.
 * Interrupter Register template slots are set to 1024 from 128.
 * Macro 'MAX_HC_INTRS' can be modified to set the interrupter limit.

==== Detailed interrupter explanation ====

There are two relevant components:

Interrupter array:
This holds the software interrupter structures and is allocated by the
xhci driver. The number of interrupters allocated is determined by the
HCSPARAMS1 register field, which specifies the supported interrupter
count.

Interrupter register slots:
This is a template struct used to access the hardware's runtime
registers. It is not allocated by the driver, the hardware defines and
owns this memory region, and the driver only maps it for MMIO access.

Each entry in the interrupter array points to its corresponding
interrupter register slot in the hardware region once that interrupter
is enabled.

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

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5eb3ea0d9e15..2015d37f863e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -237,7 +237,6 @@ static void xhci_zero_64b_regs(struct xhci_hcd *xhci)
 	struct iommu_domain *domain;
 	int err, i;
 	u64 val;
-	u32 intrs;
 
 	/*
 	 * Some Renesas controllers get into a weird state if they are
@@ -278,9 +277,7 @@ static void xhci_zero_64b_regs(struct xhci_hcd *xhci)
 	if (upper_32_bits(val))
 		xhci_write_64(xhci, 0, &xhci->op_regs->cmd_ring);
 
-	intrs = min_t(u32, xhci->max_interrupters, ARRAY_SIZE(xhci->run_regs->ir_set));
-
-	for (i = 0; i < intrs; i++) {
+	for (i = 0; i < xhci->max_interrupters; i++) {
 		struct xhci_intr_reg __iomem *ir;
 
 		ir = &xhci->run_regs->ir_set[i];
@@ -5438,7 +5435,9 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	xhci->max_slots = HCS_MAX_SLOTS(hcs_params1);
 	xhci->max_ports = min(HCS_MAX_PORTS(hcs_params1), MAX_HC_PORTS);
 	/* xhci-plat or xhci-pci might have set max_interrupters already */
-	if ((!xhci->max_interrupters) || xhci->max_interrupters > HCS_MAX_INTRS(hcs_params1))
+	if (!xhci->max_interrupters)
+		xhci->max_interrupters = min(HCS_MAX_INTRS(hcs_params1), MAX_HC_INTRS);
+	else if (xhci->max_interrupters > HCS_MAX_INTRS(hcs_params1))
 		xhci->max_interrupters = HCS_MAX_INTRS(hcs_params1);
 
 	xhci->quirks |= quirks;
@@ -5658,8 +5657,8 @@ static int __init xhci_hcd_init(void)
 	BUILD_BUG_ON(sizeof(struct xhci_erst_entry) != 4*32/8);
 	BUILD_BUG_ON(sizeof(struct xhci_cap_regs) != 8*32/8);
 	BUILD_BUG_ON(sizeof(struct xhci_intr_reg) != 8*32/8);
-	/* xhci_run_regs has eight fields and embeds 128 xhci_intr_regs */
-	BUILD_BUG_ON(sizeof(struct xhci_run_regs) != (8+8*128)*32/8);
+	/* xhci_run_regs has eight fields and embeds 1024 xhci_intr_regs */
+	BUILD_BUG_ON(sizeof(struct xhci_run_regs) != (8+8*1024)*32/8);
 
 	if (usb_disabled())
 		return -ENODEV;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e68bf547f90b..50432241961f 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -39,6 +39,11 @@
  * Valid values are in the range of 1 to 255.
  */
 #define MAX_HC_PORTS		127
+/*
+ * Max number of Interrupter Register Sets. xHCI specification section 5.3.3
+ * Valid values are in the range of 1 to 1024.
+ */
+#define MAX_HC_INTRS		128
 
 /*
  * xHCI register interface.
@@ -287,7 +292,7 @@ struct xhci_intr_reg {
 struct xhci_run_regs {
 	__le32			microframe_index;
 	__le32			rsvd[7];
-	struct xhci_intr_reg	ir_set[128];
+	struct xhci_intr_reg	ir_set[1024];
 };
 
 /**
-- 
2.50.1

[PATCH 07/13] usb: xhci: improve xhci-caps.h comments

[Niklas Neronin]

No functional changes.

This patch updates comments in xhci-caps.h for better readability and
consistency. Each Capability Register bit field now includes a brief
description of its name and valid range, following a uniform comment
format across the file.

These updates are based on the xHCI specification, revision 1.2.

Bit field comment format:
/* <bit range> - <Field name>,<noteworthy information if any> */

Why print the bit range?
 The bit range aids in identifying missing macros and reserved bit ranges.

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

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index 8390c969389e..8a435786f950 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -1,93 +1,107 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * xHCI Host Controller Capability Registers.
+ * xHCI Specification Section 5.3, Revision 1.2.
+ */
 
-/* hc_capbase bitmasks */
-/* bits 7:0 - how long is the Capabilities register */
+/* hc_capbase - bitmasks */
+/* bits 7:0 - Capability Registers Length */
 #define HC_LENGTH(p)		XHCI_HC_LENGTH(p)
-/* bits 31:16	*/
+/* bits 15:8 - Rsvd */
+/* bits 31:16 - Host Controller Interface Version Number */
 #define HC_VERSION(p)		(((p) >> 16) & 0xffff)
 
 /* HCSPARAMS1 - hcs_params1 - bitmasks */
-/* bits 0:7, Max Device Slots */
+/* bits 7:0 - Number of Device Slots */
 #define HCS_MAX_SLOTS(p)	(((p) >> 0) & 0xff)
 #define HCS_SLOTS_MASK		0xff
-/* bits 8:18, Max Interrupters */
+/* bits 18:8 - Number of Interrupters, max values is 1024 */
 #define HCS_MAX_INTRS(p)	(((p) >> 8) & 0x7ff)
 /* bits 31:24, Max Ports - max value is 255 */
 #define HCS_MAX_PORTS(p)	(((p) >> 24) & 0xff)
 
 /* HCSPARAMS2 - hcs_params2 - bitmasks */
-/* bits 0:3, frames or uframes that SW needs to queue transactions
- * ahead of the HW to meet periodic deadlines */
+/*
+ * bits 3:0 - Isochronous Scheduling Threshold, frames or uframes that SW
+ * needs to queue transactions ahead of the HW to meet periodic deadlines.
+ */
 #define HCS_IST(p)		(((p) >> 0) & 0xf)
-/* bits 4:7, max number of Event Ring segments */
+/* bits 7:4 - Event Ring Segment Table Max, 2^(n) */
 #define HCS_ERST_MAX(p)		(((p) >> 4) & 0xf)
-/* bits 21:25 Hi 5 bits of Scratchpad buffers SW must allocate for the HW */
-/* bit 26 Scratchpad restore - for save/restore HW state - not used yet */
-/* bits 27:31 Lo 5 bits of Scratchpad buffers SW must allocate for the HW */
+/* bits 20:8 - Rsvd */
+/* bits 25:21 - Max Scratchpad Buffers (Hi), 5 Most significant bits */
+/* bit 26 - Scratchpad restore, for save/restore HW state */
+/* bits 31:27 - Max Scratchpad Buffers (Lo), 5 Least significant bits */
 #define HCS_MAX_SCRATCHPAD(p)   ((((p) >> 16) & 0x3e0) | (((p) >> 27) & 0x1f))
 
 /* HCSPARAMS3 - hcs_params3 - bitmasks */
-/* bits 0:7, Max U1 to U0 latency for the roothub ports */
+/* bits 7:0 - U1 Device Exit Latency, Max U1 to U0 latency for the roothub ports */
 #define HCS_U1_LATENCY(p)	(((p) >> 0) & 0xff)
-/* bits 16:31, Max U2 to U0 latency for the roothub ports */
+/* bits 15:8 - Rsvd */
+/* bits 31:16 - U2 Device Exit Latency, Max U2 to U0 latency for the roothub ports */
 #define HCS_U2_LATENCY(p)	(((p) >> 16) & 0xffff)
 
-/* HCCPARAMS - hcc_params - bitmasks */
-/* true: HC can use 64-bit address pointers */
+/* HCCPARAMS1 - hcc_params - bitmasks */
+/* bit 0 - 64-bit Addressing Capability */
 #define HCC_64BIT_ADDR(p)	((p) & (1 << 0))
-/* true: HC can do bandwidth negotiation */
+/* bit 1 - BW Negotiation Capability */
 #define HCC_BANDWIDTH_NEG(p)	((p) & (1 << 1))
-/* true: HC uses 64-byte Device Context structures
- * FIXME 64-byte context structures aren't supported yet.
- */
+/* bit 2 - Context Size */
 #define HCC_64BYTE_CONTEXT(p)	((p) & (1 << 2))
-/* true: HC has port power switches */
+#define CTX_SIZE(_hcc)		(HCC_64BYTE_CONTEXT(_hcc) ? 64 : 32)
+/* bit 3 - Port Power Control */
 #define HCC_PPC(p)		((p) & (1 << 3))
-/* true: HC has port indicators */
+/* bit 4 - Port Indicators */
 #define HCS_INDICATOR(p)	((p) & (1 << 4))
-/* true: HC has Light HC Reset Capability */
+/* bit 5 - Light HC Reset Capability */
 #define HCC_LIGHT_RESET(p)	((p) & (1 << 5))
-/* true: HC supports latency tolerance messaging */
+/* bit 6 - Latency Tolerance Messaging Capability */
 #define HCC_LTC(p)		((p) & (1 << 6))
-/* true: no secondary Stream ID Support */
+/* bit 7 - No Secondary Stream ID Support */
 #define HCC_NSS(p)		((p) & (1 << 7))
-/* true: HC supports Stopped - Short Packet */
+/* bit 8 - Parse All Event Data */
+/* bit 9 - Short Packet Capability */
 #define HCC_SPC(p)		((p) & (1 << 9))
-/* true: HC has Contiguous Frame ID Capability */
+/* bit 10 - Stopped EDTLA Capability */
+/* bit 11 - Contiguous Frame ID Capability */
 #define HCC_CFC(p)		((p) & (1 << 11))
-/* Max size for Primary Stream Arrays - 2^(n+1), where n is bits 12:15 */
+/* bits 15:12 - Max size for Primary Stream Arrays, 2^(n+1) */
 #define HCC_MAX_PSA(p)		(1 << ((((p) >> 12) & 0xf) + 1))
-/* Extended Capabilities pointer from PCI base - section 5.3.6 */
+/* bits 31:16 - xHCI Extended Capabilities Pointer, from PCI base: 2^(n) */
 #define HCC_EXT_CAPS(p)		XHCI_HCC_EXT_CAPS(p)
 
-#define CTX_SIZE(_hcc)		(HCC_64BYTE_CONTEXT(_hcc) ? 64 : 32)
-
-/* db_off bitmask - bits 31:2 Doorbell Array Offset */
+/* DBOFF - db_off - bitmasks */
+/* bits 1:0 - Rsvd */
+/* bits 31:2 - Doorbell Array Offset */
 #define	DBOFF_MASK	(0xfffffffc)
 
-/* run_regs_off bitmask - bits 0:4 reserved */
+/* RTSOFF - run_regs_off - bitmasks */
+/* bits 4:0 - Rsvd */
+/* bits 31:5 - Runtime Register Space Offse */
 #define	RTSOFF_MASK	(~0x1f)
 
 /* HCCPARAMS2 - hcc_params2 - bitmasks */
-/* true: HC supports U3 entry Capability */
+/* bit 0 - U3 Entry Capability */
 #define	HCC2_U3C(p)		((p) & (1 << 0))
-/* true: HC supports Configure endpoint command Max exit latency too large */
+/* bit 1 - Configure Endpoint Command Max Exit Latency Too Large Capability */
 #define	HCC2_CMC(p)		((p) & (1 << 1))
-/* true: HC supports Force Save context Capability */
+/* bit 2 - Force Save Context Capabilitu */
 #define	HCC2_FSC(p)		((p) & (1 << 2))
-/* true: HC supports Compliance Transition Capability */
+/* bit 3 - Compliance Transition Capability, false: compliance is enabled by default */
 #define	HCC2_CTC(p)		((p) & (1 << 3))
-/* true: HC support Large ESIT payload Capability > 48k */
+/* bit 4 - Large ESIT Payload Capability, true: HC support ESIT payload > 48k */
 #define	HCC2_LEC(p)		((p) & (1 << 4))
-/* true: HC support Configuration Information Capability */
+/* bit 5 - Configuration Information Capability */
 #define	HCC2_CIC(p)		((p) & (1 << 5))
-/* true: HC support Extended TBC Capability, Isoc burst count > 65535 */
+/* bit 6 - Extended TBC Capability, true: Isoc burst count > 65535 */
 #define	HCC2_ETC(p)		((p) & (1 << 6))
-/* true: HC support Extended TBC TRB Status Capability */
+/* bit 7 - Extended TBC TRB Status Capability */
 #define HCC2_ETC_TSC(p)         ((p) & (1 << 7))
-/* true: HC support Get/Set Extended Property Capability */
+/* bit 8 - Get/Set Extended Property Capability */
 #define HCC2_GSC(p)             ((p) & (1 << 8))
-/* true: HC support Virtualization Based Trusted I/O Capability */
+/* bit 9 - Virtualization Based Trusted I/O Capability */
 #define HCC2_VTC(p)             ((p) & (1 << 9))
-/* true: HC support Double BW on a eUSB2 HS ISOC EP */
+/* bit 10 - Rsvd */
+/* bit 11 - HC support Double BW on a eUSB2 HS ISOC EP */
 #define HCC2_EUSB2_DIC(p)       ((p) & (1 << 11))
+/* bits 31:12 - Rsvd */
-- 
2.50.1

[PATCH 08/13] usb: xhci: simplify Isochronous Scheduling Threshold handling

[Niklas Neronin]

The IST is represented by bits 2:0, with bit 3 indicating the unit of
measurement, Frames or Microframes. Introduce xhci_ist_in_microseconds(),
which returns the IST value in Microframes, simplifying the code and
reducing duplication.

Improve documentation in xhci-caps.h to clarify the IST register specifics,
including the unit conversion details. These change removes the need to
explain it each time the IST values is retrieved.

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

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index 8a435786f950..e772d5f30d36 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -24,8 +24,15 @@
 /*
  * bits 3:0 - Isochronous Scheduling Threshold, frames or uframes that SW
  * needs to queue transactions ahead of the HW to meet periodic deadlines.
+ * - Bits 2:0: Threshold value
+ * - Bit 3: Unit indicator
+ *   - '1': Threshold in Frames
+ *   - '0': Threshold in Microframes (uframes)
+ * Note: 1 Frame = 8 Microframes
+ * xHCI specification section 5.3.4.
  */
-#define HCS_IST(p)		(((p) >> 0) & 0xf)
+#define HCS_IST_VALUE(p)	((p) & 0x7)
+#define HCS_IST_UNIT(p)		((p) & (1 << 3))
 /* bits 7:4 - Event Ring Segment Table Max, 2^(n) */
 #define HCS_ERST_MAX(p)		(((p) >> 4) & 0xf)
 /* bits 20:8 - Rsvd */
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 5f46661c8e6b..7b7c61d79d0a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3983,6 +3983,16 @@ static unsigned int xhci_get_last_burst_packet_count(struct xhci_hcd *xhci,
 	return total_packet_count - 1;
 }
 
+/* Returns the Isochronous Scheduling Threshold in Microframes. 1 Frame is 8 Microframes. */
+static int xhci_ist_in_microseconds(u32 hcs_params2)
+{
+	int ist = HCS_IST_VALUE(hcs_params2);
+
+	if (HCS_IST_UNIT(hcs_params2))
+		ist *= 8;
+	return ist;
+}
+
 /*
  * Calculates Frame ID field of the isochronous TRB identifies the
  * target frame that the Interval associated with this Isochronous
@@ -4002,17 +4012,7 @@ static int xhci_get_isoc_frame_id(struct xhci_hcd *xhci,
 	else
 		start_frame = (urb->start_frame + index * urb->interval) >> 3;
 
-	/* Isochronous Scheduling Threshold (IST, bits 0~3 in HCSPARAMS2):
-	 *
-	 * If bit [3] of IST is cleared to '0', software can add a TRB no
-	 * later than IST[2:0] Microframes before that TRB is scheduled to
-	 * be executed.
-	 * If bit [3] of IST is set to '1', software can add a TRB no later
-	 * than IST[2:0] Frames before that TRB is scheduled to be executed.
-	 */
-	ist = HCS_IST(xhci->hcs_params2) & 0x7;
-	if (HCS_IST(xhci->hcs_params2) & (1 << 3))
-		ist <<= 3;
+	ist = xhci_ist_in_microseconds(xhci->hcs_params2);
 
 	/* Software shall not schedule an Isoch TD with a Frame ID value that
 	 * is less than the Start Frame ID or greater than the End Frame ID,
@@ -4333,9 +4333,7 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
 	 * Round up to the next frame and consider the time before trb really
 	 * gets scheduled by hardare.
 	 */
-	ist = HCS_IST(xhci->hcs_params2) & 0x7;
-	if (HCS_IST(xhci->hcs_params2) & (1 << 3))
-		ist <<= 3;
+	ist = xhci_ist_in_microseconds(xhci->hcs_params2);
 	start_frame += ist + XHCI_CFC_DELAY;
 	start_frame = roundup(start_frame, 8);
 
-- 
2.50.1

Re: [PATCH 08/13] usb: xhci: simplify Isochronous Scheduling Threshold handling

[Michal Pecio]

On Mon, 10 Nov 2025 16:14:45 +0100, Niklas Neronin wrote:
> The IST is represented by bits 2:0, with bit 3 indicating the unit of
> measurement, Frames or Microframes. Introduce
> xhci_ist_in_microseconds(), which returns the IST value in
> Microframes, simplifying the code and reducing duplication.
> 
> Improve documentation in xhci-caps.h to clarify the IST register
> specifics, including the unit conversion details. These change
> removes the need to explain it each time the IST values is retrieved.
>
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> ---
>  drivers/usb/host/xhci-caps.h |  9 ++++++++-
>  drivers/usb/host/xhci-ring.c | 26 ++++++++++++--------------
>  2 files changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
> index 8a435786f950..e772d5f30d36 100644
> --- a/drivers/usb/host/xhci-caps.h
> +++ b/drivers/usb/host/xhci-caps.h
> @@ -24,8 +24,15 @@
>  /*
>   * bits 3:0 - Isochronous Scheduling Threshold, frames or uframes that SW
>   * needs to queue transactions ahead of the HW to meet periodic deadlines.
> + * - Bits 2:0: Threshold value
> + * - Bit 3: Unit indicator
> + *   - '1': Threshold in Frames
> + *   - '0': Threshold in Microframes (uframes)
> + * Note: 1 Frame = 8 Microframes
> + * xHCI specification section 5.3.4.
>   */
> -#define HCS_IST(p)		(((p) >> 0) & 0xf)
> +#define HCS_IST_VALUE(p)	((p) & 0x7)
> +#define HCS_IST_UNIT(p)		((p) & (1 << 3))
>  /* bits 7:4 - Event Ring Segment Table Max, 2^(n) */
>  #define HCS_ERST_MAX(p)		(((p) >> 4) & 0xf)
>  /* bits 20:8 - Rsvd */
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 5f46661c8e6b..7b7c61d79d0a 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3983,6 +3983,16 @@ static unsigned int xhci_get_last_burst_packet_count(struct xhci_hcd *xhci,
>  	return total_packet_count - 1;
>  }
>  
> +/* Returns the Isochronous Scheduling Threshold in Microframes. 1 Frame is 8 Microframes. */
> +static int xhci_ist_in_microseconds(u32 hcs_params2)

Not a great name, too long and "seconds" surely doesn't belong here :)

xhci_ist_microframes() seems better.

The argument could be 'xhci' to simplify callers and deduplicate more.

> +{
> +	int ist = HCS_IST_VALUE(hcs_params2);
> +
> +	if (HCS_IST_UNIT(hcs_params2))
> +		ist *= 8;
> +	return ist;
> +}
> +
>  /*
>   * Calculates Frame ID field of the isochronous TRB identifies the
>   * target frame that the Interval associated with this Isochronous
> @@ -4002,17 +4012,7 @@ static int xhci_get_isoc_frame_id(struct xhci_hcd *xhci,
>  	else
>  		start_frame = (urb->start_frame + index * urb->interval) >> 3;
>  
> -	/* Isochronous Scheduling Threshold (IST, bits 0~3 in HCSPARAMS2):
> -	 *
> -	 * If bit [3] of IST is cleared to '0', software can add a TRB no
> -	 * later than IST[2:0] Microframes before that TRB is scheduled to
> -	 * be executed.
> -	 * If bit [3] of IST is set to '1', software can add a TRB no later
> -	 * than IST[2:0] Frames before that TRB is scheduled to be executed.
> -	 */
> -	ist = HCS_IST(xhci->hcs_params2) & 0x7;
> -	if (HCS_IST(xhci->hcs_params2) & (1 << 3))
> -		ist <<= 3;
> +	ist = xhci_ist_in_microseconds(xhci->hcs_params2);
>  
>  	/* Software shall not schedule an Isoch TD with a Frame ID value that
>  	 * is less than the Start Frame ID or greater than the End Frame ID,
> @@ -4333,9 +4333,7 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
>  	 * Round up to the next frame and consider the time before trb really
>  	 * gets scheduled by hardare.
>  	 */
> -	ist = HCS_IST(xhci->hcs_params2) & 0x7;
> -	if (HCS_IST(xhci->hcs_params2) & (1 << 3))
> -		ist <<= 3;
> +	ist = xhci_ist_in_microseconds(xhci->hcs_params2);
>  	start_frame += ist + XHCI_CFC_DELAY;
>  	start_frame = roundup(start_frame, 8);
>  
> -- 
> 2.50.1
> 

[PATCH 09/13] usb: xhci: simplify Max Scratchpad buffer macros

[Niklas Neronin]

Max Scratchpad Buffers consist of two bit-fields:
bits 25:21 - Max Scratchpad Buffers High, 5 Most significant bits
bits 27:31 - Max Scratchpad Buffers Low, 5 Least significant bits
Combined they create the Max Scratchpad Buffers value.

Add two new macros, 'HCS_MAX_SP_HI' and 'HCS_MAX_SP_LO', to separately
extract the high and low parts of the Max Scratchpad Buffers. These are
then combined using 'HCS_MAX_SCRATCHPAD' macro. This change simplifies
the code and makes it similar to other split value register macros in the
xhci driver.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-caps.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index e772d5f30d36..af47aebc5ba8 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -37,9 +37,11 @@
 #define HCS_ERST_MAX(p)		(((p) >> 4) & 0xf)
 /* bits 20:8 - Rsvd */
 /* bits 25:21 - Max Scratchpad Buffers (Hi), 5 Most significant bits */
+#define HCS_MAX_SP_HI(p)	(((p) >> 21) & 0x1f)
 /* bit 26 - Scratchpad restore, for save/restore HW state */
 /* bits 31:27 - Max Scratchpad Buffers (Lo), 5 Least significant bits */
-#define HCS_MAX_SCRATCHPAD(p)   ((((p) >> 16) & 0x3e0) | (((p) >> 27) & 0x1f))
+#define HCS_MAX_SP_LO(p)	(((p) >> 27) & 0x1f)
+#define HCS_MAX_SCRATCHPAD(p)	(HCS_MAX_SP_HI(p) << 5 | HCS_MAX_SP_LO(p))
 
 /* HCSPARAMS3 - hcs_params3 - bitmasks */
 /* bits 7:0 - U1 Device Exit Latency, Max U1 to U0 latency for the roothub ports */
-- 
2.50.1

[PATCH 10/13] usb: xhci: drop xhci-caps.h dependence on xhci-ext-caps.h

[Niklas Neronin]

Drop the dependency of xhci-caps.h on xhci-ext-caps.h by eliminating 2
instances where macros in xhci-caps.h were redefined from xhci-ext-caps.h.

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

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index af47aebc5ba8..99557df89f88 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -6,7 +6,7 @@
 
 /* hc_capbase - bitmasks */
 /* bits 7:0 - Capability Registers Length */
-#define HC_LENGTH(p)		XHCI_HC_LENGTH(p)
+#define HC_LENGTH(p)		((p) & 0xff)
 /* bits 15:8 - Rsvd */
 /* bits 31:16 - Host Controller Interface Version Number */
 #define HC_VERSION(p)		(((p) >> 16) & 0xffff)
@@ -77,7 +77,7 @@
 /* bits 15:12 - Max size for Primary Stream Arrays, 2^(n+1) */
 #define HCC_MAX_PSA(p)		(1 << ((((p) >> 12) & 0xf) + 1))
 /* bits 31:16 - xHCI Extended Capabilities Pointer, from PCI base: 2^(n) */
-#define HCC_EXT_CAPS(p)		XHCI_HCC_EXT_CAPS(p)
+#define HCC_EXT_CAPS(p)		(((p) >> 16) & 0xffff)
 
 /* DBOFF - db_off - bitmasks */
 /* bits 1:0 - Rsvd */
-- 
2.50.1

[PATCH 11/13] usb: xhci: standardize single bit-field macros

[Niklas Neronin]

Convert single bit-field macros to simple masks. The change makes the
masks more universal. Multi bit-field macros are changed in the next
commit. After both changes, all masks in xhci-caps.h will follow the
same format. I plan to introduce this change to all xhci macros.

Bit shift operations on a 32-bit signed can be problematic on some
architectures. Instead use BIT() macro, which returns a 64-bit unsigned
value. This ensures that the shift operation is performed on an unsigned
type, which is safer and more portable across different architectures.
Using unsigned integers for bit shifts avoids issues related to sign bits
and ensures consistent behavior.

Switch from 32-bit to 64-bit?
As far as I am aware, this does not cause any issues.
Performing bitwise operations between 32 and 64 bit values, the smaller
operand is promoted to match the size of the larger one, resulting in a
64-bit operation. This promotion extends the 32-bit value to 64 bits,
by zero-padding (for unsigned).

Will the change to 64-bit slow down the xhci driver?
On a 64-bit architecture - No. On a 32-bit architecture, yes? but in my
opinion the performance decrease does not outweigh the readability and
other benefits of using BIT() macro.

Why not use FIELD_GET() and FIELD_PREP()?
While they can be used for single bit macros, I prefer to use simple
bitwise operation directly. Because, it takes less space, is less overhead
and is as clear as if using FIELD_GET() and FIELD_PREP().

Why not use test_bit() macro?
Same reason as with FIELD_GET() and FIELD_PREP().

Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-caps.h    | 50 +++++++++++++++++----------------
 drivers/usb/host/xhci-debugfs.c |  2 +-
 drivers/usb/host/xhci-hub.c     |  6 ++--
 drivers/usb/host/xhci-mem.c     |  7 ++---
 drivers/usb/host/xhci-ring.c    |  8 +++---
 drivers/usb/host/xhci-trace.h   |  2 +-
 drivers/usb/host/xhci.c         |  2 +-
 7 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index 99557df89f88..89dd46767885 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -4,6 +4,8 @@
  * xHCI Specification Section 5.3, Revision 1.2.
  */
 
+#include <linux/bits.h>
+
 /* hc_capbase - bitmasks */
 /* bits 7:0 - Capability Registers Length */
 #define HC_LENGTH(p)		((p) & 0xff)
@@ -32,7 +34,7 @@
  * xHCI specification section 5.3.4.
  */
 #define HCS_IST_VALUE(p)	((p) & 0x7)
-#define HCS_IST_UNIT(p)		((p) & (1 << 3))
+#define HCS_IST_UNIT		BIT(3)
 /* bits 7:4 - Event Ring Segment Table Max, 2^(n) */
 #define HCS_ERST_MAX(p)		(((p) >> 4) & 0xf)
 /* bits 20:8 - Rsvd */
@@ -52,30 +54,30 @@
 
 /* HCCPARAMS1 - hcc_params - bitmasks */
 /* bit 0 - 64-bit Addressing Capability */
-#define HCC_64BIT_ADDR(p)	((p) & (1 << 0))
+#define HCC_64BIT_ADDR		BIT(0)
 /* bit 1 - BW Negotiation Capability */
-#define HCC_BANDWIDTH_NEG(p)	((p) & (1 << 1))
+#define HCC_BANDWIDTH_NEG	BIT(1)
 /* bit 2 - Context Size */
-#define HCC_64BYTE_CONTEXT(p)	((p) & (1 << 2))
-#define CTX_SIZE(_hcc)		(HCC_64BYTE_CONTEXT(_hcc) ? 64 : 32)
+#define HCC_64BYTE_CONTEXT	BIT(2)
+#define CTX_SIZE(_hcc)		(_hcc & HCC_64BYTE_CONTEXT ? 64 : 32)
 /* bit 3 - Port Power Control */
-#define HCC_PPC(p)		((p) & (1 << 3))
+#define HCC_PPC			BIT(3)
 /* bit 4 - Port Indicators */
-#define HCS_INDICATOR(p)	((p) & (1 << 4))
+#define HCS_INDICATOR		BIT(4)
 /* bit 5 - Light HC Reset Capability */
-#define HCC_LIGHT_RESET(p)	((p) & (1 << 5))
+#define HCC_LIGHT_RESET		BIT(5)
 /* bit 6 - Latency Tolerance Messaging Capability */
-#define HCC_LTC(p)		((p) & (1 << 6))
+#define HCC_LTC			BIT(6)
 /* bit 7 - No Secondary Stream ID Support */
-#define HCC_NSS(p)		((p) & (1 << 7))
+#define HCC_NSS			BIT(7)
 /* bit 8 - Parse All Event Data */
 /* bit 9 - Short Packet Capability */
-#define HCC_SPC(p)		((p) & (1 << 9))
+#define HCC_SPC			BIT(9)
 /* bit 10 - Stopped EDTLA Capability */
 /* bit 11 - Contiguous Frame ID Capability */
-#define HCC_CFC(p)		((p) & (1 << 11))
+#define HCC_CFC			BIT(11)
 /* bits 15:12 - Max size for Primary Stream Arrays, 2^(n+1) */
-#define HCC_MAX_PSA(p)		(1 << ((((p) >> 12) & 0xf) + 1))
+#define HCC_MAX_PSA(p)		BIT(((((p) >> 12) & 0xf) + 1))
 /* bits 31:16 - xHCI Extended Capabilities Pointer, from PCI base: 2^(n) */
 #define HCC_EXT_CAPS(p)		(((p) >> 16) & 0xffff)
 
@@ -91,26 +93,26 @@
 
 /* HCCPARAMS2 - hcc_params2 - bitmasks */
 /* bit 0 - U3 Entry Capability */
-#define	HCC2_U3C(p)		((p) & (1 << 0))
+#define	HCC2_U3C		BIT(0)
 /* bit 1 - Configure Endpoint Command Max Exit Latency Too Large Capability */
-#define	HCC2_CMC(p)		((p) & (1 << 1))
+#define	HCC2_CMC		BIT(1)
 /* bit 2 - Force Save Context Capabilitu */
-#define	HCC2_FSC(p)		((p) & (1 << 2))
+#define	HCC2_FSC		BIT(2)
 /* bit 3 - Compliance Transition Capability, false: compliance is enabled by default */
-#define	HCC2_CTC(p)		((p) & (1 << 3))
+#define	HCC2_CTC		BIT(3)
 /* bit 4 - Large ESIT Payload Capability, true: HC support ESIT payload > 48k */
-#define	HCC2_LEC(p)		((p) & (1 << 4))
+#define	HCC2_LEC		BIT(4)
 /* bit 5 - Configuration Information Capability */
-#define	HCC2_CIC(p)		((p) & (1 << 5))
+#define	HCC2_CIC		BIT(5)
 /* bit 6 - Extended TBC Capability, true: Isoc burst count > 65535 */
-#define	HCC2_ETC(p)		((p) & (1 << 6))
+#define	HCC2_ETC		BIT(6)
 /* bit 7 - Extended TBC TRB Status Capability */
-#define HCC2_ETC_TSC(p)         ((p) & (1 << 7))
+#define HCC2_ETC_TSC		BIT(7)
 /* bit 8 - Get/Set Extended Property Capability */
-#define HCC2_GSC(p)             ((p) & (1 << 8))
+#define HCC2_GSC		BIT(8)
 /* bit 9 - Virtualization Based Trusted I/O Capability */
-#define HCC2_VTC(p)             ((p) & (1 << 9))
+#define HCC2_VTC		BIT(9)
 /* bit 10 - Rsvd */
 /* bit 11 - HC support Double BW on a eUSB2 HS ISOC EP */
-#define HCC2_EUSB2_DIC(p)       ((p) & (1 << 11))
+#define HCC2_EUSB2_DIC		BIT(11)
 /* bits 31:12 - Rsvd */
diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index f0475cf8eef8..e45545fa3c66 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -355,7 +355,7 @@ static ssize_t xhci_port_write(struct file *file,  const char __user *ubuf,
 
 	if (!strncmp(buf, "compliance", 10)) {
 		/* If CTC is clear, compliance is enabled by default */
-		if (!HCC2_CTC(xhci->hcc_params2))
+		if (!(xhci->hcc_params2 & HCC2_CTC))
 			return count;
 		spin_lock_irqsave(&xhci->lock, flags);
 		/* compliance mode can only be enabled on ports in RxDetect */
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index e7c1952cffc5..422028ebce49 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -110,7 +110,7 @@ static int xhci_create_usb3x_bos_desc(struct xhci_hcd *xhci, char *buf,
 	ss_cap->bU2DevExitLat = 0; /* set later */
 
 	reg = readl(&xhci->cap_regs->hcc_params);
-	if (HCC_LTC(reg))
+	if (reg & HCC_LTC)
 		ss_cap->bmAttributes |= USB_LTM_SUPPORT;
 
 	if ((xhci->quirks & XHCI_LPM_SUPPORT)) {
@@ -263,7 +263,7 @@ static void xhci_common_hub_descriptor(struct xhci_hcd *xhci,
 	desc->bNbrPorts = ports;
 	temp = 0;
 	/* Bits 1:0 - support per-port power switching, or power always on */
-	if (HCC_PPC(xhci->hcc_params))
+	if (xhci->hcc_params & HCC_PPC)
 		temp |= HUB_CHAR_INDV_PORT_LPSM;
 	else
 		temp |= HUB_CHAR_NO_LPSM;
@@ -1400,7 +1400,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			 * automatically entered as on 1.0 and prior.
 			 */
 			if (link_state == USB_SS_PORT_LS_COMP_MOD) {
-				if (!HCC2_CTC(xhci->hcc_params2)) {
+				if (!(xhci->hcc_params2 & HCC2_CTC)) {
 					xhci_dbg(xhci, "CTC flag is 0, port already supports entering compliance mode\n");
 					break;
 				}
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 98abf86e0910..06ff712c9cbe 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -463,7 +463,7 @@ struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
 		return NULL;
 
 	ctx->type = type;
-	ctx->size = HCC_64BYTE_CONTEXT(xhci->hcc_params) ? 2048 : 1024;
+	ctx->size = xhci->hcc_params & HCC_64BYTE_CONTEXT ? 2048 : 1024;
 	if (type == XHCI_CTX_TYPE_INPUT)
 		ctx->size += CTX_SIZE(xhci->hcc_params);
 
@@ -1344,7 +1344,7 @@ static u32 xhci_get_endpoint_mult(struct xhci_hcd *xhci,
 	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);
+	lec = xhci->hci_version > 0x100 && (xhci->hcc_params2 & HCC2_LEC);
 
 	/* eUSB2 double isoc bw devices are the only USB2 devices using mult */
 	if (usb_endpoint_is_hs_isoc_double(udev, ep) &&
@@ -1433,8 +1433,7 @@ 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 (usb_endpoint_is_hs_isoc_double(udev, ep) &&
-	    !HCC2_EUSB2_DIC(xhci->hcc_params2))	{
+	if (usb_endpoint_is_hs_isoc_double(udev, ep) && !(xhci->hcc_params2 & HCC2_EUSB2_DIC))	{
 		dev_dbg(&udev->dev, "Double Isoc Bandwidth not supported by xhci\n");
 		return -EINVAL;
 	}
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7b7c61d79d0a..8d69f82a997c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3988,7 +3988,7 @@ static int xhci_ist_in_microseconds(u32 hcs_params2)
 {
 	int ist = HCS_IST_VALUE(hcs_params2);
 
-	if (HCS_IST_UNIT(hcs_params2))
+	if (hcs_params2 & HCS_IST_UNIT)
 		ist *= 8;
 	return ist;
 }
@@ -4157,7 +4157,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 		/* use SIA as default, if frame id is used overwrite it */
 		sia_frame_id = TRB_SIA;
 		if (!(urb->transfer_flags & URB_ISO_ASAP) &&
-		    HCC_CFC(xhci->hcc_params)) {
+		    (xhci->hcc_params & HCC_CFC)) {
 			frame_id = xhci_get_isoc_frame_id(xhci, urb, i);
 			if (frame_id >= 0)
 				sia_frame_id = TRB_FRAME_ID(frame_id);
@@ -4241,7 +4241,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
 	}
 
 	/* store the next frame id */
-	if (HCC_CFC(xhci->hcc_params))
+	if (xhci->hcc_params & HCC_CFC)
 		xep->next_frame_id = urb->start_frame + num_tds * urb->interval;
 
 	if (xhci_to_hcd(xhci)->self.bandwidth_isoc_reqs == 0) {
@@ -4320,7 +4320,7 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
 	check_interval(urb, ep_ctx);
 
 	/* Calculate the start frame and put it in urb->start_frame. */
-	if (HCC_CFC(xhci->hcc_params) && !list_empty(&ep_ring->td_list)) {
+	if ((xhci->hcc_params & HCC_CFC) && !list_empty(&ep_ring->td_list)) {
 		if (GET_EP_CTX_STATE(ep_ctx) ==	EP_STATE_RUNNING) {
 			urb->start_frame = xep->next_frame_id;
 			goto skip_start_over;
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index bf13da417f8e..c6baf82912de 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -81,7 +81,7 @@ DECLARE_EVENT_CLASS(xhci_log_ctx,
 	),
 	TP_fast_assign(
 
-		__entry->ctx_64 = HCC_64BYTE_CONTEXT(xhci->hcc_params);
+		__entry->ctx_64 = xhci->hcc_params & HCC_64BYTE_CONTEXT;
 		__entry->ctx_type = ctx->type;
 		__entry->ctx_dma = ctx->dma;
 		__entry->ctx_va = ctx->bytes;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2015d37f863e..c7a377b34661 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5483,7 +5483,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 
 	/* Set dma_mask and coherent_dma_mask to 64-bits,
 	 * if xHC supports 64-bit addressing */
-	if (HCC_64BIT_ADDR(xhci->hcc_params) &&
+	if ((xhci->hcc_params & HCC_64BIT_ADDR) &&
 			!dma_set_mask(dev, DMA_BIT_MASK(64))) {
 		xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
 		dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
-- 
2.50.1

Re: [PATCH 11/13] usb: xhci: standardize single bit-field macros

[Sakari Ailus]

Hi Niklas,

Thanks for this!

On Mon, Nov 10, 2025 at 04:14:48PM +0100, Niklas Neronin wrote:
> Convert single bit-field macros to simple masks. The change makes the
> masks more universal. Multi bit-field macros are changed in the next
> commit. After both changes, all masks in xhci-caps.h will follow the
> same format. I plan to introduce this change to all xhci macros.
> 
> Bit shift operations on a 32-bit signed can be problematic on some
> architectures. Instead use BIT() macro, which returns a 64-bit unsigned
> value. This ensures that the shift operation is performed on an unsigned
> type, which is safer and more portable across different architectures.
> Using unsigned integers for bit shifts avoids issues related to sign bits
> and ensures consistent behavior.
> 
> Switch from 32-bit to 64-bit?
> As far as I am aware, this does not cause any issues.
> Performing bitwise operations between 32 and 64 bit values, the smaller
> operand is promoted to match the size of the larger one, resulting in a
> 64-bit operation. This promotion extends the 32-bit value to 64 bits,
> by zero-padding (for unsigned).
> 
> Will the change to 64-bit slow down the xhci driver?
> On a 64-bit architecture - No. On a 32-bit architecture, yes? but in my
> opinion the performance decrease does not outweigh the readability and
> other benefits of using BIT() macro.
> 
> Why not use FIELD_GET() and FIELD_PREP()?
> While they can be used for single bit macros, I prefer to use simple
> bitwise operation directly. Because, it takes less space, is less overhead
> and is as clear as if using FIELD_GET() and FIELD_PREP().
> 
> Why not use test_bit() macro?
> Same reason as with FIELD_GET() and FIELD_PREP().
> 
> Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> ---
>  drivers/usb/host/xhci-caps.h    | 50 +++++++++++++++++----------------
>  drivers/usb/host/xhci-debugfs.c |  2 +-
>  drivers/usb/host/xhci-hub.c     |  6 ++--
>  drivers/usb/host/xhci-mem.c     |  7 ++---
>  drivers/usb/host/xhci-ring.c    |  8 +++---
>  drivers/usb/host/xhci-trace.h   |  2 +-
>  drivers/usb/host/xhci.c         |  2 +-
>  7 files changed, 39 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
> index 99557df89f88..89dd46767885 100644
> --- a/drivers/usb/host/xhci-caps.h
> +++ b/drivers/usb/host/xhci-caps.h
> @@ -4,6 +4,8 @@
>   * xHCI Specification Section 5.3, Revision 1.2.
>   */
>  
> +#include <linux/bits.h>
> +
>  /* hc_capbase - bitmasks */
>  /* bits 7:0 - Capability Registers Length */
>  #define HC_LENGTH(p)		((p) & 0xff)
> @@ -32,7 +34,7 @@
>   * xHCI specification section 5.3.4.
>   */
>  #define HCS_IST_VALUE(p)	((p) & 0x7)
> -#define HCS_IST_UNIT(p)		((p) & (1 << 3))
> +#define HCS_IST_UNIT		BIT(3)
>  /* bits 7:4 - Event Ring Segment Table Max, 2^(n) */
>  #define HCS_ERST_MAX(p)		(((p) >> 4) & 0xf)
>  /* bits 20:8 - Rsvd */
> @@ -52,30 +54,30 @@
>  
>  /* HCCPARAMS1 - hcc_params - bitmasks */
>  /* bit 0 - 64-bit Addressing Capability */
> -#define HCC_64BIT_ADDR(p)	((p) & (1 << 0))
> +#define HCC_64BIT_ADDR		BIT(0)
>  /* bit 1 - BW Negotiation Capability */
> -#define HCC_BANDWIDTH_NEG(p)	((p) & (1 << 1))
> +#define HCC_BANDWIDTH_NEG	BIT(1)
>  /* bit 2 - Context Size */
> -#define HCC_64BYTE_CONTEXT(p)	((p) & (1 << 2))
> -#define CTX_SIZE(_hcc)		(HCC_64BYTE_CONTEXT(_hcc) ? 64 : 32)
> +#define HCC_64BYTE_CONTEXT	BIT(2)
> +#define CTX_SIZE(_hcc)		(_hcc & HCC_64BYTE_CONTEXT ? 64 : 32)
>  /* bit 3 - Port Power Control */
> -#define HCC_PPC(p)		((p) & (1 << 3))
> +#define HCC_PPC			BIT(3)
>  /* bit 4 - Port Indicators */
> -#define HCS_INDICATOR(p)	((p) & (1 << 4))
> +#define HCS_INDICATOR		BIT(4)
>  /* bit 5 - Light HC Reset Capability */
> -#define HCC_LIGHT_RESET(p)	((p) & (1 << 5))
> +#define HCC_LIGHT_RESET		BIT(5)
>  /* bit 6 - Latency Tolerance Messaging Capability */
> -#define HCC_LTC(p)		((p) & (1 << 6))
> +#define HCC_LTC			BIT(6)
>  /* bit 7 - No Secondary Stream ID Support */
> -#define HCC_NSS(p)		((p) & (1 << 7))
> +#define HCC_NSS			BIT(7)
>  /* bit 8 - Parse All Event Data */
>  /* bit 9 - Short Packet Capability */
> -#define HCC_SPC(p)		((p) & (1 << 9))
> +#define HCC_SPC			BIT(9)
>  /* bit 10 - Stopped EDTLA Capability */
>  /* bit 11 - Contiguous Frame ID Capability */
> -#define HCC_CFC(p)		((p) & (1 << 11))
> +#define HCC_CFC			BIT(11)
>  /* bits 15:12 - Max size for Primary Stream Arrays, 2^(n+1) */
> -#define HCC_MAX_PSA(p)		(1 << ((((p) >> 12) & 0xf) + 1))
> +#define HCC_MAX_PSA(p)		BIT(((((p) >> 12) & 0xf) + 1))
>  /* bits 31:16 - xHCI Extended Capabilities Pointer, from PCI base: 2^(n) */
>  #define HCC_EXT_CAPS(p)		(((p) >> 16) & 0xffff)
>  
> @@ -91,26 +93,26 @@
>  
>  /* HCCPARAMS2 - hcc_params2 - bitmasks */
>  /* bit 0 - U3 Entry Capability */
> -#define	HCC2_U3C(p)		((p) & (1 << 0))
> +#define	HCC2_U3C		BIT(0)
>  /* bit 1 - Configure Endpoint Command Max Exit Latency Too Large Capability */
> -#define	HCC2_CMC(p)		((p) & (1 << 1))
> +#define	HCC2_CMC		BIT(1)
>  /* bit 2 - Force Save Context Capabilitu */
> -#define	HCC2_FSC(p)		((p) & (1 << 2))
> +#define	HCC2_FSC		BIT(2)
>  /* bit 3 - Compliance Transition Capability, false: compliance is enabled by default */
> -#define	HCC2_CTC(p)		((p) & (1 << 3))
> +#define	HCC2_CTC		BIT(3)
>  /* bit 4 - Large ESIT Payload Capability, true: HC support ESIT payload > 48k */
> -#define	HCC2_LEC(p)		((p) & (1 << 4))
> +#define	HCC2_LEC		BIT(4)
>  /* bit 5 - Configuration Information Capability */
> -#define	HCC2_CIC(p)		((p) & (1 << 5))
> +#define	HCC2_CIC		BIT(5)
>  /* bit 6 - Extended TBC Capability, true: Isoc burst count > 65535 */
> -#define	HCC2_ETC(p)		((p) & (1 << 6))
> +#define	HCC2_ETC		BIT(6)
>  /* bit 7 - Extended TBC TRB Status Capability */
> -#define HCC2_ETC_TSC(p)         ((p) & (1 << 7))
> +#define HCC2_ETC_TSC		BIT(7)
>  /* bit 8 - Get/Set Extended Property Capability */
> -#define HCC2_GSC(p)             ((p) & (1 << 8))
> +#define HCC2_GSC		BIT(8)
>  /* bit 9 - Virtualization Based Trusted I/O Capability */
> -#define HCC2_VTC(p)             ((p) & (1 << 9))
> +#define HCC2_VTC		BIT(9)
>  /* bit 10 - Rsvd */
>  /* bit 11 - HC support Double BW on a eUSB2 HS ISOC EP */
> -#define HCC2_EUSB2_DIC(p)       ((p) & (1 << 11))
> +#define HCC2_EUSB2_DIC		BIT(11)
>  /* bits 31:12 - Rsvd */
> diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
> index f0475cf8eef8..e45545fa3c66 100644
> --- a/drivers/usb/host/xhci-debugfs.c
> +++ b/drivers/usb/host/xhci-debugfs.c
> @@ -355,7 +355,7 @@ static ssize_t xhci_port_write(struct file *file,  const char __user *ubuf,
>  
>  	if (!strncmp(buf, "compliance", 10)) {
>  		/* If CTC is clear, compliance is enabled by default */
> -		if (!HCC2_CTC(xhci->hcc_params2))
> +		if (!(xhci->hcc_params2 & HCC2_CTC))
>  			return count;
>  		spin_lock_irqsave(&xhci->lock, flags);
>  		/* compliance mode can only be enabled on ports in RxDetect */
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index e7c1952cffc5..422028ebce49 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -110,7 +110,7 @@ static int xhci_create_usb3x_bos_desc(struct xhci_hcd *xhci, char *buf,
>  	ss_cap->bU2DevExitLat = 0; /* set later */
>  
>  	reg = readl(&xhci->cap_regs->hcc_params);
> -	if (HCC_LTC(reg))
> +	if (reg & HCC_LTC)
>  		ss_cap->bmAttributes |= USB_LTM_SUPPORT;
>  
>  	if ((xhci->quirks & XHCI_LPM_SUPPORT)) {
> @@ -263,7 +263,7 @@ static void xhci_common_hub_descriptor(struct xhci_hcd *xhci,
>  	desc->bNbrPorts = ports;
>  	temp = 0;
>  	/* Bits 1:0 - support per-port power switching, or power always on */
> -	if (HCC_PPC(xhci->hcc_params))
> +	if (xhci->hcc_params & HCC_PPC)
>  		temp |= HUB_CHAR_INDV_PORT_LPSM;
>  	else
>  		temp |= HUB_CHAR_NO_LPSM;
> @@ -1400,7 +1400,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>  			 * automatically entered as on 1.0 and prior.
>  			 */
>  			if (link_state == USB_SS_PORT_LS_COMP_MOD) {
> -				if (!HCC2_CTC(xhci->hcc_params2)) {
> +				if (!(xhci->hcc_params2 & HCC2_CTC)) {
>  					xhci_dbg(xhci, "CTC flag is 0, port already supports entering compliance mode\n");
>  					break;
>  				}
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 98abf86e0910..06ff712c9cbe 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -463,7 +463,7 @@ struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
>  		return NULL;
>  
>  	ctx->type = type;
> -	ctx->size = HCC_64BYTE_CONTEXT(xhci->hcc_params) ? 2048 : 1024;
> +	ctx->size = xhci->hcc_params & HCC_64BYTE_CONTEXT ? 2048 : 1024;
>  	if (type == XHCI_CTX_TYPE_INPUT)
>  		ctx->size += CTX_SIZE(xhci->hcc_params);
>  
> @@ -1344,7 +1344,7 @@ static u32 xhci_get_endpoint_mult(struct xhci_hcd *xhci,
>  	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);
> +	lec = xhci->hci_version > 0x100 && (xhci->hcc_params2 & HCC2_LEC);
>  
>  	/* eUSB2 double isoc bw devices are the only USB2 devices using mult */
>  	if (usb_endpoint_is_hs_isoc_double(udev, ep) &&
> @@ -1433,8 +1433,7 @@ 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 (usb_endpoint_is_hs_isoc_double(udev, ep) &&
> -	    !HCC2_EUSB2_DIC(xhci->hcc_params2))	{
> +	if (usb_endpoint_is_hs_isoc_double(udev, ep) && !(xhci->hcc_params2 & HCC2_EUSB2_DIC))	{
>  		dev_dbg(&udev->dev, "Double Isoc Bandwidth not supported by xhci\n");
>  		return -EINVAL;
>  	}
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 7b7c61d79d0a..8d69f82a997c 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3988,7 +3988,7 @@ static int xhci_ist_in_microseconds(u32 hcs_params2)
>  {
>  	int ist = HCS_IST_VALUE(hcs_params2);
>  
> -	if (HCS_IST_UNIT(hcs_params2))
> +	if (hcs_params2 & HCS_IST_UNIT)
>  		ist *= 8;
>  	return ist;
>  }
> @@ -4157,7 +4157,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
>  		/* use SIA as default, if frame id is used overwrite it */
>  		sia_frame_id = TRB_SIA;
>  		if (!(urb->transfer_flags & URB_ISO_ASAP) &&
> -		    HCC_CFC(xhci->hcc_params)) {
> +		    (xhci->hcc_params & HCC_CFC)) {
>  			frame_id = xhci_get_isoc_frame_id(xhci, urb, i);
>  			if (frame_id >= 0)
>  				sia_frame_id = TRB_FRAME_ID(frame_id);
> @@ -4241,7 +4241,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
>  	}
>  
>  	/* store the next frame id */
> -	if (HCC_CFC(xhci->hcc_params))
> +	if (xhci->hcc_params & HCC_CFC)
>  		xep->next_frame_id = urb->start_frame + num_tds * urb->interval;
>  
>  	if (xhci_to_hcd(xhci)->self.bandwidth_isoc_reqs == 0) {
> @@ -4320,7 +4320,7 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
>  	check_interval(urb, ep_ctx);
>  
>  	/* Calculate the start frame and put it in urb->start_frame. */
> -	if (HCC_CFC(xhci->hcc_params) && !list_empty(&ep_ring->td_list)) {
> +	if ((xhci->hcc_params & HCC_CFC) && !list_empty(&ep_ring->td_list)) {
>  		if (GET_EP_CTX_STATE(ep_ctx) ==	EP_STATE_RUNNING) {
>  			urb->start_frame = xep->next_frame_id;
>  			goto skip_start_over;
> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
> index bf13da417f8e..c6baf82912de 100644
> --- a/drivers/usb/host/xhci-trace.h
> +++ b/drivers/usb/host/xhci-trace.h
> @@ -81,7 +81,7 @@ DECLARE_EVENT_CLASS(xhci_log_ctx,
>  	),
>  	TP_fast_assign(
>  
> -		__entry->ctx_64 = HCC_64BYTE_CONTEXT(xhci->hcc_params);
> +		__entry->ctx_64 = xhci->hcc_params & HCC_64BYTE_CONTEXT;
>  		__entry->ctx_type = ctx->type;
>  		__entry->ctx_dma = ctx->dma;
>  		__entry->ctx_va = ctx->bytes;
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 2015d37f863e..c7a377b34661 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -5483,7 +5483,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
>  
>  	/* Set dma_mask and coherent_dma_mask to 64-bits,
>  	 * if xHC supports 64-bit addressing */
> -	if (HCC_64BIT_ADDR(xhci->hcc_params) &&
> +	if ((xhci->hcc_params & HCC_64BIT_ADDR) &&
>  			!dma_set_mask(dev, DMA_BIT_MASK(64))) {
>  		xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
>  		dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
> -- 
> 2.50.1
> 

-- 
Sakari Ailus

Re: [PATCH 11/13] usb: xhci: standardize single bit-field macros

[Mathias Nyman]

On 11/11/25 10:51, Sakari Ailus wrote:
> Hi Niklas,
> 
> Thanks for this!
> 
> On Mon, Nov 10, 2025 at 04:14:48PM +0100, Niklas Neronin wrote:
>> Convert single bit-field macros to simple masks. The change makes the
>> masks more universal. Multi bit-field macros are changed in the next
>> commit. After both changes, all masks in xhci-caps.h will follow the
>> same format. I plan to introduce this change to all xhci macros.
>>
>> Bit shift operations on a 32-bit signed can be problematic on some
>> architectures. Instead use BIT() macro, which returns a 64-bit unsigned
>> value. This ensures that the shift operation is performed on an unsigned
>> type, which is safer and more portable across different architectures.
>> Using unsigned integers for bit shifts avoids issues related to sign bits
>> and ensures consistent behavior.
>>
>> Switch from 32-bit to 64-bit?
>> As far as I am aware, this does not cause any issues.
>> Performing bitwise operations between 32 and 64 bit values, the smaller
>> operand is promoted to match the size of the larger one, resulting in a
>> 64-bit operation. This promotion extends the 32-bit value to 64 bits,
>> by zero-padding (for unsigned).
>>
>> Will the change to 64-bit slow down the xhci driver?
>> On a 64-bit architecture - No. On a 32-bit architecture, yes? but in my
>> opinion the performance decrease does not outweigh the readability and
>> other benefits of using BIT() macro.
>>
>> Why not use FIELD_GET() and FIELD_PREP()?
>> While they can be used for single bit macros, I prefer to use simple
>> bitwise operation directly. Because, it takes less space, is less overhead
>> and is as clear as if using FIELD_GET() and FIELD_PREP().
>>
>> Why not use test_bit() macro?
>> Same reason as with FIELD_GET() and FIELD_PREP().
>>
>> Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
> 
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
>> ---
>>   drivers/usb/host/xhci-caps.h    | 50 +++++++++++++++++----------------
>>   drivers/usb/host/xhci-debugfs.c |  2 +-
>>   drivers/usb/host/xhci-hub.c     |  6 ++--
>>   drivers/usb/host/xhci-mem.c     |  7 ++---
>>   drivers/usb/host/xhci-ring.c    |  8 +++---
>>   drivers/usb/host/xhci-trace.h   |  2 +-
>>   drivers/usb/host/xhci.c         |  2 +-
>>   7 files changed, 39 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
>> index 99557df89f88..89dd46767885 100644
>> --- a/drivers/usb/host/xhci-caps.h
>> +++ b/drivers/usb/host/xhci-caps.h
>> @@ -4,6 +4,8 @@
>>    * xHCI Specification Section 5.3, Revision 1.2.
>>    */
>>   
>> +#include <linux/bits.h>
>> +
>>   /* hc_capbase - bitmasks */
>>   /* bits 7:0 - Capability Registers Length */
>>   #define HC_LENGTH(p)		((p) & 0xff)
>> @@ -32,7 +34,7 @@
>>    * xHCI specification section 5.3.4.
>>    */
>>   #define HCS_IST_VALUE(p)	((p) & 0x7)
>> -#define HCS_IST_UNIT(p)		((p) & (1 << 3))
>> +#define HCS_IST_UNIT		BIT(3)
>>   /* bits 7:4 - Event Ring Segment Table Max, 2^(n) */
>>   #define HCS_ERST_MAX(p)		(((p) >> 4) & 0xf)
>>   /* bits 20:8 - Rsvd */
>> @@ -52,30 +54,30 @@
>>   
>>   /* HCCPARAMS1 - hcc_params - bitmasks */
>>   /* bit 0 - 64-bit Addressing Capability */
>> -#define HCC_64BIT_ADDR(p)	((p) & (1 << 0))
>> +#define HCC_64BIT_ADDR		BIT(0)
>>   /* bit 1 - BW Negotiation Capability */
>> -#define HCC_BANDWIDTH_NEG(p)	((p) & (1 << 1))
>> +#define HCC_BANDWIDTH_NEG	BIT(1)
>>   /* bit 2 - Context Size */
>> -#define HCC_64BYTE_CONTEXT(p)	((p) & (1 << 2))
>> -#define CTX_SIZE(_hcc)		(HCC_64BYTE_CONTEXT(_hcc) ? 64 : 32)
>> +#define HCC_64BYTE_CONTEXT	BIT(2)
>> +#define CTX_SIZE(_hcc)		(_hcc & HCC_64BYTE_CONTEXT ? 64 : 32)
>>   /* bit 3 - Port Power Control */
>> -#define HCC_PPC(p)		((p) & (1 << 3))
>> +#define HCC_PPC			BIT(3)
>>   /* bit 4 - Port Indicators */
>> -#define HCS_INDICATOR(p)	((p) & (1 << 4))
>> +#define HCS_INDICATOR		BIT(4)
>>   /* bit 5 - Light HC Reset Capability */
>> -#define HCC_LIGHT_RESET(p)	((p) & (1 << 5))
>> +#define HCC_LIGHT_RESET		BIT(5)
>>   /* bit 6 - Latency Tolerance Messaging Capability */
>> -#define HCC_LTC(p)		((p) & (1 << 6))
>> +#define HCC_LTC			BIT(6)
>>   /* bit 7 - No Secondary Stream ID Support */
>> -#define HCC_NSS(p)		((p) & (1 << 7))
>> +#define HCC_NSS			BIT(7)
>>   /* bit 8 - Parse All Event Data */
>>   /* bit 9 - Short Packet Capability */
>> -#define HCC_SPC(p)		((p) & (1 << 9))
>> +#define HCC_SPC			BIT(9)
>>   /* bit 10 - Stopped EDTLA Capability */
>>   /* bit 11 - Contiguous Frame ID Capability */
>> -#define HCC_CFC(p)		((p) & (1 << 11))
>> +#define HCC_CFC			BIT(11)
>>   /* bits 15:12 - Max size for Primary Stream Arrays, 2^(n+1) */
>> -#define HCC_MAX_PSA(p)		(1 << ((((p) >> 12) & 0xf) + 1))
>> +#define HCC_MAX_PSA(p)		BIT(((((p) >> 12) & 0xf) + 1))

The HCC_MAX_PSA(p) is a bit different, the bitshift here is a power of 2
calculation.

Primary stream array size is 2^(MaxPSASize + 1) where MaxPSASize is from
HCCPARAMS1 register bits 15:12

It's not pretty to begin with, and I don't think the BIT() macro makes it
any clearer.

Maybe leave it of from this series, possibly clean it up later

Thanks
Mathias

[PATCH 12/13] usb: xhci: standardize multi bit-field macros

[Niklas Neronin]

This patch aims to unify the format of register macros and masks within
the xHCI driver. Currently, register macros have inconsistent bit-field
masks, get macros, and set macros, with varying naming conventions and
functionalities.

==================== Proposal ====================
* Introduce a standardized approach by using only mask macros for each bit
  field, leveraging GENMASK() for enhanced clarity.

  #define HCC_MAX_PSA		GENMASK(15, 12)

* Utilize FIELD_GET() and FIELD_PREP() macros directly in the C code for
  getting and setting values, ensuring consistency and readability.

  u32 psa = FIELD_GET(HCC_MAX_PSA, reg);

* Maintain exceptions for macros that perform custom operations.

  #define CTX_SIZE(_hcc)	(_hcc & HCC_64BYTE_CONTEXT ? 64 : 32)

* Note, while FIELD_*() macros are beneficial, I am not suggesting that
  they should always be used. Instead, use them where they simplify the
  code and eliminate the necessity for custom get/set macros.
  In the example below, additional FIELD_PREP() or FIELD_MODIFY() is not
  beneficial.

  #define HCS_MAX_SCRATCHPAD(p)   (FIELD_GET(HCS_MAX_SP_HI, (p)) << 5 | \
				   FIELD_GET(HCS_MAX_SP_LO, (p)))

==================== Improvements ====================
Simplified Macros:
  By reducing custom macros, the code becomes more straightforward.
  Macros FIELD_GET() and FIELD_PREP() are commonly used, which contributes
  to the code readability and consistency.

  $ git grep -n 'FIELD_GET' | wc -l
  9027
  $ git grep -n 'FIELD_PREP' | wc -l
  15407

Consistent Return Type:
  All bit macros will return unsigned 64-bit values, mitigating potential
  cross-architecture issues.

Unified Bit Range Definition:
  The mask macro will define bit ranges, eliminating separate definitions
  for get/set macros. Because, FIELD_GET() & FIELD_PREP() use mask macro.

Cleaner header file with less macros:
  Fewer macros result in a cleaner and more manageable header file.

Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
 drivers/usb/host/xhci-caps.h    | 41 ++++++++++++++++++---------------
 drivers/usb/host/xhci-debugfs.c |  3 ++-
 drivers/usb/host/xhci-histb.c   |  2 +-
 drivers/usb/host/xhci-hub.c     |  4 ++--
 drivers/usb/host/xhci-mem.c     |  3 ++-
 drivers/usb/host/xhci-mtk.c     |  4 ++--
 drivers/usb/host/xhci-pci.c     |  4 +++-
 drivers/usb/host/xhci-plat.c    |  2 +-
 drivers/usb/host/xhci-ring.c    |  4 +++-
 drivers/usb/host/xhci-tegra.c   |  5 ++--
 drivers/usb/host/xhci.c         | 21 +++++++++--------
 11 files changed, 52 insertions(+), 41 deletions(-)

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index 89dd46767885..12445a6c9b51 100644
--- a/drivers/usb/host/xhci-caps.h
+++ b/drivers/usb/host/xhci-caps.h
@@ -5,22 +5,23 @@
  */
 
 #include <linux/bits.h>
+#include <linux/bitfield.h>
 
 /* hc_capbase - bitmasks */
 /* bits 7:0 - Capability Registers Length */
-#define HC_LENGTH(p)		((p) & 0xff)
+#define HC_LENGTH		GENMASK(7, 0)
 /* bits 15:8 - Rsvd */
 /* bits 31:16 - Host Controller Interface Version Number */
-#define HC_VERSION(p)		(((p) >> 16) & 0xffff)
+#define HC_VERSION		GENMASK(31, 16)
 
 /* HCSPARAMS1 - hcs_params1 - bitmasks */
 /* bits 7:0 - Number of Device Slots */
-#define HCS_MAX_SLOTS(p)	(((p) >> 0) & 0xff)
-#define HCS_SLOTS_MASK		0xff
-/* bits 18:8 - Number of Interrupters, max values is 1024 */
-#define HCS_MAX_INTRS(p)	(((p) >> 8) & 0x7ff)
-/* bits 31:24, Max Ports - max value is 255 */
-#define HCS_MAX_PORTS(p)	(((p) >> 24) & 0xff)
+#define HCS_SLOTS_MASK		GENMASK(7, 0)
+/* bits 18:8 - Number of Interrupters, max values is 1024  */
+#define HCS_MAX_INTRS		GENMASK(18, 8)
+/* bits 23:19 - Rsvd */
+/* bits 31:24 - Max Ports, max values is 255 */
+#define HCS_MAX_PORTS		GENMASK(31, 24)
 
 /* HCSPARAMS2 - hcs_params2 - bitmasks */
 /*
@@ -33,24 +34,25 @@
  * Note: 1 Frame = 8 Microframes
  * xHCI specification section 5.3.4.
  */
-#define HCS_IST_VALUE(p)	((p) & 0x7)
+#define HCS_IST_VALUE		GENMASK(2, 0)
 #define HCS_IST_UNIT		BIT(3)
 /* bits 7:4 - Event Ring Segment Table Max, 2^(n) */
-#define HCS_ERST_MAX(p)		(((p) >> 4) & 0xf)
+#define HCS_ERST_MAX		GENMASK(7, 4)
 /* bits 20:8 - Rsvd */
 /* bits 25:21 - Max Scratchpad Buffers (Hi), 5 Most significant bits */
-#define HCS_MAX_SP_HI(p)	(((p) >> 21) & 0x1f)
+#define HCS_MAX_SP_HI		GENMASK(25, 21)
 /* bit 26 - Scratchpad restore, for save/restore HW state */
 /* bits 31:27 - Max Scratchpad Buffers (Lo), 5 Least significant bits */
-#define HCS_MAX_SP_LO(p)	(((p) >> 27) & 0x1f)
-#define HCS_MAX_SCRATCHPAD(p)	(HCS_MAX_SP_HI(p) << 5 | HCS_MAX_SP_LO(p))
+#define HCS_MAX_SP_LO		GENMASK(31, 27)
+#define HCS_MAX_SCRATCHPAD(p)	(FIELD_GET(HCS_MAX_SP_HI, (p)) << 5 | \
+				 FIELD_GET(HCS_MAX_SP_LO, (p)))
 
 /* HCSPARAMS3 - hcs_params3 - bitmasks */
 /* bits 7:0 - U1 Device Exit Latency, Max U1 to U0 latency for the roothub ports */
-#define HCS_U1_LATENCY(p)	(((p) >> 0) & 0xff)
+#define HCS_U1_LATENCY		GENMASK(7, 0)
 /* bits 15:8 - Rsvd */
 /* bits 31:16 - U2 Device Exit Latency, Max U2 to U0 latency for the roothub ports */
-#define HCS_U2_LATENCY(p)	(((p) >> 16) & 0xffff)
+#define HCS_U2_LATENCY		GENMASK(31, 16)
 
 /* HCCPARAMS1 - hcc_params - bitmasks */
 /* bit 0 - 64-bit Addressing Capability */
@@ -77,19 +79,20 @@
 /* bit 11 - Contiguous Frame ID Capability */
 #define HCC_CFC			BIT(11)
 /* bits 15:12 - Max size for Primary Stream Arrays, 2^(n+1) */
-#define HCC_MAX_PSA(p)		BIT(((((p) >> 12) & 0xf) + 1))
+#define HCC_MAX_PSA		GENMASK(15, 12)
+#define GET_MAX_PSA_SIZE(p)	BIT(FIELD_GET(HCC_MAX_PSA, (p)) + 1)
 /* bits 31:16 - xHCI Extended Capabilities Pointer, from PCI base: 2^(n) */
-#define HCC_EXT_CAPS(p)		(((p) >> 16) & 0xffff)
+#define HCC_EXT_CAPS		GENMASK(31, 16)
 
 /* DBOFF - db_off - bitmasks */
 /* bits 1:0 - Rsvd */
 /* bits 31:2 - Doorbell Array Offset */
-#define	DBOFF_MASK	(0xfffffffc)
+#define	DBOFF_MASK		GENMASK(31, 2)
 
 /* RTSOFF - run_regs_off - bitmasks */
 /* bits 4:0 - Rsvd */
 /* bits 31:5 - Runtime Register Space Offse */
-#define	RTSOFF_MASK	(~0x1f)
+#define	RTSOFF_MASK		GENMASK(31, 5)
 
 /* HCCPARAMS2 - hcc_params2 - bitmasks */
 /* bit 0 - U3 Entry Capability */
diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index e45545fa3c66..fe3982815d5c 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -9,6 +9,7 @@
 
 #include <linux/slab.h>
 #include <linux/uaccess.h>
+#include <linux/bitfield.h>
 
 #include "xhci.h"
 #include "xhci-debugfs.h"
@@ -745,7 +746,7 @@ void xhci_debugfs_init(struct xhci_hcd *xhci)
 			    xhci->debugfs_root, "reg-cap");
 
 	xhci_debugfs_regset(xhci,
-			    HC_LENGTH(readl(&xhci->cap_regs->hc_capbase)),
+			    FIELD_GET(HC_LENGTH, readl(&xhci->cap_regs->hc_capbase)),
 			    xhci_op_regs, ARRAY_SIZE(xhci_op_regs),
 			    xhci->debugfs_root, "reg-op");
 
diff --git a/drivers/usb/host/xhci-histb.c b/drivers/usb/host/xhci-histb.c
index 02396c8721dc..fddb43bf6323 100644
--- a/drivers/usb/host/xhci-histb.c
+++ b/drivers/usb/host/xhci-histb.c
@@ -276,7 +276,7 @@ static int xhci_histb_probe(struct platform_device *pdev)
 	if (ret)
 		goto put_usb3_hcd;
 
-	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
+	if (GET_MAX_PSA_SIZE(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
 
 	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 422028ebce49..356ddd4f3292 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -115,8 +115,8 @@ static int xhci_create_usb3x_bos_desc(struct xhci_hcd *xhci, char *buf,
 
 	if ((xhci->quirks & XHCI_LPM_SUPPORT)) {
 		reg = readl(&xhci->cap_regs->hcs_params3);
-		ss_cap->bU1devExitLat = HCS_U1_LATENCY(reg);
-		ss_cap->bU2DevExitLat = cpu_to_le16(HCS_U2_LATENCY(reg));
+		ss_cap->bU1devExitLat = FIELD_GET(HCS_U1_LATENCY, reg);
+		ss_cap->bU2DevExitLat = cpu_to_le16(FIELD_GET(HCS_U2_LATENCY, reg));
 	}
 
 	if (wLength < le16_to_cpu(bos->wTotalLength))
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 06ff712c9cbe..04f5722e1343 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -14,6 +14,7 @@
 #include <linux/slab.h>
 #include <linux/dmapool.h>
 #include <linux/dma-mapping.h>
+#include <linux/bitfield.h>
 
 #include "xhci.h"
 #include "xhci-trace.h"
@@ -2293,7 +2294,7 @@ xhci_alloc_interrupter(struct xhci_hcd *xhci, unsigned int segs, gfp_t flags)
 	if (!segs)
 		segs = ERST_DEFAULT_SEGS;
 
-	max_segs = BIT(HCS_ERST_MAX(xhci->hcs_params2));
+	max_segs = FIELD_GET(HCS_ERST_MAX, xhci->hcs_params2) << 2;
 	segs = min(segs, max_segs);
 
 	ir = kzalloc_node(sizeof(*ir), flags, dev_to_node(dev));
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 208558cf822d..0529c4162f4b 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -468,7 +468,7 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
 	 * MTK xHCI 0.96: PSA is 1 by default even if doesn't support stream,
 	 * and it's 3 when support it.
 	 */
-	if (xhci->hci_version < 0x100 && HCC_MAX_PSA(xhci->hcc_params) == 4)
+	if (xhci->hci_version < 0x100 && GET_MAX_PSA_SIZE(xhci->hcc_params) == 4)
 		xhci->quirks |= XHCI_BROKEN_STREAMS;
 }
 
@@ -650,7 +650,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	}
 
 	usb3_hcd = xhci_get_usb3_hcd(xhci);
-	if (usb3_hcd && HCC_MAX_PSA(xhci->hcc_params) >= 4 &&
+	if (usb3_hcd && GET_MAX_PSA_SIZE(xhci->hcc_params) >= 4 &&
 	    !(xhci->quirks & XHCI_BROKEN_STREAMS))
 		usb3_hcd->can_do_streams = 1;
 
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 6a25cbbbc4a4..e0a063166db5 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -14,6 +14,7 @@
 #include <linux/acpi.h>
 #include <linux/reset.h>
 #include <linux/suspend.h>
+#include <linux/bitfield.h>
 
 #include "xhci.h"
 #include "xhci-trace.h"
@@ -661,7 +662,8 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	}
 
 	usb3_hcd = xhci_get_usb3_hcd(xhci);
-	if (usb3_hcd && !(xhci->quirks & XHCI_BROKEN_STREAMS) && HCC_MAX_PSA(xhci->hcc_params) >= 4)
+	if (usb3_hcd && !(xhci->quirks & XHCI_BROKEN_STREAMS) &&
+	    GET_MAX_PSA_SIZE(xhci->hcc_params) >= 4)
 		usb3_hcd->can_do_streams = 1;
 
 	/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 074d9c731639..6fd595f81a30 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -340,7 +340,7 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
 	}
 
 	usb3_hcd = xhci_get_usb3_hcd(xhci);
-	if (usb3_hcd && HCC_MAX_PSA(xhci->hcc_params) >= 4 &&
+	if (usb3_hcd && GET_MAX_PSA_SIZE(xhci->hcc_params) >= 4 &&
 	    !(xhci->quirks & XHCI_BROKEN_STREAMS))
 		usb3_hcd->can_do_streams = 1;
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 8d69f82a997c..fabf9438241c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -57,6 +57,8 @@
 #include <linux/slab.h>
 #include <linux/string_choices.h>
 #include <linux/dma-mapping.h>
+#include <linux/bitfield.h>
+
 #include "xhci.h"
 #include "xhci-trace.h"
 
@@ -3986,7 +3988,7 @@ static unsigned int xhci_get_last_burst_packet_count(struct xhci_hcd *xhci,
 /* Returns the Isochronous Scheduling Threshold in Microframes. 1 Frame is 8 Microframes. */
 static int xhci_ist_in_microseconds(u32 hcs_params2)
 {
-	int ist = HCS_IST_VALUE(hcs_params2);
+	int ist = FIELD_GET(HCS_IST_VALUE, hcs_params2);
 
 	if (hcs_params2 & HCS_IST_UNIT)
 		ist *= 8;
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 5255b1002893..35ad372b9602 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2014 Google, Inc.
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
@@ -991,7 +992,7 @@ static int tegra_xusb_wait_for_falcon(struct tegra_xusb *tegra)
 	u32 value;
 
 	cap_regs = tegra->regs;
-	op_regs = tegra->regs + HC_LENGTH(readl(&cap_regs->hc_capbase));
+	op_regs = tegra->regs + FIELD_GET(HC_LENGTH, readl(&cap_regs->hc_capbase)),
 
 	ret = readl_poll_timeout(&op_regs->status, value, !(value & STS_CNR), 1000, 200000);
 
@@ -1895,7 +1896,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
 		goto remove_usb2;
 	}
 
-	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
+	if (GET_MAX_PSA_SIZE(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
 
 	err = usb_add_hcd(xhci->shared_hcd, tegra->xhci_irq, IRQF_SHARED);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c7a377b34661..79e707686c1a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -21,6 +21,7 @@
 #include <linux/dmi.h>
 #include <linux/dma-mapping.h>
 #include <linux/usb/xhci-sideband.h>
+#include <linux/bitfield.h>
 
 #include "xhci.h"
 #include "xhci-trace.h"
@@ -3482,7 +3483,7 @@ static void xhci_calculate_streams_entries(struct xhci_hcd *xhci,
 	 * level page entries), but that's an optional feature for xHCI host
 	 * controllers. xHCs must support at least 4 stream IDs.
 	 */
-	max_streams = HCC_MAX_PSA(xhci->hcc_params);
+	max_streams = GET_MAX_PSA_SIZE(xhci->hcc_params);
 	if (*num_stream_ctxs > max_streams) {
 		xhci_dbg(xhci, "xHCI HW only supports %u stream ctx entries.\n",
 				max_streams);
@@ -3612,7 +3613,7 @@ static int xhci_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
 
 	/* MaxPSASize value 0 (2 streams) means streams are not supported */
 	if ((xhci->quirks & XHCI_BROKEN_STREAMS) ||
-			HCC_MAX_PSA(xhci->hcc_params) < 4) {
+			GET_MAX_PSA_SIZE(xhci->hcc_params) < 4) {
 		xhci_dbg(xhci, "xHCI controller does not support streams.\n");
 		return -ENOSYS;
 	}
@@ -4572,7 +4573,7 @@ static int xhci_calculate_hird_besl(struct xhci_hcd *xhci,
 	int besl_device = 0;
 	u32 field;
 
-	u2del = HCS_U2_LATENCY(xhci->hcs_params3);
+	u2del = FIELD_GET(HCS_U2_LATENCY, xhci->hcs_params3);
 	field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);
 
 	if (field & USB_BESL_SUPPORT) {
@@ -5420,25 +5421,25 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	xhci->main_hcd = hcd;
 	xhci->cap_regs = hcd->regs;
 	xhci->op_regs = hcd->regs +
-		HC_LENGTH(readl(&xhci->cap_regs->hc_capbase));
+		FIELD_GET(HC_LENGTH, readl(&xhci->cap_regs->hc_capbase));
 	xhci->run_regs = hcd->regs +
 		(readl(&xhci->cap_regs->run_regs_off) & RTSOFF_MASK);
 	/* Cache read-only capability registers */
 	hcs_params1 = readl(&xhci->cap_regs->hcs_params1);
 	xhci->hcs_params2 = readl(&xhci->cap_regs->hcs_params2);
 	xhci->hcs_params3 = readl(&xhci->cap_regs->hcs_params3);
-	xhci->hci_version = HC_VERSION(readl(&xhci->cap_regs->hc_capbase));
+	xhci->hci_version = FIELD_GET(HC_VERSION, readl(&xhci->cap_regs->hc_capbase));
 	xhci->hcc_params = readl(&xhci->cap_regs->hcc_params);
 	if (xhci->hci_version > 0x100)
 		xhci->hcc_params2 = readl(&xhci->cap_regs->hcc_params2);
 
-	xhci->max_slots = HCS_MAX_SLOTS(hcs_params1);
-	xhci->max_ports = min(HCS_MAX_PORTS(hcs_params1), MAX_HC_PORTS);
+	xhci->max_slots = FIELD_GET(HCS_SLOTS_MASK, hcs_params1);
+	xhci->max_ports = min(FIELD_GET(HCS_MAX_PORTS, hcs_params1), MAX_HC_PORTS);
 	/* xhci-plat or xhci-pci might have set max_interrupters already */
 	if (!xhci->max_interrupters)
-		xhci->max_interrupters = min(HCS_MAX_INTRS(hcs_params1), MAX_HC_INTRS);
-	else if (xhci->max_interrupters > HCS_MAX_INTRS(hcs_params1))
-		xhci->max_interrupters = HCS_MAX_INTRS(hcs_params1);
+		xhci->max_interrupters = min(FIELD_GET(HCS_MAX_INTRS, hcs_params1), MAX_HC_INTRS);
+	else if (xhci->max_interrupters > FIELD_GET(HCS_MAX_INTRS, hcs_params1))
+		xhci->max_interrupters = FIELD_GET(HCS_MAX_INTRS, hcs_params1);
 
 	xhci->quirks |= quirks;
 
-- 
2.50.1

Re: [PATCH 12/13] usb: xhci: standardize multi bit-field macros

[kernel test robot]

Hi Niklas,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus tegra/for-next westeri-thunderbolt/next linus/master v6.18-rc5 next-20251110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Niklas-Neronin/usb-xhci-remove-deprecated-TODO-comment/20251110-233045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20251110151450.635410-13-niklas.neronin%40linux.intel.com
patch subject: [PATCH 12/13] usb: xhci: standardize multi bit-field macros
config: loongarch-randconfig-002-20251111 (https://download.01.org/0day-ci/archive/20251111/202511111300.UAbLCfdg-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251111/202511111300.UAbLCfdg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511111300.UAbLCfdg-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/ratelimit_types.h:5,
                    from include/linux/printk.h:9,
                    from include/asm-generic/bug.h:22,
                    from arch/loongarch/include/asm/bug.h:61,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:13,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/loongarch/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:79,
                    from include/linux/spinlock.h:56,
                    from include/linux/irq.h:14,
                    from drivers/usb/dwc3/host.c:10:
   drivers/usb/dwc3/host.c: In function 'dwc3_power_off_all_roothub_ports':
>> include/linux/bits.h:47:9: error: called object is not a function or function pointer
      47 |         ((t)(GENMASK_INPUT_CHECK(h, l) +                        \
         |         ^
   include/linux/bits.h:51:33: note: in expansion of macro 'GENMASK_TYPE'
      51 | #define GENMASK(h, l)           GENMASK_TYPE(unsigned long, h, l)
         |                                 ^~~~~~~~~~~~
   drivers/usb/dwc3/../host/xhci-caps.h:12:33: note: in expansion of macro 'GENMASK'
      12 | #define HC_LENGTH               GENMASK(7, 0)
         |                                 ^~~~~~~
   drivers/usb/dwc3/host.c:49:32: note: in expansion of macro 'HC_LENGTH'
      49 |                 op_regs_base = HC_LENGTH(readl(xhci_regs));
         |                                ^~~~~~~~~
>> include/linux/bits.h:47:9: error: called object is not a function or function pointer
      47 |         ((t)(GENMASK_INPUT_CHECK(h, l) +                        \
         |         ^
   include/linux/bits.h:51:33: note: in expansion of macro 'GENMASK_TYPE'
      51 | #define GENMASK(h, l)           GENMASK_TYPE(unsigned long, h, l)
         |                                 ^~~~~~~~~~~~
   drivers/usb/dwc3/../host/xhci-caps.h:24:33: note: in expansion of macro 'GENMASK'
      24 | #define HCS_MAX_PORTS           GENMASK(31, 24)
         |                                 ^~~~~~~
   drivers/usb/dwc3/host.c:51:28: note: in expansion of macro 'HCS_MAX_PORTS'
      51 |                 port_num = HCS_MAX_PORTS(reg);
         |                            ^~~~~~~~~~~~~


vim +47 include/linux/bits.h

31299a5e021124 Vincent Mailhol 2025-03-26  35  
19408200c09485 Vincent Mailhol 2025-03-26  36  /*
19408200c09485 Vincent Mailhol 2025-03-26  37   * Generate a mask for the specified type @t. Additional checks are made to
19408200c09485 Vincent Mailhol 2025-03-26  38   * guarantee the value returned fits in that type, relying on
19408200c09485 Vincent Mailhol 2025-03-26  39   * -Wshift-count-overflow compiler check to detect incompatible arguments.
19408200c09485 Vincent Mailhol 2025-03-26  40   * For example, all these create build errors or warnings:
19408200c09485 Vincent Mailhol 2025-03-26  41   *
19408200c09485 Vincent Mailhol 2025-03-26  42   * - GENMASK(15, 20): wrong argument order
19408200c09485 Vincent Mailhol 2025-03-26  43   * - GENMASK(72, 15): doesn't fit unsigned long
19408200c09485 Vincent Mailhol 2025-03-26  44   * - GENMASK_U32(33, 15): doesn't fit in a u32
19408200c09485 Vincent Mailhol 2025-03-26  45   */
19408200c09485 Vincent Mailhol 2025-03-26  46  #define GENMASK_TYPE(t, h, l)					\
19408200c09485 Vincent Mailhol 2025-03-26 @47  	((t)(GENMASK_INPUT_CHECK(h, l) +			\
19408200c09485 Vincent Mailhol 2025-03-26  48  	     (type_max(t) << (l) &				\
19408200c09485 Vincent Mailhol 2025-03-26  49  	      type_max(t) >> (BITS_PER_TYPE(t) - 1 - (h)))))
19408200c09485 Vincent Mailhol 2025-03-26  50  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 12/13] usb: xhci: standardize multi bit-field macros

[kernel test robot]

Hi Niklas,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus tegra/for-next westeri-thunderbolt/next linus/master v6.18-rc5 next-20251111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Niklas-Neronin/usb-xhci-remove-deprecated-TODO-comment/20251110-233045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20251110151450.635410-13-niklas.neronin%40linux.intel.com
patch subject: [PATCH 12/13] usb: xhci: standardize multi bit-field macros
config: hexagon-randconfig-002-20251111 (https://download.01.org/0day-ci/archive/20251111/202511111608.ytw8hqqD-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251111/202511111608.ytw8hqqD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511111608.ytw8hqqD-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/dwc3/host.c:49:27: error: called object type 'unsigned long' is not a function or function pointer
                   op_regs_base = HC_LENGTH(readl(xhci_regs));
                                  ~~~~~~~~~^
   drivers/usb/dwc3/host.c:51:27: error: called object type 'unsigned long' is not a function or function pointer
                   port_num = HCS_MAX_PORTS(reg);
                              ~~~~~~~~~~~~~^
   2 errors generated.


vim +49 drivers/usb/dwc3/host.c

2d2a3349521de1 Ran Wang      2024-01-24  24  
2d2a3349521de1 Ran Wang      2024-01-24  25  /**
2d2a3349521de1 Ran Wang      2024-01-24  26   * dwc3_power_off_all_roothub_ports - Power off all Root hub ports
2d2a3349521de1 Ran Wang      2024-01-24  27   * @dwc: Pointer to our controller context structure
2d2a3349521de1 Ran Wang      2024-01-24  28   */
2d2a3349521de1 Ran Wang      2024-01-24  29  static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc)
2d2a3349521de1 Ran Wang      2024-01-24  30  {
2d2a3349521de1 Ran Wang      2024-01-24  31  	void __iomem *xhci_regs;
2d2a3349521de1 Ran Wang      2024-01-24  32  	u32 op_regs_base;
2d2a3349521de1 Ran Wang      2024-01-24  33  	int port_num;
2d2a3349521de1 Ran Wang      2024-01-24  34  	u32 offset;
2d2a3349521de1 Ran Wang      2024-01-24  35  	u32 reg;
2d2a3349521de1 Ran Wang      2024-01-24  36  	int i;
2d2a3349521de1 Ran Wang      2024-01-24  37  
57d7a6b93822f7 Shen Lichuan  2024-09-30  38  	/* xhci regs are not mapped yet, do it temporarily here */
2d2a3349521de1 Ran Wang      2024-01-24  39  	if (dwc->xhci_resources[0].start) {
5ed9cc71432a8a Sven Peter    2025-10-15  40  		if (dwc->xhci_resources[0].flags & IORESOURCE_MEM_NONPOSTED)
5ed9cc71432a8a Sven Peter    2025-10-15  41  			xhci_regs = ioremap_np(dwc->xhci_resources[0].start, DWC3_XHCI_REGS_END);
5ed9cc71432a8a Sven Peter    2025-10-15  42  		else
2d2a3349521de1 Ran Wang      2024-01-24  43  			xhci_regs = ioremap(dwc->xhci_resources[0].start, DWC3_XHCI_REGS_END);
b65bdf7fd605eb Dan Carpenter 2024-01-31  44  		if (!xhci_regs) {
2d2a3349521de1 Ran Wang      2024-01-24  45  			dev_err(dwc->dev, "Failed to ioremap xhci_regs\n");
2d2a3349521de1 Ran Wang      2024-01-24  46  			return;
2d2a3349521de1 Ran Wang      2024-01-24  47  		}
2d2a3349521de1 Ran Wang      2024-01-24  48  
2d2a3349521de1 Ran Wang      2024-01-24 @49  		op_regs_base = HC_LENGTH(readl(xhci_regs));
2d2a3349521de1 Ran Wang      2024-01-24  50  		reg = readl(xhci_regs + XHCI_HCSPARAMS1);
2d2a3349521de1 Ran Wang      2024-01-24  51  		port_num = HCS_MAX_PORTS(reg);
2d2a3349521de1 Ran Wang      2024-01-24  52  
2d2a3349521de1 Ran Wang      2024-01-24  53  		for (i = 1; i <= port_num; i++) {
2d2a3349521de1 Ran Wang      2024-01-24  54  			offset = op_regs_base + XHCI_PORTSC_BASE + 0x10 * (i - 1);
2d2a3349521de1 Ran Wang      2024-01-24  55  			reg = readl(xhci_regs + offset);
2d2a3349521de1 Ran Wang      2024-01-24  56  			reg &= ~PORT_POWER;
2d2a3349521de1 Ran Wang      2024-01-24  57  			writel(reg, xhci_regs + offset);
2d2a3349521de1 Ran Wang      2024-01-24  58  		}
2d2a3349521de1 Ran Wang      2024-01-24  59  
2d2a3349521de1 Ran Wang      2024-01-24  60  		iounmap(xhci_regs);
2d2a3349521de1 Ran Wang      2024-01-24  61  	} else {
2d2a3349521de1 Ran Wang      2024-01-24  62  		dev_err(dwc->dev, "xhci base reg invalid\n");
2d2a3349521de1 Ran Wang      2024-01-24  63  	}
2d2a3349521de1 Ran Wang      2024-01-24  64  }
2d2a3349521de1 Ran Wang      2024-01-24  65  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 13/13] usb: xhci: use 64-bit Addressing Capability macro

[Niklas Neronin]

Simplify by replace BIT(0) call with its relevant macro.

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

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 79e707686c1a..294cc9226328 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -5480,7 +5480,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 	 * DMA_BIT_MASK(32)) in this xhci_gen_setup().
 	 */
 	if (xhci->quirks & XHCI_NO_64BIT_SUPPORT)
-		xhci->hcc_params &= ~BIT(0);
+		xhci->hcc_params &= ~HCC_64BIT_ADDR;
 
 	/* Set dma_mask and coherent_dma_mask to 64-bits,
 	 * if xHC supports 64-bit addressing */
-- 
2.50.1