drivers/cmsis_dap: use blocking flag instead of wait timeout

CMSIS-DAP bulk backend read op used two timeouts: transfer timeout
used in libusb_fill_bulk_transfer() and wait timeout used optionally
in libusb_handle_events_timeout_completed().

The real usage is limited to two cases only:
1) blocking read: the same timeout is used for both transfer
and wait
2) non-blocking read: transfer timeout is used in
libusb_fill_bulk_transfer(),
libusb_handle_events_timeout_completed() is called with zero timeout.

Use blocking flag as read op parameter to distinguish between
these two cases.

See also [1]

Link: [1] 8596: jtag: cmsis_dap: include helper/time_support.h | https://review.openocd.org/c/openocd/+/8596
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
Change-Id: Ia755f17dc72bb9ce8e02065fee6a064f8eec6661
Reviewed-on: https://review.openocd.org/c/openocd/+/8639
Tested-by: jenkins
Reviewed-by: Paul Fertser <fercerpav@gmail.com>

src/jtag/drivers/cmsis_dap.c

@@ -225,12 +225,6 @@ struct pending_scan_result {
 	unsigned int buffer_offset;
 };
 
-/* Read mode */
-enum cmsis_dap_blocking {
-	CMSIS_DAP_NON_BLOCKING,
-	CMSIS_DAP_BLOCKING
-};
-
 /* Each block in FIFO can contain up to pending_queue_len transfers */
 static unsigned int pending_queue_len;
 static unsigned int tfer_max_command_size;
@@ -321,7 +315,7 @@ static void cmsis_dap_flush_read(struct cmsis_dap *dap)
 	 * USB close/open so we need to flush up to 64 old packets
 	 * to be sure all buffers are empty */
 	for (i = 0; i < 64; i++) {
-		int retval = dap->backend->read(dap, 10, NULL);
+		int retval = dap->backend->read(dap, 10, CMSIS_DAP_BLOCKING);
 		if (retval == ERROR_TIMEOUT_REACHED)
 			break;
 	}
