Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57437 )
Change subject: tests: Make chip definitions static global ......................................................................
Patch Set 3:
(6 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/57437/comment/a07f7a08_8db02933 PS3, Line 7: tests: Make chip definitions static global The summary line should somehow cover all the changes. If you do much in a single commit, it might simply end up as a vague "tests: Revise chip definitions".
https://review.coreboot.org/c/flashrom/+/57437/comment/f839994d_210c4d9d PS3, Line 13: was confusing. Mock chip looks more realistic now. Generally, moving and changing code in a single commit often makes the review unnecessarily harder. Not too bad in this case.
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/57437/comment/c4df88a7_c453873f PS3, Line 24: CHIP_TOTAL_SIZE There's another chip below with a different size. Should we name it `MOCKED_CHIP_SIZE` maybe?
https://review.coreboot.org/c/flashrom/+/57437/comment/e3e24329_c22654e1 PS3, Line 24: #define CHIP_TOTAL_SIZE 8388608 Please make use of the KiB/MiB macros. This should be `(8*MiB)` (usually written without spaces around the asterisk).
https://review.coreboot.org/c/flashrom/+/57437/comment/d733ef15_1b8a06fa PS3, Line 128: static struct flashchip chip_8MiB = { Should we make them `const` and then work on a copy on the stack? That way we'd have a fresh copy for every test.
https://review.coreboot.org/c/flashrom/+/57437/comment/70b14686_488e1b5a PS3, Line 130: .total_size = 8 * 1024, CHIP_TOTAL_SIZE / KiB
-
Nico Huber (Code Review)