On Fri, Oct 23, 2009 at 03:33:33PM +0200, Carl-Daniel Hailfinger wrote:
OK, completely rewritten attempt. I'm reusing Uwe's changelog.
Most of it looks good, but there are some changeѕ required.
- The checks have to be done later in the code, after the chip has been detected, so that we only check the sizes for the chip we actually find, not for all the chips we probe for. I moved the checks and simplified them a bit (I hope).
- Use printf instead of printf_debug so that users will see the error without having to use -V.
- Abort if the chip is bigger than the supported ROM decode size. Continuing will result in a non-bootable system in almost all cases. Adding a --force option later is fine with me, but I think the default should be to abort.
Attached is an updated patch with the above changes. I tested it on the Shuttle AK38N (all operations), output looks like this:
$ flashrom v0.9.1-r753 No coreboot table found. Found chipset "VIA VT8235", enabling flash write... OK. This chipset supports the following protocols: Non-SPI. Disabling flash write protection for board "Shuttle AK38N"... OK. Calibrating delay loop... OK. Found chip "SST SST39SF040" (512 KB, Parallel) at physical address 0xfff80000. Chipset/board/programmer only supports a max. size of 256 KB. ABORTING.
I also tested that with a 256KB chip all operations still work fine.
Additionally I tested a 512KB chip on ASUS P4B266 (FWH) with a (simulated) max. decode size of 256KB and 512KB to ensure that non-parallel chips also work OK with the code.
The updated patch is
Acked-by: Uwe Hermann uwe@hermann-uwe.de
Feel free to commit.
@@ -977,6 +1004,8 @@ size = flash->total_size * 1024; buf = (uint8_t *) calloc(size, sizeof(char));
- // FIXME: Check for decode size again?
- if (erase_it) { if (flash->tested & TEST_BAD_ERASE) { fprintf(stderr, "Erase is not working on this chip. ");
Not sure about this one. Why would an additional check be needed? According to my tests there's no need for additional checks.
Uwe.