[coreboot] [PATCH] flashrom: Check return codes of spi_write_enable
Ali Nadalizadeh
nadalizadeh at gmail.com
Thu May 7 14:37:38 CEST 2009
Erasing Confirmed working on my hardware :
chip-set : "Intel ICH7M"
chip : "SST SST25VF080B"
Output image is now completely full of 0xFF (s)
Regards --
Ali Nadalizadeh
On Thu, May 7, 2009 at 4:48 PM, Carl-Daniel Hailfinger <
c-d.hailfinger.devel.2006 at gmx.net> wrote:
> On 07.05.2009 13:44, Carl-Daniel Hailfinger wrote:
> > Until the ICH SPI driver can handle preopcodes as standalone opcodes, we
> > should handle such special opcode failure gracefully on ICH and
> > compatible chipsets.
> >
> > This fixes chip erase on almost all ICH+VIA SPI masters.
> >
> > Thanks to Ali Nadalizadeh for helping track down this bug!
> >
>
> Improved version, refactored to isolate the impact to a single function.
>
> As a bonus, all invocations of spi_write_enable() now have error checking.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>
> Index: flashrom-spi_write_enable_error_checking/it87spi.c
> ===================================================================
> --- flashrom-spi_write_enable_error_checking/it87spi.c (Revision 471)
> +++ flashrom-spi_write_enable_error_checking/it87spi.c (Arbeitskopie)
> @@ -196,11 +196,14 @@
> }
>
> /* Page size is usually 256 bytes */
> -static void it8716f_spi_page_program(int block, uint8_t *buf, uint8_t
> *bios)
> +static int it8716f_spi_page_program(int block, uint8_t *buf, uint8_t
> *bios)
> {
> int i;
> + int result;
>
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> OUTB(0x06, it8716f_flashport + 1);
> OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport);
> for (i = 0; i < 256; i++) {
> @@ -212,6 +215,7 @@
> */
> while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> usleep(1000);
> + return 0;
> }
>
> /*
> @@ -222,12 +226,15 @@
> {
> int total_size = 1024 * flash->total_size;
> int i;
> + int result;
>
> fast_spi = 0;
>
> spi_disable_blockprotect();
> for (i = 0; i < total_size; i++) {
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> spi_byte_program(i, buf[i]);
> while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> myusec_delay(10);
> Index: flashrom-spi_write_enable_error_checking/spi.c
> ===================================================================
> --- flashrom-spi_write_enable_error_checking/spi.c (Revision 471)
> +++ flashrom-spi_write_enable_error_checking/spi.c (Arbeitskopie)
> @@ -88,9 +88,24 @@
> int spi_write_enable(void)
> {
> const unsigned char cmd[JEDEC_WREN_OUTSIZE] = { JEDEC_WREN };
> + int result;
>
> /* Send WREN (Write Enable) */
> - return spi_command(sizeof(cmd), 0, cmd, NULL);
> + result = spi_command(sizeof(cmd), 0, cmd, NULL);
> + if (result) {
> + printf_debug("spi_write_enable failed");
> + switch (flashbus) {
> + case BUS_TYPE_ICH7_SPI:
> + case BUS_TYPE_ICH9_SPI:
> + case BUS_TYPE_VIA_SPI:
> + printf_debug(" due to SPI master limitation,
> ignoring"
> + " and hoping it will be run as
> PREOP\n");
> + return 0;
> + default:
> + printf_debug("\n");
> + }
> + }
> + return result;
> }
>
> int spi_write_disable(void)
> @@ -361,10 +376,8 @@
> return result;
> }
> result = spi_write_enable();
> - if (result) {
> - printf_debug("spi_write_enable failed\n");
> + if (result)
> return result;
> - }
> /* Send CE (Chip Erase) */
> result = spi_command(sizeof(cmd), 0, cmd, NULL);
> if (result) {
> @@ -391,10 +404,8 @@
> return result;
> }
> result = spi_write_enable();
> - if (result) {
> - printf_debug("spi_write_enable failed\n");
> + if (result)
> return result;
> - }
> /* Send CE (Chip Erase) */
> result = spi_command(sizeof(cmd), 0, cmd, NULL);
> if (result) {
> @@ -424,11 +435,14 @@
> int spi_block_erase_52(const struct flashchip *flash, unsigned long addr)
> {
> unsigned char cmd[JEDEC_BE_52_OUTSIZE] = {JEDEC_BE_52};
> + int result;
>
> cmd[1] = (addr & 0x00ff0000) >> 16;
> cmd[2] = (addr & 0x0000ff00) >> 8;
> cmd[3] = (addr & 0x000000ff);
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> /* Send BE (Block Erase) */
> spi_command(sizeof(cmd), 0, cmd, NULL);
> /* Wait until the Write-In-Progress bit is cleared.
> @@ -447,11 +461,14 @@
> int spi_block_erase_d8(const struct flashchip *flash, unsigned long addr)
> {
> unsigned char cmd[JEDEC_BE_D8_OUTSIZE] = { JEDEC_BE_D8 };
> + int result;
>
> cmd[1] = (addr & 0x00ff0000) >> 16;
> cmd[2] = (addr & 0x0000ff00) >> 8;
> cmd[3] = (addr & 0x000000ff);
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> /* Send BE (Block Erase) */
> spi_command(sizeof(cmd), 0, cmd, NULL);
> /* Wait until the Write-In-Progress bit is cleared.
> @@ -489,11 +506,15 @@
> int spi_sector_erase(const struct flashchip *flash, unsigned long addr)
> {
> unsigned char cmd[JEDEC_SE_OUTSIZE] = { JEDEC_SE };
> + int result;
> +
> cmd[1] = (addr & 0x00ff0000) >> 16;
> cmd[2] = (addr & 0x0000ff00) >> 8;
> cmd[3] = (addr & 0x000000ff);
>
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> /* Send SE (Sector Erase) */
> spi_command(sizeof(cmd), 0, cmd, NULL);
> /* Wait until the Write-In-Progress bit is cleared.
> @@ -623,6 +644,8 @@
> {
> uint32_t pos = 2, size = flash->total_size * 1024;
> unsigned char w[6] = {0xad, 0, 0, 0, buf[0], buf[1]};
> + int result;
> +
> switch (flashbus) {
> case BUS_TYPE_WBSIO_SPI:
> fprintf(stderr, "%s: impossible with Winbond SPI masters,"
> @@ -632,7 +655,9 @@
> break;
> }
> flash->erase(flash);
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> spi_command(6, 0, w, NULL);
> while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> myusec_delay(5); /* SST25VF040B Tbp is max 10us */
> Index: flashrom-spi_write_enable_error_checking/wbsio_spi.c
> ===================================================================
> --- flashrom-spi_write_enable_error_checking/wbsio_spi.c (Revision
> 471)
> +++ flashrom-spi_write_enable_error_checking/wbsio_spi.c
> (Arbeitskopie)
> @@ -189,6 +189,7 @@
> int wbsio_spi_write(struct flashchip *flash, uint8_t *buf)
> {
> int pos, size = flash->total_size * 1024;
> + int result;
>
> if (flash->total_size > 1024) {
> fprintf(stderr, "%s: Winbond saved on 4 register bits so max
> chip size is 1024 KB!\n", __func__);
> @@ -196,7 +197,9 @@
> }
>
> flash->erase(flash);
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> for (pos = 0; pos < size; pos++) {
> spi_byte_program(pos, buf[pos]);
> while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> Index: flashrom-spi_write_enable_error_checking/sb600spi.c
> ===================================================================
> --- flashrom-spi_write_enable_error_checking/sb600spi.c (Revision 471)
> +++ flashrom-spi_write_enable_error_checking/sb600spi.c (Arbeitskopie)
> @@ -68,6 +68,7 @@
> {
> int rc = 0, i;
> int total_size = flash->total_size * 1024;
> + int result;
>
> /* Erase first */
> printf("Erasing flash before programming... ");
> @@ -77,7 +78,9 @@
> printf("Programming flash");
> for (i = 0; i < total_size; i++, buf++) {
> spi_disable_blockprotect();
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> spi_byte_program(i, *buf);
> /* wait program complete. */
> if (i % 0x8000 == 0)
>
>
> --
> http://www.hailfinger.org/
>
>
> Index: flashrom-spi_write_enable_error_checking/it87spi.c
> ===================================================================
> --- flashrom-spi_write_enable_error_checking/it87spi.c (Revision 471)
> +++ flashrom-spi_write_enable_error_checking/it87spi.c (Arbeitskopie)
> @@ -196,11 +196,14 @@
> }
>
> /* Page size is usually 256 bytes */
> -static void it8716f_spi_page_program(int block, uint8_t *buf, uint8_t
> *bios)
> +static int it8716f_spi_page_program(int block, uint8_t *buf, uint8_t
> *bios)
> {
> int i;
> + int result;
>
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> OUTB(0x06, it8716f_flashport + 1);
> OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport);
> for (i = 0; i < 256; i++) {
> @@ -212,6 +215,7 @@
> */
> while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> usleep(1000);
> + return 0;
> }
>
> /*
> @@ -222,12 +226,15 @@
> {
> int total_size = 1024 * flash->total_size;
> int i;
> + int result;
>
> fast_spi = 0;
>
> spi_disable_blockprotect();
> for (i = 0; i < total_size; i++) {
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> spi_byte_program(i, buf[i]);
> while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> myusec_delay(10);
> Index: flashrom-spi_write_enable_error_checking/spi.c
> ===================================================================
> --- flashrom-spi_write_enable_error_checking/spi.c (Revision 471)
> +++ flashrom-spi_write_enable_error_checking/spi.c (Arbeitskopie)
> @@ -88,9 +88,24 @@
> int spi_write_enable(void)
> {
> const unsigned char cmd[JEDEC_WREN_OUTSIZE] = { JEDEC_WREN };
> + int result;
>
> /* Send WREN (Write Enable) */
> - return spi_command(sizeof(cmd), 0, cmd, NULL);
> + result = spi_command(sizeof(cmd), 0, cmd, NULL);
> + if (result) {
> + printf_debug("spi_write_enable failed");
> + switch (flashbus) {
> + case BUS_TYPE_ICH7_SPI:
> + case BUS_TYPE_ICH9_SPI:
> + case BUS_TYPE_VIA_SPI:
> + printf_debug(" due to SPI master limitation,
> ignoring"
> + " and hoping it will be run as
> PREOP\n");
> + return 0;
> + default:
> + printf_debug("\n");
> + }
> + }
> + return result;
> }
>
> int spi_write_disable(void)
> @@ -361,10 +376,8 @@
> return result;
> }
> result = spi_write_enable();
> - if (result) {
> - printf_debug("spi_write_enable failed\n");
> + if (result)
> return result;
> - }
> /* Send CE (Chip Erase) */
> result = spi_command(sizeof(cmd), 0, cmd, NULL);
> if (result) {
> @@ -391,10 +404,8 @@
> return result;
> }
> result = spi_write_enable();
> - if (result) {
> - printf_debug("spi_write_enable failed\n");
> + if (result)
> return result;
> - }
> /* Send CE (Chip Erase) */
> result = spi_command(sizeof(cmd), 0, cmd, NULL);
> if (result) {
> @@ -424,11 +435,14 @@
> int spi_block_erase_52(const struct flashchip *flash, unsigned long addr)
> {
> unsigned char cmd[JEDEC_BE_52_OUTSIZE] = {JEDEC_BE_52};
> + int result;
>
> cmd[1] = (addr & 0x00ff0000) >> 16;
> cmd[2] = (addr & 0x0000ff00) >> 8;
> cmd[3] = (addr & 0x000000ff);
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> /* Send BE (Block Erase) */
> spi_command(sizeof(cmd), 0, cmd, NULL);
> /* Wait until the Write-In-Progress bit is cleared.
> @@ -447,11 +461,14 @@
> int spi_block_erase_d8(const struct flashchip *flash, unsigned long addr)
> {
> unsigned char cmd[JEDEC_BE_D8_OUTSIZE] = { JEDEC_BE_D8 };
> + int result;
>
> cmd[1] = (addr & 0x00ff0000) >> 16;
> cmd[2] = (addr & 0x0000ff00) >> 8;
> cmd[3] = (addr & 0x000000ff);
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> /* Send BE (Block Erase) */
> spi_command(sizeof(cmd), 0, cmd, NULL);
> /* Wait until the Write-In-Progress bit is cleared.
> @@ -489,11 +506,15 @@
> int spi_sector_erase(const struct flashchip *flash, unsigned long addr)
> {
> unsigned char cmd[JEDEC_SE_OUTSIZE] = { JEDEC_SE };
> + int result;
> +
> cmd[1] = (addr & 0x00ff0000) >> 16;
> cmd[2] = (addr & 0x0000ff00) >> 8;
> cmd[3] = (addr & 0x000000ff);
>
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> /* Send SE (Sector Erase) */
> spi_command(sizeof(cmd), 0, cmd, NULL);
> /* Wait until the Write-In-Progress bit is cleared.
> @@ -623,6 +644,8 @@
> {
> uint32_t pos = 2, size = flash->total_size * 1024;
> unsigned char w[6] = {0xad, 0, 0, 0, buf[0], buf[1]};
> + int result;
> +
> switch (flashbus) {
> case BUS_TYPE_WBSIO_SPI:
> fprintf(stderr, "%s: impossible with Winbond SPI masters,"
> @@ -632,7 +655,9 @@
> break;
> }
> flash->erase(flash);
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> spi_command(6, 0, w, NULL);
> while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> myusec_delay(5); /* SST25VF040B Tbp is max 10us */
> Index: flashrom-spi_write_enable_error_checking/wbsio_spi.c
> ===================================================================
> --- flashrom-spi_write_enable_error_checking/wbsio_spi.c (Revision
> 471)
> +++ flashrom-spi_write_enable_error_checking/wbsio_spi.c
> (Arbeitskopie)
> @@ -189,6 +189,7 @@
> int wbsio_spi_write(struct flashchip *flash, uint8_t *buf)
> {
> int pos, size = flash->total_size * 1024;
> + int result;
>
> if (flash->total_size > 1024) {
> fprintf(stderr, "%s: Winbond saved on 4 register bits so max
> chip size is 1024 KB!\n", __func__);
> @@ -196,7 +197,9 @@
> }
>
> flash->erase(flash);
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> for (pos = 0; pos < size; pos++) {
> spi_byte_program(pos, buf[pos]);
> while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> Index: flashrom-spi_write_enable_error_checking/sb600spi.c
> ===================================================================
> --- flashrom-spi_write_enable_error_checking/sb600spi.c (Revision 471)
> +++ flashrom-spi_write_enable_error_checking/sb600spi.c (Arbeitskopie)
> @@ -68,6 +68,7 @@
> {
> int rc = 0, i;
> int total_size = flash->total_size * 1024;
> + int result;
>
> /* Erase first */
> printf("Erasing flash before programming... ");
> @@ -77,7 +78,9 @@
> printf("Programming flash");
> for (i = 0; i < total_size; i++, buf++) {
> spi_disable_blockprotect();
> - spi_write_enable();
> + result = spi_write_enable();
> + if (result)
> + return result;
> spi_byte_program(i, *buf);
> /* wait program complete. */
> if (i % 0x8000 == 0)
>
> --
> coreboot mailing list: coreboot at coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20090507/439a2f44/attachment.html>
More information about the coreboot
mailing list