Attention is currently required from: Simon Buhrow, Angel Pons.
3 comments:
Patchset:
The clou is the condition […]
I'm generally not a fan of special cases. However, if we can find a
contained solution for the full-chip erase case, I wouldn't block it.
Technically, checks like `per_blockfn == &erase_block` are a layering
violation, i.e. something is done in a lower layer that should have
been handled at a higher level. I left an inline comment about how I
would avoid it.
Eventually, I would prefer a generic solution for the small block-size
problem, I think it's not hard to achieve. We should replace the
current fallback-to-bigger-blocks strategy with one that checks if
consecutive calls of a small erase function would completely fill
a bigger one. There's one obstacle, however: The fallback strategy was
necessary because some programmers can restrict what commands we
can send. I would handle that by filtering the list of erase functions
right after the chip was probed.
File flashrom.c:
Patch Set #5, Line 1261: if ((block_size == flashctx->chip->total_size * 1024) && per_blockfn == &erase_block){
I guess this would be much easier to handle in flashrom_flash_erase(). If the
size matches the full chip and the last eraseblocks[] entry too, then call
that eraser directly, and if it fails fall back to erase_by_layout(). No loops
and definitely no nested loops necessary.
Why change this? Doesn't the code handle this case gracefully?
To view, visit change 58467. To unsubscribe, or for help writing mail filters, visit settings.