Change in flashrom[master]: [RFC] writeprotect, cli_classic: create writeprotect foundation
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58474 )
Change subject: [RFC] writeprotect, cli_classic: create writeprotect foundation ......................................................................
Patch Set 5:
(7 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58474/comment/f0b4f333_5e2a6227 PS3, Line 691: bool
const bool
Done
https://review.coreboot.org/c/flashrom/+/58474/comment/b4e2ad97_68a7d7fd PS3, Line 695: |
nit: missing space before `|` […]
Done
File writeprotect.h:
https://review.coreboot.org/c/flashrom/+/58474/comment/7b2ca68a_3551c850 PS3, Line 21: #define MAX_WP_RANGES 128
Why 128?
I chose 128 arbitrarily, but it can be bounded by the number of range-related bits on a chip. The most I've seen is 6 (3BP+TB+SEC+CMP) so 64 would be sufficient.
https://review.coreboot.org/c/flashrom/+/58474/comment/1cfbeff3_545661e2 PS3, Line 31: uint32_t
Missing header include
Done - through flash.h. I can add explicit #includes for stdint.h and stdbool.h too though.
https://review.coreboot.org/c/flashrom/+/58474/comment/7c64ea49_5d3459fe PS3, Line 36: bool
Missing header include
Done
https://review.coreboot.org/c/flashrom/+/58474/comment/d7c03203_2f1365c5 PS3, Line 38: size_t
Missing header include
Done
File writeprotect.c:
PS3:
I find it hard to evaluate an interface without seeing its implementation. […]
That seems reasonable, I can just comment out the calls in cli_classic.c. I'll wait to see what others think though.
-
Nikolai Artemiev (Code Review)