Attention is currently required from: Sam McNally, Edward O'Callaghan, Anastasia Klimchuk.
Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71659 )
Change subject: tests/: Add non-aligned write within a region unit-test ......................................................................
Patch Set 4: Code-Review+1
(8 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/71659/comment/e022e2b5_a5c37f5c PS2, Line 7: tests/: Add subregion alignment unit-test
Done
What does non-aligned mean? The commit message can match the test name
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/71659/comment/866dc56d_59aa0572 PS2, Line 469: * {MOCK_CHIP_SUBREGION_CONTENTS} [..] {MOCK_CHIP_SUBREGION_CONTENTS}.
Done
I dont understand this: `{MOCK_CHIP_SUBREGION_CONTENTS} [..] {MOCK_CHIP_SUBREGION_CONTENTS}`
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/71659/comment/4bfa7968_2a006efd PS4, Line 443: /* FIXME: MOCK_CHIP_CONTENT is buggy within setup_chip, it should also should this comment be here? if its buggy does the test work? what bug?
https://review.coreboot.org/c/flashrom/+/71659/comment/5887cb65_e01c274f PS4, Line 447: #define MOCK_CHIP_SUBREGION_CONTENTS 0xCC CC is now the default setting of the chip, probably doesnt matter
https://review.coreboot.org/c/flashrom/+/71659/comment/ee2c82e2_129af283 PS4, Line 457: /* Expect to verify the write completed successfuly within the region. */ is the presence of these flags important for catching the bug?
If so, document why. If not, remove them.
https://review.coreboot.org/c/flashrom/+/71659/comment/bd3c251f_82049b0f PS4, Line 469: * Create subregion smaller than erase granularity of chip. subregion -> region whole file replace please
https://review.coreboot.org/c/flashrom/+/71659/comment/417ed310_671617d6 PS4, Line 498: /* .. */ remove
https://review.coreboot.org/c/flashrom/+/71659/comment/aa83260a_d1be3a51 PS4, Line 500: /* Expect a verification pass that content within the region however this doesnt make sense
-
Evan Benn (Code Review)