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 > >> <[email protected]> > > > > 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 <[email protected]> > > --- > 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 " > "[email protected]\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 [email protected]\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)) { _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
