Change in flashrom[master]: [RFC] writeprotect, cli_classic: delete old writeprotect code
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: delete old writeprotect code ......................................................................
Patch Set 7:
(6 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58474/comment/f6d4906d_42d2dd36 PS6, Line 700: complete
until support is fully implemented?
Done
https://review.coreboot.org/c/flashrom/+/58474/comment/8a5ac1dd_b27ef08f PS6, Line 702: write protect is not supported on this flash chip
is it not supported on this flash chip, or not supported in flashrom?
Most but not all chips support write protection, so usually it would be a lack of support in flashrom. I've changed the message a bit.
https://review.coreboot.org/c/flashrom/+/58474/comment/97fd6e4b_e469686e PS6, Line 762: (void) wp_mode_opt; : (void) wp_region;
sorry maybe that's a silly question, but what these two lines are doing?
It's just for avoiding compiler warnings, I've added a comment.
File writeprotect.h:
https://review.coreboot.org/c/flashrom/+/58474/comment/1f2c1b84_a2bc8cb6 PS6, Line 24: WP_MODE_DISABLED, : WP_MODE_HARDWARE, : WP_MODE_POWER_CYCLE, : WP_MODE_PERMANENT,
Is there a reason the comments on enum values have been removed?
I'm not sure how much they explain what the modes actually do, but I've added them back for now.
File writeprotect.h:
https://review.coreboot.org/c/flashrom/+/58474/comment/72ee23b4_c3589d9a PS3, Line 31: uint32_t
Done - through flash.h. I can add explicit #includes for stdint.h and stdbool.h too though.
Marking resolved
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/58474/comment/b8d7aa22_6d1c8f8d PS6, Line 18: #include <stdio.h> : #include <stdlib.h> : #include <string.h> : : #include "flash.h" : #include "chipdrivers.h"
Technically, these includes are not needed (since the code has been removed)?
We could get rid of them but they're not really doing any harm. And if I remove them now I'll probably mess up and forget to add them back in the right patches :)
-
Nikolai Artemiev (Code Review)