From 726c06ca3da324cd521a2f7074ea339ec26259d5 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Tue, 4 May 2021 11:03:34 -0700 Subject: [PATCH 1/5] Speed up a little again by buffering writes. We need to read more than one byte at a time to get the original speed back. OpenOCD mailing list suggests using fread/fwrite instead of implementing our own buffering as we did here. Change-Id: I261c743ee6c40c54bfd57a919f074512a1c40bee --- src/jtag/drivers/remote_bitbang.c | 44 +++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/src/jtag/drivers/remote_bitbang.c b/src/jtag/drivers/remote_bitbang.c index f7b88303b..90451a7d4 100644 --- a/src/jtag/drivers/remote_bitbang.c +++ b/src/jtag/drivers/remote_bitbang.c @@ -36,6 +36,8 @@ static char *remote_bitbang_host; static char *remote_bitbang_port; static int remote_bitbang_fd; +static uint8_t remote_bitbang_send_buf[512]; +static unsigned int remote_bitbang_send_buf_used; /* Circular buffer. When start == end, the buffer is empty. */ static char remote_bitbang_buf[64]; @@ -90,20 +92,37 @@ static int remote_bitbang_fill_buf(void) return ERROR_OK; } -static int remote_bitbang_putc(int c) +static int remote_bitbang_flush(void) { - char buf = c; - ssize_t count = write_socket(remote_bitbang_fd, &buf, sizeof(buf)); - if (count < 0) { - log_socket_error("remote_bitbang_putc"); - return ERROR_FAIL; + if (remote_bitbang_send_buf_used <= 0) + return ERROR_OK; + + unsigned int offset = 0; + while (offset < remote_bitbang_send_buf_used) { + ssize_t written = write_socket(remote_bitbang_fd, remote_bitbang_send_buf + offset, + remote_bitbang_send_buf_used - offset); + if (written < 0) { + log_socket_error("remote_bitbang_putc"); + remote_bitbang_send_buf_used = 0; + return ERROR_FAIL; + } + offset += written; } + remote_bitbang_send_buf_used = 0; + return ERROR_OK; +} + +static int remote_bitbang_queue(int c, bool flush) +{ + remote_bitbang_send_buf[remote_bitbang_send_buf_used++] = c; + if (flush || remote_bitbang_send_buf_used >= ARRAY_SIZE(remote_bitbang_send_buf)) + return remote_bitbang_flush(); return ERROR_OK; } static int remote_bitbang_quit(void) { - if (remote_bitbang_putc('Q') == ERROR_FAIL) + if (remote_bitbang_queue('Q', true) == ERROR_FAIL) return ERROR_FAIL; if (close_socket(remote_bitbang_fd) != 0) { @@ -135,6 +154,9 @@ static bb_value_t char_to_int(int c) /* Get the next read response. */ static bb_value_t remote_bitbang_rread(void) { + if (remote_bitbang_flush() != ERROR_OK) + return ERROR_FAIL; + /* Enable blocking access. */ socket_block(remote_bitbang_fd); char c; @@ -154,7 +176,7 @@ static int remote_bitbang_sample(void) if (remote_bitbang_fill_buf() != ERROR_OK) return ERROR_FAIL; assert(!remote_bitbang_buf_full()); - return remote_bitbang_putc('R'); + return remote_bitbang_queue('R', false); } static bb_value_t remote_bitbang_read_sample(void) @@ -171,19 +193,19 @@ static bb_value_t remote_bitbang_read_sample(void) static int remote_bitbang_write(int tck, int tms, int tdi) { char c = '0' + ((tck ? 0x4 : 0x0) | (tms ? 0x2 : 0x0) | (tdi ? 0x1 : 0x0)); - return remote_bitbang_putc(c); + return remote_bitbang_queue(c, false); } static int remote_bitbang_reset(int trst, int srst) { char c = 'r' + ((trst ? 0x2 : 0x0) | (srst ? 0x1 : 0x0)); - return remote_bitbang_putc(c); + return remote_bitbang_queue(c, false); } static int remote_bitbang_blink(int on) { char c = on ? 'B' : 'b'; - return remote_bitbang_putc(c); + return remote_bitbang_queue(c, true); } static struct bitbang_interface remote_bitbang_bitbang = { From 6f0c6f23ceb0e699e0712b4a74069fd599912c87 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Thu, 13 May 2021 17:18:38 -0700 Subject: [PATCH 2/5] Fill the buffer before blocking on 1-byte read. Speeds up spike64-2 CheckMisa from 3.49s to 2.92s. Change-Id: Id4d4042b043f560ef90d58777d99cdcc41053b18 Signed-off-by: Tim Newsome --- src/jtag/drivers/remote_bitbang.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/jtag/drivers/remote_bitbang.c b/src/jtag/drivers/remote_bitbang.c index 90451a7d4..2e7892c24 100644 --- a/src/jtag/drivers/remote_bitbang.c +++ b/src/jtag/drivers/remote_bitbang.c @@ -181,6 +181,10 @@ static int remote_bitbang_sample(void) static bb_value_t remote_bitbang_read_sample(void) { + if (remote_bitbang_start == remote_bitbang_end) { + if (remote_bitbang_fill_buf() != ERROR_OK) + return ERROR_FAIL; + } if (remote_bitbang_start != remote_bitbang_end) { int c = remote_bitbang_buf[remote_bitbang_start]; remote_bitbang_start = From 0fa729942aec4ac2093114f7425ad2204f3565f3 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Fri, 14 May 2021 13:24:58 -0700 Subject: [PATCH 3/5] Rename existing buffer to recv_buf. Change-Id: I11105e7a962e498e04758eada0f6eb589f8fcfb7 --- src/jtag/drivers/remote_bitbang.c | 48 +++++++++++++++---------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/jtag/drivers/remote_bitbang.c b/src/jtag/drivers/remote_bitbang.c index 2e7892c24..ebcc0fb5c 100644 --- a/src/jtag/drivers/remote_bitbang.c +++ b/src/jtag/drivers/remote_bitbang.c @@ -40,15 +40,15 @@ static uint8_t remote_bitbang_send_buf[512]; static unsigned int remote_bitbang_send_buf_used; /* Circular buffer. When start == end, the buffer is empty. */ -static char remote_bitbang_buf[64]; -static unsigned remote_bitbang_start; -static unsigned remote_bitbang_end; +static char remote_bitbang_recv_buf[64]; +static unsigned remote_bitbang_recv_buf_start; +static unsigned remote_bitbang_recv_buf_end; static int remote_bitbang_buf_full(void) { - return remote_bitbang_end == - ((remote_bitbang_start + sizeof(remote_bitbang_buf) - 1) % - sizeof(remote_bitbang_buf)); + return remote_bitbang_recv_buf_end == + ((remote_bitbang_recv_buf_start + sizeof(remote_bitbang_recv_buf) - 1) % + sizeof(remote_bitbang_recv_buf)); } /* Read any incoming data, placing it into the buffer. */ @@ -57,22 +57,22 @@ static int remote_bitbang_fill_buf(void) socket_nonblock(remote_bitbang_fd); while (!remote_bitbang_buf_full()) { unsigned contiguous_available_space; - if (remote_bitbang_end >= remote_bitbang_start) { - contiguous_available_space = sizeof(remote_bitbang_buf) - - remote_bitbang_end; - if (remote_bitbang_start == 0) + if (remote_bitbang_recv_buf_end >= remote_bitbang_recv_buf_start) { + contiguous_available_space = sizeof(remote_bitbang_recv_buf) - + remote_bitbang_recv_buf_end; + if (remote_bitbang_recv_buf_start == 0) contiguous_available_space -= 1; } else { - contiguous_available_space = remote_bitbang_start - - remote_bitbang_end - 1; + contiguous_available_space = remote_bitbang_recv_buf_start - + remote_bitbang_recv_buf_end - 1; } ssize_t count = read_socket(remote_bitbang_fd, - remote_bitbang_buf + remote_bitbang_end, + remote_bitbang_recv_buf + remote_bitbang_recv_buf_end, contiguous_available_space); if (count > 0) { - remote_bitbang_end += count; - if (remote_bitbang_end == sizeof(remote_bitbang_buf)) - remote_bitbang_end = 0; + remote_bitbang_recv_buf_end += count; + if (remote_bitbang_recv_buf_end == sizeof(remote_bitbang_recv_buf)) + remote_bitbang_recv_buf_end = 0; } else if (count == 0) { return ERROR_OK; } else if (count < 0) { @@ -181,14 +181,14 @@ static int remote_bitbang_sample(void) static bb_value_t remote_bitbang_read_sample(void) { - if (remote_bitbang_start == remote_bitbang_end) { + if (remote_bitbang_recv_buf_start == remote_bitbang_recv_buf_end) { if (remote_bitbang_fill_buf() != ERROR_OK) return ERROR_FAIL; } - if (remote_bitbang_start != remote_bitbang_end) { - int c = remote_bitbang_buf[remote_bitbang_start]; - remote_bitbang_start = - (remote_bitbang_start + 1) % sizeof(remote_bitbang_buf); + if (remote_bitbang_recv_buf_start != remote_bitbang_recv_buf_end) { + int c = remote_bitbang_recv_buf[remote_bitbang_recv_buf_start]; + remote_bitbang_recv_buf_start = + (remote_bitbang_recv_buf_start + 1) % sizeof(remote_bitbang_recv_buf); return char_to_int(c); } return remote_bitbang_rread(); @@ -213,7 +213,7 @@ static int remote_bitbang_blink(int on) } static struct bitbang_interface remote_bitbang_bitbang = { - .buf_size = sizeof(remote_bitbang_buf) - 1, + .buf_size = sizeof(remote_bitbang_recv_buf) - 1, .sample = &remote_bitbang_sample, .read_sample = &remote_bitbang_read_sample, .write = &remote_bitbang_write, @@ -294,8 +294,8 @@ static int remote_bitbang_init(void) { bitbang_interface = &remote_bitbang_bitbang; - remote_bitbang_start = 0; - remote_bitbang_end = 0; + remote_bitbang_recv_buf_start = 0; + remote_bitbang_recv_buf_end = 0; LOG_INFO("Initializing remote_bitbang driver"); if (remote_bitbang_port == NULL) From 621e6e43d220db1fb11ce3075de229da00154942 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Fri, 14 May 2021 13:51:08 -0700 Subject: [PATCH 4/5] Also flush in execute_queue(). This improves CheckMisa time from 3.11s to 2.94s. (Why are both these times slower than in yesterday's commit message? I have no idea. Gremlins in my PC?) Change-Id: I05bd868b8aaf4220dca265bd494dfe889552716f Signed-off-by: Tim Newsome --- src/jtag/drivers/remote_bitbang.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/jtag/drivers/remote_bitbang.c b/src/jtag/drivers/remote_bitbang.c index ebcc0fb5c..214fa43fc 100644 --- a/src/jtag/drivers/remote_bitbang.c +++ b/src/jtag/drivers/remote_bitbang.c @@ -352,8 +352,19 @@ static const struct command_registration remote_bitbang_command_handlers[] = { COMMAND_REGISTRATION_DONE, }; +static int remote_bitbang_execute_queue(void) +{ + /* process the JTAG command queue */ + int ret = bitbang_execute_queue(); + if (ret != ERROR_OK) + return ret; + + /* flush not-yet-sent characters, if any */ + return remote_bitbang_flush(); +} + static struct jtag_interface remote_bitbang_interface = { - .execute_queue = &bitbang_execute_queue, + .execute_queue = &remote_bitbang_execute_queue, }; struct adapter_driver remote_bitbang_adapter_driver = { From 4e6fe83f8362c4292b6348b6f7bb599c375855f5 Mon Sep 17 00:00:00 2001 From: Tim Newsome Date: Mon, 17 May 2021 12:19:57 -0700 Subject: [PATCH 5/5] remote bitbang, flush resets. Waiting until the next execute_queue() might mean the target doesn't see the reset signal asserted for a significant amount of time. Change-Id: Id8514ddb30e88040131a6dba2b90b65463f10b76 Signed-off-by: Tim Newsome --- src/jtag/drivers/remote_bitbang.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/jtag/drivers/remote_bitbang.c b/src/jtag/drivers/remote_bitbang.c index 214fa43fc..b9ea7e4a1 100644 --- a/src/jtag/drivers/remote_bitbang.c +++ b/src/jtag/drivers/remote_bitbang.c @@ -203,7 +203,9 @@ static int remote_bitbang_write(int tck, int tms, int tdi) static int remote_bitbang_reset(int trst, int srst) { char c = 'r' + ((trst ? 0x2 : 0x0) | (srst ? 0x1 : 0x0)); - return remote_bitbang_queue(c, false); + /* Always flush the send buffer on reset, because the reset call need not be + * followed by jtag_execute_queue(). */ + return remote_bitbang_queue(c, true); } static int remote_bitbang_blink(int on) @@ -354,6 +356,10 @@ static const struct command_registration remote_bitbang_command_handlers[] = { static int remote_bitbang_execute_queue(void) { + /* safety: the send buffer must be empty, no leftover characters from + * previous transactions */ + assert(remote_bitbang_send_buf_used == 0); + /* process the JTAG command queue */ int ret = bitbang_execute_queue(); if (ret != ERROR_OK)