July 2020 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: meson: fix compilation under uClibc-ng
by David Hendricks (Code Review) July 20, 2020

July 20, 2020

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/43599 ) Change subject: meson: fix compilation under uClibc-ng ...................................................................... meson: fix compilation under uClibc-ng fileno requires _POSIX_C_SOURCE to only be defined. nanosleep requires _POSIX_C_SOURCE to be defined to 199309L. strndup requires _POSIX_C_SOURCE to be defined to 200809L. Change-Id: Idb80937bb78e173eb03f2a0c0cdd8925fcd7bfa1 Signed-off-by: Rosen Penev <rosenp(a)gmail.com> --- M meson.build 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/43599/1 diff --git a/meson.build b/meson.build index 5374e87..ef7ac2c 100644 --- a/meson.build +++ b/meson.build @@ -25,7 +25,7 @@ cc = meson.get_compiler('c') add_project_arguments(cc.get_supported_arguments(warning_flags), language : 'c') add_project_arguments('-D_DEFAULT_SOURCE', language : 'c') -add_project_arguments('-D_POSIX_C_SOURCE', language : 'c') # required for fileno +add_project_arguments('-D_POSIX_C_SOURCE=200809L', language : 'c') # required for fileno, nanosleep, and strndup add_project_arguments('-D_BSD_SOURCE', language : 'c') # required for glibc < v2.19 add_project_arguments('-DFLASHROM_VERSION="' + meson.project_version() + '"', language : 'c') -- To view, visit https://review.coreboot.org/c/flashrom/+/43599 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idb80937bb78e173eb03f2a0c0cdd8925fcd7bfa1 Gerrit-Change-Number: 43599 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-MessageType: newchange

3 7

Change in flashrom[master]: endiantest: Fix #if expression
by David Hendricks (Code Review) July 20, 2020

July 20, 2020

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/43598 ) Change subject: endiantest: Fix #if expression ...................................................................... endiantest: Fix #if expression Without this, `gcc -E endiantest.c` can fail and return the incorrect endiannes as well as exiting with non-zero. Here is the actual error shown in the output: endiantest.c:2:31: error: #if with no expression #if __FLASHROM_LITTLE_ENDIAN__ I was able to reproduce this using gcc-6.3.0 and clang-4.0.1, but newer compilers didn't have this issue. Change-Id: Iba2febd861471ec821a494336e800c2564984332 Signed-off-by: David Hendricks <david.hendricks(a)gmail.com> --- M endiantest.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/43598/1 diff --git a/endiantest.c b/endiantest.c index de44ec5..9766061 100644 --- a/endiantest.c +++ b/endiantest.c @@ -1,5 +1,5 @@ #include "platform.h" -#if __FLASHROM_LITTLE_ENDIAN__ +#if defined __FLASHROM_LITTLE_ENDIAN__ little #else big -- To view, visit https://review.coreboot.org/c/flashrom/+/43598 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iba2febd861471ec821a494336e800c2564984332 Gerrit-Change-Number: 43598 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-MessageType: newchange

2 5

Change in flashrom[master]: raiden_debug_spi: add param for a custom reset setup
by Mary Ruthven (Code Review) July 20, 2020

July 20, 2020

Mary Ruthven has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/43527 ) Change subject: raiden_debug_spi: add param for a custom reset setup ...................................................................... raiden_debug_spi: add param for a custom reset setup Cr50 asserts EC_RST_L to flash the AP. Asserting EC_RST_L breaks flashing the AP on some boards. This change adds a custom_rst arg to tell cr50 not to assert any system reset signals during flash AP setup. BUG=b:154885210 BRANCH=none TEST=manual sudo flashrom -p raiden_debug_spi:target=EC -r sudo flashrom -p raiden_debug_spi:target=AP -r sudo flashrom -p raiden_debug_spi:target=AP,custom_rst=true -r sudo flashrom -p raiden_debug_spi:target=AP,custom_rst=bogus -r sudo flashrom -p raiden_debug_spi -r Signed-off-by: Mary Ruthven <mruthven(a)chromium.org> Change-Id: I2da26469120c5304bc129b5578fcb7ca805fc1d1 --- M raiden_debug_spi.c 1 file changed, 30 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/43527/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index b55307a..6b3468e 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -136,17 +136,17 @@ }; enum raiden_debug_spi_request { - RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, - RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001, - RAIDEN_DEBUG_SPI_REQ_ENABLE_AP = 0x0002, - RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, - RAIDEN_DEBUG_SPI_REQ_ENABLE_H1 = 0x0004, - RAIDEN_DEBUG_SPI_REQ_RESET = 0x0005, - RAIDEN_DEBUG_SPI_REQ_BOOT_CFG = 0x0006, - RAIDEN_DEBUG_SPI_REQ_SOCKET = 0x0007, - RAIDEN_DEBUG_SPI_REQ_SIGNING_START = 0x0008, - RAIDEN_DEBUG_SPI_REQ_SIGNING_SIGN = 0x0009, - + RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, + RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001, + RAIDEN_DEBUG_SPI_REQ_ENABLE_AP = 0x0002, + RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, + RAIDEN_DEBUG_SPI_REQ_ENABLE_H1 = 0x0004, + RAIDEN_DEBUG_SPI_REQ_RESET = 0x0005, + RAIDEN_DEBUG_SPI_REQ_BOOT_CFG = 0x0006, + RAIDEN_DEBUG_SPI_REQ_SOCKET = 0x0007, + RAIDEN_DEBUG_SPI_REQ_SIGNING_START = 0x0008, + RAIDEN_DEBUG_SPI_REQ_SIGNING_SIGN = 0x0009, + RAIDEN_DEBUG_SPI_REQ_ENABLE_AP_CUSTOM = 0x000a, }; #define PACKET_HEADER_SIZE (2) @@ -452,6 +452,23 @@ return 0; } +static int get_ap_request_type(void) +{ + int ap_request = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP; + char *custom_rst_str = extract_programmer_param("custom_rst"); + if (custom_rst_str) { + if (!strcasecmp(custom_rst_str, "true")) + ap_request = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP_CUSTOM; + else { + msg_perr("Invalid custom rst param: %s\n", + custom_rst_str); + ap_request = -1; + } + } + free(custom_rst_str); + return ap_request; +} + static int get_target(void) { int request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE; @@ -459,7 +476,7 @@ char *target_str = extract_programmer_param("target"); if (target_str) { if (!strcasecmp(target_str, "ap")) - request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE_AP; + request_enable = get_ap_request_type(); else if (!strcasecmp(target_str, "ec")) request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE_EC; else { @@ -468,6 +485,7 @@ } } free(target_str); + msg_pinfo("Raiden target: %d\n", request_enable); return request_enable; } -- To view, visit https://review.coreboot.org/c/flashrom/+/43527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2da26469120c5304bc129b5578fcb7ca805fc1d1 Gerrit-Change-Number: 43527 Gerrit-PatchSet: 1 Gerrit-Owner: Mary Ruthven <mruthven(a)chromium.org> Gerrit-MessageType: newchange

4 10

Change in flashrom[master]: raiden_debug_spi: add missing USB_SPI requests
by Mary Ruthven (Code Review) July 20, 2020

July 20, 2020

Mary Ruthven has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/43525 ) Change subject: raiden_debug_spi: add missing USB_SPI requests ...................................................................... raiden_debug_spi: add missing USB_SPI requests Add the missing USB_SPI requests from platform/cr50 BUG=none BRANCH=none TEST=none Signed-off-by: Mary Ruthven <mruthven(a)chromium.org> Change-Id: I49c0c28566ed36af6fa03e23a878d19462c55f70 --- M raiden_debug_spi.c 1 file changed, 11 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/43525/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index ac74c0b..b55307a 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -136,10 +136,17 @@ }; enum raiden_debug_spi_request { - RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, - RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001, - RAIDEN_DEBUG_SPI_REQ_ENABLE_AP = 0x0002, - RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, + RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, + RAIDEN_DEBUG_SPI_REQ_DISABLE = 0x0001, + RAIDEN_DEBUG_SPI_REQ_ENABLE_AP = 0x0002, + RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, + RAIDEN_DEBUG_SPI_REQ_ENABLE_H1 = 0x0004, + RAIDEN_DEBUG_SPI_REQ_RESET = 0x0005, + RAIDEN_DEBUG_SPI_REQ_BOOT_CFG = 0x0006, + RAIDEN_DEBUG_SPI_REQ_SOCKET = 0x0007, + RAIDEN_DEBUG_SPI_REQ_SIGNING_START = 0x0008, + RAIDEN_DEBUG_SPI_REQ_SIGNING_SIGN = 0x0009, + }; #define PACKET_HEADER_SIZE (2) -- To view, visit https://review.coreboot.org/c/flashrom/+/43525 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I49c0c28566ed36af6fa03e23a878d19462c55f70 Gerrit-Change-Number: 43525 Gerrit-PatchSet: 1 Gerrit-Owner: Mary Ruthven <mruthven(a)chromium.org> Gerrit-MessageType: newchange

3 3

Change in flashrom[master]: raiden_debug_spi.c: Adds support for USB SPI protocol V2
by Brian Nemec (Code Review) July 20, 2020

July 20, 2020

