David Hendricks has posted comments on this change. ( https://review.coreboot.org/17945 )
Change subject: Add functions to read/erase/write/verify by layout ......................................................................
Patch Set 2: Code-Review+1
(2 comments)
Looks pretty good, just a few cosmetic nits.
Do you by chance have testing notes or a script handy? I'd eventually like to run this thru the test script on chromium's branch, but that requires some syntax sugar that hasn't made it upstream yet...
https://review.coreboot.org/#/c/17945/2/flashrom.c File flashrom.c:
PS2, Line 1647: struct fl_layout_single *const fallback Since the fallback is always the entire chip, would it make sense to omit this and instead declare a single static fl_layout_single for use/reuse within this file? That will simplify the call sites a bit too.
PS2, Line 1862: newc - info->erase_start + info->region_end + 1 Hmmm, somehow I think this can be made clearer to the reader. We're offsetting the pointer by the distance between two physical addresses. IMO it's clearer to express that distance as a difference apart from adding to the pointer, i.e. "newc + (info->region_end + 1 - info->erase_start)".
Up to you if you want to change it. I had to commute things before I was convinced that this is technically correct.