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

@@ -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");
 }
 

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[5] = 0xfe;
-	cmd[6] = request << 2;
+	cmd[6] = request;
-	cmd[7] = request >> 6;
+	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);
+		send_recv(info.usb_link, NULL, 0, res + 2 , 1);
-		if (res[0] != 0)
+		if (res[2] != 0)
 			raise_exception(EXCEPTION_ERROR, "Low access setup failed");
-		ack = res[1] >> 2;
+		ack = res[1] & 7;
-		ack &= 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 append 8 idle cycle (SWDIO = 0)!*/
-	/* Always prepend an 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;
+		cmd[2] = 33 + 8; /* 8 idle cycle  to move data through SW-DP */
-		memset(cmd + 4, 0xff, 5);
+		memset(cmd + 4, 0xff, 6);
-		cmd[ 9] = ((value <<  2) & 0xfc);
+		cmd[10] = ((value >>  0) & 0xff);
-		cmd[10] = ((value >>  6) & 0xff);
+		cmd[11] = ((value >>  8) & 0xff);
-		cmd[11] = ((value >> 14) & 0xff);
+		cmd[12] = ((value >> 16) & 0xff);
-		cmd[12] = ((value >> 22) & 0xff);
+		cmd[13] = ((value >> 24) & 0xff);
-		cmd[13] = ((value >> 30) & 0x03);
 		int bit_count  = __builtin_popcount(value);
-		cmd[13] |= ((bit_count & 1) ? 4 : 0);
+		cmd[14] = bit_count & 1;
-		send_recv(info.usb_link, cmd, 14, res, 5);
+		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");

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) | (parity << 4);
-				cmd[index++] = MS & 0x7f;
 				steps = 0;
 			}
 		}
 		cmd[index++] = MPSSE_TMS_SHIFT;
+		cmd[index++] = 4;
 		cmd[index++] = 0;
-		cmd[index++] = parity;
 		libftdi_buffer_write(cmd, index);
 	}
 }

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:
+		/* ARM Debug Interface Architecture Specification ADIv5.0 to ADIv5.2
-		 * Because of the asynchronous clock domains SWCLK and HCLK,
+		 * tells to clock the data through SW-DP to either :
-		 * two extra SWCLK cycles are needed after a write transaction
+		 * - immediate start a new transaction
-		 * (after the parity bit) to make the write effective
+		 * - continue to drive idle cycles
-		 * internally. These cycles should be applied while driving
+		 * - or clock at least 8 idle cycles
-		 * the line low (IDLE state)
+		 *
-		 * This is particularly important when writing the CTRL/STAT
+		 * Implement last option to favour correctness over
-		 * for a power-up request. If the next transaction (requiring
+		 *   slight speed decrease
-		 * a power-up) occurs immediately, it will fail.
 		 */
-		swd_proc.swdptap_seq_out(0, 2);
+		swd_proc.swdptap_seq_out(0, 8);
 	}
-
 	return response;
 }
 

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)