Skip to contentSkip to conversation

usb: xhci: rework xHCI Port macros

PATCH New Niklas Neronin 1 participant 21 messages Sep 16

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v2 cover usb: xhci: rework xHCI Port macros

Rework xhci-port.h to standardize how USB Port register macros are defined and used.

Field macros are restricted to describing bit masks only, while the common FIELD_* helpers are used to extract, set, and update field values. This removes custom helpers and consolidates bitfield handling into a single, well-established mechanism.

Other changes include:
- Renaming macros to follow the xHCI specification.
- Relocating USB Port macros to xhci-port.h
- Updating bit field comments and format.
- Adding missing bits/macros.

Why are there 20 patches when many make similar changes?
The series is intentionally split to make review easier by keeping each patch focused and manageable. If preferred, the patches can be combined after review.

Niklas Neronin (20):
usb: xhci: replace bit shifts with the BIT() macro in xhci-port.h
usb: xhci: rework Port Speed macros
usb: xhci: rework Port Link State macros
usb: xhci: replace old Port macro prefix
usb: xhci: rework Port Indicator Control macro
usb: xhci: rework USB3 PORTPMSC macros
usb: xhci: rework USB2 PORTPMSC macros
usb: xhci: rework USB3 PORTLI macros
usb: xhci: rework USB2 PORTLI macros
usb: xhci: document USB3 PORTHLPMC register layout
usb: xhci: rework USB2 PORTHLPMC macros
usb: xhci: update port register bitfield comments for consistency
usb: xhci: rename port register macros to xHCI spec abbreviations
usb: xhci: replace magic numbers with Port macros
usb: xhci: update PORTSC aggregate macros
usb: xhci: consolidate PORTSC RW1CS bit macros
usb: xhci: relocate all port register macros to xhci-port.h
usb: xhci: preserve RW bits in xhci_port_state_to_neutral()
usb: xhci: improve xhci_port_missing_cas_quirk()
usb: xhci: use neutral helper when resuming ports

 drivers/usb/dwc3/host.c         |   2 +-
 drivers/usb/early/xhci-dbc.c    |   4 +-
 drivers/usb/host/xhci-debugfs.c |  23 +--
 drivers/usb/host/xhci-hub.c     | 301 ++++++++++++++------------------
 drivers/usb/host/xhci-pci.c     |   4 +-
 drivers/usb/host/xhci-port.h    | 299 +++++++++++++++++--------------
 drivers/usb/host/xhci-ring.c    |  25 ++-
 drivers/usb/host/xhci-tegra.c   |  23 +--
 drivers/usb/host/xhci.c         |  30 ++--
 drivers/usb/host/xhci.h         |  65 ++++---
 10 files changed, 396 insertions(+), 380 deletions(-)
Signature · 1 line
2.50.1
#1 lore raw

PATCH v2 1/20 usb: xhci: replace bit shifts with the BIT() macro in xhci-port.h

Use the modern approach.

No need for BIT_ULL() in xhciport.h

---
 drivers/usb/host/xhci-port.h | 44 +++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 21 deletions(-)
drivers/usb/host/xhci-port.h +23 -21
index 889b5fb0fcd8..45e081a9c510 100644
@@ -1,15 +1,17 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 
+#include <linux/bits.h>
+
 /* PORTSC - Port Status and Control Register - port_status_base bitmasks */
 /* true: device connected */
-#define PORT_CONNECT	(1 << 0)
+#define PORT_CONNECT	BIT(0)
 /* true: port enabled */
-#define PORT_PE		(1 << 1)
+#define PORT_PE		BIT(1)
 /* bit 2 reserved and zeroed */
 /* true: port has an over-current condition */
-#define PORT_OC		(1 << 3)
+#define PORT_OC		BIT(3)
 /* true: port reset signaling asserted */
-#define PORT_RESET	(1 << 4)
+#define PORT_RESET	BIT(4)
 /* Port Link State - bits 5:8
  * A read gives the current link PM state of the port,
  * a write with Link State Write Strobe set sets the link state.
@@ -30,7 +32,7 @@
 #define XDEV_RESUME	(0xf << 5)
 
 /* true: port has power (see HCC_PPC) */
-#define PORT_POWER	(1 << 9)
+#define PORT_POWER	BIT(9)
 /* bits 10:13 indicate device speed:
  * 0 - undefined speed - port hasn't be initialized by a reset yet
  * 1 - full speed
@@ -66,21 +68,21 @@
 #define PORT_LED_GREEN	(2 << 14)
 #define PORT_LED_MASK	(3 << 14)
 /* Port Link State Write Strobe - set this when changing link state */
-#define PORT_LINK_STROBE	(1 << 16)
+#define PORT_LINK_STROBE	BIT(16)
 /* true: connect status change */
-#define PORT_CSC	(1 << 17)
+#define PORT_CSC	BIT(17)
 /* true: port enable change */
-#define PORT_PEC	(1 << 18)
+#define PORT_PEC	BIT(18)
 /* true: warm reset for a USB 3.0 device is done.  A "hot" reset puts the port
  * into an enabled state, and the device into the default state.  A "warm" reset
  * also resets the link, forcing the device through the link training sequence.
  * SW can also look at the Port Reset register to see when warm reset is done.
  */
-#define PORT_WRC	(1 << 19)
+#define PORT_WRC	BIT(19)
 /* true: over-current change */
-#define PORT_OCC	(1 << 20)
+#define PORT_OCC	BIT(20)
 /* true: reset change - 1 to 0 transition of PORT_RESET */
-#define PORT_RC		(1 << 21)
+#define PORT_RC		BIT(21)
 /* port link status change - set on some port link state transitions:
  *  Transition				Reason
  *  ------------------------------------------------------------------------------
@@ -94,9 +96,9 @@
  *  - U0 to disabled			L1 entry error with USB 2.1 device
  *  - Any state to inactive		Error on USB 3.0 port
  */
-#define PORT_PLC	(1 << 22)
+#define PORT_PLC	BIT(22)
 /* port configure error change - port failed to configure its link partner */
-#define PORT_CEC	(1 << 23)
+#define PORT_CEC	BIT(23)
 #define PORT_CHANGE_MASK	(PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | \
 				 PORT_RC | PORT_PLC | PORT_CEC)
 
@@ -105,18 +107,18 @@
  * Sx state. Warm port reset should be perfomed to clear this bit and move port
  * to connected state.
  */
-#define PORT_CAS	(1 << 24)
+#define PORT_CAS	BIT(24)
 /* wake on connect (enable) */
-#define PORT_WKCONN_E	(1 << 25)
+#define PORT_WKCONN_E	BIT(25)
 /* wake on disconnect (enable) */
-#define PORT_WKDISC_E	(1 << 26)
+#define PORT_WKDISC_E	BIT(26)
 /* wake on over-current (enable) */
-#define PORT_WKOC_E	(1 << 27)
+#define PORT_WKOC_E	BIT(27)
 /* bits 28:29 reserved */
 /* true: device is non-removable - for USB 3.0 roothub emulation */
-#define PORT_DEV_REMOVE	(1 << 30)
+#define PORT_DEV_REMOVE	BIT(30)
 /* Initiate a warm port reset - complete when PORT_WRC is '1' */
-#define PORT_WR		(1 << 31)
+#define PORT_WR		BIT(31)
 
 /* We mark duplicate entries with -1 */
 #define DUPLICATE_ENTRY ((u8)(-1))
@@ -135,12 +137,12 @@
 /* USB2 Protocol PORTSPMSC */
 #define	PORT_L1S_MASK		7
 #define	PORT_L1S_SUCCESS	1
-#define	PORT_RWE		(1 << 3)
+#define	PORT_RWE		BIT(3)
 #define	PORT_HIRD(p)		(((p) & 0xf) << 4)
 #define	PORT_HIRD_MASK		(0xf << 4)
 #define	PORT_L1DS_MASK		(0xff << 8)
 #define	PORT_L1DS(p)		(((p) & 0xff) << 8)
-#define	PORT_HLE		(1 << 16)
+#define	PORT_HLE		BIT(16)
 #define PORT_TEST_MODE_SHIFT	28
 
 /* USB3 Protocol PORTLI  Port Link Information */
Signature · 1 line
2.50.1
#2 lore raw

PATCH v2 2/20 usb: xhci: rework Port Speed macros

In reply to Niklas Neronin

Rework the Port Speed handling to use standard bitfield helpers and explicit numeric values. Renaming is done in a later patch.

By consolidating macro usage and eliminating custom helpers, the code becomes simpler and easier to follow. The use of standard bitfield macros improves readability and enforces consistent register field handling.

Relocate Slot Context macros alongside the Slot Context definitions to keep the decoding logic self-contained and improve code organization.

---
 drivers/usb/host/xhci-debugfs.c |  6 ++---
 drivers/usb/host/xhci-hub.c     |  8 +++---
 drivers/usb/host/xhci-port.h    | 44 ++++++++++++++-------------------
 drivers/usb/host/xhci-ring.c    |  4 +--
 drivers/usb/host/xhci-tegra.c   | 13 ++++++----
 drivers/usb/host/xhci.h         | 13 ++++++++--
 6 files changed, 47 insertions(+), 41 deletions(-)
drivers/usb/host/xhci-debugfs.c +3 -3
index 2aa01d99f23d..450247b8bca5 100644
@@ -715,7 +715,7 @@ static int xhci_ss_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_SS), s);
+	ret = xhci_port_bw_show(xhci, XDEV_SS, s);
 	return ret;
 }
 
@@ -724,7 +724,7 @@ static int xhci_hs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_HS), s);
+	ret = xhci_port_bw_show(xhci, XDEV_HS, s);
 	return ret;
 }
 
@@ -733,7 +733,7 @@ static int xhci_fs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_FS), s);
+	ret = xhci_port_bw_show(xhci, XDEV_FS, s);
 	return ret;
 }
 
