Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84439?usp=email )
Change subject: Display progress for what is actually erased/written ......................................................................
Patch Set 13:
(2 comments)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/84439/comment/9bb9bb2e_8efa0d07?usp... : PS13, Line 1270: // FIXME: round up to granularity? It is a bit odd that `get_next_write` returns len, but `need_erase` returns 0 or 1, it would be cool for it to return length as well. Similar to my other comment, I think that rounding up to granularity is the level of details which should be inside `need_erase`.
However, what to do now. The granularity is default in all of the flashchips (except of just 3), and default is 256 bytes. The smallest eraseblock is 4K nowadays, which is way larger than 256 bytes. Maybe we can just leave it ? and move the fixme comment into need_erase?
https://review.coreboot.org/c/flashrom/+/84439/comment/78f61d63_e0a4f19b?usp... : PS13, Line 1281: // FIXME: should be rounded up to page size? It seems to me, this level of details is what `get_next_write` should be taking care of. And here, we should just take the len that get_next_write returns.
However currently `get_next_write` ignores the page size, not sure if it's on purpose or todo left.
I would leave the code here as is, and maybe move the fixme comment into get_next_write. What do you think about it?
-
Anastasia Klimchuk (Code Review)