[coreboot-gerrit] New patch to review for coreboot: spi: Get rid of SPI_ATOMIC_SEQUENCING

Furquan Shaikh (furquan@google.com) gerrit at coreboot.org
Thu Dec 1 17:07:59 CET 2016


Furquan Shaikh (furquan at google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17681

-gerrit

commit d1106dc652562843ff5d9d7a5334336ef1dccc20
Author: Furquan Shaikh <furquan at chromium.org>
Date:   Tue Nov 29 22:07:42 2016 -0800

    spi: Get rid of SPI_ATOMIC_SEQUENCING
    
    SPI_ATOMIC_SEQUENCING was added to accomodate spi controllers with the
    ability to perform tx and rx on SPI bus at the same time. Instead of
    introducing this notion at SPI flash driver layer, allow SPI controllers
    to decide whether tx/rx can be combined or need to be performed
    separately.
    
    BUG=chrome-os-partner:59832
    BRANCH=None
    TEST=Compiles successfully
    
    Change-Id: I4c9e78c585ad95c40c0d5af078ff8251da286236
    Signed-off-by: Furquan Shaikh <furquan at chromium.org>
---
 src/drivers/spi/Kconfig            | 10 ----------
 src/drivers/spi/spi_flash.c        | 34 ++--------------------------------
 src/mainboard/google/purin/Kconfig |  1 -
 src/soc/imgtec/pistachio/Kconfig   |  1 -
 src/soc/imgtec/pistachio/spi.c     |  4 ----
 src/soc/marvell/armada38x/spi.c    | 14 ++++++--------
 src/soc/mediatek/mt8173/Kconfig    |  1 -
 src/soc/nvidia/tegra124/spi.c      | 15 ++++++++++++++-
 src/soc/nvidia/tegra210/spi.c      | 17 +++++++++++++++--
 src/soc/qualcomm/ipq40xx/Kconfig   |  1 -
 src/soc/qualcomm/ipq806x/Kconfig   |  1 -
 src/soc/rockchip/common/spi.c      | 17 +++++++++++++++--
 src/soc/samsung/exynos5420/spi.c   | 17 +++++++++++++++--
 13 files changed, 67 insertions(+), 66 deletions(-)

diff --git a/src/drivers/spi/Kconfig b/src/drivers/spi/Kconfig
index b55de58..c8d86ff 100644
--- a/src/drivers/spi/Kconfig
+++ b/src/drivers/spi/Kconfig
@@ -61,16 +61,6 @@ config SPI_FLASH_INCLUDE_ALL_DRIVERS
 	default n if COMMON_CBFS_SPI_WRAPPER
 	default y
 
-config SPI_ATOMIC_SEQUENCING
-	bool
-	default y if ARCH_X86
-	default n if !ARCH_X86
-	help
-	  Select this option if the SPI controller uses "atomic sequencing."
-	  Atomic sequencing is when the sequence of commands is pre-programmed
-	  in the SPI controller. Hardware manages the transaction instead of
-	  software. This is common on x86 platforms.
-
 config SPI_FLASH_SMM
 	bool "SPI flash driver support in SMM"
 	default n
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index b2fdab9..2e28428 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -32,22 +32,6 @@ static void spi_flash_addr(u32 addr, u8 *cmd)
 	cmd[3] = addr >> 0;
 }
 
-/*
- * If atomic sequencing is used, the cycle type is known to the SPI
- * controller so that it can perform consecutive transfers and arbitrate
- * automatically. Otherwise the SPI controller transfers whatever the
- * user requests immediately, without regard to sequence. Atomic
- * sequencing is commonly used on x86 platforms.
- *
- * SPI flash commands are simple two-step sequences. The command byte is
- * always written first and may be followed by an address. Then data is
- * either read or written. For atomic sequencing we'll pass everything into
- * spi_xfer() at once and let the controller handle the details. Otherwise
- * we will write all output bytes first and then read if necessary.
- *
- * FIXME: This really should be abstracted better, but that will
- * require overhauling the entire SPI infrastructure.
- */
 static int do_spi_flash_cmd(struct spi_slave *spi, const void *dout,
 		unsigned int bytes_out, void *din, unsigned int bytes_in)
 {
@@ -56,23 +40,9 @@ static int do_spi_flash_cmd(struct spi_slave *spi, const void *dout,
 	if (spi_claim_bus(spi))
 		return ret;
 
-#if CONFIG_SPI_ATOMIC_SEQUENCING == 1
-	if (spi_xfer(spi, dout, bytes_out, din, bytes_in) < 0)
-		goto done;
-#else
-	if (dout && bytes_out) {
-		if (spi_xfer(spi, dout, bytes_out, NULL, 0) < 0)
-			goto done;
-	}
-
-	if (din && bytes_in) {
-		if (spi_xfer(spi, NULL, 0, din, bytes_in) < 0)
-			goto done;
-	}
-#endif
+	if (spi_xfer(spi, dout, bytes_out, din, bytes_in) == 0)
+		ret = 0;
 
-	ret = 0;
-done:
 	spi_release_bus(spi);
 	return ret;
 }
diff --git a/src/mainboard/google/purin/Kconfig b/src/mainboard/google/purin/Kconfig
index eabab2b..ca0909b 100644
--- a/src/mainboard/google/purin/Kconfig
+++ b/src/mainboard/google/purin/Kconfig
@@ -26,7 +26,6 @@ config BOARD_SPECIFIC_OPTIONS # dummy
 	select SPI_FLASH
 	select SPI_FLASH_SPANSION
 	select SPI_FLASH_STMICRO # required for the reference board BCM958305K
-	select SPI_ATOMIC_SEQUENCING
 
 config CHROMEOS
 	select VBOOT_VBNV_FLASH
diff --git a/src/soc/imgtec/pistachio/Kconfig b/src/soc/imgtec/pistachio/Kconfig
index da33cc5..1ce488c 100644
--- a/src/soc/imgtec/pistachio/Kconfig
+++ b/src/soc/imgtec/pistachio/Kconfig
@@ -22,7 +22,6 @@ config CPU_IMGTEC_PISTACHIO
 	select GENERIC_UDELAY
 	select HAVE_MONOTONIC_TIMER
 	select HAVE_UART_SPECIAL
-	select SPI_ATOMIC_SEQUENCING
 	select GENERIC_GPIO_LIB
 	select HAVE_HARD_RESET
 	select UART_OVERRIDE_REFCLK
diff --git a/src/soc/imgtec/pistachio/spi.c b/src/soc/imgtec/pistachio/spi.c
index 2b4b01c..afe3a2c 100644
--- a/src/soc/imgtec/pistachio/spi.c
+++ b/src/soc/imgtec/pistachio/spi.c
@@ -22,10 +22,6 @@
 #include <string.h>
 #include <timer.h>
 
-#if !CONFIG_SPI_ATOMIC_SEQUENCING
-#error "Unsupported SPI driver API"
-#endif
-
 /* Imgtec controller uses 16 bit packet length. */
 #define IMGTEC_SPI_MAX_TRANSFER_SIZE   ((1 << 16) - 1)
 
diff --git a/src/soc/marvell/armada38x/spi.c b/src/soc/marvell/armada38x/spi.c
index 6a0e062..7b26b70 100644
--- a/src/soc/marvell/armada38x/spi.c
+++ b/src/soc/marvell/armada38x/spi.c
@@ -477,13 +477,11 @@ int spi_xfer(struct spi_slave *slave,
 	     void *din,
 	     unsigned in_bytes)
 {
-	int ret = -1;
-
-	if (out_bytes)
-		ret = mrvl_spi_xfer(slave, out_bytes * 8, dout, din);
-	else if (in_bytes)
-		ret = mrvl_spi_xfer(slave, in_bytes * 8, dout, din);
-	else
-		die("Unexpected condition in spi_xfer\n");
+	int ret = 0;
+
+	if (dout && out_bytes)
+		ret = mrvl_spi_xfer(slave, out_bytes * 8, dout, NULL);
+	if (!ret && din && in_bytes)
+		ret = mrvl_spi_xfer(slave, in_bytes * 8, NULL, din);
 	return ret;
 }
diff --git a/src/soc/mediatek/mt8173/Kconfig b/src/soc/mediatek/mt8173/Kconfig
index ec3481e..7a6ad87 100644
--- a/src/soc/mediatek/mt8173/Kconfig
+++ b/src/soc/mediatek/mt8173/Kconfig
@@ -9,7 +9,6 @@ config SOC_MEDIATEK_MT8173
 	select ARM64_USE_ARM_TRUSTED_FIRMWARE
 	select BOOTBLOCK_CONSOLE
 	select HAVE_UART_SPECIAL
-	select SPI_ATOMIC_SEQUENCING if SPI_FLASH
 	select HAVE_MONOTONIC_TIMER
 	select GENERIC_UDELAY
 	select GENERIC_GPIO_LIB
diff --git a/src/soc/nvidia/tegra124/spi.c b/src/soc/nvidia/tegra124/spi.c
index d5be4a6..c42677a 100644
--- a/src/soc/nvidia/tegra124/spi.c
+++ b/src/soc/nvidia/tegra124/spi.c
@@ -719,7 +719,7 @@ unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len)
 	return buf_len;
 }
 
-int spi_xfer(struct spi_slave *slave, const void *dout,
+static int __spi_xfer(struct spi_slave *slave, const void *dout,
 		unsigned int out_bytes, void *din, unsigned int in_bytes)
 {
 	struct tegra_spi_channel *spi = to_tegra_spi(slave->bus);
@@ -798,6 +798,19 @@ int spi_xfer(struct spi_slave *slave, const void *dout,
 	return ret;
 }
 
+int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int out_bytes,
+	     void *din, unsigned int in_bytes)
+{
+	int ret = 0;
+
+	if (dout && out_bytes)
+		ret = __spi_xfer(slave, dout, out_bytes, NULL, 0);
+	if (!ret && din && in_bytes)
+		ret = __spi_xfer(slave, NULL, 0, din, in_bytes);
+
+	return ret;
+}
+
 struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs)
 {
 	struct tegra_spi_channel *channel = to_tegra_spi(bus);
diff --git a/src/soc/nvidia/tegra210/spi.c b/src/soc/nvidia/tegra210/spi.c
index b01289c..1949fdb 100644
--- a/src/soc/nvidia/tegra210/spi.c
+++ b/src/soc/nvidia/tegra210/spi.c
@@ -755,8 +755,8 @@ unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len)
 	return buf_len;
 }
 
-int spi_xfer(struct spi_slave *slave, const void *dout,
-		unsigned int out_bytes, void *din, unsigned int in_bytes)
+static int __spi_xfer(struct spi_slave *slave, const void *dout,
+		      unsigned int out_bytes, void *din, unsigned int in_bytes)
 {
 	struct tegra_spi_channel *spi = to_tegra_spi(slave->bus);
 	u8 *out_buf = (u8 *)dout;
@@ -834,6 +834,19 @@ int spi_xfer(struct spi_slave *slave, const void *dout,
 	return ret;
 }
 
+int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int out_bytes,
+	     void *din, unsigned int in_bytes)
+{
+	int ret = 0;
+
+	if (dout && out_bytes)
+		ret = __spi_xfer(slave, dout, out_bytes, NULL, 0);
+	if (!ret && din && in_bytes)
+		ret = __spi_xfer(slave, NULL, 0, din, in_bytes);
+
+	return ret;
+}
+
 struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs)
 {
 	struct tegra_spi_channel *channel = to_tegra_spi(bus);
diff --git a/src/soc/qualcomm/ipq40xx/Kconfig b/src/soc/qualcomm/ipq40xx/Kconfig
index f738622..05f29e4 100644
--- a/src/soc/qualcomm/ipq40xx/Kconfig
+++ b/src/soc/qualcomm/ipq40xx/Kconfig
@@ -7,7 +7,6 @@ config SOC_QC_IPQ40XX
 	select ARCH_RAMSTAGE_ARMV7
 	select BOOTBLOCK_CONSOLE
 	select HAVE_UART_SPECIAL
-	select SPI_ATOMIC_SEQUENCING
 	select GENERIC_GPIO_LIB
 	select HAVE_MONOTONIC_TIMER
 
diff --git a/src/soc/qualcomm/ipq806x/Kconfig b/src/soc/qualcomm/ipq806x/Kconfig
index 7ba5df5..32b61bc 100644
--- a/src/soc/qualcomm/ipq806x/Kconfig
+++ b/src/soc/qualcomm/ipq806x/Kconfig
@@ -7,7 +7,6 @@ config SOC_QC_IPQ806X
 	select ARCH_RAMSTAGE_ARMV7
 	select BOOTBLOCK_CONSOLE
 	select HAVE_UART_SPECIAL
-	select SPI_ATOMIC_SEQUENCING
 	select GENERIC_GPIO_LIB
 
 if SOC_QC_IPQ806X
diff --git a/src/soc/rockchip/common/spi.c b/src/soc/rockchip/common/spi.c
index 3666de3..12bbd6d 100644
--- a/src/soc/rockchip/common/spi.c
+++ b/src/soc/rockchip/common/spi.c
@@ -268,8 +268,8 @@ unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len)
 	return min(65535, buf_len);
 }
 
-int spi_xfer(struct spi_slave *slave, const void *dout,
-		unsigned int bytes_out, void *din, unsigned int bytes_in)
+static int __spi_xfer(struct spi_slave *slave, const void *dout,
+		      unsigned int bytes_out, void *din, unsigned int bytes_in)
 {
 	struct rockchip_spi *regs = to_rockchip_spi(slave)->regs;
 	int ret = 0;
@@ -339,3 +339,16 @@ int spi_xfer(struct spi_slave *slave, const void *dout,
 	rockchip_spi_enable_chip(regs, 0);
 	return ret < 0 ? ret : 0;
 }
+
+int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int out_bytes,
+	     void *din, unsigned int in_bytes)
+{
+	int ret = 0;
+
+	if (dout && out_bytes)
+		ret = __spi_xfer(slave, dout, out_bytes, NULL, 0);
+	if (!ret && din && in_bytes)
+		ret = __spi_xfer(slave, NULL, 0, din, in_bytes);
+
+	return ret;
+}
diff --git a/src/soc/samsung/exynos5420/spi.c b/src/soc/samsung/exynos5420/spi.c
index fd31a2f..13c11a3 100644
--- a/src/soc/samsung/exynos5420/spi.c
+++ b/src/soc/samsung/exynos5420/spi.c
@@ -188,8 +188,8 @@ static void spi_transfer(struct exynos_spi *regs, void *in, const void *out,
 	}
 }
 
-int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bytes_out,
-	     void *din, unsigned int bytes_in)
+static int __spi_xfer(struct spi_slave *slave, const void *dout,
+		      unsigned int bytes_out, void *din, unsigned int bytes_in)
 {
 	struct exynos_spi *regs = to_exynos_spi(slave)->regs;
 
@@ -213,6 +213,19 @@ int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bytes_out,
 	return 0;
 }
 
+int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int out_bytes,
+	     void *din, unsigned int in_bytes)
+{
+	int ret = 0;
+
+	if (dout && out_bytes)
+		ret = __spi_xfer(slave, dout, out_bytes, NULL, 0);
+	if (!ret && din && in_bytes)
+		ret = __spi_xfer(slave, NULL, 0, din, in_bytes);
+
+	return ret;
+}
+
 void spi_release_bus(struct spi_slave *slave)
 {
 	struct exynos_spi *regs = to_exynos_spi(slave)->regs;



More information about the coreboot-gerrit mailing list