[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