drivers/usb/host/xhci-hub.c +5 -3
index 470bafe1802b..c52021ed948a 100644
@@ -376,9 +376,11 @@ static void xhci_hub_descriptor(struct usb_hcd *hcd, struct xhci_hcd *xhci,
 
 static unsigned int xhci_port_speed(int portsc)
 {
-	if (DEV_LOWSPEED(portsc))
+	u32 port_speed = FIELD_GET(DEV_SPEED_MASK, portsc);
+
+	if (port_speed == XDEV_LS)
 		return USB_PORT_STAT_LOW_SPEED;
-	if (DEV_HIGHSPEED(portsc))
+	if (port_speed == XDEV_HS)
 		return USB_PORT_STAT_HIGH_SPEED;
 	/*
 	 * FIXME: Yes, we should check for full speed, but the core uses that as
@@ -1007,7 +1009,7 @@ static u32 xhci_get_ext_port_status(u32 portsc, u32 port_li)
 	int speed_id;
 
 	/* only support rx and tx lane counts of 1 in usb3.1 spec */
-	speed_id = DEV_PORT_SPEED(portsc);
+	speed_id = FIELD_GET(DEV_SPEED_MASK, portsc);
 	ext_stat |= speed_id;		/* bits 3:0, RX speed id */
 	ext_stat |= speed_id << 4;	/* bits 7:4, TX speed id */
 
drivers/usb/host/xhci-port.h +18 -26

58 lines not on this page · raw message

drivers/usb/host/xhci-ring.c +2 -2
index ec278a9f9540..635118a32466 100644
@@ -2075,7 +2075,7 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 			goto cleanup;
 		}
 
-		if (DEV_SUPERSPEED_ANY(portsc)) {
+		if (FIELD_GET(DEV_SPEED_MASK, portsc) >= XDEV_SS) {
 			xhci_dbg(xhci, "remote wake SS port %d\n", port_id);
 			/* Set a flag to say the port signaled remote wakeup,
 			 * so we can tell the difference between the end of
@@ -2108,7 +2108,7 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 	}
 
 	if ((portsc & PORT_PLC) &&
-	    DEV_SUPERSPEED_ANY(portsc) &&
+	    FIELD_GET(DEV_SPEED_MASK, portsc) >= XDEV_SS &&
 	    ((portsc & PORT_PLS_MASK) == XDEV_U0 ||
 	     (portsc & PORT_PLS_MASK) == XDEV_U1 ||
 	     (portsc & PORT_PLS_MASK) == XDEV_U2)) {
drivers/usb/host/xhci-tegra.c +8 -5
index 6f235d1e117e..c4e6b107c678 100644
@@ -2108,16 +2108,18 @@ static void tegra_xusb_restore_context(struct tegra_xusb *tegra)
 
 static enum usb_device_speed tegra_xhci_portsc_to_speed(struct tegra_xusb *tegra, u32 portsc)
 {
-	if (DEV_LOWSPEED(portsc))
+	u32 port_speed = FIELD_GET(DEV_SPEED_MASK, portsc);
+
+	if (port_speed == XDEV_LS)
 		return USB_SPEED_LOW;
 
-	if (DEV_HIGHSPEED(portsc))
+	if (port_speed == XDEV_HS)
 		return USB_SPEED_HIGH;
 
-	if (DEV_FULLSPEED(portsc))
+	if (port_speed == XDEV_FS)
 		return USB_SPEED_FULL;
 
-	if (DEV_SUPERSPEED_ANY(portsc))
+	if (port_speed >= XDEV_SS)
 		return USB_SPEED_SUPER;
 
 	return USB_SPEED_UNKNOWN;
@@ -2256,7 +2258,8 @@ static int tegra_xusb_enter_elpg(struct tegra_xusb *tegra, bool is_auto_resume)
 			continue;
 		portsc = xhci_portsc_readl(xhci->usb2_rhub.ports[i]);
 		tegra->lp0_utmi_pad_mask &= ~BIT(i);
-		if (((portsc & PORT_PLS_MASK) == XDEV_U3) || ((portsc & DEV_SPEED_MASK) == XDEV_FS))
+		if (((portsc & PORT_PLS_MASK) == XDEV_U3) ||
+		    (FIELD_GET(DEV_SPEED_MASK, portsc) == XDEV_FS))
 			tegra->lp0_utmi_pad_mask |= BIT(i);
 	}
 
drivers/usb/host/xhci.h +11 -2
index c7bfa7f028d3..8f3bd27df795 100644
@@ -12,6 +12,7 @@
 #ifndef __LINUX_XHCI_HCD_H
 #define __LINUX_XHCI_HCD_H
 
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/usb.h>
 #include <linux/timer.h>
@@ -360,9 +361,17 @@ struct xhci_slot_ctx {
 /* dev_info bitmasks */
 /* Route String - 0:19 */
 #define ROUTE_STRING_MASK	(0xfffff)
-/* Device speed - values defined by PORTSC Device Speed field - 20:23 */
+/*
+ * bits 23:20 - Device speed, values defined by PORTSC Device Speed field
+ * xHCI version 1.2 onwards these bits are Reserved
+ */
 #define DEV_SPEED	(0xf << 20)
 #define GET_DEV_SPEED(n) (((n) & DEV_SPEED) >> 20)
+#define	SLOT_SPEED_FS		(XDEV_FS << 20)
+#define	SLOT_SPEED_LS		(XDEV_LS << 20)
+#define	SLOT_SPEED_HS		(XDEV_HS << 20)
+#define	SLOT_SPEED_SS		(XDEV_SS << 20)
+#define	SLOT_SPEED_SSP		(XDEV_SSP << 20)
 /* bit 24 reserved */
 /* Is this LS/FS device connected through a HS hub? - bit 25 */
 #define DEV_MTT		BIT(25)
@@ -2407,7 +2416,7 @@ static inline const char *xhci_decode_portsc(char *str, u32 portsc)
 	if (portsc == ~(u32)0)
 		return str;
 
-	ret += sprintf(str + ret, "Speed=%d ", DEV_PORT_SPEED(portsc));
+	ret += sprintf(str + ret, "Speed=%ld ", FIELD_GET(DEV_SPEED_MASK, portsc));
 	ret += sprintf(str + ret, "Link=%s ", xhci_portsc_link_state_string(portsc));
 
 	/* RO/ROS: Read-only */
Signature · 1 line
2.50.1
#3 lore raw

PATCH v2 3/20 usb: xhci: rework Port Link State macros

In reply to Niklas Neronin

Rework the Port Link State (PLS) handling to use standard bitfield helpers and explicit numeric values. Renaming is done in a later patch.

By consolidating macro usage and eliminating custom helpers, the code becomes simpler and easier to follow. The use of standard bitfield macros improves readability and enforces consistent register field handling.

Change PLS macros in xhci_hub_report_usb3_link_state() from USB chapter 1 to xhci driver. Because the PLS value is then written to a xHCI register.

---
 drivers/usb/host/xhci-debugfs.c |  6 ++--
 drivers/usb/host/xhci-hub.c     | 49 +++++++++++++++++----------------
 drivers/usb/host/xhci-pci.c     |  2 +-
 drivers/usb/host/xhci-port.h    | 40 ++++++++++++++-------------
 drivers/usb/host/xhci-ring.c    | 13 ++++-----
 drivers/usb/host/xhci-tegra.c   |  6 ++--
 drivers/usb/host/xhci.c         |  6 ++--
 drivers/usb/host/xhci.h         |  2 +-
 8 files changed, 63 insertions(+), 61 deletions(-)
drivers/usb/host/xhci-debugfs.c +3 -3
index 450247b8bca5..b7095b2817d5 100644
@@ -361,13 +361,13 @@ static ssize_t xhci_port_write(struct file *file,  const char __user *ubuf,
 		spin_lock_irqsave(&xhci->lock, flags);
 		/* compliance mode can only be enabled on ports in RxDetect */
 		portsc = xhci_portsc_readl(port);
-		if ((portsc & PORT_PLS_MASK) != XDEV_RXDETECT) {
+		if (FIELD_GET(PORT_PLS_MASK, portsc) != XDEV_RXDETECT) {
 			spin_unlock_irqrestore(&xhci->lock, flags);
 			return -EPERM;
 		}
 		portsc = xhci_port_state_to_neutral(portsc);
-		portsc &= ~PORT_PLS_MASK;
-		portsc |= PORT_LINK_STROBE | XDEV_COMP_MODE;
+		FIELD_MODIFY(PORT_PLS_MASK, &portsc, XDEV_COMP_MODE);
+		portsc |= PORT_LINK_STROBE;
 		xhci_portsc_writel(port, portsc);
 		spin_unlock_irqrestore(&xhci->lock, flags);
 	} else {
drivers/usb/host/xhci-hub.c +25 -24

171 lines not on this page · raw message

drivers/usb/host/xhci-pci.c +1 -1
index a8889081ae82..c7b79c5daa53 100644
@@ -914,7 +914,7 @@ static int xhci_pci_poweroff_late(struct usb_hcd *hcd, bool do_wakeup)
 		port = &xhci->hw_ports[i];
 		portsc = xhci_portsc_readl(port);
 
-		if ((portsc & PORT_PLS_MASK) != XDEV_U3)
+		if (FIELD_GET(PORT_PLS_MASK, portsc) != XDEV_U3)
 			continue;
 
 		if (!port->slot_id || !xhci->devs[port->slot_id]) {
drivers/usb/host/xhci-port.h +21 -19
index 142f8010287f..51fe4cc14d74 100644
@@ -13,26 +13,28 @@
 #define PORT_OC		BIT(3)
 /* true: port reset signaling asserted */
 #define PORT_RESET	BIT(4)
-/* Port Link State - bits 5:8
- * A read gives the current link PM state of the port,
- * a write with Link State Write Strobe set sets the link state.
+/*
+ * bits 8:5 - Port Link State, by default '5'.
+ * Reading gives the current link PM state of the port.
+ * Writing sets the link state, Port Link State Write Strobe (LWS) must be set.
+ * PLS values 0-11 are defined in USB chapter 11.
  */
-#define PORT_PLS_MASK	(0xf << 5)
-#define XDEV_U0		(0x0 << 5)
-#define XDEV_U1		(0x1 << 5)
-#define XDEV_U2		(0x2 << 5)
-#define XDEV_U3		(0x3 << 5)
-#define XDEV_DISABLED	(0x4 << 5)
-#define XDEV_RXDETECT	(0x5 << 5)
-#define XDEV_INACTIVE	(0x6 << 5)
-#define XDEV_POLLING	(0x7 << 5)
-#define XDEV_RECOVERY	(0x8 << 5)
-#define XDEV_HOT_RESET	(0x9 << 5)
-#define XDEV_COMP_MODE	(0xa << 5)
-#define XDEV_TEST_MODE	(0xb << 5)
-#define XDEV_RESUME	(0xf << 5)
-
-/* true: port has power (see HCC_PPC) */
+#define PORT_PLS_MASK	GENMASK(8, 5)
+#define XDEV_U0		0
+#define XDEV_U1		1
+#define XDEV_U2		2
+#define XDEV_U3		3
+#define XDEV_DISABLED	4
+#define XDEV_RXDETECT	5
+#define XDEV_INACTIVE	6
+#define XDEV_POLLING	7
+#define XDEV_RECOVERY	8
+#define XDEV_HOT_RESET	9
+#define XDEV_COMP_MODE	10
+#define XDEV_TEST_MODE	11
+/* Values 12-14 are Reserved */
+#define XDEV_RESUME	15
+/* bit 9 - Port Power (PP) */
 #define PORT_POWER	BIT(9)
 /*
  * bits 13:10 - Port Speed
drivers/usb/host/xhci-ring.c +6 -7
index 635118a32466..de6cf2026b82 100644
@@ -2045,7 +2045,7 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 	bus_state = &port->rhub->bus_state;
 	hcd_portnum = port->hcd_portnum;
 	portsc = xhci_portsc_readl(port);
-	pls = portsc & PORT_PLS_MASK;
+	pls = FIELD_GET(PORT_PLS_MASK, portsc);
 
 	xhci_dbg(xhci, "Port change event, %d-%d, id %d, portsc: 0x%x\n",
 		 hcd->self.busnum, hcd_portnum + 1, port_id, portsc);
@@ -2066,7 +2066,8 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 		port->connected = !!(portsc & PORT_CONNECT);
 	}
 
-	if ((portsc & PORT_PLC) && (portsc & PORT_PLS_MASK) == XDEV_RESUME) {
+	pls = FIELD_GET(PORT_PLS_MASK, portsc);
+	if ((portsc & PORT_PLC) && pls == XDEV_RESUME) {
 		xhci_dbg(xhci, "port resume event for port %d\n", port_id);
 
 		cmd_reg = readl(&xhci->op_regs->command);
@@ -2107,11 +2108,9 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 		}
 	}
 
-	if ((portsc & PORT_PLC) &&
-	    FIELD_GET(DEV_SPEED_MASK, portsc) >= XDEV_SS &&
-	    ((portsc & PORT_PLS_MASK) == XDEV_U0 ||
-	     (portsc & PORT_PLS_MASK) == XDEV_U1 ||
-	     (portsc & PORT_PLS_MASK) == XDEV_U2)) {
+	pls = FIELD_GET(PORT_PLS_MASK, portsc);
+	if ((portsc & PORT_PLC) && pls <= XDEV_U2 &&
+	    FIELD_GET(DEV_SPEED_MASK, portsc) >= XDEV_SS) {
 		xhci_dbg(xhci, "resume SS port %d finished\n", port_id);
 		complete(&port->u3exit_done);
 		/* We've just brought the device into U0/1/2 through either the
drivers/usb/host/xhci-tegra.c +3 -3
index c4e6b107c678..cd646f6bbe04 100644
@@ -2037,7 +2037,7 @@ static bool xhci_hub_ports_suspended(struct xhci_hub *hub)
 		if ((value & PORT_PE) == 0)
 			continue;
 
-		if ((value & PORT_PLS_MASK) != XDEV_U3) {
+		if (FIELD_GET(PORT_PLS_MASK, value) != XDEV_U3) {
 			dev_info(dev, "%u-%u isn't suspended: %#010x\n",
 				 hub->hcd->self.busnum, i + 1, value);
 			status = false;
@@ -2258,7 +2258,7 @@ static int tegra_xusb_enter_elpg(struct tegra_xusb *tegra, bool is_auto_resume)
 			continue;
 		portsc = xhci_portsc_readl(xhci->usb2_rhub.ports[i]);
 		tegra->lp0_utmi_pad_mask &= ~BIT(i);
-		if (((portsc & PORT_PLS_MASK) == XDEV_U3) ||
+		if ((FIELD_GET(PORT_PLS_MASK, portsc) == XDEV_U3) ||
 		    (FIELD_GET(DEV_SPEED_MASK, portsc) == XDEV_FS))
 			tegra->lp0_utmi_pad_mask |= BIT(i);
 	}
@@ -2804,7 +2804,7 @@ static int tegra_xhci_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value,
 			if (!test_bit(i, &bus_state->resuming_ports))
 				continue;
 			portsc = xhci_portsc_readl(ports[i]);
-			if ((portsc & PORT_PLS_MASK) == XDEV_RESUME)
+			if (FIELD_GET(PORT_PLS_MASK, portsc) == XDEV_RESUME)
 				tegra_phy_xusb_utmi_pad_power_on(
 					tegra_xusb_get_phy(tegra, "usb2", (int) i));
 		}
drivers/usb/host/xhci.c +3 -3
index a9e47e178c28..4acffe73d00d 100644
@@ -377,7 +377,7 @@ static void compliance_mode_recovery(struct timer_list *t)
 
 	for (i = 0; i < rhub->num_ports; i++) {
 		temp = xhci_portsc_readl(rhub->ports[i]);
-		if ((temp & PORT_PLS_MASK) == USB_SS_PORT_LS_COMP_MOD) {
+		if (FIELD_GET(PORT_PLS_MASK, temp) == XDEV_COMP_MODE) {
 			/*
 			 * Compliance Mode Detected. Letting USB Core
 			 * handle the Warm Reset
@@ -929,7 +929,7 @@ static bool xhci_pending_portevent(struct xhci_hcd *xhci)
 	while (port_index--) {
 		portsc = xhci_portsc_readl(ports[port_index]);
 		if (portsc & PORT_CHANGE_MASK ||
-		    (portsc & PORT_PLS_MASK) == XDEV_RESUME)
+		    FIELD_GET(PORT_PLS_MASK, portsc) == XDEV_RESUME)
 			return true;
 	}
 	port_index = xhci->usb3_rhub.num_ports;
@@ -937,7 +937,7 @@ static bool xhci_pending_portevent(struct xhci_hcd *xhci)
 	while (port_index--) {
 		portsc = xhci_portsc_readl(ports[port_index]);
 		if (portsc & (PORT_CHANGE_MASK | PORT_CAS) ||
-		    (portsc & PORT_PLS_MASK) == XDEV_RESUME)
+		    FIELD_GET(PORT_PLS_MASK, portsc) == XDEV_RESUME)
 			return true;
 	}
 	return false;
drivers/usb/host/xhci.h +1 -1
index 8f3bd27df795..9c6c36870fb6 100644
@@ -2374,7 +2374,7 @@ static inline const char *xhci_decode_slot_context(char *str,
 
 static inline const char *xhci_portsc_link_state_string(u32 portsc)
 {
-	switch (portsc & PORT_PLS_MASK) {
+	switch (FIELD_GET(PORT_PLS_MASK, portsc)) {
 	case XDEV_U0:
 		return "U0";
 	case XDEV_U1:
Signature · 1 line
2.50.1
#4 lore raw

PATCH v2 4/20 usb: xhci: replace old Port macro prefix

In reply to Niklas Neronin

Replace prefix 'DEV_' and 'XDEV_' with 'PORT_', which better describes the macro. This affects Port Link State (PLS) and Port Speed. No functional change intended.
Verified identical object code before and after the renaming by comparing objdump -dr output.

No functional change intended.

---
 drivers/usb/host/xhci-debugfs.c | 10 ++--
 drivers/usb/host/xhci-hub.c     | 94 ++++++++++++++++-----------------
 drivers/usb/host/xhci-pci.c     |  2 +-
 drivers/usb/host/xhci-port.h    | 38 ++++++-------
 drivers/usb/host/xhci-ring.c    | 12 ++---
 drivers/usb/host/xhci-tegra.c   | 18 +++----
 drivers/usb/host/xhci.c         |  8 +--
 drivers/usb/host/xhci.h         | 38 ++++++-------
 8 files changed, 110 insertions(+), 110 deletions(-)
drivers/usb/host/xhci-debugfs.c +5 -5
index b7095b2817d5..e61ee722e2cd 100644
@@ -361,12 +361,12 @@ static ssize_t xhci_port_write(struct file *file,  const char __user *ubuf,
 		spin_lock_irqsave(&xhci->lock, flags);
 		/* compliance mode can only be enabled on ports in RxDetect */
 		portsc = xhci_portsc_readl(port);
-		if (FIELD_GET(PORT_PLS_MASK, portsc) != XDEV_RXDETECT) {
+		if (FIELD_GET(PORT_PLS_MASK, portsc) != PLS_RXDETECT) {
 			spin_unlock_irqrestore(&xhci->lock, flags);
 			return -EPERM;
 		}
 		portsc = xhci_port_state_to_neutral(portsc);
-		FIELD_MODIFY(PORT_PLS_MASK, &portsc, XDEV_COMP_MODE);
+		FIELD_MODIFY(PORT_PLS_MASK, &portsc, PLS_COMP_MODE);
 		portsc |= PORT_LINK_STROBE;
 		xhci_portsc_writel(port, portsc);
 		spin_unlock_irqrestore(&xhci->lock, flags);
@@ -715,7 +715,7 @@ static int xhci_ss_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, XDEV_SS, s);
+	ret = xhci_port_bw_show(xhci, PORT_SPEED_SS, s);
 	return ret;
 }
 
@@ -724,7 +724,7 @@ static int xhci_hs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, XDEV_HS, s);
+	ret = xhci_port_bw_show(xhci, PORT_SPEED_HS, s);
 	return ret;
 }
 
@@ -733,7 +733,7 @@ static int xhci_fs_bw_show(struct seq_file *s, void *unused)
 	int ret;
 	struct xhci_hcd		*xhci = (struct xhci_hcd *)s->private;
 
-	ret = xhci_port_bw_show(xhci, XDEV_FS, s);
+	ret = xhci_port_bw_show(xhci, PORT_SPEED_FS, s);
 	return ret;
 }
 
drivers/usb/host/xhci-hub.c +47 -47

277 lines not on this page · raw message

drivers/usb/host/xhci-pci.c +1 -1
index c7b79c5daa53..a63f85961700 100644
@@ -914,7 +914,7 @@ static int xhci_pci_poweroff_late(struct usb_hcd *hcd, bool do_wakeup)
 		port = &xhci->hw_ports[i];
 		portsc = xhci_portsc_readl(port);
 
-		if (FIELD_GET(PORT_PLS_MASK, portsc) != XDEV_U3)
+		if (FIELD_GET(PORT_PLS_MASK, portsc) != PLS_U3)
 			continue;
 
 		if (!port->slot_id || !xhci->devs[port->slot_id]) {
drivers/usb/host/xhci-port.h +19 -19
index 51fe4cc14d74..752f33e35bf5 100644
@@ -20,20 +20,20 @@
  * PLS values 0-11 are defined in USB chapter 11.
  */
 #define PORT_PLS_MASK	GENMASK(8, 5)
-#define XDEV_U0		0
-#define XDEV_U1		1
-#define XDEV_U2		2
-#define XDEV_U3		3
-#define XDEV_DISABLED	4
-#define XDEV_RXDETECT	5
-#define XDEV_INACTIVE	6
-#define XDEV_POLLING	7
-#define XDEV_RECOVERY	8
-#define XDEV_HOT_RESET	9
-#define XDEV_COMP_MODE	10
-#define XDEV_TEST_MODE	11
+#define PLS_U0		0
+#define PLS_U1		1
+#define PLS_U2		2
+#define PLS_U3		3
+#define PLS_DISABLED	4
+#define PLS_RXDETECT	5
+#define PLS_INACTIVE	6
+#define PLS_POLLING	7
+#define PLS_RECOVERY	8
+#define PLS_HOT_RESET	9
+#define PLS_COMP_MODE	10
+#define PLS_TEST_MODE	11
 /* Values 12-14 are Reserved */
-#define XDEV_RESUME	15
+#define PLS_RESUME	15
 /* bit 9 - Port Power (PP) */
 #define PORT_POWER	BIT(9)
 /*
@@ -49,12 +49,12 @@
  * 7 - SuperSpeed Gen2 x2
  * 8-15 Reserved
  */
-#define DEV_SPEED_MASK	GENMASK(13, 10)
-#define	XDEV_FS		1
-#define	XDEV_LS		2
-#define	XDEV_HS		3
-#define	XDEV_SS		4
-#define	XDEV_SSP	5
+#define PORT_SPEED_MASK	GENMASK(13, 10)
+#define	PORT_SPEED_FS	1
+#define	PORT_SPEED_LS	2
+#define	PORT_SPEED_HS	3
+#define	PORT_SPEED_SS	4
+#define	PORT_SPEED_SSP	5
 
 /* Port Indicator Control */
 #define PORT_LED_OFF	(0 << 14)
drivers/usb/host/xhci-ring.c +6 -6
index de6cf2026b82..7ea79501c035 100644
@@ -2062,12 +2062,12 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 	 * Link status is not relible while port is in reset.
 	 */
 	if (!(portsc & PORT_RESET)) {
-		port->link_inactive = (pls == XDEV_INACTIVE);
+		port->link_inactive = (pls == PLS_INACTIVE);
 		port->connected = !!(portsc & PORT_CONNECT);
 	}
 
 	pls = FIELD_GET(PORT_PLS_MASK, portsc);
-	if ((portsc & PORT_PLC) && pls == XDEV_RESUME) {
+	if ((portsc & PORT_PLC) && pls == PLS_RESUME) {
 		xhci_dbg(xhci, "port resume event for port %d\n", port_id);
 
 		cmd_reg = readl(&xhci->op_regs->command);
@@ -2076,7 +2076,7 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 			goto cleanup;
 		}
 
-		if (FIELD_GET(DEV_SPEED_MASK, portsc) >= XDEV_SS) {
+		if (FIELD_GET(PORT_SPEED_MASK, portsc) >= PORT_SPEED_SS) {
 			xhci_dbg(xhci, "remote wake SS port %d\n", port_id);
 			/* Set a flag to say the port signaled remote wakeup,
 			 * so we can tell the difference between the end of
@@ -2085,7 +2085,7 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 			bus_state->port_remote_wakeup |= 1 << hcd_portnum;
 			xhci_test_and_clear_bit(xhci, port, PORT_PLC);
 			usb_hcd_start_port_resume(&hcd->self, hcd_portnum);
-			xhci_set_link_state(xhci, port, XDEV_U0);
+			xhci_set_link_state(xhci, port, PLS_U0);
 			/* Need to wait until the next link state change
 			 * indicates the device is actually in U0.
 			 */
@@ -2109,8 +2109,8 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 	}
 
 	pls = FIELD_GET(PORT_PLS_MASK, portsc);
-	if ((portsc & PORT_PLC) && pls <= XDEV_U2 &&
-	    FIELD_GET(DEV_SPEED_MASK, portsc) >= XDEV_SS) {
+	if ((portsc & PORT_PLC) && pls <= PLS_U2 &&
+	    FIELD_GET(PORT_SPEED_MASK, portsc) >= PORT_SPEED_SS) {
 		xhci_dbg(xhci, "resume SS port %d finished\n", port_id);
 		complete(&port->u3exit_done);
 		/* We've just brought the device into U0/1/2 through either the
drivers/usb/host/xhci-tegra.c +9 -9
index cd646f6bbe04..6a8fb386c2d8 100644
@@ -2037,7 +2037,7 @@ static bool xhci_hub_ports_suspended(struct xhci_hub *hub)
 		if ((value & PORT_PE) == 0)
 			continue;
 
-		if (FIELD_GET(PORT_PLS_MASK, value) != XDEV_U3) {
+		if (FIELD_GET(PORT_PLS_MASK, value) != PLS_U3) {
 			dev_info(dev, "%u-%u isn't suspended: %#010x\n",
 				 hub->hcd->self.busnum, i + 1, value);
 			status = false;
@@ -2108,18 +2108,18 @@ static void tegra_xusb_restore_context(struct tegra_xusb *tegra)
 
 static enum usb_device_speed tegra_xhci_portsc_to_speed(struct tegra_xusb *tegra, u32 portsc)
 {
-	u32 port_speed = FIELD_GET(DEV_SPEED_MASK, portsc);
+	u32 port_speed = FIELD_GET(PORT_SPEED_MASK, portsc);
 
-	if (port_speed == XDEV_LS)
+	if (port_speed == PORT_SPEED_LS)
 		return USB_SPEED_LOW;
 
-	if (port_speed == XDEV_HS)
+	if (port_speed == PORT_SPEED_HS)
 		return USB_SPEED_HIGH;
 
-	if (port_speed == XDEV_FS)
+	if (port_speed == PORT_SPEED_FS)
 		return USB_SPEED_FULL;
 
-	if (port_speed >= XDEV_SS)
+	if (port_speed >= PORT_SPEED_SS)
 		return USB_SPEED_SUPER;
 
 	return USB_SPEED_UNKNOWN;
@@ -2258,8 +2258,8 @@ static int tegra_xusb_enter_elpg(struct tegra_xusb *tegra, bool is_auto_resume)
 			continue;
 		portsc = xhci_portsc_readl(xhci->usb2_rhub.ports[i]);
 		tegra->lp0_utmi_pad_mask &= ~BIT(i);
-		if ((FIELD_GET(PORT_PLS_MASK, portsc) == XDEV_U3) ||
-		    (FIELD_GET(DEV_SPEED_MASK, portsc) == XDEV_FS))
+		if ((FIELD_GET(PORT_PLS_MASK, portsc) == PLS_U3) ||
+		    (FIELD_GET(PORT_SPEED_MASK, portsc) == PORT_SPEED_FS))
 			tegra->lp0_utmi_pad_mask |= BIT(i);
 	}
 
@@ -2804,7 +2804,7 @@ static int tegra_xhci_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value,
 			if (!test_bit(i, &bus_state->resuming_ports))
 				continue;
 			portsc = xhci_portsc_readl(ports[i]);
-			if (FIELD_GET(PORT_PLS_MASK, portsc) == XDEV_RESUME)
+			if (FIELD_GET(PORT_PLS_MASK, portsc) == PLS_RESUME)
 				tegra_phy_xusb_utmi_pad_power_on(
 					tegra_xusb_get_phy(tegra, "usb2", (int) i));
 		}
drivers/usb/host/xhci.c +4 -4
index 4acffe73d00d..cab7bae3ecc3 100644
@@ -377,7 +377,7 @@ static void compliance_mode_recovery(struct timer_list *t)
 
 	for (i = 0; i < rhub->num_ports; i++) {
 		temp = xhci_portsc_readl(rhub->ports[i]);
-		if (FIELD_GET(PORT_PLS_MASK, temp) == XDEV_COMP_MODE) {
+		if (FIELD_GET(PORT_PLS_MASK, temp) == PLS_COMP_MODE) {
 			/*
 			 * Compliance Mode Detected. Letting USB Core
 			 * handle the Warm Reset
@@ -929,7 +929,7 @@ static bool xhci_pending_portevent(struct xhci_hcd *xhci)
 	while (port_index--) {
 		portsc = xhci_portsc_readl(ports[port_index]);
 		if (portsc & PORT_CHANGE_MASK ||
-		    FIELD_GET(PORT_PLS_MASK, portsc) == XDEV_RESUME)
+		    FIELD_GET(PORT_PLS_MASK, portsc) == PLS_RESUME)
 			return true;
 	}
 	port_index = xhci->usb3_rhub.num_ports;
@@ -937,7 +937,7 @@ static bool xhci_pending_portevent(struct xhci_hcd *xhci)
 	while (port_index--) {
 		portsc = xhci_portsc_readl(ports[port_index]);
 		if (portsc & (PORT_CHANGE_MASK | PORT_CAS) ||
-		    FIELD_GET(PORT_PLS_MASK, portsc) == XDEV_RESUME)
+		    FIELD_GET(PORT_PLS_MASK, portsc) == PLS_RESUME)
 			return true;
 	}
 	return false;
@@ -4742,7 +4742,7 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 			spin_unlock_irqrestore(&xhci->lock, flags);
 			xhci_change_max_exit_latency(xhci, udev, 0);
 			readl_poll_timeout(&ports[port_num]->port_reg->portsc, pm_val,
-					   (pm_val & PORT_PLS_MASK) == XDEV_U0,
+					   (pm_val & PORT_PLS_MASK) == PLS_U0,
 					   100, 10000);
 			return 0;
 		}
drivers/usb/host/xhci.h +19 -19
index 9c6c36870fb6..758155a772cc 100644
@@ -367,11 +367,11 @@ struct xhci_slot_ctx {
  */
 #define DEV_SPEED	(0xf << 20)
 #define GET_DEV_SPEED(n) (((n) & DEV_SPEED) >> 20)
-#define	SLOT_SPEED_FS		(XDEV_FS << 20)
-#define	SLOT_SPEED_LS		(XDEV_LS << 20)
-#define	SLOT_SPEED_HS		(XDEV_HS << 20)
-#define	SLOT_SPEED_SS		(XDEV_SS << 20)
-#define	SLOT_SPEED_SSP		(XDEV_SSP << 20)
+#define	SLOT_SPEED_FS		(PORT_SPEED_FS << 20)
+#define	SLOT_SPEED_LS		(PORT_SPEED_LS << 20)
+#define	SLOT_SPEED_HS		(PORT_SPEED_HS << 20)
+#define	SLOT_SPEED_SS		(PORT_SPEED_SS << 20)
+#define	SLOT_SPEED_SSP		(PORT_SPEED_SSP << 20)
 /* bit 24 reserved */
 /* Is this LS/FS device connected through a HS hub? - bit 25 */
 #define DEV_MTT		BIT(25)
@@ -2375,31 +2375,31 @@ static inline const char *xhci_decode_slot_context(char *str,
 static inline const char *xhci_portsc_link_state_string(u32 portsc)
 {
 	switch (FIELD_GET(PORT_PLS_MASK, portsc)) {
-	case XDEV_U0:
+	case PLS_U0:
 		return "U0";
-	case XDEV_U1:
+	case PLS_U1:
 		return "U1";
-	case XDEV_U2:
+	case PLS_U2:
 		return "U2";
-	case XDEV_U3:
+	case PLS_U3:
 		return "U3";
-	case XDEV_DISABLED:
+	case PLS_DISABLED:
 		return "Disabled";
-	case XDEV_RXDETECT:
+	case PLS_RXDETECT:
 		return "RxDetect";
-	case XDEV_INACTIVE:
+	case PLS_INACTIVE:
 		return "Inactive";
-	case XDEV_POLLING:
+	case PLS_POLLING:
 		return "Polling";
-	case XDEV_RECOVERY:
+	case PLS_RECOVERY:
 		return "Recovery";
-	case XDEV_HOT_RESET:
+	case PLS_HOT_RESET:
 		return "Hot Reset";
-	case XDEV_COMP_MODE:
+	case PLS_COMP_MODE:
 		return "Compliance mode";
-	case XDEV_TEST_MODE:
+	case PLS_TEST_MODE:
 		return "Test mode";
-	case XDEV_RESUME:
+	case PLS_RESUME:
 		return "Resume";
 	default:
 		break;
@@ -2416,7 +2416,7 @@ static inline const char *xhci_decode_portsc(char *str, u32 portsc)
 	if (portsc == ~(u32)0)
 		return str;
 
-	ret += sprintf(str + ret, "Speed=%ld ", FIELD_GET(DEV_SPEED_MASK, portsc));
+	ret += sprintf(str + ret, "Speed=%ld ", FIELD_GET(PORT_SPEED_MASK, portsc));
 	ret += sprintf(str + ret, "Link=%s ", xhci_portsc_link_state_string(portsc));
 
 	/* RO/ROS: Read-only */
Signature · 1 line
2.50.1
#5 lore raw

PATCH v2 5/20 usb: xhci: rework Port Indicator Control macro

In reply to Niklas Neronin

Use the standard GENMASK() helper with explicit values for the Port Indicator Control field. Rename the macros to match the xHCI specification defined abbreviation instead of using custom name.

---
 drivers/usb/host/xhci-port.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/usb/host/xhci-port.h +5 -5
index 752f33e35bf5..dca6d5c7ad1e 100644
@@ -56,11 +56,11 @@
 #define	PORT_SPEED_SS	4
 #define	PORT_SPEED_SSP	5
 
-/* Port Indicator Control */
-#define PORT_LED_OFF	(0 << 14)
-#define PORT_LED_AMBER	(1 << 14)
-#define PORT_LED_GREEN	(2 << 14)
-#define PORT_LED_MASK	(3 << 14)
+/* bits 15:14 - Port Indicator Control */
+#define PORT_PIC_MASK	GENMASK(15, 14)
+#define PIC_OFF		0
+#define PIC_AMBER	1
+#define PIC_GREEN	2
 /* Port Link State Write Strobe - set this when changing link state */
 #define PORT_LINK_STROBE	BIT(16)
 /* true: connect status change */
Signature · 1 line
2.50.1
#6 lore raw

PATCH v2 6/20 usb: xhci: rework USB3 PORTPMSC macros

In reply to Niklas Neronin

Define macros to describe only the bit masks, and use the standard FIELD_* helpers to extract, set, and update field values.

By consolidating macro usage and eliminating custom helpers, the code becomes simpler and easier to follow. The use of standard bitfield macros improves readability and enforces consistent register field handling.

---
 drivers/usb/host/xhci-hub.c  |  6 ++----
 drivers/usb/host/xhci-port.h | 23 ++++++++++++++---------
 2 files changed, 16 insertions(+), 13 deletions(-)
drivers/usb/host/xhci-hub.c +2 -4
index d464f261284d..e2b1986d217c 100644
@@ -1493,8 +1493,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 
 			timeout = (wIndex & 0xff00) >> 8;
 			portpmsc = readl(&port->port_reg->portpmsc);
-			portpmsc &= ~PORT_U1_TIMEOUT_MASK;
-			portpmsc |= PORT_U1_TIMEOUT(timeout);
+			FIELD_MODIFY(PORT_U1_TIMEOUT_MASK, &portpmsc, timeout);
 			writel(portpmsc, &port->port_reg->portpmsc);
 			break;
 		case USB_PORT_FEAT_U2_TIMEOUT:
@@ -1503,8 +1502,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 
 			timeout = (wIndex & 0xff00) >> 8;
 			portpmsc = readl(&port->port_reg->portpmsc);
-			portpmsc &= ~PORT_U2_TIMEOUT_MASK;
-			portpmsc |= PORT_U2_TIMEOUT(timeout);
+			FIELD_MODIFY(PORT_U2_TIMEOUT_MASK, &portpmsc, timeout);
 			writel(portpmsc, &port->port_reg->portpmsc);
 			break;
 		case USB_PORT_FEAT_TEST:
drivers/usb/host/xhci-port.h +14 -9
index dca6d5c7ad1e..fb4a2c4f7691 100644
@@ -117,16 +117,21 @@
 /* We mark duplicate entries with -1 */
 #define DUPLICATE_ENTRY ((u8)(-1))
 
-/* Port Power Management Status and Control - port_power_base bitmasks */
-/* Inactivity timer value for transitions into U1, in microseconds.
- * Timeout can be up to 127us.  0xFF means an infinite timeout.
+/* USB3 Port Power Management Status and Control (PORTPMSC) 5.4.9.1 */
+/*
+ * bits 7:0 - U1 Timeout, inactivity timer value for transitions into U1.
+ * Timeout can be 0us to 127us (0x7f), in 1us increments.
+ * Value 0xff means an infinite timeout.
  */
-#define PORT_U1_TIMEOUT(p)	((p) & 0xff)
-#define PORT_U1_TIMEOUT_MASK	0xff
-/* Inactivity timer value for transitions into U2 */
-#define PORT_U2_TIMEOUT(p)	(((p) & 0xff) << 8)
-#define PORT_U2_TIMEOUT_MASK	(0xff << 8)
-/* Bits 24:31 for port testing */
+#define PORT_U1_TIMEOUT_MASK	GENMASK(7, 0)
+/*
+ * bits 15:8 - U2 Timeout, inactivity timer value for transitions into U2.
+ * Timeout can be 0us to 65024ms (0xfe), in 256us increments.
+ * Value 0xff means an infinite timeout.
+ */
+#define PORT_U2_TIMEOUT_MASK	GENMASK(15, 8)
+/* bit 16 - Force Link PM Accept (FLA) */
+/* bits 31:17 - RsvdP */
 
 /* USB2 Protocol PORTSPMSC */
 #define	PORT_L1S_MASK		7
Signature · 1 line
2.50.1
#7 lore raw

PATCH v2 7/20 usb: xhci: rework USB2 PORTPMSC macros

In reply to Niklas Neronin

Define macros to describe only the bit masks, and use the standard FIELD_* helpers to extract, set, and update field values.

By consolidating macro usage and eliminating custom helpers, the code becomes simpler and easier to follow. The use of standard bitfield macros improves readability and enforces consistent register field handling.

Rename bits 7:4 mask from the legacy HIRD definition to BESL, as per xHCI specification.

Raname xhci_port_set_test_mode() local variabel 'temp' to 'portpmsc'.

---
 drivers/usb/host/xhci-hub.c  |  8 ++++----
 drivers/usb/host/xhci-port.h | 24 ++++++++++++++++--------
 drivers/usb/host/xhci.c      |  7 ++++---
 3 files changed, 24 insertions(+), 15 deletions(-)
drivers/usb/host/xhci-hub.c +4 -4
index e2b1986d217c..6b5867ca5d6b 100644
@@ -678,14 +678,14 @@ static void xhci_set_port_power(struct xhci_hcd *xhci, struct xhci_port *port,
 
 static void xhci_port_set_test_mode(struct xhci_hcd *xhci, u16 test_mode, int portnum)
 {
-	u32 temp;
+	u32 portpmsc;
 	struct xhci_port *port;
 
 	/* xhci only supports test mode for usb2 ports */
 	port = xhci->usb2_rhub.ports[portnum];
-	temp = readl(&port->port_reg->portpmsc);
-	temp |= test_mode << PORT_TEST_MODE_SHIFT;
-	writel(temp, &port->port_reg->portpmsc);
+	portpmsc = readl(&port->port_reg->portpmsc);
+	FIELD_MODIFY(PORT_TEST_MODE_MASK, &portpmsc, test_mode);
+	writel(portpmsc, &port->port_reg->portpmsc);
 	xhci->test_mode = test_mode;
 	if (test_mode == USB_TEST_FORCE_ENABLE)
 		xhci_start(xhci);
drivers/usb/host/xhci-port.h +16 -8
index fb4a2c4f7691..889a6b7f8721 100644
@@ -133,16 +133,24 @@
 /* bit 16 - Force Link PM Accept (FLA) */
 /* bits 31:17 - RsvdP */
 
-/* USB2 Protocol PORTSPMSC */
-#define	PORT_L1S_MASK		7
-#define	PORT_L1S_SUCCESS	1
+/* USB2 Port Power Management Status and Control (PORTPMSC) 5.4.9.2 */
+/* bits 2:0 - L1 Status */
+#define	PORT_L1S_MASK		GENMASK(2, 0)
+/* bit 3 - Remote Wake Enable */
 #define	PORT_RWE		BIT(3)
-#define	PORT_HIRD(p)		(((p) & 0xf) << 4)
-#define	PORT_HIRD_MASK		(0xf << 4)
-#define	PORT_L1DS_MASK		(0xff << 8)
-#define	PORT_L1DS(p)		(((p) & 0xff) << 8)
+/*
+ * bits 7:4 - Best Effort Service Latency
+ * Some host controllers may implement the pre-2011 USB 2.0 LPM definition,
+ * where this field is interpreted as Host Initiated Resume Duration (HIRD).
+ */
+#define	PORT_BESL_MASK		GENMASK(7, 4)
+/* bits 15:8 - L1 Device Slot */
+#define	PORT_L1DS_MASK		GENMASK(15, 8)
+/* bit 16 - Hardware LPM Enable */
 #define	PORT_HLE		BIT(16)
-#define PORT_TEST_MODE_SHIFT	28
+/* bits 27:17 - RsvdP */
+/* bits 31:28 - Port Test Control */
+#define PORT_TEST_MODE_MASK	GENMASK(31, 28)
 
 /* USB3 Protocol PORTLI  Port Link Information */
 #define PORT_LEC(p)		((p) & 0xffff)
drivers/usb/host/xhci.c +4 -3
index cab7bae3ecc3..0d4c31a86aad 100644
@@ -4725,8 +4725,9 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 			hird = xhci_calculate_hird_besl(xhci, udev);
 		}
 
-		pm_val &= ~PORT_HIRD_MASK;
-		pm_val |= PORT_HIRD(hird) | PORT_RWE | PORT_L1DS(udev->slot_id);
+		FIELD_MODIFY(PORT_BESL_MASK, &pm_val, hird);
+		FIELD_MODIFY(PORT_L1DS_MASK, &pm_val, udev->slot_id);
+		pm_val |= PORT_RWE;
 		writel(pm_val, &port_reg->portpmsc);
 		pm_val = readl(&port_reg->portpmsc);
 		pm_val |= PORT_HLE;
@@ -4734,7 +4735,7 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 		/* flush write */
 		readl(&port_reg->portpmsc);
 	} else {
-		pm_val &= ~(PORT_HLE | PORT_RWE | PORT_HIRD_MASK | PORT_L1DS_MASK);
+		pm_val &= ~(PORT_HLE | PORT_RWE | PORT_BESL_MASK | PORT_L1DS_MASK);
 		writel(pm_val, &port_reg->portpmsc);
 		/* flush write */
 		readl(&port_reg->portpmsc);
Signature · 1 line
2.50.1
#8 lore raw

PATCH v2 8/20 usb: xhci: rework USB3 PORTLI macros

In reply to Niklas Neronin

Define macros to describe only the bit masks, and use the standard FIELD_* helpers to extract, set, and update field values.

By consolidating macro usage and eliminating custom helpers, the code becomes simpler and easier to follow. The use of standard bitfield macros improves readability and enforces consistent register field handling.

---
 drivers/usb/host/xhci-debugfs.c |  6 ++++--
 drivers/usb/host/xhci-hub.c     |  4 ++--
 drivers/usb/host/xhci-port.h    | 12 ++++++++----
 3 files changed, 14 insertions(+), 8 deletions(-)
drivers/usb/host/xhci-debugfs.c +4 -2
index e61ee722e2cd..8a73d3ee3481 100644
@@ -402,8 +402,10 @@ static int xhci_portli_show(struct seq_file *s, void *unused)
 
 	/* PORTLI fields are valid if port is a USB3 or eUSB2V2 port */
 	if (port->rhub == &xhci->usb3_rhub)
-		seq_printf(s, "0x%08x LEC=%u RLC=%u TLC=%u\n", portli,
-			   PORT_LEC(portli), PORT_RX_LANES(portli), PORT_TX_LANES(portli));
+		seq_printf(s, "0x%08x LEC=%lu RLC=%lu TLC=%lu\n", portli,
+			   FIELD_GET(PORT_LEC_MASK, portli),
+			   FIELD_GET(PORT_RLC_MASK, portli),
+			   FIELD_GET(PORT_TLC_MASK, portli));
 	else if (xhci->hcc_params2 & HCC2_E2V2C)
 		seq_printf(s, "0x%08x RDR=%u TDR=%u\n", portli,
 			   PORTLI_RDR(portli), PORTLI_TDR(portli));
drivers/usb/host/xhci-hub.c +2 -2
index 6b5867ca5d6b..ff5645df55e3 100644
@@ -1013,8 +1013,8 @@ static u32 xhci_get_ext_port_status(u32 portsc, u32 port_li)
 	ext_stat |= speed_id;		/* bits 3:0, RX speed id */
 	ext_stat |= speed_id << 4;	/* bits 7:4, TX speed id */
 
-	ext_stat |= PORT_RX_LANES(port_li) << 8;  /* bits 11:8 Rx lane count */
-	ext_stat |= PORT_TX_LANES(port_li) << 12; /* bits 15:12 Tx lane count */
+	ext_stat |= FIELD_GET(PORT_RLC_MASK, port_li) << 8;  /* bits 11:8 Rx lane count */
+	ext_stat |= FIELD_GET(PORT_TLC_MASK, port_li) << 12; /* bits 15:12 Tx lane count */
 
 	return ext_stat;
 }
drivers/usb/host/xhci-port.h +8 -4
index 889a6b7f8721..e5bf7e7c083d 100644
@@ -152,10 +152,14 @@
 /* bits 31:28 - Port Test Control */
 #define PORT_TEST_MODE_MASK	GENMASK(31, 28)
 
-/* USB3 Protocol PORTLI  Port Link Information */
-#define PORT_LEC(p)		((p) & 0xffff)
-#define PORT_RX_LANES(p)	(((p) >> 16) & 0xf)
-#define PORT_TX_LANES(p)	(((p) >> 20) & 0xf)
+/* USB3 Port Link Info Register (PORTLI) 5.4.10.1 */
+/* bits 15:0 - Link Error Count */
+#define PORT_LEC_MASK		GENMASK(15, 0)
+/* bits 19:16 - Rx Lane Count */
+#define PORT_RLC_MASK		GENMASK(19, 16)
+/* bits 23:20 - Tx Lane Count */
+#define PORT_TLC_MASK		GENMASK(23, 20)
+/* bits 31:24 - RsvdP */
 
 /* eUSB2v2 protocol PORTLI Port Link information, RsvdP for normal USB2 */
 #define PORTLI_RDR(p)		((p) & 0xf)
Signature · 1 line
2.50.1
#9 lore raw

PATCH v2 9/20 usb: xhci: rework USB2 PORTLI macros

In reply to Niklas Neronin

Define macros to describe only the bit masks, and use the standard FIELD_* helpers to extract, set, and update field values.

By consolidating macro usage and eliminating custom helpers, the code becomes simpler and easier to follow. The use of standard bitfield macros improves readability and enforces consistent register field handling.

---
 drivers/usb/host/xhci-debugfs.c | 5 +++--
 drivers/usb/host/xhci-port.h    | 9 ++++++---
 2 files changed, 9 insertions(+), 5 deletions(-)
drivers/usb/host/xhci-debugfs.c +3 -2
index 8a73d3ee3481..02729682c7e2 100644
@@ -407,8 +407,9 @@ static int xhci_portli_show(struct seq_file *s, void *unused)
 			   FIELD_GET(PORT_RLC_MASK, portli),
 			   FIELD_GET(PORT_TLC_MASK, portli));
 	else if (xhci->hcc_params2 & HCC2_E2V2C)
-		seq_printf(s, "0x%08x RDR=%u TDR=%u\n", portli,
-			   PORTLI_RDR(portli), PORTLI_TDR(portli));
+		seq_printf(s, "0x%08x RDR=%lu TDR=%lu\n", portli,
+			   FIELD_GET(PORT_RDR_MASK, portli),
+			   FIELD_GET(PORT_TDR_MASK, portli));
 	else
 		seq_printf(s, "0x%08x RsvdP\n", portli);
 
drivers/usb/host/xhci-port.h +6 -3
index e5bf7e7c083d..f2194c84f9f2 100644
@@ -161,9 +161,12 @@
 #define PORT_TLC_MASK		GENMASK(23, 20)
 /* bits 31:24 - RsvdP */
 
-/* eUSB2v2 protocol PORTLI Port Link information, RsvdP for normal USB2 */
-#define PORTLI_RDR(p)		((p) & 0xf)
-#define PORTLI_TDR(p)		(((p) >> 4) & 0xf)
+/* USB2 Port Link Info Register (PORTLI) 5.4.10.2 */
+/* bits 3:0 - Rx Data Rate, if E2V2C=1 else RsvdP */
+#define PORT_RDR_MASK		GENMASK(3, 0)
+/* bits 7:4 - Tx Data Rate, if E2V2C=1 else RsvdP */
+#define PORT_TDR_MASK		GENMASK(7, 4)
+/* bits 31:8 - RsvdP */
 
 /* USB2 Protocol PORTHLPMC */
 #define PORT_HIRDM(p)((p) & 3)
Signature · 1 line
2.50.1
#10 lore raw

PATCH v2 10/20 usb: xhci: document USB3 PORTHLPMC register layout

In reply to Niklas Neronin

Add comments describing the USB3 Port Hardware LPM Control (PORTHLPMC) register fields as defined by the xHCI specification.

---
 drivers/usb/host/xhci-port.h | 4 ++++
 1 file changed, 4 insertions(+)
drivers/usb/host/xhci-port.h +4 -0
index f2194c84f9f2..070cf44f12ea 100644
@@ -168,6 +168,10 @@
 #define PORT_TDR_MASK		GENMASK(7, 4)
 /* bits 31:8 - RsvdP */
 
+/* USB3 Port Hardware LPM Control Register (PORTHLPMC) 5.4.11.1 */
+/* bits 15:0 - Link Soft Error Count, if LSECC=1 else RsvdP */
+/* bits 31:16 - RsvdP */
+
 /* USB2 Protocol PORTHLPMC */
 #define PORT_HIRDM(p)((p) & 3)
 #define PORT_L1_TIMEOUT(p)(((p) & 0xff) << 2)
Signature · 1 line
2.50.1
#11 lore raw

PATCH v2 11/20 usb: xhci: rework USB2 PORTHLPMC macros

In reply to Niklas Neronin

Define macros to describe only the bit masks, and use the standard FIELD_* helpers to extract, set, and update field values.

By consolidating macro usage and eliminating custom helpers, the code becomes simpler and easier to follow. The use of standard bitfield macros improves readability and enforces consistent register field handling.

---
 drivers/usb/host/xhci-port.h | 17 +++++++++++------
 drivers/usb/host/xhci.c      |  3 ++-
 2 files changed, 13 insertions(+), 7 deletions(-)
drivers/usb/host/xhci-port.h +11 -6
index 070cf44f12ea..064588335e74 100644
@@ -172,13 +172,18 @@
 /* bits 15:0 - Link Soft Error Count, if LSECC=1 else RsvdP */
 /* bits 31:16 - RsvdP */
 
-/* USB2 Protocol PORTHLPMC */
-#define PORT_HIRDM(p)((p) & 3)
-#define PORT_L1_TIMEOUT(p)(((p) & 0xff) << 2)
-#define PORT_BESLD(p)(((p) & 0xf) << 10)
-
-/* use 512 microseconds as USB2 LPM L1 default timeout. */
+/* USB2 Port Hardware LPM Control Register (PORTHLPMC) 5.4.11.2 */
+/* bits 1:0 - Host Initiated Resume Duration Mode */
+#define PORT_HIRDM_MASK		GENMASK(1, 0)
+/*
+ * bits 9:2 - L1 Timeout, can be 128us to 65280us (0xff), in 128us increments.
+ * The default timeout is 128us.
+ */
+#define PORT_L1_TIMEOUT_MASK	GENMASK(9, 2)
 #define XHCI_L1_TIMEOUT		512
+/* bits 13:10 - Best Effort Service Latency Deep */
+#define PORT_BESLD_MASK		GENMASK(13, 10)
+/* bits 31:14 - RsvdP */
 
 /* Set default HIRD/BESL value to 4 (350/400us) for USB2 L1 LPM resume latency.
  * Safe to use with mixed HIRD and BESL systems (host and device) and is used
drivers/usb/host/xhci.c +2 -1
index 0d4c31a86aad..3c9e3244f941 100644
@@ -4655,7 +4655,8 @@ static int xhci_calculate_usb2_hw_lpm_params(struct usb_device *udev)
 		hirdm = 1;
 	}
 
-	return PORT_BESLD(besld) | PORT_L1_TIMEOUT(l1) | PORT_HIRDM(hirdm);
+	return FIELD_PREP(PORT_BESLD_MASK, besld) | FIELD_PREP(PORT_L1_TIMEOUT_MASK, l1) |
+	       FIELD_PREP(PORT_HIRDM_MASK, hirdm);
 }
 
 static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
Signature · 1 line
2.50.1
#12 lore raw

PATCH v2 12/20 usb: xhci: update port register bitfield comments for consistency

In reply to Niklas Neronin

No functional changes.

Update comments in xhci-port.h to improve readability and ensure a consistent format across all Port Register bit fields.

Each field now includes a brief description of its name, along with the xHCI specification abbreviation when the macro uses a different name, and its valid value range where applicable.

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

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

---
 drivers/usb/host/xhci-port.h | 63 ++++++++++++++++++++----------------
 drivers/usb/host/xhci.c      |  2 +-
 2 files changed, 36 insertions(+), 29 deletions(-)
drivers/usb/host/xhci-port.h +35 -28
index 064588335e74..f10ce1f24eb4 100644
@@ -1,17 +1,21 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * xHCI Host Controller USB Port Register Set
+ * xHCI Specification Section 5.4, Revision 1.2.
+ */
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
 
-/* PORTSC - Port Status and Control Register - port_status_base bitmasks */
-/* true: device connected */
+/* Port Status and Control (PORTSC) 5.4.8 */
+/* bit 0 - Current Connect Status (CCS) */
 #define PORT_CONNECT	BIT(0)
-/* true: port enabled */
+/* bit 1 - Port Enabled/Disabled (PED) */
 #define PORT_PE		BIT(1)
-/* bit 2 reserved and zeroed */
-/* true: port has an over-current condition */
+/* bit 2 - Rsvd */
+/* bit 3 - Over-current Active (OCA) */
 #define PORT_OC		BIT(3)
-/* true: port reset signaling asserted */
+/* bit 4 - Port Reset (PR) */
 #define PORT_RESET	BIT(4)
 /*
  * bits 8:5 - Port Link State, by default '5'.
@@ -61,23 +65,26 @@
 #define PIC_OFF		0
 #define PIC_AMBER	1
 #define PIC_GREEN	2
-/* Port Link State Write Strobe - set this when changing link state */
+/* bit 16 - Port Link State Write Strobe (LWS), set this when changing link state */
 #define PORT_LINK_STROBE	BIT(16)
-/* true: connect status change */
+/* bit 17 - Connect Status Change */
 #define PORT_CSC	BIT(17)
-/* true: port enable change */
+/* bit 18 - Port Enabled/Disabled Change */
 #define PORT_PEC	BIT(18)
-/* true: warm reset for a USB 3.0 device is done.  A "hot" reset puts the port
+/*
+ * bit 19 - Warm Port Reset Change
+ * Warm reset for a USB 3.0 device is done.  A "hot" reset puts the port
  * into an enabled state, and the device into the default state.  A "warm" reset
  * also resets the link, forcing the device through the link training sequence.
  * SW can also look at the Port Reset register to see when warm reset is done.
  */
 #define PORT_WRC	BIT(19)
-/* true: over-current change */
+/* bit 20 - Over-current Change */
 #define PORT_OCC	BIT(20)
-/* true: reset change - 1 to 0 transition of PORT_RESET */
+/* bit 21 - Port Reset Change (PRC) */
 #define PORT_RC		BIT(21)
-/* port link status change - set on some port link state transitions:
+/*
+ * bit 22 - Port Link State Change, set on some port link state transitions:
  *  Transition				Reason
  *  ------------------------------------------------------------------------------
  *  - U3 to Resume			Wakeup signaling from a device
@@ -91,28 +98,27 @@
  *  - Any state to inactive		Error on USB 3.0 port
  */
 #define PORT_PLC	BIT(22)
-/* port configure error change - port failed to configure its link partner */
+/* bit 23 - Port Config Error Change, port failed to configure its link partner */
 #define PORT_CEC	BIT(23)
-#define PORT_CHANGE_MASK	(PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | \
-				 PORT_RC | PORT_PLC | PORT_CEC)
-
-
-/* Cold Attach Status - xHC can set this bit to report device attached during
- * Sx state. Warm port reset should be perfomed to clear this bit and move port
- * to connected state.
+/*
+ * bit 24 - Cold Attach Status
+ * xHC can set this bit to report device attached during Sx state.
+ * Warm port reset should be perfomed to clear this bit and move port to connected state.
  */
 #define PORT_CAS	BIT(24)
-/* wake on connect (enable) */
+/* bit 25 - Wake on Connect Enable (WCE) */
 #define PORT_WKCONN_E	BIT(25)
-/* wake on disconnect (enable) */
+/* bit 26 - Wake on Disconnect Enable (WDE) */
 #define PORT_WKDISC_E	BIT(26)
-/* wake on over-current (enable) */
+/* bit 27 - Wake on Over-current Enable (WOE) */
 #define PORT_WKOC_E	BIT(27)
-/* bits 28:29 reserved */
-/* true: device is non-removable - for USB 3.0 roothub emulation */
+/* bits 29:28 - RsvdZ */
+/* bit 30 - Device Removable (DR), for USB 3.0 roothub emulation */
 #define PORT_DEV_REMOVE	BIT(30)
-/* Initiate a warm port reset - complete when PORT_WRC is '1' */
+/* bit 31 - Warm Port Reset (WPR), complete when PORT_WRC is '1' */
 #define PORT_WR		BIT(31)
+#define PORT_CHANGE_MASK	(PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | \
+				 PORT_RC | PORT_PLC | PORT_CEC)
 
 /* We mark duplicate entries with -1 */
 #define DUPLICATE_ENTRY ((u8)(-1))
@@ -185,7 +191,8 @@
 #define PORT_BESLD_MASK		GENMASK(13, 10)
 /* bits 31:14 - RsvdP */
 
-/* Set default HIRD/BESL value to 4 (350/400us) for USB2 L1 LPM resume latency.
+/*
+ * Set default HIRD/BESL value to 4 (350/400us) for USB2 L1 LPM resume latency.
  * Safe to use with mixed HIRD and BESL systems (host and device) and is used
  * by other operating systems.
  *
drivers/usb/host/xhci.c +1 -1
index 3c9e3244f941..d3fcca94e06a 100644
@@ -4700,7 +4700,7 @@ static int xhci_set_usb2_hardware_lpm(struct usb_hcd *hcd,
 		if (udev->usb2_hw_lpm_besl_capable) {
 			/* if device doesn't have a preferred BESL value use a
 			 * default one which works with mixed HIRD and BESL
-			 * systems. See XHCI_DEFAULT_BESL definition in xhci.h
+			 * systems. See XHCI_DEFAULT_BESL definition in xhci-ports.h
 			 */
 			field = le32_to_cpu(udev->bos->ext_cap->bmAttributes);
 			if ((field & USB_BESL_SUPPORT) &&
Signature · 1 line
2.50.1
#13 lore raw

PATCH v2 13/20 usb: xhci: rename port register macros to xHCI spec abbreviations

In reply to Niklas Neronin

The driver currently uses two different naming schemes for PORTSC field macros:
- xHCI specification abbreviations (e.g. CCS, PED, OCA)
- custom driver-specific names (e.g. CONNECT, PE, OC)

The custom naming is inconsistent and does not follow a clear pattern, making the macros harder to search for and correlate with the xHCI specification.

Rename the custom macros to use the standard xHCI abbreviations so that the code matches the specification terminology. This improves readability, reduces ambiguity, and makes it easier to cross-reference with the spec.

Verified identical object code before and after the renaming by comparing objdump -dr output.

No functional change intended.

---
 drivers/usb/dwc3/host.c         |  2 +-
 drivers/usb/early/xhci-dbc.c    |  4 +-
 drivers/usb/host/xhci-debugfs.c |  2 +-
 drivers/usb/host/xhci-hub.c     | 92 ++++++++++++++++-----------------
 drivers/usb/host/xhci-pci.c     |  2 +-
 drivers/usb/host/xhci-port.h    | 50 +++++++++---------
 drivers/usb/host/xhci-ring.c    |  6 +--
 drivers/usb/host/xhci-tegra.c   |  6 +--
 drivers/usb/host/xhci.c         |  4 +-
 drivers/usb/host/xhci.h         | 24 ++++-----
 10 files changed, 96 insertions(+), 96 deletions(-)
drivers/usb/dwc3/host.c +1 -1
index c5674161b2b0..e0605b1a9c37 100644
@@ -54,7 +54,7 @@ static void dwc3_power_off_all_roothub_ports(struct dwc3 *dwc)
 		for (i = 1; i <= port_num; i++) {
 			offset = op_regs_base + XHCI_PORTSC_BASE + 0x10 * (i - 1);
 			reg = readl(xhci_regs + offset);
-			reg &= ~PORT_POWER;
+			reg &= ~PORT_PP;
 			writel(reg, xhci_regs + offset);
 		}
 
drivers/usb/early/xhci-dbc.c +2 -2
index 41118bba9197..ec2efd0e92eb 100644
@@ -359,8 +359,8 @@ static void xdbc_do_reset_debug_port(u32 id, u32 count)
 	for (i = id; i < (id + count); i++) {
 		portsc = ops_reg + 0x400 + i * 0x10;
 		val = readl(portsc);
-		if (!(val & PORT_CONNECT))
-			writel(val | PORT_RESET, portsc);
+		if (!(val & PORT_CCS))
+			writel(val | PORT_PR, portsc);
 	}
 }
 
drivers/usb/host/xhci-debugfs.c +1 -1
index 02729682c7e2..fb4c216253c1 100644
@@ -367,7 +367,7 @@ static ssize_t xhci_port_write(struct file *file,  const char __user *ubuf,
 		}
 		portsc = xhci_port_state_to_neutral(portsc);
 		FIELD_MODIFY(PORT_PLS_MASK, &portsc, PLS_COMP_MODE);
-		portsc |= PORT_LINK_STROBE;
+		portsc |= PORT_LWS;
 		xhci_portsc_writel(port, portsc);
 		spin_unlock_irqrestore(&xhci->lock, flags);
 	} else {
drivers/usb/host/xhci-hub.c +46 -46

279 lines not on this page · raw message

drivers/usb/host/xhci-pci.c +1 -1
index a63f85961700..3088ec4c3d05 100644
@@ -933,7 +933,7 @@ static int xhci_pci_poweroff_late(struct usb_hcd *hcd, bool do_wakeup)
 		xhci_dbg(xhci, "port %d-%d in U3 without wakeup, disable it\n",
 			 port->rhub->hcd->self.busnum, port->hcd_portnum + 1);
 		portsc = xhci_port_state_to_neutral(portsc);
-		xhci_portsc_writel(port, portsc | PORT_PE);
+		xhci_portsc_writel(port, portsc | PORT_PED);
 	}
 
 	return 0;
drivers/usb/host/xhci-port.h +25 -25

83 lines not on this page · raw message

drivers/usb/host/xhci-ring.c +3 -3
index 7ea79501c035..6e06c6733570 100644
@@ -2061,9 +2061,9 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 	 * Tag broken links to avoid retries while hub driver sorts it out.
 	 * Link status is not relible while port is in reset.
 	 */
-	if (!(portsc & PORT_RESET)) {
+	if (!(portsc & PORT_PR)) {
 		port->link_inactive = (pls == PLS_INACTIVE);
-		port->connected = !!(portsc & PORT_CONNECT);
+		port->connected = !!(portsc & PORT_CCS);
 	}
 
 	pls = FIELD_GET(PORT_PLS_MASK, portsc);
@@ -2146,7 +2146,7 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
 	if (hcd->speed < HCD_USB3) {
 		xhci_test_and_clear_bit(xhci, port, PORT_PLC);
 		if ((xhci->quirks & XHCI_RESET_PLL_ON_DISCONNECT) &&
-		    (portsc & PORT_CSC) && !(portsc & PORT_CONNECT))
+		    (portsc & PORT_CSC) && !(portsc & PORT_CCS))
 			xhci_cavium_reset_phy_quirk(xhci);
 	}
 
drivers/usb/host/xhci-tegra.c +3 -3
index 6a8fb386c2d8..620c7f67c8b3 100644
@@ -2034,7 +2034,7 @@ static bool xhci_hub_ports_suspended(struct xhci_hub *hub)
 
 	for (i = 0; i < hub->num_ports; i++) {
 		value = xhci_portsc_readl(hub->ports[i]);
-		if ((value & PORT_PE) == 0)
+		if ((value & PORT_PED) == 0)
 			continue;
 
 		if (FIELD_GET(PORT_PLS_MASK, value) != PLS_U3) {
@@ -2822,7 +2822,7 @@ static int tegra_xhci_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value,
 				return -EPIPE;
 			ports = rhub->ports;
 			portsc = xhci_portsc_readl(ports[port]);
-			if (portsc & PORT_CONNECT)
+			if (portsc & PORT_CCS)
 				tegra_phy_xusb_utmi_pad_power_on(phy);
 		}
 	}
@@ -2841,7 +2841,7 @@ static int tegra_xhci_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value,
 		if ((type_req == ClearPortFeature) && (value == USB_PORT_FEAT_C_CONNECTION)) {
 			ports = rhub->ports;
 			portsc = xhci_portsc_readl(ports[port]);
-			if (!(portsc & PORT_CONNECT)) {
+			if (!(portsc & PORT_CCS)) {
 				/* We don't suspend the PAD while HNP role swap happens on the OTG
 				 * port
 				 */
drivers/usb/host/xhci.c +2 -2
index d3fcca94e06a..3f80000fc5a9 100644
@@ -31,7 +31,7 @@
 #define DRIVER_AUTHOR "Sarah Sharp"
 #define DRIVER_DESC "'eXtensible' Host Controller (xHC) Driver"
 
-#define	PORT_WAKE_BITS	(PORT_WKOC_E | PORT_WKDISC_E | PORT_WKCONN_E)
+#define	PORT_WAKE_BITS	(PORT_WOE | PORT_WDE | PORT_WCE)
 
 /* Some 0.95 hardware can't handle the chain bit on a Link TRB being cleared */
 static int link_quirk;
@@ -896,7 +896,7 @@ static void xhci_disable_hub_port_wake(struct xhci_hcd *xhci,
 			t2 &= ~PORT_WAKE_BITS;
 
 		/* Don't touch csc bit if connected or connect change is set */
-		if (!(portsc & (PORT_CSC | PORT_CONNECT)))
+		if (!(portsc & (PORT_CSC | PORT_CCS)))
 			t2 |= PORT_CSC;
 
 		if (t1 != t2) {
drivers/usb/host/xhci.h +12 -12
index 758155a772cc..2808b363f2f8 100644
@@ -2420,37 +2420,37 @@ static inline const char *xhci_decode_portsc(char *str, u32 portsc)
 	ret += sprintf(str + ret, "Link=%s ", xhci_portsc_link_state_string(portsc));
 
 	/* RO/ROS: Read-only */
-	if (portsc & PORT_CONNECT)
+	if (portsc & PORT_CCS)
 		ret += sprintf(str + ret, "CCS ");
-	if (portsc & PORT_OC)
+	if (portsc & PORT_OCA)
 		ret += sprintf(str + ret, "OCA "); /* No set for USB2 ports */
 	if (portsc & PORT_CAS)
 		ret += sprintf(str + ret, "CAS ");
-	if (portsc & PORT_DEV_REMOVE)
+	if (portsc & PORT_DR)
 		ret += sprintf(str + ret, "DR ");
 
 	/* RWS; writing 1 sets the bit, writing 0 clears the bit. */
-	if (portsc & PORT_POWER)
+	if (portsc & PORT_PP)
 		ret += sprintf(str + ret, "PP ");
-	if (portsc & PORT_WKCONN_E)
+	if (portsc & PORT_WCE)
 		ret += sprintf(str + ret, "WCE ");
-	if (portsc & PORT_WKDISC_E)
+	if (portsc & PORT_WDE)
 		ret += sprintf(str + ret, "WDE ");
-	if (portsc & PORT_WKOC_E)
+	if (portsc & PORT_WOE)
 		ret += sprintf(str + ret, "WOE ");
 
 	/* RW; writing 1 sets the bit, writing 0 clears the bit */
-	if (portsc & PORT_LINK_STROBE)
+	if (portsc & PORT_LWS)
 		ret += sprintf(str + ret, "LWS "); /* LWS 0 write is ignored */
 
 	/* RW1S; writing 1 sets the bit, writing 0 has no effect */
-	if (portsc & PORT_RESET)
+	if (portsc & PORT_PR)
 		ret += sprintf(str + ret, "PR ");
-	if (portsc & PORT_WR)
+	if (portsc & PORT_WPR)
 		ret += sprintf(str + ret, "WPR "); /* RsvdZ for USB2 ports */
 
 	/* RW1CS; writing 1 clears the bit, writing 0 has no effect. */
-	if (portsc & PORT_PE)
+	if (portsc & PORT_PED)
 		ret += sprintf(str + ret, "PED ");
 	if (portsc & PORT_CSC)
 		ret += sprintf(str + ret, "CSC ");
@@ -2460,7 +2460,7 @@ static inline const char *xhci_decode_portsc(char *str, u32 portsc)
 		ret += sprintf(str + ret, "WRC "); /* RsvdZ for USB2 ports */
 	if (portsc & PORT_OCC)
 		ret += sprintf(str + ret, "OCC ");
-	if (portsc & PORT_RC)
+	if (portsc & PORT_PRC)
 		ret += sprintf(str + ret, "PRC ");
 	if (portsc & PORT_PLC)
 		ret += sprintf(str + ret, "PLC ");
Signature · 1 line
2.50.1
#14 lore raw

PATCH v2 14/20 usb: xhci: replace magic numbers with Port macros

In reply to Niklas Neronin

No functional change.

Replace open-coded bit masks with the corresponding symbolic PORT_* macros.

This removes hard-coded shifts and masks (e.g. (1 << n), (x << m)) in favor of named definitions, making the code easier to read and modify in future patches.

---
 drivers/usb/host/xhci-hub.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/usb/host/xhci-hub.c +8 -6
index c3e57acdad2a..fb75a171f5a0 100644
@@ -397,18 +397,19 @@ static unsigned int xhci_port_speed(int portsc)
  * connect status and port speed are also sticky - meaning they're in
  * the AUX well and they aren't changed by a hot, warm, or cold reset.
  */
-#define	XHCI_PORT_RO	((1<<0) | (1<<3) | (0xf<<10) | (1<<30))
+#define	XHCI_PORT_RO	(PORT_CCS | PORT_OCA | PORT_SPEED_MASK | PORT_DR)
 /*
  * These bits are RW; writing a 0 clears the bit, writing a 1 sets the bit:
- * bits 5:8, 9, 14:15, 25:27
+ * bits 5:8, 9, 14:15, 25, 26, 27
  * link state, port power, port indicator state, "wake on" enable state
  */
-#define XHCI_PORT_RWS	((0xf<<5) | (1<<9) | (0x3<<14) | (0x7<<25))
+#define XHCI_PORT_RWS	(PORT_PLS_MASK | PORT_PP | PORT_PIC_MASK | PORT_WCE | \
+			 PORT_WDE | PORT_WOE)
 /*
  * These bits are RW; writing a 1 sets the bit, writing a 0 has no effect:
  * bit 4 (port reset)
  */
-#define	XHCI_PORT_RW1S	((1<<4))
+#define	XHCI_PORT_RW1S	(PORT_PR)
 /*
  * These bits are RW; writing a 1 clears the bit, writing a 0 has no effect:
  * bits 1, 17, 18, 19, 20, 21, 22, 23
@@ -416,12 +417,13 @@ static unsigned int xhci_port_speed(int portsc)
  * change bits: connect, PED, warm port reset changed (reserved zero for USB 2.0 ports),
  * over-current, reset, link state, and L1 change
  */
-#define XHCI_PORT_RW1CS	((1<<1) | (0x7f<<17))
+#define XHCI_PORT_RW1CS	(PORT_PED | PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | PORT_PRC | \
+			 PORT_PLC | PORT_CEC)
 /*
  * Bit 16 is RW, and writing a '1' to it causes the link state control to be
  * latched in
  */
-#define	XHCI_PORT_RW	((1<<16))
+#define	XHCI_PORT_RW	(PORT_LWS)
 /*
  * These bits are Reserved Zero (RsvdZ) and zero should be written to them:
  * bits 2, 24, 28:31
Signature · 1 line
2.50.1
#15 lore raw

PATCH v2 15/20 usb: xhci: update PORTSC aggregate macros

In reply to Niklas Neronin

PORTSC bit 24 is no longer reserved and is defined as Cold Attach Status (CAS), a read-only field. Include it in 'XHCI_PORT_RO' and remove it from the reserved mask.

PORTSC bit 31 is Warm Port Reset (WPR), used on USB 3 ports and write-1-to-set. Include it in 'XHCI_PORT_RW1S', the macro is currently not in use, hence no visible issues.

---
 drivers/usb/host/xhci-hub.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/usb/host/xhci-hub.c +6 -6
index fb75a171f5a0..365b821675a7 100644
@@ -392,12 +392,12 @@ static unsigned int xhci_port_speed(int portsc)
 
 /*
  * These bits are Read Only (RO) and should be saved and written to the
- * registers: 0, 3, 10:13, 30
+ * registers: 0, 3, 10:13, 24, 30
  * connect status, over-current status, port speed, and device removable.
  * connect status and port speed are also sticky - meaning they're in
  * the AUX well and they aren't changed by a hot, warm, or cold reset.
  */
-#define	XHCI_PORT_RO	(PORT_CCS | PORT_OCA | PORT_SPEED_MASK | PORT_DR)
+#define	XHCI_PORT_RO	(PORT_CCS | PORT_OCA | PORT_SPEED_MASK | PORT_CAS | PORT_DR)
 /*
  * These bits are RW; writing a 0 clears the bit, writing a 1 sets the bit:
  * bits 5:8, 9, 14:15, 25, 26, 27
@@ -407,9 +407,9 @@ static unsigned int xhci_port_speed(int portsc)
 			 PORT_WDE | PORT_WOE)
 /*
  * These bits are RW; writing a 1 sets the bit, writing a 0 has no effect:
- * bit 4 (port reset)
+ * bits 4, 31
  */
-#define	XHCI_PORT_RW1S	(PORT_PR)
+#define	XHCI_PORT_RW1S	(PORT_PR | PORT_WPR)
 /*
  * These bits are RW; writing a 1 clears the bit, writing a 0 has no effect:
  * bits 1, 17, 18, 19, 20, 21, 22, 23
@@ -426,9 +426,9 @@ static unsigned int xhci_port_speed(int portsc)
 #define	XHCI_PORT_RW	(PORT_LWS)
 /*
  * These bits are Reserved Zero (RsvdZ) and zero should be written to them:
- * bits 2, 24, 28:31
+ * bits 2, 28:31
  */
-#define	XHCI_PORT_RZ	((1<<2) | (1<<24) | (0xf<<28))
+#define	XHCI_PORT_RZ	((1<<2) | (0xf<<28))
 
 /**
  * xhci_port_state_to_neutral() - Clean up read portsc value back into writeable
Signature · 1 line
2.50.1
#16 lore raw

PATCH v2 16/20 usb: xhci: consolidate PORTSC RW1CS bit macros

In reply to Niklas Neronin

There are currently multiple macros describing overlapping sets of write-1-to-clear (RW1CS) bits in the PORTSC register:

  PORT_CHANGE_MASK  (      CSC | PEC | WRC | OCC | PRC | PLC | CEC)
  PORT_RWC_BITS     (PED | CSC | PEC | WRC | OCC | PRC | PLC      )
  XHCI_PORT_RW1CS   (PED | CSC | PEC | WRC | OCC | PRC | PLC | CEC)

These definitions largely duplicate each other, with 'XHCI_PORT_RW1CS' being a superset of the others. This duplication adds unnecessary complexity and makes it harder to reason about which bits are being manipulated in different contexts.

Remove the redundant macros and introduce a single 'PORTSC_RW1CS_BITS' definition covering all RW1CS bits. Callers can mask out or extend the set as needed for specific use cases.

This simplifies the code, avoids duplication, and makes the intent clearer when handling PORTSC change/status bits.

---
 drivers/usb/host/xhci-hub.c  | 21 ++++-----------------
 drivers/usb/host/xhci-port.h |  5 +++--
 drivers/usb/host/xhci.c      | 10 ++++++----
 3 files changed, 13 insertions(+), 23 deletions(-)
drivers/usb/host/xhci-hub.c +4 -17
index 365b821675a7..48814dea1b60 100644
@@ -18,8 +18,6 @@
 #include "xhci-trace.h"
 
 #define	PORT_WAKE_BITS	(PORT_WOE | PORT_WDE | PORT_WCE)
-#define	PORT_RWC_BITS	(PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | \
-			 PORT_PRC | PORT_PLC | PORT_PED)
 
 /* Default sublink speed attribute of each lane */
 static u32 ssp_cap_default_ssa[] = {
@@ -410,15 +408,6 @@ static unsigned int xhci_port_speed(int portsc)
  * bits 4, 31
  */
 #define	XHCI_PORT_RW1S	(PORT_PR | PORT_WPR)
-/*
- * These bits are RW; writing a 1 clears the bit, writing a 0 has no effect:
- * bits 1, 17, 18, 19, 20, 21, 22, 23
- * port enable/disable, and
- * change bits: connect, PED, warm port reset changed (reserved zero for USB 2.0 ports),
- * over-current, reset, link state, and L1 change
- */
-#define XHCI_PORT_RW1CS	(PORT_PED | PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | PORT_PRC | \
-			 PORT_PLC | PORT_CEC)
 /*
  * Bit 16 is RW, and writing a '1' to it causes the link state control to be
  * latched in
@@ -1339,10 +1328,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 				 * Clear all change bits, so that we get a new
 				 * connection event.
 				 */
-				portsc |= PORT_CSC | PORT_PEC | PORT_WRC |
-					  PORT_OCC | PORT_PRC | PORT_PLC |
-					  PORT_CEC;
-				xhci_portsc_writel(port, portsc | PORT_PED);
+				portsc |= PORTSC_RW1CS_BITS;
+				xhci_portsc_writel(port, portsc);
 				portsc = xhci_portsc_readl(port);
 				break;
 			}
@@ -1835,7 +1822,7 @@ static bool xhci_port_missing_cas_quirk(struct xhci_port *port)
 		return false;
 
 	/* clear wakeup/change bits, and do a warm port reset */
-	portsc &= ~(PORT_RWC_BITS | PORT_CEC | PORT_WAKE_BITS);
+	portsc &= ~(PORTSC_RW1CS_BITS | PORT_WAKE_BITS);
 	portsc |= PORT_WPR;
 	xhci_portsc_writel(port, portsc);
 	/* flush write */
@@ -1915,7 +1902,7 @@ int xhci_bus_resume(struct usb_hcd *hcd)
 				break;
 			}
 		/* disable wake for all ports, write new link state if needed */
-		portsc &= ~(PORT_RWC_BITS | PORT_CEC | PORT_WAKE_BITS);
+		portsc &= ~(PORTSC_RW1CS_BITS | PORT_WAKE_BITS);
 		xhci_portsc_writel(ports[port_index], portsc);
 	}
 
drivers/usb/host/xhci-port.h +3 -2
index 69d583d1faae..78735dc0e899 100644
@@ -117,8 +117,9 @@
 #define PORT_DR		BIT(30)
 /* bit 31 - Warm Port Reset, complete when PORT_WRC is '1' */
 #define PORT_WPR		BIT(31)
-#define PORT_CHANGE_MASK	(PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | \
-				 PORT_PRC | PORT_PLC | PORT_CEC)
+/* Writing 1 clears the bit, writing 0 has no effect. */
+#define PORTSC_RW1CS_BITS	(PORT_PED | PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | PORT_PRC | \
+				 PORT_PLC | PORT_CEC)
 
 /* We mark duplicate entries with -1 */
 #define DUPLICATE_ENTRY ((u8)(-1))
drivers/usb/host/xhci.c +6 -4
index 3f80000fc5a9..9fc5b30c75ec 100644
@@ -914,6 +914,7 @@ static bool xhci_pending_portevent(struct xhci_hcd *xhci)
 	int			port_index;
 	u32			status;
 	u32			portsc;
+	u32			mask;
 
 	status = readl(&xhci->op_regs->status);
 	if (status & STS_EINT)
@@ -926,18 +927,19 @@ static bool xhci_pending_portevent(struct xhci_hcd *xhci)
 
 	port_index = xhci->usb2_rhub.num_ports;
 	ports = xhci->usb2_rhub.ports;
+	/* Check all Sticky Write-1-to-clear status bits, except for the Port Enadled bit. */
+	mask = PORTSC_RW1CS_BITS & ~PORT_PED;
 	while (port_index--) {
 		portsc = xhci_portsc_readl(ports[port_index]);
-		if (portsc & PORT_CHANGE_MASK ||
-		    FIELD_GET(PORT_PLS_MASK, portsc) == PLS_RESUME)
+		if (portsc & mask || FIELD_GET(PORT_PLS_MASK, portsc) == PLS_RESUME)
 			return true;
 	}
 	port_index = xhci->usb3_rhub.num_ports;
 	ports = xhci->usb3_rhub.ports;
+	mask |= PORT_CAS;
 	while (port_index--) {
 		portsc = xhci_portsc_readl(ports[port_index]);
-		if (portsc & (PORT_CHANGE_MASK | PORT_CAS) ||
-		    FIELD_GET(PORT_PLS_MASK, portsc) == PLS_RESUME)
+		if (portsc & mask || FIELD_GET(PORT_PLS_MASK, portsc) == PLS_RESUME)
 			return true;
 	}
 	return false;
Signature · 1 line
2.50.1
#17 lore raw

PATCH v2 17/20 usb: xhci: relocate all port register macros to xhci-port.h

In reply to Niklas Neronin

Move all port register macros to xhci-port.h and place them next to the corresponding macros for improved organization and readability.

Rename the macros to PORTSC_*_BITS to better reflect that they describe bitfields within the PORTSC register rather than the entire port register.

Remove macro XHCI_PORT_RZ.

---
 drivers/usb/host/xhci-hub.c  | 35 +----------------------------------
 drivers/usb/host/xhci-port.h | 10 ++++++++++
 drivers/usb/host/xhci.c      |  2 --
 3 files changed, 11 insertions(+), 36 deletions(-)
drivers/usb/host/xhci-hub.c +1 -34
index 48814dea1b60..5f389773041d 100644
@@ -17,8 +17,6 @@
 #include "xhci.h"
 #include "xhci-trace.h"
 
-#define	PORT_WAKE_BITS	(PORT_WOE | PORT_WDE | PORT_WCE)
-
 /* Default sublink speed attribute of each lane */
 static u32 ssp_cap_default_ssa[] = {
 	0x00050034, /* USB 3.0 SS Gen1x1 id:4 symmetric rx 5Gbps */
@@ -388,37 +386,6 @@ static unsigned int xhci_port_speed(int portsc)
 	return 0;
 }
 
-/*
- * These bits are Read Only (RO) and should be saved and written to the
- * registers: 0, 3, 10:13, 24, 30
- * connect status, over-current status, port speed, and device removable.
- * connect status and port speed are also sticky - meaning they're in
- * the AUX well and they aren't changed by a hot, warm, or cold reset.
- */
-#define	XHCI_PORT_RO	(PORT_CCS | PORT_OCA | PORT_SPEED_MASK | PORT_CAS | PORT_DR)
-/*
- * These bits are RW; writing a 0 clears the bit, writing a 1 sets the bit:
- * bits 5:8, 9, 14:15, 25, 26, 27
- * link state, port power, port indicator state, "wake on" enable state
- */
-#define XHCI_PORT_RWS	(PORT_PLS_MASK | PORT_PP | PORT_PIC_MASK | PORT_WCE | \
-			 PORT_WDE | PORT_WOE)
-/*
- * These bits are RW; writing a 1 sets the bit, writing a 0 has no effect:
- * bits 4, 31
- */
-#define	XHCI_PORT_RW1S	(PORT_PR | PORT_WPR)
-/*
- * Bit 16 is RW, and writing a '1' to it causes the link state control to be
- * latched in
- */
-#define	XHCI_PORT_RW	(PORT_LWS)
-/*
- * These bits are Reserved Zero (RsvdZ) and zero should be written to them:
- * bits 2, 28:31
- */
-#define	XHCI_PORT_RZ	((1<<2) | (0xf<<28))
-
 /**
  * xhci_port_state_to_neutral() - Clean up read portsc value back into writeable
  * @portsc: u32 port value read from portsc register to be cleanup up
@@ -437,7 +404,7 @@ static unsigned int xhci_port_speed(int portsc)
 u32 xhci_port_state_to_neutral(u32 portsc)
 {
 	/* Save read-only status and port state */
-	return (portsc & XHCI_PORT_RO) | (portsc & XHCI_PORT_RWS);
+	return (portsc & PORTSC_RO_BITS) | (portsc & PORTSC_RWS_BITS);
 }
 EXPORT_SYMBOL_GPL(xhci_port_state_to_neutral);
 
drivers/usb/host/xhci-port.h +10 -0
index 78735dc0e899..074300acf361 100644
@@ -112,14 +112,24 @@
 #define PORT_WDE	BIT(26)
 /* bit 27 - Wake on Over-current Enable */
 #define PORT_WOE	BIT(27)
+#define	PORT_WAKE_BITS	(PORT_WOE | PORT_WDE | PORT_WCE)
 /* bits 29:28 - RsvdZ */
 /* bit 30 - Device Removable, for USB 3.0 roothub emulation */
 #define PORT_DR		BIT(30)
 /* bit 31 - Warm Port Reset, complete when PORT_WRC is '1' */
 #define PORT_WPR		BIT(31)
+/* These bits are read-only, should be saved and written to the registers. */
+#define	PORTSC_RO_BITS		(PORT_CCS | PORT_OCA | PORT_SPEED_MASK | PORT_CAS | PORT_DR)
+/* Writing 0 clears the bit, writing 1 sets the bit. */
+#define PORTSC_RWS_BITS		(PORT_PLS_MASK | PORT_PP | PORT_PIC_MASK | PORT_WCE | \
+				 PORT_WDE | PORT_WOE)
 /* Writing 1 clears the bit, writing 0 has no effect. */
 #define PORTSC_RW1CS_BITS	(PORT_PED | PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | PORT_PRC | \
 				 PORT_PLC | PORT_CEC)
