Convert SPI chips to partial write, but wrap the write functions in a compat layer to allow converting the rest of flashrom later. I actually have patches for most of the remaining conversion, but I wanted to get this out and reviewed first.
Compile tested, but that's it.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-partial_write_spi_intermediate/flash.h =================================================================== --- flashrom-partial_write_spi_intermediate/flash.h (Revision 1072) +++ flashrom-partial_write_spi_intermediate/flash.h (Arbeitskopie) @@ -528,7 +528,7 @@ int ft2232_spi_init(void); int ft2232_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int ft2232_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf); +int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* bitbang_spi.c */ extern int bitbang_spi_half_period; @@ -536,7 +536,7 @@ int bitbang_spi_init(void); int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int bitbang_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf); +int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* buspirate_spi.c */ struct buspirate_spispeeds { @@ -547,7 +547,7 @@ int buspirate_spi_shutdown(void); int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int buspirate_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf); +int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* dediprog.c */ int dediprog_init(void); @@ -667,7 +667,7 @@
/* Optimized functions for this programmer */ int (*read)(struct flashchip *flash, uint8_t *buf, int start, int len); - int (*write_256)(struct flashchip *flash, uint8_t *buf); + int (*write_256)(struct flashchip *flash, uint8_t *buf, int start, int len); };
extern enum spi_controller spi_controller; @@ -688,7 +688,7 @@ int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int ich_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int ich_spi_write_256(struct flashchip *flash, uint8_t * buf); +int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len); int ich_spi_send_multicommand(struct spi_command *cmds);
/* it87spi.c */ @@ -701,13 +701,13 @@ int it8716f_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf); +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* sb600spi.c */ int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int sb600_spi_write_1(struct flashchip *flash, uint8_t *buf); +int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len); extern uint8_t *sb600_spibar;
/* wbsio_spi.c */ @@ -715,7 +715,7 @@ int wbsio_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int wbsio_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf); +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len);
/* serprog.c */ int serprog_init(void); Index: flashrom-partial_write_spi_intermediate/spi25.c =================================================================== --- flashrom-partial_write_spi_intermediate/spi25.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/spi25.c (Arbeitskopie) @@ -874,6 +874,7 @@
/* * Read a part of the flash chip. + * FIXME: Use the chunk code from Michael Karcher instead. * Each page is read separately in chunks with a maximum size of chunksize. */ int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize) @@ -913,6 +914,7 @@
/* * Write a part of the flash chip. + * FIXME: Use the chunk code from Michael Karcher instead. * Each page is written separately in chunks with a maximum size of chunksize. */ int spi_write_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize) @@ -963,20 +965,13 @@ * and for chips where memory mapped programming is impossible * (e.g. due to size constraints in IT87* for over 512 kB) */ -int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) +/* 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 total_size = 1024 * flash->total_size; int i, result = 0;
spi_disable_blockprotect(); - /* Erase first */ - msg_cinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_cerr("ERASE FAILED!\n"); - return -1; - } - msg_cinfo("done.\n"); - for (i = 0; i < total_size; i++) { + for (i = start; i < start + len; i++) { result = spi_byte_program(i, buf[i]); if (result) return 1; @@ -987,11 +982,23 @@ return 0; }
-int spi_aai_write(struct flashchip *flash, uint8_t *buf) +int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) { - uint32_t addr = 0; - uint32_t len = flash->total_size * 1024; - uint32_t pos = addr; + spi_disable_blockprotect(); + /* Erase first */ + msg_cinfo("Erasing flash before programming... "); + if (erase_flash(flash)) { + msg_cerr("ERASE FAILED!\n"); + return -1; + } + msg_cinfo("done.\n"); + + return spi_chip_write_1_new(flash, buf, 0, flash->total_size * 1024); +} + +int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int len) +{ + uint32_t pos = start; int result; unsigned char cmd[JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE] = { JEDEC_AAI_WORD_PROGRAM, @@ -1006,9 +1013,9 @@ .writecnt = JEDEC_AAI_WORD_PROGRAM_OUTSIZE, .writearr = (const unsigned char[]){ JEDEC_AAI_WORD_PROGRAM, - (pos >> 16) & 0xff, - (pos >> 8) & 0xff, - (pos & 0xff), + (start >> 16) & 0xff, + (start >> 8) & 0xff, + (start & 0xff), buf[0], buf[1] }, @@ -1026,9 +1033,9 @@ #if defined(__i386__) || defined(__x86_64__) case SPI_CONTROLLER_IT87XX: case SPI_CONTROLLER_WBSIO: - msg_cerr("%s: impossible with this SPI controller," + msg_perr("%s: impossible with this SPI controller," " degrading to byte program\n", __func__); - return spi_chip_write_1(flash, buf); + return spi_chip_write_1_new(flash, buf, start, len); #endif #endif default: @@ -1036,7 +1043,7 @@ }
/* The data sheet requires a start address with the low bit cleared. */ - if (addr % 2) { + if (start % 2) { msg_cerr("%s: start address not even! Please report a bug at " "flashrom@flashrom.org\n", __func__); return SPI_GENERIC_ERROR; @@ -1048,10 +1055,6 @@ return SPI_GENERIC_ERROR; }
- if (erase_flash(flash)) { - msg_cerr("ERASE FAILED!\n"); - return -1; - }
result = spi_send_multicommand(cmds); if (result) { @@ -1065,7 +1068,7 @@ /* We already wrote 2 bytes in the multicommand step. */ pos += 2;
- while (pos < addr + len) { + while (pos < start + len) { cmd[1] = buf[pos++]; cmd[2] = buf[pos++]; spi_send_command(JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE, 0, cmd, NULL); Index: flashrom-partial_write_spi_intermediate/it87spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/it87spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/it87spi.c (Arbeitskopie) @@ -328,30 +328,29 @@ return 0; }
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - int i; - /* * IT8716F only allows maximum of 512 kb SPI chip size for memory * mapped access. */ - if ((programmer == PROGRAMMER_IT87SPI) || (total_size > 512 * 1024)) { - spi_chip_write_1(flash, buf); +#ifdef TODO +#warning This function needs to be converted to partial write + if ((programmer == PROGRAMMER_IT87SPI) || (flash->total_size * 1024 > 512 * 1024)) { +#endif + spi_chip_write_1_new(flash, buf, start, len); +#ifdef TODO +#warning This function needs to be converted to partial write } else { + int i; spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - for (i = 0; i < total_size / 256; i++) { + + /* FIXME: Handle chips which have max writechunk size >1 and <256. */ + for (i = 0; i < flash->total_size * 1024 / 256; i++) { it8716f_spi_page_program(flash, i, buf); } } +#endif
return 0; } Index: flashrom-partial_write_spi_intermediate/buspirate_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/buspirate_spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/buspirate_spi.c (Arbeitskopie) @@ -309,18 +309,8 @@ return spi_read_chunked(flash, buf, start, len, 12); }
-int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf) +int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - spi_disable_blockprotect(); - /* Erase first. */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - return spi_write_chunked(flash, buf, 0, total_size, 12); + return spi_write_chunked(flash, buf, start, len, 12); } Index: flashrom-partial_write_spi_intermediate/bitbang_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/bitbang_spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/bitbang_spi.c (Arbeitskopie) @@ -139,10 +139,8 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
-int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf) +int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - - msg_pdbg("total_size is %d\n", total_size); - return spi_write_chunked(flash, buf, 0, total_size, 256); + spi_disable_blockprotect(); + return spi_write_chunked(flash, buf, start, len, 256); } Index: flashrom-partial_write_spi_intermediate/flashchips.c =================================================================== --- flashrom-partial_write_spi_intermediate/flashchips.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/flashchips.c (Arbeitskopie) @@ -1392,7 +1392,7 @@ .block_erase = spi_block_erase_c7, } }, - .write = spi_aai_write, + .write = spi_chip_write_1, .read = read_memmapped, },
Index: flashrom-partial_write_spi_intermediate/spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/spi.c (Arbeitskopie) @@ -66,7 +66,7 @@ .command = sb600_spi_send_command, .multicommand = default_spi_send_multicommand, .read = sb600_spi_read, - .write_256 = sb600_spi_write_1, + .write_256 = sb600_spi_write_256, },
{ /* SPI_CONTROLLER_VIA */ @@ -117,7 +117,7 @@ .command = dediprog_spi_send_command, .multicommand = default_spi_send_multicommand, .read = dediprog_spi_read, - .write_256 = spi_chip_write_1, + .write_256 = spi_chip_write_1_new, }, #endif
@@ -197,7 +197,8 @@ * Program chip using page (256 bytes) programming. * Some SPI masters can't do this, they use single byte programming instead. */ -int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +/* 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) { if (!spi_programmer[spi_controller].write_256) { msg_perr("%s called, but SPI page write is unsupported on this " @@ -206,9 +207,30 @@ return 1; }
- return spi_programmer[spi_controller].write_256(flash, buf); + 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; + + spi_disable_blockprotect(); + msg_pinfo("Erasing flash before programming... "); + if (erase_flash(flash)) { + msg_perr("ERASE FAILED!\n"); + return -1; + } + msg_pinfo("done.\n"); + 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. Index: flashrom-partial_write_spi_intermediate/ft2232_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/ft2232_spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/ft2232_spi.c (Arbeitskopie) @@ -288,20 +288,10 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
-int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf) +int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - spi_disable_blockprotect(); - /* Erase first. */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - msg_pdbg("total_size is %d\n", total_size); - return spi_write_chunked(flash, buf, 0, total_size, 256); + return spi_write_chunked(flash, buf, start, len, 256); }
#endif Index: flashrom-partial_write_spi_intermediate/wbsio_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/wbsio_spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/wbsio_spi.c (Arbeitskopie) @@ -189,16 +189,14 @@ return read_memmapped(flash, buf, start, len); }
-int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf) +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len) { - int size = flash->total_size * 1024; - - if (size > 1024 * 1024) { + if (flash->total_size * 1024 > 1024 * 1024) { msg_perr("%s: Winbond saved on 4 register bits so max chip size is 1024 KB!\n", __func__); return 1; }
- return spi_chip_write_1(flash, buf); + return spi_chip_write_1_new(flash, buf, start, len); }
#endif Index: flashrom-partial_write_spi_intermediate/sb600spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/sb600spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/sb600spi.c (Arbeitskopie) @@ -48,25 +48,10 @@ return spi_read_chunked(flash, buf, start, len, 8); }
-/* FIXME: SB600 can write 5 bytes per transaction. */ -int sb600_spi_write_1(struct flashchip *flash, uint8_t *buf) +int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = flash->total_size * 1024; - int result = 0; - spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - msg_pinfo("Programming flash"); - result = spi_write_chunked(flash, buf, 0, total_size, 5); - msg_pinfo(" done.\n"); - return result; + return spi_write_chunked(flash, buf, start, len, 5); }
static void reset_internal_fifo_pointer(void) Index: flashrom-partial_write_spi_intermediate/ichspi.c =================================================================== --- flashrom-partial_write_spi_intermediate/ichspi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/ichspi.c (Arbeitskopie) @@ -683,36 +683,15 @@ return spi_read_chunked(flash, buf, start, len, maxdata); }
-int ich_spi_write_256(struct flashchip *flash, uint8_t * buf) +int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len) { - int i, ret = 0; - int total_size = flash->total_size * 1024; - int erase_size = 64 * 1024; int maxdata = 64;
if (spi_controller == SPI_CONTROLLER_VIA) maxdata = 16;
spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - msg_pinfo("Programming page: \n"); - for (i = 0; i < total_size / erase_size; i++) { - ret = spi_write_chunked(flash, buf + (i * erase_size), - i * erase_size, erase_size, maxdata); - if (ret) - break; - } - - msg_pinfo("\n"); - - return ret; + return spi_write_chunked(flash, buf, start, len, maxdata); }
int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, Index: flashrom-partial_write_spi_intermediate/chipdrivers.h =================================================================== --- flashrom-partial_write_spi_intermediate/chipdrivers.h (Revision 1072) +++ flashrom-partial_write_spi_intermediate/chipdrivers.h (Arbeitskopie) @@ -43,6 +43,8 @@ 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_read(struct flashchip *flash, uint8_t *buf, int start, int len); uint8_t spi_read_status_register(void); int spi_disable_blockprotect(void); @@ -51,7 +53,7 @@ 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(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(chipaddr bios);
Looks good overall in preparing write functions for partial-write capability, e.g. taking a range and length rather than assuming 0 to total_size. I patched this in and it appears to be non-harmful on my NM10 setup, at least.
Note: There was one compilation issue ("it87spi.c:289: error: ‘it8716f_spi_page_program’ defined but not used") in the lines below due to the #ifdef.
+#ifdef TODO
+#warning This function needs to be converted to partial write
} else {
+ int i;
spi_disable_blockprotect();
- /* Erase first */
- msg_pinfo("Erasing flash before programming... ");
- if (erase_flash(flash)) {
- msg_perr("ERASE FAILED!\n");
- return -1;
- }
- msg_pinfo("done.\n");
- for (i = 0; i < total_size / 256; i++) {
+
+ /* FIXME: Handle chips which have max writechunk size >1 and
<256. */
+ for (i = 0; i < flash->total_size * 1024 / 256; i++) {
it8716f_spi_page_program(flash, i, buf);
}
}
+#endif
I'd like to see this go in once the compilation issue is addressed so further progress can be made w.r.t. partial writes.
On 10.07.2010 00:24, David Hendricks wrote:
Looks good overall in preparing write functions for partial-write capability, e.g. taking a range and length rather than assuming 0 to total_size. I patched this in and it appears to be non-harmful on my NM10 setup, at least.
Note: There was one compilation issue ("it87spi.c:289: error: ‘it8716f_spi_page_program’ defined but not used") in the lines below due to the #ifdef.
I'd like to see this go in once the compilation issue is addressed so further progress can be made w.r.t. partial writes.
Like this? The new it87spi code is a really nasty hack, but it looks correct. I'd love to get a thorough review for that. Maybe I should reuse the logic in spi_read_chunked and spi_write_chunked, but that logic will soon be overhauled by a patch from Michael Karcher.
Convert SPI chips to partial write, but wrap the write functions in a compat layer to allow converting the rest of flashrom later. I actually have patches for most of the remaining conversion, but I wanted to get this out and reviewed first.
Compile tested, but that's it.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-partial_write_spi_intermediate/flash.h =================================================================== --- flashrom-partial_write_spi_intermediate/flash.h (Revision 1072) +++ flashrom-partial_write_spi_intermediate/flash.h (Arbeitskopie) @@ -528,7 +528,7 @@ int ft2232_spi_init(void); int ft2232_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int ft2232_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf); +int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* bitbang_spi.c */ extern int bitbang_spi_half_period; @@ -536,7 +536,7 @@ int bitbang_spi_init(void); int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int bitbang_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf); +int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* buspirate_spi.c */ struct buspirate_spispeeds { @@ -547,7 +547,7 @@ int buspirate_spi_shutdown(void); int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int buspirate_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf); +int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* dediprog.c */ int dediprog_init(void); @@ -667,7 +667,7 @@
/* Optimized functions for this programmer */ int (*read)(struct flashchip *flash, uint8_t *buf, int start, int len); - int (*write_256)(struct flashchip *flash, uint8_t *buf); + int (*write_256)(struct flashchip *flash, uint8_t *buf, int start, int len); };
extern enum spi_controller spi_controller; @@ -688,7 +688,7 @@ int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int ich_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int ich_spi_write_256(struct flashchip *flash, uint8_t * buf); +int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len); int ich_spi_send_multicommand(struct spi_command *cmds);
/* it87spi.c */ @@ -701,13 +701,13 @@ int it8716f_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf); +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* sb600spi.c */ int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int sb600_spi_write_1(struct flashchip *flash, uint8_t *buf); +int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len); extern uint8_t *sb600_spibar;
/* wbsio_spi.c */ @@ -715,7 +715,7 @@ int wbsio_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int wbsio_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf); +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len);
/* serprog.c */ int serprog_init(void); Index: flashrom-partial_write_spi_intermediate/spi25.c =================================================================== --- flashrom-partial_write_spi_intermediate/spi25.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/spi25.c (Arbeitskopie) @@ -874,6 +874,7 @@
/* * Read a part of the flash chip. + * FIXME: Use the chunk code from Michael Karcher instead. * Each page is read separately in chunks with a maximum size of chunksize. */ int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize) @@ -913,6 +914,7 @@
/* * Write a part of the flash chip. + * FIXME: Use the chunk code from Michael Karcher instead. * Each page is written separately in chunks with a maximum size of chunksize. */ int spi_write_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize) @@ -963,20 +965,13 @@ * and for chips where memory mapped programming is impossible * (e.g. due to size constraints in IT87* for over 512 kB) */ -int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) +/* 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 total_size = 1024 * flash->total_size; int i, result = 0;
spi_disable_blockprotect(); - /* Erase first */ - msg_cinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_cerr("ERASE FAILED!\n"); - return -1; - } - msg_cinfo("done.\n"); - for (i = 0; i < total_size; i++) { + for (i = start; i < start + len; i++) { result = spi_byte_program(i, buf[i]); if (result) return 1; @@ -987,11 +982,23 @@ return 0; }
-int spi_aai_write(struct flashchip *flash, uint8_t *buf) +int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) { - uint32_t addr = 0; - uint32_t len = flash->total_size * 1024; - uint32_t pos = addr; + spi_disable_blockprotect(); + /* Erase first */ + msg_cinfo("Erasing flash before programming... "); + if (erase_flash(flash)) { + msg_cerr("ERASE FAILED!\n"); + return -1; + } + msg_cinfo("done.\n"); + + return spi_chip_write_1_new(flash, buf, 0, flash->total_size * 1024); +} + +int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int len) +{ + uint32_t pos = start; int result; unsigned char cmd[JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE] = { JEDEC_AAI_WORD_PROGRAM, @@ -1006,9 +1013,9 @@ .writecnt = JEDEC_AAI_WORD_PROGRAM_OUTSIZE, .writearr = (const unsigned char[]){ JEDEC_AAI_WORD_PROGRAM, - (pos >> 16) & 0xff, - (pos >> 8) & 0xff, - (pos & 0xff), + (start >> 16) & 0xff, + (start >> 8) & 0xff, + (start & 0xff), buf[0], buf[1] }, @@ -1026,17 +1033,21 @@ #if defined(__i386__) || defined(__x86_64__) case SPI_CONTROLLER_IT87XX: case SPI_CONTROLLER_WBSIO: - msg_cerr("%s: impossible with this SPI controller," + msg_perr("%s: impossible with this SPI controller," " degrading to byte program\n", __func__); - return spi_chip_write_1(flash, buf); + return spi_chip_write_1_new(flash, buf, start, len); #endif #endif default: break; }
+ /* 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. + */ /* The data sheet requires a start address with the low bit cleared. */ - if (addr % 2) { + if (start % 2) { msg_cerr("%s: start address not even! Please report a bug at " "flashrom@flashrom.org\n", __func__); return SPI_GENERIC_ERROR; @@ -1048,15 +1059,14 @@ return SPI_GENERIC_ERROR; }
- if (erase_flash(flash)) { - msg_cerr("ERASE FAILED!\n"); - return -1; - }
result = spi_send_multicommand(cmds); if (result) { msg_cerr("%s failed during start command execution\n", __func__); + /* FIXME: Should we send WRDI here as well to make sure the chip + * is not in AAI mode? + */ return result; } while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP) @@ -1065,7 +1075,7 @@ /* We already wrote 2 bytes in the multicommand step. */ pos += 2;
- while (pos < addr + len) { + while (pos < start + len) { cmd[1] = buf[pos++]; cmd[2] = buf[pos++]; spi_send_command(JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE, 0, cmd, NULL); Index: flashrom-partial_write_spi_intermediate/it87spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/it87spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/it87spi.c (Arbeitskopie) @@ -286,7 +286,7 @@ }
/* Page size is usually 256 bytes */ -static int it8716f_spi_page_program(struct flashchip *flash, int block, uint8_t *buf) +static int it8716f_spi_page_program(struct flashchip *flash, uint8_t *buf, int start) { int i; int result; @@ -299,7 +299,7 @@ OUTB(0x06, it8716f_flashport + 1); OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport); for (i = 0; i < 256; i++) { - chip_writeb(buf[256 * block + i], bios + 256 * block + i); + chip_writeb(buf[i], bios + start + i); } OUTB(0, it8716f_flashport); /* Wait until the Write-In-Progress bit is cleared. @@ -328,29 +328,55 @@ return 0; }
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +static int round_up(int val, int granularity) { - int total_size = 1024 * flash->total_size; - int i; + int tmp;
+ /* Premature optimization? You decide. */ + if (!(granularity & (granularity - 1))) { + /* Granularity is a power of two. */ + return (val + granularity - 1) & ~ (granularity - 1); + } + + tmp = val % granularity; + if (!tmp) + return val; + else + return val - tmp + granularity; +} + +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) +{ /* * IT8716F only allows maximum of 512 kb SPI chip size for memory * mapped access. */ - if ((programmer == PROGRAMMER_IT87SPI) || (total_size > 512 * 1024)) { - spi_chip_write_1(flash, buf); + if ((programmer == PROGRAMMER_IT87SPI) || (flash->total_size * 1024 > 512 * 1024)) { + spi_chip_write_1_new(flash, buf, start, len); } else { + int writehere; spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; + + if (start % 256) { + /* start to the end of the page or start + len, + * whichever is smaller. + */ + writehere = min(len, round_up(start, 256) - start); + spi_chip_write_1_new(flash, buf, start, writehere); + start += writehere; + len -= writehere; + buf += writehere; } - msg_pinfo("done.\n"); - for (i = 0; i < total_size / 256; i++) { - it8716f_spi_page_program(flash, i, buf); + + /* FIXME: Handle chips which have max writechunk size >1 and <256. */ + while (len >= 256) { + it8716f_spi_page_program(flash, buf, start); + start += 256; + len -= 256; + buf += 256; } + if (len) + spi_chip_write_1_new(flash, buf, start, len); }
return 0; Index: flashrom-partial_write_spi_intermediate/buspirate_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/buspirate_spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/buspirate_spi.c (Arbeitskopie) @@ -309,18 +309,8 @@ return spi_read_chunked(flash, buf, start, len, 12); }
-int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf) +int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - spi_disable_blockprotect(); - /* Erase first. */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - return spi_write_chunked(flash, buf, 0, total_size, 12); + return spi_write_chunked(flash, buf, start, len, 12); } Index: flashrom-partial_write_spi_intermediate/bitbang_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/bitbang_spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/bitbang_spi.c (Arbeitskopie) @@ -139,10 +139,8 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
-int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf) +int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - - msg_pdbg("total_size is %d\n", total_size); - return spi_write_chunked(flash, buf, 0, total_size, 256); + spi_disable_blockprotect(); + return spi_write_chunked(flash, buf, start, len, 256); } Index: flashrom-partial_write_spi_intermediate/flashchips.c =================================================================== --- flashrom-partial_write_spi_intermediate/flashchips.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/flashchips.c (Arbeitskopie) @@ -1392,7 +1392,7 @@ .block_erase = spi_block_erase_c7, } }, - .write = spi_aai_write, + .write = spi_chip_write_1, .read = read_memmapped, },
Index: flashrom-partial_write_spi_intermediate/spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/spi.c (Arbeitskopie) @@ -66,7 +66,7 @@ .command = sb600_spi_send_command, .multicommand = default_spi_send_multicommand, .read = sb600_spi_read, - .write_256 = sb600_spi_write_1, + .write_256 = sb600_spi_write_256, },
{ /* SPI_CONTROLLER_VIA */ @@ -117,7 +117,7 @@ .command = dediprog_spi_send_command, .multicommand = default_spi_send_multicommand, .read = dediprog_spi_read, - .write_256 = spi_chip_write_1, + .write_256 = spi_chip_write_1_new, }, #endif
@@ -197,7 +197,8 @@ * Program chip using page (256 bytes) programming. * Some SPI masters can't do this, they use single byte programming instead. */ -int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +/* 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) { if (!spi_programmer[spi_controller].write_256) { msg_perr("%s called, but SPI page write is unsupported on this " @@ -206,9 +207,30 @@ return 1; }
- return spi_programmer[spi_controller].write_256(flash, buf); + 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; + + spi_disable_blockprotect(); + msg_pinfo("Erasing flash before programming... "); + if (erase_flash(flash)) { + msg_perr("ERASE FAILED!\n"); + return -1; + } + msg_pinfo("done.\n"); + 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. Index: flashrom-partial_write_spi_intermediate/ft2232_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/ft2232_spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/ft2232_spi.c (Arbeitskopie) @@ -288,20 +288,10 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
-int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf) +int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - spi_disable_blockprotect(); - /* Erase first. */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - msg_pdbg("total_size is %d\n", total_size); - return spi_write_chunked(flash, buf, 0, total_size, 256); + return spi_write_chunked(flash, buf, start, len, 256); }
#endif Index: flashrom-partial_write_spi_intermediate/wbsio_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/wbsio_spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/wbsio_spi.c (Arbeitskopie) @@ -189,16 +189,14 @@ return read_memmapped(flash, buf, start, len); }
-int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf) +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len) { - int size = flash->total_size * 1024; - - if (size > 1024 * 1024) { + if (flash->total_size * 1024 > 1024 * 1024) { msg_perr("%s: Winbond saved on 4 register bits so max chip size is 1024 KB!\n", __func__); return 1; }
- return spi_chip_write_1(flash, buf); + return spi_chip_write_1_new(flash, buf, start, len); }
#endif Index: flashrom-partial_write_spi_intermediate/sb600spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/sb600spi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/sb600spi.c (Arbeitskopie) @@ -48,25 +48,10 @@ return spi_read_chunked(flash, buf, start, len, 8); }
-/* FIXME: SB600 can write 5 bytes per transaction. */ -int sb600_spi_write_1(struct flashchip *flash, uint8_t *buf) +int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = flash->total_size * 1024; - int result = 0; - spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - msg_pinfo("Programming flash"); - result = spi_write_chunked(flash, buf, 0, total_size, 5); - msg_pinfo(" done.\n"); - return result; + return spi_write_chunked(flash, buf, start, len, 5); }
static void reset_internal_fifo_pointer(void) Index: flashrom-partial_write_spi_intermediate/ichspi.c =================================================================== --- flashrom-partial_write_spi_intermediate/ichspi.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/ichspi.c (Arbeitskopie) @@ -683,36 +683,15 @@ return spi_read_chunked(flash, buf, start, len, maxdata); }
-int ich_spi_write_256(struct flashchip *flash, uint8_t * buf) +int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len) { - int i, ret = 0; - int total_size = flash->total_size * 1024; - int erase_size = 64 * 1024; int maxdata = 64;
if (spi_controller == SPI_CONTROLLER_VIA) maxdata = 16;
spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - msg_pinfo("Programming page: \n"); - for (i = 0; i < total_size / erase_size; i++) { - ret = spi_write_chunked(flash, buf + (i * erase_size), - i * erase_size, erase_size, maxdata); - if (ret) - break; - } - - msg_pinfo("\n"); - - return ret; + return spi_write_chunked(flash, buf, start, len, maxdata); }
int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, Index: flashrom-partial_write_spi_intermediate/chipdrivers.h =================================================================== --- flashrom-partial_write_spi_intermediate/chipdrivers.h (Revision 1072) +++ flashrom-partial_write_spi_intermediate/chipdrivers.h (Arbeitskopie) @@ -43,6 +43,8 @@ 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_read(struct flashchip *flash, uint8_t *buf, int start, int len); uint8_t spi_read_status_register(void); int spi_disable_blockprotect(void); @@ -51,7 +53,7 @@ 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(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(chipaddr bios);
Am Freitag, den 09.07.2010, 19:34 +0200 schrieb Carl-Daniel Hailfinger:
-int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) +/* real chunksize is 1, logical chunksize is 1 */ +int spi_chip_write_1_new(struct flashchip *flash, uint8_t *buf, int start, int len)
I think "spi_chip_write_1_range" makes a better name than "spi_chip_write_1_new". But if your plan is to remove the old spi_chip_write_1 function and rename this function at the same time to "spi_chip_write_1", I don't really mind the temporary name.
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) {
[...]
+#ifdef TODO +#warning This function needs to be converted to partial write
- if ((programmer == PROGRAMMER_IT87SPI) || (flash->total_size * 1024 > 512 * 1024)) {
+#endif
spi_chip_write_1_new(flash, buf, start, len);+#ifdef TODO +#warning This function needs to be converted to partial write
[...]
+#endif
So this function will fall back to single-byte writes every time now until it is changed in a later patch to support ranges. Did I understand the patch correctly? I think this is acceptable to keep the patch size manageable.
=================================================================== --- flashrom-partial_write_spi_intermediate/flashchips.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/flashchips.c (Arbeitskopie) @@ -1392,7 +1392,7 @@ .block_erase = spi_block_erase_c7, } },
.write = spi_aai_write,
.write = spi_chip_write_1,
You do this change to accommodate for the changed signature of spi_aai_write, I think. But you lose the information that this chip used AAI writes this way. Would adding a comment like "/* AAI capable */" be sensible?
@@ -197,7 +197,8 @@
- Program chip using page (256 bytes) programming.
- Some SPI masters can't do this, they use single byte programming instead.
*/ -int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +/* 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) { if (!spi_programmer[spi_controller].write_256) { msg_perr("%s called, but SPI page write is unsupported on this "
Oops! The comment says that single-byte programming is used on non-256-capable masters, but the code does not handle the fallback to single bytes. I also am unable to find any other place in flashrom that checks for the block write capability before calling this function. Is that a bug?
+/* Wrapper function until the generic code is converted to partial writes. */ +int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +{
- int ret;
- spi_disable_blockprotect();
- msg_pinfo("Erasing flash before programming... ");
- if (erase_flash(flash)) {
msg_perr("ERASE FAILED!\n");return -1;- }
- msg_pinfo("done.\n");
- 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;
+}
This reminds me of patch 1371. I'm all for getting that patch in before this patch and not move around the erase-on-write logic just to kill it. I will send a review soon.
-int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf) +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len) {
- int size = flash->total_size * 1024;
- if (size > 1024 * 1024) {
- if (flash->total_size * 1024 > 1024 * 1024) { msg_perr("%s: Winbond saved on 4 register bits so max chip size is 1024 KB!\n", __func__); return 1; }
- return spi_chip_write_1(flash, buf);
- return spi_chip_write_1_new(flash, buf, start, len);
}
As soon as we have explicit erase, this check is too late in the write function. Don't we even have the same problem even at read time?
Remaining stuff looks fine.
Regards, Michael Karcher
On 10.07.2010 19:21, Michael Karcher wrote:
Am Freitag, den 09.07.2010, 19:34 +0200 schrieb Carl-Daniel Hailfinger:
-int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) +/* real chunksize is 1, logical chunksize is 1 */ +int spi_chip_write_1_new(struct flashchip *flash, uint8_t *buf, int start, int len)
I think "spi_chip_write_1_range" makes a better name than "spi_chip_write_1_new". But if your plan is to remove the old spi_chip_write_1 function and rename this function at the same time to "spi_chip_write_1", I don't really mind the temporary name.
Yes, the plan is to kill the wrapper ASAP once the other chips are converted as well.
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) {
[...]
+#ifdef TODO +#warning This function needs to be converted to partial write
- if ((programmer == PROGRAMMER_IT87SPI) || (flash->total_size * 1024 > 512 * 1024)) {
+#endif
spi_chip_write_1_new(flash, buf, start, len);+#ifdef TODO +#warning This function needs to be converted to partial write
[...]
+#endif
So this function will fall back to single-byte writes every time now until it is changed in a later patch to support ranges. Did I understand the patch correctly? I think this is acceptable to keep the patch size manageable.
My updated patch fixes that, but I'd really like someone to check all corner cases of the code.
=================================================================== --- flashrom-partial_write_spi_intermediate/flashchips.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/flashchips.c (Arbeitskopie) @@ -1392,7 +1392,7 @@ .block_erase = spi_block_erase_c7, } },
.write = spi_aai_write,
.write = spi_chip_write_1,You do this change to accommodate for the changed signature of spi_aai_write, I think. But you lose the information that this chip used AAI writes this way. Would adding a comment like "/* AAI capable */" be sensible?
AFAIK all write_1 chips can do AAI. Right now AAI is broken and I'm waiting for an ack for patch 1548. Switching to write_1 instead of AAI allows me to avoid creating a wrapper for AAI.
@@ -197,7 +197,8 @@
- Program chip using page (256 bytes) programming.
- Some SPI masters can't do this, they use single byte programming instead.
*/ -int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +/* 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) { if (!spi_programmer[spi_controller].write_256) { msg_perr("%s called, but SPI page write is unsupported on this "
Oops! The comment says that single-byte programming is used on non-256-capable masters, but the code does not handle the fallback to single bytes. I also am unable to find any other place in flashrom that checks for the block write capability before calling this function. Is that a bug?
Mh. That's very non-obvious. The spi_programmer array has a .write_256 member for every controller. In case the controller can't do anything except 1-byte writes, we set .write_256 = spi_chip_write_1_new
+/* Wrapper function until the generic code is converted to partial writes. */ +int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +{
- int ret;
- spi_disable_blockprotect();
- msg_pinfo("Erasing flash before programming... ");
- if (erase_flash(flash)) {
msg_perr("ERASE FAILED!\n");return -1;- }
- msg_pinfo("done.\n");
- 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;
+}
This reminds me of patch 1371. I'm all for getting that patch in before this patch and not move around the erase-on-write logic just to kill it. I will send a review soon.
This patch is basically a part of patch 1371 with less bugs, more consistency and working backwards compatibility to make merging and reviewing easier. IMHO patch 1371 is very difficult to review, whereas this patch (and patch 1603) will allow me to post an improved version of patch 1371. I don't like the introduction of two new wrappers either, but those are a lot less of a problem than the existing per-controller wrappers and removing them later will be a lot less invasive than 1371.
-int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf) +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len) {
- int size = flash->total_size * 1024;
- if (size > 1024 * 1024) {
- if (flash->total_size * 1024 > 1024 * 1024) { msg_perr("%s: Winbond saved on 4 register bits so max chip size is 1024 KB!\n", __func__); return 1; }
- return spi_chip_write_1(flash, buf);
- return spi_chip_write_1_new(flash, buf, start, len);
}
As soon as we have explicit erase, this check is too late in the write function. Don't we even have the same problem even at read time?
Yes, this should be handled with max_rom_decode instead. I will send a followup patch for max_rom_decode.spi in wbsio and kill the wbsio-specific read+write.
Remaining stuff looks fine.
Thanks.
Regards, Carl-Daniel
Am Samstag, den 10.07.2010, 19:41 +0200 schrieb Carl-Daniel Hailfinger:
On 10.07.2010 19:21, Michael Karcher wrote:
I think "spi_chip_write_1_range" makes a better name than "spi_chip_write_1_new". But if your plan is to remove the old spi_chip_write_1 function and rename this function at the same time to "spi_chip_write_1", I don't really mind the temporary name.
Yes, the plan is to kill the wrapper ASAP once the other chips are converted as well.
OK, sounds sensible.
=================================================================== --- flashrom-partial_write_spi_intermediate/flashchips.c (Revision 1072) +++ flashrom-partial_write_spi_intermediate/flashchips.c (Arbeitskopie) @@ -1392,7 +1392,7 @@ .block_erase = spi_block_erase_c7, } },
.write = spi_aai_write,
.write = spi_chip_write_1,You do this change to accommodate for the changed signature of spi_aai_write, I think. But you lose the information that this chip used AAI writes this way. Would adding a comment like "/* AAI capable */" be sensible?
AFAIK all write_1 chips can do AAI. Right now AAI is broken and I'm waiting for an ack for patch 1548. Switching to write_1 instead of AAI allows me to avoid creating a wrapper for AAI.
What is missing for 1548? Looks like just the formal ack, as the patch seems to already be tested by den_m. In that case, I would do a review of that patch.
-int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +/* 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) { if (!spi_programmer[spi_controller].write_256) { msg_perr("%s called, but SPI page write is unsupported on this "
Oops! The comment says that single-byte programming is used on non-256-capable masters, but the code does not handle the fallback to single bytes. I also am unable to find any other place in flashrom that checks for the block write capability before calling this function. Is that a bug?
Mh. That's very non-obvious. The spi_programmer array has a .write_256 member for every controller. In case the controller can't do anything except 1-byte writes, we set .write_256 = spi_chip_write_1_new
Not every controller. SPI_CONTROLLER_DUMMY really has NULL in this field. Your patch does not change that. And please kill the test if we are sure that write_256 is always non-null.
This reminds me of patch 1371. I'm all for getting that patch in before this patch and not move around the erase-on-write logic just to kill it. I will send a review soon.
This patch is basically a part of patch 1371 with less bugs, more consistency and working backwards compatibility to make merging and reviewing easier.
I would consider it just the other way around. This patch is about partial write, and includes parts of the semantically unrelated patch about having erase outside of the write functions. Of course, one can also add the write area infrastructure before removing the erase code from the write code, but it seemed to make sense to me to first yank out erase and ad-hoc partial write from the chip driver's write functions, then convert them to a partial-write interface and finally put everything below the generalized erase-block walking.
I don't like the introduction of two new wrappers either, but those are a lot less of a problem than the existing per-controller wrappers and removing them later will be a lot less invasive than 1371.
If your plan is to base a new 1371 on top of this patch, I'm perfectly fine with these wrappers.
As soon as we have explicit erase, this check is too late in the write function. Don't we even have the same problem even at read time?
Yes, this should be handled with max_rom_decode instead. I will send a followup patch for max_rom_decode.spi in wbsio and kill the wbsio-specific read+write.
Great!
Regards, Michael Karcher
On 10.07.2010 20:15, Michael Karcher wrote:
Am Samstag, den 10.07.2010, 19:41 +0200 schrieb Carl-Daniel Hailfinger:
On 10.07.2010 19:21, Michael Karcher wrote:
AFAIK all write_1 chips can do AAI. Right now AAI is broken and I'm waiting for an ack for patch 1548. Switching to write_1 instead of AAI allows me to avoid creating a wrapper for AAI.
What is missing for 1548? Looks like just the formal ack, as the patch seems to already be tested by den_m. In that case, I would do a review of that patch.
Yes, it is tested, but den_m apparently had problems with the current code as well. If someone aborts flashrom, the chip will stay in AAI mode. That's suboptimal.
-int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +/* 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) { if (!spi_programmer[spi_controller].write_256) { msg_perr("%s called, but SPI page write is unsupported on this "
Oops! The comment says that single-byte programming is used on non-256-capable masters, but the code does not handle the fallback to single bytes. I also am unable to find any other place in flashrom that checks for the block write capability before calling this function. Is that a bug?
Mh. That's very non-obvious. The spi_programmer array has a .write_256 member for every controller. In case the controller can't do anything except 1-byte writes, we set .write_256 = spi_chip_write_1_new
Not every controller. SPI_CONTROLLER_DUMMY really has NULL in this field. Your patch does not change that. And please kill the test if we are sure that write_256 is always non-null.
Right. I wanted to add a write_256 function to the dummy programmer anyway. I'll send an updated patch. Side note: If no SPI controller is selected, write_256 is empty as well, but I'd say that's not a problem because it should never happen.
This reminds me of patch 1371. I'm all for getting that patch in before this patch and not move around the erase-on-write logic just to kill it. I will send a review soon.
This patch is basically a part of patch 1371 with less bugs, more consistency and working backwards compatibility to make merging and reviewing easier.
I would consider it just the other way around. This patch is about partial write, and includes parts of the semantically unrelated patch about having erase outside of the write functions. Of course, one can also add the write area infrastructure before removing the erase code from the write code, but it seemed to make sense to me to first yank out erase and ad-hoc partial write from the chip driver's write functions, then convert them to a partial-write interface and finally put everything below the generalized erase-block walking.
One of the reasons I basically abandoned the idea of just ripping out erase and ad-hoc partial write was the missing progress printing. As long as we don't have a good way to print progress, I'm a bit reluctant to rip out erase, but hopefully we have some good progress printing code very soon.
I don't like the introduction of two new wrappers either, but those are a lot less of a problem than the existing per-controller wrappers and removing them later will be a lot less invasive than 1371.
If your plan is to base a new 1371 on top of this patch, I'm perfectly fine with these wrappers.
Thanks. That's indeed the plan.
As soon as we have explicit erase, this check is too late in the write function. Don't we even have the same problem even at read time?
Yes, this should be handled with max_rom_decode instead. I will send a followup patch for max_rom_decode.spi in wbsio and kill the wbsio-specific read+write.
Great!
Thanks for your review.
Regards, Carl-Daniel
On 10.07.2010 21:20, Carl-Daniel Hailfinger wrote:
On 10.07.2010 20:15, Michael Karcher wrote:
Am Samstag, den 10.07.2010, 19:41 +0200 schrieb Carl-Daniel Hailfinger:
The spi_programmer array has a .write_256 member for every controller. In case the controller can't do anything except 1-byte writes, we set .write_256 = spi_chip_write_1_new
Not every controller. SPI_CONTROLLER_DUMMY really has NULL in this field. Your patch does not change that. And please kill the test if we are sure that write_256 is always non-null.
Right. I wanted to add a write_256 function to the dummy programmer anyway. I'll send an updated patch. Side note: If no SPI controller is selected, write_256 is empty as well, but I'd say that's not a problem because it should never happen.
If your plan is to base a new 1371 on top of this patch, I'm perfectly fine with these wrappers.
Thanks. That's indeed the plan.
New version, should hopefully address all comments.
Convert SPI chips to partial write, but wrap the write functions in a compat layer to allow converting the rest of flashrom later. I actually have patches for most of the remaining conversion, but I wanted to get this out and reviewed first.
Compile tested, but that's it. I would really appreciate it if someone could test this on a machine with SPI flash.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-partial_write_spi_intermediate/flash.h =================================================================== --- flashrom-partial_write_spi_intermediate/flash.h (Revision 1074) +++ flashrom-partial_write_spi_intermediate/flash.h (Arbeitskopie) @@ -456,6 +456,7 @@ int dummy_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int dummy_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); +int dummy_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len); #endif
/* nic3com.c */ @@ -529,7 +530,7 @@ int ft2232_spi_init(void); int ft2232_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int ft2232_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf); +int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* bitbang_spi.c */ extern int bitbang_spi_half_period; @@ -537,7 +538,7 @@ int bitbang_spi_init(void); int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int bitbang_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf); +int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* buspirate_spi.c */ struct buspirate_spispeeds { @@ -548,7 +549,7 @@ int buspirate_spi_shutdown(void); int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int buspirate_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf); +int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* dediprog.c */ int dediprog_init(void); @@ -668,7 +669,7 @@
/* Optimized functions for this programmer */ int (*read)(struct flashchip *flash, uint8_t *buf, int start, int len); - int (*write_256)(struct flashchip *flash, uint8_t *buf); + int (*write_256)(struct flashchip *flash, uint8_t *buf, int start, int len); };
extern enum spi_controller spi_controller; @@ -689,7 +690,7 @@ int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int ich_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int ich_spi_write_256(struct flashchip *flash, uint8_t * buf); +int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len); int ich_spi_send_multicommand(struct spi_command *cmds);
/* it87spi.c */ @@ -701,13 +702,13 @@ int it8716f_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf); +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* sb600spi.c */ int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int sb600_spi_write_1(struct flashchip *flash, uint8_t *buf); +int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len); extern uint8_t *sb600_spibar;
/* wbsio_spi.c */ @@ -715,7 +716,7 @@ int wbsio_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int wbsio_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf); +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len);
/* serprog.c */ int serprog_init(void); Index: flashrom-partial_write_spi_intermediate/spi25.c =================================================================== --- flashrom-partial_write_spi_intermediate/spi25.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/spi25.c (Arbeitskopie) @@ -874,6 +874,7 @@
/* * Read a part of the flash chip. + * FIXME: Use the chunk code from Michael Karcher instead. * Each page is read separately in chunks with a maximum size of chunksize. */ int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize) @@ -913,6 +914,7 @@
/* * Write a part of the flash chip. + * FIXME: Use the chunk code from Michael Karcher instead. * Each page is written separately in chunks with a maximum size of chunksize. */ int spi_write_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize) @@ -963,20 +965,13 @@ * and for chips where memory mapped programming is impossible * (e.g. due to size constraints in IT87* for over 512 kB) */ -int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) +/* 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 total_size = 1024 * flash->total_size; int i, result = 0;
spi_disable_blockprotect(); - /* Erase first */ - msg_cinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_cerr("ERASE FAILED!\n"); - return -1; - } - msg_cinfo("done.\n"); - for (i = 0; i < total_size; i++) { + for (i = start; i < start + len; i++) { result = spi_byte_program(i, buf[i]); if (result) return 1; @@ -987,11 +982,23 @@ return 0; }
-int spi_aai_write(struct flashchip *flash, uint8_t *buf) +int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) { - uint32_t addr = 0; - uint32_t len = flash->total_size * 1024; - uint32_t pos = addr; + spi_disable_blockprotect(); + /* Erase first */ + msg_cinfo("Erasing flash before programming... "); + if (erase_flash(flash)) { + msg_cerr("ERASE FAILED!\n"); + return -1; + } + msg_cinfo("done.\n"); + + return spi_chip_write_1_new(flash, buf, 0, flash->total_size * 1024); +} + +int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int len) +{ + uint32_t pos = start; int result; unsigned char cmd[JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE] = { JEDEC_AAI_WORD_PROGRAM, @@ -1006,9 +1013,9 @@ .writecnt = JEDEC_AAI_WORD_PROGRAM_OUTSIZE, .writearr = (const unsigned char[]){ JEDEC_AAI_WORD_PROGRAM, - (pos >> 16) & 0xff, - (pos >> 8) & 0xff, - (pos & 0xff), + (start >> 16) & 0xff, + (start >> 8) & 0xff, + (start & 0xff), buf[0], buf[1] }, @@ -1026,17 +1033,21 @@ #if defined(__i386__) || defined(__x86_64__) case SPI_CONTROLLER_IT87XX: case SPI_CONTROLLER_WBSIO: - msg_cerr("%s: impossible with this SPI controller," + msg_perr("%s: impossible with this SPI controller," " degrading to byte program\n", __func__); - return spi_chip_write_1(flash, buf); + return spi_chip_write_1_new(flash, buf, start, len); #endif #endif default: break; }
+ /* 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. + */ /* The data sheet requires a start address with the low bit cleared. */ - if (addr % 2) { + if (start % 2) { msg_cerr("%s: start address not even! Please report a bug at " "flashrom@flashrom.org\n", __func__); return SPI_GENERIC_ERROR; @@ -1048,15 +1059,14 @@ return SPI_GENERIC_ERROR; }
- if (erase_flash(flash)) { - msg_cerr("ERASE FAILED!\n"); - return -1; - }
result = spi_send_multicommand(cmds); if (result) { msg_cerr("%s failed during start command execution\n", __func__); + /* FIXME: Should we send WRDI here as well to make sure the chip + * is not in AAI mode? + */ return result; } while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP) @@ -1065,7 +1075,7 @@ /* We already wrote 2 bytes in the multicommand step. */ pos += 2;
- while (pos < addr + len) { + while (pos < start + len) { cmd[1] = buf[pos++]; cmd[2] = buf[pos++]; spi_send_command(JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE, 0, cmd, NULL); Index: flashrom-partial_write_spi_intermediate/it87spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/it87spi.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/it87spi.c (Arbeitskopie) @@ -292,7 +292,7 @@ }
/* Page size is usually 256 bytes */ -static int it8716f_spi_page_program(struct flashchip *flash, int block, uint8_t *buf) +static int it8716f_spi_page_program(struct flashchip *flash, uint8_t *buf, int start) { int i; int result; @@ -305,7 +305,7 @@ OUTB(0x06, it8716f_flashport + 1); OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport); for (i = 0; i < 256; i++) { - chip_writeb(buf[256 * block + i], bios + 256 * block + i); + chip_writeb(buf[i], bios + start + i); } OUTB(0, it8716f_flashport); /* Wait until the Write-In-Progress bit is cleared. @@ -334,29 +334,55 @@ return 0; }
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +static int round_up(int val, int granularity) { - int total_size = 1024 * flash->total_size; - int i; + int tmp;
+ /* Premature optimization? You decide. */ + if (!(granularity & (granularity - 1))) { + /* Granularity is a power of two. */ + return (val + granularity - 1) & ~ (granularity - 1); + } + + tmp = val % granularity; + if (!tmp) + return val; + else + return val - tmp + granularity; +} + +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) +{ /* * IT8716F only allows maximum of 512 kb SPI chip size for memory * mapped access. */ - if ((programmer == PROGRAMMER_IT87SPI) || (total_size > 512 * 1024)) { - spi_chip_write_1(flash, buf); + if ((programmer == PROGRAMMER_IT87SPI) || (flash->total_size * 1024 > 512 * 1024)) { + spi_chip_write_1_new(flash, buf, start, len); } else { + int writehere; spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; + + if (start % 256) { + /* start to the end of the page or start + len, + * whichever is smaller. + */ + writehere = min(len, round_up(start, 256) - start); + spi_chip_write_1_new(flash, buf, start, writehere); + start += writehere; + len -= writehere; + buf += writehere; } - msg_pinfo("done.\n"); - for (i = 0; i < total_size / 256; i++) { - it8716f_spi_page_program(flash, i, buf); + + /* FIXME: Handle chips which have max writechunk size >1 and <256. */ + while (len >= 256) { + it8716f_spi_page_program(flash, buf, start); + start += 256; + len -= 256; + buf += 256; } + if (len) + spi_chip_write_1_new(flash, buf, start, len); }
return 0; Index: flashrom-partial_write_spi_intermediate/buspirate_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/buspirate_spi.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/buspirate_spi.c (Arbeitskopie) @@ -309,18 +309,8 @@ return spi_read_chunked(flash, buf, start, len, 12); }
-int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf) +int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - spi_disable_blockprotect(); - /* Erase first. */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - return spi_write_chunked(flash, buf, 0, total_size, 12); + return spi_write_chunked(flash, buf, start, len, 12); } Index: flashrom-partial_write_spi_intermediate/bitbang_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/bitbang_spi.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/bitbang_spi.c (Arbeitskopie) @@ -139,10 +139,8 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
-int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf) +int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - - msg_pdbg("total_size is %d\n", total_size); - return spi_write_chunked(flash, buf, 0, total_size, 256); + spi_disable_blockprotect(); + return spi_write_chunked(flash, buf, start, len, 256); } Index: flashrom-partial_write_spi_intermediate/flashchips.c =================================================================== --- flashrom-partial_write_spi_intermediate/flashchips.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/flashchips.c (Arbeitskopie) @@ -1392,7 +1392,7 @@ .block_erase = spi_block_erase_c7, } }, - .write = spi_aai_write, + .write = spi_chip_write_1, .read = spi_chip_read, },
Index: flashrom-partial_write_spi_intermediate/spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/spi.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/spi.c (Arbeitskopie) @@ -66,7 +66,7 @@ .command = sb600_spi_send_command, .multicommand = default_spi_send_multicommand, .read = sb600_spi_read, - .write_256 = sb600_spi_write_1, + .write_256 = sb600_spi_write_256, },
{ /* SPI_CONTROLLER_VIA */ @@ -99,7 +99,7 @@ .command = dummy_spi_send_command, .multicommand = default_spi_send_multicommand, .read = dummy_spi_read, - .write_256 = NULL, + .write_256 = dummy_spi_write_256, }, #endif
@@ -117,7 +117,7 @@ .command = dediprog_spi_send_command, .multicommand = default_spi_send_multicommand, .read = dediprog_spi_read, - .write_256 = spi_chip_write_1, + .write_256 = spi_chip_write_1_new, }, #endif
@@ -196,8 +196,11 @@ /* * Program chip using page (256 bytes) programming. * Some SPI masters can't do this, they use single byte programming instead. + * The redirect to single byte programming is achieved by setting + * .write_256 = spi_chip_write_1 */ -int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +/* 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) { if (!spi_programmer[spi_controller].write_256) { msg_perr("%s called, but SPI page write is unsupported on this " @@ -206,9 +209,30 @@ return 1; }
- return spi_programmer[spi_controller].write_256(flash, buf); + 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; + + spi_disable_blockprotect(); + msg_pinfo("Erasing flash before programming... "); + if (erase_flash(flash)) { + msg_perr("ERASE FAILED!\n"); + return -1; + } + msg_pinfo("done.\n"); + 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. Index: flashrom-partial_write_spi_intermediate/ft2232_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/ft2232_spi.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/ft2232_spi.c (Arbeitskopie) @@ -288,20 +288,10 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
-int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf) +int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - spi_disable_blockprotect(); - /* Erase first. */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - msg_pdbg("total_size is %d\n", total_size); - return spi_write_chunked(flash, buf, 0, total_size, 256); + return spi_write_chunked(flash, buf, start, len, 256); }
#endif Index: flashrom-partial_write_spi_intermediate/wbsio_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/wbsio_spi.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/wbsio_spi.c (Arbeitskopie) @@ -189,16 +189,14 @@ return read_memmapped(flash, buf, start, len); }
-int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf) +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len) { - int size = flash->total_size * 1024; - - if (size > 1024 * 1024) { + if (flash->total_size * 1024 > 1024 * 1024) { msg_perr("%s: Winbond saved on 4 register bits so max chip size is 1024 KB!\n", __func__); return 1; }
- return spi_chip_write_1(flash, buf); + return spi_chip_write_1_new(flash, buf, start, len); }
#endif Index: flashrom-partial_write_spi_intermediate/dummyflasher.c =================================================================== --- flashrom-partial_write_spi_intermediate/dummyflasher.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/dummyflasher.c (Arbeitskopie) @@ -167,3 +167,12 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
+/* Is is impossible to trigger this code path because dummyflasher probing will + * never be successful, and the current frontend refuses to write in that case. + * Other frontends may allow writing even for non-detected chips, though. + */ +int dummy_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) +{ + spi_disable_blockprotect(); + return spi_write_chunked(flash, buf, start, len, 256); +} Index: flashrom-partial_write_spi_intermediate/sb600spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/sb600spi.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/sb600spi.c (Arbeitskopie) @@ -48,25 +48,10 @@ return spi_read_chunked(flash, buf, start, len, 8); }
-/* FIXME: SB600 can write 5 bytes per transaction. */ -int sb600_spi_write_1(struct flashchip *flash, uint8_t *buf) +int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = flash->total_size * 1024; - int result = 0; - spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - msg_pinfo("Programming flash"); - result = spi_write_chunked(flash, buf, 0, total_size, 5); - msg_pinfo(" done.\n"); - return result; + return spi_write_chunked(flash, buf, start, len, 5); }
static void reset_internal_fifo_pointer(void) Index: flashrom-partial_write_spi_intermediate/ichspi.c =================================================================== --- flashrom-partial_write_spi_intermediate/ichspi.c (Revision 1074) +++ flashrom-partial_write_spi_intermediate/ichspi.c (Arbeitskopie) @@ -683,36 +683,15 @@ return spi_read_chunked(flash, buf, start, len, maxdata); }
-int ich_spi_write_256(struct flashchip *flash, uint8_t * buf) +int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len) { - int i, ret = 0; - int total_size = flash->total_size * 1024; - int erase_size = 64 * 1024; int maxdata = 64;
if (spi_controller == SPI_CONTROLLER_VIA) maxdata = 16;
spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - msg_pinfo("Programming page: \n"); - for (i = 0; i < total_size / erase_size; i++) { - ret = spi_write_chunked(flash, buf + (i * erase_size), - i * erase_size, erase_size, maxdata); - if (ret) - break; - } - - msg_pinfo("\n"); - - return ret; + return spi_write_chunked(flash, buf, start, len, maxdata); }
int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, Index: flashrom-partial_write_spi_intermediate/chipdrivers.h =================================================================== --- flashrom-partial_write_spi_intermediate/chipdrivers.h (Revision 1074) +++ flashrom-partial_write_spi_intermediate/chipdrivers.h (Arbeitskopie) @@ -43,6 +43,8 @@ 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_read(struct flashchip *flash, uint8_t *buf, int start, int len); uint8_t spi_read_status_register(void); int spi_disable_blockprotect(void); @@ -51,7 +53,7 @@ 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(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(chipaddr bios);
Am Sonntag, den 11.07.2010, 01:02 +0200 schrieb Carl-Daniel Hailfinger:
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +static int round_up(int val, int granularity) {
- int total_size = 1024 * flash->total_size;
- int i;
int tmp;
/* Premature optimization? You decide. */
if (!(granularity & (granularity - 1))) {
/* Granularity is a power of two. */return (val + granularity - 1) & ~ (granularity - 1);}
tmp = val % granularity;
if (!tmp)
return val;else
return val - tmp + granularity;+}
I in fact *do* decide. But not about the premature optimization, but about a premature generalization. Don't write a general round_up function, but just write the power-of-two case.
If you use the bitwise round-up here: Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Regards, Michael Karcher
On 13.07.2010 17:02, Michael Karcher wrote:
Am Sonntag, den 11.07.2010, 01:02 +0200 schrieb Carl-Daniel Hailfinger:
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +static int round_up(int val, int granularity) {
- int total_size = 1024 * flash->total_size;
- int i;
int tmp;
/* Premature optimization? You decide. */
if (!(granularity & (granularity - 1))) {
/* Granularity is a power of two. */return (val + granularity - 1) & ~ (granularity - 1);}
tmp = val % granularity;
if (!tmp)
return val;else
return val - tmp + granularity;+}
I in fact *do* decide. But not about the premature optimization, but about a premature generalization. Don't write a general round_up function, but just write the power-of-two case.
Since we're only handling a very special case (round_up_to_next_multiple_of_power_of_two_granularity) and use it only one time anyway, it doesn't really make sense to provide a separate function for this. Killed as per IRC discussion.
If you use the bitwise round-up here: Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Thanks!
New patch follows.
Convert SPI chips to partial write, but wrap the write functions in a compat layer to allow converting the rest of flashrom later. I actually have patches for most of the remaining conversion, but I wanted to get this out and reviewed first.
Compile tested, and runtime tested on Intel NM10. I would really appreciate it if someone could test this on a machine with IT87 SPI flash.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Index: flashrom-partial_write_spi_intermediate/flash.h =================================================================== --- flashrom-partial_write_spi_intermediate/flash.h (Revision 1079) +++ flashrom-partial_write_spi_intermediate/flash.h (Arbeitskopie) @@ -456,6 +456,7 @@ int dummy_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int dummy_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); +int dummy_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len); #endif
/* nic3com.c */ @@ -529,7 +530,7 @@ int ft2232_spi_init(void); int ft2232_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int ft2232_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf); +int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* bitbang_spi.c */ extern int bitbang_spi_half_period; @@ -537,7 +538,7 @@ int bitbang_spi_init(void); int bitbang_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int bitbang_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf); +int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* buspirate_spi.c */ struct buspirate_spispeeds { @@ -548,7 +549,7 @@ int buspirate_spi_shutdown(void); int buspirate_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int buspirate_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf); +int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* dediprog.c */ int dediprog_init(void); @@ -668,7 +669,7 @@
/* Optimized functions for this programmer */ int (*read)(struct flashchip *flash, uint8_t *buf, int start, int len); - int (*write_256)(struct flashchip *flash, uint8_t *buf); + int (*write_256)(struct flashchip *flash, uint8_t *buf, int start, int len); };
extern enum spi_controller spi_controller; @@ -689,7 +690,7 @@ int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int ich_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int ich_spi_write_256(struct flashchip *flash, uint8_t * buf); +int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len); int ich_spi_send_multicommand(struct spi_command *cmds);
/* it87spi.c */ @@ -701,13 +702,13 @@ int it8716f_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf); +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len);
/* sb600spi.c */ int sb600_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int sb600_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int sb600_spi_write_1(struct flashchip *flash, uint8_t *buf); +int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len); extern uint8_t *sb600_spibar;
/* wbsio_spi.c */ @@ -715,7 +716,7 @@ int wbsio_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); int wbsio_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len); -int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf); +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len);
/* serprog.c */ int serprog_init(void); Index: flashrom-partial_write_spi_intermediate/spi25.c =================================================================== --- flashrom-partial_write_spi_intermediate/spi25.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/spi25.c (Arbeitskopie) @@ -874,6 +874,7 @@
/* * Read a part of the flash chip. + * FIXME: Use the chunk code from Michael Karcher instead. * Each page is read separately in chunks with a maximum size of chunksize. */ int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize) @@ -913,6 +914,7 @@
/* * Write a part of the flash chip. + * FIXME: Use the chunk code from Michael Karcher instead. * Each page is written separately in chunks with a maximum size of chunksize. */ int spi_write_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize) @@ -963,20 +965,13 @@ * and for chips where memory mapped programming is impossible * (e.g. due to size constraints in IT87* for over 512 kB) */ -int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) +/* 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 total_size = 1024 * flash->total_size; int i, result = 0;
spi_disable_blockprotect(); - /* Erase first */ - msg_cinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_cerr("ERASE FAILED!\n"); - return -1; - } - msg_cinfo("done.\n"); - for (i = 0; i < total_size; i++) { + for (i = start; i < start + len; i++) { result = spi_byte_program(i, buf[i]); if (result) return 1; @@ -987,11 +982,23 @@ return 0; }
-int spi_aai_write(struct flashchip *flash, uint8_t *buf) +int spi_chip_write_1(struct flashchip *flash, uint8_t *buf) { - uint32_t addr = 0; - uint32_t len = flash->total_size * 1024; - uint32_t pos = addr; + spi_disable_blockprotect(); + /* Erase first */ + msg_cinfo("Erasing flash before programming... "); + if (erase_flash(flash)) { + msg_cerr("ERASE FAILED!\n"); + return -1; + } + msg_cinfo("done.\n"); + + return spi_chip_write_1_new(flash, buf, 0, flash->total_size * 1024); +} + +int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int len) +{ + uint32_t pos = start; int result; unsigned char cmd[JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE] = { JEDEC_AAI_WORD_PROGRAM, @@ -1006,9 +1013,9 @@ .writecnt = JEDEC_AAI_WORD_PROGRAM_OUTSIZE, .writearr = (const unsigned char[]){ JEDEC_AAI_WORD_PROGRAM, - (pos >> 16) & 0xff, - (pos >> 8) & 0xff, - (pos & 0xff), + (start >> 16) & 0xff, + (start >> 8) & 0xff, + (start & 0xff), buf[0], buf[1] }, @@ -1026,17 +1033,21 @@ #if defined(__i386__) || defined(__x86_64__) case SPI_CONTROLLER_IT87XX: case SPI_CONTROLLER_WBSIO: - msg_cerr("%s: impossible with this SPI controller," + msg_perr("%s: impossible with this SPI controller," " degrading to byte program\n", __func__); - return spi_chip_write_1(flash, buf); + return spi_chip_write_1_new(flash, buf, start, len); #endif #endif default: break; }
+ /* 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. + */ /* The data sheet requires a start address with the low bit cleared. */ - if (addr % 2) { + if (start % 2) { msg_cerr("%s: start address not even! Please report a bug at " "flashrom@flashrom.org\n", __func__); return SPI_GENERIC_ERROR; @@ -1048,15 +1059,14 @@ return SPI_GENERIC_ERROR; }
- if (erase_flash(flash)) { - msg_cerr("ERASE FAILED!\n"); - return -1; - }
result = spi_send_multicommand(cmds); if (result) { msg_cerr("%s failed during start command execution\n", __func__); + /* FIXME: Should we send WRDI here as well to make sure the chip + * is not in AAI mode? + */ return result; } while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP) @@ -1065,7 +1075,7 @@ /* We already wrote 2 bytes in the multicommand step. */ pos += 2;
- while (pos < addr + len) { + while (pos < start + len) { cmd[1] = buf[pos++]; cmd[2] = buf[pos++]; spi_send_command(JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE, 0, cmd, NULL); Index: flashrom-partial_write_spi_intermediate/it87spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/it87spi.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/it87spi.c (Arbeitskopie) @@ -292,7 +292,7 @@ }
/* Page size is usually 256 bytes */ -static int it8716f_spi_page_program(struct flashchip *flash, int block, uint8_t *buf) +static int it8716f_spi_page_program(struct flashchip *flash, uint8_t *buf, int start) { int i; int result; @@ -305,7 +305,7 @@ OUTB(0x06, it8716f_flashport + 1); OUTB(((2 + (fast_spi ? 1 : 0)) << 4), it8716f_flashport); for (i = 0; i < 256; i++) { - chip_writeb(buf[256 * block + i], bios + 256 * block + i); + chip_writeb(buf[i], bios + start + i); } OUTB(0, it8716f_flashport); /* Wait until the Write-In-Progress bit is cleared. @@ -334,29 +334,39 @@ return 0; }
-int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +int it8716f_spi_chip_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - int i; - /* * IT8716F only allows maximum of 512 kb SPI chip size for memory * mapped access. */ - if ((programmer == PROGRAMMER_IT87SPI) || (total_size > 512 * 1024)) { - spi_chip_write_1(flash, buf); + if ((programmer == PROGRAMMER_IT87SPI) || (flash->total_size * 1024 > 512 * 1024)) { + spi_chip_write_1_new(flash, buf, start, len); } else { + int lenhere; spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; + + if (start % 256) { + /* 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, 256 - (start % 256)); + spi_chip_write_1_new(flash, buf, start, lenhere); + start += lenhere; + len -= lenhere; + buf += lenhere; } - msg_pinfo("done.\n"); - for (i = 0; i < total_size / 256; i++) { - it8716f_spi_page_program(flash, i, buf); + + /* FIXME: Handle chips which have max writechunk size >1 and <256. */ + while (len >= 256) { + it8716f_spi_page_program(flash, buf, start); + start += 256; + len -= 256; + buf += 256; } + if (len) + spi_chip_write_1_new(flash, buf, start, len); }
return 0; Index: flashrom-partial_write_spi_intermediate/buspirate_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/buspirate_spi.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/buspirate_spi.c (Arbeitskopie) @@ -309,18 +309,8 @@ return spi_read_chunked(flash, buf, start, len, 12); }
-int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf) +int buspirate_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - spi_disable_blockprotect(); - /* Erase first. */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - return spi_write_chunked(flash, buf, 0, total_size, 12); + return spi_write_chunked(flash, buf, start, len, 12); } Index: flashrom-partial_write_spi_intermediate/bitbang_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/bitbang_spi.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/bitbang_spi.c (Arbeitskopie) @@ -139,10 +139,8 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
-int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf) +int bitbang_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - - msg_pdbg("total_size is %d\n", total_size); - return spi_write_chunked(flash, buf, 0, total_size, 256); + spi_disable_blockprotect(); + return spi_write_chunked(flash, buf, start, len, 256); } Index: flashrom-partial_write_spi_intermediate/flashchips.c =================================================================== --- flashrom-partial_write_spi_intermediate/flashchips.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/flashchips.c (Arbeitskopie) @@ -1423,7 +1423,7 @@ .block_erase = spi_block_erase_c7, } }, - .write = spi_aai_write, + .write = spi_chip_write_1, .read = spi_chip_read, },
Index: flashrom-partial_write_spi_intermediate/spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/spi.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/spi.c (Arbeitskopie) @@ -66,7 +66,7 @@ .command = sb600_spi_send_command, .multicommand = default_spi_send_multicommand, .read = sb600_spi_read, - .write_256 = sb600_spi_write_1, + .write_256 = sb600_spi_write_256, },
{ /* SPI_CONTROLLER_VIA */ @@ -99,7 +99,7 @@ .command = dummy_spi_send_command, .multicommand = default_spi_send_multicommand, .read = dummy_spi_read, - .write_256 = NULL, + .write_256 = dummy_spi_write_256, }, #endif
@@ -117,7 +117,7 @@ .command = dediprog_spi_send_command, .multicommand = default_spi_send_multicommand, .read = dediprog_spi_read, - .write_256 = spi_chip_write_1, + .write_256 = spi_chip_write_1_new, }, #endif
@@ -196,8 +196,11 @@ /* * Program chip using page (256 bytes) programming. * Some SPI masters can't do this, they use single byte programming instead. + * The redirect to single byte programming is achieved by setting + * .write_256 = spi_chip_write_1 */ -int spi_chip_write_256(struct flashchip *flash, uint8_t *buf) +/* 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) { if (!spi_programmer[spi_controller].write_256) { msg_perr("%s called, but SPI page write is unsupported on this " @@ -206,9 +209,30 @@ return 1; }
- return spi_programmer[spi_controller].write_256(flash, buf); + 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; + + spi_disable_blockprotect(); + msg_pinfo("Erasing flash before programming... "); + if (erase_flash(flash)) { + msg_perr("ERASE FAILED!\n"); + return -1; + } + msg_pinfo("done.\n"); + 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. Index: flashrom-partial_write_spi_intermediate/ft2232_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/ft2232_spi.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/ft2232_spi.c (Arbeitskopie) @@ -288,20 +288,10 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
-int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf) +int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = 1024 * flash->total_size; - spi_disable_blockprotect(); - /* Erase first. */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - msg_pdbg("total_size is %d\n", total_size); - return spi_write_chunked(flash, buf, 0, total_size, 256); + return spi_write_chunked(flash, buf, start, len, 256); }
#endif Index: flashrom-partial_write_spi_intermediate/wbsio_spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/wbsio_spi.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/wbsio_spi.c (Arbeitskopie) @@ -189,16 +189,14 @@ return read_memmapped(flash, buf, start, len); }
-int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf) +int wbsio_spi_write_1(struct flashchip *flash, uint8_t *buf, int start, int len) { - int size = flash->total_size * 1024; - - if (size > 1024 * 1024) { + if (flash->total_size * 1024 > 1024 * 1024) { msg_perr("%s: Winbond saved on 4 register bits so max chip size is 1024 KB!\n", __func__); return 1; }
- return spi_chip_write_1(flash, buf); + return spi_chip_write_1_new(flash, buf, start, len); }
#endif Index: flashrom-partial_write_spi_intermediate/dummyflasher.c =================================================================== --- flashrom-partial_write_spi_intermediate/dummyflasher.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/dummyflasher.c (Arbeitskopie) @@ -167,3 +167,12 @@ return spi_read_chunked(flash, buf, start, len, 64 * 1024); }
+/* Is is impossible to trigger this code path because dummyflasher probing will + * never be successful, and the current frontend refuses to write in that case. + * Other frontends may allow writing even for non-detected chips, though. + */ +int dummy_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) +{ + spi_disable_blockprotect(); + return spi_write_chunked(flash, buf, start, len, 256); +} Index: flashrom-partial_write_spi_intermediate/sb600spi.c =================================================================== --- flashrom-partial_write_spi_intermediate/sb600spi.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/sb600spi.c (Arbeitskopie) @@ -48,25 +48,10 @@ return spi_read_chunked(flash, buf, start, len, 8); }
-/* FIXME: SB600 can write 5 bytes per transaction. */ -int sb600_spi_write_1(struct flashchip *flash, uint8_t *buf) +int sb600_spi_write_256(struct flashchip *flash, uint8_t *buf, int start, int len) { - int total_size = flash->total_size * 1024; - int result = 0; - spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - msg_pinfo("Programming flash"); - result = spi_write_chunked(flash, buf, 0, total_size, 5); - msg_pinfo(" done.\n"); - return result; + return spi_write_chunked(flash, buf, start, len, 5); }
static void reset_internal_fifo_pointer(void) Index: flashrom-partial_write_spi_intermediate/ichspi.c =================================================================== --- flashrom-partial_write_spi_intermediate/ichspi.c (Revision 1079) +++ flashrom-partial_write_spi_intermediate/ichspi.c (Arbeitskopie) @@ -683,36 +683,15 @@ return spi_read_chunked(flash, buf, start, len, maxdata); }
-int ich_spi_write_256(struct flashchip *flash, uint8_t * buf) +int ich_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len) { - int i, ret = 0; - int total_size = flash->total_size * 1024; - int erase_size = 64 * 1024; int maxdata = 64;
if (spi_controller == SPI_CONTROLLER_VIA) maxdata = 16;
spi_disable_blockprotect(); - /* Erase first */ - msg_pinfo("Erasing flash before programming... "); - if (erase_flash(flash)) { - msg_perr("ERASE FAILED!\n"); - return -1; - } - msg_pinfo("done.\n"); - - msg_pinfo("Programming page: \n"); - for (i = 0; i < total_size / erase_size; i++) { - ret = spi_write_chunked(flash, buf + (i * erase_size), - i * erase_size, erase_size, maxdata); - if (ret) - break; - } - - msg_pinfo("\n"); - - return ret; + return spi_write_chunked(flash, buf, start, len, maxdata); }
int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, Index: flashrom-partial_write_spi_intermediate/chipdrivers.h =================================================================== --- flashrom-partial_write_spi_intermediate/chipdrivers.h (Revision 1079) +++ flashrom-partial_write_spi_intermediate/chipdrivers.h (Arbeitskopie) @@ -43,6 +43,8 @@ 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_read(struct flashchip *flash, uint8_t *buf, int start, int len); uint8_t spi_read_status_register(void); int spi_disable_blockprotect(void); @@ -51,7 +53,7 @@ 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(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(chipaddr bios);
Am Mittwoch, den 14.07.2010, 16:14 +0200 schrieb Carl-Daniel Hailfinger:
Since we're only handling a very special case (round_up_to_next_multiple_of_power_of_two_granularity) and use it only one time anyway, it doesn't really make sense to provide a separate function for this. Killed as per IRC discussion.
And the single line rewritten as per further IRC discussion. IRC discussion result:
lenhere = min(len, (start | 0xff) - start + 1);
If you use the bitwise round-up here: Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Still stands. Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Regards, Michael Karcher
On 14.07.2010 18:15, Michael Karcher wrote:
Am Mittwoch, den 14.07.2010, 16:14 +0200 schrieb Carl-Daniel Hailfinger:
Since we're only handling a very special case (round_up_to_next_multiple_of_power_of_two_granularity) and use it only one time anyway, it doesn't really make sense to provide a separate function for this. Killed as per IRC discussion.
And the single line rewritten as per further IRC discussion. IRC discussion result:
lenhere = min(len, (start | 0xff) - start + 1);If you use the bitwise round-up here: Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Still stands. Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Thanks, committed in r1080.
Regards, Carl-Daniel
On Sat, Jul 10, 2010 at 4:02 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
On 10.07.2010 21:20, Carl-Daniel Hailfinger wrote:
On 10.07.2010 20:15, Michael Karcher wrote:
Am Samstag, den 10.07.2010, 19:41 +0200 schrieb Carl-Daniel Hailfinger:
The spi_programmer array has a .write_256 member for every controller. In case the controller can't do anything except 1-byte writes, we set .write_256 = spi_chip_write_1_new
Not every controller. SPI_CONTROLLER_DUMMY really has NULL in this field. Your patch does not change that. And please kill the test if we are sure that write_256 is always non-null.
Right. I wanted to add a write_256 function to the dummy programmer anyway. I'll send an updated patch. Side note: If no SPI controller is selected, write_256 is empty as well, but I'd say that's not a problem because it should never happen.
If your plan is to base a new 1371 on top of this patch, I'm perfectly fine with these wrappers.
Thanks. That's indeed the plan.
New version, should hopefully address all comments.
Convert SPI chips to partial write, but wrap the write functions in a compat layer to allow converting the rest of flashrom later. I actually have patches for most of the remaining conversion, but I wanted to get this out and reviewed first.
Compile tested, but that's it. I would really appreciate it if someone could test this on a machine with SPI flash.
FWIW, I downloaded the patch from Patchwork and tried it out on my NM10 chipset / W25Q32 chip and it seemed to work fine. Log is attached.
-
Carl-Daniel Hailfinger -
David Hendricks -
Michael Karcher