Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41533 )
Change subject: raiden_debug_spi.c: Adds support for USB SPI protocol V2 ......................................................................
Patch Set 2: Code-Review+1
(6 comments)
https://review.coreboot.org/c/flashrom/+/41533/2/raiden_debug_spi.c File raiden_debug_spi.c:
https://review.coreboot.org/c/flashrom/+/41533/2/raiden_debug_spi.c@720 PS2, Line 720: int This variable should be declared out of the loop, c.f. CB:38034
https://review.coreboot.org/c/flashrom/+/41533/2/raiden_debug_spi.c@726 PS2, Line 726: Replace 8 spaces with a tab?
https://review.coreboot.org/c/flashrom/+/41533/2/raiden_debug_spi.c@782 PS2, Line 782: status =
return directly and drop the intermediate status var?
Sounds good
https://review.coreboot.org/c/flashrom/+/41533/2/raiden_debug_spi.c@855 PS2, Line 855: PACKET_ID_V2_RESPONSE_CONTINUE) { This fits on a single line, or is it very close to the limit?
https://review.coreboot.org/c/flashrom/+/41533/2/raiden_debug_spi.c@913 PS2, Line 913: } else { How about "else if" here?
Actually, I think we should wrap these into functions and error out if the version is invalid
https://review.coreboot.org/c/flashrom/+/41533/2/raiden_debug_spi.c@977 PS2, Line 977: /* We were successful at performing the SPI transfer. */ Refactoring note (not for this commit): This "else" could be moved to line 947 to drop one indentation level