spi: Rewrite driver for new protocol, better performance

The old spi driver has atrophied in various ways. It doesn't support
the new protocol and does not build either.

Rewrite the driver to:

- Use dma for reception (rather than just reception)
    - This makes message reception more robust and allows us to process
      the new multi-byte commands
- Add timeouts for rx and tx so that we don't wait forever
- Increase buffer sizes to deal with new larger messages
- Always send a preamble byte regardless of SPI clock speed
    (previously above 10MHz we sometimes miss this)
- Use the NSS line to delineate transactions. When it drops, a
transaction is starting. When it rises the transaction is immediately
terminates regardless of state. This keeps the AP and EC in sync even
in the event of timeouts, bus errors and other oddities.
- Implement the new protocol which has a checksum, version byte, etc
- Set up tx dma in advance and kick it when ready, thus ensuring that
    a message body is always attached immediately after the preamble
- Use the new host_cmd_handle_args structure, which makes things much
easier for us, since we don't need globals, and can use the
send_response handler to know when a slow command is complete.
- Handle the new type of 'slow' commands properly

BUG=chrome-os-partner:10533
TEST=manual
build and boot to kernel on snow

Change-Id: I11767d1a6f045a86f6c9a0b4b1e943b660e4da33
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/32076
Reviewed-by: Randall Spangler <rspangler@chromium.org>

4 files changed, 246 insertions, 138 deletions

diff --git a/board/snow/board.c b/board/snow/board.c
index e973ab46e3..36dac730cd 100644
--- a/board/snow/board.c
+++ b/board/snow/board.c
@@ -61,7 +61,7 @@ const struct gpio_info gpio_list[GPIO_COUNT] = {
 	{"KB_IN07",     GPIO_D, (1<<2),  GPIO_KB_INPUT, matrix_interrupt},
 	/* Other inputs */
 	{"AC_PWRBTN_L", GPIO_A, (1<<0), GPIO_INT_BOTH, NULL},
-	{"SPI1_NSS",    GPIO_A, (1<<4), GPIO_DEFAULT, NULL},
+	{"SPI1_NSS",    GPIO_A, (1<<4), GPIO_DEFAULT, spi_event},
 	/*
 	 * I2C pins should be configured as inputs until I2C module is
 	 * initialized. This will avoid driving the lines unintentionally.
@@ -109,6 +109,16 @@ void configure_board(void)
 	 */
 	STM32_RCC_APB2ENR |= 0x1fd;
 
+#ifdef CONFIG_SPI
+	/* Enable SPI */
+	STM32_RCC_APB2ENR |= (1<<12);
+
+	/* SPI1 on pins PA4-7 (alt. function push-pull, 10MHz) */
+	val = STM32_GPIO_CRL_OFF(GPIO_A) & ~0xffff0000;
+	val |= 0x99990000;
+	STM32_GPIO_CRL_OFF(GPIO_A) = val;
+#endif
+
 	/* remap OSC_IN/OSC_OUT to PD0/PD1 */
 	STM32_GPIO_AFIO_MAPR |= 1 << 15;
 
@@ -165,6 +175,9 @@ void configure_board_late(void)
 #ifdef CONFIG_AC_POWER_STATUS
 	gpio_set_flags(GPIO_AC_STATUS, GPIO_OUT_HIGH);
 #endif
+#ifdef CONFIG_SPI
+	gpio_set_flags(GPIO_SPI1_NSS, GPIO_INT_BOTH);
+#endif
 }
 
 void board_interrupt_host(int active)
diff --git a/chip/stm32/registers.h b/chip/stm32/registers.h
index 4da23af1c4..612dce10f9 100644
--- a/chip/stm32/registers.h
+++ b/chip/stm32/registers.h
@@ -397,6 +397,17 @@ static inline uint16_t *stm32_i2c_reg(int port, int offset)
 #define STM32_SPI1_PORT             0
 #define STM32_SPI2_PORT             1
 
