flashrom-gerrit March 2020

flashrom-gerrit@flashrom.org

1 participants
77 discussions

Change in flashrom[master]: raiden_debug_spi.c: Implement retry mechanism
by Edward O'Callaghan (Code Review) 11 Mar '20

11 Mar '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39310 ) Change subject: raiden_debug_spi.c: Implement retry mechanism ...................................................................... raiden_debug_spi.c: Implement retry mechanism This overcomes a problem with the ServoMicro where USB packets can be ack'd by the device without triggering interrupts or loading data into the USB endpoints. The retry mechanism attempts the USB read 3 times before reattempting the write call to avoid performing multiple SPI transfers due to a USB problem. This process repeats 3 times before we return the last error code. Intermediary problems are reported in the status code. Based off the downstream commit: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/… Change-Id: I76cde68852fa4963582d57c7dcb9f24de32c6da8 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M raiden_debug_spi.c 1 file changed, 42 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/39310/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 08f8928..1f30b6a 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -133,6 +133,16 @@ #define PAYLOAD_SIZE (MAX_PACKET_SIZE - PACKET_HEADER_SIZE) /* + * Servo Micro has an error where it is capable of acknowledging USB packets + * without loading it into the USB endpoint buffers or triggering interrupts. + * See crbug.com/952494. Retry mechanisms have been implemented to recover + * from these rare failures allowing the process to continue. + */ +#define WRITE_RETY_ATTEMPTS (3) +#define READ_RETY_ATTEMPTS (3) +#define RETY_INTERVAL_US (100 * 1000) + +/* * This timeout is so large because the Raiden SPI timeout is 800ms. */ #define TRANSFER_TIMEOUT_MS (200 + 800) @@ -245,16 +255,40 @@ { int status = -1; - status = write_command(flash, write_count, read_count, - write_buffer, read_buffer); + for (int write_attempt = 0; write_attempt < WRITE_RETY_ATTEMPTS; + write_attempt++) { - if (status) { - return status; + status = write_command(flash, write_count, read_count, + write_buffer, read_buffer); + + if (status) { + /* Write operation failed. */ + msg_perr("Raiden: Write command failed\n" + "Write attempt = %d\n" + "status = %d\n", + write_attempt + 1, status); + programmer_delay(RETY_INTERVAL_US); + continue; + } + for (int read_attempt = 0; read_attempt < READ_RETY_ATTEMPTS; read_attempt++) { + + status = read_response(flash, write_count, read_count, + write_buffer, read_buffer); + + if (status != 0) { + /* Read operation failed. */ + msg_perr("Raiden: Read response failed\n" + "Write attempt = %d\n" + "Read attempt = %d\n" + "status = %d\n", + write_attempt + 1, read_attempt + 1, status); + programmer_delay(RETY_INTERVAL_US); + } else { + /* We were successful at performing the SPI transfer. */ + return status; + } + } } - - status = read_response(flash, write_count, read_count, - write_buffer, read_buffer); - return status; } -- To view, visit https://review.coreboot.org/c/flashrom/+/39310 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I76cde68852fa4963582d57c7dcb9f24de32c6da8 Gerrit-Change-Number: 39310 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 3

Change in flashrom[master]: raiden_debug_spi.c: Clean up RW ops into sep paths
by Edward O'Callaghan (Code Review) 11 Mar '20

