flashrom July 2014

flashrom@flashrom.org

21 participants
70 discussions

AT45DB011D read not working
by The Raven 25 Aug '14

25 Aug '14

Read is not working, other things (yet) not tested. See log for details. It ends with a "Speicherzugriffsfehler" which means "Segmentation fault". I can do more tests if needed. Tell me what you need. :-) THX

3 9

[PATCH] Add en29lv640b
by Rudolf Marek 11 Aug '14

11 Aug '14

Hi Following patch adds the en29lv640b which needs special command sequences in 8 bit mode. Also, 8 bit programming needs actually 16bit double byte program. The chip is found on bifferboard. Please check the patch and let me know. Signed-off-by: Rudolf Marek <r.marek(a)assembler.cz> Thanks Rudolf

4 6

Flashed BIOS on M4N78 SE
by Dima Veselov 08 Aug '14

08 Aug '14

Hello! I have recently used flashrom on Asus M4N78SE motherboard. Flashrom advised me to send logs. Everything worked as expected. -- Sincerelly yours

2 3

[PATCH] Allow shifted JEDEC access
by Carl-Daniel Hailfinger 07 Aug '14

07 Aug '14

Some Parallel bus chips have a 16-bit mode and an 8-bit mode. They use normal JEDEC addresses for 16-bit mode and shifted addresses (by 1 bit) for 8-bit mode. Some programmers can access them in 16-bit mode, but on all flashrom-supported programmers so far, we access them in 8-bit mode. This means we have to shift the addresses. Untested, should work. This is a patch written shortly after r886 was committed, and it has been forward-ported. Missing from this patch is the deletion of now-unused code. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Index: flashrom-jedec_shifted/jedec.c =================================================================== --- flashrom-jedec_shifted/jedec.c (Revision 1818) +++ flashrom-jedec_shifted/jedec.c (Arbeitskopie) @@ -4,7 +4,7 @@ * Copyright (C) 2000 Silicon Integrated System Corporation * Copyright (C) 2006 Giampiero Giancipoli <gianci(a)email.it> * Copyright (C) 2006 coresystems GmbH <info(a)coresystems.de> - * Copyright (C) 2007 Carl-Daniel Hailfinger + * Copyright (C) 2007-2012 Carl-Daniel Hailfinger * Copyright (C) 2009 Sean Nelson <audiohacked(a)gmail.com> * * This program is free software; you can redistribute it and/or modify @@ -115,15 +115,18 @@ static void start_program_jedec_common(const struct flashctx *flash, unsigned int mask) { chipaddr bios = flash->virtual_memory; - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); - chip_writeb(flash, 0xA0, bios + (0x5555 & mask)); + int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) ? 1 : 0; + + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); + chip_writeb(flash, 0xA0, bios + ((0x5555 << shifted) & mask)); } static int probe_jedec_common(struct flashctx *flash, unsigned int mask) { chipaddr bios = flash->virtual_memory; const struct flashchip *chip = flash->chip; + int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) ? 1 : 0; uint8_t id1, id2; uint32_t largeid1, largeid2; uint32_t flashcontent1, flashcontent2; @@ -153,31 +156,31 @@ /* Reset chip to a clean slate */ if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET) { - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); if (probe_timing_exit) programmer_delay(10); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); if (probe_timing_exit) programmer_delay(10); } - chip_writeb(flash, 0xF0, bios + (0x5555 & mask)); + chip_writeb(flash, 0xF0, bios + ((0x5555 << shifted) & mask)); if (probe_timing_exit) programmer_delay(probe_timing_exit); /* Issue JEDEC Product ID Entry command */ - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); if (probe_timing_enter) programmer_delay(10); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); if (probe_timing_enter) programmer_delay(10); - chip_writeb(flash, 0x90, bios + (0x5555 & mask)); + chip_writeb(flash, 0x90, bios + ((0x5555 << shifted) & mask)); if (probe_timing_enter) programmer_delay(probe_timing_enter); /* Read product ID */ - id1 = chip_readb(flash, bios); - id2 = chip_readb(flash, bios + 0x01); + id1 = chip_readb(flash, bios + (0x00 << shifted)); + id2 = chip_readb(flash, bios + (0x01 << shifted)); largeid1 = id1; largeid2 = id2; @@ -196,14 +199,14 @@ /* Issue JEDEC Product ID Exit command */ if ((chip->feature_bits & FEATURE_RESET_MASK) == FEATURE_LONG_RESET) { - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); if (probe_timing_exit) programmer_delay(10); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); if (probe_timing_exit) programmer_delay(10); } - chip_writeb(flash, 0xF0, bios + (0x5555 & mask)); + chip_writeb(flash, 0xF0, bios + ((0x5555 << shifted) & mask)); if (probe_timing_exit) programmer_delay(probe_timing_exit); @@ -212,8 +215,8 @@ msg_cdbg(", id1 parity violation"); /* Read the product ID location again. We should now see normal flash contents. */ - flashcontent1 = chip_readb(flash, bios); - flashcontent2 = chip_readb(flash, bios + 0x01); + flashcontent1 = chip_readb(flash, bios + (0x00 << shifted)); + flashcontent2 = chip_readb(flash, bios + (0x01 << shifted)); /* Check if it is a continuation ID, this should be a while loop. */ if (flashcontent1 == 0x7F) { @@ -244,21 +247,23 @@ unsigned int pagesize, unsigned int mask) { chipaddr bios = flash->virtual_memory; + int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) ? 1 : 0; unsigned int delay_us = 0; + if(flash->chip->probe_timing != TIMING_ZERO) delay_us = 10; /* Issue the Sector Erase command */ - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x80, bios + (0x5555 & mask)); + chip_writeb(flash, 0x80, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); programmer_delay(delay_us); chip_writeb(flash, 0x30, bios + page); programmer_delay(delay_us); @@ -274,21 +279,23 @@ unsigned int blocksize, unsigned int mask) { chipaddr bios = flash->virtual_memory; + int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) ? 1 : 0; unsigned int delay_us = 0; + if(flash->chip->probe_timing != TIMING_ZERO) delay_us = 10; /* Issue the Sector Erase command */ - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x80, bios + (0x5555 & mask)); + chip_writeb(flash, 0x80, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); programmer_delay(delay_us); chip_writeb(flash, 0x50, bios + block); programmer_delay(delay_us); @@ -303,23 +310,25 @@ static int erase_chip_jedec_common(struct flashctx *flash, unsigned int mask) { chipaddr bios = flash->virtual_memory; + int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) ? 1 : 0; unsigned int delay_us = 0; + if(flash->chip->probe_timing != TIMING_ZERO) delay_us = 10; /* Issue the JEDEC Chip Erase command */ - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x80, bios + (0x5555 & mask)); + chip_writeb(flash, 0x80, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0xAA, bios + (0x5555 & mask)); + chip_writeb(flash, 0xAA, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x55, bios + (0x2AAA & mask)); + chip_writeb(flash, 0x55, bios + ((0x2AAA << shifted) & mask)); programmer_delay(delay_us); - chip_writeb(flash, 0x10, bios + (0x5555 & mask)); + chip_writeb(flash, 0x10, bios + ((0x5555 << shifted) & mask)); programmer_delay(delay_us); toggle_ready_jedec_slow(flash, bios); Index: flashrom-jedec_shifted/82802ab.c =================================================================== --- flashrom-jedec_shifted/82802ab.c (Revision 1818) +++ flashrom-jedec_shifted/82802ab.c (Arbeitskopie) @@ -44,7 +44,7 @@ { chipaddr bios = flash->virtual_memory; uint8_t id1, id2, flashcontent1, flashcontent2; - int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) != 0; + int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) ? 1 : 0; /* Reset to get a clean state */ chip_writeb(flash, 0xFF, bios); Index: flashrom-jedec_shifted/flashchips.c =================================================================== --- flashrom-jedec_shifted/flashchips.c (Revision 1818) +++ flashrom-jedec_shifted/flashchips.c (Arbeitskopie) @@ -4892,10 +4892,10 @@ .model_id = FUJITSU_MBM29F400BC, .total_size = 512, .page_size = 64 * 1024, - .feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_EITHER_RESET, + .feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_ADDR_2AA | FEATURE_EITHER_RESET, .tested = TEST_UNTESTED, - .probe = probe_m29f400bt, - .probe_timing = TIMING_IGNORED, /* routine doesn't use probe_timing (m29f400bt.c) */ + .probe = probe_jedec, + .probe_timing = TIMING_FIXME, .block_erasers = { { @@ -4905,13 +4905,13 @@ {32 * 1024, 1}, {64 * 1024, 7}, }, - .block_erase = erase_block_shifted_jedec, + .block_erase = erase_sector_jedec, }, { .eraseblocks = { {512 * 1024, 1} }, - .block_erase = erase_chip_block_shifted_jedec, + .block_erase = erase_chip_block_jedec, }, }, - .write = write_m29f400bt, + .write = write_jedec_1, .read = read_memmapped, .voltage = {4750, 5250}, /* 4.75-5.25V for type -55, others 4.5-5.5V */ }, @@ -4924,10 +4924,10 @@ .model_id = FUJITSU_MBM29F400TC, .total_size = 512, .page_size = 64 * 1024, - .feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_EITHER_RESET, + .feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_ADDR_AAA | FEATURE_EITHER_RESET, .tested = TEST_UNTESTED, - .probe = probe_m29f400bt, - .probe_timing = TIMING_IGNORED, /* routine doesn't use probe_timing (m29f400bt.c) */ + .probe = probe_jedec, + .probe_timing = TIMING_FIXME, .block_erasers = { { @@ -4937,13 +4937,13 @@ {8 * 1024, 2}, {16 * 1024, 1}, }, - .block_erase = erase_block_shifted_jedec, + .block_erase = erase_sector_jedec, }, { .eraseblocks = { {512 * 1024, 1} }, - .block_erase = erase_chip_block_shifted_jedec, + .block_erase = erase_chip_block_jedec, }, }, - .write = write_m29f400bt, + .write = write_jedec_1, .read = read_memmapped, .voltage = {4750, 5250}, /* 4.75-5.25V for type -55, others 4.5-5.5V */ }, @@ -11388,10 +11388,10 @@ .model_id = ST_M29F400BB, .total_size = 512, .page_size = 64 * 1024, - .feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_EITHER_RESET, + .feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_ADDR_2AA | FEATURE_EITHER_RESET, .tested = TEST_UNTESTED, - .probe = probe_m29f400bt, - .probe_timing = TIMING_IGNORED, /* routine doesn't use probe_timing (m29f400bt.c) */ + .probe = probe_jedec, + .probe_timing = TIMING_FIXME, .block_erasers = { { @@ -11401,13 +11401,13 @@ {32 * 1024, 1}, {64 * 1024, 7}, }, - .block_erase = erase_block_shifted_jedec, + .block_erase = erase_sector_jedec, }, { .eraseblocks = { {512 * 1024, 1} }, - .block_erase = erase_chip_block_shifted_jedec, + .block_erase = erase_chip_block_jedec, } }, - .write = write_m29f400bt, + .write = write_jedec_1, .read = read_memmapped, .voltage = {4500, 5500}, }, @@ -11420,10 +11420,10 @@ .model_id = ST_M29F400BT, .total_size = 512, .page_size = 64 * 1024, - .feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_EITHER_RESET, + .feature_bits = FEATURE_ADDR_SHIFTED | FEATURE_ADDR_2AA | FEATURE_EITHER_RESET, .tested = TEST_UNTESTED, - .probe = probe_m29f400bt, - .probe_timing = TIMING_IGNORED, /* routine doesn't use probe_timing (m29f400bt.c) */ + .probe = probe_jedec, + .probe_timing = TIMING_FIXME, .block_erasers = { { @@ -11433,13 +11433,13 @@ {8 * 1024, 2}, {16 * 1024, 1}, }, - .block_erase = erase_block_shifted_jedec, + .block_erase = erase_sector_jedec, }, { .eraseblocks = { {512 * 1024, 1} }, - .block_erase = erase_chip_block_shifted_jedec, + .block_erase = erase_chip_block_jedec, } }, - .write = write_m29f400bt, + .write = write_jedec_1, .read = read_memmapped, .voltage = {4500, 5500}, }, -- http://www.hailfinger.org/

