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>

diff --git a/src/jtag/drivers/cmsis_dap.c b/src/jtag/drivers/cmsis_dap.c
index fd565398ff9b3876fc85d3f6ae16c1462b1e9b4a..7a5acc3140c2bb1dab7996a7d0430214a4024833 100644 (file)
--- 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 054621cd517423ef6eb6080cf1d27f88ab9559ce..a096a95c0d4c6973e5d03e1faa2e156949b6b345 100644 (file)
--- 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 0134c0376557808ab0cdfe28797a13c3a9da5b8a..7efea5166689d26339de7fcc377afaddddc2eda4 100644 (file)
--- 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 681aef171266b9b2d782e0a118382a7ef6fe52fc..b290d6f81883ea3c13507e19b0eb430ffae18ef7 100644 (file)
--- 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,
 };