New patch to review for coreboot: 1e79ef7 spi: Change spi_xfer to work in units of bytes instead of bits.
Kyösti Mälkki (kyosti.malkki@gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6175
-gerrit
commit 1e79ef75dcd108156fb5a646eb8781482a1780de Author: Gabe Black gabeblack@google.com Date: Thu Mar 27 21:52:43 2014 -0700
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 gabeblack@google.com Reviewed-on: https://chromium-review.googlesource.com/192049 Reviewed-by: Gabe Black gabeblack@chromium.org Tested-by: Gabe Black gabeblack@chromium.org Commit-Queue: Gabe Black gabeblack@chromium.org [km: cherry-pick from chromium] Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- src/cpu/samsung/exynos5420/spi.c | 6 ++---- src/drivers/spi/spi_flash.c | 6 +++--- src/ec/google/chromeec/ec_spi.c | 4 ++-- src/include/spi-generic.h | 24 +++++++----------------- src/soc/intel/baytrail/spi.c | 15 +++++---------- src/soc/intel/fsp_baytrail/spi.c | 15 +++++---------- src/southbridge/amd/agesa/hudson/spi.c | 19 ++++++------------- src/southbridge/amd/cimx/sb800/spi.c | 7 ++----- src/southbridge/intel/common/spi.c | 15 +++++---------- 9 files changed, 37 insertions(+), 74 deletions(-)
diff --git a/src/cpu/samsung/exynos5420/spi.c b/src/cpu/samsung/exynos5420/spi.c index 5c546cc..46b9ced 100644 --- a/src/cpu/samsung/exynos5420/spi.c +++ b/src/cpu/samsung/exynos5420/spi.c @@ -258,15 +258,13 @@ int spi_claim_bus(struct spi_slave *slave) return 0; }
-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 out_bytes, + void *din, unsigned int in_bytes) { - unsigned int out_bytes = bitsout / 8, in_bytes = bitsin / 8; uint8_t *out_ptr = (uint8_t *)dout, *in_ptr = (uint8_t *)din; int offset, todo, len; int ret = 0;
- ASSERT(bitsout % 8 == 0 && bitsin % 8 == 0); len = MAX(out_bytes, in_bytes);
/* diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c index 8232220..ed1164c 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 012a4ca..d6b7b0c 100644 --- a/src/ec/google/chromeec/ec_spi.c +++ b/src/ec/google/chromeec/ec_spi.c @@ -31,8 +31,8 @@ static int crosec_spi_io(uint8_t *write_bytes, size_t write_size, int rv;
spi_claim_bus(slave); - rv = spi_xfer(slave, write_bytes, write_size * 8, read_bytes, - read_size * 8); + rv = spi_xfer(slave, write_bytes, write_size, read_bytes, + read_size); spi_release_bus(slave);
if (rv != 0) { diff --git a/src/include/spi-generic.h b/src/include/spi-generic.h index b7a14f9..6c62de6 100644 --- a/src/include/spi-generic.h +++ b/src/include/spi-generic.h @@ -120,27 +120,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);
/*----------------------------------------------------------------------- * Determine if a SPI chipselect is valid. @@ -196,7 +186,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 d75caca..b649f53 100644 --- a/src/soc/intel/baytrail/spi.c +++ b/src/soc/intel/baytrail/spi.c @@ -500,7 +500,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; @@ -508,26 +508,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/intel/fsp_baytrail/spi.c b/src/soc/intel/fsp_baytrail/spi.c index 5e8da95..50f1123 100644 --- a/src/soc/intel/fsp_baytrail/spi.c +++ b/src/soc/intel/fsp_baytrail/spi.c @@ -498,7 +498,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; @@ -506,26 +506,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/amd/agesa/hudson/spi.c b/src/southbridge/amd/agesa/hudson/spi.c index c19d1a0..720b26f 100644 --- a/src/southbridge/amd/agesa/hudson/spi.c +++ b/src/southbridge/amd/agesa/hudson/spi.c @@ -87,21 +87,14 @@ void spi_init(void) }
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. */ - uint8_t cmd = *(uint8_t *)dout++; - uint8_t readoffby1; -#if !CONFIG_SOUTHBRIDGE_AMD_AGESA_YANGTZE - uint8_t readwrite; -#endif - uint8_t bytesout, bytesin; - uint8_t count; - - bitsout -= 8; - bytesout = bitsout / 8; - bytesin = bitsin / 8; + u8 cmd = *(u8 *)dout++; + u8 readoffby1; + u8 count;
+ bytesout--; readoffby1 = bytesout ? 0 : 1;
#if CONFIG_SOUTHBRIDGE_AMD_AGESA_YANGTZE @@ -110,7 +103,7 @@ int spi_xfer(struct spi_slave *slave, const void *dout, spi_write(0x1E, 6); spi_write(0x1F, bytesin); /* SpiExtRegIndx [6] - RxByteCount */ #else - readwrite = (bytesin + readoffby1) << 4 | bytesout; + u8 readwrite = (bytesin + readoffby1) << 4 | bytesout; spi_write(SPI_REG_CNTRL01, readwrite); #endif spi_write(SPI_REG_OPCODE, cmd); diff --git a/src/southbridge/amd/cimx/sb800/spi.c b/src/southbridge/amd/cimx/sb800/spi.c index d059f5e..a46349e 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/common/spi.c b/src/southbridge/intel/common/spi.c index 14d2f54..76fd226 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -551,7 +551,7 @@ static int spi_is_multichip (void) }
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; @@ -559,26 +559,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;
-
Kyösti Mälkki