[flashrom] [PATCH] Switch all chips to partial write
Warren Turkal
wt at penguintechs.org
Wed Oct 13 20:13:26 CEST 2010
Do you have an updated patch, by chance? This one doesn't apply cleanly
anymore.
Thanks,
wt
On Monday, October 11, 2010 03:58:21 pm Carl-Daniel Hailfinger wrote:
> 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>
>
> ---
> flashrom-partial_write_inner_function_cleanup_annotate/82802ab.c 2010-10-1
> 0 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-0
> 8 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-1
> 0-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)) {
More information about the flashrom
mailing list