Brian Nemec would like Brian Nemec to review this change.

View Change

raiden_debug_spi.c: Cleanup of the USB SPI protocol

Performs some cleanup of the USB SPI protocol:

* Minor cleanup of the comment descriptor for the protocol.
This adds the location of another relevant file, corrects the
omission of one of the protocol modes, makes the direction
of the packets explicit, and minor formating changes.

* Fixing typos in constants associated with the retry mechanism

* Removed an explicit comparison of 0 in a conditional

BUG=b:139058552
BRANCH=none
TEST=Builds

Signed-off-by: Brian J. Nemec <bnemec@chromium.com>
Change-Id: I17e62dabee2724eecf8d5a1a7827f06f0c7514df
---
M raiden_debug_spi.c
1 file changed, 38 insertions(+), 23 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/41597/1
diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c
index 5259981..b18d580 100644
--- a/raiden_debug_spi.c
+++ b/raiden_debug_spi.c
@@ -42,35 +42,50 @@
*
* https://chromium.googlesource.com/chromiumos/platform/ec
*
- * The protocol for the USB-SPI bridge is documented in the following file in
- * that respository:
+ * The protocol for the USB-SPI bridge is implemented in the following files
+ * in that repository:
*
+ * chip/stm32/usb_spi.h
* chip/stm32/usb_spi.c
*
- * Version 1:
- * SPI transactions of up to 62B in each direction with every command having
- * a response. The initial packet from host contains a 2B header indicating
- * write and read counts with an optional payload length equal to the write
- * count. The device will respond with a message that reports the 2B status
- * code and an optional payload response length equal to read count.
+ * bInterfaceProtocol determines which protocol is used by the USB SPI device.
*
- * Message Format:
*
- * Command Packet:
+ * USB SPI Version 1:
+ *
+ * SPI transactions of up to 62B in each direction with every command having
+ * a response. The initial packet from host contains a 2B header indicating
+ * write and read counts with an optional payload length equal to the write
+ * count. The device will respond with a message that reports the 2B status
+ * code and an optional payload response length equal to read count.
+ *
+ *
+ * Message Packets:
+ *
+ * Command First Packet (Host to Device):
+ *
+ * USB SPI command, containing the number of bytes to write and read
+ * and a payload of bytes to write.
+ *
* +------------------+-----------------+------------------------+
* | write count : 1B | read count : 1B | write payload : <= 62B |
* +------------------+-----------------+------------------------+
*
* write count: 1 byte, zero based count of bytes to write
*
- * read count: 1 byte, zero based count of bytes to read
+ * read count: 1 byte, zero based count of bytes to read. Half duplex
+ * mode is enabled with '-1'
*
* write payload: Up to 62 bytes of data to write to SPI, the total
* length of all TX packets must match write count.
* Due to data alignment constraints, this must be an
* even number of bytes unless this is the final packet.
*
- * Response Packet:
+ * Response Packet (Device to Host):
+ *
+ * USB SPI response, containing the status code and any bytes of the
+ * read payload.
+ *
* +-------------+-----------------------+
* | status : 2B | read payload : <= 62B |
* +-------------+-----------------------+
@@ -81,8 +96,8 @@
* 0x0002: Busy, try again
* This can happen if someone else has acquired the shared memory
* buffer that the SPI driver uses as /dev/null
- * 0x0003: Write count invalid (V1 > 62B)
- * 0x0004: Read count invalid (V1 > 62B)
+ * 0x0003: Write count invalid (over 62 bytes)
+ * 0x0004: Read count invalid (over 62 bytes)
* 0x0005: The SPI bridge is disabled.
* 0x8000: Unknown error mask
* The bottom 15 bits will contain the bottom 15 bits from the EC
@@ -100,7 +115,7 @@
*
* 0x00000: Status code success.
* 0x00001-0x0FFFF: Error code returned by the USB SPI device.
- * 0x10001-0x1FFFF: The host has determined an error has occurred.
+ * 0x10001-0x1FFFF: USB SPI Host error codes
* 0x20001-0x20063 Lower bits store the positive value representation
* of the libusb_error enum. See the libusb documentation:
* http://libusb.sourceforge.net/api-1.0/group__misc.html
@@ -156,9 +171,9 @@
* 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)
+#define WRITE_RETRY_ATTEMPTS (3)
+#define READ_RETRY_ATTEMPTS (3)
+#define RETRY_INTERVAL_US (100 * 1000)

/*
* This timeout is so large because the Raiden SPI timeout is 800ms.
@@ -310,7 +325,7 @@
{
int status = -1;

- for (int write_attempt = 0; write_attempt < WRITE_RETY_ATTEMPTS;
+ for (int write_attempt = 0; write_attempt < WRITE_RETRY_ATTEMPTS;
write_attempt++) {

status = write_command(flash, write_count, read_count,
@@ -326,15 +341,15 @@
/* Reattempting will not result in a recovery. */
return status;
}
- programmer_delay(RETY_INTERVAL_US);
+ programmer_delay(RETRY_INTERVAL_US);
continue;
}
- for (int read_attempt = 0; read_attempt < READ_RETY_ATTEMPTS; read_attempt++) {
+ for (int read_attempt = 0; read_attempt < READ_RETRY_ATTEMPTS; read_attempt++) {

status = read_response(flash, write_count, read_count,
write_buffer, read_buffer);

- if (status != 0) {
+ if (status) {
/* Read operation failed. */
msg_perr("Raiden: Read response failed\n"
"Write attempt = %d\n"
@@ -345,7 +360,7 @@
/* Reattempting will not result in a recovery. */
return status;
}
- programmer_delay(RETY_INTERVAL_US);
+ programmer_delay(RETRY_INTERVAL_US);
} else {
/* We were successful at performing the SPI transfer. */
return status;

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

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I17e62dabee2724eecf8d5a1a7827f06f0c7514df
Gerrit-Change-Number: 41597
Gerrit-PatchSet: 1
Gerrit-Owner: Brian Nemec <bnemec@google.com>
Gerrit-Reviewer: Brian Nemec <bnemec@chromium.org>
Gerrit-MessageType: newchange