+/* Writing 1 sets the bit, writing 0 has no effect. */
+#define PORTSC_RW1S_BITS	(PORT_PR | PORT_WPR)
+/* Writing 1 sets the bit, writing 0 clears the bit. */
+#define	PORTSC_RW_BITS		(PORT_LWS)
 
 /* We mark duplicate entries with -1 */
 #define DUPLICATE_ENTRY ((u8)(-1))
drivers/usb/host/xhci.c +0 -2
index 9fc5b30c75ec..8ccf796e5989 100644
@@ -31,8 +31,6 @@
 #define DRIVER_AUTHOR "Sarah Sharp"
 #define DRIVER_DESC "'eXtensible' Host Controller (xHC) Driver"
 
-#define	PORT_WAKE_BITS	(PORT_WOE | PORT_WDE | PORT_WCE)
-
 /* Some 0.95 hardware can't handle the chain bit on a Link TRB being cleared */
 static int link_quirk;
 module_param(link_quirk, int, S_IRUGO | S_IWUSR);
Signature · 1 line
2.50.1
#18 lore raw

PATCH v2 18/20 usb: xhci: preserve RW bits in xhci_port_state_to_neutral()

In reply to Niklas Neronin

xhci_port_state_to_neutral() prepares a PORTSC value that can be written back without changing port state by dropping bits with side effects on write (e.g. Read-Write-1-to-Clear).

