[flashrom] [PATCH] Switch all chips to partial write

Idwer Vollering vidwer at gmail.com
Tue Oct 12 15:33:09 CEST 2010


2010/10/12 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

> On 11.10.2010 15:14, Maciej Pijanka wrote:
> > On Mon, 11 Oct 2010, Carl-Daniel Hailfinger wrote:
> >
> >
> >> Switch all flash chips to partial write.
> >> The inner write functions which handle partial write are renamed to the
> >> original name of their wrappers. The write wrappers are removed.
> >>
> >> This patch depends on
> >> Re: [flashrom] [PATCH] Refactor remaining write wrappers
> >> Tests are appreciated.
> >>
> >> Signed-off-by: Carl-Daniel Hailfinger <
> c-d.hailfinger.devel.2006 at gmx.net>
> >>
> >
> >
> > This patch causes compile time problems in spi code, some functions were
> renamed
> > but *write_1_new() calls in few places remain (spi.c, it87spi.c)
> >
>
> Thanks for the report. Those are now fixed.
>
>
> > After manual fixing of those test were performed using nic3com and
> at29c512.
> > logs in attachment, everything went ok and image read back is correct.
> >
>
> Updated version with fixed compilation and some "interesting" (i.e. hard
> to know if they work) SPI changes. Write tests of SPI chips are
> appreciated, especially on boards which use the ITE IT87* Super I/O as
> flash controller.
>
> Switch all flash chips to partial write.
> The inner write functions which handle partial write are renamed to the
> original name of their wrappers. The write wrappers are removed.
>
> This patch depends on
> Re: [flashrom] [PATCH] Refactor remaining write wrappers
> Tests are appreciated.
>
> Please review this patch with lots of scrutiny. I don't really feel
> comfortable with the IT87* SPI changes nor with the AAI SPI changes,
> but they were needed to get the code to work, at least in theory.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>

Acked-by: Idwer Vollering <vidwer at gmail.com>


