No subject
Sun Dec 9 17:34:17 CET 2012
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
More information about the coreboot
mailing list