USART: Add DMA based receiver

This DMA receiver uses a separate small circular buffer to DMA into.
This allows the DMA transaction to be made circular, and thus it does
not require the interrupt latency to be low enough to setup the next
transfer before the next character comes in.

Additional diagnostics output have been added to the usart_info console
command to facilitate tuning of the FIFO size.

Signed-off-by: Anton Staaf <robotboy@chromium.org>

BRANCH=None
BUG=None
TEST=make buildall -j
     Verify DMA works by cross connecting two discovery boards

Change-Id: Idcdf95a47fadf21ec2154f0c9128cd3586e568ec
Reviewed-on: https://chromium-review.googlesource.com/292870
Tested-by: Anton Staaf <robotboy@chromium.org>
Reviewed-by: Anton Staaf <robotboy@chromium.org>
Commit-Queue: Anton Staaf <robotboy@chromium.org>
Trybot-Ready: Anton Staaf <robotboy@chromium.org>

11 files changed, 240 insertions, 3 deletions

diff --git a/board/discovery-stm32f072/board.c b/board/discovery-stm32f072/board.c
index 5aa185ffc3..360f970001 100644
--- a/board/discovery-stm32f072/board.c
+++ b/board/discovery-stm32f072/board.c
@@ -14,6 +14,7 @@
 #include "task.h"
 #include "usart-stm32f0.h"
 #include "usart_tx_dma.h"
+#include "usart_rx_dma.h"
 #include "usb_gpio.h"
 #include "usb_spi.h"
 #include "usb-stream.h"
@@ -53,12 +54,15 @@ static struct queue const loopback_queue =
 		     loopback_usart.producer,
 		     loopback_usart.consumer);
 
+static struct usart_rx_dma const loopback_rx_dma =
+	USART_RX_DMA(STM32_DMAC_CH3, 8);
+
 static struct usart_tx_dma const loopback_tx_dma =
 	USART_TX_DMA(STM32_DMAC_CH2, 16);
 
 static struct usart_config const loopback_usart =
 	USART_CONFIG(usart1_hw,
-		     usart_rx_interrupt,
+		     loopback_rx_dma.usart_rx,
 		     loopback_tx_dma.usart_tx,
 		     115200,
 		     loopback_queue,
diff --git a/board/discovery/board.c b/board/discovery/board.c
index eb58434aa7..81fe437850 100644
--- a/board/discovery/board.c
+++ b/board/discovery/board.c
@@ -11,6 +11,7 @@
 #include "registers.h"
 #include "task.h"
 #include "usart-stm32f0.h"
+#include "usart_rx_dma.h"
 #include "usart_tx_dma.h"
 #include "util.h"
 
@@ -43,12 +44,15 @@ static struct queue const loopback_queue =
 		     loopback_usart.producer,
 		     loopback_usart.consumer);
 
+static struct usart_rx_dma const loopback_rx_dma =
+	USART_RX_DMA(STM32_DMAC_CH6, 32);
+
 static struct usart_tx_dma const loopback_tx_dma =
 	USART_TX_DMA(STM32_DMAC_CH7, 16);
 
 static struct usart_config const loopback_usart =
 	USART_CONFIG(usart2_hw,
-		     usart_rx_interrupt,
+		     loopback_rx_dma.usart_rx,
 		     loopback_tx_dma.usart_tx,
 		     115200,
 		     loopback_queue,
diff --git a/chip/stm32/build.mk b/chip/stm32/build.mk
index 0af797f0e0..1c1df1a789 100644
--- a/chip/stm32/build.mk
+++ b/chip/stm32/build.mk
@@ -37,7 +37,7 @@ chip-$(CONFIG_I2C)+=i2c-$(CHIP_FAMILY).o
 chip-$(CONFIG_STREAM_USART)+=usart.o usart-$(CHIP_FAMILY).o
 chip-$(CONFIG_STREAM_USART)+=usart_rx_interrupt-$(CHIP_FAMILY).o
 chip-$(CONFIG_STREAM_USART)+=usart_tx_interrupt.o
-chip-$(CONFIG_STREAM_USART)+=usart_tx_dma.o
+chip-$(CONFIG_STREAM_USART)+=usart_rx_dma.o usart_tx_dma.o
 chip-$(CONFIG_CMD_USART_INFO)+=usart_info_command.o
 chip-$(CONFIG_STREAM_USB)+=usb-stream.o
 chip-$(CONFIG_WATCHDOG)+=watchdog.o
diff --git a/chip/stm32/usart.h b/chip/stm32/usart.h
index 8db3558b50..875dbfdc76 100644
--- a/chip/stm32/usart.h
+++ b/chip/stm32/usart.h
@@ -24,6 +24,12 @@ struct usart_state {
 	 * in the RX queue.
 	 */
 	uint32_t rx_dropped;
+
+	/*
+	 * Counter of the number of times an receive overrun condition is
+	 * detected.  This will not usually be a count of the number of bytes
+	 * that were lost due to overrun conditions.
+	 */
 	uint32_t rx_overrun;
 };
 
@@ -59,6 +65,13 @@ struct usart_rx {
 	void (*init)(struct usart_config const *config);
 	void (*interrupt)(struct usart_config const *config);
 
+	/*
+	 * Print to the console any per-strategy diagnostic information, this
+	 * is used by the usart_info command.  This can be NULL if there is
+	 * nothing interesting to display.
+	 */
+	void (*info)(struct usart_config const *config);
+
 	struct producer_ops producer_ops;
 };
 
@@ -66,6 +79,13 @@ struct usart_tx {
 	void (*init)(struct usart_config const *config);
 	void (*interrupt)(struct usart_config const *config);
 
+	/*
+	 * Print to the console any per-strategy diagnostic information, this
+	 * is used by the usart_info command.  This can be NULL if there is
+	 * nothing interesting to display.
+	 */
+	void (*info)(struct usart_config const *config);
+
 	struct consumer_ops consumer_ops;
 };
 
diff --git a/chip/stm32/usart_info_command.c b/chip/stm32/usart_info_command.c
index 7b949b10c6..836dc6d84c 100644
--- a/chip/stm32/usart_info_command.c
+++ b/chip/stm32/usart_info_command.c
@@ -26,6 +26,12 @@ static int command_usart_info(int argc, char **argv)
 			 config->hw->index + 1,
 			 atomic_read_clear(&(config->state->rx_dropped)),
 			 atomic_read_clear(&(config->state->rx_overrun)));
+
+		if (config->rx->info)
+			config->rx->info(config);
+
+		if (config->tx->info)
+			config->tx->info(config);
 	}
 
 	return EC_SUCCESS;
