Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39311 )
Change subject: raiden_debug_spi.c: Disable retry during some error codes ......................................................................
raiden_debug_spi.c: Disable retry during some error codes
Forward ports the downstream commit: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+...
Change-Id: I77def28040fea8d1ecf102463180378f8612b00e Signed-off-by: Edward O'Callaghan quasisec@google.com --- M raiden_debug_spi.c M usb_device.h 2 files changed, 59 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/11/39311/1
diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 1f30b6a..fa42268 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -120,6 +120,15 @@ #define GOOGLE_RAIDEN_SPI_SUBCLASS (0x51) #define GOOGLE_RAIDEN_SPI_PROTOCOL (0x01)
+enum usb_spi_error { + USB_SPI_SUCCESS = 0x0000, + USB_SPI_TIMEOUT = 0x0001, + USB_SPI_BUSY = 0x0002, + USB_SPI_WRITE_COUNT_INVALID = 0x0003, + USB_SPI_READ_COUNT_INVALID = 0x0004, + USB_SPI_DISABLED = 0x0005, + USB_SPI_UNKNOWN_ERROR = 0x8000, +};
enum raiden_debug_spi_request { RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, @@ -163,6 +172,29 @@ uint8_t data[PAYLOAD_SIZE]; } __attribute__((packed)) usb_spi_response_t;
+/* + * This function will return true when an error code can potentially recover + * if we attempt to write SPI data to the device or read from it. We know + * that some conditions are not recoverable in the current state so allows us + * to bypass the retry logic and terminate early. + */ +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) { + return false; + } + } else if (usb_device_is_libusb_error(error_code)) { + /* Handle error codes returned from libusb. */ + if (error_code == LIBUSB_ERROR(LIBUSB_ERROR_NO_DEVICE)) { + return false; + } + } + return true; +} + static int write_command(const struct flashctx *flash, unsigned int write_count, unsigned int read_count, @@ -267,6 +299,10 @@ "Write attempt = %d\n" "status = %d\n", write_attempt + 1, status); + if (!retry_recovery(status)) { + /* Reattempting will not result in a recovery. */ + return status; + } programmer_delay(RETY_INTERVAL_US); continue; } @@ -282,6 +318,10 @@ "Read attempt = %d\n" "status = %d\n", write_attempt + 1, read_attempt + 1, status); + if (!retry_recovery(status)) { + /* Reattempting will not result in a recovery. */ + return status; + } programmer_delay(RETY_INTERVAL_US); } else { /* We were successful at performing the SPI transfer. */ diff --git a/usb_device.h b/usb_device.h index d379675..b2c7656 100644 --- a/usb_device.h +++ b/usb_device.h @@ -27,10 +27,18 @@
#include <libusb.h> #include <stdint.h> +#include <stdbool.h> + +/* + * The LIBUSB_ERROR macro converts a libusb failure code into an error code that + * flashrom recognizes. It does so without displaying an error code allowing us + * to compare error codes against the library enumeration values. + */ +#define LIBUSB_ERROR(eror_code) (0x20000 | -eror_code)
/* * The LIBUSB macro converts a libusb failure code into an error code that - * flashrom recognizes. It also displays additional libusb specific + * flashrom recognizes. It also displays additional libusb specific * information about the failure. */ #define LIBUSB(expression) \ @@ -42,7 +50,7 @@ __FILE__, \ __LINE__, \ libusb_error_name(libusb_error__)); \ - libusb_error__ = 0x20000 | -libusb_error__; \ + libusb_error__ = LIBUSB_ERROR(libusb_error__); \ } else { \ libusb_error__ = 0; \ } \ @@ -51,6 +59,15 @@ })
/* + * Returns true if the error code falls within the range of valid libusb + * error codes. + */ +static inline bool usb_device_is_libusb_error(int error_code) +{ + return (0x20000 <= error_code && error_code < 0x20064); +} + +/* * A USB match and associated value struct are used to encode the information * about a device against which we wish to match. If the value of a * usb_match_value has been set then a device must match that value. The name
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/39311 )
Change subject: raiden_debug_spi.c: Disable retry during some error codes ......................................................................
Patch Set 1: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/39311 )
Change subject: raiden_debug_spi.c: Disable retry during some error codes ......................................................................
raiden_debug_spi.c: Disable retry during some error codes
Forward ports the downstream commit: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+...
Change-Id: I77def28040fea8d1ecf102463180378f8612b00e Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/39311 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org --- M raiden_debug_spi.c M usb_device.h 2 files changed, 59 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved
diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 1f30b6a..fa42268 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -120,6 +120,15 @@ #define GOOGLE_RAIDEN_SPI_SUBCLASS (0x51) #define GOOGLE_RAIDEN_SPI_PROTOCOL (0x01)
+enum usb_spi_error { + USB_SPI_SUCCESS = 0x0000, + USB_SPI_TIMEOUT = 0x0001, + USB_SPI_BUSY = 0x0002, + USB_SPI_WRITE_COUNT_INVALID = 0x0003, + USB_SPI_READ_COUNT_INVALID = 0x0004, + USB_SPI_DISABLED = 0x0005, + USB_SPI_UNKNOWN_ERROR = 0x8000, +};
enum raiden_debug_spi_request { RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, @@ -163,6 +172,29 @@ uint8_t data[PAYLOAD_SIZE]; } __attribute__((packed)) usb_spi_response_t;
+/* + * This function will return true when an error code can potentially recover + * if we attempt to write SPI data to the device or read from it. We know + * that some conditions are not recoverable in the current state so allows us + * to bypass the retry logic and terminate early. + */ +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) { + return false; + } + } else if (usb_device_is_libusb_error(error_code)) { + /* Handle error codes returned from libusb. */ + if (error_code == LIBUSB_ERROR(LIBUSB_ERROR_NO_DEVICE)) { + return false; + } + } + return true; +} + static int write_command(const struct flashctx *flash, unsigned int write_count, unsigned int read_count, @@ -267,6 +299,10 @@ "Write attempt = %d\n" "status = %d\n", write_attempt + 1, status); + if (!retry_recovery(status)) { + /* Reattempting will not result in a recovery. */ + return status; + } programmer_delay(RETY_INTERVAL_US); continue; } @@ -282,6 +318,10 @@ "Read attempt = %d\n" "status = %d\n", write_attempt + 1, read_attempt + 1, status); + if (!retry_recovery(status)) { + /* Reattempting will not result in a recovery. */ + return status; + } programmer_delay(RETY_INTERVAL_US); } else { /* We were successful at performing the SPI transfer. */ diff --git a/usb_device.h b/usb_device.h index d379675..b2c7656 100644 --- a/usb_device.h +++ b/usb_device.h @@ -27,10 +27,18 @@
#include <libusb.h> #include <stdint.h> +#include <stdbool.h> + +/* + * The LIBUSB_ERROR macro converts a libusb failure code into an error code that + * flashrom recognizes. It does so without displaying an error code allowing us + * to compare error codes against the library enumeration values. + */ +#define LIBUSB_ERROR(eror_code) (0x20000 | -eror_code)
/* * The LIBUSB macro converts a libusb failure code into an error code that - * flashrom recognizes. It also displays additional libusb specific + * flashrom recognizes. It also displays additional libusb specific * information about the failure. */ #define LIBUSB(expression) \ @@ -42,7 +50,7 @@ __FILE__, \ __LINE__, \ libusb_error_name(libusb_error__)); \ - libusb_error__ = 0x20000 | -libusb_error__; \ + libusb_error__ = LIBUSB_ERROR(libusb_error__); \ } else { \ libusb_error__ = 0; \ } \ @@ -51,6 +59,15 @@ })
/* + * Returns true if the error code falls within the range of valid libusb + * error codes. + */ +static inline bool usb_device_is_libusb_error(int error_code) +{ + return (0x20000 <= error_code && error_code < 0x20064); +} + +/* * A USB match and associated value struct are used to encode the information * about a device against which we wish to match. If the value of a * usb_match_value has been set then a device must match that value. The name
-
Edward O'Callaghan (Code Review) -
Stefan Reinauer (Code Review)