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

Change in flashrom[master]: raiden_debug_spi.c: Refactor to support multiple protocols
by Brian Nemec (Code Review) 21 Jul '20

21 Jul '20

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]: nicintel: add I350 support
by Martijn Berger (Code Review) 20 Jul '20

20 Jul '20

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) 20 Jul '20

20 Jul '20

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) 20 Jul '20

20 Jul '20

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

Change in flashrom[master]: raiden_debug_spi.c: Renaming Protocol V1 specific fields
by Brian Nemec (Code Review) 20 Jul '20

20 Jul '20

Hello Brian Nemec, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/41596 to review the following change. Change subject: raiden_debug_spi.c: Renaming Protocol V1 specific fields ...................................................................... raiden_debug_spi.c: Renaming Protocol V1 specific fields Renames the structures from the USB SPI which are specific to the V1 protocol. BUG=b:139058552 BRANCH=none TEST=Builds Signed-off-by: Brian J. Nemec <bnemec(a)chromium.com> Change-Id: I70b43af50d872d850dae287d99bcd768107a1cad --- M raiden_debug_spi.c 1 file changed, 27 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/41596/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index ac74c0b..5259981 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -123,7 +123,11 @@ #define GOOGLE_VID (0x18D1) #define GOOGLE_RAIDEN_SPI_SUBCLASS (0x51) -#define GOOGLE_RAIDEN_SPI_PROTOCOL (0x01) + +enum { + GOOGLE_RAIDEN_SPI_PROTOCOL_V1 = 0x01, + GOOGLE_RAIDEN_SPI_PROTOCOL_V2 = 0x02, +}; enum usb_spi_error { USB_SPI_SUCCESS = 0x0000, @@ -144,7 +148,7 @@ #define PACKET_HEADER_SIZE (2) #define MAX_PACKET_SIZE (64) -#define PAYLOAD_SIZE (MAX_PACKET_SIZE - PACKET_HEADER_SIZE) +#define PAYLOAD_SIZE_V1 (MAX_PACKET_SIZE - PACKET_HEADER_SIZE) /* * Servo Micro has an error where it is capable of acknowledging USB packets @@ -152,9 +156,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_RETY_ATTEMPTS (3) +#define READ_RETY_ATTEMPTS (3) +#define RETY_INTERVAL_US (100 * 1000) /* * This timeout is so large because the Raiden SPI timeout is 800ms. @@ -171,13 +175,13 @@ 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; /* * This function will return true when an error code can potentially recover @@ -221,15 +225,15 @@ int transferred; int ret; - usb_spi_command_t command_packet; + 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) { + if (write_count > PAYLOAD_SIZE_V1) { msg_perr("Raiden: Invalid write_count of %d\n", write_count); return SPI_INVALID_LENGTH; } - if (read_count > PAYLOAD_SIZE) { + if (read_count > PAYLOAD_SIZE_V1) { msg_perr("Raiden: Invalid read_count of %d\n", read_count); return SPI_INVALID_LENGTH; } @@ -270,7 +274,7 @@ { int transferred; int ret; - usb_spi_response_t response_packet; + 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, @@ -362,17 +366,17 @@ * 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) +#define MAX_DATA_SIZE (PAYLOAD_SIZE_V1 - 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 = 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, }; static int match_endpoint(struct libusb_endpoint_descriptor const *descriptor, @@ -494,7 +498,7 @@ 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); + 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/+/41596 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I70b43af50d872d850dae287d99bcd768107a1cad Gerrit-Change-Number: 41596 Gerrit-PatchSet: 1 Gerrit-Owner: Brian Nemec <bnemec(a)google.com> Gerrit-Reviewer: Brian Nemec <bnemec(a)chromium.org> Gerrit-MessageType: newchange

3 10

Change in flashrom[master]: Install the man file when using meson as a buildsystem
by Richard Hughes (Code Review) 15 Jul '20

15 Jul '20

