Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 )
Change subject: cli_classic: add writeprotect CLI ......................................................................
Patch Set 14:
(3 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58738/comment/36f2bac5_9e5228ba PS7, Line 63: mode
Since permanent and powercycle protection are very rarely supported and not very useful, I think we […]
Resolving
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58738/comment/aad5221e_b715a1fa PS12, Line 190: wp_cli
I remember there was a conversation about cli vs libflashrom in some other patch in this chain, I ha […]
There is some non-cli logic here (mostly for verifying configuration after writing), but moving it into libflashrom makes it more difficult to implement properly since we often need to write things to stdout/stderr and we shouldn't do that from libflashrom.
The number of libflashrom calls required to do any one operation is already quite small so I don't think we save much by moving more of the code.
https://review.coreboot.org/c/flashrom/+/58738/comment/178f63e5_af0e9ed9 PS12, Line 890: ret = wp_cli( : fill_flash, : enable_wp, : disable_wp, : print_wp_status, : print_available_ranges, : set_wp_range, : wp_start, : wp_len, : wp_mode_opt : );
Does this fit into 112 chars? If yes let's have it one line?
It would be around 150 chars, it's probably quite a bit more readable with each parameter on its own line.
-
Nikolai Artemiev (Code Review)