spi: Factor EC protocol details out of the SPI drivers.

The SPI drivers for tegra and exynos5420 have code in them which waits for a
frame header and leaves filler data out. The SPI driver shouldn't have support
for frame headers directly. If a device uses them, it should support them
itself. That makes the SPI drivers simpler and easier to write.

When moving the frame handling logic into the EC support code, EC communication
continued to work on tegra but no longer worked on exynos5420. That suggested
the SPI driver on the 5420 wasn't working correctly, so I replaced that with
the implementation in depthcharge. Unfortunately that implementation doesn't
support waiting for a frame header for the EC, so these changes are combined
into one.

BUG=None
TEST=Built and booted on pit. Built and booted on nyan. In both cases,
verified that there were no error messages from the SPI drivers or the EC
code.
BRANCH=None

Original-Change-Id: I62a68820c632f154acece94f54276ddcd1442c09
Original-Signed-off-by: Gabe Black <gabeblack@google.com>
Original-Reviewed-on: https://chromium-review.googlesource.com/191192
Original-Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Original-Commit-Queue: Gabe Black <gabeblack@chromium.org>
Original-Tested-by: Gabe Black <gabeblack@chromium.org>
(cherry picked from commit 4fcfed280ad70f14a013d5353aa0bee0af540630)
Signed-off-by: Marc Jones <marc.jones@se-eng.com>

Change-Id: Id8824523abc7afcbc214845901628833e135d142
Reviewed-on: http://review.coreboot.org/7706
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <gaumless@gmail.com>

8 files changed, 177 insertions, 271 deletions

diff --git a/src/ec/google/chromeec/ec_spi.c b/src/ec/google/chromeec/ec_spi.c
index d6b7b0c4ea..f1bdd3caaa 100644
--- a/src/ec/google/chromeec/ec_spi.c
+++ b/src/ec/google/chromeec/ec_spi.c
@@ -18,28 +18,60 @@
  */
 
 #include <console/console.h>
-#include <spi-generic.h>
-
 #include "ec.h"
 #include "ec_commands.h"
+#include <spi-generic.h>
+#include <timer.h>
+
+static const uint8_t EcFramingByte = 0xec;
 
 static int crosec_spi_io(uint8_t *write_bytes, size_t write_size,
 			 uint8_t *read_bytes, size_t read_size,
 			 void *context)
 {
 	struct spi_slave *slave = (struct spi_slave *)context;
-	int rv;
 
 	spi_claim_bus(slave);
-	rv = spi_xfer(slave, write_bytes, write_size, read_bytes,
-		      read_size);
-	spi_release_bus(slave);
 
-	if (rv != 0) {
-		printk(BIOS_ERR, "%s: Cannot complete SPI I/O\n", __func__);
+	if (spi_xfer(slave, write_bytes, write_size, NULL, 0)) {
+		printk(BIOS_ERR, "%s: Failed to send request.\n", __func__);
+		spi_release_bus(slave);
 		return -1;
 	}
 
+	uint8_t byte;
+	struct mono_time start;
+	struct rela_time rt;
+	timer_monotonic_get(&start);
+	while (1) {
+		if (spi_xfer(slave, NULL, 0, &byte, sizeof(byte))) {
+			printk(BIOS_ERR, "%s: Failed to receive byte.\n",
+			       __func__);
+			spi_release_bus(slave);
+			return -1;
+		}
+		if (byte == EcFramingByte)
+			break;
+
+		// Wait 1s for a framing byte.
+		rt = current_time_from(&start);
+		if (rela_time_in_microseconds(&rt) > 1000 * 1000) {
+			printk(BIOS_ERR,
+			       "%s: Timeout waiting for framing byte.\n",
+			       __func__);
+			spi_release_bus(slave);
+			return -1;
+		}
+	}
+
+	if (spi_xfer(slave, NULL, 0, read_bytes, read_size)) {
+		printk(BIOS_ERR, "%s: Failed to receive response.\n", __func__);
+		spi_release_bus(slave);
+		return -1;
+	}
+
+	spi_release_bus(slave);
+
 	return 0;
 }
 
diff --git a/src/mainboard/google/nyan/mainboard.c b/src/mainboard/google/nyan/mainboard.c
index a96236bcfa..ec06a7acfe 100644
--- a/src/mainboard/google/nyan/mainboard.c
+++ b/src/mainboard/google/nyan/mainboard.c
@@ -188,13 +188,7 @@ static void setup_kernel_info(void)
 
 static void setup_ec_spi(void)
 {
-	struct tegra_spi_channel *spi;
-
-	spi = tegra_spi_init(CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS);
-
-	/* Set frame header for use by CrOS EC */
-	spi->frame_header = 0xec;
-	spi->rx_frame_header_enable = 1;
+	tegra_spi_init(CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS);
 }
 
 static void mainboard_init(device_t dev)
diff --git a/src/mainboard/google/nyan_big/mainboard.c b/src/mainboard/google/nyan_big/mainboard.c
index 75ff0e7132..242f995d98 100644
--- a/src/mainboard/google/nyan_big/mainboard.c
+++ b/src/mainboard/google/nyan_big/mainboard.c
@@ -188,13 +188,7 @@ static void setup_kernel_info(void)
 
 static void setup_ec_spi(void)
 {
-	struct tegra_spi_channel *spi;
-
-	spi = tegra_spi_init(CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS);
-
-	/* Set frame header for use by CrOS EC */
-	spi->frame_header = 0xec;
-	spi->rx_frame_header_enable = 1;
+	tegra_spi_init(CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS);
 }
 
 static void mainboard_init(device_t dev)
diff --git a/src/mainboard/google/nyan_blaze/mainboard.c b/src/mainboard/google/nyan_blaze/mainboard.c
index 2f385bcbc4..b2787ec4c5 100644
--- a/src/mainboard/google/nyan_blaze/mainboard.c
+++ b/src/mainboard/google/nyan_blaze/mainboard.c
@@ -188,13 +188,7 @@ static void setup_kernel_info(void)
 
 static void setup_ec_spi(void)
 {
-	struct tegra_spi_channel *spi;
-
-	spi = tegra_spi_init(CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS);
-
-	/* Set frame header for use by CrOS EC */
-	spi->frame_header = 0xec;
-	spi->rx_frame_header_enable = 1;
+	tegra_spi_init(CONFIG_EC_GOOGLE_CHROMEEC_SPI_BUS);
 }
 
 static void mainboard_init(device_t dev)
diff --git a/src/soc/nvidia/tegra124/spi.c b/src/soc/nvidia/tegra124/spi.c
index ae8a9a7626..7ad771633c 100644
--- a/src/soc/nvidia/tegra124/spi.c
+++ b/src/soc/nvidia/tegra124/spi.c
@@ -23,6 +23,7 @@
 #include <cbfs_core.h>
 #include <inttypes.h>
 #include <spi-generic.h>
+#include <spi_flash.h>
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
@@ -722,16 +723,11 @@ int spi_xfer(struct spi_slave *slave, const void *dout,
 	u8 *out_buf = (u8 *)dout;
 	u8 *in_buf = (u8 *)din;
 	unsigned int todo;
-	int ret = 0, frame_started = 1;
+	int ret = 0;
 
 	/* tegra bus numbers start at 1 */
 	ASSERT(slave->bus >= 1 && slave->bus <= ARRAY_SIZE(tegra_spi_channels));
 
-	if (spi->rx_frame_header_enable) {
-		memset(in_buf, ~spi->frame_header, in_bytes);
-		frame_started = 0;
-	}
-
 	while (out_bytes || in_bytes) {
 		int x = 0;
 
@@ -779,41 +775,14 @@ int spi_xfer(struct spi_slave *slave, const void *dout,
 			break;
 		}
 
-		/*
-		 * Post-processing. For output, we only need to increment
-		 * the buffer and decrement the counter. Same for input if
-		 * there is no frame header to be concerned with.
-		 *
-		 * If a frame header is used and is found, the input buffer
-		 * is shifted so that the header starts at offset 0, and
-		 * in_bytes and in_buf are incremented/decremented according
-		 * to the offset where the header was originally found.
-		 */
+		/* Post-processing. */
 		if (out_bytes) {
 			out_bytes -= x;
 			out_buf += x;
 		}
 		if (in_bytes) {
-			if (spi->rx_frame_header_enable && !frame_started) {
-				int i;
-
-				for (i = 0; i < x; i++) {
-					if (in_buf[i] == spi->frame_header) {
-						frame_started = 1;
-						i++; /* discard frame header */
-						break;
-					}
-				}
-
-				if (frame_started) {
-					memmove(&in_buf[0], &in_buf[i], x - i);
-					in_bytes -= x - i;
-					in_buf += x - i;
-				}
-			} else {
-				in_bytes -= x;
-				in_buf += x;
-			}
+			in_bytes -= x;
+			in_buf += x;
 		}
 	}
 
diff --git a/src/soc/nvidia/tegra124/spi.h b/src/soc/nvidia/tegra124/spi.h
index 857c35f1b9..11e88feec5 100644
--- a/src/soc/nvidia/tegra124/spi.h
+++ b/src/soc/nvidia/tegra124/spi.h
@@ -54,9 +54,6 @@ struct tegra_spi_channel {
 	struct spi_slave slave;
 	unsigned int req_sel;
 
-	/* stuff that is specific to the attached device */
-	int rx_frame_header_enable;
-	u8 frame_header;
 	int dual_mode;		/* for x2 transfers with bit interleaving */
 
 	/* context (used internally) */
diff --git a/src/soc/samsung/exynos5420/spi.c b/src/soc/samsung/exynos5420/spi.c
index a2bbbb35b7..c8661e1852 100644
--- a/src/soc/samsung/exynos5420/spi.c
+++ b/src/soc/samsung/exynos5420/spi.c
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 #include <assert.h>
 #include <spi_flash.h>
+#include <string.h>
 
 #include "cpu.h"
 #include "spi.h"
@@ -38,9 +39,7 @@
 struct exynos_spi_slave {
 	struct spi_slave slave;
 	struct exynos_spi *regs;
-	unsigned int fifo_size;
-	uint8_t half_duplex;
-	uint8_t frame_header;  /* header byte to detect in half-duplex mode. */
+	int initialized;
 };
 
 /* TODO(hungte) Move the SPI param list to per-board configuration, probably
@@ -55,17 +54,12 @@ static struct exynos_spi_slave exynos_spi_slaves[3] = {
 	{
 		.slave = { .bus = 1, .rw = SPI_READ_FLAG, },
 		.regs = (void *)EXYNOS5_SPI1_BASE,
-		.fifo_size = 64,
-		.half_duplex = 0,
 	},
 	// SPI 2
 	{
 		.slave = { .bus = 2,
 			   .rw = SPI_READ_FLAG | SPI_WRITE_FLAG, },
 		.regs = (void *)EXYNOS5_SPI2_BASE,
-		.fifo_size = 64,
-		.half_duplex = 1,
-		.frame_header = 0xec,
 	},
 };
 
@@ -74,15 +68,69 @@ static inline struct exynos_spi_slave *to_exynos_spi(struct spi_slave *slave)
 	return container_of(slave, struct exynos_spi_slave, slave);
 }
 
+static void spi_sw_reset(struct exynos_spi *regs, int word)
+{
+	const uint32_t orig_mode_cfg = readl(&regs->mode_cfg);
+	uint32_t mode_cfg = orig_mode_cfg;
+	const uint32_t orig_swap_cfg = readl(&regs->swap_cfg);
+	uint32_t swap_cfg = orig_swap_cfg;
+
+	mode_cfg &= ~(SPI_MODE_CH_WIDTH_MASK | SPI_MODE_BUS_WIDTH_MASK);
+	if (word) {
+		mode_cfg |= SPI_MODE_CH_WIDTH_WORD | SPI_MODE_BUS_WIDTH_WORD;
+		swap_cfg |= SPI_RX_SWAP_EN |
+			    SPI_RX_BYTE_SWAP |
+			    SPI_RX_HWORD_SWAP |
+			    SPI_TX_SWAP_EN |
+			    SPI_TX_BYTE_SWAP |
+			    SPI_TX_HWORD_SWAP;
+	} else {
+		mode_cfg |= SPI_MODE_CH_WIDTH_BYTE | SPI_MODE_BUS_WIDTH_BYTE;
+		swap_cfg = 0;
+	}
+
+	if (mode_cfg != orig_mode_cfg)
+		writel(mode_cfg, &regs->mode_cfg);
+	if (swap_cfg != orig_swap_cfg)
+		writel(swap_cfg, &regs->swap_cfg);
+
+	clrbits_le32(&regs->ch_cfg, SPI_RX_CH_ON | SPI_TX_CH_ON);
+	setbits_le32(&regs->ch_cfg, SPI_CH_RST);
+	clrbits_le32(&regs->ch_cfg, SPI_CH_RST);
+	setbits_le32(&regs->ch_cfg, SPI_RX_CH_ON | SPI_TX_CH_ON);
+}
+
 void spi_init(void)
 {
-	printk(BIOS_INFO, "Exynos SPI driver initiated.\n");
+}
+
+static void exynos_spi_init(struct exynos_spi *regs)
+{
+	// Set FB_CLK_SEL.
+	writel(SPI_FB_DELAY_180, &regs->fb_clk);
+	// CPOL: Active high.
+	clrbits_le32(&regs->ch_cfg, SPI_CH_CPOL_L);
+
+	// Clear rx and tx channel if set priveously.
+	clrbits_le32(&regs->ch_cfg, SPI_RX_CH_ON | SPI_TX_CH_ON);
+
+	setbits_le32(&regs->swap_cfg,
+		     SPI_RX_SWAP_EN | SPI_RX_BYTE_SWAP | SPI_RX_HWORD_SWAP);
+	clrbits_le32(&regs->ch_cfg, SPI_CH_HS_EN);
+
+	// Do a soft reset, which will also enable both channels.
+	spi_sw_reset(regs, 1);
 }
 
 struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs)
 {
 	ASSERT(bus >= 0 && bus < 3);
-	return &(exynos_spi_slaves[bus].slave);
+	struct exynos_spi_slave *eslave = &exynos_spi_slaves[bus];
+	if (!eslave->initialized) {
+		exynos_spi_init(eslave->regs);
+		eslave->initialized = 1;
+	}
+	return &eslave->slave;
 }
 
 int spi_cs_is_valid(unsigned int bus, unsigned int cs)
@@ -103,237 +151,111 @@ void spi_cs_deactivate(struct spi_slave *slave)
 	setbits_le32(&regs->cs_reg, SPI_SLAVE_SIG_INACT);
 }
 
-static inline void exynos_spi_soft_reset(struct exynos_spi *regs)
+int spi_claim_bus(struct spi_slave *slave)
 {
-	/* The soft reset clears only FIFO and status register.
-	 * All special function registers are not changed. */
-	setbits_le32(&regs->ch_cfg, SPI_CH_RST);
-	clrbits_le32(&regs->ch_cfg, SPI_CH_RST);
+	spi_cs_activate(slave);
+	return 0;
 }
 
-static inline void exynos_spi_flush_fifo(struct exynos_spi *regs)
+static void spi_transfer(struct exynos_spi *regs, void *in, const void *out,
+			 u32 size)
 {
-	/*
-	 * Flush spi tx, rx fifos and reset the SPI controller
-	 * and clear rx/tx channel
-	 */
-	clrbits_le32(&regs->ch_cfg, SPI_RX_CH_ON | SPI_TX_CH_ON);
-	clrbits_le32(&regs->ch_cfg, SPI_CH_HS_EN);
-	exynos_spi_soft_reset(regs);
-	setbits_le32(&regs->ch_cfg, SPI_RX_CH_ON | SPI_TX_CH_ON);
-}
+	u8 *inb = in;
+	const u8 *outb = out;
 
-static void exynos_spi_request_bytes(struct exynos_spi *regs, int count,
-				     int width)
-{
-	uint32_t mode_word = SPI_MODE_CH_WIDTH_WORD | SPI_MODE_BUS_WIDTH_WORD,
-		 swap_word = (SPI_TX_SWAP_EN | SPI_RX_SWAP_EN |
-			      SPI_TX_BYTE_SWAP | SPI_RX_BYTE_SWAP |
-			      SPI_TX_HWORD_SWAP | SPI_RX_HWORD_SWAP);
-
-	/* For word address we need to swap bytes */
-	if (width == sizeof(uint32_t)) {
-		setbits_le32(&regs->mode_cfg, mode_word);
-		setbits_le32(&regs->swap_cfg, swap_word);
-		count /= width;
-	} else {
-		/* Select byte access and clear the swap configuration */
-		clrbits_le32(&regs->mode_cfg, mode_word);
-		writel(0, &regs->swap_cfg);
-	}
+	int width = (size % 4) ? 1 : 4;
 
-	exynos_spi_soft_reset(regs);
+	while (size) {
+		int packets = size / width;
+		// The packet count field is 16 bits wide.
+		packets = MIN(packets, (1 << 16) - 1);
 
-	if (count) {
-		ASSERT(count < (1 << 16));
-		writel(count | SPI_PACKET_CNT_EN, &regs->pkt_cnt);
-	} else {
-		writel(0, &regs->pkt_cnt);
-	}
-}
+		int out_bytes, in_bytes;
+		out_bytes = in_bytes = packets * width;
 
-static int spi_rx_tx(struct spi_slave *slave, uint8_t *rxp, int rx_bytes,
-		     const uint8_t *txp, int tx_bytes)
-{
-	struct exynos_spi_slave *espi = to_exynos_spi(slave);
-	struct exynos_spi *regs = espi->regs;
-
-	int step;
-	int todo = MAX(rx_bytes, tx_bytes);
-	int wait_for_frame_header = espi->half_duplex;
-
-	ASSERT(todo < EXYNOS_SPI_MAX_TRANSFER_BYTES);
-
-	/* Select transfer mode. */
-	if (espi->half_duplex) {
-		step = 1;
-	} else if ((rx_bytes | tx_bytes | (uintptr_t)rxp |(uintptr_t)txp) & 3) {
-		printk(BIOS_CRIT, "%s: WARNING: transfer mode decreased to 1B\n",
-		       __func__);
-		step = 1;
-	} else {
-		step = sizeof(uint32_t);
-	}
+		spi_sw_reset(regs, width == 4);
+		writel(packets | SPI_PACKET_CNT_EN, &regs->pkt_cnt);
+
+		while (out_bytes || in_bytes) {
+			uint32_t spi_sts = readl(&regs->spi_sts);
+			int rx_lvl = ((spi_sts >> 15) & 0x1ff);
+			int tx_lvl = ((spi_sts >> 6) & 0x1ff);
 
-	exynos_spi_request_bytes(regs, espi->half_duplex ? 0 : todo, step);
-
-	/* Note: Some device, like ChromeOS EC, tries to work in half-duplex
-	 * mode and sends a large amount of data (larger than FIFO size).
-	 * Printing lots of debug messages or doing extra delay in the loop
-	 * below may cause rx buffer to overflow and getting unexpected data
-	 * error.
-	 */
-	while (rx_bytes || tx_bytes) {
-		int temp;
-		uint32_t spi_sts = readl(&regs->spi_sts);
-		int rx_lvl = (spi_sts >> SPI_RX_LVL_OFFSET) & SPI_FIFO_LVL_MASK,
-		    tx_lvl = (spi_sts >> SPI_TX_LVL_OFFSET) & SPI_FIFO_LVL_MASK;
-		int min_tx = ((tx_bytes || !espi->half_duplex) ?
-			      (espi->fifo_size / 2) : 1);
-
-		// TODO(hungte) Abort if timeout happens in half-duplex mode.
-
-		/*
-		 * Don't completely fill the txfifo, since we don't want our
-		 * rxfifo to overflow, and it may already contain data.
-		 */
-		while (tx_lvl < min_tx) {
-			if (tx_bytes) {
-				if (step == sizeof(uint32_t)) {
-					temp = *((uint32_t *)txp);
-					txp += sizeof(uint32_t);
-				} else {
-					temp = *txp++;
+			if (tx_lvl < 32 && tx_lvl < out_bytes) {
+				uint32_t data = 0xffffffff;
+
+				if (outb) {
+					memcpy(&data, outb, width);
+					outb += width;
 				}
-				tx_bytes -= step;
-			} else {
-				temp = -1;
+				writel(data, &regs->tx_data);
+
+				out_bytes -= width;
 			}
-			writel(temp, &regs->tx_data);
-			tx_lvl += step;
-		}
 
-		while ((rx_lvl >= step) && rx_bytes) {
-			temp = readl(&regs->rx_data);
-			rx_lvl -= step;
-			if (wait_for_frame_header) {
-				if ((temp & 0xff) == espi->frame_header) {
-					wait_for_frame_header = 0;
+			if (rx_lvl >= width) {
+				uint32_t data = readl(&regs->rx_data);
+
+				if (inb) {
+					memcpy(inb, &data, width);
+					inb += width;
 				}
-				break;  /* Restart the outer loop. */
-			}
-			if (step == sizeof(uint32_t)) {
-				*((uint32_t *)rxp) = temp;
-				rxp += sizeof(uint32_t);
-			} else {
-				*rxp++ = temp;
+
+				in_bytes -= width;
 			}
-			rx_bytes -= step;
 		}
+
+		size -= packets * width;
 	}
-	return 0;
 }
 
-int spi_claim_bus(struct spi_slave *slave)
+int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bytes_out,
+	     void *din, unsigned int bytes_in)
 {
-	struct exynos_spi_slave *espi = to_exynos_spi(slave);
-	struct exynos_spi *regs = espi->regs;
+	struct exynos_spi *regs = to_exynos_spi(slave)->regs;
 
-	exynos_spi_flush_fifo(regs);
+	if (bytes_out && bytes_in) {
+		u32 min_size = MIN(bytes_out, bytes_in);
 
-	// Select Active High Clock, Format A (SCP 30.2.1.8).
-	clrbits_le32(&regs->ch_cfg, SPI_CH_CPOL_L | SPI_CH_CPHA_B);
+		spi_transfer(regs, din, dout, min_size);
 
-	// Set FeedBack Clock Selection.
-	writel(SPI_FB_DELAY_180, &regs->fb_clk);
+		bytes_out -= min_size;
+		bytes_in -= min_size;
 
-	// HIGH speed is required for Tx/Rx to work in 50MHz (SCP 30.2.1.6).
-	if (espi->half_duplex) {
-		clrbits_le32(&regs->ch_cfg, SPI_CH_HS_EN);
-		printk(BIOS_DEBUG, "%s: LOW speed.\n", __func__);
-	} else {
-		setbits_le32(&regs->ch_cfg, SPI_CH_HS_EN);
-		printk(BIOS_DEBUG, "%s: HIGH speed.\n", __func__);
+		din = (uint8_t *)din + min_size;
+		dout = (const uint8_t *)dout + min_size;
 	}
+
+	if (bytes_in)
+		spi_transfer(regs, din, NULL, bytes_in);
+	else if (bytes_out)
+		spi_transfer(regs, NULL, dout, bytes_out);
+
 	return 0;
 }
 
-int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int out_bytes,
-	     void *din, unsigned int in_bytes)
+void spi_release_bus(struct spi_slave *slave)
 {
-	uint8_t *out_ptr = (uint8_t *)dout, *in_ptr = (uint8_t *)din;
-	int offset, todo, len;
-	int ret = 0;
-
-	len = MAX(out_bytes, in_bytes);
-
-	/*
-	 * Exynos SPI limits each transfer to (2^16-1=65535) bytes. To keep
-	 * things simple (especially for word-width transfer mode), allow a
-	 * maximum of (2^16-4=65532) bytes. We could allow more in word mode,
-	 * but the performance difference is small.
-	 */
-	spi_cs_activate(slave);
-	for (offset = 0; !ret && (offset < len); offset += todo) {
-		todo = min(len - offset, (1 << 16) - 4);
-		ret = spi_rx_tx(slave, in_ptr, MIN(in_bytes, todo), out_ptr,
-				MIN(out_bytes, todo));
-		// Adjust remaining bytes and pointers.
-		if (in_bytes >= todo) {
-			in_bytes -= todo;
-			in_ptr += todo;
-		} else {
-			in_bytes = 0;
-			in_ptr = NULL;
-		}
-		if (out_bytes >= todo) {
-			out_bytes -= todo;
-			out_ptr += todo;
-		} else {
-			out_bytes = 0;
-			out_ptr = NULL;
-		}
-	}
 	spi_cs_deactivate(slave);
-
-	return ret;
 }
 
 static int exynos_spi_read(struct spi_slave *slave, void *dest, uint32_t len,
 			   uint32_t off)
 {
 	struct exynos_spi *regs = to_exynos_spi(slave)->regs;
-	int rv;
+	u32 command;
+	spi_claim_bus(slave);
 
-	// TODO(hungte) Merge the "read address" command into spi_xfer calls
-	// (full-duplex mode).
-
-	spi_cs_activate(slave);
-
-	// Specify read address (in word-width mode).
+	// Send address.
 	ASSERT(off < (1 << 24));
-	exynos_spi_request_bytes(regs, sizeof(off), sizeof(off));
-	writel(htonl((SF_READ_DATA_CMD << 24) | off), &regs->tx_data);
-	while (!(readl(&regs->spi_sts) & SPI_ST_TX_DONE)) {
-		/* Wait for TX done */
-	}
-
-	// Now, safe to transfer.
-	rv = spi_xfer(slave, NULL, 0, dest, len * 8);
-	spi_cs_deactivate(slave);
+	command = htonl(SF_READ_DATA_CMD << 24 | off);
+	spi_transfer(regs, NULL, &command, sizeof(command));
 
-	return (rv == 0) ? len : -1;
-}
+	// Read the data.
+	spi_transfer(regs, dest, NULL, len);
+	spi_release_bus(slave);
 
-void spi_release_bus(struct spi_slave *slave)
-{
-	struct exynos_spi *regs = to_exynos_spi(slave)->regs;
-	/* Reset swap mode to make sure no one relying on default values (Ex,
-	 * payload or kernel) will go wrong. */
-	clrbits_le32(&regs->mode_cfg, (SPI_MODE_CH_WIDTH_WORD |
-				       SPI_MODE_BUS_WIDTH_WORD));
-	writel(0, &regs->swap_cfg);
-	exynos_spi_flush_fifo(regs);
+	return len;
 }
 
 // SPI as CBFS media.
diff --git a/src/soc/samsung/exynos5420/spi.h b/src/soc/samsung/exynos5420/spi.h
index 20c2adb087..78cca6f416 100644
--- a/src/soc/samsung/exynos5420/spi.h
+++ b/src/soc/samsung/exynos5420/spi.h
@@ -57,8 +57,12 @@ check_member(exynos_spi, fb_clk, 0x2c);
 #define SPI_TX_CH_ON		(1 << 0)
 
 /* SPI_MODECFG */
-#define SPI_MODE_CH_WIDTH_WORD	(0x2 << 29)
+#define SPI_MODE_BUS_WIDTH_BYTE	(0x0 << 17)
 #define SPI_MODE_BUS_WIDTH_WORD	(0x2 << 17)
+#define SPI_MODE_BUS_WIDTH_MASK	(0x3 << 17)
+#define SPI_MODE_CH_WIDTH_BYTE	(0x0 << 29)
+#define SPI_MODE_CH_WIDTH_WORD	(0x2 << 29)
+#define SPI_MODE_CH_WIDTH_MASK	(0x3 << 29)
 
 /* SPI_CSREG */
 #define SPI_SLAVE_SIG_INACT	(1 << 0)