Update probe_oen.pio to use the same logic as the new probe.pio.

Fix a couple of compilation issues in the helpers for probe_oen.pio.

include/board_pico_config.h

@@ -35,7 +35,7 @@
 #define PROBE_PIN_SWCLK (PROBE_PIN_OFFSET + 0) // 2
 #define PROBE_PIN_SWDIO (PROBE_PIN_OFFSET + 1) // 3
 // Target reset config
-#define PROBE_PIN_RESET 1
+#define PROBE_PIN_RESET 0
 
 // UART config
 #define PICOPROBE_UART_TX 4

src/cdc_uart.c

@@ -139,7 +139,7 @@ void tud_cdc_line_coding_cb(uint8_t itf, cdc_line_coding_t const* line_coding)
   vTaskSuspend(uart_taskhandle);
   interval = MAX(1, micros / ((1000 * 1000) / configTICK_RATE_HZ));
   debounce_ticks = MAX(1, configTICK_RATE_HZ / (interval * DEBOUNCE_MS));
-  picoprobe_info("New baud rate %ld micros %ld interval %lu\n",
+  picoprobe_info("New baud rate %d micros %d interval %u\n",
                   line_coding->bit_rate, micros, interval);
   uart_deinit(PICOPROBE_UART_INTERFACE);
   tud_cdc_write_clear();

src/main.c

@@ -99,7 +99,6 @@ int main(void) {
     tusb_init();
 
     DAP_Setup();
-    stdio_uart_init();
 
     led_init();
 

src/picoprobe_config.h

@@ -26,39 +26,21 @@
 #ifndef PICOPROBE_H_
 #define PICOPROBE_H_
 
-#include "FreeRTOS.h"
-#include "task.h"
-
 #if false
-#define picoprobe_info(format,args...) \
+#define picoprobe_info(format,args...) printf(format, ## args)
-do { \
-	vTaskSuspendAll(); \
-	printf(format, ## args); \
-	xTaskResumeAll(); \
-} while (0)
 #else
 #define picoprobe_info(format,...) ((void)0)
 #endif
 
 
 #if false
-#define picoprobe_debug(format,args...) \
+#define picoprobe_debug(format,args...) printf(format, ## args)
-do { \
-	vTaskSuspendAll(); \
-	printf(format, ## args); \
-	xTaskResumeAll(); \
-} while (0)
 #else
 #define picoprobe_debug(format,...) ((void)0)
 #endif
 
 #if false
-#define picoprobe_dump(format,args...)\
+#define picoprobe_dump(format,args...) printf(format, ## args)
-do { \
-	vTaskSuspendAll(); \
-	printf(format, ## args); \
-	xTaskResumeAll(); \
-} while (0)
 #else
 #define picoprobe_dump(format,...) ((void)0)
 #endif

src/probe.c

@@ -62,8 +62,9 @@ static struct _probe probe;
 void probe_set_swclk_freq(uint freq_khz) {
         uint clk_sys_freq_khz = clock_get_hz(clk_sys) / 1000;
         picoprobe_info("Set swclk freq %dKHz sysclk %dkHz\n", freq_khz, clk_sys_freq_khz);
-        // Worked out with saleae
+        uint32_t divider = clk_sys_freq_khz / freq_khz / 4;
-        uint32_t divider = clk_sys_freq_khz / freq_khz / 2;
+        if (divider == 0)
+            divider = 1;
         pio_sm_set_clkdiv_int_frac(pio0, PROBE_SM, divider, 0);
 }
 
@@ -75,21 +76,39 @@ void probe_assert_reset(bool state)
 #endif
 }
 
+typedef enum probe_pio_command {
+    CMD_WRITE = 0,
+    CMD_SKIP,
+    CMD_TURNAROUND,
+    CMD_READ
+} probe_pio_command_t;
+
+static inline uint32_t fmt_probe_command(uint bit_count, bool out_en, probe_pio_command_t cmd) {
+    uint cmd_addr =
+        cmd == CMD_WRITE      ? probe.offset + probe_offset_write_cmd :
+        cmd == CMD_SKIP       ? probe.offset + probe_offset_get_next_cmd :
+        cmd == CMD_TURNAROUND ? probe.offset + probe_offset_turnaround_cmd :
+                                probe.offset + probe_offset_read_cmd;
+    return ((bit_count - 1) & 0xff) | ((uint)out_en << 8) | (cmd_addr << 9);
+}
+
 void probe_write_bits(uint bit_count, uint32_t data_byte) {
     DEBUG_PINS_SET(probe_timing, DBG_PIN_WRITE);
-    pio_sm_put_blocking(pio0, PROBE_SM, bit_count - 1);
+    pio_sm_put_blocking(pio0, PROBE_SM, fmt_probe_command(bit_count, true, CMD_WRITE));
     pio_sm_put_blocking(pio0, PROBE_SM, data_byte);
-    DEBUG_PINS_SET(probe_timing, DBG_PIN_WRITE_WAIT);
     picoprobe_dump("Write %d bits 0x%x\n", bit_count, data_byte);
-    // Wait for pio to push garbage to rx fifo so we know it has finished sending
+    // Return immediately so we can cue up the next command whilst this one runs
-    pio_sm_get_blocking(pio0, PROBE_SM);
-    DEBUG_PINS_CLR(probe_timing, DBG_PIN_WRITE_WAIT);
     DEBUG_PINS_CLR(probe_timing, DBG_PIN_WRITE);
 }
 
+void probe_hiz_clocks(uint bit_count) {
+    pio_sm_put_blocking(pio0, PROBE_SM, fmt_probe_command(bit_count, false, CMD_TURNAROUND));
+    pio_sm_put_blocking(pio0, PROBE_SM, 0);
+}
+
 uint32_t probe_read_bits(uint bit_count) {
     DEBUG_PINS_SET(probe_timing, DBG_PIN_READ);
-    pio_sm_put_blocking(pio0, PROBE_SM, bit_count - 1);
+    pio_sm_put_blocking(pio0, PROBE_SM, fmt_probe_command(bit_count, false, CMD_READ));
     uint32_t data = pio_sm_get_blocking(pio0, PROBE_SM);
     uint32_t data_shifted = data;
     if (bit_count < 32) {
@@ -101,14 +120,20 @@ uint32_t probe_read_bits(uint bit_count) {
     return data_shifted;
 }
 
+static void probe_wait_idle() {
+    pio0->fdebug = 1u << (PIO_FDEBUG_TXSTALL_LSB + PROBE_SM);
+    while (!(pio0->fdebug & (1u << (PIO_FDEBUG_TXSTALL_LSB + PROBE_SM))))
+        ;
+}
+
 void probe_read_mode(void) {
-    pio_sm_exec(pio0, PROBE_SM, pio_encode_jmp(probe.offset + probe_offset_in_posedge));
+    pio_sm_put_blocking(pio0, PROBE_SM, fmt_probe_command(0, false, CMD_SKIP));
-    while(pio_sm_get_pc(pio0, PROBE_SM) != probe.offset + probe_offset_in_idle);
+    probe_wait_idle();
 }
 
 void probe_write_mode(void) {
-    pio_sm_exec(pio0, PROBE_SM, pio_encode_jmp(probe.offset + probe_offset_out_negedge));
+    pio_sm_put_blocking(pio0, PROBE_SM, fmt_probe_command(0, true, CMD_SKIP));
-    while(pio_sm_get_pc(pio0, PROBE_SM) != probe.offset + probe_offset_out_idle);
+    probe_wait_idle();
 }
 
 void probe_init() {
@@ -123,13 +148,11 @@ void probe_init() {
         // Set up divisor
         probe_set_swclk_freq(1000);
 
-        // Enable SM
+        // Jump SM to command dispatch routine, and enable it
+        pio_sm_exec(pio0, PROBE_SM, offset + probe_offset_get_next_cmd);
         pio_sm_set_enabled(pio0, PROBE_SM, 1);
         probe.initted = 1;
     }
-
-    // Jump to write program
-    probe_write_mode();
 }
 
 void probe_deinit(void)

src/probe.h

@@ -35,8 +35,11 @@
 #endif
 
 void probe_set_swclk_freq(uint freq_khz);
+
+// Bit counts in the range 1..256
 void probe_write_bits(uint bit_count, uint32_t data_byte);
 uint32_t probe_read_bits(uint bit_count);
+void probe_hiz_clocks(uint bit_count);
 
 void probe_read_mode(void);
 void probe_write_mode(void);

src/probe.pio

@@ -1,7 +1,7 @@
 /*
  * The MIT License (MIT)
  *
- * Copyright (c) 2021 Raspberry Pi (Trading) Ltd.
+ * Copyright (c) 2021-2023 Raspberry Pi (Trading) Ltd.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -23,32 +23,50 @@
  *
  */
 
+// Every TX FIFO entry is either a command, or up to 32 bits of data.
+// Command format:
+//
+// | 13:9 |  8  |  7:0  |
+// | Cmd  | Dir | Count |
+//
+// Count is the number of bits to be transferred by this command, minus 1.
+// Dir is the output enable for the SWDIO pin.
+// Cmd is the address of the write_cmd, read_cmd or get_next_cmd label.
+//
+// write_cmd expects a FIFO data entry, but read_cmd does not.
+//
+// read_cmd pushes data to the FIFO, but write_cmd does not. (The lack of RX
+// garbage on writes allows the interface code to return early after pushing a
+// write command, as there is no need in general to poll for a command's
+// completion as long as all commands are executed in order.)
+//
+// The SWCLK period is 4 PIO SM execution cycles.
+
 .program probe
 .side_set 1 opt
 
-public out_negedge:
+public write_cmd:
-    set pindirs, 1 side 0x0 ; Init OE clock 0
+public turnaround_cmd:                      ; Alias of write, used for probe_oen.pio
-public out_idle:
+    pull
-    pull           ; Pull number of bits to shift -1 from tx fifo and put into output shift register
+write_bitloop:
-    mov x, osr     ; mov bits to shift -1 from output shift register into x
+    out pins, 1             [1]  side 0x0   ; Data is output by host on negedge
-    pull           ; Pull data to shift out
+    jmp x-- write_bitloop   [1]  side 0x1   ; ...and captured by target on posedge
-out_negedge_bitloop:
+                                            ; Fall through to next command
-    out pins, 1                  side 0x0 ; clock data out on falling edge
+.wrap_target
-    jmp  x-- out_negedge_bitloop side 0x1 ; data is present for posedge
+public get_next_cmd:
-    set pins, 1                  side 0x0 ; Drive data high (idle bus state)
+    pull                         side 0x0   ; SWCLK is initially low
-    push                                 ; Push to rx fifo just so processor knows when done
+    out x, 8                                ; Get bit count
-    jmp out_negedge                      ; Wait for next transaction
+    out pindirs, 1                          ; Set SWDIO direction
+    out pc, 5                               ; Go to command routine
+
+read_bitloop:
+    nop                                     ; Additional delay on taken loop branch
+public read_cmd:
+    in pins, 1              [1]  side 0x1   ; Data is captured by host on posedge
+    jmp x-- read_bitloop         side 0x0
+    push
+.wrap                                       ; Wrap to next command
 
-public in_posedge:
-    set pindirs, 0 side 0x0 ; INIT IE clock 0
-public in_idle:
-    pull                   ; Pull number of bits to shift -1 from tx fifo and put into output shift register
-    mov x, osr             ; mov bits to shift -1 from output shift register into x into x
-in_posedge_bitloop:
-    in pins, 1                        side 0x1 ; Generate posedge and read data
-    jmp x-- in_posedge_bitloop        side 0x0 ;
-    push                                      ; Push to rx fifo when done
-    jmp in_posedge                            ; Jump back to start
 
 ; Implement probe_gpio_init() and probe_sm_init() methods here - set pins, offsets, sidesets etc
 % c-sdk {

src/probe_oen.pio

@@ -1,39 +1,48 @@
 ; Output-enable active-low variant of the SWD probe
 
+; This program is very similar to the one in probe.pio. The only difference is
+; that here write_cmd and turnaround_cmd are split into two separate routines,
+; whose difference is OEn being high/low.
+
+; SWDIO_OEn is pin 0, SWCLK pin 1, SWDIO (out) pin 2, SWDI (in) pin 3.
+; Pin 0 and 1 are sideset pins
+
 .program probe
 .side_set 2 opt
 
-; SWDIO_OEN is pin 0, SWCLK pin 1, SWDIO (out) pin 2, SWDI (in) pin 3.
+public turnaround_cmd:
-; Pin 0 and 1 are sideset pins
+    pull
-public out_negedge:
+turnaround_bitloop:
-  set pindirs, 0x1 side 0x0    ; OE_N 0, data high, clock 0
+    nop                         [1]  side 0x1
-public out_idle:
+    jmp x-- turnaround_bitloop  [1]  side 0x3
-  pull                         ; pull nbits - 1
+    jmp get_next_cmd
-  mov x, osr
-  pull                         ; pull data
-public out_negedge_bitloop:
-  out pins, 1                  side 0x0
-  jmp x-- out_negedge_bitloop  side 0x2  ; OE_N 0, clock high
-  set pins, 1                  side 0x0  ; drive data high (idle bus state)
-  push                         ; Push to rx fifo just so processor knows when done
-  jmp out_negedge              ; Wait for next transaction
 
-public in_posedge:
+public write_cmd:
-  set pindirs, 0x0            side 0x1 ; OE_N 1, data high, clock 0
+    pull
-public in_idle:
+write_bitloop:
-  pull
+    out pins, 1                 [1]  side 0x0   ; Data is output by host on negedge
-  mov x, osr
+    jmp x-- write_bitloop       [1]  side 0x2   ; ...and captured by target on posedge
-in_posedge_bitloop:
+                                                ; Fall through to next command
-  in pins, 1                  side 0x1 ; OE_N 1, clock 0
+.wrap_target
-  jmp x-- in_posedge_bitloop  side 0x3 ; OE_N 1, clock 1
+public get_next_cmd:
-  push
+    pull                             side 0x1   ; SWCLK initially low, OEn disabled
-  jmp in_posedge
+    out x, 8                                    ; Get bit count
+    out pindirs, 1                              ; Set SWDIO direction
+    out pc, 5                                   ; Go to command routine
+
+read_bitloop:
+    nop                                         ; Additional delay on taken loop branch
+public read_cmd:
+    in pins, 1                  [1]  side 0x3   ; Data is captured by host on posedge
+    jmp x-- read_bitloop             side 0x1
+    push
+.wrap                                           ; Wrap to next command
 
 
 ; Implement probe_gpio_init() and probe_sm_init() methods here - set pins, offsets, sidesets etc
 % c-sdk {
 
-void probe_gpio_init()
+static inline void probe_gpio_init()
 {
 #if defined(PROBE_PIN_RESET)
     // Target reset pin: pull up, input to emulate open drain pin
@@ -51,10 +60,10 @@ void probe_gpio_init()
     gpio_pull_up(PROBE_PIN_SWDIOEN);
 }
 
-void probe_sm_init(pio_sm_config* sm_config) {
+static inline void probe_sm_init(pio_sm_config* sm_config) {
 
     // Set SWDIOEN and SWCLK as sideset pins
-    sm_config_set_sideset_pins(&sm_config, PROBE_PIN_SWDIOEN);
+    sm_config_set_sideset_pins(sm_config, PROBE_PIN_SWDIOEN);
 
     // Set SWDIO offset
     sm_config_set_out_pins(sm_config, PROBE_PIN_SWDIO, 1);

src/sw_dp_pio.c

@@ -133,8 +133,6 @@ uint8_t SWD_Transfer (uint32_t request, uint32_t *data) {
   probe_write_bits(8, prq);
 
   /* Turnaround (ignore read bits) */
-  probe_read_mode();
-
   ack = probe_read_bits(DAP_Data.swd_conf.turnaround + 3);
   ack >>= DAP_Data.swd_conf.turnaround;
 
@@ -154,12 +152,10 @@ uint8_t SWD_Transfer (uint32_t request, uint32_t *data) {
       picoprobe_debug("Read %02x ack %02x 0x%08x parity %01x\n",
                       prq, ack, val, bit);
       /* Turnaround for line idle */
-      probe_read_bits(DAP_Data.swd_conf.turnaround);
+      probe_hiz_clocks(DAP_Data.swd_conf.turnaround);
-      probe_write_mode();
     } else {
       /* Turnaround for write */
-      probe_read_bits(DAP_Data.swd_conf.turnaround);
+      probe_hiz_clocks(DAP_Data.swd_conf.turnaround);
-      probe_write_mode();
 
       /* Write WDATA[0:31] */
       val = *data;
@@ -178,9 +174,9 @@ uint8_t SWD_Transfer (uint32_t request, uint32_t *data) {
     /* Idle cycles - drive 0 for N clocks */
     if (DAP_Data.transfer.idle_cycles) {
       for (n = DAP_Data.transfer.idle_cycles; n; ) {
-        if (n > 32) {
+        if (n > 256) {
-          probe_write_bits(32, 0);
+          probe_write_bits(256, 0);
-          n -= 32;
+          n -= 256;
         } else {
           probe_write_bits(n, 0);
           n -= n;
@@ -195,8 +191,7 @@ uint8_t SWD_Transfer (uint32_t request, uint32_t *data) {
       /* Dummy Read RDATA[0:31] + Parity */
       probe_read_bits(33);
     }
-    probe_read_bits(DAP_Data.swd_conf.turnaround);
+    probe_hiz_clocks(DAP_Data.swd_conf.turnaround);
-    probe_write_mode();
     if (DAP_Data.swd_conf.data_phase && ((request & DAP_TRANSFER_RnW) == 0U)) {
       /* Dummy Write WDATA[0:31] + Parity */
       probe_write_bits(32, 0);
@@ -209,7 +204,6 @@ uint8_t SWD_Transfer (uint32_t request, uint32_t *data) {
   n = DAP_Data.swd_conf.turnaround + 32U + 1U;
   /* Back off data phase */
   probe_read_bits(n);
-  probe_write_mode();
   return ((uint8_t)ack);
 }