From: Spencer Oliver Date: Wed, 18 Sep 2013 19:06:26 +0000 (+0100) Subject: hla: move memory read/write functionality to driver X-Git-Tag: v0.8.0-rc1~249 X-Git-Url: https://review.openocd.org/gitweb?p=openocd.git;a=commitdiff_plain;h=cfe9ca039f4f6c058dff64effea50a857ff80f96 hla: move memory read/write functionality to driver Due to issues reported when using the jtag mode of the stlink (see Trac #61), the functionality/checking has been moved to the driver. This change also fixes unaligned 32bit memory read/write for the stlink. From testing this change also brings a 3KiB/s speed increase, this is due to the larger read/write packets. Change-Id: I8234110e7e49a683f4dadd54c442ecdc3c47b320 Signed-off-by: Spencer Oliver Reviewed-on: http://openocd.zylin.com/1632 Tested-by: jenkins Reviewed-by: Andreas Fritiofson --- diff --git a/src/jtag/drivers/stlink_usb.c b/src/jtag/drivers/stlink_usb.c index 4154b93c0a..31f08cbb8e 100644 --- a/src/jtag/drivers/stlink_usb.c +++ b/src/jtag/drivers/stlink_usb.c @@ -50,10 +50,14 @@ #define STLINK_TX_EP (2|ENDPOINT_OUT) #define STLINK_TRACE_EP (3|ENDPOINT_IN) #define STLINK_SG_SIZE (31) -#define STLINK_DATA_SIZE (4*128) +#define STLINK_DATA_SIZE (4096) #define STLINK_CMD_SIZE_V2 (16) #define STLINK_CMD_SIZE_V1 (10) +/* the current implementation of the stlink limits + * 8bit read/writes to max 64 bytes. */ +#define STLINK_MAX_RW8 (64) + enum stlink_jtag_api_version { STLINK_JTAG_API_V1 = 1, STLINK_JTAG_API_V2, @@ -86,6 +90,8 @@ struct stlink_usb_handle_s { /** */ uint8_t databuf[STLINK_DATA_SIZE]; /** */ + uint32_t max_mem_packet; + /** */ enum hl_transports transport; /** */ struct stlink_usb_version version; @@ -1317,6 +1323,12 @@ static int stlink_usb_read_mem8(void *handle, uint32_t addr, uint16_t len, assert(handle != NULL); + /* max 8bit read/write is 64bytes */ + if (len > STLINK_MAX_RW8) { + LOG_DEBUG("max buffer length exceeded"); + return ERROR_FAIL; + } + h = (struct stlink_usb_handle_s *)handle; stlink_usb_init_buffer(handle, STLINK_RX_EP, read_len); @@ -1351,6 +1363,12 @@ static int stlink_usb_write_mem8(void *handle, uint32_t addr, uint16_t len, assert(handle != NULL); + /* max 8bit read/write is 64bytes */ + if (len > STLINK_MAX_RW8) { + LOG_DEBUG("max buffer length exceeded"); + return ERROR_FAIL; + } + h = (struct stlink_usb_handle_s *)handle; stlink_usb_init_buffer(handle, STLINK_TX_EP, len); @@ -1379,9 +1397,13 @@ static int stlink_usb_read_mem32(void *handle, uint32_t addr, uint16_t len, assert(handle != NULL); - h = (struct stlink_usb_handle_s *)handle; + /* data must be a multiple of 4 and word aligned */ + if (len % 4 || addr % 4) { + LOG_DEBUG("Invalid data alignment"); + return ERROR_TARGET_UNALIGNED_ACCESS; + } - len *= 4; + h = (struct stlink_usb_handle_s *)handle; stlink_usb_init_buffer(handle, STLINK_RX_EP, len); @@ -1411,9 +1433,13 @@ static int stlink_usb_write_mem32(void *handle, uint32_t addr, uint16_t len, assert(handle != NULL); - h = (struct stlink_usb_handle_s *)handle; + /* data must be a multiple of 4 and word aligned */ + if (len % 4 || addr % 4) { + LOG_DEBUG("Invalid data alignment"); + return ERROR_TARGET_UNALIGNED_ACCESS; + } - len *= 4; + h = (struct stlink_usb_handle_s *)handle; stlink_usb_init_buffer(handle, STLINK_TX_EP, len); @@ -1432,22 +1458,134 @@ static int stlink_usb_write_mem32(void *handle, uint32_t addr, uint16_t len, return stlink_usb_get_rw_status(handle); } +static uint32_t stlink_max_block_size(uint32_t tar_autoincr_block, uint32_t address) +{ + uint32_t max_tar_block = (tar_autoincr_block - ((tar_autoincr_block - 1) & address)); + if (max_tar_block == 0) + max_tar_block = 4; + return max_tar_block; +} + static int stlink_usb_read_mem(void *handle, uint32_t addr, uint32_t size, uint32_t count, uint8_t *buffer) { - if (size == 4) - return stlink_usb_read_mem32(handle, addr, count, buffer); - else - return stlink_usb_read_mem8(handle, addr, count, buffer); + int retval = ERROR_OK; + uint32_t bytes_remaining; + struct stlink_usb_handle_s *h = (struct stlink_usb_handle_s *)handle; + + /* calculate byte count */ + count *= size; + + while (count) { + + bytes_remaining = (size == 4) ? \ + stlink_max_block_size(h->max_mem_packet, addr) : STLINK_MAX_RW8; + + if (count < bytes_remaining) + bytes_remaining = count; + + /* the stlink only supports 8/32bit memory read/writes + * honour 32bit, all others will be handled as 8bit access */ + if (size == 4) { + + /* When in jtag mode the stlink uses the auto-increment functinality. + * However it expects us to pass the data correctly, this includes + * alignment and any page boundaries. We already do this as part of the + * adi_v5 implementation, but the stlink is a hla adapter and so this + * needs implementiong manually. + * currently this only affects jtag mode, according to ST they do single + * access in SWD mode - but this may change and so we do it for both modes */ + + /* we first need to check for any unaligned bytes */ + if (addr % 4) { + + uint32_t head_bytes = 4 - (addr % 4); + retval = stlink_usb_read_mem8(handle, addr, head_bytes, buffer); + if (retval != ERROR_OK) + return retval; + buffer += head_bytes; + addr += head_bytes; + count -= head_bytes; + bytes_remaining -= head_bytes; + } + + if (bytes_remaining % 4) + retval = stlink_usb_read_mem(handle, addr, 1, bytes_remaining, buffer); + else + retval = stlink_usb_read_mem32(handle, addr, bytes_remaining, buffer); + } else + retval = stlink_usb_read_mem8(handle, addr, bytes_remaining, buffer); + + if (retval != ERROR_OK) + return retval; + + buffer += bytes_remaining; + addr += bytes_remaining; + count -= bytes_remaining; + } + + return retval; } static int stlink_usb_write_mem(void *handle, uint32_t addr, uint32_t size, uint32_t count, const uint8_t *buffer) { - if (size == 4) - return stlink_usb_write_mem32(handle, addr, count, buffer); - else - return stlink_usb_write_mem8(handle, addr, count, buffer); + int retval = ERROR_OK; + uint32_t bytes_remaining; + struct stlink_usb_handle_s *h = (struct stlink_usb_handle_s *)handle; + + /* calculate byte count */ + count *= size; + + while (count) { + + bytes_remaining = (size == 4) ? \ + stlink_max_block_size(h->max_mem_packet, addr) : STLINK_MAX_RW8; + + if (count < bytes_remaining) + bytes_remaining = count; + + /* the stlink only supports 8/32bit memory read/writes + * honour 32bit, all others will be handled as 8bit access */ + if (size == 4) { + + /* When in jtag mode the stlink uses the auto-increment functinality. + * However it expects us to pass the data correctly, this includes + * alignment and any page boundaries. We already do this as part of the + * adi_v5 implementation, but the stlink is a hla adapter and so this + * needs implementiong manually. + * currently this only affects jtag mode, according to ST they do single + * access in SWD mode - but this may change and so we do it for both modes */ + + /* we first need to check for any unaligned bytes */ + if (addr % 4) { + + uint32_t head_bytes = 4 - (addr % 4); + retval = stlink_usb_write_mem8(handle, addr, head_bytes, buffer); + if (retval != ERROR_OK) + return retval; + buffer += head_bytes; + addr += head_bytes; + count -= head_bytes; + bytes_remaining -= head_bytes; + } + + if (bytes_remaining % 4) + retval = stlink_usb_write_mem(handle, addr, 1, bytes_remaining, buffer); + else + retval = stlink_usb_write_mem32(handle, addr, bytes_remaining, buffer); + + } else + retval = stlink_usb_write_mem8(handle, addr, bytes_remaining, buffer); + if (retval != ERROR_OK) + return retval; + + buffer += bytes_remaining; + addr += bytes_remaining; + count -= bytes_remaining; + } + + return retval; } /** */ @@ -1483,9 +1621,6 @@ static int stlink_usb_open(struct hl_interface_param_s *param, void **fd) h->transport = param->transport; - /* set max read/write buffer size in bytes */ - param->max_buffer = 512; - const uint16_t vids[] = { param->vid, 0 }; const uint16_t pids[] = { param->pid, 0 }; @@ -1616,6 +1751,23 @@ static int stlink_usb_open(struct hl_interface_param_s *param, void **fd) goto error_open; } + /* get cpuid, so we can determine the max page size + * start with a safe default */ + h->max_mem_packet = (1 << 10); + + uint8_t buffer[4]; + err = stlink_usb_read_mem32(h, CPUID, 4, buffer); + if (err == ERROR_OK) { + uint32_t cpuid = le_to_h_u32(buffer); + int i = (cpuid >> 4) & 0xf; + if (i == 4 || i == 3) { + /* Cortex-M3/M4 has 4096 bytes autoincrement range */ + h->max_mem_packet = (1 << 12); + } + } + + LOG_DEBUG("Using TAR autoincrement: %" PRIu32, h->max_mem_packet); + *fd = h; return ERROR_OK; diff --git a/src/jtag/drivers/ti_icdi_usb.c b/src/jtag/drivers/ti_icdi_usb.c index ae2f240fc5..0d7d943bed 100644 --- a/src/jtag/drivers/ti_icdi_usb.c +++ b/src/jtag/drivers/ti_icdi_usb.c @@ -53,6 +53,7 @@ struct icdi_usb_handle_s { char *write_buffer; int max_packet; int read_count; + uint32_t max_rw_packet; /* max X packet (read/write memory) transfers */ }; static int icdi_usb_read_mem(void *handle, uint32_t addr, uint32_t size, @@ -592,17 +593,57 @@ static int icdi_usb_write_mem_int(void *handle, uint32_t addr, uint32_t len, con static int icdi_usb_read_mem(void *handle, uint32_t addr, uint32_t size, uint32_t count, uint8_t *buffer) { - if (size == 4) - count *= size; - return icdi_usb_read_mem_int(handle, addr, count, buffer); + int retval = ERROR_OK; + struct icdi_usb_handle_s *h = (struct icdi_usb_handle_s *)handle; + uint32_t bytes_remaining; + + /* calculate byte count */ + count *= size; + + while (count) { + + bytes_remaining = h->max_rw_packet; + if (count < bytes_remaining) + bytes_remaining = count; + + retval = icdi_usb_read_mem_int(handle, addr, bytes_remaining, buffer); + if (retval != ERROR_OK) + return retval; + + buffer += bytes_remaining; + addr += bytes_remaining; + count -= bytes_remaining; + } + + return retval; } static int icdi_usb_write_mem(void *handle, uint32_t addr, uint32_t size, uint32_t count, const uint8_t *buffer) { - if (size == 4) - count *= size; - return icdi_usb_write_mem_int(handle, addr, count, buffer); + int retval = ERROR_OK; + struct icdi_usb_handle_s *h = (struct icdi_usb_handle_s *)handle; + uint32_t bytes_remaining; + + /* calculate byte count */ + count *= size; + + while (count) { + + bytes_remaining = h->max_rw_packet; + if (count < bytes_remaining) + bytes_remaining = count; + + retval = icdi_usb_write_mem_int(handle, addr, bytes_remaining, buffer); + if (retval != ERROR_OK) + return retval; + + buffer += bytes_remaining; + addr += bytes_remaining; + count -= bytes_remaining; + } + + return retval; } static int icdi_usb_close(void *handle) @@ -707,7 +748,7 @@ static int icdi_usb_open(struct hl_interface_param_s *param, void **fd) * as we are using gdb binary packets to transfer memory we have to * reserve half the buffer for any possible escape chars plus * at least 64 bytes for the gdb packet header */ - param->max_buffer = (((h->max_packet - 64) / 4) * 4) / 2; + h->max_rw_packet = (((h->max_packet - 64) / 4) * 4) / 2; return ERROR_OK; diff --git a/src/jtag/hla/hla_interface.c b/src/jtag/hla/hla_interface.c index f8b5c61b4f..0176a48238 100644 --- a/src/jtag/hla/hla_interface.c +++ b/src/jtag/hla/hla_interface.c @@ -37,7 +37,7 @@ #include -static struct hl_interface_s hl_if = { {0, 0, 0, 0, 0, HL_TRANSPORT_UNKNOWN, 0, false, NULL, 0}, 0, 0 }; +static struct hl_interface_s hl_if = { {0, 0, 0, 0, 0, HL_TRANSPORT_UNKNOWN, false, NULL, 0}, 0, 0 }; int hl_interface_open(enum hl_transports tr) { diff --git a/src/jtag/hla/hla_interface.h b/src/jtag/hla/hla_interface.h index 398aa80623..fcf1d9e228 100644 --- a/src/jtag/hla/hla_interface.h +++ b/src/jtag/hla/hla_interface.h @@ -45,8 +45,6 @@ struct hl_interface_param_s { /** */ enum hl_transports transport; /** */ - int max_buffer; - /** */ bool connect_under_reset; /** Output file for trace data (if any) */ FILE *trace_f; diff --git a/src/jtag/hla/hla_layout.c b/src/jtag/hla/hla_layout.c index ef24fa3c3c..1d22fe13e1 100644 --- a/src/jtag/hla/hla_layout.c +++ b/src/jtag/hla/hla_layout.c @@ -50,12 +50,6 @@ static int hl_layout_open(struct hl_interface_s *adapter) return res; } - /* make sure adapter has set the buffer size */ - if (!adapter->param.max_buffer) { - LOG_ERROR("buffer size not set"); - return ERROR_FAIL; - } - return ERROR_OK; } diff --git a/src/target/hla_target.c b/src/target/hla_target.c index dc81ee89a6..a65ba805e4 100644 --- a/src/target/hla_target.c +++ b/src/target/hla_target.c @@ -758,41 +758,13 @@ static int adapter_read_memory(struct target *target, uint32_t address, uint8_t *buffer) { struct hl_interface_s *adapter = target_to_adapter(target); - int res; - uint32_t buffer_threshold = (adapter->param.max_buffer / 4); - uint32_t addr_increment = 4; - uint32_t c; if (!count || !buffer) return ERROR_COMMAND_SYNTAX_ERROR; LOG_DEBUG("%s 0x%08x %d %d", __func__, address, size, count); - /* prepare byte count, buffer threshold - * and address increment for none 32bit access - */ - if (size != 4) { - count *= size; - buffer_threshold = (adapter->param.max_buffer / 4) / 2; - addr_increment = 1; - } - - while (count) { - if (count > buffer_threshold) - c = buffer_threshold; - else - c = count; - - res = adapter->layout->api->read_mem(adapter->fd, address, size, c, buffer); - if (res != ERROR_OK) - return res; - - address += (c * addr_increment); - buffer += (c * addr_increment); - count -= c; - } - - return ERROR_OK; + return adapter->layout->api->read_mem(adapter->fd, address, size, count, buffer); } static int adapter_write_memory(struct target *target, uint32_t address, @@ -800,41 +772,13 @@ static int adapter_write_memory(struct target *target, uint32_t address, const uint8_t *buffer) { struct hl_interface_s *adapter = target_to_adapter(target); - int res; - uint32_t buffer_threshold = (adapter->param.max_buffer / 4); - uint32_t addr_increment = 4; - uint32_t c; if (!count || !buffer) return ERROR_COMMAND_SYNTAX_ERROR; LOG_DEBUG("%s 0x%08x %d %d", __func__, address, size, count); - /* prepare byte count, buffer threshold - * and address increment for none 32bit access - */ - if (size != 4) { - count *= size; - buffer_threshold = (adapter->param.max_buffer / 4) / 2; - addr_increment = 1; - } - - while (count) { - if (count > buffer_threshold) - c = buffer_threshold; - else - c = count; - - res = adapter->layout->api->write_mem(adapter->fd, address, size, c, buffer); - if (res != ERROR_OK) - return res; - - address += (c * addr_increment); - buffer += (c * addr_increment); - count -= c; - } - - return ERROR_OK; + return adapter->layout->api->write_mem(adapter->fd, address, size, count, buffer); } static const struct command_registration adapter_command_handlers[] = {