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

[Niklas Neronin <niklas.neronin@linux.intel.com>]

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>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
---
v2:
 * Revert to bitwise from BIT() operaion in HCC_MAX_PSA() macro.

 drivers/usb/host/xhci-caps.h    | 48 +++++++++++++++++----------------
 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, 38 insertions(+), 37 deletions(-)

diff --git a/drivers/usb/host/xhci-caps.h b/drivers/usb/host/xhci-caps.h
index 99557df89f88..52153c4a43a8 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,28 +54,28 @@
 
 /* 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))
 /* bits 31:16 - xHCI Extended Capabilities Pointer, from PCI base: 2^(n) */
@@ -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 6ea0ae27617e..8161fc563200 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3988,7 +3988,7 @@ static int xhci_ist_microframes(struct xhci_hcd *xhci)
 {
 	int ist = HCS_IST_VALUE(xhci->hcs_params2);
 
-	if (HCS_IST_UNIT(xhci->hcs_params2))
+	if (xhci->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