diff --git a/chip/stm32/usart_rx_dma.c b/chip/stm32/usart_rx_dma.c
new file mode 100644
index 0000000000..0e26763de6
--- /dev/null
+++ b/chip/stm32/usart_rx_dma.c
@@ -0,0 +1,90 @@
+/* Copyright 2015 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "usart_rx_dma.h"
+
+#include "atomic.h"
+#include "common.h"
+#include "console.h"
+#include "registers.h"
+#include "system.h"
+#include "util.h"
+
+void usart_rx_dma_init(struct usart_config const *config)
+{
+	struct usart_rx_dma const *dma_config =
+		DOWNCAST(config->rx, struct usart_rx_dma const, usart_rx);
+
+	intptr_t base = config->hw->base;
+
+	struct dma_option options = {
+		.channel = dma_config->channel,
+		.periph  = (void *)&STM32_USART_RDR(base),
+		.flags   = (STM32_DMA_CCR_MSIZE_8_BIT |
+			    STM32_DMA_CCR_PSIZE_8_BIT |
+			    STM32_DMA_CCR_CIRC),
+	};
+
+	STM32_USART_CR1(base) |= STM32_USART_CR1_RXNEIE;
+	STM32_USART_CR1(base) |= STM32_USART_CR1_RE;
+	STM32_USART_CR3(base) |= STM32_USART_CR3_DMAR;
+
+	dma_config->state->index     = 0;
+	dma_config->state->max_bytes = 0;
+
+	dma_start_rx(&options, dma_config->fifo_size, dma_config->fifo_buffer);
+}
+
+void usart_rx_dma_interrupt(struct usart_config const *config)
+{
+	struct usart_rx_dma const *dma_config =
+		DOWNCAST(config->rx, struct usart_rx_dma const, usart_rx);
+
+	dma_chan_t *channel  = dma_get_channel(dma_config->channel);
+	size_t     new_index = dma_bytes_done(channel, dma_config->fifo_size);
+	size_t     old_index = dma_config->state->index;
+	size_t     new_bytes = 0;
+	size_t     added     = 0;
+
+	if (new_index > old_index) {
+		new_bytes = new_index - old_index;
+
+		added = queue_add_units(config->producer.queue,
+					dma_config->fifo_buffer + old_index,
+					new_bytes);
+	} else if (new_index < old_index) {
+		/*
+		 * Handle the case where the received bytes are not contiguous
+		 * in the circular DMA buffer.  This is done with two queue
+		 * adds.
+		 */
+		new_bytes = dma_config->fifo_size - (old_index - new_index);
+
+		added = queue_add_units(config->producer.queue,
+					dma_config->fifo_buffer + old_index,
+					dma_config->fifo_size - old_index) +
+			queue_add_units(config->producer.queue,
+					dma_config->fifo_buffer,
+					new_index);
+	} else {
+		/* (new_index == old_index): nothing to add to the queue. */
+	}
+
+	atomic_add(&config->state->rx_dropped, new_bytes - added);
+
+	if (dma_config->state->max_bytes < new_bytes)
+		dma_config->state->max_bytes = new_bytes;
+
+	dma_config->state->index = new_index;
+}
+
+void usart_rx_dma_info(struct usart_config const *config)
+{
+	struct usart_rx_dma const *dma_config =
+		DOWNCAST(config->rx, struct usart_rx_dma const, usart_rx);
+
+	ccprintf("    DMA RX max_bytes %d\n",
+		 atomic_read_clear(&dma_config->state->max_bytes));
+}
diff --git a/chip/stm32/usart_rx_dma.h b/chip/stm32/usart_rx_dma.h
new file mode 100644
index 0000000000..8056c2635e
--- /dev/null
+++ b/chip/stm32/usart_rx_dma.h
@@ -0,0 +1,109 @@
+/* Copyright 2015 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * Hybrid DMA/Interrupt based USART RX driver for STM32
+ */
+#ifndef __CROS_EC_USART_RX_DMA_H
+#define __CROS_EC_USART_RX_DMA_H
+
+#include "producer.h"
+#include "dma.h"
+#include "queue.h"
+#include "usart.h"
+
+/*
+ * Only reference the usart_rx_dma_info function if CONFIG_CMD_USART_INFO
+ * is defined.  This allows the compiler to remove this function as dead code
+ * when CONFIG_CMD_USART_INFO is not defined.
+ */
+#ifdef CONFIG_CMD_USART_INFO
+#define USART_RX_DMA_INFO usart_rx_dma_info
+#else
+#define USART_RX_DMA_INFO NULL
+#endif
+
+/*
+ * Construct a USART RX instance for DMA using the given DMA channel.
+ *
+ * This macro creates a new usart_rx_dma struct, complete with in RAM state,
+ * the contained usart_rx struct can be used in initializing a usart_config
+ * struct.
+ *
+ * CHANNEL is the DMA channel to be used for reception.  This must be a valid
+ * DMA channel for the USART peripheral and any alternate channel mappings must
+ * be handled by the board specific code.
+ *
+ * FIFO_SIZE is the number of bytes (which does not need to be a power of two)
+ * to use for the DMA circular buffer.  This buffer must be large enough to
+ * hide the worst case interrupt latency the system will encounter.  The DMA
+ * RX driver adds to the output of the usart_info command a high water mark
+ * of how many bytes were transfered out of this FIFO on any one interrupt.
+ * This value can be used to correctly size the FIFO by setting the FIFO_SIZE
+ * to something large, stress test the USART, and run usart_info.  After a
+ * reasonable stress test the "DMA RX max_bytes" value will be a reasonable
+ * size for the FIFO (perhaps +10% for safety).
+ */
+#define USART_RX_DMA(CHANNEL, FIFO_SIZE)				\
+	((struct usart_rx_dma const) {					\
+		.usart_rx = {						\
+			.producer_ops = {				\
+				.read = NULL,				\
+			},						\
+									\
+			.init      = usart_rx_dma_init,			\
+			.interrupt = usart_rx_dma_interrupt,		\
+			.info      = USART_RX_DMA_INFO,			\
+		},							\
+									\
+		.state       = &((struct usart_rx_dma_state) {}),	\
+		 .fifo_buffer = ((uint8_t[FIFO_SIZE]) {}),		\
+		 .fifo_size   = FIFO_SIZE,				\
+		 .channel     = CHANNEL,				\
+	})
+
+/*
+ * In RAM state required to manage DMA based transmission.
+ */
+struct usart_rx_dma_state {
+	/*
+	 * Previous value of dma_bytes_done.  This will wrap when the DMA fills
+	 * the queue.
+	 */
+	size_t index;
+
+	/*
+	 * Maximum number of bytes transfered in any one RX interrupt.
+	 */
+	uint32_t max_bytes;
+};
+
+/*
+ * Extension of the usart_rx struct to include required configuration for
+ * DMA based transmission.
+ */
+struct usart_rx_dma {
+	struct usart_rx usart_rx;
+
+	struct usart_rx_dma_state volatile *state;
+
+	uint8_t *fifo_buffer;
+	size_t  fifo_size;
+
+	enum dma_channel channel;
+};
+
+/*
+ * Function pointers needed to intialize a usart_rx struct.  These shouldn't
+ * be called in any other context as they assume that the producer or config
+ * that they are passed was initialized with a complete usart_rx_dma struct.
+ */
+void usart_rx_dma_init(struct usart_config const *config);
+void usart_rx_dma_interrupt(struct usart_config const *config);
+
+/*
+ * Debug function, used to print DMA RX statistics to the console.
+ */
+void usart_rx_dma_info(struct usart_config const *config);
+
+#endif /* __CROS_EC_USART_RX_DMA_H */
diff --git a/chip/stm32/usart_rx_interrupt-stm32l.c b/chip/stm32/usart_rx_interrupt-stm32l.c
index e6222d1716..fbda76327a 100644
--- a/chip/stm32/usart_rx_interrupt-stm32l.c
+++ b/chip/stm32/usart_rx_interrupt-stm32l.c
@@ -61,4 +61,5 @@ struct usart_rx const usart_rx_interrupt = {
 
 	.init      = usart_rx_init,
 	.interrupt = usart_rx_interrupt_handler,
+	.info      = NULL,
 };
