From bf548e92c08f755d645d35e1ceb580f956e5b186 Mon Sep 17 00:00:00 2001 From: Uwe Bonnes Date: Mon, 9 Nov 2020 21:36:06 +0100 Subject: [PATCH] swd: After write low_access, always append 8 clk to move data through SW-DP. Especially needed when leaving the debugger or during debug unit power-up. ARM Debug Interface Architecture Specification ADIv5.0 to ADIv5.2 tells to clock the data through SW-DP to either : - immediate start a new transaction - continue to drive idle cycles - or clock at least 8 idle cycles Implement last option to favour correctness over slight speed decrease Implement only for adapters where we assemble the seq_out_parity in our code, as on firmware, ftdi and jlink. Hopefully the high level adapters do it right. Reverts 2c33cde63fe779d3019fe8f63dd4420cb960bbfe and cde7726b8730242cd40a9974d129b46af80c68af --- src/platforms/hosted/jlink.c | 2 +- src/platforms/hosted/jlink_adiv5_swdp.c | 43 ++++++++++++------------- src/platforms/hosted/libftdi_swdptap.c | 22 +++++++++---- src/target/adiv5_swdp.c | 20 ++++++------ src/target/cortexm.c | 4 --- 5 files changed, 46 insertions(+), 45 deletions(-) diff --git a/src/platforms/hosted/jlink.c b/src/platforms/hosted/jlink.c index 9b95742c..7515de5a 100644 --- a/src/platforms/hosted/jlink.c +++ b/src/platforms/hosted/jlink.c @@ -90,7 +90,7 @@ static void jlink_print_interfaces(bmp_info_t *info) DEBUG_INFO(", %s available\n", (other_interface == JLINK_IF_SWD) ? "SWD": "JTAG"); else - DEBUG_WARN(", %s not available\n", + DEBUG_INFO(", %s not available\n", ((res[0] + 1) == JLINK_IF_SWD) ? "JTAG": "SWD"); } diff --git a/src/platforms/hosted/jlink_adiv5_swdp.c b/src/platforms/hosted/jlink_adiv5_swdp.c index 6895b9ad..090c1f5b 100644 --- a/src/platforms/hosted/jlink_adiv5_swdp.c +++ b/src/platforms/hosted/jlink_adiv5_swdp.c @@ -249,20 +249,19 @@ static uint32_t jlink_adiv5_swdp_low_access(ADIv5_DP_t *dp, uint8_t RnW, uint8_t res[8]; cmd[0] = CMD_HW_JTAG3; cmd[1] = 0; - cmd[2] = 13; + cmd[2] = (RnW) ? 11 : 13; /* Turnaround inserted automatically? */ cmd[3] = 0; cmd[4] = 0xff; - cmd[5] = 0xe3; - cmd[6] = request << 2; - cmd[7] = request >> 6; + cmd[5] = 0xfe; + cmd[6] = request; + cmd[7] = 0x00; platform_timeout_set(&timeout, 2000); do { send_recv(info.usb_link, cmd, 8, res, 2); - send_recv(info.usb_link, NULL, 0, res, 1); - if (res[0] != 0) + send_recv(info.usb_link, NULL, 0, res + 2 , 1); + if (res[2] != 0) raise_exception(EXCEPTION_ERROR, "Low access setup failed"); - ack = res[1] >> 2; - ack &= 7; + ack = res[1] & 7; } while (ack == SWDP_ACK_WAIT && !platform_timeout_is_expired(&timeout)); if (ack == SWDP_ACK_WAIT) raise_exception(EXCEPTION_TIMEOUT, "SWDP ACK timeout"); @@ -276,17 +275,15 @@ static uint32_t jlink_adiv5_swdp_low_access(ADIv5_DP_t *dp, uint8_t RnW, if(ack != SWDP_ACK_OK) { if (cl_debuglevel & BMP_DEBUG_TARGET) - DEBUG_WARN( "Protocol\n"); + DEBUG_WARN( "Protocol %d\n", ack); line_reset(&info); return 0; } - cmd[3] = 0; - /* Always prepend an idle cycle (SWDIO = 0)!*/ + /* Always append 8 idle cycle (SWDIO = 0)!*/ if(RnW) { memset(cmd + 4, 0, 10); - cmd[2] = 34; + cmd[2] = 33 + 2; /* 2 idle cycles */ cmd[8] = 0xfe; - cmd[13] = 0; send_recv(info.usb_link, cmd, 14, res, 5); send_recv(info.usb_link, NULL, 0, res + 5, 1); if (res[5] != 0) @@ -294,19 +291,19 @@ static uint32_t jlink_adiv5_swdp_low_access(ADIv5_DP_t *dp, uint8_t RnW, response = res[0] | res[1] << 8 | res[2] << 16 | res[3] << 24; int parity = res[4] & 1; int bit_count = __builtin_popcount (response) + parity; - if (bit_count & 1) /* Give up on parity error */ + if (bit_count & 1) /* Give up on parity error */ raise_exception(EXCEPTION_ERROR, "SWDP Parity error"); } else { - cmd[2] = 35; - memset(cmd + 4, 0xff, 5); - cmd[ 9] = ((value << 2) & 0xfc); - cmd[10] = ((value >> 6) & 0xff); - cmd[11] = ((value >> 14) & 0xff); - cmd[12] = ((value >> 22) & 0xff); - cmd[13] = ((value >> 30) & 0x03); + cmd[2] = 33 + 8; /* 8 idle cycle to move data through SW-DP */ + memset(cmd + 4, 0xff, 6); + cmd[10] = ((value >> 0) & 0xff); + cmd[11] = ((value >> 8) & 0xff); + cmd[12] = ((value >> 16) & 0xff); + cmd[13] = ((value >> 24) & 0xff); int bit_count = __builtin_popcount(value); - cmd[13] |= ((bit_count & 1) ? 4 : 0); - send_recv(info.usb_link, cmd, 14, res, 5); + cmd[14] = bit_count & 1; + cmd[15] = 0; + send_recv(info.usb_link, cmd, 16, res, 6); send_recv(info.usb_link, NULL, 0, res, 1); if (res[0] != 0) raise_exception(EXCEPTION_ERROR, "Low access write failed"); diff --git a/src/platforms/hosted/libftdi_swdptap.c b/src/platforms/hosted/libftdi_swdptap.c index 575b7138..99ee0068 100644 --- a/src/platforms/hosted/libftdi_swdptap.c +++ b/src/platforms/hosted/libftdi_swdptap.c @@ -361,9 +361,19 @@ static void swdptap_seq_out(uint32_t MS, int ticks) } } +/* ARM Debug Interface Architecture Specification ADIv5.0 to ADIv5.2 + * tells to clock the data through SW-DP to either : + * - immediate start a new transaction + * - continue to drive idle cycles + * - or clock at least 8 idle cycles + * + * Implement last option to favour correctness over + * slight speed decrease + */ static void swdptap_seq_out_parity(uint32_t MS, int ticks) { - int parity = __builtin_parity(MS & ((1LL << ticks) - 1)) & 1; + (void) ticks; + int parity = __builtin_parity(MS) & 1; unsigned int index = 0; swdptap_turnaround(SWDIO_STATUS_DRIVE); if (do_mpsse) { @@ -373,26 +383,26 @@ static void swdptap_seq_out_parity(uint32_t MS, int ticks) DI[2] = (MS >> 16) & 0xff; DI[3] = (MS >> 24) & 0xff; DI[4] = parity; - libftdi_jtagtap_tdi_tdo_seq(NULL, 0, DI, ticks + 1); + DI[5] = 0; + libftdi_jtagtap_tdi_tdo_seq(NULL, 0, DI, 32 + 1 + 8); } else { uint8_t cmd[32]; int steps = ticks; while (steps) { cmd[index++] = MPSSE_TMS_SHIFT; + cmd[index++] = 6; if (steps >= 7) { - cmd[index++] = 6; cmd[index++] = MS & 0x7f; MS >>= 7; steps -= 7; } else { - cmd[index++] = steps - 1; - cmd[index++] = MS & 0x7f; + cmd[index++] = (MS & 0x7f) | (parity << 4); steps = 0; } } cmd[index++] = MPSSE_TMS_SHIFT; + cmd[index++] = 4; cmd[index++] = 0; - cmd[index++] = parity; libftdi_buffer_write(cmd, index); } } diff --git a/src/target/adiv5_swdp.c b/src/target/adiv5_swdp.c index 4351dd17..33698560 100644 --- a/src/target/adiv5_swdp.c +++ b/src/target/adiv5_swdp.c @@ -169,19 +169,17 @@ uint32_t firmware_swdp_low_access(ADIv5_DP_t *dp, uint8_t RnW, raise_exception(EXCEPTION_ERROR, "SWDP Parity error"); } else { swd_proc.swdptap_seq_out_parity(value, 32); - /* RM0377 Rev. 8 Chapter 27.5.4 for STM32L0x1 states: - * Because of the asynchronous clock domains SWCLK and HCLK, - * two extra SWCLK cycles are needed after a write transaction - * (after the parity bit) to make the write effective - * internally. These cycles should be applied while driving - * the line low (IDLE state) - * This is particularly important when writing the CTRL/STAT - * for a power-up request. If the next transaction (requiring - * a power-up) occurs immediately, it will fail. + /* ARM Debug Interface Architecture Specification ADIv5.0 to ADIv5.2 + * tells to clock the data through SW-DP to either : + * - immediate start a new transaction + * - continue to drive idle cycles + * - or clock at least 8 idle cycles + * + * Implement last option to favour correctness over + * slight speed decrease */ - swd_proc.swdptap_seq_out(0, 2); + swd_proc.swdptap_seq_out(0, 8); } - return response; } diff --git a/src/target/cortexm.c b/src/target/cortexm.c index 517111fe..18f28abe 100644 --- a/src/target/cortexm.c +++ b/src/target/cortexm.c @@ -535,8 +535,6 @@ void cortexm_detach(target *t) target_mem_write32(t, CORTEXM_DEMCR, ap->ap_cortexm_demcr); /* Disable debug */ target_mem_write32(t, CORTEXM_DHCSR, CORTEXM_DHCSR_DBGKEY); - /* Add some clock cycles to get the CPU running again.*/ - target_mem_read32(t, 0); } enum { DB_DHCSR, DB_DCRSR, DB_DCRDR, DB_DEMCR }; @@ -836,8 +834,6 @@ static void cortexm_halt_resume(target *t, bool step) target_mem_write32(t, CORTEXM_ICIALLU, 0); target_mem_write32(t, CORTEXM_DHCSR, dhcsr); - /* Add some clock cycles to get the CPU running again.*/ - target_mem_read32(t, 0); } static int cortexm_fault_unwind(target *t)