Skip to content

Commit 757508d

Browse files
Epicuriusgregkh
authored andcommitted
usb: xhci: standardize single bit-field macros
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> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Link: https://patch.msgid.link/20251119142417.2820519-23-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 2282ab3 commit 757508d

7 files changed

Lines changed: 38 additions & 37 deletions

File tree

drivers/usb/host/xhci-caps.h

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* xHCI Specification Section 5.3, Revision 1.2.
55
*/
66

7+
#include <linux/bits.h>
8+
79
/* hc_capbase - bitmasks */
810
/* bits 7:0 - Capability Registers Length */
911
#define HC_LENGTH(p) ((p) & 0xff)
@@ -32,7 +34,7 @@
3234
* xHCI specification section 5.3.4.
3335
*/
3436
#define HCS_IST_VALUE(p) ((p) & 0x7)
35-
#define HCS_IST_UNIT(p) ((p) & (1 << 3))
37+
#define HCS_IST_UNIT BIT(3)
3638
/* bits 7:4 - Event Ring Segment Table Max, 2^(n) */
3739
#define HCS_ERST_MAX(p) (((p) >> 4) & 0xf)
3840
/* bits 20:8 - Rsvd */
@@ -52,28 +54,28 @@
5254

5355
/* HCCPARAMS1 - hcc_params - bitmasks */
5456
/* bit 0 - 64-bit Addressing Capability */
55-
#define HCC_64BIT_ADDR(p) ((p) & (1 << 0))
57+
#define HCC_64BIT_ADDR BIT(0)
5658
/* bit 1 - BW Negotiation Capability */
57-
#define HCC_BANDWIDTH_NEG(p) ((p) & (1 << 1))
59+
#define HCC_BANDWIDTH_NEG BIT(1)
5860
/* bit 2 - Context Size */
59-
#define HCC_64BYTE_CONTEXT(p) ((p) & (1 << 2))
60-
#define CTX_SIZE(_hcc) (HCC_64BYTE_CONTEXT(_hcc) ? 64 : 32)
61+
#define HCC_64BYTE_CONTEXT BIT(2)
62+
#define CTX_SIZE(_hcc) (_hcc & HCC_64BYTE_CONTEXT ? 64 : 32)
6163
/* bit 3 - Port Power Control */
62-
#define HCC_PPC(p) ((p) & (1 << 3))
64+
#define HCC_PPC BIT(3)
6365
/* bit 4 - Port Indicators */
64-
#define HCS_INDICATOR(p) ((p) & (1 << 4))
66+
#define HCS_INDICATOR BIT(4)
6567
/* bit 5 - Light HC Reset Capability */
66-
#define HCC_LIGHT_RESET(p) ((p) & (1 << 5))
68+
#define HCC_LIGHT_RESET BIT(5)
6769
/* bit 6 - Latency Tolerance Messaging Capability */
68-
#define HCC_LTC(p) ((p) & (1 << 6))
70+
#define HCC_LTC BIT(6)
6971
/* bit 7 - No Secondary Stream ID Support */
70-
#define HCC_NSS(p) ((p) & (1 << 7))
72+
#define HCC_NSS BIT(7)
7173
/* bit 8 - Parse All Event Data */
7274
/* bit 9 - Short Packet Capability */
73-
#define HCC_SPC(p) ((p) & (1 << 9))
75+
#define HCC_SPC BIT(9)
7476
/* bit 10 - Stopped EDTLA Capability */
7577
/* bit 11 - Contiguous Frame ID Capability */
76-
#define HCC_CFC(p) ((p) & (1 << 11))
78+
#define HCC_CFC BIT(11)
7779
/* bits 15:12 - Max size for Primary Stream Arrays, 2^(n+1) */
7880
#define HCC_MAX_PSA(p) (1 << ((((p) >> 12) & 0xf) + 1))
7981
/* bits 31:16 - xHCI Extended Capabilities Pointer, from PCI base: 2^(n) */
@@ -91,26 +93,26 @@
9193

