Edward O'Callaghan submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved
raiden_debug_spi.c: Implement retry mechanism

This overcomes a problem with the ServoMicro where USB packets can be
ack'd by the device without triggering interrupts or loading data into
the USB endpoints. The retry mechanism attempts the USB read 3 times
before reattempting the write call to avoid performing multiple SPI
transfers due to a USB problem. This process repeats 3 times before we
return the last error code. Intermediary problems are reported in the
status code.

Based off the downstream commit:
https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+/2038271

Change-Id: I76cde68852fa4963582d57c7dcb9f24de32c6da8
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/39310
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
---
M raiden_debug_spi.c
1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c
index 08f8928..1f30b6a 100644
--- a/raiden_debug_spi.c
+++ b/raiden_debug_spi.c
@@ -133,6 +133,16 @@
#define PAYLOAD_SIZE (MAX_PACKET_SIZE - PACKET_HEADER_SIZE)

/*
+ * Servo Micro has an error where it is capable of acknowledging USB packets
+ * without loading it into the USB endpoint buffers or triggering interrupts.
+ * See crbug.com/952494. Retry mechanisms have been implemented to recover
+ * from these rare failures allowing the process to continue.
+ */
+#define WRITE_RETY_ATTEMPTS (3)
+#define READ_RETY_ATTEMPTS (3)
+#define RETY_INTERVAL_US (100 * 1000)
+
+/*
* This timeout is so large because the Raiden SPI timeout is 800ms.
*/
#define TRANSFER_TIMEOUT_MS (200 + 800)
@@ -245,16 +255,40 @@
{
int status = -1;

- status = write_command(flash, write_count, read_count,
- write_buffer, read_buffer);
+ for (int write_attempt = 0; write_attempt < WRITE_RETY_ATTEMPTS;
+ write_attempt++) {

- if (status) {
- return status;
+ status = write_command(flash, write_count, read_count,
+ write_buffer, read_buffer);
+
+ if (status) {
+ /* Write operation failed. */
+ msg_perr("Raiden: Write command failed\n"
+ "Write attempt = %d\n"
+ "status = %d\n",
+ write_attempt + 1, status);
+ programmer_delay(RETY_INTERVAL_US);
+ continue;
+ }
+ for (int read_attempt = 0; read_attempt < READ_RETY_ATTEMPTS; read_attempt++) {
+
+ status = read_response(flash, write_count, read_count,
+ write_buffer, read_buffer);
+
+ if (status != 0) {
+ /* Read operation failed. */
+ msg_perr("Raiden: Read response failed\n"
+ "Write attempt = %d\n"
+ "Read attempt = %d\n"
+ "status = %d\n",
+ write_attempt + 1, read_attempt + 1, status);
+ programmer_delay(RETY_INTERVAL_US);
+ } else {
+ /* We were successful at performing the SPI transfer. */
+ return status;
+ }
+ }
}
-
- status = read_response(flash, write_count, read_count,
- write_buffer, read_buffer);
-
return status;
}


To view, visit change 39310. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I76cde68852fa4963582d57c7dcb9f24de32c6da8
Gerrit-Change-Number: 39310
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged