picoprobe version 1.0.3

- The reset pin must move otherwise uart0 tx is squashed
- Don't preempt printf, it doesn't like it
- Set up the UART by default

CMakeLists.txt

@@ -17,7 +17,6 @@ add_executable(picoprobe
         src/cdc_uart.c
         src/get_serial.c
         src/sw_dp_pio.c
-        src/tusb_edpt_handler.c
 )
 
 target_sources(picoprobe PRIVATE

include/board_example_config.h

@@ -68,9 +68,6 @@
 #define PICOPROBE_UART_RX 5
 #define PICOPROBE_UART_INTERFACE uart1
 #define PICOPROBE_UART_BAUDRATE 115200
-/* Flow control - some or all of these can be omitted if not used */
-#define PICOPROBE_UART_RTS 9
-#define PICOPROBE_UART_DTR 10
 #endif
 
 /* LED config - some or all of these can be omitted if not used */

src/cdc_uart.c

@@ -34,9 +34,8 @@
 TaskHandle_t uart_taskhandle;
 TickType_t last_wake, interval = 100;
 
-/* Max 1 FIFO worth of data */
-static uint8_t tx_buf[32];
-static uint8_t rx_buf[32];
+static uint8_t tx_buf[CFG_TUD_CDC_TX_BUFSIZE];
+static uint8_t rx_buf[CFG_TUD_CDC_RX_BUFSIZE];
 // Actually s^-1 so 25ms
 #define DEBOUNCE_MS 40
 static uint debounce_ticks = 5;
@@ -55,23 +54,11 @@ void cdc_uart_init(void) {
     gpio_set_pulls(PICOPROBE_UART_TX, 1, 0);
     gpio_set_pulls(PICOPROBE_UART_RX, 1, 0);
     uart_init(PICOPROBE_UART_INTERFACE, PICOPROBE_UART_BAUDRATE);
-
-#ifdef PICOPROBE_UART_RTS
-    gpio_init(PICOPROBE_UART_RTS);
-    gpio_set_dir(PICOPROBE_UART_RTS, GPIO_OUT);
-    gpio_put(PICOPROBE_UART_RTS, 1);
-#endif
-#ifdef PICOPROBE_UART_DTR
-    gpio_init(PICOPROBE_UART_DTR);
-    gpio_set_dir(PICOPROBE_UART_DTR, GPIO_OUT);
-    gpio_put(PICOPROBE_UART_DTR, 1);
-#endif
 }
 
 void cdc_task(void)
 {
     static int was_connected = 0;
-    static uint cdc_tx_oe = 0;
     uint rx_len = 0;
 
     // Consume uart fifo regardless even if not connected
@@ -90,9 +77,6 @@ void cdc_task(void)
           rx_led_debounce = debounce_ticks;
 #endif
           written = MIN(tud_cdc_write_available(), rx_len);
-          if (rx_len > written)
-              cdc_tx_oe++;
-
           if (written > 0) {
             tud_cdc_write(rx_buf, written);
             tud_cdc_write_flush();
@@ -129,7 +113,6 @@ void cdc_task(void)
     } else if (was_connected) {
       tud_cdc_write_clear();
       was_connected = 0;
-      cdc_tx_oe = 0;
     }
 }
 
@@ -148,8 +131,6 @@ void cdc_thread(void *ptr)
 
 void tud_cdc_line_coding_cb(uint8_t itf, cdc_line_coding_t const* line_coding)
 {
-  uart_parity_t parity;
-  uint data_bits, stop_bits;
   /* Set the tick thread interval to the amount of time it takes to
    * fill up half a FIFO. Millis is too coarse for integer divide.
    */
@@ -164,63 +145,11 @@ void tud_cdc_line_coding_cb(uint8_t itf, cdc_line_coding_t const* line_coding)
   tud_cdc_write_clear();
   tud_cdc_read_flush();
   uart_init(PICOPROBE_UART_INTERFACE, line_coding->bit_rate);
-
-  switch (line_coding->parity) {
-  case CDC_LINE_CODING_PARITY_ODD:
-    parity = UART_PARITY_ODD;
-    break;
-  case CDC_LINE_CODING_PARITY_EVEN:
-    parity = UART_PARITY_EVEN;
-    break;
-  default:
-    picoprobe_info("invalid parity setting %u\n", line_coding->parity);
-    /* fallthrough */
-  case CDC_LINE_CODING_PARITY_NONE:
-    parity = UART_PARITY_NONE;
-    break;
-  }
-
-  switch (line_coding->data_bits) {
-  case 5:
-  case 6:
-  case 7:
-  case 8:
-    data_bits = line_coding->data_bits;
-    break;
-  default:
-    picoprobe_info("invalid data bits setting: %u\n", line_coding->data_bits);
-    data_bits = 8;
-    break;
-  }
-
-  /* The PL011 only supports 1 or 2 stop bits. 1.5 stop bits is translated to 2,
-   * which is safer than the alternative. */
-  switch (line_coding->stop_bits) {
-  case CDC_LINE_CONDING_STOP_BITS_1_5:
-  case CDC_LINE_CONDING_STOP_BITS_2:
-    stop_bits = 2;
-  break;
-  default:
-    picoprobe_info("invalid stop bits setting: %u\n", line_coding->stop_bits);
-    /* fallthrough */
-  case CDC_LINE_CONDING_STOP_BITS_1:
-    stop_bits = 1;
-  break;
-  }
-
-  uart_set_format(PICOPROBE_UART_INTERFACE, data_bits, stop_bits, parity);
   vTaskResume(uart_taskhandle);
 }
 
 void tud_cdc_line_state_cb(uint8_t itf, bool dtr, bool rts)
 {
-#ifdef PICOPROBE_UART_RTS
-  gpio_put(PICOPROBE_UART_RTS, !rts);
-#endif
-#ifdef PICOPROBE_UART_DTR
-  gpio_put(PICOPROBE_UART_DTR, !dtr);
-#endif
-
   /* CDC drivers use linestate as a bodge to activate/deactivate the interface.
    * Resume our UART polling on activate, stop on deactivate */
   if (!dtr && !rts) {

src/main.c

@@ -39,7 +39,6 @@
 #include "cdc_uart.h"
 #include "get_serial.h"
 #include "led.h"
-#include "tusb_edpt_handler.h"
 #include "DAP.h"
 
 // UART0 for Picoprobe debug
@@ -54,12 +53,10 @@ static uint8_t RxDataBuffer[CFG_TUD_HID_EP_BUFSIZE];
 #define TUD_TASK_PRIO  (tskIDLE_PRIORITY + 2)
 #define DAP_TASK_PRIO  (tskIDLE_PRIORITY + 1)
 
-TaskHandle_t dap_taskhandle, tud_taskhandle;
+static TaskHandle_t dap_taskhandle, tud_taskhandle;
 
 void usb_thread(void *ptr)
 {
-    TickType_t wake;
-    wake = xTaskGetTickCount();
     do {
         tud_task();
 #ifdef PICOPROBE_USB_CONNECTED_LED
@@ -68,9 +65,8 @@ void usb_thread(void *ptr)
         else
             gpio_put(PICOPROBE_USB_CONNECTED_LED, 0);
 #endif
-        // Go to sleep for up to a tick if nothing to do
-        if (!tud_task_event_ready())
-            xTaskDelayUntil(&wake, 1);
+        // Trivial delay to save power
+        vTaskDelay(1);
     } while (1);
 }
 
@@ -79,6 +75,22 @@ void usb_thread(void *ptr)
 #define tud_vendor_flush(x) ((void)0)
 #endif
 
+void dap_thread(void *ptr)
+{
+    uint32_t resp_len;
+    do {
+        if (tud_vendor_available()) {
+            tud_vendor_read(RxDataBuffer, sizeof(RxDataBuffer));
+            resp_len = DAP_ProcessCommand(RxDataBuffer, TxDataBuffer);
+            tud_vendor_write(TxDataBuffer, resp_len);
+            tud_vendor_flush();
+        } else {
+            // Trivial delay to save power
+            vTaskDelay(1);
+        }
+    } while (1);
+}
+
 int main(void) {
 
     board_init();

src/tusb_config.h

@@ -68,15 +68,8 @@
 #define CFG_TUD_MIDI            0
 #define CFG_TUD_VENDOR          1
 
-/*
- * TX bufsize (actually UART RX) is oversized because the Windows CDC-ACM
- * driver submits a grand total of _one_ URB at any one time.
- * This means the application must consume the data before the next IN token
- * is issued. At high datarates this leads to huge variation in instantaneous
- * throughput on USB, so a large runway is needed.
- */
-#define CFG_TUD_CDC_RX_BUFSIZE 128
-#define CFG_TUD_CDC_TX_BUFSIZE 4096
+#define CFG_TUD_CDC_RX_BUFSIZE 64
+#define CFG_TUD_CDC_TX_BUFSIZE 64
 
 #define CFG_TUD_VENDOR_RX_BUFSIZE 8192
 #define CFG_TUD_VENDOR_TX_BUFSIZE 8192

src/tusb_edpt_handler.c

@@ -1,281 +0,0 @@
-/**
- * Copyright (c) 2023 Raspberry Pi (Trading) Ltd.
- *
- * SPDX-License-Identifier: BSD-3-Clause
- */
-
-#include "tusb_edpt_handler.h"
-#include "DAP.h"
-
-static uint8_t itf_num;
-static uint8_t _rhport;
-
-volatile uint32_t _resp_len;
-
-static uint8_t _out_ep_addr;
-static uint8_t _in_ep_addr;
-
-static buffer_t USBRequestBuffer;
-static buffer_t USBResponseBuffer;
-
-static uint8_t DAPRequestBuffer[DAP_PACKET_SIZE];
-static uint8_t DAPResponseBuffer[DAP_PACKET_SIZE];
-
-#define WR_IDX(x) (x.wptr % DAP_PACKET_COUNT)
-#define RD_IDX(x) (x.rptr % DAP_PACKET_COUNT)
-
-#define WR_SLOT_PTR(x) &(x.data[WR_IDX(x)][0])
-#define RD_SLOT_PTR(x) &(x.data[RD_IDX(x)][0])
-
-bool buffer_full(buffer_t *buffer)
-{
-	return ((buffer->wptr + 1) % DAP_PACKET_COUNT == buffer->rptr);
-}
-
-bool buffer_empty(buffer_t *buffer)
-{
-	return (buffer->wptr == buffer->rptr);
-}
-
-void dap_edpt_init(void) {
-
-}
-
-void dap_edpt_reset(uint8_t __unused rhport)
-{
-	itf_num = 0;
-}
-
-char * dap_cmd_string[] = {
-	[ID_DAP_Info               ] = "DAP_Info",
-	[ID_DAP_HostStatus         ] = "DAP_HostStatus",
-	[ID_DAP_Connect            ] = "DAP_Connect",
-	[ID_DAP_Disconnect         ] = "DAP_Disconnect",
-	[ID_DAP_TransferConfigure  ] = "DAP_TransferConfigure",
-	[ID_DAP_Transfer           ] = "DAP_Transfer",
-	[ID_DAP_TransferBlock      ] = "DAP_TransferBlock",
-	[ID_DAP_TransferAbort      ] = "DAP_TransferAbort",
-	[ID_DAP_WriteABORT         ] = "DAP_WriteABORT",
-	[ID_DAP_Delay              ] = "DAP_Delay",
-	[ID_DAP_ResetTarget        ] = "DAP_ResetTarget",
-	[ID_DAP_SWJ_Pins           ] = "DAP_SWJ_Pins",
-	[ID_DAP_SWJ_Clock          ] = "DAP_SWJ_Clock",
-	[ID_DAP_SWJ_Sequence       ] = "DAP_SWJ_Sequence",
-	[ID_DAP_SWD_Configure      ] = "DAP_SWD_Configure",
-	[ID_DAP_SWD_Sequence       ] = "DAP_SWD_Sequence",
-	[ID_DAP_JTAG_Sequence      ] = "DAP_JTAG_Sequence",
-	[ID_DAP_JTAG_Configure     ] = "DAP_JTAG_Configure",
-	[ID_DAP_JTAG_IDCODE        ] = "DAP_JTAG_IDCODE",
-	[ID_DAP_SWO_Transport      ] = "DAP_SWO_Transport",
-	[ID_DAP_SWO_Mode           ] = "DAP_SWO_Mode",
-	[ID_DAP_SWO_Baudrate       ] = "DAP_SWO_Baudrate",
-	[ID_DAP_SWO_Control        ] = "DAP_SWO_Control",
-	[ID_DAP_SWO_Status         ] = "DAP_SWO_Status",
-	[ID_DAP_SWO_ExtendedStatus ] = "DAP_SWO_ExtendedStatus",
-	[ID_DAP_SWO_Data           ] = "DAP_SWO_Data",
-	[ID_DAP_QueueCommands      ] = "DAP_QueueCommands",
-	[ID_DAP_ExecuteCommands    ] = "DAP_ExecuteCommands",
-};
-
-
-uint16_t dap_edpt_open(uint8_t __unused rhport, tusb_desc_interface_t const *itf_desc, uint16_t max_len)
-{
-
-	TU_VERIFY(TUSB_CLASS_VENDOR_SPECIFIC == itf_desc->bInterfaceClass &&
-			PICOPROBE_INTERFACE_SUBCLASS == itf_desc->bInterfaceSubClass &&
-			PICOPROBE_INTERFACE_PROTOCOL == itf_desc->bInterfaceProtocol, 0);
-
-	//  Initialise circular buffer indices
-	USBResponseBuffer.wptr = 0;
-	USBResponseBuffer.rptr = 0;
-	USBRequestBuffer.wptr = 0;
-	USBRequestBuffer.rptr = 0;
-
-	// Initialse full/empty flags
-	USBResponseBuffer.wasFull = false;
-	USBResponseBuffer.wasEmpty = true;
-	USBRequestBuffer.wasFull = false;
-	USBRequestBuffer.wasEmpty = true;
-
-	uint16_t const drv_len = sizeof(tusb_desc_interface_t) + (itf_desc->bNumEndpoints * sizeof(tusb_desc_endpoint_t));
-	TU_VERIFY(max_len >= drv_len, 0);
-	itf_num = itf_desc->bInterfaceNumber;
-
-	// Initialising the OUT endpoint
-
-	tusb_desc_endpoint_t *edpt_desc = (tusb_desc_endpoint_t *) (itf_desc + 1);
-	uint8_t ep_addr = edpt_desc->bEndpointAddress;
-
-	_out_ep_addr = ep_addr;
-
-	// The OUT endpoint requires a call to usbd_edpt_xfer to initialise the endpoint, giving tinyUSB a buffer to consume when a transfer occurs at the endpoint
-	usbd_edpt_open(rhport, edpt_desc);
-	usbd_edpt_xfer(rhport, ep_addr, WR_SLOT_PTR(USBRequestBuffer), DAP_PACKET_SIZE);
-
-	// Initiliasing the IN endpoint
-
-	edpt_desc++;
-	ep_addr = edpt_desc->bEndpointAddress;
-
-	_in_ep_addr = ep_addr;
-
-	// The IN endpoint doesn't need a transfer to initialise it, as this will be done by the main loop of dap_thread
-	usbd_edpt_open(rhport, edpt_desc);
-
-	return drv_len;
-
-}
-
-bool dap_edpt_control_xfer_cb(uint8_t __unused rhport, uint8_t stage, tusb_control_request_t const *request)
-{
-	return false;
-}
-
-// Manage USBResponseBuffer (request) write and USBRequestBuffer (response) read indices
-bool dap_edpt_xfer_cb(uint8_t __unused rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes)
-{
-	const uint8_t ep_dir = tu_edpt_dir(ep_addr);
-
-	if(ep_dir == TUSB_DIR_IN)
-	{
-		if(xferred_bytes >= 0u && xferred_bytes <= DAP_PACKET_SIZE)
-		{
-			USBResponseBuffer.rptr++;
-
-			// This checks that the buffer was not empty in DAP thread, which means the next buffer was not queued up for the in endpoint callback
-			// So, queue up the buffer at the new read index, since we expect read to catch up to write at this point.
-			// It is possible for the read index to be multiple spaces behind the write index (if the USB callbacks are lagging behind dap thread),
-			// so we account for this by only setting wasEmpty to true if the next callback will empty the buffer
-			if(!USBResponseBuffer.wasEmpty)
-			{
-				usbd_edpt_xfer(rhport, ep_addr, RD_SLOT_PTR(USBResponseBuffer), (uint16_t) _resp_len);
-				USBResponseBuffer.wasEmpty = (USBResponseBuffer.rptr + 1) == USBResponseBuffer.wptr;
-			}
-
-			//  Wake up DAP thread after processing the callback
-			vTaskResume(dap_taskhandle);
-			return true;
-		}
-
-		return false;
-
-	} else if(ep_dir == TUSB_DIR_OUT)    {
-
-		if(xferred_bytes >= 0u && xferred_bytes <= DAP_PACKET_SIZE)
-		{
-			// Only queue the next buffer in the out callback if the buffer is not full
-			// If full, we set the wasFull flag, which will be checked by dap thread
-			if(!buffer_full(&USBRequestBuffer))
-			{
-				USBRequestBuffer.wptr++;
-				usbd_edpt_xfer(rhport, ep_addr, WR_SLOT_PTR(USBRequestBuffer), DAP_PACKET_SIZE);
-				USBRequestBuffer.wasFull = false;
-			}
-			else {
-				USBRequestBuffer.wasFull = true;
-			}
-
-			//  Wake up DAP thread after processing the callback
-			vTaskResume(dap_taskhandle);
-			return true;
-		}
-
-		return false;
-	}
-	else return false;
-}
-
-void dap_thread(void *ptr)
-{
-	uint32_t n;
-	do
-	{
-		while(USBRequestBuffer.rptr != USBRequestBuffer.wptr)
-		{
-			/*
-			 * Atomic command support - buffer QueueCommands, but don't process them
-			 * until a non-QueueCommands packet is seen.
-			 */
-			n = USBRequestBuffer.rptr;
-			while (USBRequestBuffer.data[n % DAP_PACKET_COUNT][0] == ID_DAP_QueueCommands) {
-				picoprobe_info("%u %u DAP queued cmd %s len %02x\n",
-					       USBRequestBuffer.wptr, USBRequestBuffer.rptr,
-					       dap_cmd_string[USBRequestBuffer.data[n % DAP_PACKET_COUNT][0]], USBRequestBuffer.data[n % DAP_PACKET_COUNT][1]);
-				USBRequestBuffer.data[n % DAP_PACKET_COUNT][0] = ID_DAP_ExecuteCommands;
-				n++;
-				while (n == USBRequestBuffer.wptr) {
-					/* Need yield in a loop here, as IN callbacks will also wake the thread */
-					picoprobe_info("DAP wait\n");
-					vTaskSuspend(dap_taskhandle);
-				}
-			}
-			// Read a single packet from the USB buffer into the DAP Request buffer
-			memcpy(DAPRequestBuffer, RD_SLOT_PTR(USBRequestBuffer), DAP_PACKET_SIZE);
-			picoprobe_info("%u %u DAP cmd %s len %02x\n",
-				       USBRequestBuffer.wptr, USBRequestBuffer.rptr,
-				       dap_cmd_string[DAPRequestBuffer[0]], DAPRequestBuffer[1]);
-			USBRequestBuffer.rptr++;
-
-			// If the buffer was full in the out callback, we need to queue up another buffer for the endpoint to consume, now that we know there is space in the buffer.
-			if(USBRequestBuffer.wasFull)
-			{
-				vTaskSuspendAll(); // Suspend the scheduler to safely update the write index
-				USBRequestBuffer.wptr++;
-				usbd_edpt_xfer(_rhport, _out_ep_addr, WR_SLOT_PTR(USBRequestBuffer), DAP_PACKET_SIZE);
-				USBRequestBuffer.wasFull = false;
-				xTaskResumeAll();
-			}
-
-			_resp_len = DAP_ExecuteCommand(DAPRequestBuffer, DAPResponseBuffer);
-			picoprobe_info("%u %u DAP resp %s\n",
-					USBResponseBuffer.wptr, USBResponseBuffer.rptr,
-					dap_cmd_string[DAPResponseBuffer[0]]);
-
-
-			//  Suspend the scheduler to avoid stale values/race conditions between threads
-			vTaskSuspendAll();
-
-			if(buffer_empty(&USBResponseBuffer))
-			{
-				memcpy(WR_SLOT_PTR(USBResponseBuffer), DAPResponseBuffer, (uint16_t) _resp_len);
-				USBResponseBuffer.wptr++;
-
-				usbd_edpt_xfer(_rhport, _in_ep_addr, RD_SLOT_PTR(USBResponseBuffer), (uint16_t) _resp_len);
-			} else {
-
-				memcpy(WR_SLOT_PTR(USBResponseBuffer), DAPResponseBuffer, (uint16_t) _resp_len);
-				USBResponseBuffer.wptr++;
-
-				// The In callback needs to check this flag to know when to queue up the next buffer.
-				USBResponseBuffer.wasEmpty = false;
-			}
-			xTaskResumeAll();
-		}
-
-		// Suspend DAP thread until it is awoken by a USB thread callback
-		vTaskSuspend(dap_taskhandle);
-
-	} while (1);
-
-}
-
-usbd_class_driver_t const _dap_edpt_driver =
-{
-		.init = dap_edpt_init,
-		.reset = dap_edpt_reset,
-		.open = dap_edpt_open,
-		.control_xfer_cb = dap_edpt_control_xfer_cb,
-		.xfer_cb = dap_edpt_xfer_cb,
-		.sof = NULL,
-#if CFG_TUSB_DEBUG >= 2
-		.name = "PICOPROBE ENDPOINT"
-#endif
-};
-
-// Add the custom driver to the tinyUSB stack
-usbd_class_driver_t const *usbd_app_driver_get_cb(uint8_t *driver_count)
-{
-	*driver_count = 1;
-	return &_dap_edpt_driver;
-}
-

src/tusb_edpt_handler.h

@@ -1,41 +0,0 @@
-/**
- * Copyright (c) 2023 Raspberry Pi (Trading) Ltd.
- *
- * SPDX-License-Identifier: BSD-3-Clause
- */
-
-#ifndef TUSB_EDPT_HANDLER_H
-#define TUSB_EDPT_HANDLER_H
-
-#include "tusb.h"
-
-#include "device/usbd_pvt.h"
-#include "DAP_config.h"
-
-#define PICOPROBE_INTERFACE_SUBCLASS 0x00
-#define PICOPROBE_INTERFACE_PROTOCOL 0x00
-
-typedef struct {
-	uint8_t data[DAP_PACKET_COUNT][DAP_PACKET_SIZE];
-	volatile uint32_t wptr;
-	volatile uint32_t rptr;
-	volatile bool wasEmpty;
-	volatile bool wasFull;
-} buffer_t;
-
-extern TaskHandle_t dap_taskhandle, tud_taskhandle;
-
-/* Main DAP loop */
-void dap_thread(void *ptr);
-
-/* Endpoint Handling */
-void picoprobe_edpt_init(void);
-uint16_t picoprobe_edpt_open(uint8_t __unused rhport, tusb_desc_interface_t const *itf_desc, uint16_t max_len);
-bool picoprobe_edpt_control_xfer_cb(uint8_t __unused rhport, uint8_t stage,  tusb_control_request_t const *request);
-bool picoprobe_edpt_xfer_cb(uint8_t __unused rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes);
-
-/* Helper Functions */
-bool buffer_full(buffer_t *buffer);
-bool buffer_empty(buffer_t *buffer);
-
-#endif

src/usb_descriptors.c

@@ -242,4 +242,4 @@ TU_VERIFY_STATIC(sizeof(desc_ms_os_20) == MS_OS_20_DESC_LEN, "Incorrect size");
 uint8_t const * tud_descriptor_bos_cb(void)
 {
   return desc_bos;
-}
+}