From 7ca2ded36434d388b872e548726f590c69c25c92 Mon Sep 17 00:00:00 2001 From: Vadim Bendebury Date: Tue, 13 Sep 2011 22:41:24 -0700 Subject: 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 Reviewed-on: http://gerrit.chromium.org/gerrit/7697 Reviewed-by: Simon Glass Reviewed-by: David Hendricks --- drivers/spi/ich.c | 34 +++++++++++++++++++++++----------- include/spi_flash.h | 3 ++- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c index 166a85fe028..95c594d5398 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,16 +596,29 @@ 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 525a3497cd4..f8b177a4e37 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 { -- cgit v1.2.3