+/* The SPI controller registers */
+struct spi_ctlr {
+	unsigned ctrl1;
+	unsigned ctrl2;
+	unsigned stat;
+	unsigned data;
+	unsigned crcp;
+	unsigned rxcrc;
+	unsigned txcrc;
+};
+
 /*
  * TODO(vpalatin):
  * For whatever reason, our toolchain is substandard and generate a
@@ -404,19 +415,8 @@ static inline uint16_t *stm32_i2c_reg(int port, int offset)
  *
  * That's why I have not used inline stuff in the registers definition.
  */
-#define stm32_spi_addr(port, offset) \
-	((port == 0) ? \
-		(STM32_SPI1_BASE + offset) : \
-		(STM32_SPI2_BASE + offset))
-
-#define STM32_SPI_REG16(p, l)       REG16(stm32_spi_addr((p), l))
-#define STM32_SPI_CR1(p)            STM32_SPI_REG16((p), 0x00)
-#define STM32_SPI_CR2(p)            STM32_SPI_REG16((p), 0x04)
-#define STM32_SPI_SR(p)             STM32_SPI_REG16((p), 0x08)
-#define STM32_SPI_DR(p)             STM32_SPI_REG16((p), 0x0c)
-#define STM32_SPI_CRCPR(p)          STM32_SPI_REG16((p), 0x10)
-#define STM32_SPI_RXCRCR(p)         STM32_SPI_REG16((p), 0x14)
-#define STM32_SPI_TXCRCR(p)         STM32_SPI_REG16((p), 0x18)
+#define stm32_spi_addr(port) \
+	((struct spi_ctlr *)(port == 0 ? STM32_SPI1_BASE : STM32_SPI2_BASE))
 
 /* --- Debug --- */
 
diff --git a/chip/stm32/spi.c b/chip/stm32/spi.c
index 5b59208317..6e518e7d1e 100644
--- a/chip/stm32/spi.c
+++ b/chip/stm32/spi.c
@@ -1,10 +1,11 @@
-/* Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+/*
+ * Copyright (c) 2012 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.
  *
  * SPI driver for Chrome EC.
  *
- * This uses DMA although not in an optimal way yet.
+ * This uses DMA to handle transmission and reception.
  */
 
 #include "console.h"
@@ -14,10 +15,12 @@
 #include "host_command.h"
 #include "registers.h"
 #include "spi.h"
-#include "task.h"
 #include "timer.h"
 #include "util.h"
 