Currently the helper preserves Read-Only (RO) and Read/Write Sticky (RWS) bits, but omits standard Read/Write (RW) bits. This does not currently cause issues, since the only RW bit, Port Link State Write Strobe (LWS), always reads as 0 and ignores 0 writes.

Preserve RW bits as well to make the helper consistent with its intent and robust against future changes to PORTSC definitions.

Update the function comment accordingly.

---
 drivers/usb/host/xhci-hub.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)
drivers/usb/host/xhci-hub.c +11 -14
index 5f389773041d..e0d4ee605f82 100644
@@ -386,25 +386,22 @@ static unsigned int xhci_port_speed(int portsc)
 	return 0;
 }
 
-/**
- * xhci_port_state_to_neutral() - Clean up read portsc value back into writeable
- * @portsc: u32 port value read from portsc register to be cleanup up
+/*
+ * Given a PORTSC register value, return a "neutral" value that can be written
+ * back to the register without changing the current port state.
  *
- * Given a portsc, this function returns a value that would result in the
- * port being in the same state, if the value was written to the port status
- * control register.
- * Save Read Only (RO) bits and save read/write bits where
- * writing a 0 clears the bit and writing a 1 sets the bit (RWS).
- * For all other types (RW1S, RW1CS, RW, and RZ), writing a '0' has no effect.
+ * The function preserves:
+ *  - Read-Only (RO) bits
+ *  - Read/Write (RW) bits
+ *  - Read/Write Sticky (RWS) bits
  *
- * Return: u32 value that can be written back to portsc register without
- * changing port state.
+ * For other bit types (e.g. RW1S, RW1CS, and RsvdZ), writing 0 has no effect,
+ * so they are intentionally cleared in the returned value to avoid unintended
+ * side effects.
  */
