interesting
/
riscv-openocd
mirror of https://github.com/riscv/riscv-openocd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

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 <adrian.negreanu@nxp.com>
Reviewed-on: http://openocd.zylin.com/5920
Tested-by: jenkins
Reviewed-by: Tarek BOCHKATI <tarek.bouchkati@gmail.com>
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>
This commit is contained in:
Adrian Negreanu 2020-11-06 11:57:04 +02:00 committed by Tomas Vanek
parent 68e50415a1
commit fed42ccfd3
4 changed files with 83 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
src/jtag/drivers/cmsis_dap.c
View File

@ -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;
/* 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;
}
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);
}
LOG_DEBUG("CMSIS-DAP: Packet Size = %" PRId16, pkt_sz);
}
/* INFO_ID_PKT_CNT - byte */

4
src/jtag/drivers/cmsis_dap.h
View File

@ -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

37
src/jtag/drivers/cmsis_dap_usb_bulk.c
View File

@ -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,
};

40
src/jtag/drivers/cmsis_dap_usb_hid.c
View File

@ -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,
};