Attention is currently required from: Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58477 )
Change subject: flashchips: add writeprotect bit layout map to chips ......................................................................
Patch Set 25:
(3 comments)
File flash.h:
https://review.coreboot.org/c/flashrom/+/58477/comment/c7f6bc75_a2b8ae7f PS25, Line 287: struct reg_bit_info cmp; Please add a comment about the acronyms. (Thought I mentioned that before *shrug* but can't find the comment.)
File flash.h:
https://review.coreboot.org/c/flashrom/+/58477/comment/14f7ac77_64c88f6e PS21, Line 192: 0
Done
Still, why the 0?
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/58477/comment/e9c09bdb_59a464d0 PS20, Line 6353: .tb = {STATUS1, 5, RW}, /* Called BP3 in datasheet, acts like TB */ : .sec = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like SEC */
Any thoughts about how this should be handled? […]
IMO we should definitely use the naming by the bit's function, like you did. To be visible to new developers, we could also add comments to the struct declaration, e.g. "Some datasheets call it BP3 even if it acts as TB. */
There's some documentation in the wiki, would be nice to update it and also mention this problem. https://flashrom.org/Development_Guidelines#Adding/reviewing_a_new_flash_chi...