Attention is currently required from: Nico Huber, Paul Menzel, 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 and update man page ......................................................................
Patch Set 77:
(6 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/58738/comment/4e8652ee_bc39b79c PS77, Line 10:
You can add into commit message the patch is also updating manpage for new cli commands.
Done
https://review.coreboot.org/c/flashrom/+/58738/comment/21673712_828e045e PS77, Line 14: --wp-{enable,disable,range,list,status}
There is also --wp-region command, you probably tested it too?
I think I've tested it before, but to be sure I retested with a W25Q128.W flash and a simple layout file: ``` 0:0x7fffff a 0:0x3fffff b ```
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58738/comment/10f089cf_7e560f24 PS77, Line 274: likley
Typo: likley -> likely
Done
https://review.coreboot.org/c/flashrom/+/58738/comment/bd76781e_77258a30 PS77, Line 493: uint32_t wp_start = 0, wp_len = 0;
Maybe move it above next to other wp variables? […]
Done
https://review.coreboot.org/c/flashrom/+/58738/comment/8df93538_18eea96d 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 op […]
Done
https://review.coreboot.org/c/flashrom/+/58738/comment/db0a5d5d_fd41fe7a PS77, Line 985: disable_wp
Just to double-check: can disable_wp run together with set_wp_range or set_wp_region?
Yep, they can run together since they're modifying different protection settings.