From f0d8c3b0e024d045262ba50a516a3bd66bf22686 Mon Sep 17 00:00:00 2001 From: Gabor Csapo Date: Fri, 18 Mar 2022 18:11:49 +0800 Subject: [PATCH] libusb_helper.h: Increase USB timeout When we debug a target that works as a USB device, halting the target causes the USB communication with the USB host to become unresponsive. The host will try to reconnect/reset/setup the unresponsive device during which communication with other devices on the same USB bus can get stalled for several seconds. If the JTAG adapter is on the same bus, we need to make sure openOCD will wait for packets at least as long as the host USB stack. Otherwise the USB stack might deliver a valid packet, but openOCD would ignore it due to the timeout. The xHCI spec uses 5 sec timeouts, so let's use that in openOCD with some margin. Use this value in all libusb calls. HID API might have a libusb backend and would probably be victim to the same bug, so it should use this timeout, too. Ticket: https://sourceforge.net/p/openocd/tickets/343/ Signed-off-by: Gabor Csapo Change-Id: Ia3dc1356e676fe550f57a4c72f7a24ba296b6af2 Reviewed-on: https://review.openocd.org/c/openocd/+/6882 Tested-by: jenkins Reviewed-by: Antonio Borneo --- src/jtag/drivers/cmsis_dap.c | 15 +++++++-------- src/jtag/drivers/libusb_helper.h | 14 ++++++++++++++ src/jtag/drivers/nulink_usb.c | 4 +++- src/jtag/drivers/rlink.c | 30 ++++++++++++++---------------- src/jtag/drivers/stlink_usb.c | 4 ++-- src/jtag/drivers/ti_icdi_usb.c | 4 ++-- src/jtag/drivers/ulink.c | 11 ++++------- 7 files changed, 46 insertions(+), 36 deletions(-) diff --git a/src/jtag/drivers/cmsis_dap.c b/src/jtag/drivers/cmsis_dap.c index e7562d0877..4621fdc996 100644 --- a/src/jtag/drivers/cmsis_dap.c +++ b/src/jtag/drivers/cmsis_dap.c @@ -48,6 +48,7 @@ #include #include "cmsis_dap.h" +#include "libusb_helper.h" static const struct cmsis_dap_backend *const cmsis_dap_backends[] = { #if BUILD_CMSIS_DAP_USB == 1 @@ -79,8 +80,6 @@ static uint16_t cmsis_dap_pid[MAX_USB_IDS + 1] = { 0 }; static int cmsis_dap_backend = -1; static bool swd_mode; -#define USB_TIMEOUT 1000 - /* CMSIS-DAP General Commands */ #define CMD_DAP_INFO 0x00 #define CMD_DAP_LED 0x01 @@ -360,12 +359,12 @@ static int cmsis_dap_xfer(struct cmsis_dap *dap, int txlen) } uint8_t current_cmd = cmsis_dap_handle->command[0]; - int retval = dap->backend->write(dap, txlen, USB_TIMEOUT); + int retval = dap->backend->write(dap, txlen, LIBUSB_TIMEOUT_MS); if (retval < 0) return retval; /* get reply */ - retval = dap->backend->read(dap, USB_TIMEOUT); + retval = dap->backend->read(dap, LIBUSB_TIMEOUT_MS); if (retval < 0) return retval; @@ -826,7 +825,7 @@ static void cmsis_dap_swd_write_from_queue(struct cmsis_dap *dap) } } - int retval = dap->backend->write(dap, idx, USB_TIMEOUT); + int retval = dap->backend->write(dap, idx, LIBUSB_TIMEOUT_MS); if (retval < 0) { queued_retval = retval; goto skip; @@ -854,7 +853,7 @@ static void cmsis_dap_swd_read_process(struct cmsis_dap *dap, int timeout_ms) /* get reply */ int retval = dap->backend->read(dap, timeout_ms); - if (retval == ERROR_TIMEOUT_REACHED && timeout_ms < USB_TIMEOUT) + if (retval == ERROR_TIMEOUT_REACHED && timeout_ms < LIBUSB_TIMEOUT_MS) return; if (retval <= 0) { @@ -929,7 +928,7 @@ static int cmsis_dap_swd_run_queue(void) cmsis_dap_swd_write_from_queue(cmsis_dap_handle); while (pending_fifo_block_count) - cmsis_dap_swd_read_process(cmsis_dap_handle, USB_TIMEOUT); + cmsis_dap_swd_read_process(cmsis_dap_handle, LIBUSB_TIMEOUT_MS); pending_fifo_put_idx = 0; pending_fifo_get_idx = 0; @@ -953,7 +952,7 @@ static void cmsis_dap_swd_queue_cmd(uint8_t cmd, uint32_t *dst, uint32_t data) cmsis_dap_swd_write_from_queue(cmsis_dap_handle); if (pending_fifo_block_count >= cmsis_dap_handle->packet_count) - cmsis_dap_swd_read_process(cmsis_dap_handle, USB_TIMEOUT); + cmsis_dap_swd_read_process(cmsis_dap_handle, LIBUSB_TIMEOUT_MS); } if (queued_retval != ERROR_OK) diff --git a/src/jtag/drivers/libusb_helper.h b/src/jtag/drivers/libusb_helper.h index 2ddb246b3d..9d51464a7f 100644 --- a/src/jtag/drivers/libusb_helper.h +++ b/src/jtag/drivers/libusb_helper.h @@ -22,6 +22,20 @@ #include +/* When we debug a target that works as a USB device, halting the target causes the + * USB communication with the USB host to become unresponsive. The host will try + * to reconnect/reset/setup the unresponsive device during which communication + * with other devices on the same USB bus can get stalled for several seconds. + * If the JTAG adapter is on the same bus, we need to make sure openOCD will wait + * for packets at least as long as the host USB stack. Otherwise the USB stack + * might deliver a valid packet, but openOCD would ignore it due to the timeout. + * The xHCI spec uses 5 sec timeouts, so let's use that in openOCD with some margin. + * + * Use this value in all libusb calls. HID API might have a libusb backend and + * would probably be victim to the same bug, so it should use this timeout, too. + */ +#define LIBUSB_TIMEOUT_MS (6000) + /* this callback should return a non NULL value only when the serial could not * be retrieved by the standard 'libusb_get_string_descriptor_ascii' */ typedef char * (*adapter_get_alternate_serial_fn)(struct libusb_device_handle *device, diff --git a/src/jtag/drivers/nulink_usb.c b/src/jtag/drivers/nulink_usb.c index d4b8b53bc5..84a4420e82 100644 --- a/src/jtag/drivers/nulink_usb.c +++ b/src/jtag/drivers/nulink_usb.c @@ -33,7 +33,9 @@ #include -#define NULINK_READ_TIMEOUT 1000 +#include "libusb_helper.h" + +#define NULINK_READ_TIMEOUT LIBUSB_TIMEOUT_MS #define NULINK_HID_MAX_SIZE (64) #define NULINK2_HID_MAX_SIZE (1024) diff --git a/src/jtag/drivers/rlink.c b/src/jtag/drivers/rlink.c index 73be3c57e6..0cf9dbbb23 100644 --- a/src/jtag/drivers/rlink.c +++ b/src/jtag/drivers/rlink.c @@ -59,8 +59,6 @@ #define USB_EP2IN_SIZE (USB_EP2OUT_SIZE) #define USB_EP2BANK_SIZE (512) -#define USB_TIMEOUT_MS (3 * 1000) - #define DTC_STATUS_POLL_BYTE (ST7_USB_BUF_EP0OUT + 0xff) #define ST7_PD_NBUSY_LED ST7_PD0 @@ -133,7 +131,7 @@ static int ep1_generic_commandl(struct libusb_device_handle *hdev_param, size_t hdev_param, USB_EP1OUT_ADDR, (char *)usb_buffer, sizeof(usb_buffer), - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); @@ -176,7 +174,7 @@ static ssize_t ep1_memory_read( usb_ret = jtag_libusb_bulk_write( hdev_param, USB_EP1OUT_ADDR, (char *)usb_buffer, sizeof(usb_buffer), - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); @@ -186,7 +184,7 @@ static ssize_t ep1_memory_read( usb_ret = jtag_libusb_bulk_read( hdev_param, USB_EP1IN_ADDR, (char *)buffer, length, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); @@ -241,7 +239,7 @@ static ssize_t ep1_memory_write(struct libusb_device_handle *hdev_param, uint16_ usb_ret = jtag_libusb_bulk_write( hdev_param, USB_EP1OUT_ADDR, (char *)usb_buffer, sizeof(usb_buffer), - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); @@ -432,7 +430,7 @@ static int dtc_start_download(void) usb_err = jtag_libusb_bulk_read( hdev, USB_EP1IN_ADDR, (char *)&ep2txr, 1, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); if (usb_err != ERROR_OK) @@ -462,7 +460,7 @@ static int dtc_start_download(void) usb_err = jtag_libusb_bulk_read( hdev, USB_EP1IN_ADDR, (char *)&ep2txr, 1, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); @@ -488,7 +486,7 @@ static int dtc_run_download( hdev_param, USB_EP2OUT_ADDR, (char *)command_buffer, USB_EP2BANK_SIZE, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); if (usb_err < 0) @@ -512,7 +510,7 @@ static int dtc_run_download( hdev_param, USB_EP1IN_ADDR, &dtc_status, 1, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); if (usb_err < 0) @@ -533,7 +531,7 @@ static int dtc_run_download( hdev_param, USB_EP2IN_ADDR, (char *)reply_buffer, reply_buffer_size, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); @@ -954,7 +952,7 @@ static void rlink_reset(int trst, int srst) usb_err = jtag_libusb_bulk_read( hdev, USB_EP1IN_ADDR, (char *)&bitmap, 1, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); if (usb_err != ERROR_OK || transferred < 1) { @@ -990,7 +988,7 @@ static void rlink_reset(int trst, int srst) usb_err = jtag_libusb_bulk_read( hdev, USB_EP1IN_ADDR, (char *)&bitmap, 1, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); if (usb_err != ERROR_OK || transferred < 1) { @@ -1021,7 +1019,7 @@ static void rlink_reset(int trst, int srst) usb_err = jtag_libusb_bulk_read( hdev, USB_EP1IN_ADDR, (char *)&bitmap, 1, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); if (usb_err != ERROR_OK || transferred < 1) { @@ -1576,7 +1574,7 @@ static int rlink_init(void) jtag_libusb_bulk_read( hdev, USB_EP1IN_ADDR, (char *)reply_buffer, 1, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); @@ -1601,7 +1599,7 @@ static int rlink_init(void) jtag_libusb_bulk_read( hdev, USB_EP1IN_ADDR, (char *)reply_buffer, 1, - USB_TIMEOUT_MS, + LIBUSB_TIMEOUT_MS, &transferred ); diff --git a/src/jtag/drivers/stlink_usb.c b/src/jtag/drivers/stlink_usb.c index 325848f861..2785d9b968 100644 --- a/src/jtag/drivers/stlink_usb.c +++ b/src/jtag/drivers/stlink_usb.c @@ -71,8 +71,8 @@ #define ENDPOINT_IN 0x80 #define ENDPOINT_OUT 0x00 -#define STLINK_WRITE_TIMEOUT 1000 -#define STLINK_READ_TIMEOUT 1000 +#define STLINK_WRITE_TIMEOUT (LIBUSB_TIMEOUT_MS) +#define STLINK_READ_TIMEOUT (LIBUSB_TIMEOUT_MS) #define STLINK_RX_EP (1|ENDPOINT_IN) #define STLINK_TX_EP (2|ENDPOINT_OUT) diff --git a/src/jtag/drivers/ti_icdi_usb.c b/src/jtag/drivers/ti_icdi_usb.c index c94a1102f2..e48d0e269e 100644 --- a/src/jtag/drivers/ti_icdi_usb.c +++ b/src/jtag/drivers/ti_icdi_usb.c @@ -37,8 +37,8 @@ #define ICDI_WRITE_ENDPOINT 0x02 #define ICDI_READ_ENDPOINT 0x83 -#define ICDI_WRITE_TIMEOUT 1000 -#define ICDI_READ_TIMEOUT 1000 +#define ICDI_WRITE_TIMEOUT (LIBUSB_TIMEOUT_MS) +#define ICDI_READ_TIMEOUT (LIBUSB_TIMEOUT_MS) #define ICDI_PACKET_SIZE 2048 #define PACKET_START "$" diff --git a/src/jtag/drivers/ulink.c b/src/jtag/drivers/ulink.c index 20a036a78e..50609a64b1 100644 --- a/src/jtag/drivers/ulink.c +++ b/src/jtag/drivers/ulink.c @@ -56,9 +56,6 @@ /** USB interface number */ #define USB_INTERFACE 0 -/** libusb timeout in ms */ -#define USB_TIMEOUT 5000 - /** Delay (in microseconds) to wait while EZ-USB performs ReNumeration. */ #define ULINK_RENUMERATION_DELAY 1500000 @@ -335,7 +332,7 @@ static int ulink_cpu_reset(struct ulink *device, unsigned char reset_bit) ret = libusb_control_transfer(device->usb_device_handle, (LIBUSB_ENDPOINT_OUT | LIBUSB_REQUEST_TYPE_VENDOR | LIBUSB_RECIPIENT_DEVICE), - REQUEST_FIRMWARE_LOAD, CPUCS_REG, 0, &reset_bit, 1, USB_TIMEOUT); + REQUEST_FIRMWARE_LOAD, CPUCS_REG, 0, &reset_bit, 1, LIBUSB_TIMEOUT_MS); /* usb_control_msg() returns the number of bytes transferred during the * DATA stage of the control transfer - must be exactly 1 in this case! */ @@ -478,7 +475,7 @@ static int ulink_write_firmware_section(struct ulink *device, ret = libusb_control_transfer(device->usb_device_handle, (LIBUSB_ENDPOINT_OUT | LIBUSB_REQUEST_TYPE_VENDOR | LIBUSB_RECIPIENT_DEVICE), REQUEST_FIRMWARE_LOAD, addr, FIRMWARE_ADDR, (unsigned char *)data_ptr, - chunk_size, USB_TIMEOUT); + chunk_size, LIBUSB_TIMEOUT_MS); if (ret != (int)chunk_size) { /* Abort if libusb sent less data than requested */ @@ -662,7 +659,7 @@ static int ulink_append_queue(struct ulink *device, struct ulink_cmd *ulink_cmd) if ((newsize_out > 64) || (newsize_in > 64)) { /* New command does not fit. Execute all commands in queue before starting * new queue with the current command as first entry. */ - ret = ulink_execute_queued_commands(device, USB_TIMEOUT); + ret = ulink_execute_queued_commands(device, LIBUSB_TIMEOUT_MS); if (ret == ERROR_OK) ret = ulink_post_process_queue(device); @@ -1960,7 +1957,7 @@ static int ulink_execute_queue(void) } if (ulink_handle->commands_in_queue > 0) { - ret = ulink_execute_queued_commands(ulink_handle, USB_TIMEOUT); + ret = ulink_execute_queued_commands(ulink_handle, LIBUSB_TIMEOUT_MS); if (ret != ERROR_OK) return ret; -- 2.30.2