-
 u32 xhci_port_state_to_neutral(u32 portsc)
 {
-	/* Save read-only status and port state */
-	return (portsc & PORTSC_RO_BITS) | (portsc & PORTSC_RWS_BITS);
+	return portsc & (PORTSC_RO_BITS | PORTSC_RWS_BITS | PORTSC_RW_BITS);
 }
 EXPORT_SYMBOL_GPL(xhci_port_state_to_neutral);
 
Signature · 1 line
2.50.1
#19 lore raw

PATCH v2 19/20 usb: xhci: improve xhci_port_missing_cas_quirk()

In reply to Niklas Neronin

Previously the function dropped all RW1CS and Wake bits and set the Warm Port Reset (WPR) bit.

This was done by manually clearing RW1CS and Wake bits and then setting WPR. Instead, use xhci_port_state_to_neutral(), which already drops all bits with write side effects (e.g. RW1CS, RW1S and reserved-zero fields), and produces a safe value for writeback.

Afterwards, explicitly clear the Wake bits.

Compared to the previous logic, this also correctly clears additional bits such as reserved-zero (bits 2, 28, 29) and RW1S bit (bit 4), which could otherwise lead to unintended behavior.

Overall this simplifies the code and aligns it with the generic PORTSC sanitization logic.

---
 drivers/usb/host/xhci-hub.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-hub.c +3 -2
