Report SPI driver write errors.

The ICH SPI controller can't handle more than 64 bytes in one
write transaction, but the SPI chips' drivers routinely invoke
controller transfer function (spi_xfer()) trying to write a full
SPI chip page, which often exceeds the ICH SPI controller capacity.

This condition goes unnoticed, resulting in only the first 64
bytes of the data written into the flash, the rest gets dropped
on the floor.

With this change an error value will be returned to the caller
and an error message will be printed on the screen.

Also, it turned out that the chip level flash drivers interpret
as errors only the negative values returned by the SPI interface
driver. But the SPI interface driver in ich.c was returning +1 to
report errors. This CL takes care of this discrepancy too.

BUG=chromium-os:20105
TEST=manual
  . modify the '#ifdef CONFIG_ICH_SPI' line to read
    '#ifdef  CONFIG_ICH_SPIxx' in include/spi_flash.h
  . build x86-alex bootloader image
  . program the image on an alex device
  . restart the device
  . at the 'boot>' prompt run the 'saveenv' command

Observe the command failed and the error message printed on the console.

  . restore include/spi_flash.h
  . build x86-alex bootloader image
  . program the image on an alex device
  . restart the device
  . at the 'boot>' prompt run the following commands:
     setenv xyz 'this is a string'
     saveenv
  . reboot the device
  . at the 'boot>' prompt run 'printenv'

Observe the environment include the new variable (xyz)

Change-Id: I528a85b208213c10fdf8cf3e908caf7bea94bc62
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: http://gerrit.chromium.org/gerrit/7697
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: David Hendricks <dhendrix@chromium.org>

2 files changed, 25 insertions, 12 deletions

diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
index 166a85fe02..95c594d539 100644
--- a/drivers/spi/ich.c
+++ b/drivers/spi/ich.c
@@ -557,30 +557,29 @@ int spi_xfer(struct spi_slave *slave, const void *dout,
 	/* There has to always at least be an opcode. */
 	if (!bitsout || !dout) {
 		puts("ICH SPI: No opcode for transfer\n");
-		return 1;
+		return -1;
 	}
 	/* Make sure if we read something we have a place to put it. */
 	if (bitsin != 0 && !din) {
 		puts("ICH SPI: Read but no target buffer\n");
-		return 1;
+		return -1;
 	}
 	/* Right now we don't support writing partial bytes. */
 	if (bitsout % 8 || bitsin % 8) {
 		puts("ICH SPI: Accessing partial bytes not supported\n");
-		return 1;
+		return -1;
 	}
 
-	/* 60 ms are 9.6 million cycles at 16 MHz. */
 	if (ich_status_poll(SPIS_SCIP, 0) == -1)
-		return 1;
+		return -1;
 
 	writew_(SPIS_CDS | SPIS_FCERR, cntlr.status);
 
 	spi_setup_type(&trans);
 	if ((opcode_index = spi_setup_opcode(&trans)) < 0)
-		return 1;
+		return -1;
 	if ((with_address = spi_setup_offset(&trans)) < 0)
-		return 1;
+		return -1;
 
 	/* Preset control fields */
 	control = SPIC_SCGO | ((opcode_index & 0x07) << 4);
@@ -597,17 +596,30 @@ int spi_xfer(struct spi_slave *slave, const void *dout,
 		/* wait for the result */
 		status = ich_status_poll(SPIS_CDS | SPIS_FCERR, 1);
 		if (status == -1)
-			return 1;
+			return -1;
 
 		if (status & SPIS_FCERR) {
 			puts("ICH SPI: Transaction error\n");
-			return 1;
+			return -1;
 		}
 
 		return 0;
 	}
 
 	/*
+	 * Check if this is a write command atempting to transfer more bytes
+	 * than the controller can handle. Iterations for writes are not
+	 * supported here because each SPI write command needs to be preceded
+	 * and followed by other SPI commands, and this sequence is controlled
+	 * by the SPI chip driver.
+	 */
+	if (trans.bytesout > cntlr.databytes) {
+		puts("ICH SPI: Too much to write. Does your SPI chip driver use"
+		     " CONTROLLER_PAGE_LIMIT?\n");
+		return -1;
+	}
+
+	/*
 	 * Read or write up to databytes bytes at a time until everything has
 	 * been sent.
 	 */
@@ -641,11 +653,11 @@ int spi_xfer(struct spi_slave *slave, const void *dout,
 		/* Wait for Cycle Done Status or Flash Cycle Error. */
 		status = ich_status_poll(SPIS_CDS | SPIS_FCERR, 1);
 		if (status == -1)
-			return 1;
+			return -1;
 
 		if (status & SPIS_FCERR) {
 			puts("ICH SPI: Transaction error\n");
-			return 1;
+			return -1;
 		}
 
 		if (trans.bytesin) {
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 525a3497cd..f8b177a4e3 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -33,7 +33,8 @@
 #ifdef CONFIG_ICH_SPI
 #define CONTROLLER_PAGE_LIMIT	64
 #else
-#define CONTROLLER_PAGE_LIMIT	MAX_INT
+/* any number larger than 4K would do, actually */
+#define CONTROLLER_PAGE_LIMIT	((int)(~0U>>1))
 #endif
 
 struct spi_flash {