Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58478 )
Change subject: writeprotect.h: add structure to represent chip wp configuration bits ......................................................................
Patch Set 24:
(4 comments)
File flash.h:
https://review.coreboot.org/c/flashrom/+/58478/comment/36674104_0ab70722 PS22, Line 201: Complete
`Complete` seems a bit exaggerated. Given the many different […]
The intention is that it will be kept complete, i.e. bits will be added as necessary to support new chips. The count/present fields allow unused bits to be ignored for chips that don't need them.
You're right that we'll need to add WPS for some chips.
https://review.coreboot.org/c/flashrom/+/58478/comment/e01aaec4_46ce0bd9 PS22, Line 202: chip
Isn't everything about chips? Could we just call it `flashrom_wp_config`?
I've changed it to `wp_bits`. The name is probably less important now that it is just used internally by writeprotect code, but I'm happy to change it to something else.
https://review.coreboot.org/c/flashrom/+/58478/comment/f9e6439a_d1e232e0 PS22, Line 203: srp
Please add comments what all these acronyms mean.
Done
File flash.h:
https://review.coreboot.org/c/flashrom/+/58478/comment/ee60f3a0_39f9647b PS21, Line 201: /* Complete description of a chip's write protection configuration */ : struct flashrom_wp_chip_config { : size_t srp_bit_count; : uint8_t srp[MAX_SRP_BITS]; : : bool cmp_bit_present; : uint8_t cmp; : : bool sec_bit_present; : uint8_t sec; : : bool tb_bit_present; : uint8_t tb; : : size_t bp_bit_count; : uint8_t bp[MAX_BP_BITS]; : };
Well, not by nature, probably by coincidence. I don't see why one couldn't […]
I've though about this more and I agree that it would be best to keep it private to writeprotect/wp_ranges code. I will move it to writeprotect.h.