From 75aa8228e866259bf61a05328444d3923dca29f5 Mon Sep 17 00:00:00 2001
From: Taylor Yu <code@argon.blue>
Date: Wed, 17 Jan 2024 18:11:52 -0600
Subject: [PATCH] common/swdptap: adjust timing

Make clock cycle more symmetric, especially for `no_delay` variants.

`swdptap_seq_out_no_delay` seems to need the clock clear to be moved
back to the top of the loop, to ensure minimal delay between the falling
edge and the SWDIO change.

Also use a different loop counter that compiles to a `SUBS` `BCC`/`BCS`
pair, instead of using an explicit `CMP`.
---
 src/platforms/common/swdptap.c | 64 ++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/src/platforms/common/swdptap.c b/src/platforms/common/swdptap.c
index f84e303dfb0..93b55c74e40 100644
--- a/src/platforms/common/swdptap.c
+++ b/src/platforms/common/swdptap.c
@@ -99,15 +99,24 @@ static uint32_t swdptap_seq_in_clk_delay(size_t clock_cycles) __attribute__((opt
 static uint32_t swdptap_seq_in_clk_delay(const size_t clock_cycles)
 {
 	uint32_t value = 0;
-	for (size_t cycle = 0; cycle < clock_cycles; ++cycle) {
+	if (!clock_cycles)
+		return 0;
+	for (size_t cycle = clock_cycles; cycle--;) {
 		for (volatile uint32_t counter = target_clk_divider; counter > 0; --counter)
 			continue;
-		value |= gpio_get(SWDIO_IN_PORT, SWDIO_IN_PIN) ? 1U << cycle : 0U;
+		bool bit = gpio_get(SWDIO_IN_PORT, SWDIO_IN_PIN);
 		gpio_set(SWCLK_PORT, SWCLK_PIN);
 		for (volatile uint32_t counter = target_clk_divider; counter > 0; --counter)
 			continue;
+		value >>= 1U;
+		value |= (uint32_t)bit << 31U;
+		/* Reordering barrier */
+		__asm__("" ::: "memory");
 		gpio_clear(SWCLK_PORT, SWCLK_PIN);
+		/* Reordering barrier */
+		__asm__("" ::: "memory");
 	}
+	value >>= (32U - clock_cycles);
 	return value;
 }
 
@@ -115,13 +124,24 @@ static uint32_t swdptap_seq_in_no_delay(size_t clock_cycles) __attribute__((opti
 
 static uint32_t swdptap_seq_in_no_delay(const size_t clock_cycles)
 {
+	if (!clock_cycles)
+		return 0;
 	uint32_t value = 0;
-	for (size_t cycle = 0; cycle < clock_cycles; ++cycle) {
-		value |= gpio_get(SWDIO_IN_PORT, SWDIO_IN_PIN) ? 1U << cycle : 0U;
+	for (size_t cycle = clock_cycles; cycle--;) {
+		/* Reordering barrier */
+		__asm__("" ::: "memory");
+		bool bit = gpio_get(SWDIO_IN_PORT, SWDIO_IN_PIN);
 		gpio_set(SWCLK_PORT, SWCLK_PIN);
-		__asm__("nop");
+		__asm__("nop" ::: "memory");
+		value >>= 1U;
+		value |= (uint32_t)bit << 31U;
+		/* Reordering barrier */
+		__asm__("" ::: "memory");
 		gpio_clear(SWCLK_PORT, SWCLK_PIN);
+		/* Reordering barrier */
+		__asm__("" ::: "memory");
 	}
+	value >>= (32U - clock_cycles);
 	return value;
 }
 
@@ -159,13 +179,24 @@ static void swdptap_seq_out_clk_delay(uint32_t tms_states, size_t clock_cycles)
 
 static void swdptap_seq_out_clk_delay(const uint32_t tms_states, const size_t clock_cycles)
 {
-	for (size_t cycle = 0; cycle < clock_cycles; ++cycle) {
-		gpio_set_val(SWDIO_PORT, SWDIO_PIN, tms_states & (1U << cycle));
+	uint32_t value = tms_states;
+	bool bit = value & 1;
+	if (!clock_cycles)
+		return;
+	for (size_t cycle = clock_cycles; cycle--;) {
+		/* Reordering barrier */
+		__asm__("" ::: "memory");
+		gpio_set_val(SWDIO_PORT, SWDIO_PIN, bit);
 		for (volatile uint32_t counter = target_clk_divider; counter > 0; --counter)
 			continue;
 		gpio_set(SWCLK_PORT, SWCLK_PIN);
 		for (volatile uint32_t counter = target_clk_divider; counter > 0; --counter)
 			continue;
+		__asm__("nop" ::: "memory");
+		value >>= 1U;
+		bit = value & 1;
+		/* Reordering barrier */
+		__asm__("" ::: "memory");
 		gpio_clear(SWCLK_PORT, SWCLK_PIN);
 	}
 }
@@ -174,12 +205,23 @@ static void swdptap_seq_out_no_delay(uint32_t tms_states, size_t clock_cycles) _
 
 static void swdptap_seq_out_no_delay(const uint32_t tms_states, const size_t clock_cycles)
 {
-	for (size_t cycle = 0; cycle < clock_cycles; ++cycle) {
-		gpio_set_val(SWDIO_PORT, SWDIO_PIN, tms_states & (1U << cycle));
-		gpio_set(SWCLK_PORT, SWCLK_PIN);
-		__asm__("nop");
+	uint32_t value = tms_states;
+	bool bit = value & 1;
+	if (!clock_cycles)
+		return;
+	for (size_t cycle = clock_cycles; cycle--;) {
+		/* Reordering barrier */
+		__asm__("" ::: "memory");
 		gpio_clear(SWCLK_PORT, SWCLK_PIN);
+		gpio_set_val(SWDIO_PORT, SWDIO_PIN, bit);
+		gpio_set(SWCLK_PORT, SWCLK_PIN);
+		__asm__("nop" ::: "memory");
+		value >>= 1U;
+		bit = value & 1;
+		/* Reordering barrier */
+		__asm__("" ::: "memory");
 	}
+	gpio_clear(SWCLK_PORT, SWCLK_PIN);
 }
 
 static void swdptap_seq_out(const uint32_t tms_states, const size_t clock_cycles)