[PATCH v7 1/2] usb: xhci: Add timeout argument in address_device USB HCD callback

From: Hardik Gajjar
Date: Fri Oct 27 2023 - 11:20:55 EST


- The HCD address_device callback now accepts a user-defined timeout value
in milliseconds, providing better control over command execution times.
- The default timeout value for the address_device command has been set
to 5000 ms, aligning with the USB 3.2 specification. However, this
timeout can be adjusted as needed.
- The xhci_setup_device function has been updated to accept the timeout
value, allowing it to specify the maximum wait time for the command
operation to complete.
- The hub driver has also been updated to accommodate the newly added
timeout parameter during the SET_ADDRESS request.

Signed-off-by: Hardik Gajjar <hgajjar@xxxxxxxxxxxxxx>
---
Changes from version 1 to 6:
- The changes were part of the large patch until v6
Link to v6:https://lore.kernel.org/linux-usb/20231026101551.36551-1-hgajjar@xxxxxxxxxxxxxx/
This new patch was created by splitting patch v6 into two separate patches.
---
drivers/usb/core/hub.c | 2 +-
drivers/usb/host/xhci-mem.c | 2 ++
drivers/usb/host/xhci-ring.c | 11 ++++++-----
drivers/usb/host/xhci.c | 23 ++++++++++++++++-------
drivers/usb/host/xhci.h | 9 +++++++--
include/linux/usb/hcd.h | 5 +++--
6 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3c54b218301c..376147af287f 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4639,7 +4639,7 @@ static int hub_set_address(struct usb_device *udev, int devnum)
if (udev->state != USB_STATE_DEFAULT)
return -EINVAL;
if (hcd->driver->address_device)
- retval = hcd->driver->address_device(hcd, udev);
+ retval = hcd->driver->address_device(hcd, udev, USB_CTRL_SET_TIMEOUT);
else
retval = usb_control_msg(udev, usb_sndaddr0pipe(),
USB_REQ_SET_ADDRESS, 0, devnum, 0,
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 8714ab5bf04d..4d6df03d255e 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1729,6 +1729,8 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci,
}

command->status = 0;
+ /* set default timeout to 5000 ms */
+ command->timeout_ms = XHCI_CMD_DEFAULT_TIMEOUT;
INIT_LIST_HEAD(&command->cmd_list);
return command;
}
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1dde53f6eb31..8f36c2914938 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -366,9 +366,10 @@ void xhci_ring_cmd_db(struct xhci_hcd *xhci)
readl(&xhci->dba->doorbell[0]);
}

-static bool xhci_mod_cmd_timer(struct xhci_hcd *xhci, unsigned long delay)
+static bool xhci_mod_cmd_timer(struct xhci_hcd *xhci)
{
- return mod_delayed_work(system_wq, &xhci->cmd_timer, delay);
+ return mod_delayed_work(system_wq, &xhci->cmd_timer,
+ msecs_to_jiffies(xhci->current_cmd->timeout_ms));
}

static struct xhci_command *xhci_next_queued_cmd(struct xhci_hcd *xhci)
@@ -412,7 +413,7 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci,
if ((xhci->cmd_ring->dequeue != xhci->cmd_ring->enqueue) &&
!(xhci->xhc_state & XHCI_STATE_DYING)) {
xhci->current_cmd = cur_cmd;
- xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT);
+ xhci_mod_cmd_timer(xhci);
xhci_ring_cmd_db(xhci);
}
}
@@ -1786,7 +1787,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
if (!list_is_singular(&xhci->cmd_list)) {
xhci->current_cmd = list_first_entry(&cmd->cmd_list,
struct xhci_command, cmd_list);
- xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT);
+ xhci_mod_cmd_timer(xhci);
} else if (xhci->current_cmd == cmd) {
xhci->current_cmd = NULL;
}
@@ -4301,7 +4302,7 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
/* if there are no other commands queued we start the timeout timer */
if (list_empty(&xhci->cmd_list)) {
xhci->current_cmd = cmd;
- xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT);
+ xhci_mod_cmd_timer(xhci);
}

list_add_tail(&cmd->cmd_list, &xhci->cmd_list);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e1b1b64a0723..7c972d5b475b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3997,12 +3997,18 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
return 0;
}

-/*
- * Issue an Address Device command and optionally send a corresponding
- * SetAddress request to the device.
+/**
+ * xhci_setup_device - issues an Address Device command to assign a unique
+ * USB bus address.
+ * @hcd: USB host controller data structure.
+ * @udev: USB dev structure representing the connected device.
+ * @setup: Enum specifying setup mode: address only or with context.
+ * @timeout_ms: Max wait time (ms) for the command operation to complete.
+ *
+ * Return: 0 if successful; otherwise, negative error code.
*/
static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
- enum xhci_setup_dev setup)
+ enum xhci_setup_dev setup, unsigned int timeout_ms)
{
const char *act = setup == SETUP_CONTEXT_ONLY ? "context" : "address";
unsigned long flags;
@@ -4059,6 +4065,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
}

command->in_ctx = virt_dev->in_ctx;
+ command->timeout_ms = timeout_ms;

slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
ctrl_ctx = xhci_get_input_control_ctx(virt_dev->in_ctx);
@@ -4185,14 +4192,16 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
return ret;
}

-static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
+static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev,
+ unsigned int timeout_ms)
{
- return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS);
+ return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS, timeout_ms);
}

static int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev)
{
- return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY);
+ return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY,
+ XHCI_CMD_DEFAULT_TIMEOUT);
}

/*
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 7e282b4522c0..43193af562f4 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -818,6 +818,8 @@ struct xhci_command {
struct completion *completion;
union xhci_trb *command_trb;
struct list_head cmd_list;
+ /* xHCI command response timeout in milliseconds */
+ unsigned int timeout_ms;
};

/* drop context bitmasks */
@@ -1576,8 +1578,11 @@ struct xhci_td {
unsigned int num_trbs;
};

-/* xHCI command default timeout value */
-#define XHCI_CMD_DEFAULT_TIMEOUT (5 * HZ)
+/*
+ * xHCI command default timeout value in milliseconds.
+ * USB 3.2 spec, section 9.2.6.1
+ */
+#define XHCI_CMD_DEFAULT_TIMEOUT 5000

/* command descriptor */
struct xhci_cd {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 61d4f0b793dc..d0e19ac3ba6c 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -372,8 +372,9 @@ struct hc_driver {
* or bandwidth constraints.
*/
void (*reset_bandwidth)(struct usb_hcd *, struct usb_device *);
- /* Returns the hardware-chosen device address */
- int (*address_device)(struct usb_hcd *, struct usb_device *udev);
+ /* Set the hardware-chosen device address */
+ int (*address_device)(struct usb_hcd *, struct usb_device *udev,
+ unsigned int timeout_ms);
/* prepares the hardware to send commands to the device */
int (*enable_device)(struct usb_hcd *, struct usb_device *udev);
/* Notifies the HCD after a hub descriptor is fetched.
--
2.17.1