From 152df810ff435b40c0667c55317f2b1d0c50d81e Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Thu, 27 Mar 2014 21:52:43 -0700 Subject: [PATCH] spi: Change spi_xfer to work in units of bytes instead of bits. Whenever spi_xfer is called and whenver it's implemented, the natural unit for the amount of data being transfered is bytes. The API expected things to be expressed in bits, however, which led to a lot of multiplying and dividing by eight, and checkes to make sure things were multiples of eight. All of that can now be removed. BUG=None TEST=Built and booted on link, falco, peach_pit and nyan and looked for SPI errors in the firmware log. Built for rambi. BRANCH=None Change-Id: I02365bdb6960a35def7be7a0cd1aa0a2cc09392f Signed-off-by: Gabe Black Reviewed-on: https://chromium-review.googlesource.com/192049 Reviewed-by: Gabe Black Tested-by: Gabe Black Commit-Queue: Gabe Black --- src/drivers/spi/spi_flash.c | 6 +++--- src/ec/google/chromeec/ec_spi.c | 6 +++--- src/include/spi-generic.h | 24 +++++++----------------- src/soc/intel/baytrail/spi.c | 15 +++++---------- src/soc/nvidia/tegra124/spi.c | 9 +++------ src/soc/samsung/exynos5420/spi.c | 6 ++---- src/southbridge/amd/agesa/hudson/spi.c | 7 ++----- src/southbridge/amd/cimx/sb800/spi.c | 7 ++----- src/southbridge/intel/bd82x6x/spi.c | 15 +++++---------- src/southbridge/intel/lynxpoint/spi.c | 15 +++++---------- 10 files changed, 37 insertions(+), 73 deletions(-) diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index 1dd6d9543e..3486500830 100644 --- a/src/drivers/spi/spi_flash.c +++ b/src/drivers/spi/spi_flash.c @@ -27,7 +27,7 @@ static void spi_flash_addr(u32 addr, u8 *cmd) int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len) { - int ret = spi_xfer(spi, &cmd, 8, response, len * 8); + int ret = spi_xfer(spi, &cmd, sizeof(cmd), response, len); if (ret) printk(BIOS_WARNING, "SF: Failed to send command %02x: %d\n", cmd, ret); @@ -37,7 +37,7 @@ int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len) int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd, size_t cmd_len, void *data, size_t data_len) { - int ret = spi_xfer(spi, cmd, cmd_len * 8, data, data_len * 8); + int ret = spi_xfer(spi, cmd, cmd_len, data, data_len); if (ret) { printk(BIOS_WARNING, "SF: Failed to send read command (%zu bytes): %d\n", data_len, ret); @@ -54,7 +54,7 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len, memcpy(buff, cmd, cmd_len); memcpy(buff + cmd_len, data, data_len); - ret = spi_xfer(spi, buff, (cmd_len + data_len) * 8, NULL, 0); + ret = spi_xfer(spi, buff, cmd_len + data_len, NULL, 0); if (ret) { printk(BIOS_WARNING, "SF: Failed to send write command (%zu bytes): %d\n", data_len, ret); diff --git a/src/ec/google/chromeec/ec_spi.c b/src/ec/google/chromeec/ec_spi.c index 137b2e8854..f1bdd3caaa 100644 --- a/src/ec/google/chromeec/ec_spi.c +++ b/src/ec/google/chromeec/ec_spi.c @@ -33,7 +33,7 @@ static int crosec_spi_io(uint8_t *write_bytes, size_t write_size, spi_claim_bus(slave); - if (spi_xfer(slave, write_bytes, write_size * 8, NULL, 0)) { + 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; @@ -44,7 +44,7 @@ static int crosec_spi_io(uint8_t *write_bytes, size_t write_size, struct rela_time rt; timer_monotonic_get(&start); while (1) { - if (spi_xfer(slave, NULL, 0, &byte, 8)) { + if (spi_xfer(slave, NULL, 0, &byte, sizeof(byte))) { printk(BIOS_ERR, "%s: Failed to receive byte.\n", __func__); spi_release_bus(slave); @@ -64,7 +64,7 @@ static int crosec_spi_io(uint8_t *write_bytes, size_t write_size, } } - if (spi_xfer(slave, NULL, 0, read_bytes, read_size * 8)) { + 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; diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h index f6d4bed045..fbd1b132a6 100644 --- a/src/include/spi-generic.h +++ b/src/include/spi-generic.h @@ -102,27 +102,17 @@ void spi_release_bus(struct spi_slave *slave); /*----------------------------------------------------------------------- * SPI transfer * - * This writes "bitlen" bits out the SPI MOSI port and simultaneously clocks - * "bitlen" bits in the SPI MISO port. That's just the way SPI works. - * - * The source of the outgoing bits is the "dout" parameter and the - * destination of the input bits is the "din" parameter. Note that "dout" - * and "din" can point to the same memory location, in which case the - * input data overwrites the output data (since both are buffered by - * temporary variables, this is OK). - * * spi_xfer() interface: * slave: The SPI slave which will be sending/receiving the data. - * dout: Pointer to a string of bits to send out. The bits are - * held in a byte array and are sent MSB first. - * bitsout: How many bits to write. - * din: Pointer to a string of bits that will be filled in. - * bitsin: How many bits to read. + * dout: Pointer to a string of bytes to send out. + * bytesout: How many bytes to write. + * din: Pointer to a string of bytes that will be filled in. + * bytesin: How many bytes to read. * * Returns: 0 on success, not 0 on failure */ -int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bitsout, - void *din, unsigned int bitsin); +int spi_xfer(struct spi_slave *slave, const void *dout, unsigned int bytesout, + void *din, unsigned int bytesin); /*----------------------------------------------------------------------- * Write 8 bits, then read 8 bits. @@ -142,7 +132,7 @@ static inline int spi_w8r8(struct spi_slave *slave, unsigned char byte) dout[0] = byte; dout[1] = 0; - ret = spi_xfer(slave, dout, 16, din, 16); + ret = spi_xfer(slave, dout, 2, din, 2); return ret < 0 ? ret : din[1]; } diff --git a/src/soc/intel/baytrail/spi.c b/src/soc/intel/baytrail/spi.c index 61856006d5..e78d5c8bbe 100644 --- a/src/soc/intel/baytrail/spi.c +++ b/src/soc/intel/baytrail/spi.c @@ -484,7 +484,7 @@ static int ich_status_poll(u16 bitmask, int wait_til_set) } int spi_xfer(struct spi_slave *slave, const void *dout, - unsigned int bitsout, void *din, unsigned int bitsin) + unsigned int bytesout, void *din, unsigned int bytesin) { uint16_t control; int16_t opcode_index; @@ -492,26 +492,21 @@ int spi_xfer(struct spi_slave *slave, const void *dout, int status; spi_transaction trans = { - dout, bitsout / 8, - din, bitsin / 8, + dout, bytesout, + din, bytesin, 0xff, 0xff, 0 }; /* There has to always at least be an opcode. */ - if (!bitsout || !dout) { + if (!bytesout || !dout) { printk(BIOS_DEBUG, "ICH SPI: No opcode for transfer\n"); return -1; } /* Make sure if we read something we have a place to put it. */ - if (bitsin != 0 && !din) { + if (bytesin != 0 && !din) { printk(BIOS_DEBUG, "ICH SPI: Read but no target buffer\n"); return -1; } - /* Right now we don't support writing partial bytes. */ - if (bitsout % 8 || bitsin % 8) { - printk(BIOS_DEBUG, "ICH SPI: Accessing partial bytes not supported\n"); - return -1; - } if (ich_status_poll(SPIS_SCIP, 0) == -1) return -1; diff --git a/src/soc/nvidia/tegra124/spi.c b/src/soc/nvidia/tegra124/spi.c index 2b5d2d781b..6e97a76d6d 100644 --- a/src/soc/nvidia/tegra124/spi.c +++ b/src/soc/nvidia/tegra124/spi.c @@ -715,17 +715,14 @@ static int xfer_finish(struct tegra_spi_channel *spi) } int spi_xfer(struct spi_slave *slave, const void *dout, - unsigned int bitsout, void *din, unsigned int bitsin) + unsigned int out_bytes, void *din, unsigned int in_bytes) { - unsigned int out_bytes = bitsout / 8, in_bytes = bitsin / 8; struct tegra_spi_channel *spi = to_tegra_spi(slave->bus); u8 *out_buf = (u8 *)dout; u8 *in_buf = (u8 *)din; unsigned int todo; int ret = 0; - ASSERT(bitsout % 8 == 0 && bitsin % 8 == 0); - /* tegra bus numbers start at 1 */ ASSERT(slave->bus >= 1 && slave->bus <= ARRAY_SIZE(tegra_spi_channels)); @@ -852,7 +849,7 @@ static size_t tegra_spi_cbfs_read(struct cbfs_media *media, void *dest, spi_claim_bus(spi->slave); if (spi_xfer(spi->slave, spi_read_cmd, - read_cmd_bytes * 8, NULL, 0) < 0) { + read_cmd_bytes, NULL, 0) < 0) { ret = -1; printk(BIOS_ERR, "%s: Failed to transfer %u bytes\n", __func__, sizeof(spi_read_cmd)); @@ -862,7 +859,7 @@ static size_t tegra_spi_cbfs_read(struct cbfs_media *media, void *dest, if (channel->dual_mode) { setbits_le32(&channel->regs->command1, SPI_CMD1_BOTH_EN_BIT); } - if (spi_xfer(spi->slave, NULL, 0, dest, count * 8)) { + if (spi_xfer(spi->slave, NULL, 0, dest, count)) { ret = -1; printk(BIOS_ERR, "%s: Failed to transfer %u bytes\n", __func__, count); diff --git a/src/soc/samsung/exynos5420/spi.c b/src/soc/samsung/exynos5420/spi.c index 0bbd7779bc..36742a7279 100644 --- a/src/soc/samsung/exynos5420/spi.c +++ b/src/soc/samsung/exynos5420/spi.c @@ -193,12 +193,10 @@ 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 bitsout, - void *din, unsigned int bitsin) +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; - u32 bytes_out = bitsout / 8; - u32 bytes_in = bitsin / 8; if (bytes_out && bytes_in) { u32 min_size = MIN(bytes_out, bytes_in); diff --git a/src/southbridge/amd/agesa/hudson/spi.c b/src/southbridge/amd/agesa/hudson/spi.c index 051c0bdeb1..0f331645bb 100644 --- a/src/southbridge/amd/agesa/hudson/spi.c +++ b/src/southbridge/amd/agesa/hudson/spi.c @@ -56,18 +56,15 @@ void spi_init() } int spi_xfer(struct spi_slave *slave, const void *dout, - unsigned int bitsout, void *din, unsigned int bitsin) + unsigned int bytesout, void *din, unsigned int bytesin) { /* First byte is cmd which can not being sent through FIFO. */ u8 cmd = *(u8 *)dout++; u8 readoffby1; u8 readwrite; - u8 bytesout, bytesin; u8 count; - bitsout -= 8; - bytesout = bitsout / 8; - bytesin = bitsin / 8; + bytesout--; readoffby1 = bytesout ? 0 : 1; diff --git a/src/southbridge/amd/cimx/sb800/spi.c b/src/southbridge/amd/cimx/sb800/spi.c index c3c5ee16d2..d542d3e497 100644 --- a/src/southbridge/amd/cimx/sb800/spi.c +++ b/src/southbridge/amd/cimx/sb800/spi.c @@ -57,18 +57,15 @@ void spi_init() } int spi_xfer(struct spi_slave *slave, const void *dout, - unsigned int bitsout, void *din, unsigned int bitsin) + unsigned int bytesout, void *din, unsigned int bytesin) { /* First byte is cmd which can not being sent through FIFO. */ u8 cmd = *(u8 *)dout++; u8 readoffby1; u8 readwrite; - u8 bytesout, bytesin; u8 count; - bitsout -= 8; - bytesout = bitsout / 8; - bytesin = bitsin / 8; + bytesout--; readoffby1 = bytesout ? 0 : 1; diff --git a/src/southbridge/intel/bd82x6x/spi.c b/src/southbridge/intel/bd82x6x/spi.c index bb72fcd03c..54be15d0dc 100644 --- a/src/southbridge/intel/bd82x6x/spi.c +++ b/src/southbridge/intel/bd82x6x/spi.c @@ -575,7 +575,7 @@ static int ich_status_poll(u16 bitmask, int wait_til_set) } int spi_xfer(struct spi_slave *slave, const void *dout, - unsigned int bitsout, void *din, unsigned int bitsin) + unsigned int bytesout, void *din, unsigned int bytesin) { uint16_t control; int16_t opcode_index; @@ -583,26 +583,21 @@ int spi_xfer(struct spi_slave *slave, const void *dout, int status; spi_transaction trans = { - dout, bitsout / 8, - din, bitsin / 8, + dout, bytesout, + din, bytesin, 0xff, 0xff, 0 }; /* There has to always at least be an opcode. */ - if (!bitsout || !dout) { + if (!bytesout || !dout) { printk(BIOS_DEBUG, "ICH SPI: No opcode for transfer\n"); return -1; } /* Make sure if we read something we have a place to put it. */ - if (bitsin != 0 && !din) { + if (bytesin != 0 && !din) { printk(BIOS_DEBUG, "ICH SPI: Read but no target buffer\n"); return -1; } - /* Right now we don't support writing partial bytes. */ - if (bitsout % 8 || bitsin % 8) { - printk(BIOS_DEBUG, "ICH SPI: Accessing partial bytes not supported\n"); - return -1; - } if (ich_status_poll(SPIS_SCIP, 0) == -1) return -1; diff --git a/src/southbridge/intel/lynxpoint/spi.c b/src/southbridge/intel/lynxpoint/spi.c index 5a5680f793..0ce01733aa 100644 --- a/src/southbridge/intel/lynxpoint/spi.c +++ b/src/southbridge/intel/lynxpoint/spi.c @@ -487,7 +487,7 @@ static int ich_status_poll(u16 bitmask, int wait_til_set) } int spi_xfer(struct spi_slave *slave, const void *dout, - unsigned int bitsout, void *din, unsigned int bitsin) + unsigned int bytesout, void *din, unsigned int bytesin) { uint16_t control; int16_t opcode_index; @@ -495,26 +495,21 @@ int spi_xfer(struct spi_slave *slave, const void *dout, int status; spi_transaction trans = { - dout, bitsout / 8, - din, bitsin / 8, + dout, bytesout, + din, bytesin, 0xff, 0xff, 0 }; /* There has to always at least be an opcode. */ - if (!bitsout || !dout) { + if (!bytesout || !dout) { printk(BIOS_DEBUG, "ICH SPI: No opcode for transfer\n"); return -1; } /* Make sure if we read something we have a place to put it. */ - if (bitsin != 0 && !din) { + if (bytesin != 0 && !din) { printk(BIOS_DEBUG, "ICH SPI: Read but no target buffer\n"); return -1; } - /* Right now we don't support writing partial bytes. */ - if (bitsout % 8 || bitsin % 8) { - printk(BIOS_DEBUG, "ICH SPI: Accessing partial bytes not supported\n"); - return -1; - } if (ich_status_poll(SPIS_SCIP, 0) == -1) return -1;