>
> --- flashrom-partial_write_inner_function_cleanup_annotate/82802ab.c
>  2010-10-10 22:33:45.000000000 +0200
> +++ flashrom-partial_write_switchover/82802ab.c 2010-10-11
> 14:16:48.000000000 +0200
> @@ -144,7 +144,8 @@
>        return 0;
>  }
>
> -int write_page_82802ab(struct flashchip *flash, uint8_t *src, int start,
> int len)
> +/* chunksize is 1 */
> +int write_82802ab(struct flashchip *flash, uint8_t *src, int start, int
> len)
>  {
>        int i;
>        chipaddr dst = flash->virtual_memory + start;
> @@ -160,12 +161,6 @@
>        return 0;
>  }
>
> -/* chunksize is 1 */
> -int write_82802ab(struct flashchip *flash, uint8_t *buf)
> -{
> -       return write_page_82802ab(flash, buf, 0, flash->total_size * 1024);
> -}
> -
>  int unlock_28f004s5(struct flashchip *flash)
>  {
>        chipaddr bios = flash->virtual_memory;
> --- flashrom-partial_write_inner_function_cleanup_annotate/chipdrivers.h
>      2010-10-10 22:25:29.000000000 +0200
> +++ flashrom-partial_write_switchover/chipdrivers.h     2010-10-11
> 14:19:04.000000000 +0200
> @@ -39,10 +39,8 @@
>  int spi_block_erase_d8(struct flashchip *flash, unsigned int addr,
> unsigned int blocklen);
>  int spi_block_erase_60(struct flashchip *flash, unsigned int addr,
> unsigned int blocklen);
>  int spi_block_erase_c7(struct flashchip *flash, unsigned int addr,
> unsigned int blocklen);
> -int spi_chip_write_1(struct flashchip *flash, uint8_t *buf);
> -int spi_chip_write_256(struct flashchip *flash, uint8_t *buf);
> -int spi_chip_write_1_new(struct flashchip *flash, uint8_t *buf, int start,
> int len);
> -int spi_chip_write_256_new(struct flashchip *flash, uint8_t *buf, int
> start, int len);
> +int spi_chip_write_1(struct flashchip *flash, uint8_t *buf, int start, int
> len);
> +int spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start,
> int len);
>  int spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int
> len);
>  uint8_t spi_read_status_register(void);
>  int spi_prettyprint_status_register_at25df(struct flashchip *flash);
> @@ -61,16 +59,14 @@
>  int spi_nbyte_read(int addr, uint8_t *bytes, int len);
>  int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int start, int
> len, int chunksize);
>  int spi_write_chunked(struct flashchip *flash, uint8_t *buf, int start,
> int len, int chunksize);
> -int spi_aai_write_new(struct flashchip *flash, uint8_t *buf, int start,
> int len);
> -int spi_aai_write(struct flashchip *flash, uint8_t *buf);
> +int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int
> len);
>
>  /* 82802ab.c */
>  uint8_t wait_82802ab(struct flashchip *flash);
>  int probe_82802ab(struct flashchip *flash);
>  int erase_block_82802ab(struct flashchip *flash, unsigned int page,
> unsigned int pagesize);
> -int write_82802ab(struct flashchip *flash, uint8_t *buf);
> +int write_82802ab(struct flashchip *flash, uint8_t *buf, int start, int
> len);
>  void print_status_82802ab(uint8_t status);
> -int write_page_82802ab(struct flashchip *flash, uint8_t *src, int start,
> int len);
>  int unlock_82802ab(struct flashchip *flash);
>  int unlock_28f004s5(struct flashchip *flash);
>
> @@ -81,21 +77,18 @@
>  int write_byte_program_jedec(chipaddr bios, uint8_t *src,
>                             chipaddr dst);
>  int probe_jedec(struct flashchip *flash);
> -int write_jedec(struct flashchip *flash, uint8_t *buf);
> -int write_jedec_1(struct flashchip *flash, uint8_t *buf);
> +int write_jedec(struct flashchip *flash, uint8_t *buf, int start, int
> len);
> +int write_jedec_1(struct flashchip *flash, uint8_t *buf, int start, int
> len);
>  int erase_sector_jedec(struct flashchip *flash, unsigned int page,
> unsigned int pagesize);
>  int erase_block_jedec(struct flashchip *flash, unsigned int page, unsigned
> int blocksize);
>  int erase_chip_block_jedec(struct flashchip *flash, unsigned int page,
> unsigned int blocksize);
> -int write_sector_jedec_common(struct flashchip *flash, uint8_t *src, int
> start, int len);
> -int write_page_write_jedec_common(struct flashchip *flash, uint8_t *src,
> int start, int page_size);
>
>  /* m29f400bt.c */
>  int probe_m29f400bt(struct flashchip *flash);
>  int block_erase_m29f400bt(struct flashchip *flash, unsigned int start,
> unsigned int len);
>  int block_erase_chip_m29f400bt(struct flashchip *flash, unsigned int
> start, unsigned int len);
> -int write_m29f400bt(struct flashchip *flash, uint8_t *buf);
> +int write_m29f400bt(struct flashchip *flash, uint8_t *buf, int start, int
> len);
>  void protect_m29f400bt(chipaddr bios);
> -int write_page_m29f400bt(struct flashchip *flash, uint8_t *src, int start,
> int len);
>
>  /* pm49fl00x.c */
>  int unlock_49fl00x(struct flashchip *flash);
> @@ -104,8 +97,7 @@
>  /* sst28sf040.c */
>  int erase_chip_28sf040(struct flashchip *flash, unsigned int addr,
> unsigned int blocklen);
>  int erase_sector_28sf040(struct flashchip *flash, unsigned int address,
> unsigned int sector_size);
> -int write_28sf040(struct flashchip *flash, uint8_t *buf);
> -int write_sector_28sf040(struct flashchip *flash, uint8_t *src, int start,
> int len);
> +int write_28sf040(struct flashchip *flash, uint8_t *buf, int start, int
> len);
>  int unprotect_28sf040(struct flashchip *flash);
>  int protect_28sf040(struct flashchip *flash);
>
> --- flashrom-partial_write_inner_function_cleanup_annotate/flash.h
>  2010-10-10 18:30:10.000000000 +0200
> +++ flashrom-partial_write_switchover/flash.h   2010-10-11
> 14:15:07.000000000 +0200
> @@ -135,7 +135,7 @@
>
>        int (*printlock) (struct flashchip *flash);
>        int (*unlock) (struct flashchip *flash);
> -       int (*write) (struct flashchip *flash, uint8_t *buf);
> +       int (*write) (struct flashchip *flash, uint8_t *buf, int start, int
> len);
>        int (*read) (struct flashchip *flash, uint8_t *buf, int start, int
> len);
>
>        /* Some flash devices have an additional register space. */
> --- flashrom-partial_write_inner_function_cleanup_annotate/flashrom.c
> 2010-10-08 23:34:41.000000000 +0200
> +++ flashrom-partial_write_switchover/flashrom.c        2010-10-11
> 14:24:06.000000000 +0200
> @@ -1639,7 +1639,7 @@
>                        return 1;
>                }
>                msg_cinfo("Writing flash chip... ");
> -               ret = flash->write(flash, buf);
> +               ret = flash->write(flash, buf, 0, flash->total_size *
> 1024);
>                if (ret) {
>                        msg_cerr("FAILED!\n");
>                        emergency_help_message();
> --- flashrom-partial_write_inner_function_cleanup_annotate/it87spi.c
>  2010-10-08 23:34:41.000000000 +0200
> +++ flashrom-partial_write_switchover/it87spi.c 2010-10-12
> 00:36:12.000000000 +0200
> @@ -305,7 +305,7 @@
>        /* FIXME: The command below seems to be redundant or wrong. */
>        OUTB(0x06, it8716f_flashport + 1);
>        OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport);
> -       for (i = 0; i < 256; i++) {
> +       for (i = 0; i < flash->page_size; i++) {
>                chip_writeb(buf[i], bios + start + i);
>        }
>        OUTB(0, it8716f_flashport);
> @@ -339,34 +339,36 @@
>  {
>        /*
>         * IT8716F only allows maximum of 512 kb SPI chip size for memory
> -        * mapped access.
> +        * mapped access. It also can't write more than 1+3+256 bytes at
> once.
>         */
> -       if ((programmer == PROGRAMMER_IT87SPI) || (flash->total_size * 1024
> > 512 * 1024)) {
> -               spi_chip_write_1_new(flash, buf, start, len);
> +       if ((programmer == PROGRAMMER_IT87SPI) ||
> +           (flash->total_size * 1024 > 512 * 1024) ||
> +           (flash->page_size > 256)) {
> +               spi_chip_write_1(flash, buf, start, len);
>        } else {
>                int lenhere;
>
> -               if (start % 256) {
> +               if (start % flash->page_size) {
>                        /* start to the end of the page or start + len,
>                         * whichever is smaller. Page length is hardcoded to
>                         * 256 bytes (IT87 SPI hardware limitation).
>                         */
> -                       lenhere = min(len, (start | 0xff) - start + 1);
> -                       spi_chip_write_1_new(flash, buf, start, lenhere);
> +                       lenhere = min(len, flash->page_size - start %
> flash->page_size);
> +                       spi_chip_write_1(flash, buf, start, lenhere);
>                        start += lenhere;
>                        len -= lenhere;
>                        buf += lenhere;
>                }
>
>                /* FIXME: Handle chips which have max writechunk size >1 and
> <256. */
> -               while (len >= 256) {
> +               while (len >= flash->page_size) {
>                        it8716f_spi_page_program(flash, buf, start);
> -                       start += 256;
> -                       len -= 256;
> -                       buf += 256;
> +                       start += flash->page_size;
> +                       len -= flash->page_size;
> +                       buf += flash->page_size;
>                }
>                if (len)
> -                       spi_chip_write_1_new(flash, buf, start, len);
> +                       spi_chip_write_1(flash, buf, start, len);
>        }
>
>        return 0;
> --- flashrom-partial_write_inner_function_cleanup_annotate/jedec.c
>  2010-10-11 14:13:31.000000000 +0200
> +++ flashrom-partial_write_switchover/jedec.c   2010-10-11
> 14:16:16.000000000 +0200
> @@ -336,7 +336,8 @@
>        return failed;
>  }
>
> -int write_sector_jedec_common(struct flashchip *flash, uint8_t *src, int
> start, int len)
> +/* chunksize is 1 */
> +int write_jedec_1(struct flashchip *flash, uint8_t *src, int start, int
> len)
>  {
>        int i, failed = 0;
>        chipaddr dst = flash->virtual_memory + start;
> @@ -398,13 +399,14 @@
>        return failed;
>  }
>
> +/* chunksize is page_size */
>  /*
>  * Write a part of the flash chip.
>  * FIXME: Use the chunk code from Michael Karcher instead.
>  * This function is a slightly modified copy of spi_write_chunked.
>  * Each page is written separately in chunks with a maximum size of
> chunksize.
>  */
> -int write_jedec_pages(struct flashchip *flash, uint8_t *buf, int start,
> int len)
> +int write_jedec(struct flashchip *flash, uint8_t *buf, int start, int len)
>  {
>        int i, starthere, lenhere;
>        /* FIXME: page_size is the wrong variable. We need
> max_writechunk_size
> @@ -437,18 +439,6 @@
>        return 0;
>  }
>
> -/* chunksize is page_size */
> -int write_jedec(struct flashchip *flash, uint8_t *buf)
> -{
> -       return write_jedec_pages(flash, buf, 0, flash->total_size * 1024);
> -}
> -
> -/* chunksize is 1 */
> -int write_jedec_1(struct flashchip *flash, uint8_t * buf)
> -{
> -       return write_sector_jedec_common(flash, buf, 0, flash->total_size *
> 1024);
> -}
> -
>  /* erase chip with block_erase() prototype */
>  int erase_chip_block_jedec(struct flashchip *flash, unsigned int addr,
>                           unsigned int blocksize)
> --- flashrom-partial_write_inner_function_cleanup_annotate/m29f400bt.c
>  2010-10-10 22:33:58.000000000 +0200
> +++ flashrom-partial_write_switchover/m29f400bt.c       2010-10-11
> 14:19:52.000000000 +0200
> @@ -27,7 +27,8 @@
>    0x555 instead of 0x2AA. Do *not* blindly replace with standard JEDEC
>    functions. */
>
> -int write_page_m29f400bt(struct flashchip *flash, uint8_t *src, int start,
> int len)
> +/* chunksize is 1 */
> +int write_m29f400bt(struct flashchip *flash, uint8_t *src, int start, int
> len)
>  {
>        int i;
>        chipaddr bios = flash->virtual_memory;
> @@ -139,9 +140,3 @@
>        }
>        return erase_m29f400bt(flash);
>  }
> -
> -/* chunksize is 1 */
> -int write_m29f400bt(struct flashchip *flash, uint8_t *buf)
> -{
> -       return write_page_m29f400bt(flash, buf, 0, flash->total_size *
> 1024);
> -}
> --- flashrom-partial_write_inner_function_cleanup_annotate/spi25.c
>  2010-10-08 23:34:42.000000000 +0200
> +++ flashrom-partial_write_switchover/spi25.c   2010-10-12
> 00:49:56.000000000 +0200
> @@ -1304,7 +1304,7 @@
>  * (e.g. due to size constraints in IT87* for over 512 kB)
>  */
>  /* real chunksize is 1, logical chunksize is 1 */
> -int spi_chip_write_1_new(struct flashchip *flash, uint8_t *buf, int start,
> int len)
> +int spi_chip_write_1(struct flashchip *flash, uint8_t *buf, int start, int
> len)
>  {
>        int i, result = 0;
>
> @@ -1319,12 +1319,7 @@
>        return 0;
>  }
>
> -int spi_chip_write_1(struct flashchip *flash, uint8_t *buf)
> -{
> -       return spi_chip_write_1_new(flash, buf, 0, flash->total_size *
> 1024);
> -}
> -
> -int spi_aai_write_new(struct flashchip *flash, uint8_t *buf, int start,
> int len)
> +int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int
> len)
>  {
>        uint32_t pos = start;
>        int result;
> @@ -1363,7 +1358,7 @@
>        case SPI_CONTROLLER_WBSIO:
>                msg_perr("%s: impossible with this SPI controller,"
>                                " degrading to byte program\n", __func__);
> -               return spi_chip_write_1_new(flash, buf, start, len);
> +               return spi_chip_write_1(flash, buf, start, len);
>  #endif
>  #endif
>        default:
> @@ -1373,18 +1368,24 @@
>        /* The even start address and even length requirements can be either
>         * honored outside this function, or we can call spi_byte_program
>         * for the first and/or last byte and use AAI for the rest.
> +        * FIXME: Move this to generic code.
>         */
>        /* The data sheet requires a start address with the low bit cleared.
> */
>        if (start % 2) {
>                msg_cerr("%s: start address not even! Please report a bug at
> "
>                         "flashrom at flashrom.org\n", __func__);
> -               return SPI_GENERIC_ERROR;
> +               if (spi_chip_write_1(flash, buf, start, start % 2))
> +                       return SPI_GENERIC_ERROR;
> +               pos += start % 2;
> +               /* Do not return an error for now. */
> +               //return SPI_GENERIC_ERROR;
>        }
>        /* The data sheet requires total AAI write length to be even. */
>        if (len % 2) {
>                msg_cerr("%s: total write length not even! Please report a "
>                         "bug at flashrom at flashrom.org\n", __func__);
> -               return SPI_GENERIC_ERROR;
> +               /* Do not return an error for now. */
> +               //return SPI_GENERIC_ERROR;
>        }
>
>
> @@ -1403,7 +1404,8 @@
>        /* We already wrote 2 bytes in the multicommand step. */
>        pos += 2;
>
> -       while (pos < start + len) {
> +       /* Are there at least two more bytes to write? */
> +       while (pos < start + len - 1) {
>                cmd[1] = buf[pos++];
>                cmd[2] = buf[pos++];
>                spi_send_command(JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE, 0,
> cmd, NULL);
> @@ -1411,13 +1413,17 @@
>                        programmer_delay(10);
>        }
>
> -       /* Use WRDI to exit AAI mode. */
> +       /* Use WRDI to exit AAI mode. This needs to be done before issuing
> any
> +        * other non-AAI command.
> +        */
>         spi_write_disable();
> -       return 0;
> -}
>
> -int spi_aai_write(struct flashchip *flash, uint8_t *buf)
> -{
> -       return spi_aai_write_new(flash, buf, 0, flash->total_size * 1024);
> -}
> +       /* Write remaining byte (if any). */
> +       if (pos < start + len) {
> +               if (spi_chip_write_1(flash, buf + pos, pos, pos % 2))
> +                       return SPI_GENERIC_ERROR;
> +               pos += pos % 2;
> +       }
>
> +       return 0;
> +}
> --- flashrom-partial_write_inner_function_cleanup_annotate/spi.c
>  2010-10-08 23:34:42.000000000 +0200
> +++ flashrom-partial_write_switchover/spi.c     2010-10-11
> 20:30:50.000000000 +0200
> @@ -80,7 +80,7 @@
>                .command = wbsio_spi_send_command,
>                .multicommand = default_spi_send_multicommand,
>                .read = wbsio_spi_read,
> -               .write_256 = spi_chip_write_1_new,
> +               .write_256 = spi_chip_write_1,
>        },
>
>        { /* SPI_CONTROLLER_MCP6X_BITBANG */
> @@ -124,7 +124,7 @@
>                .command = dediprog_spi_send_command,
>                .multicommand = default_spi_send_multicommand,
>                .read = dediprog_spi_read,
> -               .write_256 = spi_chip_write_1_new,
> +               .write_256 = spi_chip_write_1,
>        },
>  #endif
>
> @@ -245,7 +245,7 @@
>  * .write_256 = spi_chip_write_1
>  */
>  /* real chunksize is up to 256, logical chunksize is 256 */
> -int spi_chip_write_256_new(struct flashchip *flash, uint8_t *buf, int
> start, int len)
> +int spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start,
> int len)
>  {
>        if (!spi_programmer[spi_controller].write_256) {
>                msg_perr("%s called, but SPI page write is unsupported on
> this "
> @@ -257,20 +257,6 @@
>        return spi_programmer[spi_controller].write_256(flash, buf, start,
> len);
>  }
>
> -/* Wrapper function until the generic code is converted to partial writes.
> */
> -int spi_chip_write_256(struct flashchip *flash, uint8_t *buf)
> -{
> -       int ret;
> -
> -       msg_pinfo("Programming flash... ");
> -       ret = spi_chip_write_256_new(flash, buf, 0, flash->total_size *
> 1024);
> -       if (!ret)
> -               msg_pinfo("done.\n");
> -       else
> -               msg_pinfo("\n");
> -       return ret;
> -}
> -
>  /*
>  * Get the lowest allowed address for read accesses. This often happens to
>  * be the lowest allowed address for all commands which take an address.
> --- flashrom-partial_write_inner_function_cleanup_annotate/sst28sf040.c
> 2010-10-10 22:40:48.000000000 +0200
> +++ flashrom-partial_write_switchover/sst28sf040.c      2010-10-11
> 14:20:24.000000000 +0200
> @@ -78,7 +78,8 @@
>        return 0;
>  }
>
> -int write_sector_28sf040(struct flashchip *flash, uint8_t *src, int start,
> int len)
> +/* chunksize is 1 */
> +int write_28sf040(struct flashchip *flash, uint8_t *src, int start, int
> len)
>  {
>        int i;
>        chipaddr bios = flash->virtual_memory;
> @@ -119,12 +120,6 @@
>        return 0;
>  }
>
> -/* chunksize is 1 */
> -int write_28sf040(struct flashchip *flash, uint8_t *buf)
> -{
> -       return write_sector_28sf040(flash, buf, 0, flash->total_size *
> 1024);
> -}
> -
>  int erase_chip_28sf040(struct flashchip *flash, unsigned int addr,
> unsigned int blocklen)
>  {
>        if ((addr != 0) || (blocklen != flash->total_size * 1024)) {
>
>
> --
> http://www.hailfinger.org/
>
>
> _______________________________________________
> flashrom mailing list
> flashrom at flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20101012/1cba27d4/attachment.html>


More information about the flashrom mailing list