Hello Brian Nemec, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/41533 to review the following change. Change subject: raiden_debug_spi.c: Adds support for USB SPI protocol V2 ...................................................................... raiden_debug_spi.c: Adds support for USB SPI protocol V2 Adds support for the USB SPI V2 protocol and documentation of it. The protocol version number uses the bInterfaceProtocol field in USB to identify which device to use, this enables us to support both V1 and V2 with the same host. The USB SPI V2 protocol adds the ability to perform multi-packet USB SPI transfers. This results in fewer USB messages exchanged and faster flashing speeds. BUG=b:139058552 BRANCH=none TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verification of the EC firmware on Nami with a USB SPI V1 protocol device TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verification of the EC firmware on Nami with a USB SPI V2 protocol device TEST=Builds Signed-off-by: Brian J. Nemec <bnemec(a)chromium.com> Change-Id: Ie356c63b521c0cc11a4946ffac128ec7139f0bec --- M raiden_debug_spi.c 1 file changed, 470 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/41533/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 6cb32b3..5e07967 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -48,7 +48,10 @@ * include/usb_spi.h * common/usb_spi.c * - * bInterfaceProtocol determines which protocol is used by the USB SPI device. + * 2 Versions of the protocol exist in deployed devices: + * The raiden_debug_spi.c interface used will switch between them automatically + * depending on the protocol version reported in the bInterfaceProtocol + * * * * USB SPI Version 1: @@ -117,8 +120,191 @@ * 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 + * + * + * + * USB SPI Version 2: + * + * USB SPI version 2 adds support for larger SPI transfers and reduces the + * number of USB packets transferred. This improves performance when + * writing or reading large chunks of memory from a device. A packet ID + * field is used to distinguish the different packet types. Additional + * packets have been included to query the device for it's configuration + * allowing the interface to be used on platforms with different SPI + * limitations. It includes validation and a packet to recover from the + * situations where USB packets are lost. + * + * + * Example: USB SPI request with 128 byte write and 0 byte read. + * + * Packet #1 Host to Device: + * packet id = PACKET_ID_V2_COMMAND + * write count = 128 + * read count = 0 + * payload = First 58 bytes from the write buffer, + * start is byte 0 + * + * Packet #2 Host to Device: + * packet id = PACKET_ID_V2_COMMAND_CONTINUE + * data index = 58 + * payload = next 60 bytes from the write buffer, + * starting at byte 58 + * + * Packet #3 Host to Device: + * packet id = PACKET_ID_V2_COMMAND_CONTINUE + * data index = 118 + * payload = next 10 bytes from the write buffer, + * starting at byte 118 + * + * Packet #4 Device to Host: + * packet id = PACKET_ID_V2_RESPONSE + * status code = status code from device + * payload = 0 bytes + * + * Command Packet (Host to Device): + * + * Start of USB SPI command, containing the number of bytes to write and + * read and a payload of bytes to write. If the payload is unable to fit + * in one USB packet, it contains the first 58 bytes. + * + * +----------------+------------------+-----------------+------------------------+ + * | packet id : 2B | write count : 2B | read count : 2B | write payload : <= 58B | + * +----------------+------------------+-----------------+------------------------+ + * + * packet id: 2 byte enum defined by packet_id_type + * Valid values packet id = PACKET_ID_V2_COMMAND + * + * write count: 2 byte, zero based count of bytes to write + * + * read count: 2 byte, zero based count of bytes to read + * + * 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 (Device to Host): + * + * Start of the USB SPI response, containing the status code and + * any bytes read in the payload. If read buffer can not fit in + * a single packet, it represents the first 60 bytes. + * + * +----------------+------------------+-----------------------+ + * | packet id : 2B | status code : 2B | read payload : <= 60B | + * +----------------+------------------+-----------------------+ + * + * packet id: 2 byte enum defined by packet_id_type + * Valid values packet id = PACKET_ID_V2_RESPONSE + * + * status code: 2 byte status code + * 0x0000: Success + * 0x0001: SPI timeout + * 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. The byte limit is platform specific + * and is set during the configure USB SPI response. + * 0x0004: Read count invalid. The byte limit is platform specific + * and is set during the configure USB SPI response. + * 0x0005: The SPI bridge is disabled. + * 0x0006: The RX continue packet's data index is invalid. This + * can indicate a USB transfer failure to the device. + * 0x0007: The RX endpoint has received more data than write count. + * This can indicate a USB transfer failure to the device. + * 0x0008: An unexpected packet arrived that the device could not + * process. + * 0x8000: Unknown error mask + * The bottom 15 bits will contain the bottom 14 bits from the EC + * error code. + * + * read payload: Up to 62 bytes of data read from SPI, the total + * length of all RX packets must match read count + * unless an error status was returned. Due to data + * alignment constraints, this must be a even number + * of bytes unless this is the final packet. + * + * + * Continue Packet (Bidirectional): + * + * Continuation packet for the writes and read buffers. Both packets + * follow the same format, a data index counts the number of bytes + * previously transferred in the USB SPI transfer and a payload of bytes. + * + * +----------------+-----------------+-------------------------------+ + * | packet id : 2B | data index : 2B | write / read payload : <= 60B | + * +----------------+-----------------+-------------------------------+ + * + * packet id: 2 byte enum defined by packet_id_type + * The packet id has 2 values depending on direction: + * packet id = PACKET_ID_V2_COMMAND_CONTINUE indicates the + * packet is being transmitted from the host to the device + * packet id = PACKET_ID_V2_RESPONSE_CONTINUE indicates the + * packet is being transmitted from the device to the host. + * + * data index: The data index indicates the number of bytes in the + * read or write buffers have already been transmitted. + * It is used to validate that no packets have been dropped + * and that the prior packets have been correctly decoded. + * this value corresponds to the position in the destination + * destination to start copying the payload into. + * + * read and write payload: + * Contains up to 60 bytes of payload data to transfer to + * the SPI write buffer or from the SPI read buffer. + * + * + * Command Get Configuration Packet (Host to Device): + * + * Query the device to request it's USB SPI configuration indicating + * the number of bytes it can write and read. + * + * +----------------+ + * | packet id : 2B | + * +----------------+ + * + * packet id: 2 byte enum PACKET_ID_V2_COMMAND_GET_USB_SPI_CONFIG + * + * Response Configuration Packet (Device to Host): + * + * Response packet form the device to report the maximum write and + * read size supported by the device. + * + * +----------------+----------------------+---------------------+ + * | packet id : 2B | max write count : 2B | max read count : 2B | + * +----------------+----------------------+---------------------+ + * + * packet id: 2 byte enum PACKET_ID_V2_COMMAND_GET_USB_SPI_CONFIG + * + * max write count : 2 byte count of the maximum number of bytes + * the device can write to SPI in one transaction. + * + * max read count : 2 byte count of the maximum number of bytes + * the device can read from SPI in one transaction. + * + * Command Restart Response Packet (Host to Device): + * + * Command to restart the response transfer from the device. This enables + * the host to recover from a lost packet when reading the response + * without restarting the SPI transfer. + * + * +----------------+ + * | packet id : 2B | + * +----------------+ + * + * packet id: 2 byte enum PACKET_ID_V2_COMMAND_GET_USB_SPI_CONFIG + * + * send_command return codes have the following format: + * + * 0x00000: Status code success. + * 0x00001-0x0FFFF: Error code returned by the USB SPI device. + * 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 */ + #include "programmer.h" #include "spi.h" #include "usb_device.h" @@ -138,6 +324,7 @@ #define GOOGLE_VID (0x18D1) #define GOOGLE_RAIDEN_SPI_SUBCLASS (0x51) #define GOOGLE_RAIDEN_SPI_PROTOCOL_V1 (0x01) +#define GOOGLE_RAIDEN_SPI_PROTOCOL_V2 (0x02) enum { /* The host failed to transfer the data with no libusb error. */ @@ -185,6 +372,7 @@ */ #define WRITE_RETRY_ATTEMPTS (3) #define READ_RETRY_ATTEMPTS (3) +#define GET_CONFIG_RETRY_ATTEMPTS (3) #define RETRY_INTERVAL_US (100 * 1000) /* @@ -200,6 +388,17 @@ * All of the USB SPI packets have size equal to the max USB packet size of 64B */ #define PAYLOAD_SIZE_V1 (62) +#define PACKET_HEADER_SIZE (2) + +#define HOST_VERSION (1) + +#define PAYLOAD_SIZE_V2_START (58) + +#define PAYLOAD_SIZE_V2_RESPONSE (60) + +#define PAYLOAD_SIZE_V2_CONTINUE (60) + +#define PAYLOAD_SIZE_V2_ERROR (60) #define SPI_TRANSFER_V1_MAX (PAYLOAD_SIZE_V1) @@ -224,10 +423,86 @@ usb_spi_response_v1_t response; } __attribute__((packed)) usb_spi_packet_v1_t; +/* + * Version 2 protocol specific attributes + */ + +enum packet_id_type { + + /* Request USB SPI configuration data from device. */ + PACKET_ID_V2_COMMAND_GET_USB_SPI_CONFIG = 0, + /* USB SPI configuration data from device. */ + PACKET_ID_V2_RESPONSE_USB_SPI_CONFIG = 1, + + /* Start a USB SPI transfer and deliver first packet of data to write. */ + PACKET_ID_V2_COMMAND = 2, + /* Additional packets containing write payload. */ + PACKET_ID_V2_COMMAND_CONTINUE = 3, + + /* + * Request the device restart the response enabling us to recover from + * packet loss without another SPI transfer. + */ + PACKET_ID_V2_COMMAND_RESTART_RESPONSE = 4, + + /* First packet of USB SPI response with status code and read payload. */ + PACKET_ID_V2_RESPONSE = 5, + /* Additional packets containing read payload. */ + PACKET_ID_V2_RESPONSE_CONTINUE = 6, +}; + +typedef struct { + int16_t packet_id; +} __attribute__((packed)) usb_spi_command_get_configuration_v2_t; + +typedef struct { + int16_t packet_id; + uint16_t max_write_count; + uint16_t max_read_count; +} __attribute__((packed)) usb_spi_response_configuration_v2_t; + +typedef struct { + int16_t packet_id; + int16_t write_count; + /* -1 Indicates readback all on halfduplex compliant devices. */ + int16_t read_count; + uint8_t data[PAYLOAD_SIZE_V2_START]; +} __attribute__((packed)) usb_spi_command_v2_t; + +typedef struct { + int16_t packet_id; + uint16_t status_code; + uint8_t data[PAYLOAD_SIZE_V2_RESPONSE]; +} __attribute__((packed)) usb_spi_response_v2_t; + +typedef struct { + int16_t packet_id; + uint16_t data_index; + uint8_t data[PAYLOAD_SIZE_V2_CONTINUE]; +} __attribute__((packed)) usb_spi_continue_v2_t; + +typedef struct { + int16_t packet_id; +} __attribute__((packed)) usb_spi_command_restart_response_v2_t; + +typedef struct { + union { + int16_t packet_id; + usb_spi_command_get_configuration_v2_t command_get_config; + usb_spi_response_configuration_v2_t response_config; + usb_spi_command_restart_response_v2_t restart_response; + usb_spi_command_v2_t command; + usb_spi_response_v2_t response; + usb_spi_continue_v2_t command_continue; + usb_spi_continue_v2_t response_continue; + }; +} __attribute__((packed)) usb_spi_packet_v2_t; + typedef struct { union { uint8_t bytes[MAX_USB_PACKET_SIZE]; usb_spi_packet_v1_t packet_v1; + usb_spi_packet_v2_t packet_v2; }; /* * By storing the number of bytes in the header and knowing that the @@ -427,6 +702,177 @@ return status; } +/* + * Version 2 Protocol + */ + +int get_spi_size_v2(struct raiden_debug_spi_data *ctx_data) +{ + int status; + usb_spi_packet_ctx_t response_config; + + usb_spi_packet_ctx_t command_get_config = { + .header_size = sizeof(usb_spi_command_get_configuration_v2_t), + .packet_size = sizeof(usb_spi_command_get_configuration_v2_t), + .packet_v2.packet_id = PACKET_ID_V2_COMMAND_GET_USB_SPI_CONFIG + }; + + for (int config_attempt = 0; config_attempt < GET_CONFIG_RETRY_ATTEMPTS; + config_attempt++) { + + status = transmit_packet(ctx_data, &command_get_config); + if (status) { + msg_perr("Raiden: Failed to transmit get config\n" + " config attempt = %d\n" + " status = %d\n", + config_attempt + 1, status); + programmer_delay(RETRY_INTERVAL_US); + continue; + } + + status = receive_packet(ctx_data, &response_config); + if (status) { + msg_perr("Raiden: Failed to receive packet\n" + " config attempt = %d\n" + " status = %d\n", + config_attempt + 1, status); + programmer_delay(RETRY_INTERVAL_US); + continue; + } + + /* + * Perform validation on the packet received to verify it is a valid + * configuration. If it is, we are ready to perform transfers. + */ + if ((response_config.packet_v2.packet_id == + PACKET_ID_V2_RESPONSE_USB_SPI_CONFIG) || + (response_config.packet_size == + sizeof(usb_spi_response_configuration_v2_t))) { + + /* Set the parameters from the configuration. */ + ctx_data->max_spi_write_count = + response_config.packet_v2.response_config.max_write_count; + ctx_data->max_spi_read_count = + response_config.packet_v2.response_config.max_read_count; + return status; + } + + msg_perr("Raiden: Packet is not a valid config\n" + " config attempt = %d\n" + " packet id = %d\n" + " packet size = %d\n", + config_attempt + 1, + response_config.packet_v2.packet_id, + response_config.packet_size); + programmer_delay(RETRY_INTERVAL_US); + } + return USB_SPI_HOST_INIT_FAILURE; +} + +int restart_response_v2(const struct raiden_debug_spi_data *ctx_data) +{ + int status; + + usb_spi_packet_ctx_t restart_response = { + .header_size = sizeof(usb_spi_command_restart_response_v2_t), + .packet_size = sizeof(usb_spi_command_restart_response_v2_t), + .packet_v2.packet_id = PACKET_ID_V2_COMMAND_RESTART_RESPONSE + }; + + status = transmit_packet(ctx_data, &restart_response); + return status; +} + + +int write_command_v2(const struct raiden_debug_spi_data * ctx_data, + usb_spi_transmit_ctx_t* write, + usb_spi_receive_ctx_t* read) +{ + int status; + usb_spi_packet_ctx_t continue_packet; + + usb_spi_packet_ctx_t start_usb_spi_packet = { + .header_size = offsetof(usb_spi_command_v2_t, data), + .packet_v2.command.packet_id = PACKET_ID_V2_COMMAND, + .packet_v2.command.write_count = write->transmit_size, + .packet_v2.command.read_count = read->receive_size + }; + + fill_usb_packet(&start_usb_spi_packet, write); + status = transmit_packet(ctx_data, &start_usb_spi_packet); + if (status) { + return status; + } + + while (write->transmit_index < write->transmit_size) { + /* Transmit any continue packets. */ + continue_packet.header_size = offsetof(usb_spi_continue_v2_t, data); + continue_packet.packet_v2.command_continue.packet_id = + PACKET_ID_V2_COMMAND_CONTINUE; + continue_packet.packet_v2.command_continue.data_index = + write->transmit_index; + + fill_usb_packet(&continue_packet, write); + + status = transmit_packet(ctx_data, &continue_packet); + if (status) { + return status; + } + } + return status; +} + +int read_response_v2(const struct raiden_debug_spi_data * ctx_data, + usb_spi_transmit_ctx_t* write, + usb_spi_receive_ctx_t* read) +{ + int status = -1; + usb_spi_packet_ctx_t response; + + /* Receive the payload to the servo micro. */ + while (read->receive_index < read->receive_size) { + + status = receive_packet(ctx_data, &response); + if (status) { + /* Return the transfer error. */ + return status; + } + if (response.packet_v2.packet_id == PACKET_ID_V2_RESPONSE) { + /* + * The host should only see this packet if an error occurs + * on the device or if it's the first response packet. + */ + if (response.packet_v2.response.status_code) { + return response.packet_v2.response.status_code; + } + if (read->receive_index) { + msg_perr("Raiden: Unexpected packet id = %d", + response.packet_v2.response.packet_id); + return USB_SPI_HOST_RX_UNEXPECTED_PACKET; + } + response.header_size = offsetof(usb_spi_response_v2_t, data); + } if (response.packet_v2.packet_id == + PACKET_ID_V2_RESPONSE_CONTINUE) { + + /* We validate that no packets were missed. */ + if (read->receive_index != + response.packet_v2.response_continue.data_index) { + return USB_SPI_HOST_RX_BAD_DATA_INDEX; + } + response.header_size = offsetof(usb_spi_continue_v2_t, data); + } else { + msg_perr("Raiden: Unexpected packet id = %d", + response.packet_v2.packet_id); + return USB_SPI_HOST_RX_UNEXPECTED_PACKET; + } + status = read_usb_packet(read, &response); + if (status) { + return status; + } + } + return status; +} + static int send_command(const struct flashctx *flash, unsigned int write_count, unsigned int read_count, @@ -464,6 +910,8 @@ write_ctx.transmit_index = 0; if (ctx_data->protocol_version == GOOGLE_RAIDEN_SPI_PROTOCOL_V1) { status = write_command_v1(ctx_data, &write_ctx, &read_ctx); + } else { + status = write_command_v2(ctx_data, &write_ctx, &read_ctx); } if (status) { @@ -490,6 +938,8 @@ read_ctx.receive_index = 0; if (ctx_data->protocol_version == GOOGLE_RAIDEN_SPI_PROTOCOL_V1) { status = read_response_v1(ctx_data, &write_ctx, &read_ctx); + } else { + status = read_response_v2(ctx_data, &write_ctx, &read_ctx); } if (status == 0) { @@ -513,6 +963,15 @@ /* Reattempting will not result in a recovery. */ return status; } + /* + * Protocol version 2 includes multi-packet messages, we can + * restart only the response operation without performing SPI + * transfer. + */ + if (ctx_data->protocol_version == + GOOGLE_RAIDEN_SPI_PROTOCOL_V2) { + restart_response_v2(ctx_data); + } programmer_delay(RETRY_INTERVAL_US); } else { /* We were successful at performing the SPI transfer. */ @@ -579,6 +1038,7 @@ static int configure_protocol(struct spi_master *ctx_spi) { + int status = 0; struct raiden_debug_spi_data *ctx_data = (struct raiden_debug_spi_data *)ctx_spi->data; @@ -593,6 +1053,15 @@ ctx_data->max_spi_write_count= SPI_TRANSFER_V1_MAX; ctx_data->max_spi_read_count = SPI_TRANSFER_V1_MAX; break; + case GOOGLE_RAIDEN_SPI_PROTOCOL_V2: + /* + * Protocol V2 requires the host to query the device for + * it's maximum read and write sizes + */ + status = get_spi_size_v2(ctx_data); + if (status) { + return status; + } default: msg_pdbg("Raiden: Unknown USB SPI protocol version = %d", ctx_data->protocol_version); @@ -692,7 +1161,6 @@ usb_match_value_default(&match.vid, GOOGLE_VID); usb_match_value_default(&match.class, LIBUSB_CLASS_VENDOR_SPEC); usb_match_value_default(&match.subclass, GOOGLE_RAIDEN_SPI_SUBCLASS); - usb_match_value_default(&match.protocol, GOOGLE_RAIDEN_SPI_PROTOCOL_V1); ret = LIBUSB(libusb_init(NULL)); if (ret != 0) { -- To view, visit https://review.coreboot.org/c/flashrom/+/41533 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie356c63b521c0cc11a4946ffac128ec7139f0bec Gerrit-Change-Number: 41533 Gerrit-PatchSet: 1 Gerrit-Owner: Brian Nemec <bnemec(a)google.com> Gerrit-Reviewer: Brian Nemec <bnemec(a)chromium.org> Gerrit-MessageType: newchange

3 34

Change in flashrom[master]: raiden_debug_spi.c: Refactor to support multiple protocols
by Brian Nemec (Code Review) July 20, 2020

July 20, 2020

Hello Brian Nemec, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/41532 to review the following change. Change subject: raiden_debug_spi.c: Refactor to support multiple protocols ...................................................................... raiden_debug_spi.c: Refactor to support multiple protocols Refactoring of the USB SPI protocol to support multiple protocols. The changes include: * Setting of USB SPI limits based on the device's USB protocol * Creation of context states to keep track of bytes written to and read from the device and individual USB packets. * Creation of helper functions for handling transfer of data between the USB packets and the data buffers. * Removal of magic numbers on the USB return codes. Some renaming of to make the protocol version clear. BUG=b:139058552 BRANCH=none TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verification of the EC firmware on Nami. TEST=Builds Signed-off-by: Brian J. Nemec <bnemec(a)chromium.com> Change-Id: Id404af14e55fa0884e29f28880206aaad4deba66 --- M raiden_debug_spi.c 1 file changed, 370 insertions(+), 166 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/41532/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index ac74c0b..b08d9e0 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -42,28 +42,37 @@ * * 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.c + * include/usb_spi.h + * common/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. + * + * + * 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. @@ -71,6 +80,10 @@ * even number of bytes unless this is the final packet. * * Response Packet: + * + * USB SPI response, containing the status code and any bytes of the + * read payload. + * * +-------------+-----------------------+ * | status : 2B | read payload : <= 62B | * +-------------+-----------------------+ @@ -81,8 +94,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 @@ -115,24 +128,46 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <stddef.h> /* FIXME: Add some programmer IDs here */ const struct dev_entry devs_raiden[] = { {0}, }; -#define GOOGLE_VID (0x18D1) -#define GOOGLE_RAIDEN_SPI_SUBCLASS (0x51) -#define GOOGLE_RAIDEN_SPI_PROTOCOL (0x01) +#define GOOGLE_VID (0x18D1) +#define GOOGLE_RAIDEN_SPI_SUBCLASS (0x51) +#define GOOGLE_RAIDEN_SPI_PROTOCOL_V1 (0x01) + +enum { + /* The host failed to transfer the data with no libusb error. */ + USB_SPI_HOST_TX_BAD_TRANSFER = 0x10001, + /* We did not receive the expected USB packet. */ + USB_SPI_HOST_RX_UNEXPECTED_PACKET = 0x18002, + /* We received a continue packet with an invalid data index.*/ + USB_SPI_HOST_RX_BAD_DATA_INDEX = 0x18003, + /* We too much data. */ + USB_SPI_HOST_RX_DATA_OVERFLOW = 0x18004, + /* We too little data. */ + USB_SPI_HOST_RX_READ_FAILURE = 0x18005, + /* We were unable to configure the device. */ + USB_SPI_HOST_INIT_FAILURE = 0x18006, +}; 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, + 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, + /* The RX continue packet's data index is invalid. */ + USB_SPI_RX_BAD_DATA_INDEX = 0x0006, + /* The RX endpoint has received more data than write count. */ + USB_SPI_RX_DATA_OVERFLOW = 0x0007, + /* An unexpected packet arrived on the device. */ + USB_SPI_RX_UNEXPECTED_PACKET = 0x0008, + USB_SPI_UNKNOWN_ERROR = 0x8000, }; enum raiden_debug_spi_request { @@ -142,42 +177,95 @@ RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, }; -#define PACKET_HEADER_SIZE (2) -#define MAX_PACKET_SIZE (64) -#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) +#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. */ #define TRANSFER_TIMEOUT_MS (200 + 800) -struct raiden_debug_spi_data { - struct usb_device *dev; - uint8_t in_ep; - uint8_t out_ep; -}; +/* + * USB permits a maximum bulk transfer of 64B. + */ +#define MAX_USB_PACKET_SIZE (64) +/* + * All of the USB SPI packets have size equal to the max USB packet size of 64B + */ +#define PAYLOAD_SIZE_V1 (62) + +#define SPI_TRANSFER_V1_MAX (PAYLOAD_SIZE_V1) + +/* + * Version 1 protocol specific attributes + */ typedef struct { int8_t write_count; /* -1 Indicates readback all on halfduplex compliant devices. */ int8_t read_count; - uint8_t data[PAYLOAD_SIZE]; -} __attribute__((packed)) usb_spi_command_t; + uint8_t data[PAYLOAD_SIZE_V1]; +} __attribute__((packed)) usb_spi_command_v1_t; typedef struct { uint16_t status_code; - uint8_t data[PAYLOAD_SIZE]; -} __attribute__((packed)) usb_spi_response_t; + uint8_t data[PAYLOAD_SIZE_V1]; +} __attribute__((packed)) usb_spi_response_v1_t; + +typedef union { + usb_spi_command_v1_t command; + usb_spi_response_v1_t response; +} __attribute__((packed)) usb_spi_packet_v1_t; + +typedef struct { + union { + uint8_t bytes[MAX_USB_PACKET_SIZE]; + usb_spi_packet_v1_t packet_v1; + }; + /* + * By storing the number of bytes in the header and knowing that the + * USB data packets are all 64B long, we are able to use the header + * size to store the offset of the buffer and it's size without + * duplicating variables that can go out of sync. + */ + int header_size; + /* Number of bytes in the packet */ + int packet_size; +} usb_spi_packet_ctx_t; + +typedef struct { + /* Buffer we are reading data from. */ + const uint8_t *buffer; + /* Total size of the transmit transfer. */ + int transmit_size; + /* Number of bytes transmitted. */ + int transmit_index; +} usb_spi_transmit_ctx_t; + +typedef struct { + /* Buffer we are writing data into. */ + uint8_t *buffer; + /* Total size of the receive transfer. */ + int receive_size; + /* Number of bytes received. */ + int receive_index; +} usb_spi_receive_ctx_t; + +struct raiden_debug_spi_data { + struct usb_device *dev; + uint8_t in_ep; + uint8_t out_ep; + uint8_t protocol_version; + uint16_t max_spi_write_count; + uint16_t max_spi_read_count; +}; /* * This function will return true when an error code can potentially recover @@ -212,167 +300,239 @@ return (const struct raiden_debug_spi_data *)flash->mst->spi.data; } -static int write_command(const struct flashctx *flash, - unsigned int write_count, - unsigned int read_count, - const unsigned char *write_buffer, - unsigned char *read_buffer) -{ +/* + * Fill the data section in the USB packets. + */ +static int fill_usb_packet(usb_spi_packet_ctx_t* dst, + usb_spi_transmit_ctx_t* src) { + int transfer_size = src->transmit_size - src->transmit_index; + int max_buffer_size = MAX_USB_PACKET_SIZE - dst->header_size; + uint8_t *packet_buffer = dst->bytes + dst->header_size; - int transferred; - int ret; - usb_spi_command_t command_packet; - const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash); - - if (write_count > PAYLOAD_SIZE) { - msg_perr("Raiden: Invalid write_count of %d\n", write_count); - return SPI_INVALID_LENGTH; + if (transfer_size > max_buffer_size) { + transfer_size = max_buffer_size; } + memcpy(packet_buffer, src->buffer + src->transmit_index, transfer_size); - if (read_count > PAYLOAD_SIZE) { - msg_perr("Raiden: Invalid read_count of %d\n", read_count); - return SPI_INVALID_LENGTH; - } - - command_packet.write_count = write_count; - command_packet.read_count = read_count; - - memcpy(command_packet.data, write_buffer, write_count); - - ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, - ctx_data->out_ep, - (void*)&command_packet, - write_count + PACKET_HEADER_SIZE, - &transferred, - TRANSFER_TIMEOUT_MS)); - if (ret != 0) { - msg_perr("Raiden: OUT transfer failed\n" - " write_count = %d\n" - " read_count = %d\n", - write_count, read_count); - return ret; - } - - if ((unsigned) transferred != write_count + PACKET_HEADER_SIZE) { - msg_perr("Raiden: Write failure (wrote %d, expected %d)\n", - transferred, write_count + PACKET_HEADER_SIZE); - return 0x10001; - } - + dst->packet_size = dst->header_size + transfer_size; + src->transmit_index += transfer_size; return 0; } -static int read_response(const struct flashctx *flash, - unsigned int write_count, - unsigned int read_count, - const unsigned char *write_buffer, - unsigned char *read_buffer) -{ +static int read_usb_packet(usb_spi_receive_ctx_t* dst, + const usb_spi_packet_ctx_t* src) { + int max_read_length = dst->receive_size - dst->receive_index; + int bytes_in_buffer = src->packet_size - src->header_size; + const uint8_t *packet_buffer = src->bytes + src->header_size; + + if (bytes_in_buffer > max_read_length) { + /* + * An error occurred, we should not receive more data than + * the buffer can support. + */ + msg_perr("Raiden: Receive packet overflowed\n" + " bytes_in_buffer = %d\n" + " max_read_length = %d\n" + " receive_index = %d\n" + " receive_size = %d\n", + bytes_in_buffer, max_read_length, + dst->receive_size, dst->receive_index); + return USB_SPI_HOST_RX_DATA_OVERFLOW; + } + memcpy(dst->buffer + dst->receive_index, packet_buffer, bytes_in_buffer); + + dst->receive_index += bytes_in_buffer; + return 0; +} + +static int transmit_packet(const struct raiden_debug_spi_data * ctx_data, + usb_spi_packet_ctx_t* packet) { int transferred; - int ret; - usb_spi_response_t response_packet; - const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash); + int status = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, + ctx_data->out_ep, + packet->bytes, + packet->packet_size, + &transferred, + TRANSFER_TIMEOUT_MS)); + if (status || transferred != packet->packet_size) { + msg_perr("Raiden: OUT transfer failed\n" + " transferred = %d\n" + " packet_size = %d\n" + " status = %d\n", + transferred, packet->packet_size, status); + if (status == 0) { + return USB_SPI_HOST_TX_BAD_TRANSFER; + } + } + return status; +} - ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, - ctx_data->in_ep, - (void*)&response_packet, - read_count + PACKET_HEADER_SIZE, - &transferred, - TRANSFER_TIMEOUT_MS)); - if (ret != 0) { +static int receive_packet(const struct raiden_debug_spi_data * ctx_data, + usb_spi_packet_ctx_t* packet) { + int transferred; + int status = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, + ctx_data->in_ep, + packet->bytes, + MAX_USB_PACKET_SIZE, + &transferred, + TRANSFER_TIMEOUT_MS)); + packet->packet_size = transferred; + if (status) { msg_perr("Raiden: IN transfer failed\n" - " write_count = %d\n" - " read_count = %d\n", - write_count, read_count); - return ret; + " transferred = %d\n" + " status = %d\n", + transferred, status); } + return status; +} - if ((unsigned) transferred != read_count + PACKET_HEADER_SIZE) { - msg_perr("Raiden: Read failure (read %d, expected %d)\n", - transferred, read_count + PACKET_HEADER_SIZE); - return 0x10002; +/* + * Version 1 Protocol + */ + +static int write_command_v1(const struct raiden_debug_spi_data * ctx_data, + usb_spi_transmit_ctx_t* write, + usb_spi_receive_ctx_t* read) +{ + int status; + usb_spi_packet_ctx_t command = { + .header_size = offsetof(usb_spi_command_v1_t, data), + .packet_v1.command.write_count = write->transmit_size, + .packet_v1.command.read_count = read->receive_size + }; + + fill_usb_packet(&command, write); + status = transmit_packet(ctx_data, &command); + return status; +} + +static int read_response_v1(const struct raiden_debug_spi_data * ctx_data, + usb_spi_transmit_ctx_t* write, + usb_spi_receive_ctx_t* read) +{ + int status; + usb_spi_packet_ctx_t response; + + status = receive_packet(ctx_data, &response); + if (status) { + /* Return the transfer error since the status_code is unreliable */ + return status; } + if (response.packet_v1.response.status_code) { + return response.packet_v1.response.status_code; + } + response.header_size = offsetof(usb_spi_response_v1_t, data); - memcpy(read_buffer, response_packet.data, read_count); - - return response_packet.status_code; + status = read_usb_packet(read, &response); + return status; } static int send_command(const struct flashctx *flash, - unsigned int write_count, - unsigned int read_count, - const unsigned char *write_buffer, - unsigned char *read_buffer) + unsigned int write_count, + unsigned int read_count, + const unsigned char *write_buffer, + unsigned char *read_buffer) { + const struct raiden_debug_spi_data * ctx_data = + get_raiden_data_from_context(flash); int status = -1; + usb_spi_transmit_ctx_t write_ctx = { + .buffer = write_buffer, + .transmit_size = write_count + }; + usb_spi_receive_ctx_t read_ctx = { + .buffer = read_buffer, + .receive_size = read_count + }; - for (int write_attempt = 0; write_attempt < WRITE_RETY_ATTEMPTS; + if (write_count > ctx_data->max_spi_write_count) { + msg_perr("Raiden: Invalid write count of %d, Max %d, \n", + write_count, ctx_data->max_spi_write_count); + return SPI_INVALID_LENGTH; + } + + if (read_count > ctx_data->max_spi_read_count) { + msg_perr("Raiden: Invalid read count of %d, Max %d\n", + read_count, ctx_data->max_spi_write_count); + return SPI_INVALID_LENGTH; + } + + for (int write_attempt = 0; write_attempt < WRITE_RETRY_ATTEMPTS; write_attempt++) { - status = write_command(flash, write_count, read_count, - write_buffer, read_buffer); + /* Make sure we always reset the write context. */ + write_ctx.transmit_index = 0; + if (ctx_data->protocol_version == GOOGLE_RAIDEN_SPI_PROTOCOL_V1) { + status = write_command_v1(ctx_data, &write_ctx, &read_ctx); + } if (status) { /* Write operation failed. */ msg_perr("Raiden: Write command failed\n" - "Write attempt = %d\n" - "status = %d\n", - write_attempt + 1, status); + " write count = %d\n" + " read count = %d\n" + " transmitted bytes = %d\n" + " write attempt = %d\n" + " status = %d\n", + write_count, read_count, write_ctx.transmit_index, + write_attempt + 1, status); if (!retry_recovery(status)) { /* 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); + /* Make sure we always reset the read context. */ + read_ctx.receive_index = 0; + if (ctx_data->protocol_version == GOOGLE_RAIDEN_SPI_PROTOCOL_V1) { + status = read_response_v1(ctx_data, &write_ctx, &read_ctx); + } - if (status != 0) { + if (status == 0) { + if (read_ctx.receive_size != read_ctx.receive_index) { + status = USB_SPI_HOST_RX_READ_FAILURE; + } + } + + if (status) { /* 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); + " write count = %d\n" + " read count = %d\n" + " received bytes = %d\n" + " write attempt = %d\n" + " read attempt = %d\n" + " status = %d\n", + write_count, read_count, read_ctx.receive_index, + 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); + programmer_delay(RETRY_INTERVAL_US); } else { /* We were successful at performing the SPI transfer. */ return status; } } } + return status; } -/* - * Unfortunately there doesn't seem to be a way to specify the maximum number - * of bytes that your SPI device can read/write, these values are the maximum - * data chunk size that flashrom will package up with an additional five bytes - * of command for the flash device, resulting in a 62 byte packet, that we then - * add two bytes to in either direction, making our way up to the 64 byte - * maximum USB packet size for the device. - * - * The largest command that flashrom generates is the byte program command, so - * we use that command header maximum size here. - */ -#define MAX_DATA_SIZE (PAYLOAD_SIZE - JEDEC_BYTE_PROGRAM_OUTSIZE) - static struct spi_master spi_master_raiden_debug = { - .features = SPI_MASTER_4BA, - .max_data_read = MAX_DATA_SIZE, - .max_data_write = MAX_DATA_SIZE, - .command = send_command, - .multicommand = default_spi_send_multicommand, - .read = default_spi_read, - .write_256 = default_spi_write_256, - .write_aai = default_spi_write_aai, + .features = SPI_MASTER_4BA, + .max_data_read = 0, + .max_data_write = 0, + .command = send_command, + .multicommand = default_spi_send_multicommand, + .read = default_spi_read, + .write_256 = default_spi_write_256, + .write_aai = default_spi_write_aai, }; static int match_endpoint(struct libusb_endpoint_descriptor const *descriptor, @@ -405,9 +565,9 @@ if (in_count != 1 || out_count != 1) { msg_perr("Raiden: Failed to find one IN and one OUT endpoint\n" - " found %d IN and %d OUT endpoints\n", - in_count, - out_count); + " found %d IN and %d OUT endpoints\n", + in_count, + out_count); return 1; } @@ -417,22 +577,60 @@ return 0; } +static int configure_protocol(struct spi_master *ctx_spi) +{ + struct raiden_debug_spi_data *ctx_data = + (struct raiden_debug_spi_data *)ctx_spi->data; + + ctx_data->protocol_version = + ctx_data->dev->interface_descriptor->bInterfaceProtocol; + switch(ctx_data->protocol_version) { + case GOOGLE_RAIDEN_SPI_PROTOCOL_V1: + /* + * Protocol V1 is supported by adjusting the max data + * read and write sizes which results in no continue packets. + */ + ctx_data->max_spi_write_count= SPI_TRANSFER_V1_MAX; + ctx_data->max_spi_read_count = SPI_TRANSFER_V1_MAX; + break; + default: + msg_pdbg("Raiden: Unknown USB SPI protocol version = %d", + ctx_data->protocol_version); + return USB_SPI_HOST_INIT_FAILURE; + } + /* + * Unfortunately there doesn't seem to be a way to specify the maximum number + * of bytes that your SPI device can read/write, these values are the maximum + * data chunk size that flashrom will package up with an additional five bytes + * of command for the flash device. + * + * The largest command that flashrom generates is the byte program command, so + * we use that command header maximum size here. + */ + + ctx_spi->max_data_write = ctx_data->max_spi_write_count - + JEDEC_BYTE_PROGRAM_OUTSIZE; + ctx_spi->max_data_read = ctx_data->max_spi_read_count - + JEDEC_BYTE_PROGRAM_OUTSIZE; + return 0; +} + static int raiden_debug_spi_shutdown(void * data) { - struct raiden_debug_spi_data * ctx_data = + struct raiden_debug_spi_data * ctx_data = (struct raiden_debug_spi_data *)data; int ret = LIBUSB(libusb_control_transfer( - ctx_data->dev->handle, - LIBUSB_ENDPOINT_OUT | - LIBUSB_REQUEST_TYPE_VENDOR | - LIBUSB_RECIPIENT_INTERFACE, - RAIDEN_DEBUG_SPI_REQ_DISABLE, - 0, - ctx_data->dev->interface_descriptor->bInterfaceNumber, - NULL, - 0, - TRANSFER_TIMEOUT_MS)); + ctx_data->dev->handle, + LIBUSB_ENDPOINT_OUT | + LIBUSB_REQUEST_TYPE_VENDOR | + LIBUSB_RECIPIENT_INTERFACE, + RAIDEN_DEBUG_SPI_REQ_DISABLE, + 0, + ctx_data->dev->interface_descriptor->bInterfaceNumber, + NULL, + 0, + TRANSFER_TIMEOUT_MS)); if (ret != 0) { msg_perr("Raiden: Failed to disable SPI bridge\n"); return ret; @@ -610,7 +808,13 @@ data->in_ep = in_endpoint; data->out_ep = out_endpoint; + /* The host side needs to be configured based on the device connected */ spi_master_raiden_debug.data = data; + if (configure_protocol(&spi_master_raiden_debug)) { + msg_perr("Raiden: Error configuring protocol %d\n", + data->dev->interface_descriptor->bInterfaceProtocol); + return 1; + } register_spi_master(&spi_master_raiden_debug); register_shutdown(raiden_debug_spi_shutdown, data); -- To view, visit https://review.coreboot.org/c/flashrom/+/41532 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id404af14e55fa0884e29f28880206aaad4deba66 Gerrit-Change-Number: 41532 Gerrit-PatchSet: 1 Gerrit-Owner: Brian Nemec <bnemec(a)google.com> Gerrit-Reviewer: Brian Nemec <bnemec(a)chromium.org> Gerrit-MessageType: newchange

3 45

Change in flashrom[master]: raiden_debug_spi.c: Adds USB context states and helper functions
by Brian Nemec (Code Review) July 20, 2020

July 20, 2020

Hello Brian Nemec, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/43550 to review the following change. Change subject: raiden_debug_spi.c: Adds USB context states and helper functions ...................................................................... raiden_debug_spi.c: Adds USB context states and helper functions Adds context states to handle the USB packets, these allow us to simplify the process of loading data from the transmit buffer into a USB packets' data section and from a USB packet to it's receive buffers. These will also keep track of the size of the USB packet allowing a simpler interface to transmit them. Helper functions have been added to help with copying data between the transmit and receive context states to and from the USB packets. BUG=b:139058552 BRANCH=none TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verification of the EC firmware on Nami. TEST=Builds Signed-off-by: Brian J. Nemec <bnemec(a)chromium.com> Change-Id: Id7b598b39923b4b8c1b6905e5d5c5a2be4078f96 --- M raiden_debug_spi.c 1 file changed, 202 insertions(+), 59 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/43550/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index cb7f809..1711ce6 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -155,6 +155,26 @@ GOOGLE_RAIDEN_SPI_PROTOCOL_V2 = 0x02, }; +enum { + /* The host failed to transfer the data with no libusb error. */ + USB_SPI_HOST_TX_BAD_TRANSFER = 0x10001, + /* The number of bytes written did not match expected. */ + USB_SPI_HOST_TX_WRITE_FAILURE = 0x10002, + + + /* We did not receive the expected USB packet. */ + USB_SPI_HOST_RX_UNEXPECTED_PACKET = 0x11001, + /* We received a continue packet with an invalid data index. */ + USB_SPI_HOST_RX_BAD_DATA_INDEX = 0x11002, + /* We too much data. */ + USB_SPI_HOST_RX_DATA_OVERFLOW = 0x11003, + /* The number of bytes read did not match expected. */ + USB_SPI_HOST_RX_READ_FAILURE = 0x11004, + + /* We were unable to configure the device. */ + USB_SPI_HOST_INIT_FAILURE = 0x12001, +}; + enum usb_spi_error { USB_SPI_SUCCESS = 0x0000, USB_SPI_TIMEOUT = 0x0001, @@ -213,6 +233,27 @@ uint8_t data[PAYLOAD_SIZE_V1]; } __attribute__((packed)); +union usb_spi_packet_v1 { + struct usb_spi_command_v1 command; + struct usb_spi_response_v1 response; +} __attribute__((packed)); + +struct usb_spi_packet_ctx { + union { + uint8_t bytes[USB_MAX_PACKET_SIZE]; + union usb_spi_packet_v1 packet_v1; + }; + /* + * By storing the number of bytes in the header and knowing that the + * USB data packets are all 64B long, we are able to use the header + * size to store the offset of the buffer and it's size without + * duplicating variables that can go out of sync. + */ + size_t header_size; + /* Number of bytes in the packet */ + size_t packet_size; +}; + struct usb_spi_transmit_ctx { /* Buffer we are reading data from. */ const uint8_t *buffer; @@ -265,9 +306,131 @@ } /* - * Version 1 Protocol: Responsible for constructing the packet to start - * a USB SPI transfer. Write and read counts and payloads to write from - * the write_buffer are transmitted to the device. + * Read data into the receive buffer. + * + * @param dst Destination receive context we are writing data to. + * @param src Source packet context we are reading data from. + * + * @returns status code 0 on success. + * USB_SPI_HOST_RX_DATA_OVERFLOW if the source packet is too + * large to fit in read buffer. + */ +static int read_usb_packet(struct usb_spi_receive_ctx *dst, + const struct usb_spi_packet_ctx *src) +{ + size_t max_read_length = dst->receive_size - dst->receive_index; + size_t bytes_in_buffer = src->packet_size - src->header_size; + const uint8_t *packet_buffer = src->bytes + src->header_size; + + if (bytes_in_buffer > max_read_length) { + /* + * An error occurred, we should not receive more data than + * the buffer can support. + */ + msg_perr("Raiden: Receive packet overflowed\n" + " bytes_in_buffer = %zu\n" + " max_read_length = %zu\n" + " receive_index = %zu\n" + " receive_size = %zu\n", + bytes_in_buffer, max_read_length, + dst->receive_size, dst->receive_index); + return USB_SPI_HOST_RX_DATA_OVERFLOW; + } + memcpy(dst->buffer + dst->receive_index, packet_buffer, + bytes_in_buffer); + + dst->receive_index += bytes_in_buffer; + return 0; +} + +/* + * Fill the USB packet with data from the transmit buffer. + * + * @param dst Destination packet context we are writing data to. + * @param src Source transmit context we are reading data from. + */ +static void fill_usb_packet(struct usb_spi_packet_ctx *dst, + struct usb_spi_transmit_ctx *src) +{ + size_t transmit_size = src->transmit_size - src->transmit_index; + size_t max_buffer_size = USB_MAX_PACKET_SIZE - dst->header_size; + uint8_t *packet_buffer = dst->bytes + dst->header_size; + + if (transmit_size > max_buffer_size) + transmit_size = max_buffer_size; + + memcpy(packet_buffer, src->buffer + src->transmit_index, transmit_size); + + dst->packet_size = dst->header_size + transmit_size; + src->transmit_index += transmit_size; +} + +/* + * Receive the data from the device USB endpoint and store in the packet. + * + * @param ctx_data Raiden SPI config. + * @param packet Destination packet used to store the endpoint data. + * + * @returns Returns status code with 0 on success. + */ +static int receive_packet(const struct flashctx *flash, + struct usb_spi_packet_ctx *packet) +{ + int received; + const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash); + int status = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, + ctx_data->in_ep, + packet->bytes, + USB_MAX_PACKET_SIZE, + &received, + TRANSFER_TIMEOUT_MS)); + packet->packet_size = received; + if (status) { + msg_perr("Raiden: IN transfer failed\n" + " received = %d\n" + " status = 0x%05x\n", + received, status); + } + return status; +} + +/* + * Transmit data from the packet to the device's USB endpoint. + * + * @param flash Flash context storing SPI capabilities and USB device + * information. + * @param packet Source packet we will write to the endpoint data. + * + * @returns Returns status code with 0 on success. + */ +static int transmit_packet(const struct flashctx *flash, + struct usb_spi_packet_ctx *packet) +{ + int transferred; + const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash); + int status = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, + ctx_data->out_ep, + packet->bytes, + packet->packet_size, + &transferred, + TRANSFER_TIMEOUT_MS)); + if (status || (size_t)transferred != packet->packet_size) { + if (!status) { + /* No error was reported, but we didn't transmit the data expected. */ + status = USB_SPI_HOST_TX_BAD_TRANSFER; + } + msg_perr("Raiden: OUT transfer failed\n" + " transferred = %d\n" + " packet_size = %zu\n" + " status = 0x%05x\n", + transferred, packet->packet_size, status); + + } + return status; +} + +/* + * Version 1 protocol command to start a USB SPI transfer and write the payload. * * @param flash Flash context storing SPI capabilities and USB device * information. @@ -280,38 +443,17 @@ struct usb_spi_transmit_ctx *write, struct usb_spi_receive_ctx *read) { + struct usb_spi_packet_ctx command = { + .header_size = offsetof(struct usb_spi_command_v1, data), + .packet_v1.command.write_count = write->transmit_size, + .packet_v1.command.read_count = read->receive_size + }; - int transferred; - int ret; - struct usb_spi_command_v1 command_packet; - const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash); + /* Reset the write context to the start. */ + write->transmit_index = 0; - command_packet.write_count = write->transmit_size; - command_packet.read_count = read->receive_size; - - memcpy(command_packet.data, write->buffer, write->transmit_size); - - ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, - ctx_data->out_ep, - (void*)&command_packet, - write->transmit_size + PACKET_HEADER_SIZE, - &transferred, - TRANSFER_TIMEOUT_MS)); - if (ret != 0) { - msg_perr("Raiden: OUT transfer failed\n" - " write_count = %zu\n" - " read_count = %zu\n", - write->transmit_size, read->receive_size); - return ret; - } - - if ((unsigned) transferred != write->transmit_size + PACKET_HEADER_SIZE) { - msg_perr("Raiden: Write failure (wrote %d, expected %zu)\n", - transferred, write->transmit_size + PACKET_HEADER_SIZE); - return 0x10001; - } - - return 0; + fill_usb_packet(&command, write); + return transmit_packet(flash, &command); } /* @@ -330,34 +472,24 @@ struct usb_spi_transmit_ctx *write, struct usb_spi_receive_ctx *read) { - int transferred; - int ret; - struct usb_spi_response_v1 response_packet; - const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash); + int status; + struct usb_spi_packet_ctx response; - ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, - ctx_data->in_ep, - (void*)&response_packet, - read->receive_size + PACKET_HEADER_SIZE, - &transferred, - TRANSFER_TIMEOUT_MS)); - if (ret != 0) { - msg_perr("Raiden: IN transfer failed\n" - " write_count = %zu\n" - " read_count = %zu\n", - write->transmit_size, read->receive_size); - return ret; + /* Reset the read context to the start. */ + read->receive_index = 0; + + status = receive_packet(flash, &response); + if (status) { + /* Return the transfer error since the status_code is unreliable */ + return status; } - - if ((unsigned) transferred != read->receive_size + PACKET_HEADER_SIZE) { - msg_perr("Raiden: Read failure (read %d, expected %zu)\n", - transferred, read->receive_size + PACKET_HEADER_SIZE); - return 0x10002; + if (response.packet_v1.response.status_code) { + return response.packet_v1.response.status_code; } + response.header_size = offsetof(struct usb_spi_response_v1, data); - memcpy(read->buffer, response_packet.data, read->receive_size); - - return response_packet.status_code; + status = read_usb_packet(read, &response); + return status; } /* @@ -413,6 +545,12 @@ status = write_command_v1(flash, &write_ctx, &read_ctx); + if (!status && + (write_ctx.transmit_index != write_ctx.transmit_size)) { + /* No errors were reported, but write is incomplete. */ + status = USB_SPI_HOST_TX_WRITE_FAILURE; + } + if (status) { /* Write operation failed. */ msg_perr("Raiden: Write command failed\n" @@ -437,8 +575,13 @@ status = read_response_v1(flash, &write_ctx, &read_ctx); if (!status) { - /* Successful transfer. */ - return status; + if (read_ctx.receive_size == read_ctx.receive_index) { + /* Successful transfer. */ + return status; + } else { + /* Report the error from the failed read. */ + status = USB_SPI_HOST_RX_READ_FAILURE; + } } if (status) { -- To view, visit https://review.coreboot.org/c/flashrom/+/43550 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id7b598b39923b4b8c1b6905e5d5c5a2be4078f96 Gerrit-Change-Number: 43550 Gerrit-PatchSet: 1 Gerrit-Owner: Brian Nemec <bnemec(a)google.com> Gerrit-Reviewer: Brian Nemec <bnemec(a)chromium.org> Gerrit-MessageType: newchange

3 14

Change in flashrom[master]: nicintel: add I350 support
by Martijn Berger (Code Review) July 20, 2020

July 20, 2020

Martijn Berger has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39963 ) Change subject: nicintel: add I350 support ...................................................................... nicintel: add I350 support While trying to flash I350 Intel NIC's on arm64 linux I had to change some things to make it work well Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Signed-off-by: Martijn Berger <martijn.berger(a)gmail.com> --- M nicintel_eeprom.c M nicintel_spi.c 2 files changed, 39 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/39963/1 diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index 4d45f79..be03e41 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -27,6 +27,9 @@ * 8.4.3: EEPROM-Mode Read Register * 8.4.6: EEPROM-Mode Write Register * Write process inspired on kernel e1000_i210.c + * + * Intel I350 Gigabit controller Datasheet + * https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/eth… */ #include <stdlib.h> @@ -75,8 +78,6 @@ static struct pci_dev *nicintel_pci; static bool done_i20_write = false; -#define UNPROG_DEVICE 0x1509 - /* * Warning: is_i210() below makes assumptions on these PCI ids. * It may have to be updated when this list is extended. @@ -87,7 +88,7 @@ {PCI_VENDOR_ID_INTEL, 0x1510, NT , "Intel", "82580 Quad Gigabit Ethernet Controller (Backplane)"}, {PCI_VENDOR_ID_INTEL, 0x1511, NT , "Intel", "82580 Quad Gigabit Ethernet Controller (Ext. PHY)"}, {PCI_VENDOR_ID_INTEL, 0x1511, NT , "Intel", "82580 Dual Gigabit Ethernet Controller (Copper)"}, - {PCI_VENDOR_ID_INTEL, UNPROG_DEVICE, OK, "Intel", "Unprogrammed 82580 Quad/Dual Gigabit Ethernet Controller"}, + {PCI_VENDOR_ID_INTEL, 0x1509, OK, "Intel", "Unprogrammed 82580 Quad/Dual Gigabit Ethernet Controller"}, {PCI_VENDOR_ID_INTEL, 0x1531, NT, "Intel", "I210 Gigabit Network Connection Unprogrammed"}, {PCI_VENDOR_ID_INTEL, 0x1532, NT, "Intel", "I211 Gigabit Network Connection Unprogrammed"}, {PCI_VENDOR_ID_INTEL, 0x1533, OK, "Intel", "I210 Gigabit Network Connection"}, @@ -95,9 +96,26 @@ {PCI_VENDOR_ID_INTEL, 0x1537, NT, "Intel", "I210 Gigabit Network Connection SERDES Backplane"}, {PCI_VENDOR_ID_INTEL, 0x1538, NT, "Intel", "I210 Gigabit Network Connection SGMII"}, {PCI_VENDOR_ID_INTEL, 0x1539, NT, "Intel", "I211 Gigabit Network Connection"}, + {PCI_VENDOR_ID_INTEL, 0x151f, OK, "Intel", "I350 Gigabit Network Connection Unprogrammed"}, + {PCI_VENDOR_ID_INTEL, 0x1521, OK, "Intel", "I350 Gigabit Network Connection Copper"}, + {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection Fiber"}, + {PCI_VENDOR_ID_INTEL, 0x1523, NT, "Intel", "I350 Gigabit Network Connection 1000BASE-KX / 1000BASE-BX backplane"}, + {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"}, {0}, }; +static inline bool is_unprog_device(uint16_t device_id) +{ + switch (device_id) + { + case 0x1509: // Unprogrammed 82580 Quad/Dual Gigabit Ethernet Controller + case 0x151f: // I350 Gigabit Network Connection Unprogrammed + return true; + default: + return false; + } +} + static inline bool is_i210(uint16_t device_id) { return (device_id & 0xfff0) == 0x1530; @@ -118,7 +136,7 @@ static int nicintel_ee_probe_82580(struct flashctx *flash) { - if (nicintel_pci->device_id == UNPROG_DEVICE) + if (is_unprog_device(nicintel_pci->device_id)) flash->chip->total_size = 16; /* Fall back to minimum supported size. */ else { uint32_t tmp = pci_mmio_readl(nicintel_eebar + EEC); @@ -456,7 +474,7 @@ if (!dev) return 1; - uint32_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); + uintptr_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); if (!io_base_addr) return 1; @@ -466,7 +484,7 @@ return 1; nicintel_pci = dev; - if ((dev->device_id != UNPROG_DEVICE)) { + if (!is_unprog_device(dev->device_id)) { uint32_t eec = pci_mmio_readl(nicintel_eebar + EEC); /* C.f. 3.3.1.5 for the detection mechanism (maybe? contradicting diff --git a/nicintel_spi.c b/nicintel_spi.c index 1173ef7..09a37dc 100644 --- a/nicintel_spi.c +++ b/nicintel_spi.c @@ -28,6 +28,9 @@ * * Intel 82599 10 GbE Controller Datasheet (331520) * http://www.intel.com/content/dam/www/public/us/en/documents/datasheets/8259… + * + * Intel I350 Gigabit controller Datasheet + * https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/eth… */ #include <stdlib.h> @@ -104,6 +107,11 @@ {PCI_VENDOR_ID_INTEL, 0x1538, NT, "Intel", "I210 Gigabit Network Connection SGMII"}, {PCI_VENDOR_ID_INTEL, 0x1539, NT, "Intel", "I211 Gigabit Network Connection"}, + {PCI_VENDOR_ID_INTEL, 0x151f, NT, "Intel", "I350 Gigabit Network Connection Unprogrammed"}, + {PCI_VENDOR_ID_INTEL, 0x1521, NT, "Intel", "I350 Gigabit Network Connection Copper"}, + {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection Fiber"}, + {PCI_VENDOR_ID_INTEL, 0x1523, NT, "Intel", "I350 Gigabit Network Connection 1000BASE-KX / 1000BASE-BX backplane"}, + {PCI_VENDOR_ID_INTEL, 0x1522, NT, "Intel", "I350 Gigabit Network Connection External SGMII PHY"}, {0}, }; @@ -252,7 +260,7 @@ if (!dev) return 1; - uint32_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); + uintptr_t io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0); if (!io_base_addr) return 1; @@ -266,6 +274,12 @@ MEMMAP_SIZE); if (!nicintel_spibar || nicintel_spi_82599_enable_flash()) return 1; + } else if ((dev->device_id & 0xfff0) == 0x1510 || + (dev->device_id & 0xfff0) == 0x1520) { + nicintel_spibar = rphysmap("Intel I350 Gigabit w/ SPI flash", io_base_addr, + MEMMAP_SIZE); + if (!nicintel_spibar || nicintel_spi_i210_enable_flash()) + return 1; } else { nicintel_spibar = rphysmap("Intel 10 Gigabit NIC w/ SPI flash", io_base_addr + 0x10000, MEMMAP_SIZE); -- To view, visit https://review.coreboot.org/c/flashrom/+/39963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id203d2ce247cf39a8d650887322ccced0151c6a9 Gerrit-Change-Number: 39963 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-MessageType: newchange

3 13

Change in flashrom[master]: raiden_debug_spi.c: Adds context states and helper functions
by Brian Nemec (Code Review) July 19, 2020

July 19, 2020

Hello Brian Nemec, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/41608 to review the following change. Change subject: raiden_debug_spi.c: Adds context states and helper functions ...................................................................... raiden_debug_spi.c: Adds context states and helper functions Adds context states to handle the read and write buffers as transmit and receive states. These are used to keep track of the number of bytes transmitted and received allowing future support of multi-packet messages in the v2 protocol. Adds context states to handle the USB packets, these allow us to simplify the process of loading data from the transmit buffer into a USB packets' data section and from a USB packet to it's receive buffers. These will also keep track of the size of the USB packet allowing a simpler interface to transmit them. Helper functions have been added to help with copying data between the transmit and receive context states to and from the USB packets. BUG=b:139058552 BRANCH=none TEST=Manual testing of ServoMicro and Flashrom when performing reads, writes, and verification of the EC firmware on Nami. TEST=Builds Signed-off-by: Brian J. Nemec <bnemec(a)chromium.com> Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c --- M raiden_debug_spi.c 1 file changed, 241 insertions(+), 94 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/41608/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index b18d580..4de8038 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -141,7 +141,21 @@ enum { GOOGLE_RAIDEN_SPI_PROTOCOL_V1 = 0x01, - GOOGLE_RAIDEN_SPI_PROTOCOL_V2 = 0x02, +}; + +enum { + /* The host failed to transfer the data with no libusb error. */ + USB_SPI_HOST_TX_BAD_TRANSFER = 0x10001, + /* We did not receive the expected USB packet. */ + USB_SPI_HOST_RX_UNEXPECTED_PACKET = 0x18002, + /* We received a continue packet with an invalid data index.*/ + USB_SPI_HOST_RX_BAD_DATA_INDEX = 0x18003, + /* We too much data. */ + USB_SPI_HOST_RX_DATA_OVERFLOW = 0x18004, + /* We too little data. */ + USB_SPI_HOST_RX_READ_FAILURE = 0x18005, + /* We were unable to configure the device. */ + USB_SPI_HOST_INIT_FAILURE = 0x18006, }; enum usb_spi_error { @@ -162,8 +176,8 @@ }; #define PACKET_HEADER_SIZE (2) -#define MAX_PACKET_SIZE (64) -#define PAYLOAD_SIZE_V1 (MAX_PACKET_SIZE - PACKET_HEADER_SIZE) +#define MAX_USB_PACKET_SIZE (64) +#define PAYLOAD_SIZE_V1 (MAX_USB_PACKET_SIZE - PACKET_HEADER_SIZE) /* * Servo Micro has an error where it is capable of acknowledging USB packets @@ -180,11 +194,10 @@ */ #define TRANSFER_TIMEOUT_MS (200 + 800) -struct raiden_debug_spi_data { - struct usb_device *dev; - uint8_t in_ep; - uint8_t out_ep; -}; + +/* + * Version 1 protocol specific attributes + */ typedef struct { int8_t write_count; @@ -198,6 +211,51 @@ uint8_t data[PAYLOAD_SIZE_V1]; } __attribute__((packed)) usb_spi_response_v1_t; +typedef union { + usb_spi_command_v1_t command; + usb_spi_response_v1_t response; +} __attribute__((packed)) usb_spi_packet_v1_t; + +typedef struct { + union { + uint8_t bytes[MAX_USB_PACKET_SIZE]; + usb_spi_packet_v1_t packet_v1; + }; + /* + * By storing the number of bytes in the header and knowing that the + * USB data packets are all 64B long, we are able to use the header + * size to store the offset of the buffer and it's size without + * duplicating variables that can go out of sync. + */ + int header_size; + /* Number of bytes in the packet */ + int packet_size; +} usb_spi_packet_ctx_t; + +typedef struct { + /* Buffer we are reading data from. */ + const uint8_t *buffer; + /* Total size of the transmit transfer. */ + int transmit_size; + /* Number of bytes transmitted. */ + int transmit_index; +} usb_spi_transmit_ctx_t; + +typedef struct { + /* Buffer we are writing data into. */ + uint8_t *buffer; + /* Total size of the receive transfer. */ + int receive_size; + /* Number of bytes received. */ + int receive_index; +} usb_spi_receive_ctx_t; + +struct raiden_debug_spi_data { + struct usb_device *dev; + uint8_t in_ep; + uint8_t out_ep; +}; + /* * 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 @@ -213,7 +271,7 @@ * during transfer errors to the device and can be recovered. */ if (USB_SPI_READ_COUNT_INVALID <= error_code && - error_code <= USB_SPI_DISABLED) { + error_code <= USB_SPI_DISABLED) { return false; } } else if (usb_device_is_libusb_error(error_code)) { @@ -231,90 +289,139 @@ return (const struct raiden_debug_spi_data *)flash->mst->spi.data; } -static int write_command(const struct flashctx *flash, - unsigned int write_count, - unsigned int read_count, - const unsigned char *write_buffer, - unsigned char *read_buffer) +/* + * Fill the data section in the USB packets. + */ +static int fill_usb_packet(usb_spi_packet_ctx_t* dst, + usb_spi_transmit_ctx_t* src) { + int transfer_size = src->transmit_size - src->transmit_index; + int max_buffer_size = MAX_USB_PACKET_SIZE - dst->header_size; + uint8_t *packet_buffer = dst->bytes + dst->header_size; - int transferred; - int ret; - usb_spi_command_v1_t command_packet; - const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash); - - if (write_count > PAYLOAD_SIZE_V1) { - msg_perr("Raiden: Invalid write_count of %d\n", write_count); - return SPI_INVALID_LENGTH; + if (transfer_size > max_buffer_size) { + transfer_size = max_buffer_size; } + memcpy(packet_buffer, src->buffer + src->transmit_index, transfer_size); - if (read_count > PAYLOAD_SIZE_V1) { - msg_perr("Raiden: Invalid read_count of %d\n", read_count); - return SPI_INVALID_LENGTH; - } - - command_packet.write_count = write_count; - command_packet.read_count = read_count; - - memcpy(command_packet.data, write_buffer, write_count); - - ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, - ctx_data->out_ep, - (void*)&command_packet, - write_count + PACKET_HEADER_SIZE, - &transferred, - TRANSFER_TIMEOUT_MS)); - if (ret != 0) { - msg_perr("Raiden: OUT transfer failed\n" - " write_count = %d\n" - " read_count = %d\n", - write_count, read_count); - return ret; - } - - if ((unsigned) transferred != write_count + PACKET_HEADER_SIZE) { - msg_perr("Raiden: Write failure (wrote %d, expected %d)\n", - transferred, write_count + PACKET_HEADER_SIZE); - return 0x10001; - } - + dst->packet_size = dst->header_size + transfer_size; + src->transmit_index += transfer_size; return 0; } -static int read_response(const struct flashctx *flash, - unsigned int write_count, - unsigned int read_count, - const unsigned char *write_buffer, - unsigned char *read_buffer) +static int read_usb_packet(usb_spi_receive_ctx_t* dst, + const usb_spi_packet_ctx_t* src) +{ + int max_read_length = dst->receive_size - dst->receive_index; + int bytes_in_buffer = src->packet_size - src->header_size; + const uint8_t *packet_buffer = src->bytes + src->header_size; + + if (bytes_in_buffer > max_read_length) { + /* + * An error occurred, we should not receive more data than + * the buffer can support. + */ + msg_perr("Raiden: Receive packet overflowed\n" + " bytes_in_buffer = %d\n" + " max_read_length = %d\n" + " receive_index = %d\n" + " receive_size = %d\n", + bytes_in_buffer, max_read_length, + dst->receive_size, dst->receive_index); + return USB_SPI_HOST_RX_DATA_OVERFLOW; + } + memcpy(dst->buffer + dst->receive_index, packet_buffer, bytes_in_buffer); + + dst->receive_index += bytes_in_buffer; + return 0; +} + +static int transmit_packet(const struct raiden_debug_spi_data * ctx_data, + usb_spi_packet_ctx_t* packet) { int transferred; - int ret; - usb_spi_response_v1_t response_packet; - const struct raiden_debug_spi_data * ctx_data = get_raiden_data_from_context(flash); - - ret = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, - ctx_data->in_ep, - (void*)&response_packet, - read_count + PACKET_HEADER_SIZE, + int status = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, + ctx_data->out_ep, + packet->bytes, + packet->packet_size, &transferred, TRANSFER_TIMEOUT_MS)); - if (ret != 0) { + if (status || transferred != packet->packet_size) { + if (!status) { + /* No error was reported, but we didn't transmit the data expected. */ + status = USB_SPI_HOST_TX_BAD_TRANSFER; + } + msg_perr("Raiden: OUT transfer failed\n" + " transferred = %d\n" + " packet_size = %d\n" + " status = %d\n", + transferred, packet->packet_size, status); + + } + return status; +} + +static int receive_packet(const struct raiden_debug_spi_data * ctx_data, + usb_spi_packet_ctx_t* packet) +{ + int received; + int status = LIBUSB(libusb_bulk_transfer(ctx_data->dev->handle, + ctx_data->in_ep, + packet->bytes, + MAX_USB_PACKET_SIZE, + &received, + TRANSFER_TIMEOUT_MS)); + packet->packet_size = received; + if (status) { msg_perr("Raiden: IN transfer failed\n" - " write_count = %d\n" - " read_count = %d\n", - write_count, read_count); - return ret; + " received = %d\n" + " status = %d\n", + received, status); } + return status; +} - if ((unsigned) transferred != read_count + PACKET_HEADER_SIZE) { - msg_perr("Raiden: Read failure (read %d, expected %d)\n", - transferred, read_count + PACKET_HEADER_SIZE); - return 0x10002; +/* + * Version 1 Protocol + */ + +static int write_command_v1(const struct raiden_debug_spi_data * ctx_data, + usb_spi_transmit_ctx_t* write, usb_spi_receive_ctx_t* read) +{ + usb_spi_packet_ctx_t command = { + .header_size = offsetof(usb_spi_command_v1_t, data), + .packet_v1.command.write_count = write->transmit_size, + .packet_v1.command.read_count = read->receive_size + }; + + /* Reset the write context to the start. */ + write->transmit_index = 0; + + fill_usb_packet(&command, write); + return transmit_packet(ctx_data, &command); +} + +static int read_response_v1(const struct raiden_debug_spi_data * ctx_data, + usb_spi_transmit_ctx_t* write, usb_spi_receive_ctx_t* read) +{ + int status; + usb_spi_packet_ctx_t response; + + /* Reset the read context to the start. */ + read->receive_index = 0; + + status = receive_packet(ctx_data, &response); + if (status) { + /* Return the transfer error since the status_code is unreliable */ + return status; } + if (response.packet_v1.response.status_code) { + return response.packet_v1.response.status_code; + } + response.header_size = offsetof(usb_spi_response_v1_t, data); - memcpy(read_buffer, response_packet.data, read_count); - - return response_packet.status_code; + status = read_usb_packet(read, &response); + return status; } static int send_command(const struct flashctx *flash, @@ -323,20 +430,50 @@ const unsigned char *write_buffer, unsigned char *read_buffer) { + const struct raiden_debug_spi_data * ctx_data = + get_raiden_data_from_context(flash); int status = -1; - for (int write_attempt = 0; write_attempt < WRITE_RETRY_ATTEMPTS; - write_attempt++) { + usb_spi_transmit_ctx_t write_ctx = { + .buffer = write_buffer, + .transmit_size = write_count + }; + usb_spi_receive_ctx_t read_ctx = { + .buffer = read_buffer, + .receive_size = read_count + }; - status = write_command(flash, write_count, read_count, - write_buffer, read_buffer); + if (write_count > PAYLOAD_SIZE_V1) { + msg_perr("Raiden: Invalid read count\n" + " write count = %d\n" + " max write = %d\n", + read_count, PAYLOAD_SIZE_V1); + return SPI_INVALID_LENGTH; + } + + if (read_count > PAYLOAD_SIZE_V1) { + msg_perr("Raiden: Invalid read count\n" + " read count = %d\n" + " max read = %d\n", + read_count, PAYLOAD_SIZE_V1); + return SPI_INVALID_LENGTH; + } + + for (int write_attempt = 0; write_attempt < WRITE_RETRY_ATTEMPTS; + write_attempt++) { + + status = write_command_v1(ctx_data, &write_ctx, &read_ctx); if (status) { /* Write operation failed. */ msg_perr("Raiden: Write command failed\n" - "Write attempt = %d\n" - "status = %d\n", - write_attempt + 1, status); + " write count = %d\n" + " read count = %d\n" + " transmitted bytes = %d\n" + " write attempt = %d\n" + " status = %d\n", + write_count, read_count, write_ctx.transmit_index, + write_attempt + 1, status); if (!retry_recovery(status)) { /* Reattempting will not result in a recovery. */ return status; @@ -344,26 +481,36 @@ programmer_delay(RETRY_INTERVAL_US); continue; } - for (int read_attempt = 0; read_attempt < READ_RETRY_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); + status = read_response_v1(ctx_data, &write_ctx, &read_ctx); + + if (!status) { + if (read_ctx.receive_size == read_ctx.receive_index) { + /* No errors and read is complete. */ + return status; + } else { + status = USB_SPI_HOST_RX_READ_FAILURE; + } + } if (status) { /* 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); + " write count = %d\n" + " read count = %d\n" + " received bytes = %d\n" + " write attempt = %d\n" + " read attempt = %d\n" + " status = %d\n", + write_count, read_count, read_ctx.receive_index, + write_attempt + 1, read_attempt + 1, status); if (!retry_recovery(status)) { /* Reattempting will not result in a recovery. */ return status; } programmer_delay(RETRY_INTERVAL_US); - } else { - /* We were successful at performing the SPI transfer. */ - return status; } } } -- To view, visit https://review.coreboot.org/c/flashrom/+/41608 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic6eea82ffc604ec56278f7aaa0deafe0cf75973c Gerrit-Change-Number: 41608 Gerrit-PatchSet: 1 Gerrit-Owner: Brian Nemec <bnemec(a)google.com> Gerrit-Reviewer: Brian Nemec <bnemec(a)chromium.org> Gerrit-MessageType: newchange

3 24

Change in flashrom[master]: raiden_debug_spi.c: Cleanup of the USB SPI protocol
by Brian Nemec (Code Review) July 19, 2020

July 19, 2020

Hello Brian Nemec, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/41597 to review the following change. Change subject: raiden_debug_spi.c: Cleanup of the USB SPI protocol ...................................................................... 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(a)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 https://review.coreboot.org/c/flashrom/+/41597 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I17e62dabee2724eecf8d5a1a7827f06f0c7514df Gerrit-Change-Number: 41597 Gerrit-PatchSet: 1 Gerrit-Owner: Brian Nemec <bnemec(a)google.com> Gerrit-Reviewer: Brian Nemec <bnemec(a)chromium.org> Gerrit-MessageType: newchange

3 17