+/* Console output macros */
+#define CPUTS(outstr) cputs(CC_SPI, outstr)
+#define CPRINTF(format, args...) cprintf(CC_SPI, format, ## args)
 
 /* Status register flags that we use */
 enum {
@@ -36,54 +39,74 @@ enum {
  * Since message.c no longer supports our protocol, we must do it all here.
  *
  * We allow a preamble and a header byte so that SPI can function at all.
- * We also add a 16-bit length so that we can tell that we got the whole
+ * We also add an 8-bit length so that we can tell that we got the whole
  * message, since the master decides how many bytes to read.
  */
 enum {
 	/* The bytes which appear before the header in a message */
-	SPI_MSG_PREAMBLE	= 0xff,
+	SPI_MSG_PREAMBLE_BYTE	= 0xff,
 
 	/* The header byte, which follows the preamble */
-	SPI_MSG_HEADER		= 0xec,
+	SPI_MSG_HEADER_BYTE1	= 0xfe,
+	SPI_MSG_HEADER_BYTE2	= 0xec,
+
+	SPI_MSG_HEADER_LEN	= 4,
+	SPI_MSG_TRAILER_LEN	= 2,
+	SPI_MSG_PROTO_LEN	= SPI_MSG_HEADER_LEN + SPI_MSG_TRAILER_LEN,
 
-	SPI_MSG_HEADER_BYTES	= 3,
-	SPI_MSG_TRAILER_BYTES	= 2,
-	SPI_MSG_PROTO_BYTES	= SPI_MSG_HEADER_BYTES + SPI_MSG_TRAILER_BYTES,
+	/*
+	 * Timeout to wait for SPI command
+	 * TODO(sjg@chromium.org): Support much slower SPI clocks. For 4096
+	 * we have a delay of 4ms. For the largest message (68 bytes) this
+	 * is 130KhZ, assuming that the master starts sending bytes as soon
+	 * as it drops NSS. In practice, this timeout seems adequately high
+	 * for a 1MHz clock which is as slow as we would reasonably want it.
+	 */
+	SPI_CMD_RX_TIMEOUT_US	= 8192,
 };
 
 /*
  * Our input and output buffers. These must be large enough for our largest
  * message, including protocol overhead.
  */
-static char out_msg[32];
-static char in_msg[32];
+static uint8_t out_msg[EC_HOST_PARAM_SIZE + SPI_MSG_PROTO_LEN];
+static uint8_t in_msg[EC_HOST_PARAM_SIZE + SPI_MSG_PROTO_LEN];
+static uint8_t active;
+static struct host_cmd_handler_args args;
 
 /**
- * Monitor the SPI bus
+ * Wait until we have received a certain number of bytes
  *
- * At present this function is very simple - it hangs the system until we
- * have sent the response, then clears things away. This is equivalent to
- * not using DMA at all.
+ * Watch the DMA receive channel until it has the required number of bytes,
+ * or a timeout occurs
  *
- * TODO(sjg): Use an interrupt on NSS to triggler this function.
+ * We keep an eye on the NSS line - if this goes high then the transaction is
+ * over so there is no point in trying to receive the bytes.
  *
+ * @param rxdma		RX DMA channel to watch
+ * @param needed	Number of bytes that are needed
+ * @param nss_regs	GPIO register for NSS control line
+ * @param nss_mask	Bit to check in GPIO register (when high, we abort)
+ * @return 0 if bytes received, -1 if we hit a timeout or NSS went high
  */
-void spi_task(void)
+static int wait_for_bytes(struct dma_channel *rxdma, int needed,
+			  uint16_t *nss_reg, uint32_t nss_mask)
 {
-	int port = STM32_SPI1_PORT;
-
-	while (1) {
-		task_wait_event(-1);
-
-		/* Wait for the master to let go of our slave select */
-		while (!gpio_get_level(GPIO_SPI1_NSS))
-			;
-
-		/* Transfer is now complete, so reset everything */
-		dma_disable(DMA_CHANNEL_FOR_SPI_RX(port));
-		dma_disable(DMA_CHANNEL_FOR_SPI_TX(port));
-		STM32_SPI_CR2(port) &= ~CR2_TXDMAEN;
-		STM32_SPI_DR(port) = 0xff;
+	timestamp_t deadline;
+
+	ASSERT(needed <= sizeof(in_msg));
+	deadline.val = 0;
+	for (;;) {
+		if (dma_bytes_done(rxdma, sizeof(in_msg)) >= needed)
+			return 0;
+		if (REG16(nss_reg) & nss_mask)
+			return -1;
+		if (!deadline.val) {
+			deadline = get_time();
+			deadline.val += SPI_CMD_RX_TIMEOUT_US;
+		}
+		if (timestamp_expired(deadline, NULL))
+			return -1;
 	}
 }
 
@@ -95,15 +118,15 @@ void spi_task(void)
  *
  * The format of a reply is a sequence of bytes:
  *
- * <hdr> <len_lo> <len_hi> <msg bytes> <sum> <preamble bytes>
+ * <hdr> <status> <len> <msg bytes> <sum> [<preamble byte>...]
  *
  * The hdr byte is just a tag to indicate that the real message follows. It
  * signals the end of any preamble required by the interface.
  *
- * The 16-bit length is the entire packet size, including the header, length
- * bytes, message payload, checksum, and postamble byte.
+ * The length is the entire packet size, including the header, length bytes,
+ * message payload, checksum, and postamble byte.
  *
- * The preamble is typically 2 bytes, but can be longer if the STM takes ages
+ * The preamble is at least 2 bytes, but can be longer if the STM takes ages
  * to react to the incoming message. Since we send our first byte as the AP
  * sends us the command, we clearly can't send anything sensible for that
  * byte. The second byte must be written to the output register just when the
@@ -114,118 +137,179 @@ void spi_task(void)
  * It is interesting to note that it seems to be possible to run the SPI
  * interface faster than the CPU clock with this approach.
  *
- * @param port		Port to send reply back on (STM32_SPI0/1_PORT)
- * @param msg		Message to send, which starts SPI_MSG_HEADER_BYTES
- *			bytes into the buffer
- * @param msg_len	Length of message in bytes, including checksum
+ * We keep an eye on the NSS line - if this goes high then the transaction is
+ * over so there is no point in trying to send the reply.
+ *
+ * @param spi		SPI controller to send data on
+ * @param txdma		TX DMA channel to send on
+ * @param status	Status result to send
+ * @param msg_ptr	Message payload to send, which normally starts
+ *			SPI_MSG_HEADER_LEN bytes into out_msg
+ * @param msg_len	Number of message bytes to send
  */
-static void reply(int port, char *msg, int msg_len)
+static void reply(struct spi_ctlr *spi, struct dma_channel *txdma,
+		  enum ec_status status, char *msg_ptr, int msg_len)
 {
+	char *msg;
 	int sum, i;
-	int dmac;
+	int copy;
 
-	/* Add our header bytes */
-	msg_len += SPI_MSG_HEADER_BYTES + SPI_MSG_TRAILER_BYTES;
-	msg[0] = SPI_MSG_HEADER;
-	msg[1] = msg_len & 0xff;
-	msg[2] = (msg_len >> 8) & 0xff;
+	msg = out_msg;
+	copy = msg_ptr != msg + SPI_MSG_HEADER_LEN;
+
+	/* Add our header bytes - the first one might not actually be sent */
+	msg[0] = SPI_MSG_HEADER_BYTE1;
+	msg[1] = SPI_MSG_HEADER_BYTE2;
+	msg[2] = status;
+	msg[3] = msg_len & 0xff;
+	sum = status + msg_len;
 
 	/* Calculate the checksum */
-	for (i = sum = 0; i < msg_len - 2; i++)
-		sum += msg[i];
+	for (i = 0; i < msg_len; i++) {
+		int ch;
+
+		ch = msg_ptr[i];
+		sum += ch;
+		if (copy)
+			msg[i + SPI_MSG_HEADER_LEN] = ch;
+	}
+	msg_len += SPI_MSG_PROTO_LEN;
+	ASSERT(msg_len < sizeof(out_msg));
+
+	/* Add the checksum and get ready to send */
 	msg[msg_len - 2] = sum & 0xff;
-	msg[msg_len - 1] = SPI_MSG_PREAMBLE;
+	msg[msg_len - 1] = SPI_MSG_PREAMBLE_BYTE;
+	dma_prepare_tx(txdma, msg_len, (void *)&spi->data, msg);
 
-	/*
-	 * This method is not really suitable for very large messages. If
-	 * we need these, we should set up a second DMA transfer to do
-	 * the message, and then a third to do the trailer, rather than
-	 * copying the message around.
-	 */
-	STM32_SPI_CR2(port) |= CR2_TXDMAEN;
-	dmac = DMA_CHANNEL_FOR_SPI_TX(port);
-	dma_start_tx(dmac, msg_len, (void *)&STM32_SPI_DR(port), out_msg);
+	/* Kick off the DMA to send the data */
+	dma_go(txdma);
 }
 
-/* dummy handler for SPI - will be filled in later */
-static void spi_send_response(struct host_cmd_handler_args *args)
+/**
+ * Get ready to receive a message from the master.
+ *
+ * Set up our RX DMA and disable our TX DMA. Set up the data output so that
+ * we will send preamble bytes.
+ */
+static void setup_for_transaction(struct spi_ctlr *spi)
 {
+	int dmac;
+
+	/* We are no longer actively processing a transaction */
+	active = 0;
+
+	/* write 0xfd which will be our default output value */
+	REG16(&spi->data) = 0xfd;
+	dma_disable(DMA_CHANNEL_FOR_SPI_TX(spi));
+	*in_msg = 0xff;
+
+	/* read a byte in case there is one, and the rx dma gets it */
+	dmac = REG16(&spi->data);
+	dmac = DMA_CHANNEL_FOR_SPI_RX(spi);
+	dma_start_rx(dmac, sizeof(in_msg), (void *)&spi->data, in_msg);
 }
 
 /**
- * Handles an interrupt on the specified port.
- *
- * This signals the start of a transfer. We read the command byte (which is
- * the first byte), star the RX DMA and set up our reply accordingly.
+ * Called to indicate that a command has completed
  *
- * We will not get interrupts on subsequent bytes since the DMA will handle
- * the incoming data.
+ * Some commands can continue for a while. This function is called by
+ * host_command when it completes.
  *
- * @param port	Port that the interrupt came in on (STM32_SPI0/1_PORT)
  */
-static void spi_interrupt(int port)
+static void spi_send_response(struct host_cmd_handler_args *args)
 {
-	struct host_cmd_handler_args args;
-	enum ec_status status;
-	int msg_len;
-	int dmac;
-	int cmd;
+	struct spi_ctlr *spi;
+	enum ec_status result = args->result;
+	struct dma_channel *txdma;
 
-	/* Make sure there is a byte available */
-	if (!(STM32_SPI_SR(port) & SR_RXNE))
+	/* If we are too late, don't bother */
+	if (!active)
 		return;
 
-	/* Get the command byte */
-	cmd = STM32_SPI_DR(port);
+	spi = (struct spi_ctlr *)stm32_spi_addr(SPI_PORT_HOST);
+	if (args->response_size > EC_HOST_PARAM_SIZE)
+		result = EC_RES_INVALID_RESPONSE;
+
+	if ((uint8_t *)args->response >= out_msg &&
+			(uint8_t *)args->response < out_msg + sizeof(out_msg))
+		ASSERT(args->response == out_msg + SPI_MSG_HEADER_LEN);
 
-	/* Read the rest of the message - for now we do nothing with it */
-	dmac = DMA_CHANNEL_FOR_SPI_RX(port);
-	dma_start_rx(dmac, sizeof(in_msg), (void *)&STM32_SPI_DR(port),
-		     in_msg);
+	/* Transmit the reply */
+	txdma = dma_get_channel(DMA_CHANNEL_FOR_SPI_TX(spi));
+	reply(spi, txdma, result, args->response, args->response_size);
+}
+
+/**
+ * Handle an event on the NSS pin
+ *
+ * A falling edge of NSS indicates that the master is starting a new
+ * transaction. A rising edge indicates that we have finsihed
+ *
+ * @param signal	GPIO signal for the NSS pin
+ */
+void spi_event(enum gpio_signal signal)
+{
+	struct dma_channel *rxdma;
+	struct spi_ctlr *spi;
+	uint16_t *nss_reg;
+	uint32_t nss_mask;
+
+	spi = (struct spi_ctlr *)stm32_spi_addr(SPI_PORT_HOST);
 
 	/*
-	 * Process the command and send the reply.
-	 *
-	 * This is kind of ugly, because the host command interface can
-	 * only call host_send_response() for one host bus, but stm32 could
-	 * potentially have both I2C and SPI active at the same time on the
-	 * current devel board.
+	 * If NSS is rising, we have finished the transaction, so prepare
+	 * for the next.
 	 */
-	args.command = cmd;
-	args.result = EC_RES_SUCCESS;
-	args.send_response = spi_send_response;
-	args.version = 0;
-	args.params = out_msg + SPI_MSG_HEADER_BYTES + 1;
-	args.params_size = sizeof(out_msg) - SPI_MSG_PROTO_BYTES;
-	/* TODO: use a different initial buffer for params vs. response */
-	args.response = args.params;
-	args.response_max = sizeof(out_msg) - SPI_MSG_PROTO_BYTES;
-	args.response_size = 0;
+	nss_reg = gpio_get_level_reg(GPIO_SPI1_NSS, &nss_mask);
+	if (REG16(nss_reg) & nss_mask) {
+		setup_for_transaction(spi);
+		return;
+	}
 
-	status = host_command_process(&args);
+	active = 1;
+	rxdma = dma_get_channel(DMA_CHANNEL_FOR_SPI_RX(spi));
 
-	if (args.response_size < 0 || args.response_size > EC_PARAM_SIZE)
-		status = EC_RES_INVALID_RESPONSE;
-	else if (args.response != args.params)
-		memcpy(args.response, args.params, args.response_size);
+	/* Wait for version, command, length bytes */
+	if (wait_for_bytes(rxdma, 3, nss_reg, nss_mask)) {
+		setup_for_transaction(spi);
+		return;
+	}
 
-	out_msg[SPI_MSG_HEADER_BYTES] = status;
-	reply(port, out_msg, args.response_size);
+	if (in_msg[0] >= EC_CMD_VERSION0) {
+		args.version = in_msg[0] - EC_CMD_VERSION0;
+		args.command = in_msg[1];
+		args.params_size = in_msg[2];
+		args.params = in_msg + 3;
+	} else {
+		args.version = 0;
+		args.command = in_msg[0];
+		args.params = in_msg + 1;
+		args.params_size = 0;
+		args.version = 0;
+	}
 
-	/* Wake up the task that watches for end of the incoming message */
-	task_wake(TASK_ID_SPI);
-}
+	/* Wait for parameters */
+	if (wait_for_bytes(rxdma, 3 + args.params_size, nss_reg, nss_mask)) {
+		setup_for_transaction(spi);
+		return;
+	}
 
-/* The interrupt code cannot pass a parameters, so handle this here */
-static void spi1_interrupt(void) { spi_interrupt(STM32_SPI1_PORT); };
+	/* Process the command and send the reply */
+	args.send_response = spi_send_response;
 
-DECLARE_IRQ(STM32_IRQ_SPI1, spi1_interrupt, 2);
+	/* Allow room for the header bytes */
+	args.response = out_msg + SPI_MSG_HEADER_LEN;
+	args.response_max = sizeof(out_msg) - SPI_MSG_PROTO_LEN;
+	args.response_size = 0;
+	args.result = EC_RES_SUCCESS;
+
+	host_command_received(&args);
+}
 
 static int spi_init(void)
 {
-	int port;
+	struct spi_ctlr *spi;
 
-#if defined(BOARD_daisy) || defined(BOARD_snow)
 	/**
 	 * SPI1
 	 * PA7: SPI1_MOSI
@@ -235,22 +319,17 @@ static int spi_init(void)
 	 *
 	 * 8-bit data, master mode, full-duplex, clock is fpclk / 2
 	 */
-	port = STM32_SPI1_PORT;
+	spi = (struct spi_ctlr *)stm32_spi_addr(SPI_PORT_HOST);
 
-	/* enable rx buffer not empty interrupt, and rx DMA */
-	STM32_SPI_CR2(port) |= CR2_RXNEIE | CR2_RXDMAEN;
+	/* Enable rx DMA and get ready to receive our first transaction */
+	REG16(&spi->ctrl2) = CR2_RXDMAEN | CR2_TXDMAEN;
 
-	/* set up an interrupt when we get the first byte of a packet */
-	task_enable_irq(STM32_IRQ_SPI1);
+	/* enable the SPI peripheral */
+	REG16(&spi->ctrl1) |= CR1_SPE;
 
-	/* write 0xff which will be our default output value */
-	STM32_SPI_DR(port) = 0xff;
+	setup_for_transaction(spi);
 
-	/* enable the SPI peripheral */
-	STM32_SPI_CR1(port) |= CR1_SPE;
-#else
-#error "Need to know how to set up SPI for this board"
-#endif
+	gpio_enable_interrupt(GPIO_SPI1_NSS);
 	return EC_SUCCESS;
 }
 DECLARE_HOOK(HOOK_INIT, spi_init, HOOK_PRIO_DEFAULT);
diff --git a/include/spi.h b/include/spi.h
index 4af03bc365..a03e04f134 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -19,4 +19,20 @@ int spi_enable(int enable);
 int spi_transaction(const uint8_t *txdata, int txlen,
 		    uint8_t *rxdata, int rxlen);
 
+#ifdef CONFIG_SPI
+/**
+ * Called when the NSS level changes, signalling the start or end of a SPI
+ * transaction.
+ *
+ * @param signal	GPIO signal that changed
+ */
+void spi_event(enum gpio_signal signal);
+
+#else
+static inline void spi_event(enum gpio_signal signal)
+{
+}
+
+#endif
+
 #endif  /* __CROS_EC_SPI_H */