Menu
▾
▴
[OpenOCD-commit] Main OpenOCD repository branch, master, updated. v0.11.0-70-gfed42ccfd
|
From: OpenOCD-Gerrit <ope...@us...> - 2021-04-10 19:52:34
|
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Main OpenOCD repository".
The branch, master has been updated
via fed42ccfd3dff0c3dcfa7f017bbd26eff3d4f15c (commit)
from 68e50415a115f3b80b7d1a1b580287745b281a5c (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit fed42ccfd3dff0c3dcfa7f017bbd26eff3d4f15c
Author: Adrian Negreanu <adr...@nx...>
Date: Fri Nov 6 11:57:04 2020 +0200
cmsis-dap: don't update the packet size across backends.
The hidapi cmsis-dap backend is using a packet_size of
64+1: 64 is the bMaxPacketSize0 and 1 is the hid Report-Id.
In hidapi::hid_write(), the packet_size is decremented by 1 and
stored for the next transfer.
The packet_size is now valid bMaxPacketSize0=64,
so when hid_read() is called, libusb_bulk_transfer() finishes w/o timeout.
For the libusb bulk backend, the same packet_size of 64+1 is used,
but there's no hid_write() to decrement and store it for the next read.
So the next time a read is done, it will try to read 64+1 bytes.
Fix this by putting the packet logic within each backend.
Use calloc() to allocate the struct cmsis_dap to be on safer side.
Change-Id: I0c450adbc7674d5fcd8208dd23062d5cdd209efd
Signed-off-by: Adrian Negreanu <adr...@nx...>
Reviewed-on: http://openocd.zylin.com/5920
Tested-by: jenkins
Reviewed-by: Tarek BOCHKATI <tar...@gm...>
Reviewed-by: Tomas Vanek <va...@fb...>
diff --git a/src/jtag/drivers/cmsis_dap.c b/src/jtag/drivers/cmsis_dap.c
index fd565398f..7a5acc314 100644
--- a/src/jtag/drivers/cmsis_dap.c
+++ b/src/jtag/drivers/cmsis_dap.c
@@ -225,16 +225,12 @@ static int cmsis_dap_open(void)
{
const struct cmsis_dap_backend *backend = NULL;
- struct cmsis_dap *dap = malloc(sizeof(struct cmsis_dap));
+ struct cmsis_dap *dap = calloc(1, sizeof(struct cmsis_dap));
if (dap == NULL) {
LOG_ERROR("unable to allocate memory");
return ERROR_FAIL;
}
- dap->caps = 0;
- dap->mode = 0;
- dap->packet_size = 0; /* initialized by backend */
-
if (cmsis_dap_backend >= 0) {
/* Use forced backend */
backend = cmsis_dap_backends[cmsis_dap_backend];
@@ -257,17 +253,7 @@ static int cmsis_dap_open(void)
return ERROR_FAIL;
}
- assert(dap->packet_size > 0);
-
dap->backend = backend;
- dap->packet_buffer = malloc(dap->packet_size);
-
- if (dap->packet_buffer == NULL) {
- LOG_ERROR("unable to allocate memory");
- dap->backend->close(dap);
- free(dap);
- return ERROR_FAIL;
- }
cmsis_dap_handle = dap;
@@ -929,24 +915,20 @@ static int cmsis_dap_init(void)
if (data[0] == 2) { /* short */
uint16_t pkt_sz = data[1] + (data[2] << 8);
+ if (pkt_sz != cmsis_dap_handle->packet_size) {
- /* 4 bytes of command header + 5 bytes per register
- * write. For bulk read sequences just 4 bytes are
- * needed per transfer, so this is suboptimal. */
- pending_queue_len = (pkt_sz - 4) / 5;
-
- if (cmsis_dap_handle->packet_size != pkt_sz + 1) {
- /* reallocate buffer */
- cmsis_dap_handle->packet_size = pkt_sz + 1;
- cmsis_dap_handle->packet_buffer = realloc(cmsis_dap_handle->packet_buffer,
- cmsis_dap_handle->packet_size);
- if (cmsis_dap_handle->packet_buffer == NULL) {
- LOG_ERROR("unable to reallocate memory");
- return ERROR_FAIL;
- }
- }
+ /* 4 bytes of command header + 5 bytes per register
+ * write. For bulk read sequences just 4 bytes are
+ * needed per transfer, so this is suboptimal. */
+ pending_queue_len = (pkt_sz - 4) / 5;
- LOG_DEBUG("CMSIS-DAP: Packet Size = %" PRId16, pkt_sz);
+ free(cmsis_dap_handle->packet_buffer);
+ retval = cmsis_dap_handle->backend->packet_buffer_alloc(cmsis_dap_handle, pkt_sz);
+ if (retval != ERROR_OK)
+ return retval;
+
+ LOG_DEBUG("CMSIS-DAP: Packet Size = %" PRIu16, pkt_sz);
+ }
}
/* INFO_ID_PKT_CNT - byte */
diff --git a/src/jtag/drivers/cmsis_dap.h b/src/jtag/drivers/cmsis_dap.h
index 054621cd5..a096a95c0 100644
--- a/src/jtag/drivers/cmsis_dap.h
+++ b/src/jtag/drivers/cmsis_dap.h
@@ -13,6 +13,7 @@ struct cmsis_dap {
uint16_t packet_size;
int packet_count;
uint8_t *packet_buffer;
+ uint16_t packet_buffer_size;
uint8_t caps;
uint8_t mode;
};
@@ -23,10 +24,13 @@ struct cmsis_dap_backend {
void (*close)(struct cmsis_dap *dap);
int (*read)(struct cmsis_dap *dap, int timeout_ms);
int (*write)(struct cmsis_dap *dap, int len, int timeout_ms);
+ int (*packet_buffer_alloc)(struct cmsis_dap *dap, unsigned int pkt_sz);
};
extern const struct cmsis_dap_backend cmsis_dap_hid_backend;
extern const struct cmsis_dap_backend cmsis_dap_usb_backend;
extern const struct command_registration cmsis_dap_usb_subcommand_handlers[];
+#define REPORT_ID_SIZE 1
+
#endif
diff --git a/src/jtag/drivers/cmsis_dap_usb_bulk.c b/src/jtag/drivers/cmsis_dap_usb_bulk.c
index 0134c0376..7efea5166 100644
--- a/src/jtag/drivers/cmsis_dap_usb_bulk.c
+++ b/src/jtag/drivers/cmsis_dap_usb_bulk.c
@@ -50,6 +50,9 @@ struct cmsis_dap_backend_data {
static int cmsis_dap_usb_interface = -1;
+static void cmsis_dap_usb_close(struct cmsis_dap *dap);
+static int cmsis_dap_usb_alloc(struct cmsis_dap *dap, unsigned int pkt_sz);
+
static int cmsis_dap_usb_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t pids[], char *serial)
{
int err;
@@ -356,12 +359,21 @@ static int cmsis_dap_usb_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
return ERROR_FAIL;
}
- dap->packet_size = packet_size + 1; /* "+ 1" for compatibility with the HID backend */
+ dap->packet_size = packet_size;
+ dap->packet_buffer_size = packet_size + REPORT_ID_SIZE;
dap->bdata->usb_ctx = ctx;
dap->bdata->dev_handle = dev_handle;
dap->bdata->ep_out = ep_out;
dap->bdata->ep_in = ep_in;
dap->bdata->interface = interface_num;
+
+ dap->packet_buffer = malloc(dap->packet_buffer_size);
+ if (dap->packet_buffer == NULL) {
+ LOG_ERROR("unable to allocate memory");
+ cmsis_dap_usb_close(dap);
+ return ERROR_FAIL;
+ }
+
return ERROR_OK;
}
@@ -381,6 +393,8 @@ static void cmsis_dap_usb_close(struct cmsis_dap *dap)
libusb_exit(dap->bdata->usb_ctx);
free(dap->bdata);
dap->bdata = NULL;
+ free(dap->packet_buffer);
+ dap->packet_buffer = NULL;
}
static int cmsis_dap_usb_read(struct cmsis_dap *dap, int timeout_ms)
@@ -399,7 +413,7 @@ static int cmsis_dap_usb_read(struct cmsis_dap *dap, int timeout_ms)
}
}
- memset(&dap->packet_buffer[transferred], 0, dap->packet_size - transferred);
+ memset(&dap->packet_buffer[transferred], 0, dap->packet_buffer_size - transferred);
return transferred;
}
@@ -411,7 +425,7 @@ static int cmsis_dap_usb_write(struct cmsis_dap *dap, int txlen, int timeout_ms)
/* skip the first byte that is only used by the HID backend */
err = libusb_bulk_transfer(dap->bdata->dev_handle, dap->bdata->ep_out,
- dap->packet_buffer + 1, txlen - 1, &transferred, timeout_ms);
+ dap->packet_buffer + REPORT_ID_SIZE, txlen - REPORT_ID_SIZE, &transferred, timeout_ms);
if (err) {
if (err == LIBUSB_ERROR_TIMEOUT) {
return ERROR_TIMEOUT_REACHED;
@@ -424,6 +438,22 @@ static int cmsis_dap_usb_write(struct cmsis_dap *dap, int txlen, int timeout_ms)
return transferred;
}
+static int cmsis_dap_usb_alloc(struct cmsis_dap *dap, unsigned int pkt_sz)
+{
+ unsigned int packet_buffer_size = pkt_sz + REPORT_ID_SIZE;
+ uint8_t *buf = malloc(packet_buffer_size);
+ if (buf == NULL) {
+ LOG_ERROR("unable to allocate CMSIS-DAP packet buffer");
+ return ERROR_FAIL;
+ }
+
+ dap->packet_buffer = buf;
+ dap->packet_size = pkt_sz;
+ dap->packet_buffer_size = packet_buffer_size;
+
+ return ERROR_OK;
+}
+
COMMAND_HANDLER(cmsis_dap_handle_usb_interface_command)
{
if (CMD_ARGC == 1)
@@ -451,4 +481,5 @@ const struct cmsis_dap_backend cmsis_dap_usb_backend = {
.close = cmsis_dap_usb_close,
.read = cmsis_dap_usb_read,
.write = cmsis_dap_usb_write,
+ .packet_buffer_alloc = cmsis_dap_usb_alloc,
};
diff --git a/src/jtag/drivers/cmsis_dap_usb_hid.c b/src/jtag/drivers/cmsis_dap_usb_hid.c
index 681aef171..b290d6f81 100644
--- a/src/jtag/drivers/cmsis_dap_usb_hid.c
+++ b/src/jtag/drivers/cmsis_dap_usb_hid.c
@@ -40,12 +40,13 @@
#include "cmsis_dap.h"
-#define PACKET_SIZE (64 + 1) /* 64 bytes plus report id */
-
struct cmsis_dap_backend_data {
hid_device *dev_handle;
};
+static void cmsis_dap_hid_close(struct cmsis_dap *dap);
+static int cmsis_dap_hid_alloc(struct cmsis_dap *dap, unsigned int pkt_sz);
+
static int cmsis_dap_hid_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t pids[], char *serial)
{
hid_device *dev = NULL;
@@ -145,7 +146,7 @@ static int cmsis_dap_hid_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
* without this info we cannot communicate with the adapter.
* For the moment we have to hard code the packet size */
- dap->packet_size = PACKET_SIZE;
+ unsigned int packet_size = 64;
/* atmel cmsis-dap uses 512 byte reports */
/* except when it doesn't e.g. with mEDBG on SAMD10 Xplained
@@ -153,10 +154,16 @@ static int cmsis_dap_hid_open(struct cmsis_dap *dap, uint16_t vids[], uint16_t p
/* TODO: HID report descriptor should be parsed instead of
* hardcoding a match by VID */
if (target_vid == 0x03eb && target_pid != 0x2145 && target_pid != 0x2175)
- dap->packet_size = 512 + 1;
+ packet_size = 512;
dap->bdata->dev_handle = dev;
+ int retval = cmsis_dap_hid_alloc(dap, packet_size);
+ if (retval != ERROR_OK) {
+ cmsis_dap_hid_close(dap);
+ return ERROR_FAIL;
+ }
+
return ERROR_OK;
}
@@ -166,11 +173,13 @@ static void cmsis_dap_hid_close(struct cmsis_dap *dap)
hid_exit();
free(dap->bdata);
dap->bdata = NULL;
+ free(dap->packet_buffer);
+ dap->packet_buffer = NULL;
}
static int cmsis_dap_hid_read(struct cmsis_dap *dap, int timeout_ms)
{
- int retval = hid_read_timeout(dap->bdata->dev_handle, dap->packet_buffer, dap->packet_size, timeout_ms);
+ int retval = hid_read_timeout(dap->bdata->dev_handle, dap->packet_buffer, dap->packet_buffer_size, timeout_ms);
if (retval == 0) {
return ERROR_TIMEOUT_REACHED;
@@ -187,10 +196,10 @@ static int cmsis_dap_hid_write(struct cmsis_dap *dap, int txlen, int timeout_ms)
(void) timeout_ms;
/* Pad the rest of the TX buffer with 0's */
- memset(dap->packet_buffer + txlen, 0, dap->packet_size - txlen);
+ memset(dap->packet_buffer + txlen, 0, dap->packet_buffer_size - txlen);
/* write data to device */
- int retval = hid_write(dap->bdata->dev_handle, dap->packet_buffer, dap->packet_size);
+ int retval = hid_write(dap->bdata->dev_handle, dap->packet_buffer, dap->packet_buffer_size);
if (retval == -1) {
LOG_ERROR("error writing data: %ls", hid_error(dap->bdata->dev_handle));
return ERROR_FAIL;
@@ -199,10 +208,27 @@ static int cmsis_dap_hid_write(struct cmsis_dap *dap, int txlen, int timeout_ms)
return retval;
}
+static int cmsis_dap_hid_alloc(struct cmsis_dap *dap, unsigned int pkt_sz)
+{
+ unsigned int packet_buffer_size = pkt_sz + REPORT_ID_SIZE;
+ uint8_t *buf = malloc(packet_buffer_size);
+ if (buf == NULL) {
+ LOG_ERROR("unable to allocate CMSIS-DAP packet buffer");
+ return ERROR_FAIL;
+ }
+
+ dap->packet_buffer = buf;
+ dap->packet_size = pkt_sz;
+ dap->packet_buffer_size = packet_buffer_size;
+
+ return ERROR_OK;
+}
+
const struct cmsis_dap_backend cmsis_dap_hid_backend = {
.name = "hid",
.open = cmsis_dap_hid_open,
.close = cmsis_dap_hid_close,
.read = cmsis_dap_hid_read,
.write = cmsis_dap_hid_write,
+ .packet_buffer_alloc = cmsis_dap_hid_alloc,
};
-----------------------------------------------------------------------
Summary of changes:
src/jtag/drivers/cmsis_dap.c | 44 +++++++++++------------------------
src/jtag/drivers/cmsis_dap.h | 4 ++++
src/jtag/drivers/cmsis_dap_usb_bulk.c | 37 ++++++++++++++++++++++++++---
src/jtag/drivers/cmsis_dap_usb_hid.c | 40 +++++++++++++++++++++++++------
4 files changed, 84 insertions(+), 41 deletions(-)
hooks/post-receive
--
Main OpenOCD repository
|