index e0d4ee605f82..e5f542ae3106 100644
@@ -1785,8 +1785,9 @@ static bool xhci_port_missing_cas_quirk(struct xhci_port *port)
 	if (pls != PLS_POLLING && pls != PLS_COMP_MODE)
 		return false;
 
-	/* clear wakeup/change bits, and do a warm port reset */
-	portsc &= ~(PORTSC_RW1CS_BITS | PORT_WAKE_BITS);
+	portsc = xhci_port_state_to_neutral(portsc);
+	/* clear wakeup and do a warm port reset */
+	portsc &= ~PORT_WAKE_BITS;
 	portsc |= PORT_WPR;
 	xhci_portsc_writel(port, portsc);
 	/* flush write */
Signature · 1 line
2.50.1
#20 lore raw

PATCH v2 20/20 usb: xhci: use neutral helper when resuming ports

In reply to Niklas Neronin

xhci_bus_resume() sanitizes PORTSC before writing it back. Previously, this was done by manually clearing RW1CS and Wake bits and then setting Warm Port Reset (WPR).

Replace this with xhci_port_state_to_neutral(), which already clears all bits with write side effects (e.g. RW1CS, RW1S, and reserved-zero fields) and produces a safe value for writeback. Afterwards, explicitly clear the Wake bits as required.

