Attention is currently required from: Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58482 )
Change subject: [RFC] writeprotect: implement wp_{get,set}_range() ......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS6:
I guess a separate tool that handles only block protection (or write > protection or flash configuration in general) would be much easier to maintain.
I think you mean building a separate binary from common sources right?
Right, nothing else makes sense, IMO :)
I'm fine with building a separate binary, though I think we would need to be careful about how we partition the functionality to avoid feature duplication.
I think the first sentence of flashrom's manpage provides a reasonable scope for one binary:
"flashrom is a utility for detecting, reading, writing, verifying and erasing flash chips."
I guess one could even argue that this is already too much. But that's what it always was, and it seems convenient enough. ;)
For future binaries, I would just use the smallest set of functions that overlap somehow. For instance in flashrom, `writing` overlaps with all the other functions. We could start a new binary for the configuration of write protection, and then only add there what overlaps with write protection (ATM, nothing comes to my mind).
I would much prefer to only call libflashrom functions from any CLI code.
I agree, I definitely want to move the writeprotect interface into libflashrom. I thought it would be easier to call the functions directly for now, but it shouldn't take too much work to squash writeprotect.h into libflashrom.h.
Sounds like a plan. However, for libflashrom, we should make sure to get the abstraction right from the beginning. The API is not considered stable, but I still wouldn't like to change too much later on.
-
Nico Huber (Code Review)