On Sat, Nov 29, 2008 at 9:15 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
This is a patch which stores eraseblock sizes in struct flashchip. I decided to fill in the info for a few chips to illustrate how this works both for uniform and non-uniform sector sizes.
This is to unify different kinds of things.
Index: flashrom-eraseblocks/flashchips.c
...
{"AMIC", "A29002B", AMIC_ID_NOPREFIX, AMIC_A29002B, 256, 64 * 1024, {{16384,1},{8192,2},{32768,1},{65536,3}},TEST_UNTESTED, probe_29f002, erase_29f002, NULL, write_29f002},{"AMIC", "A29002T", AMIC_ID_NOPREFIX, AMIC_A29002T, 256, 64 * 1024, {{65536,3},{32768,1},{8192,2},{16384,1}},TEST_OK_PREW, probe_29f002, erase_29f002, NULL, write_29f002},
...
{"EON", "EN29F002(A)(N)B", EON_ID, EN_29F002B, 256, 256, {{16384,1},{8192,2},{32768,1},{65536,3}},TEST_UNTESTED, probe_jedec, erase_chip_jedec, NULL, write_en29f002a},{"EON", "EN29F002(A)(N)T", EON_ID, EN_29F002T, 256, 256, {{65536,3},{32768,1},{8192,2},{16384,1}},TEST_OK_PREW, probe_jedec, erase_chip_jedec, NULL, write_en29f002a},{"Fujitsu", "MBM29F004BC", FUJITSU_ID, MBM29F004BC, 512, 64 * 1024, {{16384,1},{8192,2},{32768,1},{65536,7}},TEST_UNTESTED, probe_jedec, NULL, NULL, NULL},{"Fujitsu", "MBM29F004TC", FUJITSU_ID, MBM29F004TC, 512, 64 * 1024, {{65536,7},{32768,1},{8192,2},{16384,1}},TEST_UNTESTED, probe_jedec, NULL, NULL, NULL},{"Fujitsu", "MBM29F400BC", FUJITSU_ID, MBM29F400BC, 512, 64 * 1024, {{16384,1},{8192,2},{32768,1},{65536,7}},TEST_UNTESTED, probe_m29f400bt, erase_m29f400bt, NULL, write_coreboot_m29f400bt},{"Fujitsu", "MBM29F400TC", FUJITSU_ID, MBM29F400TC, 512, 64 * 1024, {{65536,7},{32768,1},{8192,2},{16384,1}},TEST_UNTESTED, probe_m29f400bt, erase_m29f400bt, NULL, write_coreboot_m29f400bt},
...
{"Macronix", "MX25L512", MX_ID, MX_25L512, 64, 256, {{4096,16}}, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, NULL, spi_chip_write, spi_chip_read},{"Macronix", "MX25L1005", MX_ID, MX_25L1005, 128, 256, {{4096,32}}, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, NULL, spi_chip_write, spi_chip_read},{"Macronix", "MX25L2005", MX_ID, MX_25L2005, 256, 256, {{4096,64}}, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, NULL, spi_chip_write, spi_chip_read},{"Macronix", "MX25L4005", MX_ID, MX_25L4005, 512, 256, {{4096,128}}, TEST_OK_PREW, probe_spi_rdid, spi_chip_erase_c7, NULL, spi_chip_write, spi_chip_read},{"Macronix", "MX25L8005", MX_ID, MX_25L8005, 1024, 256, {{4096,256}}, TEST_OK_PREW, probe_spi_rdid, spi_chip_erase_c7, NULL, spi_chip_write, spi_chip_read},{"Macronix", "MX25L1605", MX_ID, MX_25L1605, 2048, 256, {{4096,512}}, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, NULL, spi_chip_write, spi_chip_read},{"Macronix", "MX25L3205", MX_ID, MX_25L3205, 4096, 256, {{4096,1024}}, TEST_OK_PREW, probe_spi_rdid, spi_chip_erase_c7, NULL, spi_chip_write, spi_chip_read},{"Macronix", "MX25L6405", MX_ID, MX_25L6405, 8192, 256, {{4096,2048}}, TEST_UNTESTED, probe_spi_rdid, spi_chip_erase_c7, NULL, spi_chip_write, spi_chip_read},{"Macronix", "MX29F002B", MX_ID, MX_29F002B, 256, 64 * 1024, {{16384,1},{8192,2},{32768,1},{65536,3}},TEST_UNTESTED, probe_29f002, erase_29f002, NULL, write_29f002},{"Macronix", "MX29F002T", MX_ID, MX_29F002T, 256, 64 * 1024, {{65536,3},{32768,1},{8192,2},{16384,1}},TEST_UNTESTED, probe_29f002, erase_29f002, NULL, write_29f002},
...
From the flash chip names, I guess the chips with uniform erase block
size are serial flash chips, and the ones with non-uniform size are parallel flash chips. I propose a new design the 'flashchip' structure. In this new design, serial and parallel flash chip information is stored in 'serial_flashchip' and 'parallel_flashchip' seperately, with a common 'flashchip_common' internal struct as their first field.
serial_flashchip +------------------+------------------------------+ | flashchip_common | sector_size, block_size, ... | +------------------+------------------------------+
parallel_flashchip +------------------+------------------------------+ | flashchip_common | erase_block | +------------------+------------------------------+
'flashchip_common' contains names, id's and common operation functions.
The design above looks clearer. And There is another advantage.
I guess(confirm and disproof welcome) one chipset only provides one of the two flash interface. When a chipset is detected, we can probe only one kind of them, (hopefully) faster and safer. It is ok if we want to probe the other kind.
Index: flashrom-eraseblocks/flashrom.c
--- flashrom-eraseblocks/flashrom.c (Revision 3776) +++ flashrom-eraseblocks/flashrom.c (Arbeitskopie) @@ -534,11 +534,33 @@
if (erase_it) { printf("Erasing flash chip.\n");
if (!flash->erase) {fprintf(stderr, "Error: flashrom has no erase function for this flash chip.\n");
if (!flash->block_erase && flash->eraseblocks[0].count) {fprintf(stderr, "Hint: flashrom knows the eraseblock ""layout, but there is no blockwise erase ""function for this flash chip. ""Using whole-chip erase.\n");}if (flash->block_erase && !flash->eraseblocks[0].count) {fprintf(stderr, "Hint: flashrom has a blockwise erase ""function for this flash chip, but the ""eraseblock layout is unknown. ""Using whole-chip erase.\n");}if (flash->block_erase && flash->eraseblocks[0].count) {unsigned long done = 0;int i, j;for (i = 0; done < flash->total_size * 1024; i++) {for (j = 0; j < flash->eraseblocks[i].count; j++) {flash->block_erase(flash, done + flash->eraseblocks[i].size * j);}done += flash->eraseblocks[i].count * flash->eraseblocks[i].size;}} else if (flash->erase) {flash->erase(flash);} else {fprintf(stderr, "Error: flashrom has no chip erase function for this flash chip.\n"); return 1; }
flash->erase(flash); exit(0); } else if (read_it) { if ((image = fopen(filename, "w")) == NULL) {
No comment to the logic. But the new code has a different degree of detail compared to other 'if (do_it)' blocks(i.e., the for loop).
yu ning