Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58474 )
Change subject: writeprotect, cli_classic: delete old writeprotect code ......................................................................
Patch Set 9:
(3 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58474/comment/5ef3da0b_34f46a4c PS9, Line 159: int set_wp_enable, : int set_wp_disable, : int wp_status, : int set_wp_range, : int wp_list, These all are commands/actions? I noticed some of them start with verb "set" and others start with a noun "wp". It would be great to have consistent names for all actions. Not sure whether this better be in this patch or later. This chunk of code is new so can be renamed without consequences... on the other hand renaming args here would make them inconsistent with existing variables in cli_classic.
https://review.coreboot.org/c/flashrom/+/58474/comment/957cb995_739a5865 PS9, Line 786: set_wp_range || (set_wp_region && wp_region) A tiny bit confusing, because function param is named simply `set_wp_range` however actual argument is more than that. Maybe add a comment? What is the difference between wp range and wp region?
File libflashrom.h:
https://review.coreboot.org/c/flashrom/+/58474/comment/82a1ca93_29fb627a PS9, Line 4: 2010 It is 2021 now ;)
-
Anastasia Klimchuk (Code Review)