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