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);