2 3

Re: [flashrom] [PATCH] Add support for S25FL128P and S25FL128P......0 chips.
by Stefan Tauner 04 Aug '14

04 Aug '14

Thanks for the test, there are two issues here. One is that I swapped the erase block sizes of the P models. This is clearly shown in your logs. The ...0 model should have 64 kB erase blocks, but does not: > Trying erase function 0... 0x000000-0x03ffff:EFAILED at 0x00010000! Expected=0xff, Found=0xe5 0x03ffff is 262143 in decimal... I have fixed that locally, but this is not the main problem here. The chip erase and the subsequent write should have worked, and your second try with the definition of the ...1 model too (because it contains the right block size). But both failed and I don't understand why yet. The first failed byte is equal in both cases but not all of them are (a different number of bytes fail in the two cases). This indicates that the behavior might be indeterministic. The contents of the "rog.bin" file were equal for both tries, right? What happens if you retry the second test again? /usr/src/flashrompatch/flashrom -p usbblaster_spi -o write2.log -c S25FL128P......1 -w rog.bin If you can not reproduce exactly the same output, i.e. Verifying flash... FAILED at 0x0000005b! Expected=0xd3, Found=0xfb, failed byte count from 0x00000000-0x00ffffff: 0xaf66ea Then we are on the right track. In that case I would ask you how the chip is connected exactly, especially how WP# and HOLD# are driven. PS: You dont need to install flashrom after recompiling. You can execute the built binary in the source directory (make; ./flashrom ...). -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

3 4

ichspi: fix missing set_addr and crossing of 256 boundary
by Vladimir 'φ-coder/phcoder' Serbinenko 03 Aug '14

03 Aug '14

Accoring to PCH documentation crossing 256-byte boundary is invalid and causes wraparound. Please apply attached patch

2 1

New mainboard - Shuttle AB61
by olofolleola4_pub＠riseup.net 02 Aug '14

02 Aug '14

Hello! I've tested flashrom v0.9.7-r1832 with this mainboard. The factory BIOS image had this: DMI string chassis-type: "Unknown". When I tried flashrom with the factory image applied to the flash chip, I needed to specify this line 'internal:laptop=this_is_not_a_laptop' When I updated the BIOS for this mainboard to the latest version with flashrom, the BIOS image now have this instead: DMI string chassis-type: "Desktop". This means that the latest BIOS image doesn't require any extra information (internal:laptop=this_is_not_a_laptop) specified with flashrom. This mainboard works fine. Original flash chip on this mainboard: SST SST39SF020A Mainboard URL: http://www.shuttle.eu/_archive/older/de/ab61.htm I've attached several logs in this e-mail.

2 1

Re: [flashrom] [PATCH] Reorder block_erasers to favor large block erasers over small block erasers
by Stefan Tauner 01 Aug '14

01 Aug '14

On Thu, 31 Jul 2014 12:08:11 +0000 Pablo Cases <pablo.cases(a)flatfrog.com> wrote: > This patch reorders the block_erasers by increasing block count with the assumption that the fewer block erase commands that have to be issued the lower the erase times. E.g. this will favor chip erase instructions (as their block count is 1) even though they are generally listed last in the block_erasers list for each chip in flashchips.c > > As an example this reduces the erase time for a W25Q64.V chip from 14 minutes to 8 seconds as a single Chip Erase (60h) instruction will be used instead of issuing a sequence of 2048 Sector Erase (20h) commands. > Hi Pablo, i think there is one major problem with this: it breaks partial updates. If the data on the chip and the data requested (i.e. the data residing in the file) are equal, flashrom skips the whole write process. If there are changes, flashrom tries the smallest block size first and modifies only those blocks that are different. AFAICS your patch would reverse the order and hence any change would require to erase and write the flash completely even if only one byte has changed and the flash chip (and programmer) support smaller blocks. The partial updates are not only a performance strategy: If some blocks are write-protected without a (working) way to unlock them your patch would introduce a regression in the case only unprotected blocks are to be modified. Your benchmark seems to measure an extreme case. What programmer do you use, something USB-based I guess, a bus pirate maybe? Even for that 14 mins seem to be quite slow (almost half a second for one block, which is btw about the maximum erase time stated in the datasheet, although that is probably not really the bottleneck). Also, the 8 seconds are half of the specified typical chip erase time(!). No idea how you managed to do that. :) I was looking at revision C of the W25Q64CV datasheet for the timing values. To sum up: I don't think this is the right way to do this and a proper implementation would require quite some effort (pre-calculating necessary erases, estimating the best block eraser(s) to use etc). -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

2 1

[PATCH] Clean up action parameter handling
by Nate Case 31 Jul '14

31 Jul '14

Condense 'read_it', 'write_it', 'erase_it', 'verify_it', and 'force' into a single 'flags' parameter. This makes the function signatures much more sane, while also making it much easier to add new features/flags in the future. Signed-off-by: Nate Case <ncase(a)xes-inc.com> --- cli_classic.c | 34 +++++++++++++++++----------------- flash.h | 13 ++++++++++++- flashrom.c | 44 +++++++++++++++++++------------------------- 3 files changed, 48 insertions(+), 43 deletions(-) Index: flashrom.c =================================================================== --- flashrom.c (revision 1832) +++ flashrom.c (working copy) @@ -1850,30 +1850,26 @@ } } -/* FIXME: This function signature needs to be improved once doit() has a better - * function signature. - */ -int chip_safety_check(const struct flashctx *flash, int force, int read_it, int write_it, int erase_it, - int verify_it) +int chip_safety_check(const struct flashctx *flash, int flags) { const struct flashchip *chip = flash->chip; - if (!programmer_may_write && (write_it || erase_it)) { + if (!programmer_may_write && (flags & (ACTION_FLAG_WRITE | ACTION_FLAG_ERASE))) { msg_perr("Write/erase is not working yet on your programmer in " "its current configuration.\n"); /* --force is the wrong approach, but it's the best we can do * until the generic programmer parameter parser is merged. */ - if (!force) + if (!(flags & ACTION_FLAG_FORCE)) return 1; msg_cerr("Continuing anyway.\n"); } - if (read_it || erase_it || write_it || verify_it) { + if (flags & ACTION_FLAGS_RWEV) { /* Everything needs read. */ if (chip->tested.read == BAD) { msg_cerr("Read is not working on this chip. "); - if (!force) + if (!(flags & ACTION_FLAG_FORCE)) return 1; msg_cerr("Continuing anyway.\n"); } @@ -1883,7 +1879,7 @@ return 1; } } - if (erase_it || write_it) { + if (flags & (ACTION_FLAG_ERASE | ACTION_FLAG_WRITE)) { /* Write needs erase. */ if (chip->tested.erase == NA) { msg_cerr("Erase is not possible on this chip.\n"); @@ -1891,7 +1887,7 @@ } if (chip->tested.erase == BAD) { msg_cerr("Erase is not working on this chip. "); - if (!force) + if (!(flags & ACTION_FLAG_FORCE)) return 1; msg_cerr("Continuing anyway.\n"); } @@ -1901,14 +1897,14 @@ return 1; } } - if (write_it) { + if (flags & ACTION_FLAG_WRITE) { if (chip->tested.write == NA) { msg_cerr("Write is not possible on this chip.\n"); return 1; } if (chip->tested.write == BAD) { msg_cerr("Write is not working on this chip. "); - if (!force) + if (!(flags & ACTION_FLAG_FORCE)) return 1; msg_cerr("Continuing anyway.\n"); } @@ -1921,19 +1917,17 @@ return 0; } -/* This function signature is horrible. We need to design a better interface, - * but right now it allows us to split off the CLI code. - * Besides that, the function itself is a textbook example of abysmal code flow. +/* + * FIXME: Abysmal code flow */ -int doit(struct flashctx *flash, int force, const char *filename, int read_it, - int write_it, int erase_it, int verify_it) +int doit(struct flashctx *flash, const char *filename, int flags) { uint8_t *oldcontents; uint8_t *newcontents; int ret = 0; unsigned long size = flash->chip->total_size * 1024; - if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) { + if (chip_safety_check(flash, flags)) { msg_cerr("Aborting.\n"); return 1; } @@ -1949,7 +1943,7 @@ if (flash->chip->unlock) flash->chip->unlock(flash); - if (read_it) { + if (flags & ACTION_FLAG_READ) { return read_flash_to_file(flash, filename); } @@ -1973,7 +1967,7 @@ * before we can write. */ - if (erase_it) { + if (flags & ACTION_FLAG_ERASE) { /* FIXME: Do we really want the scary warning if erase failed? * After all, after erase the chip is either blank or partially * blank or it has the old contents. A blank chip won't boot, @@ -1987,7 +1981,7 @@ goto out; } - if (write_it || verify_it) { + if (flags & (ACTION_FLAG_WRITE | ACTION_FLAG_VERIFY)) { if (read_buf_from_file(newcontents, size, filename)) { ret = 1; goto out; @@ -2026,7 +2020,7 @@ // //////////////////////////////////////////////////////////// - if (write_it) { + if (flags & ACTION_FLAG_WRITE) { if (erase_and_write_flash(flash, oldcontents, newcontents)) { msg_cerr("Uh oh. Erase/write failed. Checking if anything has changed.\n"); msg_cinfo("Reading current flash chip contents... "); @@ -2047,10 +2041,10 @@ } /* Verify only if we either did not try to write (verify operation) or actually changed something. */ - if (verify_it && (!write_it || !all_skipped)) { + if ((flags & ACTION_FLAG_VERIFY) && (!(flags & ACTION_FLAG_WRITE) || !all_skipped)) { msg_cinfo("Verifying flash... "); - if (write_it) { + if (flags & ACTION_FLAG_WRITE) { /* Work around chips which need some time to calm down. */ programmer_delay(1000*1000); ret = verify_range(flash, newcontents, 0, size); Index: cli_classic.c =================================================================== --- cli_classic.c (revision 1832) +++ cli_classic.c (working copy) @@ -98,11 +98,11 @@ struct flashctx *fill_flash; const char *name; int namelen, opt, i, j; - int startchip = -1, chipcount = 0, option_index = 0, force = 0; + int startchip = -1, chipcount = 0, option_index = 0; #if CONFIG_PRINT_WIKI == 1 int list_supported_wiki = 0; #endif - int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0; + int flags = 0; int dont_verify_it = 0, list_supported = 0, operation_specified = 0; enum programmer prog = PROGRAMMER_INVALID; int ret = 0; @@ -156,7 +156,7 @@ cli_classic_abort_usage(); } filename = strdup(optarg); - read_it = 1; + flags |= ACTION_FLAG_READ; break; case 'w': if (++operation_specified > 1) { @@ -165,7 +165,7 @@ cli_classic_abort_usage(); } filename = strdup(optarg); - write_it = 1; + flags |= ACTION_FLAG_WRITE; break; case 'v': //FIXME: gracefully handle superfluous -v @@ -179,10 +179,10 @@ cli_classic_abort_usage(); } filename = strdup(optarg); - verify_it = 1; + flags |= ACTION_FLAG_VERIFY; break; case 'n': - if (verify_it) { + if (flags & ACTION_FLAG_VERIFY) { fprintf(stderr, "--verify and --noverify are mutually exclusive. Aborting.\n"); cli_classic_abort_usage(); } @@ -202,10 +202,10 @@ "specified. Aborting.\n"); cli_classic_abort_usage(); } - erase_it = 1; + flags |= ACTION_FLAG_ERASE; break; case 'f': - force = 1; + flags |= ACTION_FLAG_FORCE; break; case 'l': if (layoutfile) { @@ -327,7 +327,7 @@ cli_classic_abort_usage(); } - if ((read_it | write_it | verify_it) && check_filename(filename, "image")) { + if ((flags & ACTION_FLAGS_RWV) && check_filename(filename, "image")) { cli_classic_abort_usage(); } if (layoutfile && check_filename(layoutfile, "layout")) { @@ -369,7 +369,7 @@ ret = 1; goto out; } - if (layoutfile != NULL && !write_it) { + if (layoutfile != NULL && !(flags & ACTION_FLAG_WRITE)) { msg_gerr("Layout files are currently supported for write operations only.\n"); ret = 1; goto out; @@ -447,11 +447,11 @@ goto out_shutdown; } else if (!chipcount) { msg_cinfo("No EEPROM/flash device found.\n"); - if (!force || !chip_to_probe) { + if (!(flags & ACTION_FLAG_FORCE) || !chip_to_probe) { msg_cinfo("Note: flashrom can never write if the flash chip isn't found " "automatically.\n"); } - if (force && read_it && chip_to_probe) { + if ((flags & ACTION_FLAG_FORCE) && (flags & ACTION_FLAG_READ) && chip_to_probe) { struct registered_master *mst; int compatible_masters = 0; msg_cinfo("Force read (-f -r -c) requested, pretending the chip is there:\n"); @@ -502,27 +502,27 @@ check_chip_supported(fill_flash->chip); size = fill_flash->chip->total_size * 1024; - if (check_max_decode(fill_flash->mst->buses_supported & fill_flash->chip->bustype, size) && (!force)) { + if (check_max_decode(fill_flash->mst->buses_supported & fill_flash->chip->bustype, size) && !(flags & ACTION_FLAG_FORCE)) { msg_cerr("Chip is too big for this programmer (-V gives details). Use --force to override.\n"); ret = 1; goto out_shutdown; } - if (!(read_it | write_it | verify_it | erase_it)) { + if (!(flags & ACTION_FLAGS_RWEV)) { msg_ginfo("No operations were specified.\n"); goto out_shutdown; } /* Always verify write operations unless -n is used. */ - if (write_it && !dont_verify_it) - verify_it = 1; + if ((flags & ACTION_FLAG_WRITE) && !dont_verify_it) + flags |= ACTION_FLAG_VERIFY; /* FIXME: We should issue an unconditional chip reset here. This can be * done once we have a .reset function in struct flashchip. * Give the chip time to settle. */ programmer_delay(100000); - ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it); + ret |= doit(fill_flash, filename, flags); out_shutdown: programmer_shutdown(); Index: flash.h =================================================================== --- flash.h (revision 1832) +++ flash.h (working copy) @@ -121,6 +121,17 @@ #define FEATURE_OTP (1 << 8) #define FEATURE_QPI (1 << 9) +#define ACTION_FLAG_READ (1 << 1) +#define ACTION_FLAG_WRITE (1 << 2) +#define ACTION_FLAG_ERASE (1 << 3) +#define ACTION_FLAG_VERIFY (1 << 4) +#define ACTION_FLAG_FORCE (1 << 5) + +#define ACTION_FLAGS_RWEV (ACTION_FLAG_READ | ACTION_FLAG_WRITE | \ + ACTION_FLAG_ERASE | ACTION_FLAG_VERIFY) +#define ACTION_FLAGS_RWV (ACTION_FLAG_READ | ACTION_FLAG_WRITE | \ + ACTION_FLAG_VERIFY) + enum test_state { OK = 0, NT = 1, /* Not tested */ @@ -268,7 +279,7 @@ void print_banner(void); void list_programmers_linebreak(int startcol, int cols, int paren); int selfcheck(void); -int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it); +int doit(struct flashctx *flash, const char *filename, int flags); int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename); int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename);

2 7

[PATCH] Reorder block_erasers to favor large block erasers over small block erasers
by Pablo Cases 31 Jul '14

31 Jul '14

This patch reorders the block_erasers by increasing block count with the assumption that the fewer block erase commands that have to be issued the lower the erase times. E.g. this will favor chip erase instructions (as their block count is 1) even though they are generally listed last in the block_erasers list for each chip in flashchips.c As an example this reduces the erase time for a W25Q64.V chip from 14 minutes to 8 seconds as a single Chip Erase (60h) instruction will be used instead of issuing a sequence of 2048 Sector Erase (20h) commands. Signed-off-by: Pablo Cases <pablo.cases at flatfrog.com> Index: flashrom.c =================================================================== --- flashrom.c (revision 1832) +++ flashrom.c (working copy) @@ -1432,22 +1432,20 @@ return 0; } -static int check_block_eraser(const struct flashctx *flash, int k, int log) +static int check_block_eraser2(struct block_eraser* eraser, int log) { - struct block_eraser eraser = flash->chip->block_erasers[k]; - - if (!eraser.block_erase && !eraser.eraseblocks[0].count) { + if (!eraser->block_erase && !eraser->eraseblocks[0].count) { if (log) msg_cdbg("not defined. "); return 1; } - if (!eraser.block_erase && eraser.eraseblocks[0].count) { + if (!eraser->block_erase && eraser->eraseblocks[0].count) { if (log) msg_cdbg("eraseblock layout is known, but matching " "block erase function is not implemented. "); return 1; } - if (eraser.block_erase && !eraser.eraseblocks[0].count) { + if (eraser->block_erase && !eraser->eraseblocks[0].count) { if (log) msg_cdbg("block erase function found, but " "eraseblock layout is not defined. "); @@ -1457,6 +1455,38 @@ return 0; } +static int check_block_eraser(const struct flashctx *flash, int k, int log) +{ + struct block_eraser eraser = flash->chip->block_erasers[k]; + return check_block_eraser2(&eraser, log); +} + +static struct block_eraser* get_block_erasers(struct flashctx* flash) +{ + return flash->chip->block_erasers; +} + +static int compare_by_block_count(const void *a, const void *b) +{ + struct block_eraser* bea = (struct block_eraser*)a; + struct block_eraser* beb = (struct block_eraser*)b; + + if (check_block_eraser2(bea, 0) && check_block_eraser2(beb, 0)) + // both erasers are invalid + return 0; + else if (!check_block_eraser2(bea, 0) && check_block_eraser2(beb, 0)) + // first eraser valid, second eraser invalid + return -1; + else if (check_block_eraser2(bea, 0) && !check_block_eraser2(beb, 0)) + // second eraser valid, first eraser invalid + return 1; + else { + // both erasers are valid, ignore that some chips have + // differently sized erase regions + return bea->eraseblocks[0].count - beb->eraseblocks[0].count; + } +} + int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents) { int k, ret = 1; @@ -1463,6 +1493,7 @@ uint8_t *curcontents; unsigned long size = flash->chip->total_size * 1024; unsigned int usable_erasefunctions = count_usable_erasers(flash); + struct block_eraser* sorted_erasers; msg_cinfo("Erasing and writing flash chip... "); curcontents = malloc(size); @@ -1473,6 +1504,22 @@ /* Copy oldcontents to curcontents to avoid clobbering oldcontents. */ memcpy(curcontents, oldcontents, size); + /* only get the pointer to the block_erasers, + i.e. block_erasers are sorted in place at their original location, + hence the order might no longer match the order defined in + flashchips.c */ + sorted_erasers = get_block_erasers(flash); + qsort(sorted_erasers, NUM_ERASEFUNCTIONS, sizeof(struct block_eraser), + compare_by_block_count); + + /* block_erasers are now sorted by ascending block count + with the assumption on that a low block count results in lower + erase times than a high block counts */ + msg_gspew("\nSorted erase functions listed by ascending block count:\n"); + msg_gspew("Block Size\tBlock Count\n"); + for (k = 0; k < NUM_ERASEFUNCTIONS; k++) + msg_gspew("%d\t\t%d\n", sorted_erasers[k].eraseblocks[0].size, sorted_erasers[k].eraseblocks[0].count); + for (k = 0; k < NUM_ERASEFUNCTIONS; k++) { if (k != 0) msg_cinfo("Looking for another erase function.\n"); ---------------------------------------------------------------------- Pablo Cases, M.Sc. Senior Software Engineer FlatFrog Laboratories AB

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom July 2014