On Sat, 16 Jul 2011 02:13:44 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 16.04.2011 13:37 schrieb Stefan Tauner:
i would change the check_block_eraser method to return different values for "not suitable" and "not defined/no more methods"
(there cant be holes in the eraser array, right?).
this is wrong. there can be holes. anyway, my current approach looks similar to yours.
That would fix the bug and simplify the code at the cost of additional lines in the log for the following case: erase method 0 is suitable, but fails (no change) ... suitable but fails (no change) erase method n is not suitable (my version: not printed, your version: printed) ...not suitable (see above) erase method m is not defined (no change) ...not defined (no change) END OF ARRAY
i am not 100% sure what you mean. i did not specify how/when i would print the state of erase functions... if the behavior of the attached patch has this problem please specify.
A few LPC/FWH chips have a chip-erase function which is only available in parallel programmer mode (i.e. not in a mainboard), and those would get those additional lines.
how and where are those en/disabled?
the first attached patch includes a fix similar to yours, but a little bit extended. some modification i have done to test it are in a second patch (not for merge of course). i have done testing with my spi-capable serprog programmer. is the dummy programmer able to simulate such things too? i didnt bother to look :)
my patch changes the output a bit... to the better imho. - it removes a line break after each call of walk_eraseregions. what's the purpose of this? it makes holes in the log in the case something in walk_eraseregions ends with \n (lots of things do). this does not fix all problems though... see -EV output below. - it informs the user about the (potentially long) read of the whole chip after an erase failed. this is very important for slow programmers imho. - it adds a message to check_block_eraser in the case of success. since each caller can decide if any output is printed in check_block_eraser this is ok and we can improve the user experience in verbose mode.
it also changes the long comment because it is no longer true. we may wanna make it true again by checking the other erasers too and continuing the loop after checking for walk_eraseregions > 0 afterwards, but with your/mine patch it is just not true anymore.
below is the output of erase runs with different verbosity levels (note that the last erase function succeeds, so there is no test for the actual oob problem... but that is obviously fixed anyway... and i have tested this too.):
./flashrom -pserprog:dev=/dev/ttyU2flasher:115200 -c W25Q32 -E flashrom v0.9.3-r1372 on Linux 2.6.35-30-generic (x86_64), built with libpci 3.1.7, GCC 4.4.5, little endian flashrom is free software, get the source code at http://www.flashrom.org
Calibrating delay loop... OK. serprog: Programmer name is "atmegaXXu2 " Found chip "Winbond W25Q32" (4096 kB, SPI) on serprog. Erasing and writing flash chip... ERASE FAILED! Reading current chip content... <very long pause>done. Trying next erase function. Done.
without the new "Reading current chip content..." there would be only "ERASE FAILED!" be printed for a quite long time. i think that's scary for users, and that they may hit ctrl+c because of it.
./flashrom -pserprog:dev=/dev/ttyU2flasher:115200 -c W25Q32 -EV flashrom v0.9.3-r1372 on Linux 2.6.35-30-generic (x86_64), built with libpci 3.1.7, GCC 4.4.5, little endian flashrom is free software, get the source code at http://www.flashrom.org
Calibrating delay loop... OS timer resolution is 1 usecs, 1454M loops per second, 10 myus = 10 us, 100 myus = 99 us, 1000 myus = 994 us, 10000 myus = 10003 us, 4 myus = 4 us, OK. Initializing serprog programmer serprog: connected - attempting to synchronize . serprog: Synchronized serprog: Interface version ok. serprog: Bus support: parallel=off, LPC=off, FWH=off, SPI=on serprog: Maximum write-n length is 16777215 serprog: Maximum read-n length is 16777215 serprog: Programmer name is "atmegaXXu2 " serprog: Serial buffer size is 65535 Probing for Winbond W25Q32, 4096 kB: probe_spi_rdid_generic: id1 0xef, id2 0x4016 Chip status register is 00 Found chip "Winbond W25Q32" (4096 kB, SPI) on serprog. Note: using internal_delay, because the attached programmer does not support the delay opcode. Erasing and writing flash chip... Looking at erase function 0... looks ok. Trying... 0x000000-0x000fff:ENote: using internal_delay, because the attached programmer does not support the delay opcode. ERASE FAILED!
Reading current chip content... <very long pause>done. Trying next erase function. Looking at erase function 1... block erase function found, but eraseblock layout is not defined. Looking for another erase function. Looking at erase function 2... eraseblock layout is known, but matching block erase function is not implemented. Looking for another erase function. Looking at erase function 3... not defined. Looking for another erase function. Looking at erase function 4... looks ok. Trying... 0x000000-0x3fffff:S Done.
nice verbose output explaining what really happens.. the extra line break is from inside the erase_and_write_block_helper i think.
./flashrom -pserprog:dev=/dev/ttyU2flasher:115200 -c W25Q32 -EVV flashrom v0.9.3-r1372 on Linux 2.6.35-30-generic (x86_64), built with libpci 3.1.7, GCC 4.4.5, little endian flashrom is free software, get the source code at http://www.flashrom.org
Calibrating delay loop... OS timer resolution is 1 usecs, 1451M loops per second, 10 myus = 10 us, 100 myus = 99 us, 1000 myus = 1009 us, 10000 myus = 10244 us, 4 myus = 4 us, OK. Initializing serprog programmer serprog: connected - attempting to synchronize . serprog: Synchronized serprog: Interface version ok. serprog: Bus support: parallel=off, LPC=off, FWH=off, SPI=on serprog: Maximum write-n length is 16777215 serprog: Maximum read-n length is 16777215 serprog: Programmer name is "atmegaXXu2 " serprog: Serial buffer size is 65535 Probing for Winbond W25Q32, 4096 kB: serprog_spi_send_command, writecnt=1, readcnt=3 RDID returned 0xef 0x40 0x16. probe_spi_rdid_generic: id1 0xef, id2 0x4016 serprog_spi_send_command, writecnt=1, readcnt=2 Chip status register is 00 Found chip "Winbond W25Q32" (4096 kB, SPI) on serprog. serprog_delay usecs=100000 Note: using internal_delay, because the attached programmer does not support the delay opcode. serprog_spi_send_command, writecnt=1, readcnt=2 Erasing and writing flash chip... Looking at erase function 0... looks ok. Trying... 0x000000-0x000fff:Eserprog_spi_send_command, writecnt=1, readcnt=0 serprog_spi_send_command, writecnt=4, readcnt=0 serprog_spi_send_command, writecnt=1, readcnt=2 serprog_delay usecs=10000 Note: using internal_delay, because the attached programmer does not support the delay opcode. serprog_spi_send_command, writecnt=1, readcnt=2 serprog_spi_send_command, writecnt=4, readcnt=4096 ERASE FAILED!
Reading current chip content... serprog_spi_send_command, writecnt=4, readcnt=4194304<very long pause> done. Trying next erase function. Looking at erase function 1... block erase function found, but eraseblock layout is not defined. Looking for another erase function. Looking at erase function 2... eraseblock layout is known, but matching block erase function is not implemented. Looking for another erase function. Looking at erase function 3... not defined. Looking for another erase function. Looking at erase function 4... looks ok. Trying... 0x000000-0x3fffff:S Done. serprog_shutdown