9294
/* HCCPARAMS2 - hcc_params2 - bitmasks */
9395
/* bit 0 - U3 Entry Capability */
94-
#define HCC2_U3C(p) ((p) & (1 << 0))
96+
#define HCC2_U3C BIT(0)
9597
/* bit 1 - Configure Endpoint Command Max Exit Latency Too Large Capability */
96-
#define HCC2_CMC(p) ((p) & (1 << 1))
98+
#define HCC2_CMC BIT(1)
9799
/* bit 2 - Force Save Context Capabilitu */
98-
#define HCC2_FSC(p) ((p) & (1 << 2))
100+
#define HCC2_FSC BIT(2)
99101
/* bit 3 - Compliance Transition Capability, false: compliance is enabled by default */
100-
#define HCC2_CTC(p) ((p) & (1 << 3))
102+
#define HCC2_CTC BIT(3)
101103
/* bit 4 - Large ESIT Payload Capability, true: HC support ESIT payload > 48k */
102-
#define HCC2_LEC(p) ((p) & (1 << 4))
104+
#define HCC2_LEC BIT(4)
103105
/* bit 5 - Configuration Information Capability */
104-
#define HCC2_CIC(p) ((p) & (1 << 5))
106+
#define HCC2_CIC BIT(5)
105107
/* bit 6 - Extended TBC Capability, true: Isoc burst count > 65535 */
106-
#define HCC2_ETC(p) ((p) & (1 << 6))
108+
#define HCC2_ETC BIT(6)
107109
/* bit 7 - Extended TBC TRB Status Capability */
108-
#define HCC2_ETC_TSC(p) ((p) & (1 << 7))
110+
#define HCC2_ETC_TSC BIT(7)
109111
/* bit 8 - Get/Set Extended Property Capability */
110-
#define HCC2_GSC(p) ((p) & (1 << 8))
112+
#define HCC2_GSC BIT(8)
111113
/* bit 9 - Virtualization Based Trusted I/O Capability */
112-
#define HCC2_VTC(p) ((p) & (1 << 9))
114+
#define HCC2_VTC BIT(9)
113115
/* bit 10 - Rsvd */
114116
/* bit 11 - HC support Double BW on a eUSB2 HS ISOC EP */
115-
#define HCC2_EUSB2_DIC(p) ((p) & (1 << 11))
117+
#define HCC2_EUSB2_DIC BIT(11)
116118
/* bits 31:12 - Rsvd */