This also ensures that additional bits, such as reserved-zero bits (e.g. 2, 28, 29) and RW1S bits (e.g. bit 4), are consistently cleared, avoiding unintended effects when writing back PORTSC.

xhci_port_state_to_neutral() preserves Port Link State (PLS), allowing the subsequent resume logic to operate on the current link state.

---
 drivers/usb/host/xhci-hub.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/usb/host/xhci-hub.c +7 -5
index e5f542ae3106..446c66030f8f 100644
@@ -1838,8 +1838,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
 	}
 	port_index = max_ports;
 	while (port_index--) {
-		portsc = xhci_portsc_readl(ports[port_index]);
-
 		/* warm reset CAS limited ports stuck in polling/compliance */
 		if ((xhci->quirks & XHCI_MISSING_CAS) &&
 		    (hcd->speed >= HCD_USB3) &&
@@ -1849,11 +1847,17 @@ int xhci_bus_resume(struct usb_hcd *hcd)
 			clear_bit(port_index, &bus_state->bus_suspended);
 			continue;
 		}
+
+		portsc = xhci_portsc_readl(ports[port_index]);
+		portsc = xhci_port_state_to_neutral(portsc);
+
+		/* Disable wake for all ports */
+		portsc &= ~PORT_WAKE_BITS;
+
 		/* resume if we suspended the link, and it is still suspended */
 		if (test_bit(port_index, &bus_state->bus_suspended))
 			switch (FIELD_GET(PORT_PLS_MASK, portsc)) {
 			case PLS_U3:
-				portsc = xhci_port_state_to_neutral(portsc);
 				FIELD_MODIFY(PORT_PLS_MASK, &portsc, next_state);
 				portsc |= PORT_LWS;
 				break;
@@ -1866,8 +1870,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)
 					  &bus_state->bus_suspended);
 				break;
 			}
-		/* disable wake for all ports, write new link state if needed */
-		portsc &= ~(PORTSC_RW1CS_BITS | PORT_WAKE_BITS);
 		xhci_portsc_writel(ports[port_index], portsc);
 	}
 
Signature · 1 line
2.50.1