11 Mar '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39309 ) Change subject: raiden_debug_spi.c: Clean up RW ops into sep paths ...................................................................... raiden_debug_spi.c: Clean up RW ops into sep paths - The USB SPI interface has been split up into write and read stages. - The packet packing has been transitioned from array based to a struct. This was based off the downstream commit: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/… Change-Id: Id3a2a544c1c7e1d969a5157977b8a1c7af18371b Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M raiden_debug_spi.c 1 file changed, 73 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/39309/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 08db583..08f8928 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -128,8 +128,9 @@ RAIDEN_DEBUG_SPI_REQ_ENABLE_EC = 0x0003, }; -#define PACKET_HEADER_SIZE 2 -#define MAX_PACKET_SIZE 64 +#define PACKET_HEADER_SIZE (2) +#define MAX_PACKET_SIZE (64) +#define PAYLOAD_SIZE (MAX_PACKET_SIZE - PACKET_HEADER_SIZE) /* * This timeout is so large because the Raiden SPI timeout is 800ms. @@ -140,37 +141,50 @@ uint8_t in_endpoint = 0; uint8_t out_endpoint = 0; -static int send_command(struct flashctx *flash, - unsigned int write_count, - unsigned int read_count, - const unsigned char *write_buffer, - unsigned char *read_buffer) -{ - uint8_t buffer[MAX_PACKET_SIZE]; - int transferred; - int ret; +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; - if (write_count > MAX_PACKET_SIZE - PACKET_HEADER_SIZE) { +typedef struct { + uint16_t status_code; + uint8_t data[PAYLOAD_SIZE]; +} __attribute__((packed)) usb_spi_response_t; + +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) +{ + + int transferred; + int ret; + usb_spi_command_t command_packet; + + if (write_count > PAYLOAD_SIZE) { msg_perr("Raiden: Invalid write_count of %d\n", write_count); return SPI_INVALID_LENGTH; } - if (read_count > MAX_PACKET_SIZE - PACKET_HEADER_SIZE) { + if (read_count > PAYLOAD_SIZE) { msg_perr("Raiden: Invalid read_count of %d\n", read_count); return SPI_INVALID_LENGTH; } - buffer[0] = write_count; - buffer[1] = read_count; + command_packet.write_count = write_count; + command_packet.read_count = read_count; - memcpy(buffer + PACKET_HEADER_SIZE, write_buffer, write_count); + memcpy(command_packet.data, write_buffer, write_count); ret = LIBUSB(libusb_bulk_transfer(device->handle, - out_endpoint, - buffer, - write_count + PACKET_HEADER_SIZE, - &transferred, - TRANSFER_TIMEOUT_MS)); + out_endpoint, + (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" @@ -185,9 +199,22 @@ return 0x10001; } + 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) +{ + int transferred; + int ret; + usb_spi_response_t response_packet; + ret = LIBUSB(libusb_bulk_transfer(device->handle, in_endpoint, - buffer, + (void*)&response_packet, read_count + PACKET_HEADER_SIZE, &transferred, TRANSFER_TIMEOUT_MS)); @@ -205,9 +232,30 @@ return 0x10002; } - memcpy(read_buffer, buffer + PACKET_HEADER_SIZE, read_count); + memcpy(read_buffer, response_packet.data, read_count); - return buffer[0] | (buffer[1] << 8); + return response_packet.status_code; +} + +static int send_command(struct flashctx *flash, + unsigned int write_count, + unsigned int read_count, + const unsigned char *write_buffer, + unsigned char *read_buffer) +{ + int status = -1; + + status = write_command(flash, write_count, read_count, + write_buffer, read_buffer); + + if (status) { + return status; + } + + status = read_response(flash, write_count, read_count, + write_buffer, read_buffer); + + return status; } /* @@ -221,9 +269,7 @@ * The largest command that flashrom generates is the byte program command, so * we use that command header maximum size here. */ -#define MAX_DATA_SIZE (MAX_PACKET_SIZE - \ - PACKET_HEADER_SIZE - \ - JEDEC_BYTE_PROGRAM_OUTSIZE) +#define MAX_DATA_SIZE (PAYLOAD_SIZE - JEDEC_BYTE_PROGRAM_OUTSIZE) static const struct spi_master spi_master_raiden_debug = { .features = SPI_MASTER_4BA, -- To view, visit https://review.coreboot.org/c/flashrom/+/39309 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3a2a544c1c7e1d969a5157977b8a1c7af18371b Gerrit-Change-Number: 39309 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 2

Change in flashrom[master]: raiden_debug_spi.c: Fix indents to be consistent
by Edward O'Callaghan (Code Review) 11 Mar '20

11 Mar '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39308 ) Change subject: raiden_debug_spi.c: Fix indents to be consistent ...................................................................... raiden_debug_spi.c: Fix indents to be consistent Change-Id: I414d6e5fcb590a006dd53fa93df80ec2a765c5d1 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M raiden_debug_spi.c 1 file changed, 40 insertions(+), 40 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/39308/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 84b99ab..08db583 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -151,12 +151,12 @@ int ret; if (write_count > MAX_PACKET_SIZE - PACKET_HEADER_SIZE) { - msg_perr("Raiden: invalid write_count of %d\n", write_count); + msg_perr("Raiden: Invalid write_count of %d\n", write_count); return SPI_INVALID_LENGTH; } if (read_count > MAX_PACKET_SIZE - PACKET_HEADER_SIZE) { - msg_perr("Raiden: invalid read_count of %d\n", read_count); + msg_perr("Raiden: Invalid read_count of %d\n", read_count); return SPI_INVALID_LENGTH; } @@ -166,11 +166,11 @@ memcpy(buffer + PACKET_HEADER_SIZE, write_buffer, write_count); ret = LIBUSB(libusb_bulk_transfer(device->handle, - out_endpoint, + out_endpoint, buffer, - write_count + PACKET_HEADER_SIZE, - &transferred, - TRANSFER_TIMEOUT_MS)); + write_count + PACKET_HEADER_SIZE, + &transferred, + TRANSFER_TIMEOUT_MS)); if (ret != 0) { msg_perr("Raiden: OUT transfer failed\n" " write_count = %d\n" @@ -186,11 +186,11 @@ } ret = LIBUSB(libusb_bulk_transfer(device->handle, - in_endpoint, + in_endpoint, buffer, - read_count + PACKET_HEADER_SIZE, - &transferred, - TRANSFER_TIMEOUT_MS)); + read_count + PACKET_HEADER_SIZE, + &transferred, + TRANSFER_TIMEOUT_MS)); if (ret != 0) { msg_perr("Raiden: IN transfer failed\n" " write_count = %d\n" @@ -201,7 +201,7 @@ if ((unsigned) transferred != read_count + PACKET_HEADER_SIZE) { msg_perr("Raiden: Read failure (read %d, expected %d)\n", - transferred, read_count + PACKET_HEADER_SIZE); + transferred, read_count + PACKET_HEADER_SIZE); return 0x10002; } @@ -281,16 +281,16 @@ static int shutdown(void * data) { int ret = LIBUSB(libusb_control_transfer( - device->handle, - LIBUSB_ENDPOINT_OUT | - LIBUSB_REQUEST_TYPE_VENDOR | - LIBUSB_RECIPIENT_INTERFACE, - RAIDEN_DEBUG_SPI_REQ_DISABLE, - 0, - device->interface_descriptor->bInterfaceNumber, - NULL, - 0, - TRANSFER_TIMEOUT_MS)); + device->handle, + LIBUSB_ENDPOINT_OUT | + LIBUSB_REQUEST_TYPE_VENDOR | + LIBUSB_RECIPIENT_INTERFACE, + RAIDEN_DEBUG_SPI_REQ_DISABLE, + 0, + device->interface_descriptor->bInterfaceNumber, + NULL, + 0, + TRANSFER_TIMEOUT_MS)); if (ret != 0) { msg_perr("Raiden: Failed to disable SPI bridge\n"); return ret; @@ -368,15 +368,15 @@ device = current; if (find_endpoints(device, &in_endpoint, &out_endpoint)) { - msg_pdbg("Raiden: Failed to find valid endpoints on device"); - usb_device_show(" ", current); - goto loop_end; + msg_pdbg("Raiden: Failed to find valid endpoints on device"); + usb_device_show(" ", current); + goto loop_end; } if (usb_device_claim(device)) { - msg_pdbg("Raiden: Failed to claim USB device"); - usb_device_show(" ", current); - goto loop_end; + msg_pdbg("Raiden: Failed to claim USB device"); + usb_device_show(" ", current); + goto loop_end; } if (!serial) { @@ -395,9 +395,9 @@ } rc = libusb_get_string_descriptor_ascii(device->handle, - descriptor.iSerialNumber, - dev_serial, - sizeof(dev_serial)); + descriptor.iSerialNumber, + dev_serial, + sizeof(dev_serial)); if (rc < 0) { LIBUSB(rc); } else { @@ -427,16 +427,16 @@ free_dev_list(&current); ret = LIBUSB(libusb_control_transfer( - device->handle, - LIBUSB_ENDPOINT_OUT | - LIBUSB_REQUEST_TYPE_VENDOR | - LIBUSB_RECIPIENT_INTERFACE, - request_enable, - 0, - device->interface_descriptor->bInterfaceNumber, - NULL, - 0, - TRANSFER_TIMEOUT_MS)); + device->handle, + LIBUSB_ENDPOINT_OUT | + LIBUSB_REQUEST_TYPE_VENDOR | + LIBUSB_RECIPIENT_INTERFACE, + request_enable, + 0, + device->interface_descriptor->bInterfaceNumber, + NULL, + 0, + TRANSFER_TIMEOUT_MS)); if (ret != 0) { msg_perr("Raiden: Failed to enable SPI bridge\n"); return ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/39308 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I414d6e5fcb590a006dd53fa93df80ec2a765c5d1 Gerrit-Change-Number: 39308 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 2