diff --git a/chip/stm32/usart_rx_interrupt.c b/chip/stm32/usart_rx_interrupt.c
index d4fb48f5a0..5fdaafaa0b 100644
--- a/chip/stm32/usart_rx_interrupt.c
+++ b/chip/stm32/usart_rx_interrupt.c
@@ -45,4 +45,5 @@ struct usart_rx const usart_rx_interrupt = {
 
 	.init      = usart_rx_init,
 	.interrupt = usart_rx_interrupt_handler,
+	.info      = NULL,
 };
diff --git a/chip/stm32/usart_tx_dma.h b/chip/stm32/usart_tx_dma.h
index a1e4e0831d..1dd3d7da41 100644
--- a/chip/stm32/usart_tx_dma.h
+++ b/chip/stm32/usart_tx_dma.h
@@ -39,6 +39,7 @@
 								\
 			.init      = usart_tx_dma_init,		\
 			.interrupt = usart_tx_dma_interrupt,	\
+			.info      = NULL,			\
 		},						\
 								\
 		.state     = &((struct usart_tx_dma_state){}),	\
diff --git a/chip/stm32/usart_tx_interrupt.c b/chip/stm32/usart_tx_interrupt.c
index 60f28d5a1d..ef2443ec96 100644
--- a/chip/stm32/usart_tx_interrupt.c
+++ b/chip/stm32/usart_tx_interrupt.c
@@ -87,4 +87,5 @@ struct usart_tx const usart_tx_interrupt = {
 
 	.init      = usart_tx_init,
 	.interrupt = usart_tx_interrupt_handler,
+	.info      = NULL,
 };