Richard Hughes has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38939 ) Change subject: Install the man file when using meson as a buildsystem ...................................................................... Install the man file when using meson as a buildsystem This fixes a regression with the Fedora package. Change-Id: I881bd5002a842072ce9dadea033c51a2668f9e7c Signed-off-by: Richard Hughes <richard(a)hughsie.com> --- M meson.build 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/38939/1 diff --git a/meson.build b/meson.build index 375089c..df39290 100644 --- a/meson.build +++ b/meson.build @@ -299,6 +299,7 @@ prefix = get_option('prefix') sbindir = join_paths(prefix, get_option('sbindir')) libdir = join_paths(prefix, get_option('libdir')) +mandir = join_paths(prefix, get_option('mandir')) install_headers([ 'libflashrom.h', @@ -372,6 +373,14 @@ description : 'library to interact with flashrom', ) +configure_file( + input : 'flashrom.8.tmpl', + output : 'flashrom.8', + copy: true, + install: true, + install_dir: join_paths(mandir, 'man8'), +) + flashrom_dep = declare_dependency( link_with : flashrom, include_directories : include_directories('.'), -- To view, visit https://review.coreboot.org/c/flashrom/+/38939 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I881bd5002a842072ce9dadea033c51a2668f9e7c Gerrit-Change-Number: 38939 Gerrit-PatchSet: 1 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-MessageType: newchange

7 14

Change in flashrom[master]: pcidev: On arm64 we are not getting a valid BAR0 address
by Martijn Berger (Code Review) 14 Jul '20

14 Jul '20

Martijn Berger has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39965 ) Change subject: pcidev: On arm64 we are not getting a valid BAR0 address ...................................................................... pcidev: On arm64 we are not getting a valid BAR0 address It seems that there is some code to work around a possible bug in old versions of libpci. This trusts libpci if it is new enough Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Signed-off-by: Martijn Berger <martijn.berger(a)gmail.com> --- M pcidev.c M programmer.h 2 files changed, 30 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/39965/1 diff --git a/pcidev.c b/pcidev.c index 54c1fd3..973acde 100644 --- a/pcidev.c +++ b/pcidev.c @@ -30,20 +30,47 @@ TYPE_UNKNOWN }; +int pci_bar_nuber_from_offset(int offset) +{ + switch (offset) { + case PCI_BASE_ADDRESS_0: + return 0; + case PCI_BASE_ADDRESS_1: + return 1; + case PCI_BASE_ADDRESS_2: + return 2; + case PCI_BASE_ADDRESS_3: + return 3; + case PCI_BASE_ADDRESS_4: + return 4; + case PCI_BASE_ADDRESS_5: + return 5; + default: + return -1; + } + return -1; +} + uintptr_t pcidev_readbar(struct pci_dev *dev, int bar) { uint64_t addr; uint32_t upperaddr; uint8_t headertype; uint16_t supported_cycles; + int bar_number; enum pci_bartype bartype = TYPE_UNKNOWN; headertype = pci_read_byte(dev, PCI_HEADER_TYPE) & 0x7f; msg_pspew("PCI header type 0x%02x\n", headertype); + bar_number = pci_bar_nuber_from_offset(bar); - /* Don't use dev->base_addr[x] (as value for 'bar'), won't work on older libpci. */ - addr = pci_read_long(dev, bar); + if (PCI_LIB_VERSION >= 0x030500 && bar_number > -1) { + addr = dev->base_addr[bar_number]; + } else { + /* Don't use dev->base_addr[x] (as value for 'bar'), won't work on older libpci. */ + addr = pci_read_long(dev, bar); + } /* Sanity checks. */ switch (headertype) { diff --git a/programmer.h b/programmer.h index 9a41be1..5d4c1d7 100644 --- a/programmer.h +++ b/programmer.h @@ -190,6 +190,7 @@ // FIXME: This needs to be local, not global(?) extern struct pci_access *pacc; int pci_init_common(void); +int pci_bar_nuber_from_offset(int offset); uintptr_t pcidev_readbar(struct pci_dev *dev, int bar); struct pci_dev *pcidev_init(const struct dev_entry *devs, int bar); /* rpci_write_* are reversible writes. The original PCI config space register -- To view, visit https://review.coreboot.org/c/flashrom/+/39965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8bd32c6344b0831a949c3853abeb84905420922a Gerrit-Change-Number: 39965 Gerrit-PatchSet: 1 Gerrit-Owner: Martijn Berger Gerrit-MessageType: newchange

4 5

Change in flashrom[master]: sb600spi -> fchspi: Rename AMD SPI programmer
by Edward O'Callaghan (Code Review) 24 Jun '20

24 Jun '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36434 ) Change subject: sb600spi -> fchspi: Rename AMD SPI programmer ...................................................................... sb600spi -> fchspi: Rename AMD SPI programmer The AMD FCH controller has two main SPI ip core variants, the so-called 'sb600' variant and the newer 'spi100' variant. The sb600spi.c driver supports both variants and so its name isn't very general. Here we rename it to be more general. Change-Id: Iad92fb224954d66968e7aeb75cb515f075a26ac8 Signed-off-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M Makefile M chipset_enable.c R fchspi.c M meson.build M programmer.h 5 files changed, 11 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/36434/1 diff --git a/Makefile b/Makefile index f0a1b61..143a8c1 100644 --- a/Makefile +++ b/Makefile @@ -767,7 +767,7 @@ FEATURE_CFLAGS += -D'CONFIG_INTERNAL=1' PROGRAMMER_OBJS += processor_enable.o chipset_enable.o board_enable.o cbtable.o internal.o ifeq ($(ARCH), x86) -PROGRAMMER_OBJS += it87spi.o it85spi.o sb600spi.o amd_imc.o wbsio_spi.o mcp6x_spi.o +PROGRAMMER_OBJS += it87spi.o it85spi.o fchspi.o amd_imc.o wbsio_spi.o mcp6x_spi.o PROGRAMMER_OBJS += ichspi.o dmi.o ifeq ($(CONFIG_INTERNAL_DMI), yes) FEATURE_CFLAGS += -D'CONFIG_INTERNAL_DMI=1' diff --git a/chipset_enable.c b/chipset_enable.c index b55852c..e57a18e 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1297,7 +1297,7 @@ return enable_flash_amd_via(dev, name, 0xE0); } -static int enable_flash_sb600(struct pci_dev *dev, const char *name) +static int enable_flash_fch(struct pci_dev *dev, const char *name) { uint32_t prot; uint8_t reg; @@ -1330,7 +1330,7 @@ internal_buses_supported &= BUS_LPC | BUS_FWH; - ret = sb600_probe_spi(dev); + ret = fch_probe_spi(dev); /* Read ROM strap override register. */ OUTB(0x8f, 0xcd6); @@ -1685,16 +1685,16 @@ const struct penable chipset_enables[] = { #if defined(__i386__) || defined(__x86_64__) {0x1002, 0x4377, B_PFL, OK, "ATI", "SB400", enable_flash_sb400}, - {0x1002, 0x438d, B_FLS, OK, "AMD", "SB600", enable_flash_sb600}, - {0x1002, 0x439d, B_FLS, OK, "AMD", "SB7x0/SB8x0/SB9x0", enable_flash_sb600}, + {0x1002, 0x438d, B_FLS, OK, "AMD", "SB600", enable_flash_fch}, + {0x1002, 0x439d, B_FLS, OK, "AMD", "SB7x0/SB8x0/SB9x0", enable_flash_fch}, {0x100b, 0x0510, B_PFL, NT, "AMD", "SC1100", enable_flash_sc1100}, {0x1022, 0x2080, B_PFL, OK, "AMD", "CS5536", enable_flash_cs5536}, {0x1022, 0x2090, B_PFL, OK, "AMD", "CS5536", enable_flash_cs5536}, {0x1022, 0x3000, B_PFL, OK, "AMD", "Elan SC520", get_flashbase_sc520}, {0x1022, 0x7440, B_PFL, OK, "AMD", "AMD-768", enable_flash_amd_768_8111}, {0x1022, 0x7468, B_PFL, OK, "AMD", "AMD-8111", enable_flash_amd_768_8111}, - {0x1022, 0x780e, B_FLS, OK, "AMD", "FCH", enable_flash_sb600}, - {0x1022, 0x790e, B_FLS, OK, "AMD", "FP4", enable_flash_sb600}, + {0x1022, 0x780e, B_FLS, OK, "AMD", "FCH", enable_flash_fch}, + {0x1022, 0x790e, B_FLS, OK, "AMD", "FP4", enable_flash_fch}, {0x1039, 0x0406, B_PFL, NT, "SiS", "501/5101/5501", enable_flash_sis501}, {0x1039, 0x0496, B_PFL, NT, "SiS", "85C496+497", enable_flash_sis85c496}, {0x1039, 0x0530, B_PFL, OK, "SiS", "530", enable_flash_sis530}, diff --git a/sb600spi.c b/fchspi.c similarity index 99% rename from sb600spi.c rename to fchspi.c index 7bdd11f..e0eab07 100644 --- a/sb600spi.c +++ b/fchspi.c @@ -579,7 +579,7 @@ return amd_imc_shutdown(dev); } -int sb600_probe_spi(struct pci_dev *dev) +int fch_probe_spi(struct pci_dev *dev) { struct pci_dev *smbus_dev; uint32_t tmp; diff --git a/meson.build b/meson.build index d1f663f..d4bbff7 100644 --- a/meson.build +++ b/meson.build @@ -182,7 +182,7 @@ srcs += 'it85spi.c' srcs += 'it87spi.c' srcs += 'mcp6x_spi.c' - srcs += 'sb600spi.c' + srcs += 'fchspi.c' srcs += 'wbsio_spi.c' endif config_bitbang_spi = true diff --git a/programmer.h b/programmer.h index e42c6bb..e2cadd9 100644 --- a/programmer.h +++ b/programmer.h @@ -657,8 +657,8 @@ /* mcp6x_spi.c */ int mcp6x_spi_init(int want_spi); -/* sb600spi.c */ -int sb600_probe_spi(struct pci_dev *dev); +/* fchspi.c */ +int fch_probe_spi(struct pci_dev *dev); /* wbsio_spi.c */ int wbsio_check_for_spi(void); -- To view, visit https://review.coreboot.org/c/flashrom/+/36434 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iad92fb224954d66968e7aeb75cb515f075a26ac8 Gerrit-Change-Number: 36434 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 2

Change in flashrom[master]: sb600spi.c: Improve IMC runtime detection in handle_imc()
by Edward O'Callaghan (Code Review) 24 Jun '20

24 Jun '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36424 ) Change subject: sb600spi.c: Improve IMC runtime detection in handle_imc() ...................................................................... sb600spi.c: Improve IMC runtime detection in handle_imc() The IMC detection in sb600spi is very rudimentary and incomplete. This expands it so that it can actually detect not only whether an IMC is in the system, but whether it's active or not. This is based off ChromiumOS's fork of flashrom where the solution looks to be originally authored by Martin Roth in b/62141938. Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Signed-off-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M sb600spi.c 1 file changed, 34 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/36424/1 diff --git a/sb600spi.c b/sb600spi.c index c89d5a9..3c83b7b 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -55,6 +55,15 @@ #define FIFO_SIZE_OLD 8 #define FIFO_SIZE_YANGTZE 71 +#define PM_INDEX 0xCD6 +#define PM_DATA 0xCD7 +#define ISA_CONTROL 0x04 +#define MMIO_EN 0x02 +#define ACPI_MMIO_BASE 0xFED80000 +#define MISC_BASE 0xE00 +#define STRAP_STATUS 0x80 +#define EC_ENABLE_STRAP 0x04 + static int sb600_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); static int spi100_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, @@ -80,6 +89,12 @@ .write_aai = default_spi_write_aai, }; +static uint8_t pm_ioread(uint8_t addr) +{ + OUTB(addr, PM_INDEX); + return INB(PM_DATA); +} + static int find_smbus_dev(uint16_t vendor, uint16_t device) { struct pci_dev *smbus_dev = pci_dev_find(vendor, device); @@ -522,6 +537,25 @@ return 0; } + /* On Yangtze and newer chips, check the EcEnableStrap bit */ + if ((amd_gen >= CHIPSET_YANGTZE) && (pm_ioread(ISA_CONTROL) & MMIO_EN )) { + uint8_t *AcpiMMIO = rphysmap("AMD ACPI registers", ACPI_MMIO_BASE & 0xfffff000, 0x1000); + if (AcpiMMIO != ERROR_PTR) { + AcpiMMIO += MISC_BASE + STRAP_STATUS; + uint32_t straps = mmio_readl(AcpiMMIO); + if ((straps & EC_ENABLE_STRAP) == 0){ + msg_pinfo("IMC is not active.\n"); + return 0; + } else { + msg_pinfo("IMC IS active! Straps: %08x (%p)\n", straps, (void *)AcpiMMIO); + msg_pinfo("StrapOverride: %08x\n", mmio_readl(AcpiMMIO + 4)); + } + + } else { + msg_pinfo("Error reading AMD ACPI registers.\n"); + } + } + if (!amd_imc_force) programmer_may_write = 0; msg_pinfo("Writes have been disabled for safety reasons because the presence of the IMC\n" -- To view, visit https://review.coreboot.org/c/flashrom/+/36424 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13fb2300475c06aad09a4f0e8cd032e212556b14 Gerrit-Change-Number: 36424 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 4

Change in flashrom[master]: [WIP] Add .clang-format to help with patch formatting
by David Hendricks (Code Review) 23 Jun '20

23 Jun '20

Hello Vadim Bendebury, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/38673 to review the following change. Change subject: [WIP] Add .clang-format to help with patch formatting ...................................................................... [WIP] Add .clang-format to help with patch formatting This is based on CB:38208 by Vadim, but with a few added changes. Here is the original commit message. This is a copy from the coreboot tree, could be tweaked to better match the flashrom tree if there are any differences. Once this file is in place a script could be deployed which would format only new/changed lines in git patches. Change-Id: I1fe22809c4a2d73fa02f4de55290d990a9750b86 Signed-off-by: Vadim Bendebury <vbendeb(a)chromium.org> Signed-off-by: David Hendricks <david.hendricks(a)gmail.com> --- A .clang-format 1 file changed, 21 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/38673/1 diff --git a/.clang-format b/.clang-format new file mode 100644 index 0000000..45c116b --- /dev/null +++ b/.clang-format @@ -0,0 +1,21 @@ +BasedOnStyle: LLVM +Language: Cpp +IndentWidth: 8 +UseTab: Always +BreakBeforeBraces: Linux +AllowShortIfStatementsOnASingleLine: false +IndentCaseLabels: false +SortIncludes: false +ContinuationIndentWidth: 8 +ColumnLimit: 112 +AlwaysBreakBeforeMultilineStrings: true +AllowShortLoopsOnASingleLine: false +AllowShortFunctionsOnASingleLine: false +AlignEscapedNewlinesLeft: false +AlignTrailingComments: true +AllowAllParametersOfDeclarationOnNextLine: false +AlignAfterOpenBracket: true +SpaceAfterCStyleCast: false +MaxEmptyLinesToKeep: 2 +BreakBeforeBinaryOperators: NonAssignment +BreakStringLiterals: false -- To view, visit https://review.coreboot.org/c/flashrom/+/38673 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1fe22809c4a2d73fa02f4de55290d990a9750b86 Gerrit-Change-Number: 38673 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Vadim Bendebury <vbendeb(a)chromium.org> Gerrit-MessageType: newchange

2 7

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2020