Brian Nemec 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:
(3 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@854 PS2, Line 854: } if
Um, missing an else?
Yup, that vanished somewhere.
https://review.coreboot.org/c/flashrom/+/41533/2/raiden_debug_spi.c@864 PS2, Line 864: msg_perr("Raiden: Unexpected packet id = %d",
Maybe you can swap the if/else branches to drop one indentation level on the currently-if block?
The indentation levels here correspond to a function, a loop, and a single level of conditions, I don't think you can cleanly beat that. There's some blocks above with 2 levels of conditionals, but I don't think it'll be cleaner combining those. I'll do another pass to see if this can be cleaned up during the next round of edits.
https://review.coreboot.org/c/flashrom/+/41533/2/raiden_debug_spi.c@913 PS2, Line 913: } else {
Or even, make sure that those function pointers can never be null in the first place. […]
That applies to the ctx_data->write_command but not supporting a 'restart_response_v2' response isn't an error, just a difference in the protocol. We didn't make the original protocol robust to the those types of USB errors so it never supported recovering in that state without redoing the SPI transfer.