Attention is currently required from: Nico Huber, Paul Menzel, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 )
Change subject: cli_classic: add writeprotect CLI ......................................................................
Patch Set 77:
(6 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/58738/comment/80a3b817_b093abd0 PS77, Line 10: You can add into commit message the patch is also updating manpage for new cli commands.
https://review.coreboot.org/c/flashrom/+/58738/comment/3e369d60_08a6a4dd PS77, Line 14: --wp-{enable,disable,range,list,status} There is also --wp-region command, you probably tested it too?
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58738/comment/fd674d56_969c7934 PS77, Line 274: likley Typo: likley -> likely
https://review.coreboot.org/c/flashrom/+/58738/comment/14c0b4ed_51080cc8 PS77, Line 493: uint32_t wp_start = 0, wp_len = 0; Maybe move it above next to other wp variables? Between #471 and #472 for example.
https://review.coreboot.org/c/flashrom/+/58738/comment/5bd64738_4ba7f776 PS77, Line 770: case OPTION_WP_SET_REGION: Is there a reason why OPTION_WP_SET_REGION comes last in the switch, and not next to all other WP options?
https://review.coreboot.org/c/flashrom/+/58738/comment/4669960f_18c8a54f PS77, Line 985: disable_wp Just to double-check: can disable_wp run together with set_wp_range or set_wp_region?