Am Samstag, den 10.07.2010, 15:56 +0200 schrieb Carl-Daniel Hailfinger:
> -int erase_flash(struct flashchip *flash)
> +static int walk_eraseregions(struct flashchip *flash, int erasefunction, int
> (*do_something) (struct flashchip *flash, unsigned int addr, unsigned int
> len))
> {
> - int i, j, k, ret = 0, found = 0;
> + int i, j;
> + unsigned int done = 0;
> unsigned int start, len;
> + struct block_eraser eraser = flash->block_erasers[erasefunction];
> + for (i = 0; i < NUM_ERASEREGIONS; i++) {
> + /* count==0 for all automatically initialized array
> + * members so the loop below won't be executed for them.
> + */
> + for (j = 0; j < eraser.eraseblocks[i].count; j++) {
> + start = done + eraser.eraseblocks[i].size * j;
See below.
> + len = eraser.eraseblocks[i].size;
You could pull this line out of the loop, but you don't have to.
> + msg_cdbg("0x%06x-0x%06x, ", start,
> + start + len - 1);
> + if (do_something(flash, start, len))
> + return 1;
You are just moving this code, but I still have a comment on it: Why
don't you increase "done" here? You don't need the "start" variable any
more and you don't have to write the explicit multiplication below.
> + }
> + done += eraser.eraseblocks[i].count *
> + eraser.eraseblocks[i].size;
> + }
> + return 0;
> +}
>
The patch as-is just refactors without any functionality change and thus
is
Acked-by: Michael Karcher <[email protected]>
On the other hand, if you change the function as indicated above (and
think a second about whether my suggestions really make sense), this is
also
Acked-by: Michael Karcher <[email protected]>
Regards,
Michael Karcher
_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom
