Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59074 )
Change subject: [RFC][WPTST] dummyflasher: write protection for W25Q128FV ......................................................................
Patch Set 10:
(7 comments)
Patchset:
PS10: d thi
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/59074/comment/58829516_cad538d8 PS10, Line 63: bool wp; /* state of hardware write protection */ And this name also can be hwwp
https://review.coreboot.org/c/flashrom/+/59074/comment/ce4ca40d_245a88be PS10, Line 244: write_flash_part The name is a bit confusing, what "part" means here? And same for erase_flash_part
https://review.coreboot.org/c/flashrom/+/59074/comment/a3e866b5_1043ac6b PS10, Line 264: /* XXX: should data->erase_to_zero be taken into account here? */ Is it a TODO, or work-in-progress note?
https://review.coreboot.org/c/flashrom/+/59074/comment/f2a6c347_80b280ba PS10, Line 967: will have WP enabled Hardware WP?
File flashrom.8.tmpl:
https://review.coreboot.org/c/flashrom/+/59074/comment/1bbcd0b2_09fb5294 PS10, Line 696: Winbond " W25Q128FV " SPI flash chip (16384 kB, RDID)" Oh wow this was missing in the man for all the time?! Thank you for spotting! I think, just this one line, can be a separate patch outside of WP chain, and can be submitted independently without waiting for everything else to get through reviews.
https://review.coreboot.org/c/flashrom/+/59074/comment/ab0561e7_0c5e1108 PS10, Line 792: wp=state I remember we discussed that in the patch with test, yes now I see that "hwwp" would be a better name. Especially that description explains what it is exactly.
-
Anastasia Klimchuk (Code Review)