Change in flashrom[master]: Subject: raiden_debug_spi.c: Disable USB retry during some error codes
Hello Brian Nemec,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/flashrom/+/41152
to review the following change.
Change subject: Subject: raiden_debug_spi.c: Disable USB retry during some error codes ......................................................................
Subject: raiden_debug_spi.c: Disable USB retry during some error codes
Enables the USB SPI transfer retry mechanism when the write count error code is returned. This error code can indicate a recoverable transfer failure.
BUG=b:153887087 BRANCH=none TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verifications of the EC firmware on Nami. TEST=Modified servo micro to produce periodic errors when reading the packet length to verify the recovery is successful.
Change-Id: I9e6b2ccec0b06aab0d6920f1bddf108058e5d6b1 --- M raiden_debug_spi.c 1 file changed, 7 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/41152/1
diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 2127f69..ac74c0b 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -188,9 +188,13 @@ static bool retry_recovery(int error_code) { if (error_code < 0x10000) { - /* Handle error codes returned from the device. */ - if (USB_SPI_WRITE_COUNT_INVALID <= error_code && - error_code <= USB_SPI_DISABLED) { + /* + * Handle error codes returned from the device. USB_SPI_TIMEOUT, + * USB_SPI_BUSY, and USB_SPI_WRITE_COUNT_INVALID have been observed + * during transfer errors to the device and can be recovered. + */ + if (USB_SPI_READ_COUNT_INVALID <= error_code && + error_code <= USB_SPI_DISABLED) { return false; } } else if (usb_device_is_libusb_error(error_code)) {
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41152 )
Change subject: Subject: raiden_debug_spi.c: Disable USB retry during some error codes ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/flashrom/+/41152/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/41152/1//COMMIT_MSG@7 PS1, Line 7: Subject: Remove.
https://review.coreboot.org/c/flashrom/+/41152/1//COMMIT_MSG@7 PS1, Line 7: during for?
https://review.coreboot.org/c/flashrom/+/41152/1//COMMIT_MSG@21 PS1, Line 21: Missing Signed-off-by line?
Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/41152
to look at the new patch set (#2).
Change subject: raiden_debug_spi.c: Enables USB retry for invalid write count ......................................................................
raiden_debug_spi.c: Enables USB retry for invalid write count
Enables the USB SPI transfer retry mechanism when the error code USB_SPI_READ_COUNT_INVALID is returned. This error code can indicate a recoverable USB transfer failure.
BUG=b:153887087 BRANCH=none TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verification of the EC firmware on Nami. TEST=Modified ServoMicro to randomly corrupt USB packets when reading the packet length to replicate bad packets and the verify recovery is successful.
Change-Id: I9e6b2ccec0b06aab0d6920f1bddf108058e5d6b1 Signed-off-by: Brian J. Nemec bnemec@chromium.com --- M raiden_debug_spi.c 1 file changed, 7 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/41152/2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41152 )
Change subject: raiden_debug_spi.c: Enables USB retry for invalid write count ......................................................................
Patch Set 2: Code-Review+2
Hello build bot (Jenkins), Brian Nemec, Edward O'Callaghan,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/41152
to look at the new patch set (#3).
Change subject: raiden_debug_spi.c: Enables USB retry for invalid write count ......................................................................
raiden_debug_spi.c: Enables USB retry for invalid write count
Enables the USB SPI transfer retry mechanism when the error code USB_SPI_WRITE_COUNT_INVALID is returned. This error code can indicate a recoverable USB transfer failure.
BUG=b:153887087 BRANCH=none TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verification of the EC firmware on Nami. TEST=Modified ServoMicro to randomly corrupt USB packets when reading the packet length to replicate bad packets and the verify recovery is successful.
Change-Id: I9e6b2ccec0b06aab0d6920f1bddf108058e5d6b1 Signed-off-by: Brian J. Nemec bnemec@chromium.com --- M raiden_debug_spi.c 1 file changed, 7 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/41152/3
Brian Nemec has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41152 )
Change subject: raiden_debug_spi.c: Enables USB retry for invalid write count ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/flashrom/+/41152/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/41152/1//COMMIT_MSG@7 PS1, Line 7: during
for?
Done
https://review.coreboot.org/c/flashrom/+/41152/1//COMMIT_MSG@7 PS1, Line 7: Subject:
Remove.
Done
https://review.coreboot.org/c/flashrom/+/41152/1//COMMIT_MSG@21 PS1, Line 21:
Missing Signed-off-by line?
Done
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/41152 )
Change subject: raiden_debug_spi.c: Enables USB retry for invalid write count ......................................................................
raiden_debug_spi.c: Enables USB retry for invalid write count
Enables the USB SPI transfer retry mechanism when the error code USB_SPI_WRITE_COUNT_INVALID is returned. This error code can indicate a recoverable USB transfer failure.
BUG=b:153887087 BRANCH=none TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verification of the EC firmware on Nami. TEST=Modified ServoMicro to randomly corrupt USB packets when reading the packet length to replicate bad packets and the verify recovery is successful.
Change-Id: I9e6b2ccec0b06aab0d6920f1bddf108058e5d6b1 Signed-off-by: Brian J. Nemec bnemec@chromium.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/41152 Reviewed-by: Edward O'Callaghan quasisec@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M raiden_debug_spi.c 1 file changed, 7 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 2127f69..ac74c0b 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -188,9 +188,13 @@ static bool retry_recovery(int error_code) { if (error_code < 0x10000) { - /* Handle error codes returned from the device. */ - if (USB_SPI_WRITE_COUNT_INVALID <= error_code && - error_code <= USB_SPI_DISABLED) { + /* + * Handle error codes returned from the device. USB_SPI_TIMEOUT, + * USB_SPI_BUSY, and USB_SPI_WRITE_COUNT_INVALID have been observed + * during transfer errors to the device and can be recovered. + */ + if (USB_SPI_READ_COUNT_INVALID <= error_code && + error_code <= USB_SPI_DISABLED) { return false; } } else if (usb_device_is_libusb_error(error_code)) {
-
Brian Nemec (Code Review) -
Edward O'Callaghan (Code Review)
-
Paul Menzel (Code Review)