serprog works!

bsp/rp2040/cdc_uart.c

@@ -32,13 +32,8 @@
 #include "protocfg.h"
 #include "protos.h"
 
-// declare these so other CDC interfaces can use the same buffers, decreasing
+static uint8_t rx_buf[CFG_TUD_CDC_RX_BUFSIZE];
-// overall memory usage. however, this means the contents of these buffers
+static uint8_t tx_buf[CFG_TUD_CDC_TX_BUFSIZE];
-// can't be relied upon to persist between two cdc_*_task() calls
-extern uint8_t rx_buf[CFG_TUD_CDC_RX_BUFSIZE];
-extern uint8_t tx_buf[CFG_TUD_CDC_TX_BUFSIZE];
-__attribute__((__weak__)) uint8_t rx_buf[CFG_TUD_CDC_RX_BUFSIZE];
-__attribute__((__weak__)) uint8_t tx_buf[CFG_TUD_CDC_TX_BUFSIZE];
 
 void cdc_uart_init(void) {
     gpio_set_function(PICOPROBE_UART_TX, GPIO_FUNC_UART);

bsp/rp2040/spi_serprog.c

@@ -21,14 +21,15 @@ void sp_spi_init(void) {
 
 	spi_init(PROBE_SPI_DEV, 512*1000); // default to 512 kHz
 
-	gpio_init(PROBE_SPI_nCS);
-	gpio_put(PROBE_SPI_nCS, 1);
-	gpio_set_dir(PROBE_SPI_nCS, GPIO_OUT);
-
 	gpio_set_function(PROBE_SPI_MISO, GPIO_FUNC_SPI);
 	gpio_set_function(PROBE_SPI_MOSI, GPIO_FUNC_SPI);
 	gpio_set_function(PROBE_SPI_SCLK, GPIO_FUNC_SPI);
 
+	//gpio_set_function(PROBE_SPI_nCS, GPIO_FUNC_SIO);
+	gpio_init(PROBE_SPI_nCS);
+	gpio_put(PROBE_SPI_nCS, 1);
+	gpio_set_dir(PROBE_SPI_nCS, GPIO_OUT);
+
 	bi_decl(bi_3pins_with_func(PROBE_SPI_MISO, PROBE_SPI_MOSI, PROBE_SPI_SCLK, GPIO_FUNC_SPI));
 	bi_decl(bi_1pin_with_name(PROBE_SPI_nCS, "SPI #CS"));
 }
@@ -49,10 +50,10 @@ void __not_in_flash_func(sp_spi_cs_select)(void) {
 }
 
 void __not_in_flash_func(sp_spi_op_begin)(void) {
-	if (!cs_asserted) sp_spi_cs_select();
+	sp_spi_cs_select();
 }
 void __not_in_flash_func(sp_spi_op_end)(void) {
-	if (cs_asserted) sp_spi_cs_deselect();
+	sp_spi_cs_deselect();
 }
 
 // TODO: use dma?

cdc_serprog.c

@@ -9,6 +9,7 @@
 #ifdef DBOARD_HAS_SERPROG
 
 #include "protos.h"
+#include "thread.h"
 
 #include "serprog.h"
 
@@ -31,124 +32,172 @@ static const char serprog_pgmname[16] = {
     'D','a','p','p','e','r','M','i','m','e','-','J','T','A','G',0 // TODO
 };
 
-// declare these so other CDC interfaces can use the same buffers, decreasing
+static uint8_t rx_buf[CFG_TUD_CDC_RX_BUFSIZE];
-// overall memory usage. however, this means the contents of these buffers
+static uint8_t tx_buf[CFG_TUD_CDC_TX_BUFSIZE];
-// can't be relied upon to persist between two cdc_*_task() calls
-extern uint8_t rx_buf[CFG_TUD_CDC_RX_BUFSIZE];
-extern uint8_t tx_buf[CFG_TUD_CDC_TX_BUFSIZE];
-__attribute__((__weak__)) uint8_t rx_buf[CFG_TUD_CDC_RX_BUFSIZE];
-__attribute__((__weak__)) uint8_t tx_buf[CFG_TUD_CDC_TX_BUFSIZE];
 
-static uint32_t bufpos;
+static uint32_t rxavail, rxpos;
 
 void cdc_serprog_init(void) {
-    bufpos = 0;
+    rxavail = 0;
+    rxpos = 0;
 
     sp_spi_init();
 }
 
-static uint32_t serprog_handle_cmd(const uint8_t* buf, uint32_t nread) {
+static uint8_t read_byte(void) {
-    if (nread == 0) return 0;
+    while (rxavail <= 0) {
+        if (!tud_cdc_n_connected(CDC_N_SERPROG) || !tud_cdc_n_available(CDC_N_SERPROG)) {
+            thread_yield();
+            continue;
+        }
 
-    uint32_t nresp = 0, rv = 1;
+        rxpos = 0;
+        rxavail = tud_cdc_n_read(CDC_N_SERPROG, rx_buf, sizeof rx_buf);
 
-    switch (buf[0]) {
+        if (rxavail == 0) thread_yield();
+    }
+
+    uint8_t rv = rx_buf[rxpos];
+    ++rxpos;
+    --rxavail;
+    //printf("r %02x\n",rv);
+    return rv;
+}
+
+static void handle_cmd(void) {
+    uint32_t nresp = 0;
+
+    uint8_t cmd = read_byte();
+    switch (cmd) {
     case S_CMD_NOP:
         printf("nop\n");
         tx_buf[0] = S_ACK;
         nresp = 1;
         break;
     case S_CMD_SYNCNOP:
-        printf("syncnop\n");
+        printf("snop\n");
         tx_buf[0] = S_NAK;
         tx_buf[1] = S_ACK;
         nresp = 2;
         break;
     case S_CMD_Q_IFACE:
-        printf("q_iface\n");
+        printf("q_if\n");
         tx_buf[0] = S_ACK;
         tx_buf[1] =  SERPROG_IFACE_VERSION       & 0xff;
         tx_buf[2] = (SERPROG_IFACE_VERSION >> 8) & 0xff;
         nresp = 3;
         break;
     case S_CMD_Q_CMDMAP:
-        printf("q_cmdmap\n");
+        printf("q_cmap\n");
         tx_buf[0] = S_ACK;
         memcpy(&tx_buf[1], serprog_cmdmap, sizeof serprog_cmdmap);
         nresp = sizeof(serprog_cmdmap) + 1;
         break;
     case S_CMD_Q_PGMNAME:
-        printf("q_pgmname\n");
+        printf("q_pgm\n");
         tx_buf[0] = S_ACK;
         memcpy(&tx_buf[1], serprog_pgmname, sizeof serprog_pgmname);
         nresp = sizeof(serprog_pgmname) + 1;
         break;
     case S_CMD_Q_SERBUF:
-        printf("q_serbuf\n");
+        printf("q_sbuf\n");
         tx_buf[0] = S_ACK;
         tx_buf[1] =  sizeof(rx_buf)       & 0xff;
         tx_buf[2] = (sizeof(rx_buf) >> 8) & 0xff;
         nresp = 3;
         break;
     case S_CMD_Q_BUSTYPE:
-        printf("q_bustype\n");
+        printf("q_btyp\n");
         tx_buf[0] = S_ACK;
         tx_buf[1] = 1<<3; // SPI only
         nresp = 2;
         break;
     case S_CMD_Q_WRNMAXLEN:
-        printf("q_wrnmaxlen\n");
+        printf("q_wlen\n");
         tx_buf[0] = S_ACK;
         tx_buf[1] =  (sizeof(tx_buf)-1)       & 0xff;
         tx_buf[2] = ((sizeof(tx_buf)-1) >> 8) & 0xff;
-        nresp = 3;
+        tx_buf[3] = ((sizeof(tx_buf)-1) >>16) & 0xff;
+        nresp = 4;
         break;
     case S_CMD_Q_RDNMAXLEN:
-        printf("q_rdnmaxlen\n");
+        printf("q_rlen\n");
         tx_buf[0] = S_ACK;
         tx_buf[1] =  (sizeof(rx_buf)-1)       & 0xff;
         tx_buf[2] = ((sizeof(rx_buf)-1) >> 8) & 0xff;
-        nresp = 3;
+        tx_buf[3] = ((sizeof(rx_buf)-1) >>16) & 0xff;
+        nresp = 4;
         break;
     case S_CMD_S_BUSTYPE:
-        printf("s_bustype\n");
+        printf("s_btyp\n");
-        if (nread < 2) return 0; // need more data
 
-        if (buf[1] == (1<<3)) {
+        if (read_byte()/* bus type to set */ == (1<<3)) {
             tx_buf[0] = S_ACK;
         } else {
             tx_buf[0] = S_NAK;
         }
         nresp = 1;
-        rv = 2;
         break;
 
     case S_CMD_SPIOP: {
         printf("spiop\n");
-        if (nread < 7) return 0; // need more data
 
-        uint32_t
+        uint32_t slen, rlen;
-            slen = (uint32_t)buf[1] | ((uint32_t)buf[2] << 8) | ((uint32_t)buf[3]),
+        slen  = (uint32_t)read_byte();
-            rlen = (uint32_t)buf[4] | ((uint32_t)buf[5] << 8) | ((uint32_t)buf[6]);
+        slen |= (uint32_t)read_byte() <<  8;
+        slen |= (uint32_t)read_byte() << 16;
+        rlen  = (uint32_t)read_byte();
+        rlen |= (uint32_t)read_byte() <<  8;
+        rlen |= (uint32_t)read_byte() << 16;
 
-        if (7 + slen > sizeof(rx_buf) || 1 + rlen > sizeof(tx_buf)) {
+        sp_spi_op_begin();
-            // TODO: stream buffers in and out here for larger xfers
+        size_t this_batch;
-            tx_buf[0] = S_NAK;
-            nresp = 1;
-        } else {
-            if (nread < 7 + slen) return 0;
 
-            tx_buf[0] = S_ACK;
+        // 1. write slen data bytes
-            sp_spi_op_do(slen, &buf[7], rlen, &tx_buf[1]);
+        // we're going to use the tx buf for all operations here
-            nresp = 7 + rlen;
+        while (slen > 0) {
-            rv = 7 + slen;
+            this_batch = sizeof(tx_buf);
+            if (this_batch > slen) this_batch = slen;
+
+            for (size_t i = 0; i < this_batch; ++i) tx_buf[i] = read_byte();
+            sp_spi_op_write(this_batch, tx_buf);
+
+            slen -= this_batch;
         }
+
+        // 2. write data
+        // first, do a batch of 63, because we also need to send an ACK byte
+        this_batch = sizeof(tx_buf)-1;
+        if (this_batch > rlen) this_batch = rlen;
+        sp_spi_op_read(this_batch, &tx_buf[1]);
+        tx_buf[0] = S_ACK;
+        tud_cdc_n_write(CDC_N_SERPROG, tx_buf, this_batch+1);
+        rlen -= this_batch;
+
+        // now do in batches of 64
+        while (rlen > 0) {
+            this_batch = sizeof(tx_buf);
+            if (this_batch > rlen) this_batch = rlen;
+
+            sp_spi_op_read(this_batch, tx_buf);
+            tud_cdc_n_write(CDC_N_SERPROG, tx_buf, this_batch);
+
+            rlen -= this_batch;
+        }
+        tud_cdc_n_write_flush(CDC_N_SERPROG);
+
+        // that's it!
+        sp_spi_op_end();
+        nresp = 0; // we sent our own response manually
         }
         break;
     case S_CMD_S_SPI_FREQ: {
         printf("s_spi_freq\n");
-        if (nread < 5) return 0; // need more data
+        uint32_t freq;
-        uint32_t freq = (uint32_t)buf[1] | ((uint32_t)buf[2] << 8) | ((uint32_t)buf[3] << 16) | ((uint32_t)buf[4] << 24);
+        freq  = (uint32_t)read_byte();
+        freq |= (uint32_t)read_byte() <<  8;
+        freq |= (uint32_t)read_byte() << 16;
+        freq |= (uint32_t)read_byte() << 24;
 
         uint32_t nfreq = sp_spi_set_freq(freq);
         tx_buf[0] = S_ACK;
@@ -157,24 +206,21 @@ static uint32_t serprog_handle_cmd(const uint8_t* buf, uint32_t nread) {
         tx_buf[3] = (nfreq >> 16) & 0xff;
         tx_buf[4] = (nfreq >> 24) & 0xff;
         nresp = 5;
-        rv = 5;
         }
         break;
     case S_CMD_S_PINSTATE: {
-        printf("s_pinstate\n");
+        printf("s_pins\n");
-        if (nread < 2) return 0; // need more data
 
-        if (buf[1] == 0) sp_spi_cs_deselect();
+        if (read_byte() == 0) sp_spi_cs_deselect();
         else sp_spi_cs_select();
 
         tx_buf[0] = S_ACK;
         nresp = 1;
-        rv = 2;
         }
         break;
 
     default:
-        printf("illcmd %d\n", buf[0]);
+        printf("ill %d\n", cmd);
         tx_buf[0] = S_NAK;
         nresp = 1;
         break;
@@ -184,57 +230,59 @@ static uint32_t serprog_handle_cmd(const uint8_t* buf, uint32_t nread) {
         tud_cdc_n_write(CDC_N_SERPROG, tx_buf, nresp);
         tud_cdc_n_write_flush(CDC_N_SERPROG);
     }
-
-    return rv;
 }
 
-extern void cdc_uart_task();
+//extern void cdc_uart_task();
+//void cdc_serprog_task(void) {
+//    bool conn = tud_cdc_n_connected(CDC_N_SERPROG),
+//         avail = tud_cdc_n_available(CDC_N_SERPROG);
+//    //printf("hi conn=%c avail=%c\n", conn?'y':'n', avail?'y':'n');
+//    // TODO: this is, apparently, not at all how this works: in practice,
+//    //       bytes seem to be sent one by one, so its probably better to rework
+//    //       this, a lot
+//    if (conn && avail) {
+//        //printf("rbp=%d\n", bufpos);
+//        uint32_t nread = tud_cdc_n_read(CDC_N_SERPROG, &rx_buf[bufpos], sizeof(rx_buf) - bufpos);
+//        printf("got %d\n", nread);
+//        cdc_uart_task();
+//
+//        bufpos = 0;
+//        do {
+//            //printf("hbp=%d\n", /*rx_buf[bufpos],*/ bufpos);
+//            cdc_uart_task();
+//            uint32_t dec = serprog_handle_cmd(&rx_buf[bufpos], nread);
+//            cdc_uart_task();
+//            printf("dec=%d\n", dec);
+//
+//            cdc_uart_task();
+//
+//            // didn't do a decrement => not enough data, wait for the next
+//            // task() call to read it in
+//            if (dec == 0) {
+//                // so we move the leftover data to the start of the buffer,
+//                // and make sure the next call will put the new data right
+//                // after it
+//                //printf("mv %d %d %d ", nread, bufpos, rx_buf[bufpos]);
+//                memmove(rx_buf, &rx_buf[bufpos], nread);
+//                //printf("%d\n", rx_buf[0]);
+//                bufpos = nread;
+//                break;
+//            }
+//
+//            nread -= dec;
+//            bufpos += dec;
+//            // read everything left in the buffer => we're done here
+//            if (nread == 0) {
+//                // and we can start using the full rx buffer again
+//                bufpos = 0;
+//                break;
+//            }
+//        } while (tud_cdc_n_connected(CDC_N_SERPROG) && tud_cdc_n_available(CDC_N_SERPROG));
+//    }
+//}
 void cdc_serprog_task(void) {
-    bool conn = tud_cdc_n_connected(CDC_N_SERPROG),
+    handle_cmd();
-         avail = tud_cdc_n_available(CDC_N_SERPROG);
+    printf("d\n");
-    //printf("hi conn=%c avail=%c\n", conn?'y':'n', avail?'y':'n');
-    // TODO: this is, apparently, not at all how this works: in practice,
-    //       bytes seem to be sent one by one, so its probably better to rework
-    //       this, a lot
-    if (conn && avail) {
-        //printf("rbp=%d\n", bufpos);
-        uint32_t nread = tud_cdc_n_read(CDC_N_SERPROG, &rx_buf[bufpos], sizeof(rx_buf) - bufpos);
-        printf("got %d\n", nread);
-        cdc_uart_task();
-
-        bufpos = 0;
-        do {
-            //printf("hbp=%d\n", /*rx_buf[bufpos],*/ bufpos);
-            cdc_uart_task();
-            uint32_t dec = serprog_handle_cmd(&rx_buf[bufpos], nread);
-            cdc_uart_task();
-            printf("dec=%d\n", dec);
-
-            cdc_uart_task();
-
-            // didn't do a decrement => not enough data, wait for the next
-            // task() call to read it in
-            if (dec == 0) {
-                // so we move the leftover data to the start of the buffer,
-                // and make sure the next call will put the new data right
-                // after it
-                //printf("mv %d %d %d ", nread, bufpos, rx_buf[bufpos]);
-                memmove(rx_buf, &rx_buf[bufpos], nread);
-                //printf("%d\n", rx_buf[0]);
-                bufpos = nread;
-                break;
-            }
-
-            nread -= dec;
-            bufpos += dec;
-            // read everything left in the buffer => we're done here
-            if (nread == 0) {
-                // and we can start using the full rx buffer again
-                bufpos = 0;
-                break;
-            }
-        } while (tud_cdc_n_connected(CDC_N_SERPROG) && tud_cdc_n_available(CDC_N_SERPROG));
-    }
 }
 
 #endif /* DBOARD_HAS_SERPROG */

main.c

@@ -119,6 +119,8 @@ int main(void)
 
   while (1)
   {
+    //printf("hi\n");
+
     tud_task(); // tinyusb device task
 #ifdef DBOARD_HAS_UART
     //cdc_uart_task();