Use DMA for UART RX instead of interrupts.

DMA is more efficient and less prone to miss characters than interrupts.

An open question is if circular mode is really the best. If someone
copy-pastes more than the RX buffer size of configuration into the CLI,
we risk the DMA controller catching up with the reader and overwriting
data not yet read.

Since we don't have flow control back to the users terminal, we will
always fail if too much data is entered before we can process it. The
question is if failing to stuff new data at the end of a buffer might be
better than data being overwritten - thus messing up the commands in
unpredictable ways.

7 files changed, 113 insertions, 120 deletions

diff --git a/projects/cli-test/cli-test.c b/projects/cli-test/cli-test.c
index b6178af..ed6aac3 100644
--- a/projects/cli-test/cli-test.c
+++ b/projects/cli-test/cli-test.c
@@ -71,23 +71,6 @@ void do_early_dfu_jump(void)
     while (1);
 }
 
-/* Callback for HAL_UART_Receive_IT(). */
-void HAL_UART_RxCpltCallback(UART_HandleTypeDef *huart)
-{
-    if (huart->Instance == huart_mgmt.Instance) {
-	mgmt_cli_uart_isr((const uint8_t *) &uart_rx, 1);
-
-	/* Set things up to receive another byte. */
-	HAL_UART_Receive_IT(huart, (uint8_t *) &uart_rx, 1);
-    }
-}
-
-void HAL_UART_ErrorCallback(UART_HandleTypeDef *huart)
-{
-    led_on(LED_RED);
-    led_on(LED_YELLOW);
-}
-
 int
 main()
 {
diff --git a/projects/cli-test/filetransfer b/projects/cli-test/filetransfer
index 025a6ac..f704e31 100755
--- a/projects/cli-test/filetransfer
+++ b/projects/cli-test/filetransfer
@@ -77,11 +77,9 @@ def parse_args():
 
 
 def _write(dst, data):
-    for i in range(len(data)):
-        dst.write(data[i])
-        time.sleep(0.1)
+    dst.write(data)
     if len(data) == 4:
-        print("Wrote 0x{:02x}{:02x}{:02x}{:02x}".format(ord(data[0]), ord(data[1]), ord(data[2]), ord(data[3])))
+        print("Wrote 0x{!s}".format(data.encode('hex')))
     else:
         print("Wrote {!r}".format(data))
 
diff --git a/projects/cli-test/mgmt-cli.c b/projects/cli-test/mgmt-cli.c
index abbd999..3b4ffe9 100644
--- a/projects/cli-test/mgmt-cli.c
+++ b/projects/cli-test/mgmt-cli.c
@@ -41,18 +41,17 @@
 extern uint8_t uart_rx;
 
 struct uart_ringbuf_t {
-    uint32_t enabled, ridx, widx, overflow;
+    uint32_t enabled, ridx;
+    enum mgmt_cli_dma_state rx_state;
     uint8_t buf[CLI_UART_RECVBUF_SIZE];
 };
 
-volatile struct uart_ringbuf_t uart_ringbuf = {1, 0, 0, 0, {0}};
+volatile struct uart_ringbuf_t uart_ringbuf = {1, 0, DMA_RX_STOP, {0}};
 
 #define RINGBUF_RIDX(rb)       (rb.ridx & CLI_UART_RECVBUF_MASK)
-#define RINGBUF_WIDX(rb)       (rb.widx & CLI_UART_RECVBUF_MASK)
-#define RINGBUF_COUNT(rb)      ((unsigned)(rb.widx - rb.ridx))
-#define RINGBUF_FULL(rb)       (RINGBUF_RIDX(rb) == ((rb.widx + 1) & CLI_UART_RECVBUF_MASK))
+#define RINGBUF_WIDX(rb)       (sizeof(rb.buf) - __HAL_DMA_GET_COUNTER(huart_mgmt.hdmarx))
+#define RINGBUF_COUNT(rb)      ((unsigned)(RINGBUF_WIDX(rb) - RINGBUF_RIDX(rb)))
 #define RINGBUF_READ(rb, dst)  {dst = rb.buf[RINGBUF_RIDX(rb)]; rb.buf[RINGBUF_RIDX(rb)] = '.'; rb.ridx++;}
-#define RINGBUF_WRITE(rb, src) {rb.buf[RINGBUF_WIDX(rb)] = src; rb.widx++;}
 
 void uart_cli_print(struct cli_def *cli __attribute__ ((unused)), const char *buf)
 {
@@ -64,23 +63,16 @@ void uart_cli_print(struct cli_def *cli __attribute__ ((unused)), const char *bu
 int uart_cli_read(struct cli_def *cli __attribute__ ((unused)), void *buf, size_t count)
 {
     uint32_t timeout = 0xffffff;
-
-    /* Always explicitly enable the RX interrupt when we get here.
-     * Prevents us getting stuck waiting for an interrupt that will never come.
-     */
-    __HAL_UART_FLUSH_DRREGISTER(&huart_mgmt);
-    HAL_UART_Receive_IT(&huart_mgmt, (uint8_t *) &uart_rx, 1);
-
-    while (count && timeout--) {
+    while (count && timeout) {
 	if (RINGBUF_COUNT(uart_ringbuf)) {
 	    RINGBUF_READ(uart_ringbuf, *(uint8_t *) buf);
 	    buf++;
 	    count--;
-	} else {
-	    led_toggle(LED_GREEN);
-	    HAL_Delay(10);
 	}
+	timeout--;
     }
+    if (! timeout) return 0;
+
     return 1;
 }
 
@@ -90,6 +82,26 @@ int uart_cli_write(struct cli_def *cli __attribute__ ((unused)), const void *buf
     return (int) count;
 }
 
+int control_mgmt_uart_dma_rx(enum mgmt_cli_dma_state state)
+{
+    if (state == DMA_RX_START) {
+	if (uart_ringbuf.rx_state != DMA_RX_START) {
+	    memset((void *) uart_ringbuf.buf, 0, sizeof(uart_ringbuf.buf));
+
+	    /* Start receiving data from the UART using DMA */
+	    HAL_UART_Receive_DMA(&huart_mgmt, (uint8_t *) uart_ringbuf.buf, sizeof(uart_ringbuf.buf));
+	    uart_ringbuf.ridx = 0;
+	    uart_ringbuf.rx_state = DMA_RX_START;
+	}
+	return 1;
+    } else if (state == DMA_RX_STOP) {
+	if (HAL_UART_DMAStop(&huart_mgmt) != HAL_OK) return 0;
+	uart_ringbuf.rx_state = DMA_RX_STOP;
+	return 1;
+    }
+    return 0;
+}
+
 int embedded_cli_loop(struct cli_def *cli)
 {
     unsigned char c;
@@ -110,15 +122,18 @@ int embedded_cli_loop(struct cli_def *cli)
     while (1) {
 	cli_loop_start_new_command(cli, &ctx);
 
+	control_mgmt_uart_dma_rx(DMA_RX_START);
+
 	while (1) {
 	    cli_loop_show_prompt(cli, &ctx);
 
 	    n = cli_loop_read_next_char(cli, &ctx, &c);
 
 	    /*
-	    cli_print(cli, "Next char: '%c'/%i, ringbuf ridx %i, widx %i (%i/%i) - count %i",
-		      c, (int) c, uart_ringbuf.ridx, uart_ringbuf.widx, RINGBUF_RIDX(uart_ringbuf),
-		      RINGBUF_WIDX(uart_ringbuf), RINGBUF_COUNT(uart_ringbuf));
+	    cli_print(cli, "Next char: '%c'/%i, ringbuf ridx %i, widx %i",
+		      c, (int) c,
+		      uart_ringbuf.ridx,
+		      RINGBUF_WIDX(uart_ringbuf)
 	    */
 	    if (n == CLI_LOOP_CTRL_BREAK)
 		break;
@@ -143,22 +158,6 @@ int embedded_cli_loop(struct cli_def *cli)
     return CLI_OK;
 }
 
-/* Interrupt service routine to be called when data has been received on the MGMT UART. */
-void mgmt_cli_uart_isr(const uint8_t *buf, size_t count)
-{
-    if (! uart_ringbuf.enabled) return;
-
-    while (count) {
-	if (RINGBUF_FULL(uart_ringbuf)) {
-	    uart_ringbuf.overflow++;
-	    return;
-	}
-	RINGBUF_WRITE(uart_ringbuf, *buf);
-	buf++;
-	count--;
-    }
-}
-
 void mgmt_cli_init(struct cli_def *cli)
 {
     cli_init(cli);
diff --git a/projects/cli-test/mgmt-cli.h b/projects/cli-test/mgmt-cli.h
index 3b7f503..16c9fbd98 100644
--- a/projects/cli-test/mgmt-cli.h
+++ b/projects/cli-test/mgmt-cli.h
@@ -71,11 +71,16 @@
 #define CLI_UART_RECVBUF_SIZE  256  /* This must be a power of 2 */
 #define CLI_UART_RECVBUF_MASK  (CLI_UART_RECVBUF_SIZE - 1)
 
+enum mgmt_cli_dma_state {
+    DMA_RX_STOP,
+    DMA_RX_START,
+};
+
 extern void uart_cli_print(struct cli_def *cli __attribute__ ((unused)), const char *buf);
 extern int uart_cli_read(struct cli_def *cli __attribute__ ((unused)), void *buf, size_t count);
 extern int uart_cli_write(struct cli_def *cli __attribute__ ((unused)), const void *buf, size_t count);
 extern int embedded_cli_loop(struct cli_def *cli);
 extern void mgmt_cli_init(struct cli_def *cli);
-extern void mgmt_cli_uart_isr(const uint8_t *buf, size_t count);
+extern int control_mgmt_uart_dma_rx(enum mgmt_cli_dma_state state);
 
 #endif /* __STM32_MGMT_CLI_H */
diff --git a/projects/cli-test/mgmt-fpga.c b/projects/cli-test/mgmt-fpga.c
index c20b7b2..8c1b2a8 100644
--- a/projects/cli-test/mgmt-fpga.c
+++ b/projects/cli-test/mgmt-fpga.c
@@ -35,21 +35,29 @@
 #include "stm-init.h"
 #include "stm-uart.h"
 #include "stm-fpgacfg.h"
+
 #include "mgmt-cli.h"
 #include "mgmt-fpga.h"
+#include "mgmt-misc.h"
 
 #include <string.h>
 
 
-extern uint32_t update_crc(uint32_t crc, uint8_t *buf, int len);
+volatile uint32_t dfu_offset = 0;
+
 
+int _flash_write_callback(uint8_t *buf, size_t len) {
+    int res = fpgacfg_write_data(dfu_offset, buf, BITSTREAM_UPLOAD_CHUNK_SIZE) == 1;
+    dfu_offset += BITSTREAM_UPLOAD_CHUNK_SIZE;
+    return res;
+}
 
 int cmd_fpga_bitstream_upload(struct cli_def *cli, const char *command, char *argv[], int argc)
 {
-    uint32_t filesize = 0, crc = 0, my_crc = 0, counter = 0, i;
-    uint32_t offset = 0, n = BITSTREAM_UPLOAD_CHUNK_SIZE;
     uint8_t buf[BITSTREAM_UPLOAD_CHUNK_SIZE];
 
+    dfu_offset = 0;
+
     fpgacfg_access_control(ALLOW_ARM);
 
     cli_print(cli, "Checking if FPGA config memory is accessible");
@@ -58,57 +66,11 @@ int cmd_fpga_bitstream_upload(struct cli_def *cli, const char *command, char *ar
 	return CLI_ERROR;
     }
 
-    cli_print(cli, "OK, write FPGA bitstream file size (4 bytes), data in 4096 byte chunks, CRC-32 (4 bytes)");
-
-    /* Read file size (4 bytes) */
-    uart_receive_bytes(STM_UART_MGMT, (void *) &filesize, 4, 1000);
-    cli_print(cli, "File size %li", filesize);
-
-    while (filesize) {
-	/* By initializing buf to the same value that erased flash has (0xff), we don't
-	 * have to try and be smart when writing the last page of data to the memory.
-	 */
-	memset(buf, 0xff, sizeof(buf));
-
-	if (filesize < n) {
-	    n = filesize;
-	}
-
-	if (uart_receive_bytes(STM_UART_MGMT, (void *) &buf, n, 1000) != HAL_OK) {
-	    cli_print(cli, "Receive timed out");
-	    return CLI_ERROR;
-	}
-	filesize -= n;
-
-	/* After reception of 4 KB but before ACKing we have "all" the time in the world to
-	 * calculate CRC and write it to flash.
-	 */
-	my_crc = update_crc(my_crc, buf, n);
-
-	if ((i = fpgacfg_write_data(offset, buf, BITSTREAM_UPLOAD_CHUNK_SIZE)) != 1) {
-	    cli_print(cli, "Failed writing data at offset %li (counter = %li): %li", offset, counter, i);
-	    return CLI_ERROR;
-	}
-
-	offset += BITSTREAM_UPLOAD_CHUNK_SIZE;
-
-	/* ACK this chunk by sending the current chunk counter (4 bytes) */
-	counter++;
-	uart_send_bytes(STM_UART_MGMT, (void *) &counter, 4);
-    }
-
-    /* The sending side will now send it's calculated CRC-32 */
-    cli_print(cli, "Send CRC-32");
-    uart_receive_bytes(STM_UART_MGMT, (void *) &crc, 4, 1000);
-    cli_print(cli, "CRC-32 %li", crc);
-    if (crc == my_crc) {
-	cli_print(cli, "CRC checksum MATCHED");
-    } else {
-	cli_print(cli, "CRC checksum did NOT match");
-    }
+    cli_receive_data(cli, &buf[0], sizeof(buf), _flash_write_callback);
 
     fpgacfg_access_control(ALLOW_FPGA);
 
+    cli_print(cli, "DFU offset now: %li (%li chunks)", dfu_offset, dfu_offset / BITSTREAM_UPLOAD_CHUNK_SIZE);
     return CLI_OK;
 }
 
diff --git a/projects/cli-test/mgmt-misc.c b/projects/cli-test/mgmt-misc.c
index a98bf8d..aea790a 100644
--- a/projects/cli-test/mgmt-misc.c
+++ b/projects/cli-test/mgmt-misc.c
@@ -43,34 +43,64 @@
 
 extern uint32_t update_crc(uint32_t crc, uint8_t *buf, int len);
 
-int cmd_filetransfer(struct cli_def *cli, const char *command, char *argv[], int argc)
+
+volatile uint32_t demo_crc = 0;
+
+
+int _count_bytes_callback(uint8_t *buf, size_t len) {
+    demo_crc = update_crc(demo_crc, buf, len);
+    return 1;
+}
+
+int cli_receive_data(struct cli_def *cli, uint8_t *buf, size_t len, cli_data_callback data_callback)
 {
-    uint32_t filesize = 0, crc = 0, my_crc = 0, n = 256, counter = 0;
-    uint8_t buf[256];
+    uint32_t filesize = 0, crc = 0, my_crc = 0, counter = 0;
+    size_t n = len;
 
-    cli_print(cli, "OK, write file size (4 bytes), data in %li byte chunks, CRC-32 (4 bytes)", n);
+    if (! control_mgmt_uart_dma_rx(DMA_RX_STOP)) {
+	cli_print(cli, "Failed stopping DMA");
+	return CLI_OK;
+    }
+
+    cli_print(cli, "OK, write size (4 bytes), data in %li byte chunks, CRC-32 (4 bytes)", (uint32_t) n);
 
-    uart_receive_bytes(STM_UART_MGMT, (void *) &filesize, 4, 1000);
-    cli_print(cli, "File size %li", filesize);
+    if (uart_receive_bytes(STM_UART_MGMT, (void *) &filesize, 4, 1000) != HAL_OK) {
+	cli_print(cli, "Receive timed out");
+	return CLI_ERROR;
+    }
+
+    cli_print(cli, "Send %li bytes of data", filesize);
 
     while (filesize) {
-	if (filesize < n) {
-	    n = filesize;
-	}
+	/* By initializing buf to the same value that erased flash has (0xff), we don't
+	 * have to try and be smart when writing the last page of data to a flash memory.
+	 */
+	memset(buf, 0xff, len);
 
-	if (uart_receive_bytes(STM_UART_MGMT, (void *) &buf, n, 1000) != HAL_OK) {
+	if (filesize < n) n = filesize;
+
+	if (uart_receive_bytes(STM_UART_MGMT, (void *) buf, n, 1000) != HAL_OK) {
 	    cli_print(cli, "Receive timed out");
 	    return CLI_ERROR;
 	}
 	filesize -= n;
 	my_crc = update_crc(my_crc, buf, n);
+
+	/* After reception of a chunk but before ACKing we have "all" the time in the world to
+	 * calculate CRC and invoke the data_callback.
+	 */
+	if (data_callback != NULL && ! data_callback(buf, (size_t) n)) {
+	    cli_print(cli, "Data processing failed");
+	    return CLI_OK;
+	}
+
 	counter++;
 	uart_send_bytes(STM_UART_MGMT, (void *) &counter, 4);
     }
 
     cli_print(cli, "Send CRC-32");
     uart_receive_bytes(STM_UART_MGMT, (void *) &crc, 4, 1000);
-    cli_print(cli, "CRC-32 %li", crc);
+    cli_print(cli, "CRC-32 0x%x, calculated CRC 0x%x", (unsigned int) crc, (unsigned int) my_crc);
     if (crc == my_crc) {
 	cli_print(cli, "CRC checksum MATCHED");
     } else {
@@ -80,6 +110,16 @@ int cmd_filetransfer(struct cli_def *cli, const char *command, char *argv[], int
     return CLI_OK;
 }
 
+int cmd_filetransfer(struct cli_def *cli, const char *command, char *argv[], int argc)
+{
+    uint8_t buf[FILETRANSFER_UPLOAD_CHUNK_SIZE];
+
+    demo_crc = 0;
+    cli_receive_data(cli, &buf[0], sizeof(buf), _count_bytes_callback);
+    cli_print(cli, "Demo CRC is: %li/0x%x", demo_crc, (unsigned int) demo_crc);
+    return CLI_OK;
+}
+
 int cmd_reboot(struct cli_def *cli, const char *command, char *argv[], int argc)
 {
     cli_print(cli, "\n\n\nRebooting\n\n\n");
diff --git a/projects/cli-test/mgmt-misc.h b/projects/cli-test/mgmt-misc.h
index caf7024..b7eb4f4 100644
--- a/projects/cli-test/mgmt-misc.h
+++ b/projects/cli-test/mgmt-misc.h
@@ -38,6 +38,12 @@
 #include "stm-init.h"
 #include <libcli.h>
 
+
+#define FILETRANSFER_UPLOAD_CHUNK_SIZE 256
+
+typedef int (*cli_data_callback)(uint8_t *, size_t);
+
 extern void configure_cli_misc(struct cli_def *cli);
+extern int cli_receive_data(struct cli_def *cli, uint8_t *buf, size_t len, cli_data_callback data_callback);
 
 #endif /* __STM32_CLI_MGMT_MISC_H */