Change in flashrom[master]: ft2232_spi: Fix broken GPIOL cs_bits state (#126)
by Samir Ibradžić (Code Review) 09 Mar '20

09 Mar '20

Samir Ibradžić has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38898 ) Change subject: ft2232_spi: Fix broken GPIOL cs_bits state (#126) ...................................................................... ft2232_spi: Fix broken GPIOL cs_bits state (#126) This only sets 3rd CS# bit be asserted during read/write operations. Tested and confirmed working on 4232H & PicoTap ft2232 programmers against MX25R6435F & S25FL128S chips. Signed-off-by: Samir Ibradzic <sibradzic(a)gmail.com> Change-Id: Ia0ac14b9a52f251306887500dae3e57d73322157 --- M ft2232_spi.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/38898/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index 3e4dc9e..1a5b2fe 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -495,7 +495,7 @@ */ msg_pspew("Assert CS#\n"); buf[i++] = SET_BITS_LOW; - buf[i++] = 0 & ~cs_bits; /* assertive */ + buf[i++] = ~ 0x08 & cs_bits; /* assert CS (3rd) bit only */ buf[i++] = pindir; if (writecnt) { -- To view, visit https://review.coreboot.org/c/flashrom/+/38898 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia0ac14b9a52f251306887500dae3e57d73322157 Gerrit-Change-Number: 38898 Gerrit-PatchSet: 1 Gerrit-Owner: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-MessageType: newchange

2 4

Change in flashrom[master]: ft2232_spi: Enhance csgpiol parameter for FT2232
by Samir Ibradžić (Code Review) 09 Mar '20

09 Mar '20

Samir Ibradžić has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38705 ) Change subject: ft2232_spi: Enhance csgpiol parameter for FT2232 ...................................................................... ft2232_spi: Enhance csgpiol parameter for FT2232 This allows multiple 'csgpiol' bits to be set to active state at the same time. Previously, only one GPIOL could be activated. I have an use-case such that FT4232H is wired to two different SPI chips, and in order to select one of them two GPIOLs have to be set. Now, one can enable any particular GPIOL, for example: csgpiol=01 would activate GPIOL0 and GPIOL1 at the same time. The change is backward-compatible with previous csgpiol formatting. Signed-off-by: Samir Ibradzic <sibradzic(a)gmail.com> Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b --- M ft2232_spi.c 1 file changed, 26 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/38705/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index 5bdc2a7..947050f 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -86,10 +86,17 @@ /* The variables cs_bits and pindir store the values for the "set data bits low byte" MPSSE command that * sets the initial state and the direction of the I/O pins. The pin offsets are as follows: - * SCK is bit 0. - * DO is bit 1. - * DI is bit 2. - * CS is bit 3. + * TCK/SK is bit 0. + * TDI/DO is bit 1. + * TDO/DI is bit 2. + * TMS/CS is bit 3. + * GPIOL0 is bit 4. + * GPIOL1 is bit 5. + * GPIOL2 is bit 6. + * GPIOL3 is bit 7. + * + * The pin signal direction bit offsets follow the same order; 0 means that + * pin at the matching bit index is an input, 1 means pin is an output. * * The default values (set below) are used for most devices: * value: 0x08 CS=high, DI=low, DO=low, SK=low @@ -331,19 +338,24 @@ } free(arg); + /* Allows setting multiple GPIOL states, for example: csgpiol=012 */ arg = extract_programmer_param("csgpiol"); if (arg) { - char *endptr; - unsigned int temp = strtoul(arg, &endptr, 10); - if (*endptr || endptr == arg || temp > 3) { - msg_perr("Error: Invalid GPIOL specified: \"%s\".\n" - "Valid values are between 0 and 3.\n", arg); - free(arg); - return -2; + unsigned int ngpios = strlen(arg); + for (unsigned int i=0; i<=ngpios; i++) { + int temp = arg[i] - '0'; + if (ngpios == 0 || (ngpios != i && (temp < 0 || temp > 3))) { + msg_perr("Error: Invalid GPIOLs specified: \"%s\".\n" + "Valid values are numbers between 0 and 3. " + "Multiple GPIOLs can be specified.\n", arg); + free(arg); + return -2; + } else { + unsigned int pin = temp + 4; + cs_bits |= 1 << pin; + pindir |= 1 << pin; + } } - unsigned int pin = temp + 4; - cs_bits |= 1 << pin; - pindir |= 1 << pin; } free(arg); -- To view, visit https://review.coreboot.org/c/flashrom/+/38705 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I645ddaa9852e9995bd2a6764862fda2b2ef0c26b Gerrit-Change-Number: 38705 Gerrit-PatchSet: 1 Gerrit-Owner: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-MessageType: newchange

3 9

Change in flashrom[master]: sb600spi: Add spireadmode
by Rob Barnes (Code Review) 03 Mar '20

03 Mar '20

Rob Barnes has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38833 ) Change subject: sb600spi: Add spireadmode ...................................................................... sb600spi: Add spireadmode Added spireadmode for >= Bolton. Do not override speed or read mode for >= Bolton if parameter not specified. Minor cleanup of sb600spi.c code. TEST=Manual: deploy on tremblye read flash using various parameters BUG=b:147665085,b:147666328 BRANCH=master Change-Id: Id7fec7eb87ff811148217dc56a86dca3fef122ff Signed-off-by: Rob Barnes <robbarnes(a)google.com> --- M flashrom.8.tmpl M sb600spi.c 2 files changed, 90 insertions(+), 44 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/33/38833/1 diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index aa5bcd4..fde98c0 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -497,11 +497,29 @@ .sp * SB6xx, SB7xx, SP5xxx: from 16.5 MHz up to and including 33 MHz .sp +-The default is to use 16.5 MHz and disable Fast Reads. +.sp * SB8xx, SB9xx, Hudson: from 16.5 MHz up to and including 66 MHz .sp +-The default is to use 16.5 MHz and disable Fast Reads. +.sp * Yangtze (with SPI 100 engine as found in Kabini and Tamesh): all of them .sp -The default is to use 16.5 MHz and disable Fast Reads. +-The default is to use the frequency that is currently configured. +.sp +An optional +.B spireadmode +parameter specifies the read mode of the SPI bus where applicable (Bolton or later). +Syntax is +.sp +.B " flashrom \-p internal:spireadmode=mode" +.sp +where +.B mode +can be +.BR "'Normal\ (up\ to\ 33 MHz)'" ", " "'Normal\ (up\ to\ 66 MHz)'" ", " "'Dual\ IO\ (1-1-2)'" ", " "'Quad\ IO\ (1-1-4)'" ", " "'Dual\ IO\ (1-2-2)'" ", " "'Quad\ IO\ (1-4-4)'" ", or " "'Fast\ Read'" "." +.sp +The default is to use the read mode that is currently configured. .TP .B Intel chipsets .sp diff --git a/sb600spi.c b/sb600spi.c index cb5c4a4..49fe37f 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -347,23 +347,33 @@ const uint8_t speed; }; -static const struct spispeed spispeeds[] = { - { "66 MHz", 0x00 }, - { "33 MHz", 0x01 }, - { "22 MHz", 0x02 }, - { "16.5 MHz", 0x03 }, - { "100 MHz", 0x04 }, - { "Reserved", 0x05 }, - { "Reserved", 0x06 }, - { "800 kHz", 0x07 }, +static const char* spispeeds[] = { + "66 MHz", + "33 MHz", + "22 MHz", + "16.5 MHz", + "100 MHz", + "Reserved", + "Reserved", + "800 kHz", }; -static int set_speed(struct pci_dev *dev, const struct spispeed *spispeed) +static const char* spireadmodes[] = { + "Normal (up to 33 MHz)", + "Reserved", + "Dual IO (1-1-2)", + "Quad IO (1-1-4)", + "Dual IO (1-2-2)", + "Quad IO (1-4-4)", + "Normal (up to 66 MHz)", + "Fast Read", +}; + +static int set_speed(struct pci_dev *dev, uint8_t speed) { bool success = false; - uint8_t speed = spispeed->speed; - msg_pdbg("Setting SPI clock to %s (0x%x).\n", spispeed->name, speed); + msg_pdbg("Setting SPI clock to %s (%i).\n", spispeeds[speed], speed); if (amd_gen >= CHIPSET_YANGTZE) { rmmio_writew((speed << 12) | (speed << 8) | (speed << 4) | speed, sb600_spibar + 0x22); uint16_t tmp = mmio_readw(sb600_spibar + 0x22); @@ -375,33 +385,40 @@ } if (!success) { - msg_perr("Setting SPI clock failed.\n"); + msg_perr("Setting SPI clock to %s failed.\n", spispeeds[speed]); return 1; } + msg_pdbg("Setting SPI clock to %s succeeded.\n", spispeeds[speed]); return 0; } -static int set_mode(struct pci_dev *dev, uint8_t read_mode) +static int set_mode(struct pci_dev *dev, uint8_t mode) { + msg_pdbg("Setting SPI read mode to %s (%i).\n", spireadmodes[mode], mode); uint32_t tmp = mmio_readl(sb600_spibar + 0x00); tmp &= ~(0x6 << 28 | 0x1 << 18); /* Clear mode bits */ - tmp |= ((read_mode & 0x6) << 28) | ((read_mode & 0x1) << 18); + tmp |= ((mode & 0x6) << 28) | ((mode & 0x1) << 18); rmmio_writel(tmp, sb600_spibar + 0x00); - if (tmp != mmio_readl(sb600_spibar + 0x00)) + if (tmp != mmio_readl(sb600_spibar + 0x00)) { + msg_perr("Setting SPI read mode to %s failed.\n", spireadmodes[mode]); return 1; + } + msg_pdbg("Setting SPI read mode to %s succeeded.\n", spireadmodes[mode]); return 0; } static int handle_speed(struct pci_dev *dev) { uint32_t tmp; - uint8_t spispeed_idx = 3; /* Default to 16.5 MHz */ - + int16_t spispeed_idx = -1; + int16_t spireadmode_idx = -1; char *spispeed = extract_programmer_param("spispeed"); + char *spireadmode = extract_programmer_param("spireadmode"); + if (spispeed != NULL) { unsigned int i; for (i = 0; i < ARRAY_SIZE(spispeeds); i++) { - if (strcasecmp(spispeeds[i].name, spispeed) == 0) { + if (strcasecmp(spispeeds[i], spispeed) == 0) { spispeed_idx = i; break; } @@ -421,32 +438,37 @@ free(spispeed); } + if (spireadmode != NULL) { + unsigned int i; + for (i = 0; i < ARRAY_SIZE(spireadmodes); i++) { + if (strcasecmp(spireadmodes[i], spireadmode) == 0) { + spireadmode_idx = i; + break; + } + } + if ((strcasecmp(spireadmode, "reserved") == 0) || + (i == ARRAY_SIZE(spireadmodes)) || + (amd_gen < CHIPSET_BOLTON)) { + msg_perr("Error: Invalid spireadmode value: '%s'.\n", spireadmode); + free(spireadmode); + return 1; + } + free(spireadmode); + } + /* See the chipset support matrix for SPI Base_Addr below for an explanation of the symbols used. * bit 6xx 7xx/SP5100 8xx 9xx hudson1 hudson234 bolton/yangtze * 18 rsvd <- fastReadEnable ? <- ? SpiReadMode[0] * 29:30 rsvd <- <- ? <- ? SpiReadMode[2:1] */ if (amd_gen >= CHIPSET_BOLTON) { - static const char *spireadmodes[] = { - "Normal (up to 33 MHz)", /* 0 */ - "Reserved", /* 1 */ - "Dual IO (1-1-2)", /* 2 */ - "Quad IO (1-1-4)", /* 3 */ - "Dual IO (1-2-2)", /* 4 */ - "Quad IO (1-4-4)", /* 5 */ - "Normal (up to 66 MHz)", /* 6 */ - "Fast Read", /* 7 (Not defined in the Bolton datasheet.) */ - }; + tmp = mmio_readl(sb600_spibar + 0x00); uint8_t read_mode = ((tmp >> 28) & 0x6) | ((tmp >> 18) & 0x1); - msg_pdbg("SpiReadMode=%s (%i)\n", spireadmodes[read_mode], read_mode); - if (read_mode != 6) { - read_mode = 6; /* Default to "Normal (up to 66 MHz)" */ - if (set_mode(dev, read_mode) != 0) { - msg_perr("Setting read mode to \"%s\" failed.\n", spireadmodes[read_mode]); - return 1; - } - msg_pdbg("Setting read mode to \"%s\" succeeded.\n", spireadmodes[read_mode]); + msg_pdbg("SPI read mode is %s (%i)\n", + spireadmodes[read_mode], read_mode); + if (spireadmode_idx >= 0 && set_mode(dev, spireadmode_idx) != 0) { + return 1; } if (amd_gen >= CHIPSET_YANGTZE) { @@ -463,10 +485,10 @@ } tmp = mmio_readw(sb600_spibar + 0x22); /* SPI 100 Speed Config */ - msg_pdbg("NormSpeedNew is %s\n", spispeeds[(tmp >> 12) & 0xf].name); - msg_pdbg("FastSpeedNew is %s\n", spispeeds[(tmp >> 8) & 0xf].name); - msg_pdbg("AltSpeedNew is %s\n", spispeeds[(tmp >> 4) & 0xf].name); - msg_pdbg("TpmSpeedNew is %s\n", spispeeds[(tmp >> 0) & 0xf].name); + msg_pdbg("NormSpeedNew is %s\n", spispeeds[(tmp >> 12) & 0xf]); + msg_pdbg("FastSpeedNew is %s\n", spispeeds[(tmp >> 8) & 0xf]); + msg_pdbg("AltSpeedNew is %s\n", spispeeds[(tmp >> 4) & 0xf]); + msg_pdbg("TpmSpeedNew is %s\n", spispeeds[(tmp >> 0) & 0xf]); } } else { if (amd_gen >= CHIPSET_SB89XX && amd_gen <= CHIPSET_HUDSON234) { @@ -479,9 +501,15 @@ } } tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3; - msg_pdbg("NormSpeed is %s\n", spispeeds[tmp].name); + msg_pdbg("NormSpeed is %s\n", spispeeds[tmp]); + if (spispeed_idx < 0) { + spispeed_idx = 3; /* Default to 16.5 MHz */ + } } - return set_speed(dev, &spispeeds[spispeed_idx]); + if (spispeed_idx >= 0 && set_speed(dev, spispeed_idx) != 0) { + return 1; + } + return 0; } static int handle_imc(struct pci_dev *dev) -- To view, visit https://review.coreboot.org/c/flashrom/+/38833 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id7fec7eb87ff811148217dc56a86dca3fef122ff Gerrit-Change-Number: 38833 Gerrit-PatchSet: 1 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-MessageType: newchange

3 17

Change in flashrom[master]: raiden_debug: Upstream ChromiumOS usb_device helpers
by Edward O'Callaghan (Code Review) 02 Mar '20

02 Mar '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38936 ) Change subject: raiden_debug: Upstream ChromiumOS usb_device helpers ...................................................................... raiden_debug: Upstream ChromiumOS usb_device helpers These are helpful usb device accessors and helpers that are later used for the so-called Raiden debugger programmer. BUG=b:143389556 BRANCH=none TEST=builds Change-Id: Ic928220fc919fe4958c8150e61e11470dac88f13 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M Makefile M meson.build A usb_device.c A usb_device.h 4 files changed, 559 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/38936/1 diff --git a/Makefile b/Makefile index 7242b09..bfd4d1e 100644 --- a/Makefile +++ b/Makefile @@ -1088,7 +1088,7 @@ ifneq ($(NEED_LIBUSB1), ) CHECK_LIBUSB1 = yes FEATURE_CFLAGS += -D'NEED_LIBUSB1=1' -PROGRAMMER_OBJS += usbdev.o +PROGRAMMER_OBJS += usbdev.o usb_device.o # FreeBSD and DragonflyBSD use a reimplementation of libusb-1.0 that is simply called libusb ifeq ($(TARGET_OS),$(filter $(TARGET_OS),FreeBSD DragonFlyBSD)) USB1LIBS += -lusb diff --git a/meson.build b/meson.build index 375089c..45a3681 100644 --- a/meson.build +++ b/meson.build @@ -84,6 +84,7 @@ # some programmers require libusb if get_option('usb') srcs += 'usbdev.c' + srcs += 'usb_device.c' deps += dependency('libusb-1.0') else config_ch341a_spi = false diff --git a/usb_device.c b/usb_device.c new file mode 100644 index 0000000..48833fb --- /dev/null +++ b/usb_device.c @@ -0,0 +1,393 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2020, Google Inc. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "programmer.h" +#include "spi.h" +#include "usb_device.h" + +#include <assert.h> +#include <libusb.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +/* + * Possibly extract a programmer parameter and use it to initialize the given + * match value structure. + */ +static void usb_match_value_init(struct usb_match_value *match, + char const *parameter) +{ + char *string = extract_programmer_param(parameter); + + match->name = parameter; + + if (string) { + match->set = 1; + match->value = strtol(string, NULL, 0); + } else { + match->set = 0; + } + + free(string); +} + +#define USB_MATCH_VALUE_INIT(NAME) \ + usb_match_value_init(&match->NAME, #NAME) + +void usb_match_init(struct usb_match *match) +{ + USB_MATCH_VALUE_INIT(vid); + USB_MATCH_VALUE_INIT(pid); + USB_MATCH_VALUE_INIT(bus); + USB_MATCH_VALUE_INIT(address); + USB_MATCH_VALUE_INIT(config); + USB_MATCH_VALUE_INIT(interface); + USB_MATCH_VALUE_INIT(altsetting); + USB_MATCH_VALUE_INIT(class); + USB_MATCH_VALUE_INIT(subclass); + USB_MATCH_VALUE_INIT(protocol); +} + +void usb_match_value_default(struct usb_match_value *value, + long int default_value) +{ + if (value->set) + return; + + value->set = 1; + value->value = default_value; +} + +/* + * Match the value against a possible user supplied parameter. + * + * Return: + * 0: The user supplied the given parameter and it did not match the value. + * 1: Either the user didn't supply the parameter, or they did and it + * matches the given value. + */ +static int check_match(struct usb_match_value const *match_value, int value) +{ + int reject = match_value->set && (match_value->value != value); + + if (reject) + msg_pdbg("USB: Rejecting device because %s = %d != %d\n", + match_value->name, + value, + match_value->value); + + return !reject; +} + +/* + * Allocate a copy of device and add it to the head of the devices list. + */ +static void add_device(struct usb_device *device, + struct usb_device **devices) +{ + struct usb_device *copy = malloc(sizeof(struct usb_device)); + + assert(copy != NULL); + + *copy = *device; + + copy->next = *devices; + *devices = copy; + + libusb_ref_device(copy->device); +} + +/* + * Look through the interfaces of the current device config for a match. Stop + * looking after the first valid match is found. + * + * Return: + * 0: No matching interface was found. + * 1: A complete match was found and added to the devices list. + */ +static int find_interface(struct usb_match const *match, + struct usb_device *current, + struct usb_device **devices) +{ + int i, j; + + for (i = 0; i < current->config_descriptor->bNumInterfaces; ++i) { + struct libusb_interface const *interface; + + interface = &current->config_descriptor->interface[i]; + + for (j = 0; j < interface->num_altsetting; ++j) { + struct libusb_interface_descriptor const *descriptor; + + descriptor = &interface->altsetting[j]; + + if (check_match(&match->interface, + descriptor->bInterfaceNumber) && + check_match(&match->altsetting, + descriptor->bAlternateSetting) && + check_match(&match->class, + descriptor->bInterfaceClass) && + check_match(&match->subclass, + descriptor->bInterfaceSubClass) && + check_match(&match->protocol, + descriptor->bInterfaceProtocol)) { + current->interface_descriptor = descriptor; + add_device(current, devices); + msg_pdbg("USB: Found matching device\n"); + return 1; + } + } + } + + return 0; +} + +/* + * Look through the configs of the current device for a match. Stop looking + * after the first valid match is found. + * + * Return: + * 0: All configurations successfully checked, one may have been added to + * the list. + * non-zero: There was a failure while checking for a match. + */ +static int find_config(struct usb_match const *match, + struct usb_device *current, + struct libusb_device_descriptor const *device_descriptor, + struct usb_device **devices) +{ + int i; + + for (i = 0; i < device_descriptor->bNumConfigurations; ++i) { + int ret = LIBUSB(libusb_get_config_descriptor( + current->device, i, + &current->config_descriptor)); + if (ret != 0) { + msg_perr("USB: Failed to get config descriptor"); + return ret; + } + + if (check_match(&match->config, + current->config_descriptor-> + bConfigurationValue) && + find_interface(match, current, devices)) + break; + + libusb_free_config_descriptor(current->config_descriptor); + } + + return 0; +} + +int usb_device_find(struct usb_match const *match, struct usb_device **devices) +{ + libusb_device **list; + ssize_t count; + ssize_t i; + + *devices = NULL; + + int ret = LIBUSB(count = libusb_get_device_list(NULL, &list)); + if (ret != 0) { + msg_perr("USB: Failed to get device list"); + return ret; + } + + for (i = 0; i < count; ++i) { + struct libusb_device_descriptor descriptor; + struct usb_device current = { + .device = list[i], + .handle = NULL, + .next = NULL, + }; + + uint8_t bus = libusb_get_bus_number(list[i]); + uint8_t address = libusb_get_device_address(list[i]); + + msg_pdbg("USB: Inspecting device (Bus %d, Address %d)\n", + bus, + address); + + ret = LIBUSB(libusb_get_device_descriptor(list[i], + &descriptor)); + if (ret != 0) { + msg_perr("USB: Failed to get device descriptor"); + return ret; + } + + if (check_match(&match->vid, descriptor.idVendor) && + check_match(&match->pid, descriptor.idProduct) && + check_match(&match->bus, bus) && + check_match(&match->address, address)) { + ret = find_config(match, + &current, + &descriptor, + devices); + if (ret != 0) { + msg_perr("USB: Failed to find config"); + return ret; + } + } + } + + libusb_free_device_list(list, 1); + + return (*devices == NULL); +} + +/* + * If the underlying libusb device is not open, open it. + * + * Return: + * 0: The device was already open or was successfully opened. + * non-zero: There was a failure while opening the device. + */ +static int usb_device_open(struct usb_device *device) +{ + if (device->handle == NULL) { + int ret = LIBUSB(libusb_open(device->device, &device->handle)); + if (ret != 0) { + msg_perr("USB: Failed to open device\n"); + return ret; + } + } + + return 0; +} + +int usb_device_show(char const *prefix, struct usb_device *device) +{ + struct libusb_device_descriptor descriptor; + unsigned char product[256]; + int ret; + + ret = usb_device_open(device); + if (ret != 0) { + msg_perr("USB: Failed to open device\n"); + return ret; + } + + ret = LIBUSB(libusb_get_device_descriptor(device->device, &descriptor)); + if (ret != 0) { + msg_perr("USB: Failed to get device descriptor\n"); + return ret; + } + + ret = LIBUSB(libusb_get_string_descriptor_ascii( + device->handle, + descriptor.iProduct, + product, + sizeof(product))); + if (ret != 0) { + msg_perr("USB: Failed to get device product string\n"); + return ret; + } + + product[255] = '\0'; + + msg_perr("%sbus=0x%02x,address=0x%02x | %s\n", + prefix, + libusb_get_bus_number(device->device), + libusb_get_device_address(device->device), + product); + + return 0; +} + +int usb_device_claim(struct usb_device *device) +{ + int current_config; + + int ret = usb_device_open(device); + if (ret != 0) { + msg_perr("USB: Failed to open device\n"); + return ret; + } + + ret = LIBUSB(libusb_get_configuration(device->handle, + &current_config)); + if (ret != 0) { + msg_perr("USB: Failed to get current device configuration\n"); + return ret; + } + + if (current_config != device->config_descriptor->bConfigurationValue) { + ret = LIBUSB(libusb_set_configuration( + device->handle, + device-> + config_descriptor-> + bConfigurationValue)); + if (ret != 0) { + msg_perr("USB: Failed to set new configuration from %d to %d\n", + current_config, + device->config_descriptor->bConfigurationValue); + return ret; + } + } + + ret = LIBUSB(libusb_set_auto_detach_kernel_driver(device->handle, 1)); + if (ret != 0) { + msg_perr("USB: Failed to enable auto kernel driver detach\n"); + return ret; + } + + ret = LIBUSB(libusb_claim_interface(device->handle, + device-> + interface_descriptor-> + bInterfaceNumber)); + if (ret != 0) { + msg_perr("USB: Could not claim device interface %d\n", + device->interface_descriptor->bInterfaceNumber); + return ret; + } + + if (device->interface_descriptor->bAlternateSetting != 0) { + ret = LIBUSB(libusb_set_interface_alt_setting( + device->handle, + device-> + interface_descriptor-> + bInterfaceNumber, + device-> + interface_descriptor-> + bAlternateSetting)); + if (ret != 0) { + msg_perr("USB: Failed to set alternate setting %d\n", + device->interface_descriptor->bAlternateSetting); + return ret; + } + } + + return 0; +} + +struct usb_device *usb_device_free(struct usb_device *device) +{ + struct usb_device *next = device->next; + + if (device->handle != NULL) + libusb_close(device->handle); + + /* + * This unref balances the ref added in the add_device function. + */ + libusb_unref_device(device->device); + libusb_free_config_descriptor(device->config_descriptor); + + free(device); + + return next; +} diff --git a/usb_device.h b/usb_device.h new file mode 100644 index 0000000..d379675 --- /dev/null +++ b/usb_device.h @@ -0,0 +1,164 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2020, Google Inc. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef USB_DEVICE_H +#define USB_DEVICE_H + +/* + * USB device matching framework + * + * This can be used to match a USB device by a number of different parameters. + * The parameters can be passed on the command line and defaults can be set + * by the programmer. + */ + +#include <libusb.h> +#include <stdint.h> + +/* + * The LIBUSB macro converts a libusb failure code into an error code that + * flashrom recognizes. It also displays additional libusb specific + * information about the failure. + */ +#define LIBUSB(expression) \ + ({ \ + int libusb_error__ = (expression); \ + \ + if (libusb_error__ < 0) { \ + msg_perr("libusb error: %s:%d %s\n", \ + __FILE__, \ + __LINE__, \ + libusb_error_name(libusb_error__)); \ + libusb_error__ = 0x20000 | -libusb_error__; \ + } else { \ + libusb_error__ = 0; \ + } \ + \ + libusb_error__; \ + }) + +/* + * A USB match and associated value struct are used to encode the information + * about a device against which we wish to match. If the value of a + * usb_match_value has been set then a device must match that value. The name + * of the usb_match_value is used to fetch the programmer parameter from the + * flashrom command line and is the same as the name of the corresponding + * field in usb_match. + */ +struct usb_match_value { + char const *name; + int value; + int set; +}; + +struct usb_match { + struct usb_match_value bus; + struct usb_match_value address; + struct usb_match_value vid; + struct usb_match_value pid; + struct usb_match_value serial; + struct usb_match_value config; + struct usb_match_value interface; + struct usb_match_value altsetting; + struct usb_match_value class; + struct usb_match_value subclass; + struct usb_match_value protocol; +}; + +/* + * Initialize a usb_match structure so that each value's name matches the + * values name in the usb_match structure (so bus.name == "bus"...), and + * look for each value in the flashrom command line via + * extract_programmer_param. If the value is found convert it to an integer + * using strtol, accepting hex, decimal and octal encoding. + */ +void usb_match_init(struct usb_match *match); + +/* + * Add a default value to a usb_match_value. This must be done after calling + * usb_match_init. If usb_match_init already set the value of a usb_match_value + * we do nothing, otherwise set the value to default_value. This ensures that + * parameters passed on the command line override defaults. + */ +void usb_match_value_default(struct usb_match_value *match, + long int default_value); + +/* + * The usb_device structure is an entry in a linked list of devices that were + * matched by usb_device_find. + */ +struct usb_device { + struct libusb_device *device; + struct libusb_config_descriptor *config_descriptor; + struct libusb_interface_descriptor const *interface_descriptor; + + /* + * Initially NULL, the libusb_device_handle is only valid once the + * usb_device has been successfully passed to usb_device_show or + * usb_device_claim. + */ + struct libusb_device_handle *handle; + + /* + * Link to next device, or NULL + */ + struct usb_device *next; +}; + +/* + * Find and return a list of all compatible devices. Each device is added to + * the list with its first valid configuration and interface. If an alternate + * configuration (config, interface, altsetting...) is desired the specifics + * can be supplied as programmer parameters. + * + * Return: + * 0: At least one matching device was found. + * 1: No matching devices were found. + */ +int usb_device_find(struct usb_match const *match, struct usb_device **devices); + +/* + * Display the devices bus and address as well as its product string. The + * underlying libusb device is opened if it is not already open. + * + * Return: + * 0: The device information was displayed. + * non-zero: There was a failure while displaying the device information. + */ +int usb_device_show(char const *prefix, struct usb_device *device); + +/* + * Open the underlying libusb device, set its config, claim the interface and + * select the correct alternate interface. + * + * Return: + * 0: The device was successfully claimed. + * non-zero: There was a failure while trying to claim the device. + */ +int usb_device_claim(struct usb_device *device); + +/* + * Free a usb_device structure. + * + * This ensures that the libusb device is closed and that all allocated + * handles and descriptors are freed. + * + * Return: + * The next device in the device list. + */ +struct usb_device *usb_device_free(struct usb_device *device); + +#endif /* USB_DEVICE_H */ -- To view, visit https://review.coreboot.org/c/flashrom/+/38936 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic928220fc919fe4958c8150e61e11470dac88f13 Gerrit-Change-Number: 38936 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 4

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2020