From: Niklas Neronin <niklas.neronin@linux.intel.com>
To: mathias.nyman@linux.intel.com
Cc: linux-usb@vger.kernel.org,
Niklas Neronin <niklas.neronin@linux.intel.com>
Subject: [PATCH 1/9] usb: xhci: stop treating 'wIndex' as a mutable port number
Date: Thu, 19 Mar 2026 14:56:16 +0100 [thread overview]
Message-ID: <20260319135626.3077643-2-niklas.neronin@linux.intel.com> (raw)
In-Reply-To: <20260319135626.3077643-1-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>
---
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.50.1
next prev parent reply other threads:[~2026-03-19 13:57 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-19 13:56 [PATCH 0/9] usb: xhci: rework USB request parameter handling Niklas Neronin
2026-03-19 13:56 ` Niklas Neronin [this message]
2026-03-19 14:10 ` [PATCH 1/9] usb: xhci: stop treating 'wIndex' as a mutable port number Oliver Neukum
2026-03-19 13:56 ` [PATCH 2/9] usb: xhci: rename 'wIndex' parameters to 'portnum' Niklas Neronin
2026-03-19 13:56 ` [PATCH 3/9] usb: xhci: clean up handling of upper bits in SetPortFeature wIndex Niklas Neronin
2026-03-19 13:56 ` [PATCH 4/9] usb: xhci: clean up 'wValue' handling in xhci_hub_control() Niklas Neronin
2026-03-19 13:56 ` [PATCH 5/9] usb: xhci: separate use of USB Chapter 11 PLS macros from xHCI-specific PLS macros Niklas Neronin
2026-03-19 13:56 ` [PATCH 6/9] usb: xhci: add PORTPMSC variable to xhci_hub_control() Niklas Neronin
2026-03-19 13:56 ` [PATCH 7/9] usb: xhci: add PORTSC " Niklas Neronin
2026-03-19 13:56 ` [PATCH 8/9] usb: xhci: rename parameter to match argument 'portsc' Niklas Neronin
2026-03-19 13:56 ` [PATCH 9/9] usb: xhci: cleanup xhci_hub_report_usb3_link_state() Niklas Neronin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260319135626.3077643-2-niklas.neronin@linux.intel.com \
--to=niklas.neronin@linux.intel.com \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox