Ronald G. Minnich (rminnich@gmail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/18075
-gerrit
commit 6fd68813bf9ce1855f62d456fa018d2f5c10707b Author: Ronald G. Minnich rminnich@google.com Date: Mon Jan 9 15:24:32 2017 -0800
Revert "spi: Get rid of SPI_ATOMIC_SEQUENCING"
It broke the KGPE-D16
This reverts commit c2973d196d1224a1253478dc29d5f8fa004eaab8.
Change-Id: Ib7c598c8a1bfc14bbeac0074c7c5c82e1c7e4c5e Signed-off-by: Ronald G. Minnich rminnich@google.com --- src/drivers/spi/Kconfig | 10 ++++ src/drivers/spi/spi-generic.c | 99 -------------------------------- src/drivers/spi/spi_flash.c | 47 ++++++++++----- src/include/spi-generic.h | 62 +------------------- src/mainboard/google/purin/Kconfig | 1 + src/soc/broadcom/cygnus/spi.c | 1 - src/soc/imgtec/pistachio/Kconfig | 1 + src/soc/imgtec/pistachio/spi.c | 7 ++- src/soc/intel/baytrail/spi.c | 1 - src/soc/intel/braswell/spi.c | 1 - src/soc/intel/broadwell/spi.c | 1 - src/soc/intel/fsp_baytrail/spi.c | 1 - src/soc/intel/fsp_broadwell_de/spi.c | 1 - src/soc/mediatek/mt8173/Kconfig | 1 + src/soc/mediatek/mt8173/spi.c | 1 - src/soc/qualcomm/ipq40xx/Kconfig | 1 + src/soc/qualcomm/ipq806x/Kconfig | 1 + src/southbridge/amd/agesa/hudson/spi.c | 1 - src/southbridge/amd/cimx/sb800/spi.c | 1 - src/southbridge/amd/sb700/spi.c | 1 - src/southbridge/intel/common/spi.c | 1 - src/southbridge/intel/fsp_rangeley/spi.c | 1 - 22 files changed, 54 insertions(+), 188 deletions(-)
diff --git a/src/drivers/spi/Kconfig b/src/drivers/spi/Kconfig index c8d86ff..b55de58 100644 --- a/src/drivers/spi/Kconfig +++ b/src/drivers/spi/Kconfig @@ -61,6 +61,16 @@ 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-generic.c b/src/drivers/spi/spi-generic.c index 805e17a..4fcd04c 100644 --- a/src/drivers/spi/spi-generic.c +++ b/src/drivers/spi/spi-generic.c @@ -14,7 +14,6 @@ * GNU General Public License for more details. */
-#include <assert.h> #include <spi-generic.h> #include <string.h>
@@ -33,55 +32,10 @@ void spi_release_bus(const struct spi_slave *slave) ctrlr->release_bus(slave); }
-static int spi_xfer_single_op(const struct spi_slave *slave, - struct spi_op *op) -{ - const struct spi_ctrlr *ctrlr = slave->ctrlr; - int ret; - - if (!ctrlr || !ctrlr->xfer) - return -1; - - ret = ctrlr->xfer(slave, op->dout, op->bytesout, op->din, op->bytesin); - if (ret) - op->status = SPI_OP_FAILURE; - else - op->status = SPI_OP_SUCCESS; - - return ret; -} - -static int spi_xfer_vector_default(const struct spi_slave *slave, - struct spi_op vectors[], size_t count) -{ - size_t i; - int ret; - - for (i = 0; i < count; i++) { - ret = spi_xfer_single_op(slave, &vectors[i]); - if (ret) - return ret; - } - - return 0; -} - -int spi_xfer_vector(const struct spi_slave *slave, - struct spi_op vectors[], size_t count) -{ - const struct spi_ctrlr *ctrlr = slave->ctrlr; - - if (ctrlr && ctrlr->xfer_vector) - return ctrlr->xfer_vector(slave, vectors, count); - - return spi_xfer_vector_default(slave, vectors, count); -} - int spi_xfer(const struct spi_slave *slave, const void *dout, size_t bytesout, void *din, size_t bytesin) { const struct spi_ctrlr *ctrlr = slave->ctrlr; - if (ctrlr && ctrlr->xfer) return ctrlr->xfer(slave, dout, bytesout, din, bytesin);
@@ -122,56 +76,3 @@ int __attribute__((weak)) spi_setup_slave(unsigned int bus, unsigned int cs,
return 0; } - -static int spi_xfer_combine_two_vectors(const struct spi_slave *slave, - struct spi_op *v1, struct spi_op *v2) -{ - struct spi_op op = { - .dout = v1->dout, .bytesout = v1->bytesout, - .din = v2->din, .bytesin = v2->bytesin, - }; - int ret; - - /* - * Combine two vectors only if: - * v1 has non-NULL dout and NULL din and - * v2 has non-NULL din and NULL dout and - * - * In all other cases, do not combine the two vectors. - */ - if ((!v1->dout || v1->din) || (v2->dout || !v2->din)) - return -1; - - ret = spi_xfer_single_op(slave, &op); - v1->status = v2->status = op.status; - - return ret; -} - -/* - * Helper function to allow chipsets to combine two vectors if possible. This - * function can only handle upto 2 vectors. - * - * Two vectors are combined if first vector has a non-NULL dout and NULL din and - * second vector has a non-NULL din and NULL dout. Otherwise, each vector is - * operated upon one at a time. - * - * Returns 0 on success and non-zero on failure. - */ -int spi_xfer_two_vectors(const struct spi_slave *slave, - struct spi_op vectors[], size_t count) -{ - int ret; - - assert (count <= 2); - - if (count == 2) { - ret = spi_xfer_combine_two_vectors(slave, &vectors[0], - &vectors[1]); - - if (!ret || (vectors[0].status != SPI_OP_NOT_EXECUTED)) - return ret; - } - - return spi_xfer_vector_default(slave, vectors, count); -} diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index a0e3105..3b4272b 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -32,30 +32,47 @@ 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(const struct spi_slave *spi, const void *dout, size_t bytes_out, void *din, size_t bytes_in) { int ret = 1; - /* - * SPI flash requires command-response kind of behavior. Thus, two - * separate SPI vectors are required -- first to transmit dout and other - * to receive in din. If some specialized SPI flash controllers - * (e.g. x86) can perform both command and response together, it should - * be handled at SPI flash controller driver level. - */ - struct spi_op vectors[] = { - [0] = { .dout = dout, .bytesout = bytes_out, - .din = NULL, .bytesin = 0, }, - [1] = { .dout = NULL, .bytesout = 0, - .din = din, .bytesin = bytes_in }, - };
if (spi_claim_bus(spi)) return ret;
- if (spi_xfer_vector(spi, vectors, ARRAY_SIZE(vectors)) == 0) - ret = 0; +#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
+ ret = 0; +done: spi_release_bus(spi); return ret; } diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h index 7eb18a6..d28fefd 100644 --- a/src/include/spi-generic.h +++ b/src/include/spi-generic.h @@ -36,46 +36,20 @@ struct spi_slave { const struct spi_ctrlr *ctrlr; };
-/* Representation of SPI operation status. */ -enum spi_op_status { - SPI_OP_NOT_EXECUTED = 0, - SPI_OP_SUCCESS = 1, - SPI_OP_FAILURE = 2, -}; - -/* - * Representation of a SPI operation. - * - * dout: Pointer to data to send. - * bytesout: Count of data in bytes to send. - * din: Pointer to store received data. - * bytesin: Count of data in bytes to receive. - */ -struct spi_op { - const void *dout; - size_t bytesout; - void *din; - size_t bytesin; - enum spi_op_status status; -}; - /*----------------------------------------------------------------------- * Representation of a SPI contoller. * * claim_bus: Claim SPI bus and prepare for communication. * release_bus: Release SPI bus. + * xfer: SPI transfer * setup: Setup given SPI device bus. - * xfer: Perform one SPI transfer operation. - * xfer_vector: Vector of SPI transfer operations. */ struct spi_ctrlr { int (*claim_bus)(const struct spi_slave *slave); void (*release_bus)(const struct spi_slave *slave); - int (*setup)(const struct spi_slave *slave); int (*xfer)(const struct spi_slave *slave, const void *dout, size_t bytesout, void *din, size_t bytesin); - int (*xfer_vector)(const struct spi_slave *slave, - struct spi_op vectors[], size_t count); + int (*setup)(const struct spi_slave *slave); };
/*----------------------------------------------------------------------- @@ -160,19 +134,6 @@ void spi_release_bus(const struct spi_slave *slave); int spi_xfer(const struct spi_slave *slave, const void *dout, size_t bytesout, void *din, size_t bytesin);
-/*----------------------------------------------------------------------- - * Vector of SPI transfer operations - * - * spi_xfer_vector() interface: - * slave: The SPI slave which will be sending/receiving the data. - * vectors: Array of SPI op structures. - * count: Number of SPI op vectors. - * - * Returns: 0 on success, not 0 on failure - */ -int spi_xfer_vector(const struct spi_slave *slave, - struct spi_op vectors[], size_t count); - unsigned int spi_crop_chunk(unsigned int cmd_len, unsigned int buf_len);
/*----------------------------------------------------------------------- @@ -197,23 +158,4 @@ static inline int spi_w8r8(const struct spi_slave *slave, unsigned char byte) return ret < 0 ? ret : din[1]; }
-/* - * Helper function to allow chipsets to combine two vectors if possible. It can - * only handle upto 2 vectors. - * - * This function is provided to support command-response kind of transactions - * expected by users like flash. Some special SPI flash controllers can handle - * such command-response operations in a single transaction. For these special - * controllers, separate command and response vectors can be combined into a - * single operation. - * - * Two vectors are combined if first vector has a non-NULL dout and NULL din and - * second vector has a non-NULL din and NULL dout. Otherwise, each vector is - * operated upon one at a time. - * - * Returns 0 on success and non-zero on failure. - */ -int spi_xfer_two_vectors(const struct spi_slave *slave, - struct spi_op vectors[], size_t count); - #endif /* _SPI_GENERIC_H_ */ diff --git a/src/mainboard/google/purin/Kconfig b/src/mainboard/google/purin/Kconfig index ca0909b..eabab2b 100644 --- a/src/mainboard/google/purin/Kconfig +++ b/src/mainboard/google/purin/Kconfig @@ -26,6 +26,7 @@ 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/broadcom/cygnus/spi.c b/src/soc/broadcom/cygnus/spi.c index f03d453..e597efc 100644 --- a/src/soc/broadcom/cygnus/spi.c +++ b/src/soc/broadcom/cygnus/spi.c @@ -279,7 +279,6 @@ static const struct spi_ctrlr spi_ctrlr = { .claim_bus = spi_ctrlr_claim_bus, .release_bus = spi_ctrlr_release_bus, .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/soc/imgtec/pistachio/Kconfig b/src/soc/imgtec/pistachio/Kconfig index 1ce488c..da33cc5 100644 --- a/src/soc/imgtec/pistachio/Kconfig +++ b/src/soc/imgtec/pistachio/Kconfig @@ -22,6 +22,7 @@ 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 2b706f0..e956e46 100644 --- a/src/soc/imgtec/pistachio/spi.c +++ b/src/soc/imgtec/pistachio/spi.c @@ -22,6 +22,10 @@ #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)
@@ -492,7 +496,7 @@ static int do_spi_xfer(const struct spi_slave *slave, const void *dout, }
static int spi_ctrlr_xfer(const struct spi_slave *slave, const void *dout, - size_t bytesout, void *din, size_t bytesin) + size_t bytesout, void *din, size_t bytesin) { unsigned int in_sz, out_sz; int ret; @@ -537,7 +541,6 @@ static const struct spi_ctrlr spi_ctrlr = { .claim_bus = spi_ctrlr_claim_bus, .release_bus = spi_ctrlr_release_bus, .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
/* Set up communications parameters for a SPI slave. */ diff --git a/src/soc/intel/baytrail/spi.c b/src/soc/intel/baytrail/spi.c index 639954b..0f7b0c6 100644 --- a/src/soc/intel/baytrail/spi.c +++ b/src/soc/intel/baytrail/spi.c @@ -612,7 +612,6 @@ static int spi_ctrlr_xfer(const struct spi_slave *slave, const void *dout,
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/soc/intel/braswell/spi.c b/src/soc/intel/braswell/spi.c index 86b3351..2a0ddf8 100644 --- a/src/soc/intel/braswell/spi.c +++ b/src/soc/intel/braswell/spi.c @@ -596,7 +596,6 @@ static int spi_ctrlr_xfer(const struct spi_slave *slave, const void *dout,
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/soc/intel/broadwell/spi.c b/src/soc/intel/broadwell/spi.c index 7a7eaf7..d2ae943 100644 --- a/src/soc/intel/broadwell/spi.c +++ b/src/soc/intel/broadwell/spi.c @@ -646,7 +646,6 @@ int spi_flash_protect(u32 start, u32 size)
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/soc/intel/fsp_baytrail/spi.c b/src/soc/intel/fsp_baytrail/spi.c index dee7440..997bd13 100644 --- a/src/soc/intel/fsp_baytrail/spi.c +++ b/src/soc/intel/fsp_baytrail/spi.c @@ -592,7 +592,6 @@ spi_xfer_exit:
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/soc/intel/fsp_broadwell_de/spi.c b/src/soc/intel/fsp_broadwell_de/spi.c index a6b6f35..5848966 100644 --- a/src/soc/intel/fsp_broadwell_de/spi.c +++ b/src/soc/intel/fsp_broadwell_de/spi.c @@ -609,7 +609,6 @@ spi_xfer_exit:
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/soc/mediatek/mt8173/Kconfig b/src/soc/mediatek/mt8173/Kconfig index 7a6ad87..ec3481e 100644 --- a/src/soc/mediatek/mt8173/Kconfig +++ b/src/soc/mediatek/mt8173/Kconfig @@ -9,6 +9,7 @@ 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/mediatek/mt8173/spi.c b/src/soc/mediatek/mt8173/spi.c index 415764a..53d5b8c 100644 --- a/src/soc/mediatek/mt8173/spi.c +++ b/src/soc/mediatek/mt8173/spi.c @@ -293,7 +293,6 @@ static const struct spi_ctrlr spi_ctrlr = { .claim_bus = spi_ctrlr_claim_bus, .release_bus = spi_ctrlr_release_bus, .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/soc/qualcomm/ipq40xx/Kconfig b/src/soc/qualcomm/ipq40xx/Kconfig index 05f29e4..f738622 100644 --- a/src/soc/qualcomm/ipq40xx/Kconfig +++ b/src/soc/qualcomm/ipq40xx/Kconfig @@ -7,6 +7,7 @@ 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 32b61bc..7ba5df5 100644 --- a/src/soc/qualcomm/ipq806x/Kconfig +++ b/src/soc/qualcomm/ipq806x/Kconfig @@ -7,6 +7,7 @@ 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/southbridge/amd/agesa/hudson/spi.c b/src/southbridge/amd/agesa/hudson/spi.c index 00f6b29..8a4adfb 100644 --- a/src/southbridge/amd/agesa/hudson/spi.c +++ b/src/southbridge/amd/agesa/hudson/spi.c @@ -167,7 +167,6 @@ int chipset_volatile_group_end(const struct spi_flash *flash)
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/southbridge/amd/cimx/sb800/spi.c b/src/southbridge/amd/cimx/sb800/spi.c index 1e84743..edf192a 100644 --- a/src/southbridge/amd/cimx/sb800/spi.c +++ b/src/southbridge/amd/cimx/sb800/spi.c @@ -158,7 +158,6 @@ int chipset_volatile_group_end(const struct spi_flash *flash)
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/southbridge/amd/sb700/spi.c b/src/southbridge/amd/sb700/spi.c index 5d56415..2e16ca8 100644 --- a/src/southbridge/amd/sb700/spi.c +++ b/src/southbridge/amd/sb700/spi.c @@ -120,7 +120,6 @@ static int spi_ctrlr_xfer(const struct spi_slave *slave, const void *dout,
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index ee94937..093017e 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -659,7 +659,6 @@ static int spi_ctrlr_xfer(const struct spi_slave *slave, const void *dout,
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave) diff --git a/src/southbridge/intel/fsp_rangeley/spi.c b/src/southbridge/intel/fsp_rangeley/spi.c index 0bffeb5..8026698 100644 --- a/src/southbridge/intel/fsp_rangeley/spi.c +++ b/src/southbridge/intel/fsp_rangeley/spi.c @@ -724,7 +724,6 @@ spi_xfer_exit:
static const struct spi_ctrlr spi_ctrlr = { .xfer = spi_ctrlr_xfer, - .xfer_vector = spi_xfer_two_vectors, };
int spi_setup_slave(unsigned int bus, unsigned int cs, struct spi_slave *slave)
-
Ronald G. Minnich