@@ -338,7 +332,7 @@ static int cmsis_dap_xfer(struct cmsis_dap *dap, int txlen)
 	if (dap->pending_fifo_block_count) {
 		LOG_ERROR("pending %u blocks, flushing", dap->pending_fifo_block_count);
 		while (dap->pending_fifo_block_count) {
-			dap->backend->read(dap, 10, NULL);
+			dap->backend->read(dap, 10, CMSIS_DAP_BLOCKING);
 			dap->pending_fifo_block_count--;
 		}
 		dap->pending_fifo_put_idx = 0;
@@ -351,7 +345,7 @@ static int cmsis_dap_xfer(struct cmsis_dap *dap, int txlen)
 		return retval;
 
 	/* get reply */
-	retval = dap->backend->read(dap, LIBUSB_TIMEOUT_MS, NULL);
+	retval = dap->backend->read(dap, LIBUSB_TIMEOUT_MS, CMSIS_DAP_BLOCKING);
 	if (retval < 0)
 		return retval;
 
@@ -885,7 +879,7 @@ static void cmsis_dap_swd_read_process(struct cmsis_dap *dap, enum cmsis_dap_blo
 
 	if (queued_retval != ERROR_OK) {
 		/* keep reading blocks until the pipeline is empty */
-		retval = dap->backend->read(dap, 10, NULL);
+		retval = dap->backend->read(dap, 10, CMSIS_DAP_BLOCKING);
 		if (retval == ERROR_TIMEOUT_REACHED || retval == 0) {
 			/* timeout means that we flushed the pipeline,
 			 * we can safely discard remaining pending requests */
@@ -896,11 +890,7 @@ static void cmsis_dap_swd_read_process(struct cmsis_dap *dap, enum cmsis_dap_blo
 	}
 
 	/* get reply */
-	struct timeval tv = {
+	retval = dap->backend->read(dap, LIBUSB_TIMEOUT_MS, blocking);
-		.tv_sec = 0,
-		.tv_usec = 0
-	};
-	retval = dap->backend->read(dap, LIBUSB_TIMEOUT_MS, blocking ? NULL : &tv);
 	bool timeout = (retval == ERROR_TIMEOUT_REACHED || retval == 0);
 	if (timeout && blocking == CMSIS_DAP_NON_BLOCKING)
 		return;

src/jtag/drivers/cmsis_dap.h

@@ -58,12 +58,18 @@ struct cmsis_dap {
 	bool trace_enabled;
 };
 
+/* Read mode */
+enum cmsis_dap_blocking {
+	CMSIS_DAP_NON_BLOCKING,
+	CMSIS_DAP_BLOCKING
+};
+
 struct cmsis_dap_backend {
 	const char *name;
 	int (*open)(struct cmsis_dap *dap, uint16_t vids[], uint16_t pids[], const char *serial);
 	void (*close)(struct cmsis_dap *dap);
 	int (*read)(struct cmsis_dap *dap, int transfer_timeout_ms,
-			    struct timeval *wait_timeout);
+				enum cmsis_dap_blocking blocking);
 	int (*write)(struct cmsis_dap *dap, int len, int timeout_ms);
 	int (*packet_buffer_alloc)(struct cmsis_dap *dap, unsigned int pkt_sz);
 	void (*packet_buffer_free)(struct cmsis_dap *dap);

src/jtag/drivers/cmsis_dap_usb_bulk.c

@@ -441,7 +441,7 @@ static void LIBUSB_CALL cmsis_dap_usb_callback(struct libusb_transfer *transfer)
 }
 
 static int cmsis_dap_usb_read(struct cmsis_dap *dap, int transfer_timeout_ms,
-							  struct timeval *wait_timeout)
+							  enum cmsis_dap_blocking blocking)
 {
 	int transferred = 0;
 	int err;
@@ -464,20 +464,23 @@ static int cmsis_dap_usb_read(struct cmsis_dap *dap, int transfer_timeout_ms,
 		}
 	}
 
-	struct timeval tv = {
+	struct timeval tv;
-		.tv_sec = transfer_timeout_ms / 1000,
+	if (blocking == CMSIS_DAP_NON_BLOCKING) {
-		.tv_usec = transfer_timeout_ms % 1000 * 1000
+		tv.tv_sec = 0;
-	};
+		tv.tv_usec = 0;
+	} else {
+		tv.tv_sec = transfer_timeout_ms / 1000;
+		tv.tv_usec = transfer_timeout_ms % 1000 * 1000;
+	}
 
 	while (tr->status == CMSIS_DAP_TRANSFER_PENDING) {
-		err = libusb_handle_events_timeout_completed(dap->bdata->usb_ctx,
+		err = libusb_handle_events_timeout_completed(dap->bdata->usb_ctx, &tv,
-												 wait_timeout ? wait_timeout : &tv,
 												 &tr->status);
 		if (err) {
 			LOG_ERROR("error handling USB events: %s", libusb_strerror(err));
 			return ERROR_FAIL;
 		}
-		if (wait_timeout)
+		if (tv.tv_sec == 0 && tv.tv_usec == 0)
 			break;
 	}
 

src/jtag/drivers/cmsis_dap_usb_hid.c

@@ -206,17 +206,13 @@ static void cmsis_dap_hid_close(struct cmsis_dap *dap)
 }
 
 static int cmsis_dap_hid_read(struct cmsis_dap *dap, int transfer_timeout_ms,
-							  struct timeval *wait_timeout)
+							  enum cmsis_dap_blocking blocking)
 {
-	int timeout_ms;
+	int wait_ms = (blocking == CMSIS_DAP_NON_BLOCKING) ? 0 : transfer_timeout_ms;
-	if (wait_timeout)
-		timeout_ms = wait_timeout->tv_usec / 1000 + wait_timeout->tv_sec * 1000;
-	else
-		timeout_ms = transfer_timeout_ms;
 
 	int retval = hid_read_timeout(dap->bdata->dev_handle,
 								  dap->packet_buffer, dap->packet_buffer_size,
-								  timeout_ms);
+								  wait_ms);
 	if (retval == 0) {
 		return ERROR_TIMEOUT_REACHED;
 	} else if (retval == -1) {