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>

diff --git a/src/jtag/drivers/cmsis_dap.c b/src/jtag/drivers/cmsis_dap.c
index 2f776cb387287ad1946eac92665f1cb6a512f29f..e9fd93ad1b937c26df9b6b227a4324f5aae2ba21 100644 (file)
--- a/src/jtag/drivers/cmsis_dap.c
+++ b/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 = {
-               .tv_sec = 0,
-               .tv_usec = 0
-       };
-       retval = dap->backend->read(dap, LIBUSB_TIMEOUT_MS, blocking ? NULL : &tv);
+       retval = dap->backend->read(dap, LIBUSB_TIMEOUT_MS, blocking);
        bool timeout = (retval == ERROR_TIMEOUT_REACHED || retval == 0);
        if (timeout && blocking == CMSIS_DAP_NON_BLOCKING)
                return;

diff --git a/src/jtag/drivers/cmsis_dap.h b/src/jtag/drivers/cmsis_dap.h
index e47697d1f9ad2106d2ea4b2842bafcacc62efe9d..aded0e54a3937a05c1fe3aca18d4c68f8705328b 100644 (file)
--- a/src/jtag/drivers/cmsis_dap.h
+++ b/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);

diff --git a/src/jtag/drivers/cmsis_dap_usb_bulk.c b/src/jtag/drivers/cmsis_dap_usb_bulk.c
index 8d0cb544d7bb481d4bd7f2514b70236c0def9a8b..50d4a9f8d3f651dd6e512d03363b7ccc1690403d 100644 (file)
--- a/src/jtag/drivers/cmsis_dap_usb_bulk.c
+++ b/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 = {
-               .tv_sec = transfer_timeout_ms / 1000,
-               .tv_usec = transfer_timeout_ms % 1000 * 1000
-       };
+       struct timeval tv;
+       if (blocking == CMSIS_DAP_NON_BLOCKING) {
+               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,
-                                                                                                wait_timeout ? wait_timeout : &tv,
+               err = libusb_handle_events_timeout_completed(dap->bdata->usb_ctx, &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;
        }
 

diff --git a/src/jtag/drivers/cmsis_dap_usb_hid.c b/src/jtag/drivers/cmsis_dap_usb_hid.c
index aeec685b9d1db162a7aa7102c53fd7b44bcd5292..a4058ec803f468e9fb28ddea2256c94125d85fd7 100644 (file)
--- a/src/jtag/drivers/cmsis_dap_usb_hid.c
+++ b/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;
-       if (wait_timeout)
-               timeout_ms = wait_timeout->tv_usec / 1000 + wait_timeout->tv_sec * 1000;
-       else
-               timeout_ms = transfer_timeout_ms;
+       int wait_ms = (blocking == CMSIS_DAP_NON_BLOCKING) ? 0 : 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) {