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/+/59072 )
Change subject: dummyflasher: add SR2 and SR3 emulation harness ......................................................................
Patch Set 34: Code-Review+1
(3 comments)
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/59072/comment/047eff4d_254cf665 PS16, Line 347: other chips might differ
Updated comment.
Just checking: you are saying "updated comment" but the comment is removed, is it as expected?
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/59072/comment/aa177688_f9ea9fe6 PS34, Line 169: assert Just wanted to check whether everyone is fine with `assert` in the code? It is not often used in flashrom code, I don't remember whether I saw it ever. On the other hand I see that it makes code easier, and allows using return value as a result (not as error code).
File flashrom.8.tmpl:
https://review.coreboot.org/c/flashrom/+/59072/comment/42170e18_b3a9ae97 PS34, Line 817: is a hexadecimal value of up to 24 bits. For example, 0x332211 assigns 0x11 to : SR1, 0x22 to SR2 and 0x33 to SR3. Shorter value is padded to 24 bits with : zeroes on the left. See datasheet for chosen chip for details about the : registers content. Based on this , it would be great to have 3 test scenarios listed in commit message, 3 times `flashrom -p dummy:spi_status=content` with content 8bits, 16bits and 24bits. You most likely already did those scenarios when testing the patch, then just list them.
-
Anastasia Klimchuk (Code Review)