* [PATCH 17/25] usb: xhci: separate use of USB Chapter 11 PLS macros from xHCI-specific PLS macros
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
The xhci driver uses two different sources for Port Link State (PLS):
1. The PLS field in the PORTSC register (bits 8:5).
2. The PLS value encoded in bits 15:8 of the USB request wIndex,
received by xhci_hub_control().
While both represent similar link states, they differ in a few details,
for example, xHCI's Resume State. Because of these differences, the xhci
driver defines its own set of PLS macros in xhci-port.h, which are intended
to be used when reading and writing PORTSC. The generic USB Chapter 11
macros in ch11.h should only be used when parsing or replying to hub-class
USB requests.
To avoid mixing these two representations and prevent incorrect state
reporting, replace all uses of Chapter 11 PLS macros with the xHCI
versions when interacting with the PORTSC register.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-hub.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index b57fe0967e10..7fb17799cfdc 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -866,8 +866,8 @@ static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
* unless we're already in compliance
* or the inactive state.
*/
- if (pls != USB_SS_PORT_LS_COMP_MOD &&
- pls != USB_SS_PORT_LS_SS_INACTIVE) {
+ if (pls != XDEV_COMP_MODE &&
+ pls != XDEV_INACTIVE) {
pls = USB_SS_PORT_LS_COMP_MOD;
}
/* Return also connection bit -
@@ -895,7 +895,7 @@ static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
* caused by a delay on the host-device negotiation.
*/
if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) &&
- (pls == USB_SS_PORT_LS_COMP_MOD))
+ (pls == XDEV_COMP_MODE))
pls |= USB_PORT_STAT_CONNECTION;
}
@@ -1365,7 +1365,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
if (link_state == USB_SS_PORT_LS_RX_DETECT) {
xhci_dbg(xhci, "Enable port %d-%d\n",
hcd->self.busnum, portnum + 1);
- xhci_set_link_state(xhci, port, link_state);
+ xhci_set_link_state(xhci, port, XDEV_RXDETECT);
temp = xhci_portsc_readl(port);
break;
}
@@ -1397,7 +1397,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
xhci_dbg(xhci, "Enable compliance mode transition for port %d-%d\n",
hcd->self.busnum, portnum + 1);
- xhci_set_link_state(xhci, port, link_state);
+ xhci_set_link_state(xhci, port, XDEV_COMP_MODE);
temp = xhci_portsc_readl(port);
break;
@@ -1435,7 +1435,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
reinit_completion(&port->u3exit_done);
}
if (pls <= XDEV_U3) /* U1, U2, U3 */
- xhci_set_link_state(xhci, port, USB_SS_PORT_LS_U0);
+ xhci_set_link_state(xhci, port, XDEV_U0);
if (!wait_u0) {
if (pls > XDEV_U3)
goto error;
@@ -1461,7 +1461,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
xhci_stop_device(xhci, port->slot_id, 1);
spin_lock_irqsave(&xhci->lock, flags);
}
- xhci_set_link_state(xhci, port, USB_SS_PORT_LS_U3);
+ xhci_set_link_state(xhci, port, XDEV_U3);
spin_unlock_irqrestore(&xhci->lock, flags);
while (retries--) {
usleep_range(4000, 8000);
--
2.43.0
^ permalink raw reply related
* [PATCH 16/25] usb: xhci: clean up 'wValue' handling in xhci_hub_control()
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Several hub control requests encode a descriptor type in the upper byte
of 'wValue'. Clean this up by extracting the descriptor type into a local
variable and using it for all relevant requests.
Replace magic value (0x02) with the appropriate macro (HUB_EXT_PORT_STATUS)
This improves readability and makes the handling of 'wValue' consistent.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-hub.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index daa04ac811fc..b57fe0967e10 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1209,6 +1209,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
u16 wake_mask;
u8 timeout;
u8 test_mode;
+ u8 desc_type;
struct xhci_hub *rhub;
struct xhci_port **ports;
struct xhci_port *port;
@@ -1226,13 +1227,13 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
memset(buf, 0, 4);
break;
case GetHubDescriptor:
+ desc_type = (wValue & 0xff00) >> 8;
/* Check to make sure userspace is asking for the USB 3.0 hub
* descriptor for the USB 3.0 roothub. If not, we stall the
* endpoint, like external hubs do.
*/
if (hcd->speed >= HCD_USB3 &&
- (wLength < USB_DT_SS_HUB_SIZE ||
- wValue != (USB_DT_SS_HUB << 8))) {
+ (wLength < USB_DT_SS_HUB_SIZE || desc_type != USB_DT_SS_HUB)) {
xhci_dbg(xhci, "Wrong hub descriptor type for "
"USB 3.0 roothub.\n");
goto error;
@@ -1241,7 +1242,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
(struct usb_hub_descriptor *) buf);
break;
case DeviceRequest | USB_REQ_GET_DESCRIPTOR:
- if ((wValue & 0xff00) != (USB_DT_BOS << 8))
+ desc_type = (wValue & 0xff00) >> 8;
+ if (desc_type != USB_DT_BOS)
goto error;
if (hcd->speed < HCD_USB3)
@@ -1272,7 +1274,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
put_unaligned(cpu_to_le32(status), (__le32 *) buf);
/* if USB 3.1 extended port status return additional 4 bytes */
- if (wValue == 0x02) {
+ if (wValue == HUB_EXT_PORT_STATUS) {
u32 port_li;
if (hcd->speed < HCD_USB31 || wLength != 8) {
--
2.43.0
^ permalink raw reply related
* [PATCH 15/25] usb: xhci: clean up handling of upper bits in SetPortFeature wIndex
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
In Set Port Feature requests, the upper byte of 'wIndex' encodes
feature-specific parameters. The current code reads these upper bits in
an early pre-processing block, and then the same feature is handled again
later in the main switch statement. This results in duplicated condition
checks and makes the control flow harder to follow.
Move all feature-specific extraction of 'wIndex' upper bits into the
main SetPortFeature logic so that each feature is handled in exactly one
place. This reduces duplication, makes the handling clearer, and keeps
'wIndex' parsing local to the code that actually uses the values.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-hub.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index de843ecc7d63..daa04ac811fc 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1205,10 +1205,10 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
u32 temp, status;
int retval = 0;
struct xhci_bus_state *bus_state;
- u16 link_state = 0;
- u16 wake_mask = 0;
- u16 timeout = 0;
- u16 test_mode = 0;
+ u16 link_state;
+ u16 wake_mask;
+ u8 timeout;
+ u8 test_mode;
struct xhci_hub *rhub;
struct xhci_port **ports;
struct xhci_port *port;
@@ -1286,15 +1286,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
}
break;
case SetPortFeature:
- if (wValue == USB_PORT_FEAT_LINK_STATE)
- link_state = (wIndex & 0xff00) >> 3;
- if (wValue == USB_PORT_FEAT_REMOTE_WAKE_MASK)
- wake_mask = wIndex & 0xff00;
- if (wValue == USB_PORT_FEAT_TEST)
- test_mode = (wIndex & 0xff00) >> 8;
- /* The MSB of wIndex is the U1/U2 timeout */
- timeout = (wIndex & 0xff00) >> 8;
-
portnum = (wIndex & 0xff) - 1;
if (!in_range(portnum, 0, max_ports))
goto error;
@@ -1349,6 +1340,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
bus_state->suspended_ports |= 1 << portnum;
break;
case USB_PORT_FEAT_LINK_STATE:
+ link_state = (wIndex & 0xff00) >> 3;
temp = xhci_portsc_readl(port);
/* Disable port */
if (link_state == USB_SS_PORT_LS_SS_DISABLED) {
@@ -1498,6 +1490,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
hcd->self.busnum, portnum + 1, temp);
break;
case USB_PORT_FEAT_REMOTE_WAKE_MASK:
+ wake_mask = wIndex & 0xff00;
xhci_set_remote_wake_mask(xhci, port, wake_mask);
temp = xhci_portsc_readl(port);
xhci_dbg(xhci, "set port remote wake mask, actual port %d-%d status = 0x%x\n",
@@ -1511,6 +1504,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
case USB_PORT_FEAT_U1_TIMEOUT:
if (hcd->speed < HCD_USB3)
goto error;
+
+ timeout = (wIndex & 0xff00) >> 8;
temp = readl(&port->port_reg->portpmsc);
temp &= ~PORT_U1_TIMEOUT_MASK;
temp |= PORT_U1_TIMEOUT(timeout);
@@ -1519,6 +1514,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
case USB_PORT_FEAT_U2_TIMEOUT:
if (hcd->speed < HCD_USB3)
goto error;
+
+ timeout = (wIndex & 0xff00) >> 8;
temp = readl(&port->port_reg->portpmsc);
temp &= ~PORT_U2_TIMEOUT_MASK;
temp |= PORT_U2_TIMEOUT(timeout);
@@ -1528,6 +1525,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
/* 4.19.6 Port Test Modes (USB2 Test Mode) */
if (hcd->speed != HCD_USB2)
goto error;
+
+ test_mode = (wIndex & 0xff00) >> 8;
if (test_mode > USB_TEST_FORCE_ENABLE ||
test_mode < USB_TEST_J)
goto error;
--
2.43.0
^ permalink raw reply related
* [PATCH 14/25] usb: xhci: rename 'wIndex' parameters to 'portnum'
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Several helper functions take a parameter named 'wIndex', but the
value they receive is not the raw USB request wIndex field. The only
function that actually processes the USB hub request parameter is
xhci_hub_control(), which extracts the relevant port number (and other
upper-byte fields) before passing them down.
To avoid confusion between the USB request parameter and the derived
0-based port index, rename all such function parameters from 'wIndex'
to 'portnum'. This improves readability and makes the call intentions
clearer.
When a function accept struct 'xhci_port' pointer, use its port number
instead.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-hub.c | 60 +++++++++++++++++--------------------
1 file changed, 27 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 4730beae2478..de843ecc7d63 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -577,8 +577,8 @@ static void xhci_disable_port(struct xhci_hcd *xhci, struct xhci_port *port)
hcd->self.busnum, port->hcd_portnum + 1, portsc);
}
-static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue,
- u16 wIndex, struct xhci_port *port, u32 port_status)
+static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue, struct xhci_port *port,
+ u32 port_status)
{
char *port_change_bit;
u32 status;
@@ -625,7 +625,7 @@ static void xhci_clear_port_change_bit(struct xhci_hcd *xhci, u16 wValue,
port_status = xhci_portsc_readl(port);
xhci_dbg(xhci, "clear port%d %s change, portsc: 0x%x\n",
- wIndex + 1, port_change_bit, port_status);
+ port->hcd_portnum + 1, port_change_bit, port_status);
}
struct xhci_hub *xhci_get_rhub(struct usb_hcd *hcd)
@@ -675,14 +675,13 @@ static void xhci_set_port_power(struct xhci_hcd *xhci, struct xhci_port *port,
spin_lock_irqsave(&xhci->lock, *flags);
}
-static void xhci_port_set_test_mode(struct xhci_hcd *xhci,
- u16 test_mode, u16 wIndex)
+static void xhci_port_set_test_mode(struct xhci_hcd *xhci, u16 test_mode, int portnum)
{
u32 temp;
struct xhci_port *port;
/* xhci only supports test mode for usb2 ports */
- port = xhci->usb2_rhub.ports[wIndex];
+ 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);
@@ -691,8 +690,8 @@ static void xhci_port_set_test_mode(struct xhci_hcd *xhci,
xhci_start(xhci);
}
-static int xhci_enter_test_mode(struct xhci_hcd *xhci,
- u16 test_mode, u16 wIndex, unsigned long *flags)
+static int xhci_enter_test_mode(struct xhci_hcd *xhci, u16 test_mode, int portnum,
+ unsigned long *flags)
__must_hold(&xhci->lock)
{
int i, retval;
@@ -726,10 +725,8 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
/* Disable runtime PM for test mode */
pm_runtime_forbid(xhci_to_hcd(xhci)->self.controller);
/* Set PORTPMSC.PTC field to enter selected test mode */
- /* Port is selected by wIndex. port_id = wIndex + 1 */
- xhci_dbg(xhci, "Enter Test Mode: %d, Port_id=%d\n",
- test_mode, wIndex + 1);
- xhci_port_set_test_mode(xhci, test_mode, wIndex);
+ xhci_dbg(xhci, "Enter Test Mode: %u, Port_id=%d\n", test_mode, portnum + 1);
+ xhci_port_set_test_mode(xhci, test_mode, portnum);
return retval;
}
@@ -913,8 +910,7 @@ static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
* the compliance mode timer is deleted. A port won't enter
* compliance mode if it has previously entered U0.
*/
-static void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32 status,
- u16 wIndex)
+static void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32 status, int portnum)
{
u32 all_ports_seen_u0 = ((1 << xhci->usb3_rhub.num_ports) - 1);
bool port_in_u0 = ((status & PORT_PLS_MASK) == XDEV_U0);
@@ -923,7 +919,7 @@ static void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32 status,
return;
if ((xhci->port_status_u0 != all_ports_seen_u0) && port_in_u0) {
- xhci->port_status_u0 |= 1 << wIndex;
+ xhci->port_status_u0 |= 1 << portnum;
if (xhci->port_status_u0 == all_ports_seen_u0) {
timer_delete_sync(&xhci->comp_mode_recovery_timer);
xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
@@ -941,12 +937,12 @@ static int xhci_handle_usb2_port_link_resume(struct xhci_port *port,
struct xhci_bus_state *bus_state;
struct xhci_hcd *xhci;
struct usb_hcd *hcd;
- u32 wIndex;
+ int portnum;
hcd = port->rhub->hcd;
bus_state = &port->rhub->bus_state;
xhci = hcd_to_xhci(hcd);
- wIndex = port->hcd_portnum;
+ portnum = port->hcd_portnum;
if ((portsc & PORT_RESET) || !(portsc & PORT_PE)) {
return -EINVAL;
@@ -954,7 +950,7 @@ static int xhci_handle_usb2_port_link_resume(struct xhci_port *port,
/* did port event handler already start resume timing? */
if (!port->resume_timestamp) {
/* If not, maybe we are in a host initiated resume? */
- if (test_bit(wIndex, &bus_state->resuming_ports)) {
+ if (test_bit(portnum, &bus_state->resuming_ports)) {
/* Host initiated resume doesn't time the resume
* signalling using resume_done[].
* It manually sets RESUME state, sleeps 20ms
@@ -968,20 +964,20 @@ static int xhci_handle_usb2_port_link_resume(struct xhci_port *port,
unsigned long timeout = jiffies +
msecs_to_jiffies(USB_RESUME_TIMEOUT);
- set_bit(wIndex, &bus_state->resuming_ports);
+ set_bit(portnum, &bus_state->resuming_ports);
port->resume_timestamp = timeout;
mod_timer(&hcd->rh_timer, timeout);
- usb_hcd_start_port_resume(&hcd->self, wIndex);
+ usb_hcd_start_port_resume(&hcd->self, portnum);
}
/* Has resume been signalled for USB_RESUME_TIME yet? */
} else if (time_after_eq(jiffies, port->resume_timestamp)) {
int time_left;
xhci_dbg(xhci, "resume USB2 port %d-%d\n",
- hcd->self.busnum, wIndex + 1);
+ hcd->self.busnum, portnum + 1);
port->resume_timestamp = 0;
- clear_bit(wIndex, &bus_state->resuming_ports);
+ clear_bit(portnum, &bus_state->resuming_ports);
reinit_completion(&port->rexit_done);
port->rexit_active = true;
@@ -1005,7 +1001,7 @@ static int xhci_handle_usb2_port_link_resume(struct xhci_port *port,
int port_status = xhci_portsc_readl(port);
xhci_warn(xhci, "Port resume timed out, port %d-%d: 0x%x\n",
- hcd->self.busnum, wIndex + 1, port_status);
+ hcd->self.busnum, portnum + 1, port_status);
/*
* keep rexit_active set if U0 transition failed so we
* know to report PORT_STAT_SUSPEND status back to
@@ -1014,9 +1010,9 @@ static int xhci_handle_usb2_port_link_resume(struct xhci_port *port,
*/
}
- usb_hcd_end_port_resume(&hcd->self, wIndex);
- bus_state->port_c_suspend |= 1 << wIndex;
- bus_state->suspended_ports &= ~(1 << wIndex);
+ usb_hcd_end_port_resume(&hcd->self, portnum);
+ bus_state->port_c_suspend |= 1 << portnum;
+ bus_state->suspended_ports &= ~(1 << portnum);
}
return 0;
@@ -1153,10 +1149,8 @@ static void xhci_get_usb2_port_status(struct xhci_port *port, u32 *status,
* - Stop the Synopsys redriver Compliance Mode polling.
* - Drop and reacquire the xHCI lock, in order to wait for port resume.
*/
-static u32 xhci_get_port_status(struct usb_hcd *hcd,
- struct xhci_bus_state *bus_state,
- u16 wIndex, u32 raw_port_status,
- unsigned long *flags)
+static u32 xhci_get_port_status(struct usb_hcd *hcd, struct xhci_bus_state *bus_state,
+ int portnum, u32 raw_port_status, unsigned long *flags)
__releases(&xhci->lock)
__acquires(&xhci->lock)
{
@@ -1165,7 +1159,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
struct xhci_port *port;
rhub = xhci_get_rhub(hcd);
- port = rhub->ports[wIndex];
+ port = rhub->ports[portnum];
/* common wPortChange bits */
if (raw_port_status & PORT_CSC)
@@ -1196,7 +1190,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
xhci_get_usb2_port_status(port, &status, raw_port_status,
flags);
- if (bus_state->port_c_suspend & (1 << wIndex))
+ if (bus_state->port_c_suspend & (1 << portnum))
status |= USB_PORT_STAT_C_SUSPEND << 16;
return status;
@@ -1598,7 +1592,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
case USB_PORT_FEAT_C_ENABLE:
case USB_PORT_FEAT_C_PORT_LINK_STATE:
case USB_PORT_FEAT_C_PORT_CONFIG_ERROR:
- xhci_clear_port_change_bit(xhci, wValue, portnum, port, temp);
+ xhci_clear_port_change_bit(xhci, wValue, port, temp);
break;
case USB_PORT_FEAT_ENABLE:
xhci_disable_port(xhci, port);
--
2.43.0
^ permalink raw reply related
* [PATCH 13/25] usb: xhci: stop treating 'wIndex' as a mutable port number
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
The USB request parameter 'wIndex' is a 16-bit field whose meaning depends
on the request type. For hub port operations, only bits 7:0 encode the port
number (1..MaxPorts). Despite this, the current code extracts the port
number into 'portnum1' while also modifying and using 'wIndex' directly as
a 0-based port index. This dual use is both confusing and error-prone,
since 'wIndex' is not always a pure port number.
Clean this up by deriving a single 0-based 'portnum' from 'wIndex' and
using it throughout the function. The original 'wIndex' value is no longer
modified or treated as a port number. This also matches existing xhci code.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-hub.c | 65 +++++++++++++++++--------------------
1 file changed, 30 insertions(+), 35 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 04cc3d681495..4730beae2478 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1218,13 +1218,12 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
struct xhci_hub *rhub;
struct xhci_port **ports;
struct xhci_port *port;
- int portnum1;
+ int portnum;
rhub = xhci_get_rhub(hcd);
ports = rhub->ports;
max_ports = rhub->num_ports;
bus_state = &rhub->bus_state;
- portnum1 = wIndex & 0xff;
spin_lock_irqsave(&xhci->lock, flags);
switch (typeReq) {
@@ -1258,11 +1257,11 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
spin_unlock_irqrestore(&xhci->lock, flags);
return retval;
case GetPortStatus:
- if (!portnum1 || portnum1 > max_ports)
+ portnum = (wIndex & 0xff) - 1;
+ if (!in_range(portnum, 0, max_ports))
goto error;
- wIndex--;
- port = ports[portnum1 - 1];
+ port = ports[portnum];
temp = xhci_portsc_readl(port);
if (temp == ~(u32)0) {
xhci_hc_died(xhci);
@@ -1270,13 +1269,12 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
break;
}
trace_xhci_get_port_status(port, temp);
- status = xhci_get_port_status(hcd, bus_state, wIndex, temp,
- &flags);
+ status = xhci_get_port_status(hcd, bus_state, portnum, temp, &flags);
if (status == 0xffffffff)
goto error;
xhci_dbg(xhci, "Get port status %d-%d read: 0x%x, return 0x%x",
- hcd->self.busnum, portnum1, temp, status);
+ hcd->self.busnum, portnum + 1, temp, status);
put_unaligned(cpu_to_le32(status), (__le32 *) buf);
/* if USB 3.1 extended port status return additional 4 bytes */
@@ -1303,12 +1301,11 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
/* The MSB of wIndex is the U1/U2 timeout */
timeout = (wIndex & 0xff00) >> 8;
- wIndex &= 0xff;
- if (!portnum1 || portnum1 > max_ports)
+ portnum = (wIndex & 0xff) - 1;
+ if (!in_range(portnum, 0, max_ports))
goto error;
- port = ports[portnum1 - 1];
- wIndex--;
+ port = ports[portnum];
temp = xhci_portsc_readl(port);
if (temp == ~(u32)0) {
xhci_hc_died(xhci);
@@ -1335,7 +1332,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
if ((temp & PORT_PE) == 0 || (temp & PORT_RESET)
|| (temp & PORT_PLS_MASK) >= XDEV_U3) {
xhci_warn(xhci, "USB core suspending port %d-%d not in U0/U1/U2\n",
- hcd->self.busnum, portnum1);
+ hcd->self.busnum, portnum + 1);
goto error;
}
@@ -1355,14 +1352,14 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
spin_lock_irqsave(&xhci->lock, flags);
temp = xhci_portsc_readl(port);
- bus_state->suspended_ports |= 1 << wIndex;
+ bus_state->suspended_ports |= 1 << portnum;
break;
case USB_PORT_FEAT_LINK_STATE:
temp = xhci_portsc_readl(port);
/* Disable port */
if (link_state == USB_SS_PORT_LS_SS_DISABLED) {
xhci_dbg(xhci, "Disable port %d-%d\n",
- hcd->self.busnum, portnum1);
+ hcd->self.busnum, portnum + 1);
temp = xhci_port_state_to_neutral(temp);
/*
* Clear all change bits, so that we get a new
@@ -1379,7 +1376,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
/* Put link in RxDetect (enable port) */
if (link_state == USB_SS_PORT_LS_RX_DETECT) {
xhci_dbg(xhci, "Enable port %d-%d\n",
- hcd->self.busnum, portnum1);
+ hcd->self.busnum, portnum + 1);
xhci_set_link_state(xhci, port, link_state);
temp = xhci_portsc_readl(port);
break;
@@ -1411,7 +1408,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
}
xhci_dbg(xhci, "Enable compliance mode transition for port %d-%d\n",
- hcd->self.busnum, portnum1);
+ hcd->self.busnum, portnum + 1);
xhci_set_link_state(xhci, port, link_state);
temp = xhci_portsc_readl(port);
@@ -1425,7 +1422,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
/* Can't set port link state above '3' (U3) */
if (link_state > USB_SS_PORT_LS_U3) {
xhci_warn(xhci, "Cannot set port %d-%d link state %d\n",
- hcd->self.busnum, portnum1, link_state);
+ hcd->self.busnum, portnum + 1, link_state);
goto error;
}
@@ -1460,7 +1457,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
if (!wait_for_completion_timeout(&port->u3exit_done,
msecs_to_jiffies(500)))
xhci_dbg(xhci, "missing U0 port change event for port %d-%d\n",
- hcd->self.busnum, portnum1);
+ hcd->self.busnum, portnum + 1);
spin_lock_irqsave(&xhci->lock, flags);
temp = xhci_portsc_readl(port);
break;
@@ -1486,7 +1483,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
}
spin_lock_irqsave(&xhci->lock, flags);
temp = xhci_portsc_readl(port);
- bus_state->suspended_ports |= 1 << wIndex;
+ bus_state->suspended_ports |= 1 << portnum;
}
break;
case USB_PORT_FEAT_POWER:
@@ -1504,13 +1501,13 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp = xhci_portsc_readl(port);
xhci_dbg(xhci, "set port reset, actual port %d-%d status = 0x%x\n",
- hcd->self.busnum, portnum1, temp);
+ hcd->self.busnum, portnum + 1, temp);
break;
case USB_PORT_FEAT_REMOTE_WAKE_MASK:
xhci_set_remote_wake_mask(xhci, port, wake_mask);
temp = xhci_portsc_readl(port);
xhci_dbg(xhci, "set port remote wake mask, actual port %d-%d status = 0x%x\n",
- hcd->self.busnum, portnum1, temp);
+ hcd->self.busnum, portnum + 1, temp);
break;
case USB_PORT_FEAT_BH_PORT_RESET:
temp |= PORT_WR;
@@ -1540,8 +1537,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
if (test_mode > USB_TEST_FORCE_ENABLE ||
test_mode < USB_TEST_J)
goto error;
- retval = xhci_enter_test_mode(xhci, test_mode, wIndex,
- &flags);
+ retval = xhci_enter_test_mode(xhci, test_mode, portnum, &flags);
break;
default:
goto error;
@@ -1550,12 +1546,11 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp = xhci_portsc_readl(port);
break;
case ClearPortFeature:
- if (!portnum1 || portnum1 > max_ports)
+ portnum = (wIndex & 0xff) - 1;
+ if (!in_range(portnum, 0, max_ports))
goto error;
- port = ports[portnum1 - 1];
-
- wIndex--;
+ port = ports[portnum];
temp = xhci_portsc_readl(port);
if (temp == ~(u32)0) {
xhci_hc_died(xhci);
@@ -1575,17 +1570,17 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
if ((temp & PORT_PE) == 0)
goto error;
- set_bit(wIndex, &bus_state->resuming_ports);
- usb_hcd_start_port_resume(&hcd->self, wIndex);
+ set_bit(portnum, &bus_state->resuming_ports);
+ usb_hcd_start_port_resume(&hcd->self, portnum);
xhci_set_link_state(xhci, port, XDEV_RESUME);
spin_unlock_irqrestore(&xhci->lock, flags);
msleep(USB_RESUME_TIMEOUT);
spin_lock_irqsave(&xhci->lock, flags);
xhci_set_link_state(xhci, port, XDEV_U0);
- clear_bit(wIndex, &bus_state->resuming_ports);
- usb_hcd_end_port_resume(&hcd->self, wIndex);
+ clear_bit(portnum, &bus_state->resuming_ports);
+ usb_hcd_end_port_resume(&hcd->self, portnum);
}
- bus_state->port_c_suspend |= 1 << wIndex;
+ bus_state->port_c_suspend |= 1 << portnum;
if (!port->slot_id) {
xhci_dbg(xhci, "slot_id is zero\n");
@@ -1594,7 +1589,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
xhci_ring_device(xhci, port->slot_id);
break;
case USB_PORT_FEAT_C_SUSPEND:
- bus_state->port_c_suspend &= ~(1 << wIndex);
+ bus_state->port_c_suspend &= ~(1 << portnum);
fallthrough;
case USB_PORT_FEAT_C_RESET:
case USB_PORT_FEAT_C_BH_PORT_RESET:
@@ -1603,7 +1598,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
case USB_PORT_FEAT_C_ENABLE:
case USB_PORT_FEAT_C_PORT_LINK_STATE:
case USB_PORT_FEAT_C_PORT_CONFIG_ERROR:
- xhci_clear_port_change_bit(xhci, wValue, wIndex, port, temp);
+ xhci_clear_port_change_bit(xhci, wValue, portnum, port, temp);
break;
case USB_PORT_FEAT_ENABLE:
xhci_disable_port(xhci, port);
--
2.43.0
^ permalink raw reply related
* [PATCH 12/25] usb: xhci: optimize resuming from S4 (suspend-to-disk)
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
On resume from S4 (power loss after suspend/hibernation), the xHCI
driver previously freed, reallocated, and fully reinitialized all
data structures. Most of this is unnecessary because the data is
restored from a saved image; only the xHCI registers lose their values.
This patch optimizes S4 resume by performing only a host controller
reset, which includes:
* Freeing or clearing runtime-created data.
* Rewriting xHCI registers.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 4 +--
drivers/usb/host/xhci.c | 51 ++++++++++++++++++++++---------------
drivers/usb/host/xhci.h | 2 ++
3 files changed, 34 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index f1b4f06d4b8b..4156822eb000 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -936,7 +936,7 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev,
* that tt_info, then free the child first. Recursive.
* We can't rely on udev at this point to find child-parent relationships.
*/
-static void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_id)
+void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_id)
{
struct xhci_virt_device *vdev;
struct list_head *tt_list_head;
@@ -1905,7 +1905,7 @@ void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct xhci_interrup
EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
/* Cleanup roothub bandwidth data */
-static void xhci_rh_bw_cleanup(struct xhci_hcd *xhci)
+void xhci_rh_bw_cleanup(struct xhci_hcd *xhci)
{
struct xhci_root_port_bw_info *rh_bw;
struct xhci_tt_bw_info *tt_info, *tt_next;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2c573aad4464..ece3ff7916ff 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1082,9 +1082,11 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
{
u32 command, temp = 0;
struct usb_hcd *hcd = xhci_to_hcd(xhci);
+ struct xhci_segment *seg;
int retval = 0;
bool pending_portevent = false;
bool suspended_usb3_devs = false;
+ bool reset_registers = false;
if (!hcd->state)
return 0;
@@ -1103,10 +1105,11 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
spin_lock_irq(&xhci->lock);
- if (xhci->quirks & XHCI_RESET_ON_RESUME || xhci->broken_suspend)
- power_lost = true;
-
- if (!power_lost) {
+ if (power_lost || xhci->broken_suspend || xhci->quirks & XHCI_RESET_ON_RESUME) {
+ xhci_dbg(xhci, "HC state lost, performing host controller reset\n");
+ reset_registers = true;
+ } else {
+ xhci_dbg(xhci, "HC state intact, continuing without reset\n");
/*
* Some controllers might lose power during suspend, so wait
* for controller not ready bit to clear, just as in xHC init.
@@ -1144,11 +1147,11 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
temp = readl(&xhci->op_regs->status);
if ((temp & (STS_SRE | STS_HCE)) && !(xhci->xhc_state & XHCI_STATE_REMOVING)) {
xhci_warn(xhci, "xHC error in resume, USBSTS 0x%x, Reinit\n", temp);
- power_lost = true;
+ reset_registers = true;
}
}
- if (power_lost) {
+ if (reset_registers) {
if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) &&
!(xhci_all_ports_seen_u0(xhci))) {
timer_delete_sync(&xhci->comp_mode_recovery_timer);
@@ -1172,27 +1175,33 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
if (retval)
return retval;
- xhci_dbg(xhci, "// Disabling event ring interrupts\n");
- temp = readl(&xhci->op_regs->status);
- writel((temp & ~0x1fff) | STS_EINT, &xhci->op_regs->status);
- xhci_disable_interrupter(xhci, xhci->interrupters[0]);
+ cancel_delayed_work_sync(&xhci->cmd_timer);
+
+ /* Delete all remaining commands */
+ xhci_cleanup_command_queue(xhci);
+
+ /* Clear data which is re-initilized during runtime */
+ xhci_for_each_ring_seg(xhci->interrupters[0]->event_ring->first_seg, seg)
+ memset(seg->trbs, 0, sizeof(union xhci_trb) * TRBS_PER_SEGMENT);
+
+ for (int i = xhci->max_slots; i > 0; i--)
+ xhci_free_virt_devices_depth_first(xhci, i);
+
+ xhci_rh_bw_cleanup(xhci);
+
+ xhci->cmd_ring_reserved_trbs = 0;
+ xhci_for_each_ring_seg(xhci->cmd_ring->first_seg, seg)
+ memset(seg->trbs, 0, sizeof(union xhci_trb) * TRBS_PER_SEGMENT);
- xhci_dbg(xhci, "cleaning up memory\n");
- xhci_mem_cleanup(xhci);
xhci_debugfs_exit(xhci);
- xhci_dbg(xhci, "xhci_stop completed - status = %x\n",
- readl(&xhci->op_regs->status));
- /* USB core calls the PCI reinit and start functions twice:
+ xhci_init(hcd);
+
+ /*
+ * USB core calls the PCI reinit and start functions twice:
* first with the primary HCD, and then with the secondary HCD.
* If we don't do the same, the host will never be started.
*/
- retval = xhci_mem_init(xhci, GFP_KERNEL);
- if (retval)
- return retval;
-
- xhci_init(hcd);
-
xhci_dbg(xhci, "Start the primary HCD\n");
retval = xhci_run(hcd);
if (!retval && xhci->shared_hcd) {
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 06f6da4d982f..aeecd301f207 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1793,6 +1793,7 @@ void xhci_dbg_trace(struct xhci_hcd *xhci, void (*trace)(struct va_format *),
void xhci_mem_cleanup(struct xhci_hcd *xhci);
int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags);
void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev, int slot_id);
+void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_id);
int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id, struct usb_device *udev, gfp_t flags);
int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *udev);
void xhci_copy_ep0_dequeue_into_input_ctx(struct xhci_hcd *xhci,
@@ -1804,6 +1805,7 @@ void xhci_update_tt_active_eps(struct xhci_hcd *xhci,
struct xhci_virt_device *virt_dev,
int old_active_eps);
void xhci_clear_endpoint_bw_info(struct xhci_bw_info *bw_info);
+void xhci_rh_bw_cleanup(struct xhci_hcd *xhci);
void xhci_update_bw_info(struct xhci_hcd *xhci,
struct xhci_container_ctx *in_ctx,
struct xhci_input_control_ctx *ctrl_ctx,
--
2.43.0
^ permalink raw reply related
* [PATCH 11/25] usb: xhci: improve debug messages during suspend
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Improve debug output for suspend failures, particularly when the controller
handshake does not complete. This will become important as upcoming patches
significantly rework the resume path, making more detailed suspend-side
messages valuable for debugging.
Add an explicit check of the Save/Restore Error (SRE) flag after a
successful Save State (CSS) operation. The xHCI specification
(note in section 4.23.2) states:
"After a Save or Restore State operation completes, the
Save/Restore Error (SRE) flag in USBSTS should be checked to
ensure the operation completed successfully."
Currently, the SRE error is only observed and warning is printed.
This patch does not introduce deeper error handling, as the correct
response is unclear and changes to suspend behavior may risk regressions
once the resume path is updated.
Additionally, simplify and clean up the suspend USBSTS CSS/SSS
handling code, improving readability and quirk handling for AMD
SNPS xHC controllers that occasionally do not clear the SSS bit.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.c | 65 +++++++++++++++++++++++------------------
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 9e2e2c2ed0e0..2c573aad4464 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -957,11 +957,11 @@ static bool xhci_pending_portevent(struct xhci_hcd *xhci)
*/
int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
{
- int rc = 0;
+ int err;
unsigned int delay = XHCI_MAX_HALT_USEC * 2;
struct usb_hcd *hcd = xhci_to_hcd(xhci);
u32 command;
- u32 res;
+ u32 usbsts;
if (!hcd->state)
return 0;
@@ -1007,11 +1007,10 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
/* Some chips from Fresco Logic need an extraordinary delay */
delay *= (xhci->quirks & XHCI_SLOW_SUSPEND) ? 10 : 1;
- if (xhci_handshake(&xhci->op_regs->status,
- STS_HALT, STS_HALT, delay)) {
- xhci_warn(xhci, "WARN: xHC CMD_RUN timeout\n");
- spin_unlock_irq(&xhci->lock);
- return -ETIMEDOUT;
+ err = xhci_handshake(&xhci->op_regs->status, STS_HALT, STS_HALT, delay);
+ if (err) {
+ xhci_warn(xhci, "Clearing Run/Stop bit failed %d\n", err);
+ goto handshake_error;
}
xhci_clear_command_ring(xhci);
@@ -1022,28 +1021,34 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
command = readl(&xhci->op_regs->command);
command |= CMD_CSS;
writel(command, &xhci->op_regs->command);
+
+ err = xhci_handshake(&xhci->op_regs->status, STS_SAVE, 0, 20 * USEC_PER_MSEC);
+ usbsts = readl(&xhci->op_regs->status);
xhci->broken_suspend = 0;
- if (xhci_handshake(&xhci->op_regs->status,
- STS_SAVE, 0, 20 * 1000)) {
- /*
- * AMD SNPS xHC 3.0 occasionally does not clear the
- * SSS bit of USBSTS and when driver tries to poll
- * to see if the xHC clears BIT(8) which never happens
- * and driver assumes that controller is not responding
- * and times out. To workaround this, its good to check
- * if SRE and HCE bits are not set (as per xhci
- * Section 5.4.2) and bypass the timeout.
- */
- res = readl(&xhci->op_regs->status);
- if ((xhci->quirks & XHCI_SNPS_BROKEN_SUSPEND) &&
- (((res & STS_SRE) == 0) &&
- ((res & STS_HCE) == 0))) {
- xhci->broken_suspend = 1;
- } else {
- xhci_warn(xhci, "WARN: xHC save state timeout\n");
- spin_unlock_irq(&xhci->lock);
- return -ETIMEDOUT;
+ if (err) {
+ /*
+ * AMD SNPS xHC 3.0 occasionally does not clear the
+ * SSS bit of USBSTS and when driver tries to poll
+ * to see if the xHC clears BIT(8) which never happens
+ * and driver assumes that controller is not responding
+ * and times out. To workaround this, its good to check
+ * if SRE and HCE bits are not set (as per xhci
+ * Section 5.4.2) and bypass the timeout.
+ */
+ if (!(xhci->quirks & XHCI_SNPS_BROKEN_SUSPEND)) {
+ xhci_warn(xhci, "Controller Save State failed %d\n", err);
+ goto handshake_error;
+ }
+
+ if (usbsts & (STS_SRE | STS_HCE)) {
+ xhci_warn(xhci, "Controller Save State failed, USBSTS 0x%08x\n", usbsts);
+ goto handshake_error;
}
+
+ xhci_dbg(xhci, "SNPS broken suspend, save state unreliable\n");
+ xhci->broken_suspend = 1;
+ } else if (usbsts & STS_SRE) {
+ xhci_warn(xhci, "Suspend Save Error (SRE), USBSTS 0x%08x\n", usbsts);
}
spin_unlock_irq(&xhci->lock);
@@ -1059,7 +1064,11 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
__func__);
}
- return rc;
+ return 0;
+
+handshake_error:
+ spin_unlock_irq(&xhci->lock);
+ return -ETIMEDOUT;
}
EXPORT_SYMBOL_GPL(xhci_suspend);
--
2.43.0
^ permalink raw reply related
* [PATCH 10/25] usb: xhci: split core allocation and initialization
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Separate allocation and initialization in the xHCI core:
* xhci_mem_init() now only handles memory allocation.
* xhci_init() now only handles initialization.
This split allows xhci_init() to be reused when resuming from S4
suspend-to-disk.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 3 +++
drivers/usb/host/xhci.c | 30 ++++++++++--------------------
2 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 2cd6111c9707..f1b4f06d4b8b 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2421,6 +2421,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
dma_addr_t dma;
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Starting %s", __func__);
+
/*
* xHCI section 5.4.6 - Device Context array must be
* "physically contiguous and 64-byte (cache line) aligned".
@@ -2510,6 +2512,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
if (xhci_setup_port_arrays(xhci, flags))
goto fail;
+ xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Finished %s", __func__);
return 0;
fail:
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 674bd40e4e2d..9e2e2c2ed0e0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -536,24 +536,13 @@ static void xhci_set_dev_notifications(struct xhci_hcd *xhci)
writel(dev_notf, &xhci->op_regs->dev_notification);
}
-/*
- * Initialize memory for HCD and xHC (one-time init).
- *
- * Program the PAGESIZE register, initialize the device context array, create
- * device contexts (?), set up a command ring segment (or two?), create event
- * ring (one for now).
- */
-static int xhci_init(struct usb_hcd *hcd)
+/* Setup basic xHCI registers */
+static void xhci_init(struct usb_hcd *hcd)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
- int retval;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Starting %s", __func__);
- retval = xhci_mem_init(xhci, GFP_KERNEL);
- if (retval)
- return retval;
-
/* Set the Number of Device Slots Enabled to the maximum supported value */
xhci_enable_max_dev_slots(xhci);
@@ -589,7 +578,6 @@ static int xhci_init(struct usb_hcd *hcd)
}
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Finished %s", __func__);
- return 0;
}
/*-------------------------------------------------------------------------*/
@@ -1190,11 +1178,12 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
* first with the primary HCD, and then with the secondary HCD.
* If we don't do the same, the host will never be started.
*/
- xhci_dbg(xhci, "Initialize the xhci_hcd\n");
- retval = xhci_init(hcd);
+ retval = xhci_mem_init(xhci, GFP_KERNEL);
if (retval)
return retval;
+ xhci_init(hcd);
+
xhci_dbg(xhci, "Start the primary HCD\n");
retval = xhci_run(hcd);
if (!retval && xhci->shared_hcd) {
@@ -5533,12 +5522,13 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
memset(xhci->devs, 0, MAX_HC_SLOTS * sizeof(*xhci->devs));
- xhci_dbg(xhci, "Calling HCD init\n");
- /* Initialize HCD and host controller data structures. */
- retval = xhci_init(hcd);
+ /* Allocate xHCI data structures */
+ retval = xhci_mem_init(xhci, GFP_KERNEL);
if (retval)
return retval;
- xhci_dbg(xhci, "Called HCD init\n");
+
+ /* Initialize HCD and host controller data structures */
+ xhci_init(hcd);
if (xhci_hcd_is_usb3(hcd))
xhci_hcd_init_usb3_data(xhci, hcd);
--
2.43.0
^ permalink raw reply related
* [PATCH 09/25] usb: xhci: move initialization for lifetime objects
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Initialize objects that exist for the lifetime of the driver only once,
rather than repeatedly. These objects do not require re-initialization
after events such as S4 (suspend-to-disk).
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 1 -
drivers/usb/host/xhci.c | 15 ++++++++-------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index ca4463eebc49..2cd6111c9707 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2009,7 +2009,6 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
xhci->port_caps = NULL;
xhci->interrupters = NULL;
- xhci->page_size = 0;
xhci->usb2_rhub.bus_state.bus_suspended = 0;
xhci->usb3_rhub.bus_state.bus_suspended = 0;
}
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index dd495dc740c3..674bd40e4e2d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -549,13 +549,6 @@ static int xhci_init(struct usb_hcd *hcd)
int retval;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Starting %s", __func__);
- spin_lock_init(&xhci->lock);
-
- INIT_LIST_HEAD(&xhci->cmd_list);
- INIT_DELAYED_WORK(&xhci->cmd_timer, xhci_handle_command_timeout);
- init_completion(&xhci->cmd_ring_stop_completion);
- xhci_hcd_page_size(xhci);
- memset(xhci->devs, 0, MAX_HC_SLOTS * sizeof(*xhci->devs));
retval = xhci_mem_init(xhci, GFP_KERNEL);
if (retval)
@@ -5532,6 +5525,14 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
}
+ spin_lock_init(&xhci->lock);
+ INIT_LIST_HEAD(&xhci->cmd_list);
+ INIT_DELAYED_WORK(&xhci->cmd_timer, xhci_handle_command_timeout);
+ init_completion(&xhci->cmd_ring_stop_completion);
+ xhci_hcd_page_size(xhci);
+
+ memset(xhci->devs, 0, MAX_HC_SLOTS * sizeof(*xhci->devs));
+
xhci_dbg(xhci, "Calling HCD init\n");
/* Initialize HCD and host controller data structures. */
retval = xhci_init(hcd);
--
2.43.0
^ permalink raw reply related
* [PATCH 08/25] usb: xhci: move ring initialization
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Move ring initialization from xhci_ring_alloc() to xhci_ring_init().
Call xhci_ring_init() after xhci_ring_alloc(); in the future,
it can also be used to re-initialize the ring during resume.
Additionally, remove xhci_dbg_trace() from xhci_mem_init(). The command
ring's first segment DMA address is now printed during the trace call in
xhci_ring_init().
This refactoring lays also the groundwork for eventually replacing:
* xhci_dbc_ring_init()
* xhci_clear_command_ring()
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 19 ++++++++++++++-----
drivers/usb/host/xhci.c | 3 +++
drivers/usb/host/xhci.h | 1 +
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 45638ab13635..ca4463eebc49 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -129,6 +129,13 @@ static void xhci_initialize_ring_segments(struct xhci_hcd *xhci, struct xhci_rin
ring->last_seg->trbs[TRBS_PER_SEGMENT - 1].link.control |= cpu_to_le32(LINK_TOGGLE);
}
+void xhci_ring_init(struct xhci_hcd *xhci, struct xhci_ring *ring)
+{
+ xhci_initialize_ring_segments(xhci, ring);
+ xhci_initialize_ring_info(ring);
+ trace_xhci_ring_alloc(ring);
+}
+
/*
* Link the src ring segments to the dst ring.
* Set Toggle Cycle for the new ring if needed.
@@ -389,9 +396,6 @@ struct xhci_ring *xhci_ring_alloc(struct xhci_hcd *xhci, unsigned int num_segs,
if (ret)
goto fail;
- xhci_initialize_ring_segments(xhci, ring);
- xhci_initialize_ring_info(ring);
- trace_xhci_ring_alloc(ring);
return ring;
fail:
@@ -668,6 +672,8 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct xhci_hcd *xhci,
cur_ring = stream_info->stream_rings[cur_stream];
if (!cur_ring)
goto cleanup_rings;
+
+ xhci_ring_init(xhci, cur_ring);
cur_ring->stream_id = cur_stream;
cur_ring->trb_address_map = &stream_info->trb_address_map;
/* Set deq ptr, cycle bit, and stream context type */
@@ -1011,6 +1017,8 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,
if (!dev->eps[0].ring)
goto fail;
+ xhci_ring_init(xhci, dev->eps[0].ring);
+
dev->udev = udev;
/* Point to output device context in dcbaa. */
@@ -1492,6 +1500,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
virt_dev->eps[ep_index].skip = false;
ep_ring = virt_dev->eps[ep_index].new_ring;
+ xhci_ring_init(xhci, ep_ring);
/* Fill the endpoint context */
ep_ctx->ep_info = cpu_to_le32(EP_MAX_ESIT_PAYLOAD_HI(max_esit_payload) |
@@ -2370,6 +2379,8 @@ xhci_create_secondary_interrupter(struct usb_hcd *hcd, unsigned int segs,
if (!ir)
return NULL;
+ xhci_ring_init(xhci, ir->event_ring);
+
spin_lock_irq(&xhci->lock);
if (!intr_num) {
/* Find available secondary interrupter, interrupter 0 is reserved for primary */
@@ -2482,8 +2493,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
goto fail;
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Allocated command ring at %p", xhci->cmd_ring);
- xhci_dbg_trace(xhci, trace_xhci_dbg_init, "First segment DMA is 0x%pad",
- &xhci->cmd_ring->first_seg->dma);
/* Allocate and set up primary interrupter 0 with an event ring. */
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Allocating primary event ring");
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index b9fa941425c5..dd495dc740c3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -564,6 +564,8 @@ static int xhci_init(struct usb_hcd *hcd)
/* Set the Number of Device Slots Enabled to the maximum supported value */
xhci_enable_max_dev_slots(xhci);
+ /* Initialize the Command ring */
+ xhci_ring_init(xhci, xhci->cmd_ring);
/*
* Reserve one command ring TRB for disabling LPM.
* Since the USB core grabs the shared usb_bus bandwidth mutex before
@@ -583,6 +585,7 @@ static int xhci_init(struct usb_hcd *hcd)
xhci_set_dev_notifications(xhci);
/* Initialize the Primary interrupter */
+ xhci_ring_init(xhci, xhci->interrupters[0]->event_ring);
xhci_add_interrupter(xhci, 0);
xhci->interrupters[0]->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 1bef4301e2b4..06f6da4d982f 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1824,6 +1824,7 @@ void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring *ring);
int xhci_ring_expansion(struct xhci_hcd *xhci, struct xhci_ring *ring,
unsigned int num_trbs, gfp_t flags);
void xhci_initialize_ring_info(struct xhci_ring *ring);
+void xhci_ring_init(struct xhci_hcd *xhci, struct xhci_ring *ring);
void xhci_free_endpoint_ring(struct xhci_hcd *xhci,
struct xhci_virt_device *virt_dev,
unsigned int ep_index);
--
2.43.0
^ permalink raw reply related
* [PATCH 07/25] usb: xhci: move reserving command ring trb
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Move the command ring TRB reservation from xhci_mem_init() to xhci_init().
Function xhci_mem_init() is intended for memory allocation,
while xhci_init() is for initialization.
This split allows xhci_init() to be reused when resuming from S4
suspend-to-disk.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 7 -------
drivers/usb/host/xhci.c | 6 ++++++
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d4a9dbed8f16..45638ab13635 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2485,13 +2485,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "First segment DMA is 0x%pad",
&xhci->cmd_ring->first_seg->dma);
- /*
- * Reserve one command ring TRB for disabling LPM.
- * Since the USB core grabs the shared usb_bus bandwidth mutex before
- * disabling LPM, we only need to reserve one TRB for all devices.
- */
- xhci->cmd_ring_reserved_trbs++;
-
/* Allocate and set up primary interrupter 0 with an event ring. */
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Allocating primary event ring");
xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters),
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index fdd3a19b7c9c..b9fa941425c5 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -564,6 +564,12 @@ static int xhci_init(struct usb_hcd *hcd)
/* Set the Number of Device Slots Enabled to the maximum supported value */
xhci_enable_max_dev_slots(xhci);
+ /*
+ * Reserve one command ring TRB for disabling LPM.
+ * Since the USB core grabs the shared usb_bus bandwidth mutex before
+ * disabling LPM, we only need to reserve one TRB for all devices.
+ */
+ xhci->cmd_ring_reserved_trbs = 1;
/* Set the address in the Command Ring Control register */
xhci_set_cmd_ring_deq(xhci);
--
2.43.0
^ permalink raw reply related
* [PATCH 06/25] usb: xhci: factor out roothub bandwidth cleanup
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
Introduce xhci_rh_bw_cleanup() to release all bandwidth tracking
structures associated with xHCI roothub ports.
The new helper clears:
* TT bandwidth entries
* Per-interval endpoint lists
This refactors and consolidates the existing per-port cleanup logic
previously embedded in xhci_mem_cleanup(), reducing duplication and
making the teardown sequence easier to follow.
The helper will also be reused for upcoming S4 resume handling.
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-mem.c | 50 +++++++++++++++++++++----------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 75bc1eb98b76..d4a9dbed8f16 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1895,10 +1895,36 @@ void xhci_remove_secondary_interrupter(struct usb_hcd *hcd, struct xhci_interrup
}
EXPORT_SYMBOL_GPL(xhci_remove_secondary_interrupter);
+/* Cleanup roothub bandwidth data */
+static void xhci_rh_bw_cleanup(struct xhci_hcd *xhci)
+{
+ struct xhci_root_port_bw_info *rh_bw;
+ struct xhci_tt_bw_info *tt_info, *tt_next;
+ struct list_head *eps, *ep, *ep_next;
+
+ for (int i = 0; i < xhci->max_ports; i++) {
+ rh_bw = &xhci->rh_bw[i];
+
+ /* Clear and free all TT bandwidth entries */
+ list_for_each_entry_safe(tt_info, tt_next, &rh_bw->tts, tt_list) {
+ list_del(&tt_info->tt_list);
+ kfree(tt_info);
+ }
+
+ /* Clear per-interval endpoint lists */
+ for (int j = 0; j < XHCI_MAX_INTERVAL; j++) {
+ eps = &rh_bw->bw_table.interval_bw[j].endpoints;
+
+ list_for_each_safe(ep, ep_next, eps)
+ list_del_init(ep);
+ }
+ }
+}
+
void xhci_mem_cleanup(struct xhci_hcd *xhci)
{
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
- int i, j;
+ int i;
cancel_delayed_work_sync(&xhci->cmd_timer);
@@ -1917,15 +1943,6 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Freed command ring");
xhci_cleanup_command_queue(xhci);
- for (i = 0; i < xhci->max_ports && xhci->rh_bw; i++) {
- struct xhci_interval_bw_table *bwt = &xhci->rh_bw[i].bw_table;
- for (j = 0; j < XHCI_MAX_INTERVAL; j++) {
- struct list_head *ep = &bwt->interval_bw[j].endpoints;
- while (!list_empty(ep))
- list_del_init(ep->next);
- }
- }
-
for (i = xhci->max_slots; i > 0; i--)
xhci_free_virt_devices_depth_first(xhci, i);
@@ -1959,18 +1976,9 @@ void xhci_mem_cleanup(struct xhci_hcd *xhci)
scratchpad_free(xhci);
- if (!xhci->rh_bw)
- goto no_bw;
-
- for (i = 0; i < xhci->max_ports; i++) {
- struct xhci_tt_bw_info *tt, *n;
- list_for_each_entry_safe(tt, n, &xhci->rh_bw[i].tts, tt_list) {
- list_del(&tt->tt_list);
- kfree(tt);
- }
- }
+ if (xhci->rh_bw)
+ xhci_rh_bw_cleanup(xhci);
-no_bw:
xhci->cmd_ring_reserved_trbs = 0;
xhci->usb2_rhub.num_ports = 0;
xhci->usb3_rhub.num_ports = 0;
--
2.43.0
^ permalink raw reply related
* [PATCH 05/25] usb: xhci: relocate Restore/Controller error check
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Andy Shevchenko, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
A Restore Error or Host Controller Error indicates that the host controller
failed to resume after suspend. In such cases, the xhci driver is fully
re-initialized, similar to a post-hibernation scenario.
The existing error check is only relevant when 'power_lost' is false.
If 'power_lost' is true, a Restore or Controller error has no effect:
no warning is printed and the 'power_lost' state remains unchanged.
Move the entire error check into the if '!power_lost' condition
to make this dependency explicit and simplify the resume logic.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8e0b6a673868..fdd3a19b7c9c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1140,16 +1140,13 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
spin_unlock_irq(&xhci->lock);
return -ETIMEDOUT;
}
- }
-
- temp = readl(&xhci->op_regs->status);
- /* re-initialize the HC on Restore Error, or Host Controller Error */
- if ((temp & (STS_SRE | STS_HCE)) &&
- !(xhci->xhc_state & XHCI_STATE_REMOVING)) {
- if (!power_lost)
+ /* re-initialize the HC on Restore Error, or Host Controller Error */
+ temp = readl(&xhci->op_regs->status);
+ if ((temp & (STS_SRE | STS_HCE)) && !(xhci->xhc_state & XHCI_STATE_REMOVING)) {
xhci_warn(xhci, "xHC error in resume, USBSTS 0x%x, Reinit\n", temp);
- power_lost = true;
+ power_lost = true;
+ }
}
if (power_lost) {
--
2.43.0
^ permalink raw reply related
* [PATCH 04/25] usb: xhci: simplify CMRT initialization logic
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Niklas Neronin, Andy Shevchenko, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Niklas Neronin <niklas.neronin@linux.intel.com>
The function compliance_mode_recovery_timer_init() is called from
xhci_init() because the Compliance Mode Recovery Timer (CMRT) must be set
up before xhci_run() when the xhci driver is re-initialized.
To handle this case, the boolean flag 'comp_timer_running' was introduced
to track whether xhci_run() had already been called, ensuring that
xhci_resume() would not invoke compliance_mode_recovery_timer_init()
a second time.
This can be simplified by moving the 'done' label in xhci_resume() to
after the compliance_mode_recovery_timer_init() call. With this change,
the timer initialization runs only when the xhci driver has not been
re-initialized, making the 'comp_timer_running' flag unnecessary and
allowing it to be removed.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Niklas Neronin <niklas.neronin@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index eb6927779b1e..8e0b6a673868 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1084,7 +1084,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
u32 command, temp = 0;
struct usb_hcd *hcd = xhci_to_hcd(xhci);
int retval = 0;
- bool comp_timer_running = false;
bool pending_portevent = false;
bool suspended_usb3_devs = false;
@@ -1196,7 +1195,6 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
retval = xhci_init(hcd);
if (retval)
return retval;
- comp_timer_running = true;
xhci_dbg(xhci, "Start the primary HCD\n");
retval = xhci_run(hcd);
@@ -1265,16 +1263,16 @@ int xhci_resume(struct xhci_hcd *xhci, bool power_lost, bool is_auto_resume)
usb_hcd_resume_root_hub(hcd);
}
}
-done:
+
/*
* If system is subject to the Quirk, Compliance Mode Timer needs to
* be re-initialized Always after a system resume. Ports are subject
* to suffer the Compliance Mode issue again. It doesn't matter if
* ports have entered previously to U0 before system's suspension.
*/
- if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !comp_timer_running)
+ if (xhci->quirks & XHCI_COMP_MODE_QUIRK)
compliance_mode_recovery_timer_init(xhci);
-
+done:
if (xhci->quirks & XHCI_ASMEDIA_MODIFY_FLOWCONTROL)
usb_asmedia_modifyflowcontrol(to_pci_dev(hcd->self.controller));
--
2.43.0
^ permalink raw reply related
* [PATCH 03/25] usb: xhci: Fix debugfs bandwidth reporting
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Michal Pecio, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Michal Pecio <michal.pecio@gmail.com>
Replace kernel USB speed numbers with xHCI protocol IDs expected by HW.
They are numerically equal up to high speed, but instead of SuperSpeed
we were querying SuperSpeed+.
Gen1 hardware rejects such commands with TRB Error, which resulted in
zero available bandwidth being shown.
While at that, report failures properly. No attempt made at "tunneling"
all possible comp codes through errno, debugfs users may inspect the
result through event-ring/trbs.
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-debugfs.c | 10 +++++++---
drivers/usb/host/xhci.c | 9 ++++++++-
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index ade178ab34a7..d07276192256 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -700,6 +700,10 @@ static int xhci_port_bw_show(struct xhci_hcd *xhci, u8 dev_speed,
seq_printf(s, "port[%d] available bw: %d%%.\n", i,
ctx->bytes[i]);
err_out:
+ if (ret == -EIO) {
+ seq_puts(s, "Get Port Bandwidth failed\n");
+ ret = 0;
+ }
pm_runtime_put_sync(dev);
xhci_free_port_bw_ctx(xhci, ctx);
return ret;
@@ -710,7 +714,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, USB_SPEED_SUPER, s);
+ ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_SS), s);
return ret;
}
@@ -719,7 +723,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, USB_SPEED_HIGH, s);
+ ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_HS), s);
return ret;
}
@@ -728,7 +732,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, USB_SPEED_FULL, s);
+ ret = xhci_port_bw_show(xhci, DEV_PORT_SPEED(XDEV_FS), s);
return ret;
}
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ef6d8662adec..eb6927779b1e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3201,7 +3201,12 @@ void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
}
EXPORT_SYMBOL_GPL(xhci_reset_bandwidth);
-/* Get the available bandwidth of the ports under the xhci roothub */
+/*
+ * Get the available bandwidth of the ports under the xhci roothub.
+ * EIO means the command failed: command not implemented or unsupported
+ * speed (TRB Error), some ASMedia complete with Parameter Error when
+ * querying the root hub (slot_id = 0), or other error or timeout.
+ */
int xhci_get_port_bandwidth(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
u8 dev_speed)
{
@@ -3230,6 +3235,8 @@ int xhci_get_port_bandwidth(struct xhci_hcd *xhci, struct xhci_container_ctx *ct
spin_unlock_irqrestore(&xhci->lock, flags);
wait_for_completion(cmd->completion);
+ if (cmd->status != COMP_SUCCESS)
+ ret = -EIO;
err_out:
kfree(cmd->completion);
kfree(cmd);
--
2.43.0
^ permalink raw reply related
* [PATCH 02/25] usb: xhci: Simplify clearing the Event Interrupt bit
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Michal Pecio, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Michal Pecio <michal.pecio@gmail.com>
USBSTS is mostly RW1C, so to clear EINT we should write just this
one bit. Remove pointless code which ORs the bit with current value
of the register, even though the bit is already known to be set,
and writes the result back, which clears all active RW1C flags.
We used to inadvertently clear PCD and SRE in this way. PCD isn't
used by the driver and SRE is only used at resume, so clearing them
should make no difference. Don't clear them anymore.
Tested by connecting and mounting a storage device on a few HCs.
Before: xhci_irq USBSTS 0x00000018 EINT PCD -> 0x00000000
xhci_irq USBSTS 0x00000008 EINT -> 0x00000000
After: xhci_irq USBSTS 0x00000018 EINT PCD -> 0x00000010 PCD
xhci_irq USBSTS 0x00000018 EINT PCD -> 0x00000010 PCD
Some flags are RsvdZ - should be written as zero regardless of the
value read, so technically it was a bug. But no problems are known.
Signed-off-by: Michal Pecio <michal.pecio@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci-ring.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1cbefee3c4ca..3589af0e2768 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3208,10 +3208,9 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd)
/*
* Clear the op reg interrupt status first,
* so we can receive interrupts from other MSI-X interrupters.
- * Write 1 to clear the interrupt status.
+ * USBSTS bits are write 1 to clear.
*/
- status |= STS_EINT;
- writel(status, &xhci->op_regs->status);
+ writel(STS_EINT, &xhci->op_regs->status);
/* This is the handler of the primary interrupter */
xhci_handle_events(xhci, xhci->interrupters[0], false);
--
2.43.0
^ permalink raw reply related
* [PATCH 01/25] xhci: use BIT macro
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Oliver Neukum, Mathias Nyman
In-Reply-To: <20260402131342.2628648-1-mathias.nyman@linux.intel.com>
From: Oliver Neukum <oneukum@suse.com>
We have the macro. Use it.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
drivers/usb/host/xhci.h | 121 ++++++++++++++++++++--------------------
1 file changed, 61 insertions(+), 60 deletions(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2b0796f6d00e..1bef4301e2b4 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -12,6 +12,7 @@
#ifndef __LINUX_XHCI_HCD_H
#define __LINUX_XHCI_HCD_H
+#include <linux/bits.h>
#include <linux/usb.h>
#include <linux/timer.h>
#include <linux/kernel.h>
@@ -125,17 +126,17 @@ struct xhci_op_regs {
* PCI config regs). HC does NOT drive a USB reset on the downstream ports.
* The xHCI driver must reinitialize the xHC after setting this bit.
*/
-#define CMD_RESET (1 << 1)
+#define CMD_RESET BIT(1)
/* Event Interrupt Enable - a '1' allows interrupts from the host controller */
#define CMD_EIE XHCI_CMD_EIE
/* Host System Error Interrupt Enable - get out-of-band signal for HC errors */
#define CMD_HSEIE XHCI_CMD_HSEIE
/* bits 4:6 are reserved (and should be preserved on writes). */
/* light reset (port status stays unchanged) - reset completed when this is 0 */
-#define CMD_LRESET (1 << 7)
+#define CMD_LRESET BIT(7)
/* host controller save/restore state. */
-#define CMD_CSS (1 << 8)
-#define CMD_CRS (1 << 9)
+#define CMD_CSS BIT(8)
+#define CMD_CRS BIT(9)
/* Enable Wrap Event - '1' means xHC generates an event when MFINDEX wraps. */
#define CMD_EWE XHCI_CMD_EWE
/* MFINDEX power management - '1' means xHC can stop MFINDEX counter if all root
@@ -143,9 +144,9 @@ struct xhci_op_regs {
* '0' means the xHC can power it off if all ports are in the disconnect,
* disabled, or powered-off state.
*/
-#define CMD_PM_INDEX (1 << 11)
+#define CMD_PM_INDEX BIT(11)
/* bit 14 Extended TBC Enable, changes Isoc TRB fields to support larger TBC */
-#define CMD_ETE (1 << 14)
+#define CMD_ETE BIT(14)
/* bits 15:31 are reserved (and should be preserved on writes). */
#define XHCI_RESET_LONG_USEC (10 * 1000 * 1000)
@@ -155,22 +156,22 @@ struct xhci_op_regs {
/* HC not running - set to 1 when run/stop bit is cleared. */
#define STS_HALT XHCI_STS_HALT
/* serious error, e.g. PCI parity error. The HC will clear the run/stop bit. */
-#define STS_FATAL (1 << 2)
+#define STS_FATAL BIT(2)
/* event interrupt - clear this prior to clearing any IP flags in IR set*/
-#define STS_EINT (1 << 3)
+#define STS_EINT BIT(3)
/* port change detect */
-#define STS_PORT (1 << 4)
+#define STS_PORT BIT(4)
/* bits 5:7 reserved and zeroed */
/* save state status - '1' means xHC is saving state */
-#define STS_SAVE (1 << 8)
+#define STS_SAVE BIT(8)
/* restore state status - '1' means xHC is restoring state */
-#define STS_RESTORE (1 << 9)
+#define STS_RESTORE BIT(9)
/* true: save or restore error */
-#define STS_SRE (1 << 10)
+#define STS_SRE BIT(10)
/* true: Controller Not Ready to accept doorbell or op reg writes after reset */
#define STS_CNR XHCI_STS_CNR
/* true: internal Host Controller Error - SW needs to reset and reinitialize */
-#define STS_HCE (1 << 12)
+#define STS_HCE BIT(12)
/* bits 13:31 reserved and should be preserved */
/*
@@ -182,17 +183,17 @@ struct xhci_op_regs {
/* Most of the device notification types should only be used for debug.
* SW does need to pay attention to function wake notifications.
*/
-#define DEV_NOTE_FWAKE (1 << 1)
+#define DEV_NOTE_FWAKE BIT(1)
/* CRCR - Command Ring Control Register - cmd_ring bitmasks */
/* bit 0 - Cycle bit indicates the ownership of the command ring */
-#define CMD_RING_CYCLE (1 << 0)
+#define CMD_RING_CYCLE BIT(0)
/* stop ring operation after completion of the currently executing command */
-#define CMD_RING_PAUSE (1 << 1)
+#define CMD_RING_PAUSE BIT(1)
/* stop ring immediately - abort the currently executing command */
-#define CMD_RING_ABORT (1 << 2)
+#define CMD_RING_ABORT BIT(2)
/* true: command ring is running */
-#define CMD_RING_RUNNING (1 << 3)
+#define CMD_RING_RUNNING BIT(3)
/* bits 63:6 - Command Ring pointer */
#define CMD_RING_PTR_MASK GENMASK_ULL(63, 6)
@@ -200,9 +201,9 @@ struct xhci_op_regs {
/* bits 0:7 - maximum number of device slots enabled (NumSlotsEn) */
#define MAX_DEVS(p) ((p) & 0xff)
/* bit 8: U3 Entry Enabled, assert PLC when root port enters U3, xhci 1.1 */
-#define CONFIG_U3E (1 << 8)
+#define CONFIG_U3E BIT(8)
/* bit 9: Configuration Information Enable, xhci 1.1 */
-#define CONFIG_CIE (1 << 9)
+#define CONFIG_CIE BIT(9)
/* bits 10:31 - reserved and should be preserved */
/* bits 15:0 - HCD page shift bit */
@@ -235,9 +236,9 @@ struct xhci_intr_reg {
/* iman bitmasks */
/* bit 0 - Interrupt Pending (IP), whether there is an interrupt pending. Write-1-to-clear. */
-#define IMAN_IP (1 << 0)
+#define IMAN_IP BIT(0)
/* bit 1 - Interrupt Enable (IE), whether the interrupter is capable of generating an interrupt */
-#define IMAN_IE (1 << 1)
+#define IMAN_IE BIT(1)
/* imod bitmasks */
/*
@@ -267,7 +268,7 @@ struct xhci_intr_reg {
* bit 3 - Event Handler Busy (EHB), whether the event ring is scheduled to be serviced by
* a work queue (or delayed service routine)?
*/
-#define ERST_EHB (1 << 3)
+#define ERST_EHB BIT(3)
/* bits 63:4 - Event Ring Dequeue Pointer */
#define ERST_PTR_MASK GENMASK_ULL(63, 4)
@@ -356,15 +357,15 @@ struct xhci_slot_ctx {
#define GET_DEV_SPEED(n) (((n) & DEV_SPEED) >> 20)
/* bit 24 reserved */
/* Is this LS/FS device connected through a HS hub? - bit 25 */
-#define DEV_MTT (0x1 << 25)
+#define DEV_MTT BIT(25)
/* Set if the device is a hub - bit 26 */
-#define DEV_HUB (0x1 << 26)
+#define DEV_HUB BIT(26)
/* Index of the last valid endpoint context in this device context - 27:31 */
#define LAST_CTX_MASK (0x1f << 27)
#define LAST_CTX(p) ((p) << 27)
#define LAST_CTX_TO_EP_NUM(p) (((p) >> 27) - 1)
-#define SLOT_FLAG (1 << 0)
-#define EP0_FLAG (1 << 1)
+#define SLOT_FLAG BIT(0)
+#define EP0_FLAG BIT(1)
/* dev_info2 bitmasks */
/* Max Exit Latency (ms) - worst case time to wake up all links in dev path */
@@ -463,7 +464,7 @@ struct xhci_ep_ctx {
#define EP_MAXPSTREAMS(p) (((p) << 10) & EP_MAXPSTREAMS_MASK)
#define CTX_TO_EP_MAXPSTREAMS(p) (((p) & EP_MAXPSTREAMS_MASK) >> 10)
/* Endpoint is set up with a Linear Stream Array (vs. Secondary Stream Array) */
-#define EP_HAS_LSA (1 << 15)
+#define EP_HAS_LSA BIT(15)
/* hosts with LEC=1 use bits 31:24 as ESIT high bits. */
#define CTX_TO_MAX_ESIT_PAYLOAD_HI(p) (((p) >> 24) & 0xff)
@@ -498,7 +499,7 @@ struct xhci_ep_ctx {
#define CTX_TO_MAX_ESIT_PAYLOAD(p) (((p) >> 16) & 0xffff)
/* deq bitmasks */
-#define EP_CTX_CYCLE_MASK (1 << 0)
+#define EP_CTX_CYCLE_MASK BIT(0)
/* bits 63:4 - TR Dequeue Pointer */
#define TR_DEQ_PTR_MASK GENMASK_ULL(63, 4)
@@ -661,18 +662,18 @@ struct xhci_virt_ep {
struct xhci_ring *new_ring;
unsigned int err_count;
unsigned int ep_state;
-#define SET_DEQ_PENDING (1 << 0)
-#define EP_HALTED (1 << 1) /* For stall handling */
-#define EP_STOP_CMD_PENDING (1 << 2) /* For URB cancellation */
+#define SET_DEQ_PENDING BIT(0)
+#define EP_HALTED BIT(1) /* For stall handling */
+#define EP_STOP_CMD_PENDING BIT(2) /* For URB cancellation */
/* Transitioning the endpoint to using streams, don't enqueue URBs */
-#define EP_GETTING_STREAMS (1 << 3)
-#define EP_HAS_STREAMS (1 << 4)
+#define EP_GETTING_STREAMS BIT(3)
+#define EP_HAS_STREAMS BIT(4)
/* Transitioning the endpoint to not using streams, don't enqueue URBs */
-#define EP_GETTING_NO_STREAMS (1 << 5)
-#define EP_HARD_CLEAR_TOGGLE (1 << 6)
-#define EP_SOFT_CLEAR_TOGGLE (1 << 7)
+#define EP_GETTING_NO_STREAMS BIT(5)
+#define EP_HARD_CLEAR_TOGGLE BIT(6)
+#define EP_SOFT_CLEAR_TOGGLE BIT(7)
/* usb_hub_clear_tt_buffer is in progress */
-#define EP_CLEARING_TT (1 << 8)
+#define EP_CLEARING_TT BIT(8)
/* ---- Related to URB cancellation ---- */
struct list_head cancelled_td_list;
struct xhci_hcd *xhci;
@@ -954,7 +955,7 @@ struct xhci_link_trb {
};
/* control bitfields */
-#define LINK_TOGGLE (0x1<<1)
+#define LINK_TOGGLE BIT(1)
/* Command completion event TRB */
struct xhci_event_cmd {
@@ -968,13 +969,13 @@ struct xhci_event_cmd {
#define COMP_PARAM(p) ((p) & 0xffffff) /* Command Completion Parameter */
/* Address device - disable SetAddress */
-#define TRB_BSR (1<<9)
+#define TRB_BSR BIT(9)
/* Configure Endpoint - Deconfigure */
-#define TRB_DC (1<<9)
+#define TRB_DC BIT(9)
/* Stop Ring - Transfer State Preserve */
-#define TRB_TSP (1<<9)
+#define TRB_TSP BIT(9)
enum xhci_ep_reset_type {
EP_HARD_RESET,
@@ -1017,13 +1018,13 @@ enum xhci_setup_dev {
#define SCT_FOR_TRB(p) (((p) & 0x7) << 1)
/* Link TRB specific fields */
-#define TRB_TC (1<<1)
+#define TRB_TC BIT(1)
/* Port Status Change Event TRB fields */
/* Port ID - bits 31:24 */
#define GET_PORT_ID(p) (((p) & (0xff << 24)) >> 24)
-#define EVENT_DATA (1 << 2)
+#define EVENT_DATA BIT(2)
/* Normal TRB fields */
/* transfer_len bitmasks - bits 0:16 */
@@ -1038,36 +1039,36 @@ enum xhci_setup_dev {
#define GET_INTR_TARGET(p) (((p) >> 22) & 0x3ff)
/* Cycle bit - indicates TRB ownership by HC or HCD */
-#define TRB_CYCLE (1<<0)
+#define TRB_CYCLE BIT(0)
/*
* Force next event data TRB to be evaluated before task switch.
* Used to pass OS data back after a TD completes.
*/
-#define TRB_ENT (1<<1)
+#define TRB_ENT BIT(1)
/* Interrupt on short packet */
-#define TRB_ISP (1<<2)
+#define TRB_ISP BIT(2)
/* Set PCIe no snoop attribute */
-#define TRB_NO_SNOOP (1<<3)
+#define TRB_NO_SNOOP BIT(3)
/* Chain multiple TRBs into a TD */
-#define TRB_CHAIN (1<<4)
+#define TRB_CHAIN BIT(4)
/* Interrupt on completion */
-#define TRB_IOC (1<<5)
+#define TRB_IOC BIT(5)
/* The buffer pointer contains immediate data */
-#define TRB_IDT (1<<6)
+#define TRB_IDT BIT(6)
/* TDs smaller than this might use IDT */
#define TRB_IDT_MAX_SIZE 8
/* Block Event Interrupt */
-#define TRB_BEI (1<<9)
+#define TRB_BEI BIT(9)
/* Control transfer TRB specific fields */
-#define TRB_DIR_IN (1<<16)
+#define TRB_DIR_IN BIT(16)
#define TRB_TX_TYPE(p) ((p) << 16)
#define TRB_DATA_OUT 2
#define TRB_DATA_IN 3
/* Isochronous TRB specific fields */
-#define TRB_SIA (1<<31)
+#define TRB_SIA BIT(31)
#define TRB_FRAME_ID(p) (((p) & 0x7ff) << 20)
#define GET_FRAME_ID(p) (((p) >> 20) & 0x7ff)
/* Total burst count field, Rsvdz on xhci 1.1 with Extended TBC enabled (ETE) */
@@ -1535,9 +1536,9 @@ struct xhci_hcd {
struct xhci_interrupter **interrupters;
struct xhci_ring *cmd_ring;
unsigned int cmd_ring_state;
-#define CMD_RING_STATE_RUNNING (1 << 0)
-#define CMD_RING_STATE_ABORTED (1 << 1)
-#define CMD_RING_STATE_STOPPED (1 << 2)
+#define CMD_RING_STATE_RUNNING BIT(0)
+#define CMD_RING_STATE_ABORTED BIT(1)
+#define CMD_RING_STATE_STOPPED BIT(2)
struct list_head cmd_list;
unsigned int cmd_ring_reserved_trbs;
struct delayed_work cmd_timer;
@@ -1578,9 +1579,9 @@ struct xhci_hcd {
*
* There are no reports of xHCI host controllers that display this issue.
*/
-#define XHCI_STATE_DYING (1 << 0)
-#define XHCI_STATE_HALTED (1 << 1)
-#define XHCI_STATE_REMOVING (1 << 2)
+#define XHCI_STATE_DYING BIT(0)
+#define XHCI_STATE_HALTED BIT(1)
+#define XHCI_STATE_REMOVING BIT(2)
unsigned long long quirks;
#define XHCI_LINK_TRB_QUIRK BIT_ULL(0)
#define XHCI_RESET_EP_QUIRK BIT_ULL(1) /* Deprecated */
--
2.43.0
^ permalink raw reply related
* [PATCH 00/25] xhci features for usb-next
From: Mathias Nyman @ 2026-04-02 13:13 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Mathias Nyman
Hi Greg
A couple small fixes and a lot of xhci refactoring, decoupling and cleanups
for usb-next.
Thanks
Mathias
Michal Pecio (3):
usb: xhci: Simplify clearing the Event Interrupt bit
usb: xhci: Fix debugfs bandwidth reporting
usb: xhci: Make usb_host_endpoint.hcpriv survive endpoint_disable()
Niklas Neronin (21):
usb: xhci: simplify CMRT initialization logic
usb: xhci: relocate Restore/Controller error check
usb: xhci: factor out roothub bandwidth cleanup
usb: xhci: move reserving command ring trb
usb: xhci: move ring initialization
usb: xhci: move initialization for lifetime objects
usb: xhci: split core allocation and initialization
usb: xhci: improve debug messages during suspend
usb: xhci: optimize resuming from S4 (suspend-to-disk)
usb: xhci: stop treating 'wIndex' as a mutable port number
usb: xhci: rename 'wIndex' parameters to 'portnum'
usb: xhci: clean up handling of upper bits in SetPortFeature wIndex
usb: xhci: clean up 'wValue' handling in xhci_hub_control()
usb: xhci: separate use of USB Chapter 11 PLS macros from
xHCI-specific PLS macros
usb: xhci: add PORTPMSC variable to xhci_hub_control()
usb: xhci: add PORTSC variable to xhci_hub_control()
usb: xhci: rename parameter to match argument 'portsc'
usb: xhci: cleanup xhci_hub_report_usb3_link_state()
usb: xhci: simpilfy resume root hub code
usb: xhci: move roothub port limit validation
usb: xhci: remove duplicate '0x' prefix
Oliver Neukum (1):
xhci: use BIT macro
drivers/usb/host/xhci-debugfs.c | 10 +-
drivers/usb/host/xhci-hub.c | 381 +++++++++++++++-----------------
drivers/usb/host/xhci-mem.c | 133 +++++------
drivers/usb/host/xhci-ring.c | 9 +-
drivers/usb/host/xhci.c | 229 ++++++++++---------
drivers/usb/host/xhci.h | 124 ++++++-----
include/linux/usb.h | 3 +-
7 files changed, 444 insertions(+), 445 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v4] usb: gadget: uvc: fix NULL pointer dereference during unbind race
From: Roshan Binu Paul @ 2026-04-02 12:29 UTC (permalink / raw)
To: hhhuuu@google.com
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi Jimmy,
I have tested this patch on a Xilinx ZynqMP platform running a custom
UVC video streaming application.
Test Environment
----------------
Platform : Xilinx ZynqMP
Kernel : 6.6.40-xilinx-g7c7280e64b31
Application : Custom UVC video streaming application
Base : PetaLinux 2024.2
Verification
------------
Confirmed patch is applied by checking for uvc_vdev_release symbol
in /proc/kallsyms.
Test Methodology
----------------
1. Start UVC video streaming application
2. Send SIGKILL to force terminate the application
3. Wait for application auto-restart
4. Check kernel messages for NULL pointer dereference or panic
5. Repeat for 10 cycles
Test Results
------------
SIGKILL stress test : 10/10 cycles (3 Reps) PASSED
NULL pointer crashes: None observed
Kernel panics : None observed
System stability : Stable throughout testing
Prior to this patch, we observed NULL pointer dereferences in
usb_gadget_deactivate() when applications were force-killed during
active streaming sessions. This patch resolves that issue.
Tested-by: Roshan Binu Paul <roshan.binu@dicortech.com>
Thanks for the fix.
Best regards,
Roshan Binu Paul
DicorTech
^ permalink raw reply
* Re: [PATCH] usb: typec: altmode: Fix altmode to handle multiple parners
From: François Scala @ 2026-04-02 12:27 UTC (permalink / raw)
To: Greg KH; +Cc: linux-usb, Heikki Krogerus
In-Reply-To: <2026040254-exposure-stubbly-7239@gregkh>
On 02/04/2026 14.18, Greg KH wrote:
> And what commit id does this fix?
The altmode struct was added in commit
8a37d87d72f0c69f837229c04d2fcd7117ea57e7
^ permalink raw reply
* Re: [PATCH] usb: typec: altmode: Fix altmode to handle multiple parners
From: Greg KH @ 2026-04-02 12:18 UTC (permalink / raw)
To: François Scala; +Cc: linux-usb, Heikki Krogerus
In-Reply-To: <20260402120433.22967-1-francois@scala.name>
On Thu, Apr 02, 2026 at 02:04:32PM +0200, François Scala wrote:
> From: François Scala <francois@scala.name>
>
> The partner field in the altmode struct is a single pointer and the sysfs
> symlink uses a fixed name. This cause an error when a second partner is
> registered.
>
> sysfs: cannot create duplicate filename '/devices/platform/USBC000:00/typec/port0/port0.0/partner'
>
> The field is replaced by an array of pointers and the symlink use the
> device name to avoid conflict.
>
> /sys/devices/platform/USBC000:00/typec/port0/port0.0/
> lrwxrwxrwx 1 root root 0 Apr 2 09:12 port0-partner.0 -> ../port0-partner/port0-partner.0
> lrwxrwxrwx 1 root root 0 Apr 2 09:12 port0-partner.1 -> ../port0-partner/port0-partner.1
>
> Signed-off-by: François Scala <francois@scala.name>
>
> Thanks
> François
No need for the "Thanks" here :)
And what commit id does this fix?
thanks,
greg k-h
^ permalink raw reply
* [PATCH] usb: typec: altmode: Fix altmode to handle multiple parners
From: François Scala @ 2026-04-02 12:04 UTC (permalink / raw)
To: linux-usb; +Cc: Heikki Krogerus, François Scala
From: François Scala <francois@scala.name>
The partner field in the altmode struct is a single pointer and the sysfs
symlink uses a fixed name. This cause an error when a second partner is
registered.
sysfs: cannot create duplicate filename '/devices/platform/USBC000:00/typec/port0/port0.0/partner'
The field is replaced by an array of pointers and the symlink use the
device name to avoid conflict.
/sys/devices/platform/USBC000:00/typec/port0/port0.0/
lrwxrwxrwx 1 root root 0 Apr 2 09:12 port0-partner.0 -> ../port0-partner/port0-partner.0
lrwxrwxrwx 1 root root 0 Apr 2 09:12 port0-partner.1 -> ../port0-partner/port0-partner.1
Signed-off-by: François Scala <francois@scala.name>
Thanks
François
---
drivers/usb/typec/bus.c | 133 ++++++++++++++++++++++++++++++++------
drivers/usb/typec/bus.h | 8 ++-
drivers/usb/typec/class.c | 99 +++++++++++++++++++++++++---
3 files changed, 209 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index e84b134a3381..e09ceb844342 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -7,6 +7,8 @@
*/
#include <linux/usb/pd_vdo.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
#include "bus.h"
#include "class.h"
@@ -62,7 +64,7 @@ static int typec_altmode_set_state(struct typec_altmode *adev,
bool is_port = is_typec_port(adev->dev.parent);
struct altmode *port_altmode;
- port_altmode = is_port ? to_altmode(adev) : to_altmode(adev)->partner;
+ port_altmode = is_port ? to_altmode(adev) : to_altmode(adev)->partners[0];
return typec_altmode_set_switches(port_altmode, conf, data);
}
@@ -89,6 +91,7 @@ int typec_altmode_notify(struct typec_altmode *adev,
bool is_port;
struct altmode *altmode;
struct altmode *partner;
+ int partner_idx;
int ret;
if (!adev)
@@ -96,11 +99,17 @@ int typec_altmode_notify(struct typec_altmode *adev,
altmode = to_altmode(adev);
- if (!altmode->partner)
+ partner_idx = typec_altmode_get_partner_idx_by_name(altmode, dev_name(&adev->dev));
+ if (partner_idx == -1) {
+ dev_info(&adev->dev, "%s adev is not in the partners array\n", __func__);
+ return -ENODEV;
+ }
+
+ if (!altmode->partners[partner_idx])
return -ENODEV;
is_port = is_typec_port(adev->dev.parent);
- partner = altmode->partner;
+ partner = altmode->partners[partner_idx];
ret = typec_altmode_set_switches(is_port ? altmode : partner, conf, data);
if (ret)
@@ -125,13 +134,18 @@ EXPORT_SYMBOL_GPL(typec_altmode_notify);
*/
int typec_altmode_enter(struct typec_altmode *adev, u32 *vdo)
{
- struct altmode *partner = to_altmode(adev)->partner;
- struct typec_altmode *pdev = &partner->adev;
+ struct altmode *partner;
+ struct typec_altmode *pdev;
int ret;
+ partner = to_altmode(adev)->partners[0];
+ pdev = &partner->adev;
+
if (!adev || adev->active)
return 0;
+ typec_altmode_dump("typec_altmode_enter partner", partner);
+
if (!pdev->ops || !pdev->ops->enter)
return -EOPNOTSUPP;
@@ -156,13 +170,15 @@ EXPORT_SYMBOL_GPL(typec_altmode_enter);
*/
int typec_altmode_exit(struct typec_altmode *adev)
{
- struct altmode *partner = to_altmode(adev)->partner;
+ struct altmode *partner = to_altmode(adev)->partners[0];
struct typec_altmode *pdev = &partner->adev;
int ret;
if (!adev || !adev->active)
return 0;
+ typec_altmode_dump("typec_altmode_exit partner", partner);
+
if (!pdev->ops || !pdev->ops->exit)
return -EOPNOTSUPP;
@@ -185,7 +201,7 @@ EXPORT_SYMBOL_GPL(typec_altmode_exit);
*/
int typec_altmode_attention(struct typec_altmode *adev, u32 vdo)
{
- struct altmode *partner = to_altmode(adev)->partner;
+ struct altmode *partner = to_altmode(adev)->partners[0];
struct typec_altmode *pdev;
if (!partner)
@@ -220,12 +236,15 @@ int typec_altmode_vdm(struct typec_altmode *adev,
if (!adev)
return 0;
+
altmode = to_altmode(adev);
- if (!altmode->partner)
+ typec_altmode_dump("typec_altmode_vdm altmode", altmode);
+
+ if (!altmode->partners[0])
return -ENODEV;
- pdev = &altmode->partner->adev;
+ pdev = &altmode->partners[0]->adev;
if (!pdev->ops || !pdev->ops->vdm)
return -EOPNOTSUPP;
@@ -237,10 +256,10 @@ EXPORT_SYMBOL_GPL(typec_altmode_vdm);
const struct typec_altmode *
typec_altmode_get_partner(struct typec_altmode *adev)
{
- if (!adev || !to_altmode(adev)->partner)
+ if (!adev || !to_altmode(adev)->partners[0])
return NULL;
- return &to_altmode(adev)->partner->adev;
+ return &to_altmode(adev)->partners[0]->adev;
}
EXPORT_SYMBOL_GPL(typec_altmode_get_partner);
@@ -258,7 +277,7 @@ EXPORT_SYMBOL_GPL(typec_altmode_get_partner);
*/
int typec_cable_altmode_enter(struct typec_altmode *adev, enum typec_plug_index sop, u32 *vdo)
{
- struct altmode *partner = to_altmode(adev)->partner;
+ struct altmode *partner = to_altmode(adev)->partners[0];
struct typec_altmode *pdev;
if (!adev || adev->active)
@@ -288,7 +307,7 @@ EXPORT_SYMBOL_GPL(typec_cable_altmode_enter);
*/
int typec_cable_altmode_exit(struct typec_altmode *adev, enum typec_plug_index sop)
{
- struct altmode *partner = to_altmode(adev)->partner;
+ struct altmode *partner = to_altmode(adev)->partners[0];
struct typec_altmode *pdev;
if (!adev || !adev->active)
@@ -330,9 +349,9 @@ int typec_cable_altmode_vdm(struct typec_altmode *adev, enum typec_plug_index so
altmode = to_altmode(adev);
if (is_typec_plug(adev->dev.parent)) {
- if (!altmode->partner)
+ if (!altmode->partners[0])
return -ENODEV;
- pdev = &altmode->partner->adev;
+ pdev = &altmode->partners[0]->adev;
} else {
if (!altmode->plug[sop])
return -ENODEV;
@@ -360,7 +379,7 @@ EXPORT_SYMBOL_GPL(typec_cable_altmode_vdm);
struct typec_altmode *typec_altmode_get_plug(struct typec_altmode *adev,
enum typec_plug_index index)
{
- struct altmode *port = to_altmode(adev)->partner;
+ struct altmode *port = to_altmode(adev)->partners[0];
if (port->plug[index]) {
get_device(&port->plug[index]->adev.dev);
@@ -494,17 +513,58 @@ static int typec_uevent(const struct device *dev, struct kobj_uevent_env *env)
return add_uevent_var(env, "MODALIAS=typec:id%04X", altmode->svid);
}
+void typec_altmode_dump(char *name, struct altmode *alt)
+{
+ if (alt) {
+ for (int i = 0; i < TYPEC_ALTMODE_MAX_PARTNERS; i++) {
+ if (alt->partners[i]) {
+ dev_info(&alt->adev.dev, "%s[%p]->partners[%d] = %p / adev name = %s",
+ name,
+ alt,
+ i,
+ alt->partners[i],
+ dev_name(&alt->partners[i]->adev.dev)
+ );
+ }
+ }
+ for (int i = 0; i < 2; i++) {
+ if (alt->plug[i]) {
+ dev_info(&alt->adev.dev, "%s[%p]->plug[%d] = %p / adev name = %s",
+ name,
+ alt,
+ i,
+ alt->plug[i],
+ dev_name(&alt->plug[i]->adev.dev)
+ );
+ }
+ }
+ } else {
+ dev_info(&alt->adev.dev, "alt == NULL !");
+ }
+}
+
static int typec_altmode_create_links(struct altmode *alt)
{
- struct device *port_dev = &alt->partner->adev.dev;
+ struct device *port_dev = &alt->partners[0]->adev.dev;
struct device *dev = &alt->adev.dev;
int err;
+ dev_info(&alt->adev.dev, "%s \"port\" -> %s\n",
+ __func__,
+ dev_name(port_dev)
+ );
+
err = sysfs_create_link(&dev->kobj, &port_dev->kobj, "port");
if (err)
return err;
- err = sysfs_create_link(&port_dev->kobj, &dev->kobj, "partner");
+ dev_info(port_dev, "%s \"%s\" -> %s\n",
+ __func__,
+ dev_name(port_dev),
+ dev_name(dev)
+ );
+
+ err = sysfs_create_link(&port_dev->kobj, &dev->kobj, dev_name(dev));
if (err)
sysfs_remove_link(&dev->kobj, "port");
@@ -513,7 +573,17 @@ static int typec_altmode_create_links(struct altmode *alt)
static void typec_altmode_remove_links(struct altmode *alt)
{
- sysfs_remove_link(&alt->partner->adev.dev.kobj, "partner");
+ struct altmode *partner = alt->partners[0];
+
+ dev_info(&partner->adev.dev, "%s \"%s\"\n",
+ __func__,
+ dev_name(&alt->adev.dev)
+ );
+
+ sysfs_remove_link(&partner->adev.dev.kobj, dev_name(&alt->adev.dev));
+
+ dev_info(&alt->adev.dev, "%s \"port\"\n", __func__);
+
sysfs_remove_link(&alt->adev.dev.kobj, "port");
}
@@ -525,7 +595,7 @@ static int typec_probe(struct device *dev)
int ret;
/* Fail if the port does not support the alternate mode */
- if (!altmode->partner)
+ if (!altmode->partners[0])
return -ENODEV;
ret = typec_altmode_create_links(altmode);
@@ -561,6 +631,29 @@ static void typec_remove(struct device *dev)
adev->ops = NULL;
}
+int typec_altmode_get_partner_idx_by_name(struct altmode *altmode, const char *name)
+{
+ if (!altmode)
+ return -1;
+
+ for (int i = 0; i < TYPEC_ALTMODE_MAX_PARTNERS; i++) {
+ if (!altmode->partners[i])
+ continue;
+ if (!strcmp(name, dev_name(&altmode->partners[i]->adev.dev)))
+ return i;
+ }
+ return -1;
+}
+
+bool typec_altmode_partners_is_empty(struct altmode *altmode)
+{
+ for (int i = 0; i < TYPEC_ALTMODE_MAX_PARTNERS; i++) {
+ if (altmode->partners[i])
+ return false;
+ }
+ return true;
+}
+
const struct bus_type typec_bus = {
.name = "typec",
.dev_groups = typec_groups,
diff --git a/drivers/usb/typec/bus.h b/drivers/usb/typec/bus.h
index 7df5deb1dd3a..3db1d407c322 100644
--- a/drivers/usb/typec/bus.h
+++ b/drivers/usb/typec/bus.h
@@ -8,6 +8,8 @@
struct typec_mux;
struct typec_retimer;
+#define TYPEC_ALTMODE_MAX_PARTNERS 2
+
struct altmode {
unsigned int id;
struct typec_altmode adev;
@@ -21,10 +23,14 @@ struct altmode {
struct attribute_group group;
const struct attribute_group *groups[2];
- struct altmode *partner;
+ struct altmode *partners[TYPEC_ALTMODE_MAX_PARTNERS];
struct altmode *plug[2];
};
#define to_altmode(d) container_of(d, struct altmode, adev)
+void typec_altmode_dump(char *name, struct altmode *alt);
+int typec_altmode_get_partner_idx_by_name(struct altmode *altmode, const char *name);
+bool typec_altmode_partners_is_empty(struct altmode *altmode);
+
#endif /* __USB_TYPEC_ALTMODE_H__ */
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 831430909471..0ad9d7488bb0 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -241,6 +241,15 @@ static int altmode_match(struct device *dev, const void *data)
return (adev->svid == id->svid);
}
+static int typec_altmode_partners_get_free_slot(struct altmode *altmode)
+{
+ for (int i = 0; i < TYPEC_ALTMODE_MAX_PARTNERS; i++) {
+ if (altmode->partners[i] == NULL)
+ return i;
+ }
+ return -1;
+}
+
static void typec_altmode_set_partner(struct altmode *altmode)
{
struct typec_altmode *adev = &altmode->adev;
@@ -253,9 +262,18 @@ static void typec_altmode_set_partner(struct altmode *altmode)
if (!dev)
return;
+ dev_info(&altmode->adev.dev, "%s dev %s\n",
+ __func__,
+ dev_name(dev)
+ );
+
+ typec_altmode_dump("altmode before set_partner", altmode);
+
/* Bind the port alt mode to the partner/plug alt mode. */
partner = to_altmode(to_typec_altmode(dev));
- altmode->partner = partner;
+ typec_altmode_dump("partner before set_partner", partner);
+
+ altmode->partners[0] = partner;
/* Bind the partner/plug alt mode to the port alt mode. */
if (is_typec_plug(adev->dev.parent)) {
@@ -263,18 +281,51 @@ static void typec_altmode_set_partner(struct altmode *altmode)
partner->plug[plug->index] = altmode;
} else {
- partner->partner = altmode;
+ int free_spot = typec_altmode_partners_get_free_slot(partner);
+
+ if (free_spot == -1) {
+ dev_info(&altmode->adev.dev, "typec altmode, no free slot in partners table");
+ return;
+ }
+ partner->partners[free_spot] = altmode;
}
+
+ typec_altmode_dump("altmode after set_partner", altmode);
+ typec_altmode_dump("partner after set_partner", partner);
+
}
static void typec_altmode_put_partner(struct altmode *altmode)
{
- struct altmode *partner = altmode->partner;
+ struct altmode *partner = altmode->partners[0];
struct typec_altmode *adev;
struct typec_altmode *partner_adev;
- if (!partner)
+ if (!partner) {
+ dev_info(&altmode->adev.dev, "%s partner is already NULL\n", __func__);
+ return;
+ }
+
+ typec_altmode_dump("altmode before put_partner", altmode);
+ typec_altmode_dump("partner before put_partner", partner);
+
+ int partner_idx = typec_altmode_get_partner_idx_by_name(
+ partner,
+ dev_name(&altmode->adev.dev)
+ );
+
+ dev_info(&altmode->adev.dev, "%s partner_idx = %d\n", __func__, partner_idx);
+
+ if (partner_idx == -1) {
+ dev_info(&altmode->adev.dev, "partner_idx altmod not found in partners list");
return;
+ }
+
+ dev_info(&altmode->adev.dev, "%s altmode->adev = %p partner->adev = %p\n",
+ __func__,
+ &altmode->adev,
+ &partner->adev
+ );
adev = &altmode->adev;
partner_adev = &partner->adev;
@@ -284,9 +335,22 @@ static void typec_altmode_put_partner(struct altmode *altmode)
partner->plug[plug->index] = NULL;
} else {
- partner->partner = NULL;
+ partner->partners[partner_idx] = NULL;
}
- put_device(&partner_adev->dev);
+
+ typec_altmode_dump("partner after set to NULL", partner);
+
+ if (typec_altmode_partners_is_empty(partner)) {
+ dev_info(&altmode->adev.dev, "%s partner->partners is empty -> put_device(...)\n",
+ __func__
+ );
+ put_device(&partner_adev->dev);
+ } else {
+ dev_info(&altmode->adev.dev, "%s partner->partners is not empty -> keeping it\n",
+ __func__
+ );
+ }
+
}
/**
@@ -370,9 +434,17 @@ static ssize_t active_store(struct device *dev, struct device_attribute *attr,
{
struct typec_altmode *adev = to_typec_altmode(dev);
struct altmode *altmode = to_altmode(adev);
+ int partner_idx = typec_altmode_get_partner_idx_by_name(altmode, dev_name(dev));
bool enter;
int ret;
+ dev_info(dev, "%s buf %s altmode %s partner_idx %d\n",
+ __func__,
+ buf,
+ dev_name(&altmode->adev.dev),
+ partner_idx
+ );
+
ret = kstrtobool(buf, &enter);
if (ret)
return ret;
@@ -380,14 +452,21 @@ static ssize_t active_store(struct device *dev, struct device_attribute *attr,
if (adev->active == enter)
return size;
+ if (partner_idx == -1) {
+ dev_warn(dev, "dev is not in the altmode partners array");
+ return 0;
+ }
+
if (is_typec_port(adev->dev.parent)) {
typec_altmode_update_active(adev, enter);
/* Make sure that the partner exits the mode before disabling */
- if (altmode->partner && !enter && altmode->partner->adev.active)
- typec_altmode_exit(&altmode->partner->adev);
- } else if (altmode->partner) {
- if (enter && !altmode->partner->adev.active) {
+ if (altmode->partners[partner_idx]
+ && !enter
+ && altmode->partners[partner_idx]->adev.active)
+ typec_altmode_exit(&altmode->partners[partner_idx]->adev);
+ } else if (altmode->partners[partner_idx]) {
+ if (enter && !altmode->partners[partner_idx]->adev.active) {
dev_warn(dev, "port has the mode disabled\n");
return -EPERM;
}
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] usb: core: Fix bandwidth for devices with invalid wBytesPerInterval
From: Xuetao (kirin) @ 2026-04-02 11:55 UTC (permalink / raw)
To: Michal Pecio; +Cc: gregkh, linux-usb, linux-kernel, caiyadong, stable
In-Reply-To: <20260402114421.738e375a.michal.pecio@gmail.com>
在 2026/4/2 17:44, Michal Pecio 写道:
> On Thu, 2 Apr 2026 10:14:00 +0800, Tao Xue wrote:
>> As specified in Section 4.14.2 of the xHCI Specification, the xHC
>> reserves bandwidth for periodic endpoints according to bInterval and
>> wBytesPerInterval (Max ESIT Payload).
>
> For SuperSpeed endpoints, yes.
> This follows from USB3 spec 9.6.7.
>
>> Some peripherals report an invalid wBytesPerInterval in their device
>> descriptor, which is either 0 or smaller than the actual data length
>> transmitted. This issue is observed on ASIX AX88179 series USB 3.0
>> Ethernet adapters.
>
> Damn, it really does.
>
> Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x81 EP 1 IN
> bmAttributes 3
> Transfer Type Interrupt
> Synch Type None
> Usage Type Data
> wMaxPacketSize 0x0008 1x 8 bytes
> bInterval 11
> bMaxBurst 0
> wBytesPerInterval 0
>
> Any other examples besides AX88179?
We tested 18 different USB 3.0 docks, but they only contained two
different types of USB 3.0 Ethernet devices based on VID/PID.
The dwc3 controller works fine with all of the devices mentioned above.
The other USB host controller works fine with all 12 Realtek devices,
but fails with all 6 ASIX devices.
1. Realtek USB 10/100/1000 LAN (12 devices)
All 12 devices share the same VID/PID and descriptor values.
VID = 0x0BDA, PID = 0x8153
wMaxPacketSize = 0x10
bMaxBurst = 0
wBytesPerInterval = 0x8
2. ASIX AX88179 USB 3.0 to Gigabit Ethernet Adapter (6 devices)
All 6 devices share the same VID/PID.
VID = 0x0B95, PID = 0x1790
(a) 4 devices:
wMaxPacketSize = 0x10
bMaxBurst = 0
wBytesPerInterval = 0x0
(b) 2 devices:
wMaxPacketSize = 0x10
bMaxBurst = 0
wBytesPerInterval = 0x8
We also tested 3 different USB 3.0 camera devices. On all of these
devices, the wBytesPerInterval of all SuperSpeed isochronous endpoints
is equal to Mult * wMaxPacketSize * (bMaxBurst + 1).
>
>> These errors may lead to unexpected behavior on certain USB host
>> controllers, causing USB peripherals to malfunction.
>
> Out of curiosity, Bandwidth Overrun Error or something worse?
>
This host controller reports a Babble error.
> It's an oversight that these URBs aren't rejected with EMSGSIZE in the
> first place. IIRC zero-length interrupt transfers are allowed by USB
> specs and a zero-payload endpoint is probably legal per xHCI, but then
> submitting non-empty URBs to it is not.
>
>> To address the issue, return max(wBytesPerInterval, max_payload) when
>> calculating bandwidth reservation.
>>
>> Fixes: 9238f25d5d32 ("USB: xhci: properly set endpoint context fields for periodic eps.")
>> Cc: <stable@kernel.org>
>> Signed-off-by: Tao Xue <xuetao09@huawei.com>
>> ---
>> drivers/usb/core/usb.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> index e9a10a33534c..8f2e05a5a015 100644
>> --- a/drivers/usb/core/usb.c
>> +++ b/drivers/usb/core/usb.c
>> @@ -1125,6 +1125,8 @@ EXPORT_SYMBOL_GPL(usb_free_noncoherent);
>> u32 usb_endpoint_max_periodic_payload(struct usb_device *udev,
>> const struct usb_host_endpoint *ep)
>> {
>> + u32 max_payload;
>> +
>> if (!usb_endpoint_xfer_isoc(&ep->desc) &&
>> !usb_endpoint_xfer_int(&ep->desc))
>> return 0;
>> @@ -1135,7 +1137,12 @@ u32 usb_endpoint_max_periodic_payload(struct usb_device *udev,
>> return le32_to_cpu(ep->ssp_isoc_ep_comp.dwBytesPerInterval);
>> fallthrough;
>> case USB_SPEED_SUPER:
>> - return le16_to_cpu(ep->ss_ep_comp.wBytesPerInterval);
>> + max_payload = usb_endpoint_maxp(&ep->desc) * (ep->ss_ep_comp.bMaxBurst + 1);
>> + if (usb_endpoint_xfer_isoc(&ep->desc))
>> + return max_t(u32, max_payload * USB_SS_MULT(ep->ss_ep_comp.bmAttributes),
>> + ep->ss_ep_comp.wBytesPerInterval);
>> + else
>> + return max_t(u32, max_payload, ep->ss_ep_comp.wBytesPerInterval);
>
> Obviously a kludge is necessary here to make these abominable devices
> work reliably with xHCI, but OTOH exceeding wBytesPerInterval violates
> USB3 9.6.7 and it's unclear if all devices would be happy.
>
> There are devices which define such odd isochronous alt settings with
> apparent intent to allow fine-grained bandwidth reservation:
>
> wMaxPacketSize 0x0400 1x 1024 bytes
> bInterval 1
> bMaxBurst 0
> wBytesPerInterval 512
>
> wMaxPacketSize 0x0400 1x 1024 bytes
> bInterval 1
> bMaxBurst 0
> wBytesPerInterval 1024
>
> wMaxPacketSize 0x0400 1x 1024 bytes
> bInterval 1
> bMaxBurst 1 # 2 packets per interval
> wBytesPerInterval 1536
>
> Isochronous drivers use this function to size their URBs or select the
> right altsetting for given bandwidth. UVC has obeyed wBytesPerInterval
> since forever with no apparent issues and UAC has recently been patched
> to work like that too with no issues so far AFAIK.
>
> Maybe start with something specific to the known bogus hardware, i.e.
> interrupt endpoint with one packet and zero payload? In such case
> it's high chance that the device actually meant it to be wMaxPacket.
>
>> default:
>> if (usb_endpoint_is_hs_isoc_double(udev, ep))
>> return le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
>> --
>> 2.17.1
>>
^ permalink raw reply
* Re: [GIT PULL] USB serial device ids for 7.0-rc7
From: Greg Kroah-Hartman @ 2026-04-02 11:28 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-usb, linux-kernel
In-Reply-To: <ac47qRXeYt92MbW_@hovoldconsulting.com>
On Thu, Apr 02, 2026 at 11:49:29AM +0200, Johan Hovold wrote:
> The following changes since commit f338e77383789c0cae23ca3d48adcc5e9e137e3c:
>
> Linux 7.0-rc4 (2026-03-15 13:52:05 -0700)
>
> are available in the Git repository at:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git tags/usb-serial-7.0-rc7
Pulled and pushed out,t hanks.
greg k-h
^ permalink raw reply
* Re: [PATCH v2] dma-debug: suppress cacheline overlap warning when arch has no DMA alignment requirement
From: Marek Szyprowski @ 2026-04-02 11:26 UTC (permalink / raw)
To: Robin Murphy, Andy Shevchenko
Cc: Mikhail Gavrilov, iommu, linux-kernel, linux-usb, linux-mm, harry,
vbabka, akpm, stern, linux, hch, Jeff.kirsher, catalin.marinas
In-Reply-To: <75f65aa7-4f89-4ef8-8941-51b1d54d1ad3@arm.com>
On 01.04.2026 17:26, Robin Murphy wrote:
> On 2026-04-01 2:25 pm, Andy Shevchenko wrote:
>> On Wed, Apr 1, 2026 at 3:11 PM Robin Murphy <robin.murphy@arm.com> wrote:
>>> On 2026-03-30 8:44 am, Marek Szyprowski wrote:
>>>> On 27.03.2026 13:41, Mikhail Gavrilov wrote:
>>
>> ...
>>
>>> TBH I'd be inclined to have CONFIG_DMA_DEBUG raise ARCH_DMA_MINALIGN as
>>> appropriate such that genuine false-positives can't happen, rather than
>>> effectively defeat the whole check,
>>
>> I dunno if you read v1 thread, where I proposed to unroll the check
>> and use pr_debug_once() for the cases which we expect not to panic,
>> but would be good to have a track of.
>
> I had not seen v1, as I took the last 3 days off and hadn't got that far up my inbox yet - I guess it's at least reassuring to have reached similar conclusions independently :)
>
> The fundamental issue here is that dma-debug doesn't realistically have a way to know whether the thing being mapped is intentionally a whole dedicated kmalloc allocation - where we can trust SLUB (and DMA_BOUNCE_UNALIGNED_KMALLOC if appropriate) to do the right thing across different systems - or just something which might happen to line up by coincidence on someone's development machine, but for portability they definitely do still need to take explicit care about (e.g. struct devres::data).
Right, the debug code cannot distinguish them. I'm still convinced that increasing
ARCH_DMA_MINALIGN when DEBUG is enabled is not a good idea. I also agree that the
current exceptions for DMA_BOUNCE_UNALIGNED_KMALLOC and dma_get_cache_alignment() <
L1_CACHE_BYTES are wider than really needed. What we can do about it?
For the (dma_get_cache_alignment() < L1_CACHE_BYTES) case I came up with the idea
of reducing the check only to the dma_get_cache_alignment()-size overlapping:
diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 86f87e43438c..9bf4b5c368f9 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -418,13 +418,13 @@ static void hash_bucket_del(struct dma_debug_entry *entry)
static RADIX_TREE(dma_active_cacheline, GFP_ATOMIC | __GFP_NOWARN);
static DEFINE_SPINLOCK(radix_lock);
#define ACTIVE_CACHELINE_MAX_OVERLAP ((1 << RADIX_TREE_MAX_TAGS) - 1)
-#define CACHELINE_PER_PAGE_SHIFT (PAGE_SHIFT - L1_CACHE_SHIFT)
-#define CACHELINES_PER_PAGE (1 << CACHELINE_PER_PAGE_SHIFT)
static phys_addr_t to_cacheline_number(struct dma_debug_entry *entry)
{
- return ((entry->paddr >> PAGE_SHIFT) << CACHELINE_PER_PAGE_SHIFT) +
- (offset_in_page(entry->paddr) >> L1_CACHE_SHIFT);
+ if (dma_get_cache_alignment() >= L1_CACHE_BYTES)
+ return entry->paddr >> L1_CACHE_SHIFT;
+ else
+ return entry->paddr >> ilog2(dma_get_cache_alignment());
}
static int active_cacheline_read_overlap(phys_addr_t cln)
For the remaining cases (DMA_BOUNCE_UNALIGNED_KMALLOC mainly), based on the Catalin's
suggestion, I came up with the following idea:
diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
index 0677918f06a8..f9b6a989ac15 100644
--- a/kernel/dma/debug.c
+++ b/kernel/dma/debug.c
@@ -18,6 +18,7 @@
#include <linux/uaccess.h>
#include <linux/export.h>
#include <linux/device.h>
+#include <linux/dma-direct.h>
#include <linux/types.h>
#include <linux/sched.h>
#include <linux/ctype.h>
@@ -590,6 +591,14 @@ static int dump_show(struct seq_file *seq, void *v)
}
DEFINE_SHOW_ATTRIBUTE(dump);
+static inline void dma_debug_fixup_bounced(struct dma_debug_entry *entry)
+{
+ if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) &&
+ dma_kmalloc_needs_bounce(entry->dev, entry->size, entry->direction) &&
+ is_swiotlb_active(entry->dev))
+ entry->paddr = dma_to_phys(entry->dev, entry->dev_addr);
+}
+
/*
* Wrapper function for adding an entry to the hash.
* This function takes care of locking itself.
@@ -601,6 +610,8 @@ static void add_dma_entry(struct dma_debug_entry *entry, unsigned long attrs)
unsigned long flags;
int rc;
+ dma_debug_fixup_bounced(entry);
+
entry->is_cache_clean = attrs & (DMA_ATTR_DEBUGGING_IGNORE_CACHELINES |
DMA_ATTR_REQUIRE_COHERENT);
@@ -614,9 +625,7 @@ static void add_dma_entry(struct dma_debug_entry *entry, unsigned long attrs)
global_disable = true;
} else if (rc == -EEXIST &&
!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
- !(entry->is_cache_clean && overlap_cache_clean) &&
- !(IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) &&
- is_swiotlb_active(entry->dev))) {
+ !(entry->is_cache_clean && overlap_cache_clean)) {
err_printk(entry->dev, entry,
"cacheline tracking EEXIST, overlapping mappings aren't supported\n");
}
@@ -981,6 +990,8 @@ static void check_unmap(struct dma_debug_entry *ref)
struct hash_bucket *bucket;
unsigned long flags;
+ dma_debug_fixup_bounced(ref);
+
bucket = get_hash_bucket(ref, &flags);
entry = bucket_find_exact(bucket, ref);
What do You think about it? Both at least allows to detect errors like multiple,
incompatible mappings of the same memory.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox