Use the register mapping feature bit. All functions which just call probe_jedec and then map flash registers are replaced by probe_jedec. All functions which call probe_jedec, map flash registers and do something else can at least eliminate mapping flash registers. Fix logic inversion in probe_jedec to map flash registers on success instead of on failure. Change a few TIMING_IGNORED to TIMING_FIXME where probe_jedec is used.
Total savings: One probe function simplified, three probe functions eliminated.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-registermap_featurebit/pm49fl00x.c =================================================================== --- flashrom-registermap_featurebit/pm49fl00x.c (Revision 834) +++ flashrom-registermap_featurebit/pm49fl00x.c (Arbeitskopie) @@ -36,16 +36,6 @@ } }
-int probe_49fl00x(struct flashchip *flash) -{ - int ret = probe_jedec(flash); - - if (ret == 1) - map_flash_registers(flash); - - return ret; -} - int erase_49fl00x(struct flashchip *flash) { int i; Index: flashrom-registermap_featurebit/jedec.c =================================================================== --- flashrom-registermap_featurebit/jedec.c (Revision 834) +++ flashrom-registermap_featurebit/jedec.c (Arbeitskopie) @@ -189,13 +189,13 @@ printf_debug(", id2 is normal flash content");
printf_debug("\n"); - if (largeid1 == flash->manufacture_id && largeid2 == flash->model_id) - return 1; + if (largeid1 != flash->manufacture_id || largeid2 != flash->model_id) + return 0;
if (flash->feature_bits & FEATURE_REGISTERMAP) map_flash_registers(flash);
- return 0; + return 1; }
int erase_sector_jedec_common(struct flashchip *flash, unsigned int page, Index: flashrom-registermap_featurebit/flashchips.c =================================================================== --- flashrom-registermap_featurebit/flashchips.c (Revision 834) +++ flashrom-registermap_featurebit/flashchips.c (Arbeitskopie) @@ -827,7 +827,7 @@ .total_size = 64, .page_size = 128, .tested = TEST_OK_PRW, - .probe = probe_jedec, + .probe = probe_jedec, .probe_timing = 10000, /* 10mS, Enter=Exec */ .erase = NULL, .block_erasers = @@ -1252,8 +1252,9 @@ .model_id = AMIC_A49LF040A, .total_size = 512, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PREW, - .probe = probe_49fl00x, + .probe = probe_jedec, .probe_timing = TIMING_ZERO, /* routine is wrapper to probe_jedec (pm49fl00x.c) */ .erase = erase_49fl00x, .write = write_49fl00x, @@ -2473,8 +2474,9 @@ .model_id = PMC_49FL002, .total_size = 256, .page_size = 16 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PREW, - .probe = probe_49fl00x, + .probe = probe_jedec, .probe_timing = TIMING_ZERO, /* routine is wrapper to probe_jedec (pm49fl00x.c) */ .erase = erase_49fl00x, .write = write_49fl00x, @@ -2489,8 +2491,9 @@ .model_id = PMC_49FL004, .total_size = 512, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PREW, - .probe = probe_49fl00x, + .probe = probe_jedec, .probe_timing = TIMING_ZERO, /* routine is wrapper to probe_jedec (pm49fl00x.c) */ .erase = erase_49fl00x, .write = write_49fl00x, @@ -2965,6 +2968,7 @@ .model_id = SST_49LF002A, .total_size = 256, .page_size = 16 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PREW, .probe = probe_sst_fwhub, .probe_timing = 1, /* 150 ns | routine is wrapper to probe_jedec (sst_fwhub.c) */ @@ -2981,6 +2985,7 @@ .model_id = SST_49LF003A, .total_size = 384, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PR, .probe = probe_sst_fwhub, .probe_timing = 1, /* 150 ns | routine is wrapper to probe_jedec (sst_fwhub.c) */ @@ -3000,6 +3005,7 @@ .model_id = SST_49LF004A, .total_size = 512, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PREW, .probe = probe_sst_fwhub, .probe_timing = 1, /* 150 ns | routine is wrapper to probe_jedec (sst_fwhub.c) */ @@ -3045,6 +3051,7 @@ .model_id = SST_49LF008A, .total_size = 1024, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PREW, .probe = probe_sst_fwhub, .probe_timing = 1, /* 150 ns | routine is wrapper to probe_jedec (sst_fwhub.c) */ @@ -3141,6 +3148,7 @@ .model_id = SST_49LF040B, .total_size = 512, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PREW, .probe = probe_sst_fwhub, .probe_timing = 1, /* 150ns | routine is wrapper to probe_jedec (sst_fwhub.c) */ @@ -3483,8 +3491,9 @@ .model_id = ST_M50FLW040A, .total_size = 512, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_UNTESTED, - .probe = probe_stm50flw0x0x, + .probe = probe_jedec, .probe_timing = TIMING_IGNORED, /* routine don't use probe_timing (stm50flw0x0x.c) */ .erase = erase_stm50flw0x0x, .write = write_stm50flw0x0x, @@ -3499,8 +3508,9 @@ .model_id = ST_M50FLW040B, .total_size = 512, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_UNTESTED, - .probe = probe_stm50flw0x0x, + .probe = probe_jedec, .probe_timing = TIMING_IGNORED, /* routine don't use probe_timing (stm50flw0x0x.c) */ .erase = erase_stm50flw0x0x, .write = write_stm50flw0x0x, @@ -3515,8 +3525,9 @@ .model_id = ST_M50FLW080A, .total_size = 1024, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PREW, - .probe = probe_stm50flw0x0x, + .probe = probe_jedec, .probe_timing = TIMING_IGNORED, /* routine don't use probe_timing (stm50flw0x0x.c) */ .erase = erase_stm50flw0x0x, .write = write_stm50flw0x0x, @@ -3531,8 +3542,9 @@ .model_id = ST_M50FLW080B, .total_size = 1024, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_UNTESTED, - .probe = probe_stm50flw0x0x, + .probe = probe_jedec, .probe_timing = TIMING_IGNORED, /* routine don't use probe_timing (stm50flw0x0x.c) */ .erase = erase_stm50flw0x0x, .write = write_stm50flw0x0x, @@ -4027,9 +4039,10 @@ .model_id = W_39V080FA, .total_size = 1024, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_OK_PREW, - .probe = probe_winbond_fwhub, - .probe_timing = TIMING_IGNORED, /* routine don't use probe_timing (w39v080fa.c) */ + .probe = probe_jedec, + .probe_timing = TIMING_FIXME, .erase = erase_winbond_fwhub, .write = write_winbond_fwhub, .read = read_memmapped, @@ -4043,9 +4056,10 @@ .model_id = W_39V080FA_DM, .total_size = 512, .page_size = 64 * 1024, + .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_UNTESTED, - .probe = probe_winbond_fwhub, - .probe_timing = TIMING_IGNORED, /* routine don't use probe_timing (w39v080fa.c) */ + .probe = probe_jedec, + .probe_timing = TIMING_FIXME, .erase = erase_winbond_fwhub, .write = write_winbond_fwhub, .read = read_memmapped, Index: flashrom-registermap_featurebit/stm50flw0x0x.c =================================================================== --- flashrom-registermap_featurebit/stm50flw0x0x.c (Revision 834) +++ flashrom-registermap_featurebit/stm50flw0x0x.c (Arbeitskopie) @@ -31,18 +31,6 @@ #include "flash.h" #include "flashchips.h"
-int probe_stm50flw0x0x(struct flashchip *flash) -{ - int result = probe_jedec(flash); - - if (!result) - return result; - - map_flash_registers(flash); - - return 1; -} - static void wait_stm50flw0x0x(chipaddr bios) { chip_writeb(0x70, bios); Index: flashrom-registermap_featurebit/sst_fwhub.c =================================================================== --- flashrom-registermap_featurebit/sst_fwhub.c (Revision 834) +++ flashrom-registermap_featurebit/sst_fwhub.c (Arbeitskopie) @@ -89,8 +89,6 @@ if (probe_jedec(flash) == 0) return 0;
- map_flash_registers(flash); - for (i = 0; i < flash->total_size * 1024; i += flash->page_size) check_sst_fwhub_block_lock(flash, i);
Index: flashrom-registermap_featurebit/w39v080fa.c =================================================================== --- flashrom-registermap_featurebit/w39v080fa.c (Revision 834) +++ flashrom-registermap_featurebit/w39v080fa.c (Arbeitskopie) @@ -20,18 +20,6 @@
#include "flash.h"
-int probe_winbond_fwhub(struct flashchip *flash) -{ - int result = probe_jedec(flash); - - if (!result) - return result; - - map_flash_registers(flash); - - return 1; -} - static int unlock_block_winbond_fwhub(struct flashchip *flash, int offset) { chipaddr wrprotect = flash->virtual_registers + offset + 2;
Am Donnerstag, den 07.01.2010, 19:31 +0100 schrieb Carl-Daniel Hailfinger:
Use the register mapping feature bit. All functions which just call probe_jedec and then map flash registers are replaced by probe_jedec. All functions which call probe_jedec, map flash registers and do something else can at least eliminate mapping flash registers. Fix logic inversion in probe_jedec to map flash registers on success instead of on failure. Change a few TIMING_IGNORED to TIMING_FIXME where probe_jedec is used.
Total savings: One probe function simplified, three probe functions eliminated.
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-registermap_featurebit/flashchips.c
--- flashrom-registermap_featurebit/flashchips.c (Revision 834) +++ flashrom-registermap_featurebit/flashchips.c (Arbeitskopie) @@ -827,7 +827,7 @@ .total_size = 64, .page_size = 128, .tested = TEST_OK_PRW,
.probe = probe_jedec,
.probe_timing = 10000, /* 10mS, Enter=Exec */ .erase = NULL, .block_erasers =.probe = probe_jedec,
Is this whitespace-only change (removes a trailing space that shouldn't have been there from the beginning) intentional?
Regards, Michael Karcher
On 09.01.2010 01:07, Michael Karcher wrote:
Am Donnerstag, den 07.01.2010, 19:31 +0100 schrieb Carl-Daniel Hailfinger:
Use the register mapping feature bit.
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
On 09.01.2010 01:37, Sean Nelson wrote:
Acked-by: Sean Nelson audiohacked@gmail.com
Thanks for the reviews, committed in r839.
On 09.01.2010 01:07, Michael Karcher wrote:
Index: flashrom-registermap_featurebit/flashchips.c
--- flashrom-registermap_featurebit/flashchips.c (Revision 834) +++ flashrom-registermap_featurebit/flashchips.c (Arbeitskopie) @@ -827,7 +827,7 @@ .total_size = 64, .page_size = 128, .tested = TEST_OK_PRW,
.probe = probe_jedec,
.probe_timing = 10000, /* 10mS, Enter=Exec */ .erase = NULL, .block_erasers =.probe = probe_jedec,
Is this whitespace-only change (removes a trailing space that shouldn't have been there from the beginning) intentional?
Yes, but I should have mentioned it in the changelog. In the future, I should run some check for trailing whitespace on all commits.
Regards, Carl-Daniel
My sign off is probably not necessary but I might as well, Acked-by: Sean Nelson audiohacked@gmail.com