From 91078943beb26cfbf68a83f0f2ea0253330ce44c Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 11 Feb 2023 12:11:58 +0000 Subject: [PATCH 1/9] common/jtagtap: No need to do 51 idle cycles instead of 50, and jtagtap_init()'s calling jtagtap_soft_reset() duplicates work --- src/platforms/common/jtagtap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/platforms/common/jtagtap.c b/src/platforms/common/jtagtap.c index ebdc87b016a..f34942348f3 100644 --- a/src/platforms/common/jtagtap.c +++ b/src/platforms/common/jtagtap.c @@ -48,10 +48,9 @@ void jtagtap_init(void) jtag_proc.tap_idle_cycles = 1; /* Ensure we're in JTAG mode */ - for (size_t i = 0; i <= 50U; ++i) + for (size_t i = 0; i < 50U; ++i) jtagtap_next(true, false); /* 50 idle cylces for SWD reset */ jtagtap_tms_seq(0xe73cU, 16U); /* SWD to JTAG sequence */ - jtagtap_soft_reset(); } static void jtagtap_reset(void) From b18d942a3e67def281d4ccc73dc07d2e2618225e Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 11 Feb 2023 12:12:34 +0000 Subject: [PATCH 2/9] hosted/remote_jtagtap.c: Fixed the BMDA remote protocol implementation not initialising jtag_proc.tap_idle_cycles, breaking things subtly --- src/platforms/hosted/remote_jtagtap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/platforms/hosted/remote_jtagtap.c b/src/platforms/hosted/remote_jtagtap.c index 5068cdd496a..f418f94bd1d 100644 --- a/src/platforms/hosted/remote_jtagtap.c +++ b/src/platforms/hosted/remote_jtagtap.c @@ -64,6 +64,7 @@ bool remote_jtagtap_init(void) jtag_proc.jtagtap_tms_seq = jtagtap_tms_seq; jtag_proc.jtagtap_tdi_tdo_seq = jtagtap_tdi_tdo_seq; jtag_proc.jtagtap_tdi_seq = jtagtap_tdi_seq; + jtag_proc.tap_idle_cycles = 1; platform_buffer_write((uint8_t *)REMOTE_HL_CHECK_STR, sizeof(REMOTE_HL_CHECK_STR)); length = platform_buffer_read((uint8_t *)buffer, REMOTE_MAX_MSG_SIZE); From 0540c1de4754b0801ad3605bb52393b391cad3a3 Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 11 Feb 2023 12:13:07 +0000 Subject: [PATCH 3/9] jtag_scan: Fixed jtag_dev_write_ir() doing naughty things with numbers --- src/target/jtag_scan.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/target/jtag_scan.c b/src/target/jtag_scan.c index bfe08660639..2ed50afaa3b 100644 --- a/src/target/jtag_scan.c +++ b/src/target/jtag_scan.c @@ -234,13 +234,16 @@ uint32_t jtag_scan(const uint8_t *irlens) void jtag_dev_write_ir(const uint8_t dev_index, const uint32_t ir) { jtag_dev_s *device = &jtag_devs[dev_index]; + /* If the request would duplicate work already done, do nothing */ if (ir == device->current_ir) return; + /* Set all the other devices IR's to being in bypass */ for (size_t device = 0; device < jtag_dev_count; device++) - jtag_devs[device].current_ir = -1; + jtag_devs[device].current_ir = UINT32_MAX; device->current_ir = ir; + /* Do the work to make the scanchain match the jtag_devs state */ jtagtap_shift_ir(); jtag_proc.jtagtap_tdi_seq(false, ones, device->ir_prescan); jtag_proc.jtagtap_tdi_seq(!device->ir_postscan, (const uint8_t *)&ir, device->ir_len); From 9df12b4411b1a17975fb8e882d14d638d4f503dc Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 11 Feb 2023 12:13:38 +0000 Subject: [PATCH 4/9] jtag_scan: jtag_proc.tap_idle_cycles will always be 1 during scan, so don't overcomplicate things --- src/target/jtag_scan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/jtag_scan.c b/src/target/jtag_scan.c index 2ed50afaa3b..9c8dced89d1 100644 --- a/src/target/jtag_scan.c +++ b/src/target/jtag_scan.c @@ -194,7 +194,7 @@ uint32_t jtag_scan(const uint8_t *irlens) } DEBUG_INFO("Return to Run-Test/Idle\n"); jtag_proc.jtagtap_next(true, true); - jtagtap_return_idle(jtag_proc.tap_idle_cycles); + jtagtap_return_idle(1); #if PC_HOSTED == 1 /*Transfer needed device information to firmware jtag_devs */ From f84177239c8a9c3e684defbaaadd59347902494b Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 11 Feb 2023 12:14:10 +0000 Subject: [PATCH 5/9] jtag_scan: Fixed another instance of using integers as booleans for jtagtap_next() --- src/target/jtag_scan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/target/jtag_scan.c b/src/target/jtag_scan.c index 9c8dced89d1..dd29559a7bc 100644 --- a/src/target/jtag_scan.c +++ b/src/target/jtag_scan.c @@ -151,7 +151,7 @@ uint32_t jtag_scan(const uint8_t *irlens) } DEBUG_INFO("Return to Run-Test/Idle\n"); - jtag_proc.jtagtap_next(1, 1); + jtag_proc.jtagtap_next(true, true); jtagtap_return_idle(1); /* All devices should be in BYPASS now */ From 40502625ddff806b2e168e7537acc724e33cb132 Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 11 Feb 2023 12:32:10 +0000 Subject: [PATCH 6/9] common/jtagtap: Retimed jtagtap_tdi_seq_no_delay() as the timings were far too narrow and it was causing clock glitch pulses --- src/platforms/common/jtagtap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/platforms/common/jtagtap.c b/src/platforms/common/jtagtap.c index f34942348f3..6e4427ad36d 100644 --- a/src/platforms/common/jtagtap.c +++ b/src/platforms/common/jtagtap.c @@ -230,6 +230,7 @@ static void jtagtap_tdi_seq_swd_delay(const uint8_t *const data_in, const bool f static void jtagtap_tdi_seq_no_delay(const uint8_t *const data_in, const bool final_tms, size_t clock_cycles) { for (size_t cycle = 0; cycle < clock_cycles; ++cycle) { + gpio_clear(TCK_PORT, TCK_PIN); const size_t bit = cycle & 7U; const size_t byte = cycle >> 3U; /* On the last tick, assert final_tms to TMS_PIN */ @@ -238,8 +239,10 @@ static void jtagtap_tdi_seq_no_delay(const uint8_t *const data_in, const bool fi gpio_set_val(TDI_PORT, TDI_PIN, data_in[byte] & (1U << bit)); gpio_set(TCK_PORT, TCK_PIN); /* Finish the clock cycle */ - gpio_clear(TCK_PORT, TCK_PIN); } + __asm__("nop"); + __asm__("nop"); + gpio_clear(TCK_PORT, TCK_PIN); } static void jtagtap_tdi_seq(const bool final_tms, const uint8_t *const data_in, const size_t clock_cycles) From a4abf0347ab3fbd9a3519492a08cbb12adba8a16 Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 11 Feb 2023 14:20:26 +0000 Subject: [PATCH 7/9] hosted/remote_jtagtap: Fixed a refactoring mistake that had final_tms handling in jtagtap_tdi_tdo_seq() broken, and added lots of documentation on what's going on --- src/platforms/hosted/remote_jtagtap.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/platforms/hosted/remote_jtagtap.c b/src/platforms/hosted/remote_jtagtap.c index f418f94bd1d..fe32ee22211 100644 --- a/src/platforms/hosted/remote_jtagtap.c +++ b/src/platforms/hosted/remote_jtagtap.c @@ -117,25 +117,36 @@ static void jtagtap_tdi_tdo_seq( char buffer[REMOTE_MAX_MSG_SIZE]; size_t in_offset = 0; size_t out_offset = 0; + char tms_state = REMOTE_TDITDO_NOTMS; + /* Loop through the data to send/receive and handle it in chunks of up to 64 bits */ for (size_t cycle = 0; cycle < clock_cycles;) { - size_t chunk; + /* Calculate how many bits need to be in this chunk, capped at 64 */ + size_t chunk = 64; if (clock_cycles - cycle <= 64) chunk = clock_cycles - cycle; - else - chunk = 64; cycle += chunk; + /* If the result would complete the transaction, check if TMS needs to be high at the end */ + if (cycle == clock_cycles && final_tms) + /* If it does, we then need to tell the firmware this */ + tms_state = REMOTE_TDITDO_TMS; + /* Build a representation of the data to send safely */ uint64_t data = 0; const size_t bytes = (chunk + 7U) >> 3U; if (data_in) { - for (size_t i = 0; i < bytes; ++i) - data |= data_in[in_offset++] << (i * 8U); + for (size_t idx = 0; idx < bytes; ++idx, ++in_offset) + data |= data_in[in_offset] << (idx * 8U); } - /* PRIx64 differs with system. Use it explicit in the format string*/ - int length = snprintf(buffer, REMOTE_MAX_MSG_SIZE, "!J%c%02zx%" PRIx64 "%c", - !clock_cycles && final_tms ? REMOTE_TDITDO_TMS : REMOTE_TDITDO_NOTMS, chunk, data, REMOTE_EOM); + /* + * Build the remote protocol message to send, and send it. + * This uses its own copy of the REMOTE_JTAG_TDIDO_STR to correct for how + * formatting a uint64_t is platform-specific. + */ + int length = + snprintf(buffer, REMOTE_MAX_MSG_SIZE, "!J%c%02zx%" PRIx64 "%c", tms_state, chunk, data, REMOTE_EOM); platform_buffer_write((uint8_t *)buffer, length); + /* Receive the response and check if it's an error response */ length = platform_buffer_read((uint8_t *)buffer, REMOTE_MAX_MSG_SIZE); if (!length || buffer[0] == REMOTE_RESP_ERR) { DEBUG_WARN("jtagtap_tdi_tdo_seq failed, error %s\n", length ? buffer + 1 : "unknown"); From b0f9876e9748512a17df055f7abbef3c01afc9b0 Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sat, 11 Feb 2023 16:44:33 +0000 Subject: [PATCH 8/9] common/jtagtap: Completely re-timed jtagtap_tdi_tdo_seq_no_delay() to fix a setup-and-hold timing violation --- src/platforms/common/jtagtap.c | 35 +++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/platforms/common/jtagtap.c b/src/platforms/common/jtagtap.c index 6e4427ad36d..513ecb91607 100644 --- a/src/platforms/common/jtagtap.c +++ b/src/platforms/common/jtagtap.c @@ -168,33 +168,42 @@ static void jtagtap_tdi_tdo_seq_swd_delay( data_out[byte] = value; } +static inline void jtag_next_tms_tdi(const bool tms, const bool tdi) +{ + /* Initiate the falling edge on the bus */ + gpio_clear(TCK_PORT, TCK_PIN); + /* It is now safe to change TMS and TDI */ + gpio_set_val(TMS_PORT, TMS_PIN, tms); + gpio_set_val(TDI_PORT, TDI_PIN, tdi); +} + static void jtagtap_tdi_tdo_seq_no_delay( const uint8_t *const data_in, uint8_t *const data_out, const bool final_tms, const size_t clock_cycles) { uint8_t value = 0; - for (size_t cycle = 0; cycle < clock_cycles; ++cycle) { - const size_t bit = cycle & 7U; + for (size_t cycle = 0; cycle < clock_cycles;) { + /* Calculate the next bit and byte to consume data from */ + const uint8_t bit = cycle & 7U; const size_t byte = cycle >> 3U; - /* On the last tick, assert final_tms to TMS_PIN */ - gpio_set_val(TMS_PORT, TMS_PIN, cycle + 1U >= clock_cycles && final_tms); - /* Set up the TDI pin and start the clock cycle */ - gpio_set_val(TDI_PORT, TDI_PIN, data_in[byte] & (1U << bit)); + /* Configure the bus for the next cycle */ + jtag_next_tms_tdi(cycle + 1U >= clock_cycles && final_tms, data_in[byte] & (1U << bit)); + /* Increment the cycle counter */ + ++cycle; + /* Block the compiler from re-ordering the calculations to preserve timings */ + __asm__ volatile("" ::: "memory"); + /* Start the clock cycle */ gpio_set(TCK_PORT, TCK_PIN); /* If TDO is high, store a 1 in the appropriate position in the value being accumulated */ if (gpio_get(TDO_PORT, TDO_PIN)) - value |= (1 << bit); - /* If we've got to the next whole byte, store the accumulated value and reset state */ + value |= 1U << bit; + /* If we've got the next whole byte, store the accumulated value and reset state */ if (bit == 7U) { data_out[byte] = value; value = 0; } /* Finish the clock cycle */ - gpio_clear(TCK_PORT, TCK_PIN); } - const size_t bit = (clock_cycles - 1U) & 7U; - const size_t byte = (clock_cycles - 1U) >> 3U; - if (bit) - data_out[byte] = value; + gpio_clear(TCK_PORT, TCK_PIN); } static void jtagtap_tdi_tdo_seq( From e88bff2e201f8240ebc443461c536adc87ea078c Mon Sep 17 00:00:00 2001 From: dragonmux Date: Sun, 12 Feb 2023 12:17:42 +0000 Subject: [PATCH 9/9] jtag_scan: Made the `ones` array available for use by the RISC-V and AVR support branches --- src/target/jtag_scan.c | 2 +- src/target/jtag_scan.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/target/jtag_scan.c b/src/target/jtag_scan.c index dd29559a7bc..68be0bd0de3 100644 --- a/src/target/jtag_scan.c +++ b/src/target/jtag_scan.c @@ -35,7 +35,7 @@ jtag_dev_s jtag_devs[JTAG_MAX_DEVS + 1U]; uint32_t jtag_dev_count = 0; /* bucket of ones for don't care TDI */ -static const uint8_t ones[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; +const uint8_t ones[8] = {0xffU, 0xffU, 0xffU, 0xffU, 0xffU, 0xffU, 0xffU, 0xffU}; #if PC_HOSTED == 0 void jtag_add_device(const uint32_t dev_index, const jtag_dev_s *jtag_dev) diff --git a/src/target/jtag_scan.h b/src/target/jtag_scan.h index bafc3046416..3e737f0f575 100644 --- a/src/target/jtag_scan.h +++ b/src/target/jtag_scan.h @@ -46,6 +46,7 @@ typedef struct jtag_dev { extern jtag_dev_s jtag_devs[JTAG_MAX_DEVS + 1U]; extern uint32_t jtag_dev_count; +extern const uint8_t ones[8]; void jtag_dev_write_ir(uint8_t jd_index, uint32_t ir); void jtag_dev_shift_dr(uint8_t jd_index, uint8_t *dout, const uint8_t *din, size_t ticks);