Skip to content

Commit

Permalink
drivers/cmsis_dap: use blocking flag instead of wait timeout
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@tom-van
tom-van committed Jan 9, 2025
1 parent 250ab10 commit 23796ef
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 31 deletions.
20 changes: 5 additions & 15 deletions src/jtag/drivers/cmsis_dap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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 */
Expand All @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion src/jtag/drivers/cmsis_dap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 11 additions & 8 deletions src/jtag/drivers/cmsis_dap_usb_bulk.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
10 changes: 3 additions & 7 deletions src/jtag/drivers/cmsis_dap_usb_hid.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 23796ef

Please sign in to comment.