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 flashrom@mkarcher.dialup.fu-berlin.de
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 flashrom@mkarcher.dialup.fu-berlin.de
Regards, Michael Karcher