drivers/usb/host/xhci-debugfs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ static ssize_t xhci_port_write(struct file *file, const char __user *ubuf,
355355

356356
if (!strncmp(buf, "compliance", 10)) {
357357
/* If CTC is clear, compliance is enabled by default */
358-
if (!HCC2_CTC(xhci->hcc_params2))
358+
if (!(xhci->hcc_params2 & HCC2_CTC))
359359
return count;
360360
spin_lock_irqsave(&xhci->lock, flags);
361361
/* compliance mode can only be enabled on ports in RxDetect */

drivers/usb/host/xhci-hub.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ static int xhci_create_usb3x_bos_desc(struct xhci_hcd *xhci, char *buf,
110110
ss_cap->bU2DevExitLat = 0; /* set later */
111111

112112
reg = readl(&xhci->cap_regs->hcc_params);
113-
if (HCC_LTC(reg))
113+
if (reg & HCC_LTC)
114114
ss_cap->bmAttributes |= USB_LTM_SUPPORT;
115115

116116
if ((xhci->quirks & XHCI_LPM_SUPPORT)) {
@@ -263,7 +263,7 @@ static void xhci_common_hub_descriptor(struct xhci_hcd *xhci,
263263
desc->bNbrPorts = ports;
264264
temp = 0;
265265
/* Bits 1:0 - support per-port power switching, or power always on */
266-
if (HCC_PPC(xhci->hcc_params))
266+
if (xhci->hcc_params & HCC_PPC)
267267
temp |= HUB_CHAR_INDV_PORT_LPSM;
268268
else
269269
temp |= HUB_CHAR_NO_LPSM;
@@ -1400,7 +1400,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
14001400
* automatically entered as on 1.0 and prior.
14011401
*/
14021402
if (link_state == USB_SS_PORT_LS_COMP_MOD) {
1403-
if (!HCC2_CTC(xhci->hcc_params2)) {
1403+
if (!(xhci->hcc_params2 & HCC2_CTC)) {
14041404
xhci_dbg(xhci, "CTC flag is 0, port already supports entering compliance mode\n");
14051405
break;
14061406
}

drivers/usb/host/xhci-mem.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ struct xhci_container_ctx *xhci_alloc_container_ctx(struct xhci_hcd *xhci,
463463
return NULL;
464464

465465
ctx->type = type;
466-
ctx->size = HCC_64BYTE_CONTEXT(xhci->hcc_params) ? 2048 : 1024;
466+
ctx->size = xhci->hcc_params & HCC_64BYTE_CONTEXT ? 2048 : 1024;
467467
if (type == XHCI_CTX_TYPE_INPUT)
468468
ctx->size += CTX_SIZE(xhci->hcc_params);
469469

@@ -1344,7 +1344,7 @@ static u32 xhci_get_endpoint_mult(struct xhci_hcd *xhci,
13441344
bool lec;
13451345

13461346
/* xHCI 1.1 with LEC set does not use mult field, except intel eUSB2 */
1347-
lec = xhci->hci_version > 0x100 && HCC2_LEC(xhci->hcc_params2);
1347+
lec = xhci->hci_version > 0x100 && (xhci->hcc_params2 & HCC2_LEC);
13481348

13491349
/* eUSB2 double isoc bw devices are the only USB2 devices using mult */
13501350
if (usb_endpoint_is_hs_isoc_double(udev, ep) &&
@@ -1433,8 +1433,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
14331433
ring_type = usb_endpoint_type(&ep->desc);
14341434

14351435
/* Ensure host supports double isoc bandwidth for eUSB2 devices */
1436-
if (usb_endpoint_is_hs_isoc_double(udev, ep) &&
1437-
!HCC2_EUSB2_DIC(xhci->hcc_params2)) {
1436+
if (usb_endpoint_is_hs_isoc_double(udev, ep) && !(xhci->hcc_params2 & HCC2_EUSB2_DIC)) {
14381437
dev_dbg(&udev->dev, "Double Isoc Bandwidth not supported by xhci\n");
14391438
return -EINVAL;
14401439
}

drivers/usb/host/xhci-ring.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3966,7 +3966,7 @@ static int xhci_ist_microframes(struct xhci_hcd *xhci)
39663966
{
39673967
int ist = HCS_IST_VALUE(xhci->hcs_params2);
39683968

3969-
if (HCS_IST_UNIT(xhci->hcs_params2))
3969+
if (xhci->hcs_params2 & HCS_IST_UNIT)
39703970
ist *= 8;
39713971
return ist;
39723972
}
@@ -4135,7 +4135,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
41354135
/* use SIA as default, if frame id is used overwrite it */
41364136
sia_frame_id = TRB_SIA;
41374137
if (!(urb->transfer_flags & URB_ISO_ASAP) &&
4138-
HCC_CFC(xhci->hcc_params)) {
4138+
(xhci->hcc_params & HCC_CFC)) {
41394139
frame_id = xhci_get_isoc_frame_id(xhci, urb, i);
41404140
if (frame_id >= 0)
41414141
sia_frame_id = TRB_FRAME_ID(frame_id);
@@ -4219,7 +4219,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
42194219
}
42204220

42214221
/* store the next frame id */
4222-
if (HCC_CFC(xhci->hcc_params))
4222+
if (xhci->hcc_params & HCC_CFC)
42234223
xep->next_frame_id = urb->start_frame + num_tds * urb->interval;
42244224

42254225
if (xhci_to_hcd(xhci)->self.bandwidth_isoc_reqs == 0) {
@@ -4298,7 +4298,7 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
42984298
check_interval(urb, ep_ctx);
42994299

43004300
/* Calculate the start frame and put it in urb->start_frame. */
4301-
if (HCC_CFC(xhci->hcc_params) && !list_empty(&ep_ring->td_list)) {
4301+
if ((xhci->hcc_params & HCC_CFC) && !list_empty(&ep_ring->td_list)) {
43024302
if (GET_EP_CTX_STATE(ep_ctx) == EP_STATE_RUNNING) {
43034303
urb->start_frame = xep->next_frame_id;
43044304
goto skip_start_over;

drivers/usb/host/xhci-trace.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ DECLARE_EVENT_CLASS(xhci_log_ctx,
8181
),
8282
TP_fast_assign(
8383

84-
__entry->ctx_64 = HCC_64BYTE_CONTEXT(xhci->hcc_params);
84+
__entry->ctx_64 = xhci->hcc_params & HCC_64BYTE_CONTEXT;
8585
__entry->ctx_type = ctx->type;
8686
__entry->ctx_dma = ctx->dma;
8787
__entry->ctx_va = ctx->bytes;

drivers/usb/host/xhci.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5495,7 +5495,7 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
54955495

54965496
/* Set dma_mask and coherent_dma_mask to 64-bits,
54975497
* if xHC supports 64-bit addressing */
5498-
if (HCC_64BIT_ADDR(xhci->hcc_params) &&
5498+
if ((xhci->hcc_params & HCC_64BIT_ADDR) &&
54995499
!dma_set_mask(dev, DMA_BIT_MASK(64))) {
55005500
xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
55015501
dma_set_coherent_mask(dev, DMA_BIT_MASK(64));

0 commit comments

Comments
 (0)