Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 )
Change subject: cli_classic: add writeprotect CLI ......................................................................
Patch Set 18:
(2 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58738/comment/cb7f99e4_76b86565 PS15, Line 66: " --wp-range=<start>,<len> set write protect range\n" : " --wp-region <region> set write protect region\n"
I am just wondering, there are commands to set wp range/region, but how to unset? Or is it --wp-disa […]
You unset it by setting an empty range (it's included in --wp-list), --wp-disable doesn't change the range.
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58738/comment/a1f1b022_aebf4e70 PS17, Line 220: ret = flashrom_wp_set_range(flash, &cfg, &range); : if (ret) { : msg_gerr("The chip does not support the requested range.\n"); : return ret; : } : : /* Write range before other operations (i.e. enabling HW protection) */ : ret = write_wp_config(flash, &cfg); : if (ret) : return ret;
Continuing the conversation about libflashrom vs cli. […]
The structure of the API is: 1. Read state from chip 2. Modify state in memory 3. Write state to chip
So no harm in forgetting to state.
The API in its current form allows fetching state, setting both mode and range and writing it in one go. If it's changed to perform writes on every operation and you incorrectly set mode before the range, setting range might fail (possibly permanently). Current design seems safer: you make changes, handle errors caught by the library and only then try to change state of the chip.
-
Sergii Dmytruk (Code Review)