[flashrom] [PATCH] SPI flashing speedup

[Michael Karcher]

Am Mittwoch, den 16.06.2010, 14:13 +0200 schrieb Carl-Daniel Hailfinger:
> Michael, I'd like to hear your thoughts on the impact of reading the
> status register in a tight loop instead of sleeping in between for
> systems which use flash translation (e.g. by EC).
In shared flash environments, a short sleep between polls of the status
register might be very useful. I don't see a big timing impact of a 10us
sleep on flashrom, but it will greatly reduce SPI load. Also continous
polling is a host CPU hog.

So I would suggest the following:
 - 10us delay where a quick response is expected
 - usleep(1) where a slow response is expected (Should give up a time
slice, don't really know how it behaves on current tickless linux,
though.)

> There are two options how we can handle this:
> - Remove the delays completely.
> - Mark the delays as optional, e.g. programmer_odelay() and let it
> default to a nop.
Third option: Make the delays so short that they don't matter. I'm all
for the third option.

I would suggest to factor out the wait-WIP code and introduce three
delay levels: near-immediate (10us), short (usleep(1)) and long
(something like 50ms). Then the chip definition can state what polling
delay is recommended for chip erase, sector erase and write.

It might be an even better idea to state the expected duration of the
erase/write instruction in the chip definition. This would enable
flashrom to make a coarse guess about total time, and make choosing of a
small polling interval possible.

The reason I suggest this definitely more convoluted approach than just
killing the delays is that I dislike the CPU maxing out at 100% for
several minutes when waiting for chip-erase complete on a slow SPI chip.
Especially if this is a shared flash system, I have no idea whether you
could get thermal issues by that. One would hope that the "idle" mode of
the EC used during flashing puts the fan to maximum - but who can be
sure?

For these reasons, I am not going to ack this patch as is.

Regards,
  Michael Karcher