[coreboot-gerrit] New patch to review for coreboot: beafb2d spi_flash: Move (de-)assertion of /CS to single location

Marc Jones (marc.jones@se-eng.com) gerrit at coreboot.org
Tue Dec 16 23:35:42 CET 2014


Marc Jones (marc.jones at se-eng.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/7829

-gerrit

commit beafb2dfc263c1954257942ce9eff2428e118eb4
Author: David Hendricks <dhendrix at chromium.org>
Date:   Fri Apr 11 19:48:55 2014 -0700

    spi_flash: Move (de-)assertion of /CS to single location
    
    This consolidates all calls to spi_claim_bus() and spi_release_bus()
    to a single location where spi_xfer() is called. This avoids confusing
    (and potentially redundant) calls that were being done throughout the
    generic spi_flash.c functions and chip-specific functions.
    
    I don't think the current approach could even work since many chip
    drivers assert /CS once and then issue multiple commands such as page
    program followed by reading the status register. I suspect the reason
    we didn't notice it on x86 is because the ICH/PCH handled each
    individual command correctly (spi_claim_bus() and spi_release_bus()
    are noops) in spite of the broken code.
    
    BUG=none
    BRANCH=none
    TEST=tested on nyan and link
    Signed-off-by: David Hendricks <dhendrix at chromium.org>
    
    Original-Change-Id: I3257e2f6a2820834f4c9018069f90fcf2bab05f6
    Original-Reviewed-on: https://chromium-review.googlesource.com/194510
    Original-Reviewed-by: David Hendricks <dhendrix at chromium.org>
    Original-Commit-Queue: David Hendricks <dhendrix at chromium.org>
    Original-Tested-by: David Hendricks <dhendrix at chromium.org>
    (cherry picked from commit d3394d34fb49e9e252f67371674d5b3aa220bc9e)
    Signed-off-by: Marc Jones <marc.jones at se-eng.com>
    
    Change-Id: Ieb62309b18090d8f974f91a6e448af3d65dd3d1d
---
 src/drivers/spi/eon.c        |  9 +--------
 src/drivers/spi/gigadevice.c |  9 +--------
 src/drivers/spi/macronix.c   |  9 +--------
 src/drivers/spi/spansion.c   |  9 +--------
 src/drivers/spi/spi_flash.c  | 21 ++++-----------------
 src/drivers/spi/sst.c        |  8 +-------
 src/drivers/spi/stmicro.c    |  7 +------
 src/drivers/spi/winbond.c    |  8 +-------
 8 files changed, 11 insertions(+), 69 deletions(-)

diff --git a/src/drivers/spi/eon.c b/src/drivers/spi/eon.c
index 85567eb..69869e9 100644
--- a/src/drivers/spi/eon.c
+++ b/src/drivers/spi/eon.c
@@ -73,7 +73,7 @@ static int eon_write(struct spi_flash *flash,
 	unsigned long page_size;
 	size_t chunk_len;
 	size_t actual;
-	int ret;
+	int ret = 0;
 	u8 cmd[4];
 
 	page_size = eon->params->page_size;
@@ -81,13 +81,7 @@ static int eon_write(struct spi_flash *flash,
 	byte_addr = offset % page_size;
 
 	flash->spi->rw = SPI_WRITE_FLAG;
-	ret = spi_claim_bus(flash->spi);
-	if (ret) {
-		printk(BIOS_WARNING, "SF: Unable to claim SPI bus\n");
-		return ret;
-	}
 
-	ret = 0;
 	for (actual = 0; actual < len; actual += chunk_len) {
 		chunk_len = min(len - actual, page_size - byte_addr);
 
@@ -128,7 +122,6 @@ static int eon_write(struct spi_flash *flash,
 	      len, offset);
 #endif
 
-	spi_release_bus(flash->spi);
 	return ret;
 }
 
diff --git a/src/drivers/spi/gigadevice.c b/src/drivers/spi/gigadevice.c
index d9d4e17..68b487e 100644
--- a/src/drivers/spi/gigadevice.c
+++ b/src/drivers/spi/gigadevice.c
@@ -126,19 +126,13 @@ static int gigadevice_write(struct spi_flash *flash, u32 offset,
 	unsigned long page_size;
 	size_t chunk_len;
 	size_t actual;
-	int ret;
+	int ret = 0;
 	u8 cmd[4];
 
 	page_size = 1 << stm->params->l2_page_size;
 	byte_addr = offset % page_size;
 
 	flash->spi->rw = SPI_WRITE_FLAG;
-	ret = spi_claim_bus(flash->spi);
-	if (ret) {
-		printk(BIOS_WARNING,
-		       "SF gigadevice.c: Unable to claim SPI bus\n");
-		return ret;
-	}
 
 	for (actual = 0; actual < len; actual += chunk_len) {
 		chunk_len = min(len - actual, page_size - byte_addr);
@@ -187,7 +181,6 @@ static int gigadevice_write(struct spi_flash *flash, u32 offset,
 	ret = 0;
 
 out:
-	spi_release_bus(flash->spi);
 	return ret;
 }
 
diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c
index 1516770..51d8956 100644
--- a/src/drivers/spi/macronix.c
+++ b/src/drivers/spi/macronix.c
@@ -161,20 +161,14 @@ static int macronix_write(struct spi_flash *flash,
 	unsigned long page_size;
 	size_t chunk_len;
 	size_t actual;
-	int ret;
+	int ret = 0;
 	u8 cmd[4];
 
 	page_size = mcx->params->page_size;
 	byte_addr = offset % page_size;
 
 	flash->spi->rw = SPI_WRITE_FLAG;
-	ret = spi_claim_bus(flash->spi);
-	if (ret) {
-		printk(BIOS_WARNING, "SF: Unable to claim SPI bus\n");
-		return ret;
-	}
 
-	ret = 0;
 	for (actual = 0; actual < len; actual += chunk_len) {
 		chunk_len = min(len - actual, page_size - byte_addr);
 		chunk_len = spi_crop_chunk(sizeof(cmd), chunk_len);
@@ -215,7 +209,6 @@ static int macronix_write(struct spi_flash *flash,
 	      " 0x%lx\n", len, (unsigned long)(offset - len));
 #endif
 
-	spi_release_bus(flash->spi);
 	return ret;
 }
 
diff --git a/src/drivers/spi/spansion.c b/src/drivers/spi/spansion.c
index fc96e40..fba816a 100644
--- a/src/drivers/spi/spansion.c
+++ b/src/drivers/spi/spansion.c
@@ -141,7 +141,7 @@ static int spansion_write(struct spi_flash *flash,
 	unsigned long page_size;
 	size_t chunk_len;
 	size_t actual;
-	int ret;
+	int ret = 0;
 	u8 cmd[4];
 
 	page_size = spsn->params->page_size;
@@ -149,13 +149,7 @@ static int spansion_write(struct spi_flash *flash,
 	byte_addr = offset % page_size;
 
 	flash->spi->rw = SPI_WRITE_FLAG;
-	ret = spi_claim_bus(flash->spi);
-	if (ret) {
-		printk(BIOS_WARNING, "SF: Unable to claim SPI bus\n");
-		return ret;
-	}
 
-	ret = 0;
 	for (actual = 0; actual < len; actual += chunk_len) {
 		chunk_len = min(len - actual, page_size - byte_addr);
 
@@ -196,7 +190,6 @@ static int spansion_write(struct spi_flash *flash,
 	      len, offset);
 #endif
 
-	spi_release_bus(flash->spi);
 	return ret;
 }
 
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index 0d67c43..d737ee9 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -46,6 +46,9 @@ static int do_spi_flash_cmd(struct spi_slave *spi, const void *dout,
 {
 	int ret = 1;
 
+	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;
@@ -63,6 +66,7 @@ static int do_spi_flash_cmd(struct spi_slave *spi, const void *dout,
 
 	ret = 0;
 done:
+	spi_release_bus(spi);
 	return ret;
 }
 
@@ -111,9 +115,7 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
 	int ret;
 
 	spi->rw = SPI_READ_FLAG;
-	spi_claim_bus(spi);
 	ret = spi_flash_cmd_read(spi, cmd, cmd_len, data, data_len);
-	spi_release_bus(spi);
 
 	return ret;
 }
@@ -188,11 +190,6 @@ int spi_flash_cmd_erase(struct spi_flash *flash, u8 erase_cmd,
 	}
 
 	flash->spi->rw = SPI_WRITE_FLAG;
-	ret = spi_claim_bus(flash->spi);
-	if (ret) {
-		printk(BIOS_WARNING, "SF: Unable to claim SPI bus\n");
-		return ret;
-	}
 
 	cmd[0] = erase_cmd;
 	start = offset;
@@ -222,7 +219,6 @@ int spi_flash_cmd_erase(struct spi_flash *flash, u8 erase_cmd,
 	printk(BIOS_DEBUG, "SF: Successfully erased %zu bytes @ %#x\n", len, start);
 
 out:
-	spi_release_bus(flash->spi);
 	return ret;
 }
 
@@ -307,11 +303,6 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs)
 	}
 
 	spi->rw = SPI_READ_FLAG;
-	ret = spi_claim_bus(spi);
-	if (ret) {
-		printk(BIOS_WARNING, "SF: Failed to claim SPI bus: %d\n", ret);
-		goto err_claim_bus;
-	}
 
 	if (spi->force_programmer_specific && spi->programmer_specific_probe) {
 		flash = spi->programmer_specific_probe (spi);
@@ -375,13 +366,9 @@ flash_detected:
 	printk(BIOS_INFO, "SF: Detected %s with page size %x, total %x\n",
 			flash->name, flash->sector_size, flash->size);
 
-	spi_release_bus(spi);
-
 	return flash;
 
 err_manufacturer_probe:
 err_read_id:
-	spi_release_bus(spi);
-err_claim_bus:
 	return NULL;
 }
diff --git a/src/drivers/spi/sst.c b/src/drivers/spi/sst.c
index d81888a..f681b7e 100644
--- a/src/drivers/spi/sst.c
+++ b/src/drivers/spi/sst.c
@@ -146,15 +146,10 @@ static int
 sst_write(struct spi_flash *flash, u32 offset, size_t len, const void *buf)
 {
 	size_t actual, cmd_len;
-	int ret;
+	int ret = 0;
 	u8 cmd[4];
 
 	flash->spi->rw = SPI_WRITE_FLAG;
-	ret = spi_claim_bus(flash->spi);
-	if (ret) {
-		printk(BIOS_WARNING, "SF: Unable to claim SPI bus\n");
-		return ret;
-	}
 
 	/* If the data is not word aligned, write out leading single byte */
 	actual = offset % 2;
@@ -209,7 +204,6 @@ done:
 	printk(BIOS_SPEW, "SF: SST: program %s %zu bytes @ 0x%lx\n",
 	      ret ? "failure" : "success", len, (unsigned long)offset - actual);
 #endif
-	spi_release_bus(flash->spi);
 	return ret;
 }
 
diff --git a/src/drivers/spi/stmicro.c b/src/drivers/spi/stmicro.c
index fda6124..70ff107 100644
--- a/src/drivers/spi/stmicro.c
+++ b/src/drivers/spi/stmicro.c
@@ -167,18 +167,13 @@ static int stmicro_write(struct spi_flash *flash,
 	unsigned long page_size;
 	size_t chunk_len;
 	size_t actual;
-	int ret;
+	int ret = 0;
 	u8 cmd[4];
 
 	page_size = stm->params->page_size;
 	byte_addr = offset % page_size;
 
 	flash->spi->rw = SPI_WRITE_FLAG;
-	ret = spi_claim_bus(flash->spi);
-	if (ret) {
-		printk(BIOS_WARNING, "SF: Unable to claim SPI bus\n");
-		return ret;
-	}
 
 	for (actual = 0; actual < len; actual += chunk_len) {
 		chunk_len = min(len - actual, page_size - byte_addr);
diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c
index 907e52d..4c4150f 100644
--- a/src/drivers/spi/winbond.c
+++ b/src/drivers/spi/winbond.c
@@ -136,18 +136,13 @@ static int winbond_write(struct spi_flash *flash,
 	unsigned long page_size;
 	size_t chunk_len;
 	size_t actual;
-	int ret;
+	int ret = 0;
 	u8 cmd[4];
 
 	page_size = 1 << stm->params->l2_page_size;
 	byte_addr = offset % page_size;
 
 	flash->spi->rw = SPI_WRITE_FLAG;
-	ret = spi_claim_bus(flash->spi);
-	if (ret) {
-		printk(BIOS_WARNING, "SF: Unable to claim SPI bus\n");
-		return ret;
-	}
 
 	for (actual = 0; actual < len; actual += chunk_len) {
 		chunk_len = min(len - actual, page_size - byte_addr);
@@ -191,7 +186,6 @@ static int winbond_write(struct spi_flash *flash,
 	ret = 0;
 
 out:
-	spi_release_bus(flash->spi);
 	return ret;
 }
 



More information about the coreboot-gerrit mailing list