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

Change in flashrom[master]: TEST-ONLY: push patch
by Dino Li (Code Review) 25 Mar '20

25 Mar '20

Dino Li has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/39818 ) Change subject: TEST-ONLY: push patch ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/flashrom/+/39818 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic36635f9dbdbe8d2a652ad3516f4153cdc35dfa9 Gerrit-Change-Number: 39818 Gerrit-PatchSet: 2 Gerrit-Owner: Dino Li <Dino.Li(a)ite.com.tw> Gerrit-MessageType: abandon

1 0

Change in flashrom[master]: flashchips: Add Macronix MX25R3235F
by Samir Ibradžić (Code Review) 24 Mar '20

24 Mar '20

Samir Ibradžić has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39532 ) Change subject: flashchips: Add Macronix MX25R3235F ...................................................................... flashchips: Add Macronix MX25R3235F 32Mbit (4MB) [x1/x2/x4] Wide Voltage Range (VCC 1.65V-3.6V), data-sheet: https://www.macronix.com/Lists/Datasheet/Attachments/7534/MX25R3235F,%20Wid… Similar to already supported MX25R6435F, just half the size. Tested on ch341a, 4232H & PicoTAP (2232H) programmers, confirmed working probe, read, erase & write. Change-Id: I6e79115adba17d13d24bc85d78707d53fd4a0be5 Signed-off-by: Samir Ibradzic <sibradzic(a)gmail.com> --- M flashchips.c M flashchips.h 2 files changed, 40 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/39532/1 diff --git a/flashchips.c b/flashchips.c index 58dd4f3..69ea270 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8735,6 +8735,45 @@ { .vendor = "Macronix", + .name = "MX25R3235F", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25R3235F, + .total_size = 4096, + .page_size = 256, + /* OTP: 1024B total; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP, + .tested = TEST_OK_PREW, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 1024} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {64 * 1024, 64} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {32 * 1024, 128} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {4 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { {4 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ + .unlock = spi_disable_blockprotect_bp3_srwd, + .write = spi_chip_write_256, + .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {1650, 3600}, + }, + + { + .vendor = "Macronix", .name = "MX25R6435F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, diff --git a/flashchips.h b/flashchips.h index 14ab6de..36990e2 100644 --- a/flashchips.h +++ b/flashchips.h @@ -503,6 +503,7 @@ #define MACRONIX_MX25L3235D 0x5E16 /* MX25L3225D/MX25L3235D/MX25L3237D */ #define MACRONIX_MX25L6495F 0x9517 +#define MACRONIX_MX25R3235F 0x2816 #define MACRONIX_MX25R6435F 0x2817 #define MACRONIX_MX29F001B 0x19 -- To view, visit https://review.coreboot.org/c/flashrom/+/39532 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e79115adba17d13d24bc85d78707d53fd4a0be5 Gerrit-Change-Number: 39532 Gerrit-PatchSet: 1 Gerrit-Owner: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-MessageType: newchange

4 20

Change in flashrom[master]: dediprog: add serial argument
by David Hendricks (Code Review) 22 Mar '20

22 Mar '20

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37406 ) Change subject: dediprog: add serial argument ...................................................................... dediprog: add serial argument A quick hack to be able to select dediprogs by USB serial argument by just adding a @serial_number parameter to dediprog_open() and using it in preference to @id if available (since it is more specific). Change-Id: I9cdfbce6cf941c16bf7b7364aa4166b91369e661 Signed-off-by: Inaky Perez-Gonzalez <inaky.perez-gonzalez(a)intel.com> --- M dediprog.c M flashrom.8.tmpl 2 files changed, 23 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/37406/1 diff --git a/dediprog.c b/dediprog.c index 175e099..480cf67 100644 --- a/dediprog.c +++ b/dediprog.c @@ -1006,15 +1006,19 @@ /* * Open a dediprog_handle with the USB device at the given index. * @index index of the USB device + * @serial_number serial number of the USB device (id is ignored then) * @return 0 for success, -1 for error, -2 for busy device */ -static int dediprog_open(int index) +static int dediprog_open(int index, char *serial_number) { const uint16_t vid = devs_dediprog[0].vendor_id; const uint16_t pid = devs_dediprog[0].device_id; int ret; - dediprog_handle = usb_dev_get_by_vid_pid_number(usb_ctx, vid, pid, (unsigned int) index); + if (serial_number) + dediprog_handle = usb_dev_get_by_vid_pid_serial(usb_ctx, vid, pid, serial_number); + else + dediprog_handle = usb_dev_get_by_vid_pid_number(usb_ctx, vid, pid, (unsigned int) index); if (!dediprog_handle) { msg_perr("Could not find a Dediprog programmer on USB.\n"); libusb_exit(usb_ctx); @@ -1057,7 +1061,8 @@ int dediprog_init(void) { - char *voltage, *id_str, *device, *spispeed, *target_str; + char *voltage, *id_str, *device, *spispeed, *target_str, + *serial_number; int spispeed_idx = 1; int millivolt = 3500; int id = -1; /* -1 defaults to enumeration order */ @@ -1091,6 +1096,7 @@ msg_pinfo("Setting voltage to %i mV\n", millivolt); } + serial_number = extract_programmer_param("serial"); id_str = extract_programmer_param("id"); if (id_str) { char prefix0, prefix1; @@ -1183,9 +1189,14 @@ return 1; } - if (id != -1) { + if (serial_number) { + if (dediprog_open(0, serial_number)) { + return 1; + } + found_id = dediprog_read_id(); + } else if (id != -1) { for (i = 0; ; i++) { - ret = dediprog_open(i); + ret = dediprog_open(i, NULL); if (ret == -1) { /* no dev */ libusb_exit(usb_ctx); @@ -1218,7 +1229,7 @@ break; } } else { - if (dediprog_open(usedevice)) { + if (dediprog_open(usedevice, NULL)) { return 1; } found_id = dediprog_read_id(); @@ -1276,6 +1287,5 @@ if (register_spi_master(&spi_master_dediprog) || dediprog_set_leds(LED_NONE)) return 1; - return 0; } diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index 27f3846..5af1a38 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -928,6 +928,12 @@ .BR 0V ", " 1.8V ", " 2.5V ", " 3.5V or the equivalent in mV. .sp +You can use the +.B serial +parameter to explicitly specify which dediprog device should be used +based on their USB serial number:: +.sp +.B " flashrom \-p dediprog:serial=1230A12" An optional .B device parameter specifies which of multiple connected Dediprog devices should be used. -- To view, visit https://review.coreboot.org/c/flashrom/+/37406 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9cdfbce6cf941c16bf7b7364aa4166b91369e661 Gerrit-Change-Number: 37406 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-MessageType: newchange

5 10

Change in flashrom[refs/meta/config]: Review access change
by Nico Huber (Code Review) 22 Mar '20

22 Mar '20

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38321 ) Change subject: Review access change ...................................................................... Review access change Change-Id: I91f942850335487436433fc937627271870dcaa5 --- M project.config 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/38321/1 diff --git a/project.config b/project.config index a11f362..e3b7a6d 100644 --- a/project.config +++ b/project.config @@ -48,6 +48,8 @@ delete = group user/Nico Huber (Nico) delete = group user/Stefan T (stefanct) submit = group flashrom developers + label-Code-Review = -2..+2 group flashrom developers + label-Verified = -1..+1 group flashrom developers [access "refs/for/refs/heads/stable"] exclusiveGroupPermissions = push [project] -- To view, visit https://review.coreboot.org/c/flashrom/+/38321 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: refs/meta/config Gerrit-Change-Id: I91f942850335487436433fc937627271870dcaa5 Gerrit-Change-Number: 38321 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newchange

3 6

Change in flashrom[master]: Add a udev rule for ch341a programmer
by Iru Cai (vimacs) (Code Review) 22 Mar '20

22 Mar '20

Hello Iru Cai, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/39749 to review the following change. Change subject: Add a udev rule for ch341a programmer ...................................................................... Add a udev rule for ch341a programmer The udev rule will let users use the programmer without root permission. Change-Id: I5347f2c9f5a7c8613c74ee989d572d3f0b9a7e76 Signed-off-by: Iru Cai <mytbk920423(a)gmail.com> --- A 99-flashrom.rules M Makefile 2 files changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/39749/1 diff --git a/99-flashrom.rules b/99-flashrom.rules new file mode 100644 index 0000000..6e505a7 --- /dev/null +++ b/99-flashrom.rules @@ -0,0 +1,2 @@ +# CH341A +SUBSYSTEM=="usb", ATTRS{idVendor}=="1a86", ATTRS{idProduct}=="5512", MODE="0666" diff --git a/Makefile b/Makefile index 28ceab6..d0900db 100644 --- a/Makefile +++ b/Makefile @@ -1560,8 +1560,10 @@ install: $(PROGRAM)$(EXEC_SUFFIX) $(PROGRAM).8 mkdir -p $(DESTDIR)$(PREFIX)/sbin mkdir -p $(DESTDIR)$(MANDIR)/man8 + mkdir -p $(DESTDIR)/etc/udev/rules.d $(INSTALL) -m 0755 $(PROGRAM)$(EXEC_SUFFIX) $(DESTDIR)$(PREFIX)/sbin $(INSTALL) -m 0644 $(PROGRAM).8 $(DESTDIR)$(MANDIR)/man8 + $(INSTALL) -m 0644 99-flashrom.rules $(DESTDIR)/etc/udev/rules.d libinstall: libflashrom.a libflashrom.h mkdir -p $(DESTDIR)$(PREFIX)/lib -- To view, visit https://review.coreboot.org/c/flashrom/+/39749 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5347f2c9f5a7c8613c74ee989d572d3f0b9a7e76 Gerrit-Change-Number: 39749 Gerrit-PatchSet: 1 Gerrit-Owner: Iru Cai (vimacs) <mytbk920423(a)gmail.com> Gerrit-Reviewer: Iru Cai <mytbk920423(a)gmail.com> Gerrit-MessageType: newchange

1 3

Change in flashrom[master]: chipset_enable: Mark Intel HM75 as DEP
by Evgeny Zinoviev (Code Review) 19 Mar '20

19 Mar '20

Evgeny Zinoviev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39389 ) Change subject: chipset_enable: Mark Intel HM75 as DEP ...................................................................... chipset_enable: Mark Intel HM75 as DEP Tested reading and writing on a Samsung laptop (see CB:39388). Change-Id: Idbb9c719a6f794a35293bb3b167cc1491d24d4fa Signed-off-by: Evgeny Zinoviev <me(a)ch1p.io> --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/89/39389/1 diff --git a/chipset_enable.c b/chipset_enable.c index 84e4b6b..2c39714 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1832,7 +1832,7 @@ {0x8086, 0x1e57, B_FS, DEP, "Intel", "HM77", enable_flash_pch7}, {0x8086, 0x1e58, B_FS, NT, "Intel", "UM77", enable_flash_pch7}, {0x8086, 0x1e59, B_FS, DEP, "Intel", "HM76", enable_flash_pch7}, - {0x8086, 0x1e5d, B_FS, NT, "Intel", "HM75", enable_flash_pch7}, + {0x8086, 0x1e5d, B_FS, DEP, "Intel", "HM75", enable_flash_pch7}, {0x8086, 0x1e5e, B_FS, NT, "Intel", "HM70", enable_flash_pch7}, {0x8086, 0x1e5f, B_FS, DEP, "Intel", "NM70", enable_flash_pch7}, {0x8086, 0x1f38, B_FS, DEP, "Intel", "Avoton/Rangeley", enable_flash_silvermont}, -- To view, visit https://review.coreboot.org/c/flashrom/+/39389 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idbb9c719a6f794a35293bb3b167cc1491d24d4fa Gerrit-Change-Number: 39389 Gerrit-PatchSet: 1 Gerrit-Owner: Evgeny Zinoviev <me(a)ch1p.io> Gerrit-MessageType: newchange

3 2

Change in flashrom[master]: chipset_enable.c: Mark Skylake U Premium as DEP
by Angel Pons (Code Review) 19 Mar '20

19 Mar '20

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/39173 ) Change subject: chipset_enable.c: Mark Skylake U Premium as DEP ...................................................................... chipset_enable.c: Mark Skylake U Premium as DEP Tested reading, writing and erasing the internal flash chip using an Acer Aspire ES1-572 laptop with an Intel i3-6006U. However, since all ME-enabled chipsets are marked as DEP instead of OK, this one shall follow suit as well. Change-Id: Ib8ee9b5e811df74d2f48bd409806c72fe862bc24 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/39173/1 diff --git a/chipset_enable.c b/chipset_enable.c index 84e4b6b..4afb0e4 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1987,7 +1987,7 @@ {0x8086, 0x9d41, B_S, NT, "Intel", "Skylake / Kaby Lake Sample", enable_flash_pch100}, {0x8086, 0x9d43, B_S, NT, "Intel", "Skylake U Base", enable_flash_pch100}, {0x8086, 0x9d46, B_S, NT, "Intel", "Skylake Y Premium", enable_flash_pch100}, - {0x8086, 0x9d48, B_S, NT, "Intel", "Skylake U Premium", enable_flash_pch100}, + {0x8086, 0x9d48, B_S, DEP, "Intel", "Skylake U Premium", enable_flash_pch100}, {0x8086, 0x9d4b, B_S, NT, "Intel", "Kaby Lake Y w/ iHDCP2.2 Prem.", enable_flash_pch100}, {0x8086, 0x9d4e, B_S, DEP, "Intel", "Kaby Lake U w/ iHDCP2.2 Prem.", enable_flash_pch100}, {0x8086, 0x9d50, B_S, NT, "Intel", "Kaby Lake U w/ iHDCP2.2 Base", enable_flash_pch100}, -- To view, visit https://review.coreboot.org/c/flashrom/+/39173 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib8ee9b5e811df74d2f48bd409806c72fe862bc24 Gerrit-Change-Number: 39173 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

4 4

Change in flashrom[master]: raiden_debug_spi.c: Add a delay following AP/EC flash enable
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/+/39312 ) Change subject: raiden_debug_spi.c: Add a delay following AP/EC flash enable ...................................................................... raiden_debug_spi.c: Add a delay following AP/EC flash enable Add a delay following the AP and EC flash enable requests. This allows any power rails enabled by these signals to settle and to meet the power on to first SPI write timing requirements. Forward ports the downstream commit: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/… Change-Id: I4c1777777ee67580605c6e6f4c0c228cccc392c7 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M raiden_debug_spi.c 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/12/39312/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index fa42268..6914455 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -562,6 +562,15 @@ return ret; } + /* + * Allow for power to settle on the AP and EC flash devices. + * Load switches can have a 1-3 ms turn on time, and SPI flash devices + * can require up to 10 ms from power on to the first write. + */ + if ((request_enable == RAIDEN_DEBUG_SPI_REQ_ENABLE_AP) || + (request_enable == RAIDEN_DEBUG_SPI_REQ_ENABLE_EC)) + usleep(50 * 1000); + register_spi_master(&spi_master_raiden_debug); register_shutdown(shutdown, NULL); -- To view, visit https://review.coreboot.org/c/flashrom/+/39312 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c1777777ee67580605c6e6f4c0c228cccc392c7 Gerrit-Change-Number: 39312 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 5

Change in flashrom[master]: raiden_debug_spi.c: Disable retry during some error codes
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/+/39311 ) Change subject: raiden_debug_spi.c: Disable retry during some error codes ...................................................................... raiden_debug_spi.c: Disable retry during some error codes Forward ports the downstream commit: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/… Change-Id: I77def28040fea8d1ecf102463180378f8612b00e Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M raiden_debug_spi.c M usb_device.h 2 files changed, 59 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/11/39311/1 diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index 1f30b6a..fa42268 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -120,6 +120,15 @@ #define GOOGLE_RAIDEN_SPI_SUBCLASS (0x51) #define GOOGLE_RAIDEN_SPI_PROTOCOL (0x01) +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, +}; enum raiden_debug_spi_request { RAIDEN_DEBUG_SPI_REQ_ENABLE = 0x0000, @@ -163,6 +172,29 @@ uint8_t data[PAYLOAD_SIZE]; } __attribute__((packed)) usb_spi_response_t; +/* + * 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 + * that some conditions are not recoverable in the current state so allows us + * to bypass the retry logic and terminate early. + */ +static bool retry_recovery(int error_code) +{ + if (error_code < 0x10000) { + /* Handle error codes returned from the device. */ + if (USB_SPI_WRITE_COUNT_INVALID <= error_code && + error_code <= USB_SPI_DISABLED) { + return false; + } + } else if (usb_device_is_libusb_error(error_code)) { + /* Handle error codes returned from libusb. */ + if (error_code == LIBUSB_ERROR(LIBUSB_ERROR_NO_DEVICE)) { + return false; + } + } + return true; +} + static int write_command(const struct flashctx *flash, unsigned int write_count, unsigned int read_count, @@ -267,6 +299,10 @@ "Write attempt = %d\n" "status = %d\n", write_attempt + 1, status); + if (!retry_recovery(status)) { + /* Reattempting will not result in a recovery. */ + return status; + } programmer_delay(RETY_INTERVAL_US); continue; } @@ -282,6 +318,10 @@ "Read attempt = %d\n" "status = %d\n", 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); } else { /* We were successful at performing the SPI transfer. */ diff --git a/usb_device.h b/usb_device.h index d379675..b2c7656 100644 --- a/usb_device.h +++ b/usb_device.h @@ -27,10 +27,18 @@ #include <libusb.h> #include <stdint.h> +#include <stdbool.h> + +/* + * The LIBUSB_ERROR macro converts a libusb failure code into an error code that + * flashrom recognizes. It does so without displaying an error code allowing us + * to compare error codes against the library enumeration values. + */ +#define LIBUSB_ERROR(eror_code) (0x20000 | -eror_code) /* * The LIBUSB macro converts a libusb failure code into an error code that - * flashrom recognizes. It also displays additional libusb specific + * flashrom recognizes. It also displays additional libusb specific * information about the failure. */ #define LIBUSB(expression) \ @@ -42,7 +50,7 @@ __FILE__, \ __LINE__, \ libusb_error_name(libusb_error__)); \ - libusb_error__ = 0x20000 | -libusb_error__; \ + libusb_error__ = LIBUSB_ERROR(libusb_error__); \ } else { \ libusb_error__ = 0; \ } \ @@ -51,6 +59,15 @@ }) /* + * Returns true if the error code falls within the range of valid libusb + * error codes. + */ +static inline bool usb_device_is_libusb_error(int error_code) +{ + return (0x20000 <= error_code && error_code < 0x20064); +} + +/* * 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 -- To view, visit https://review.coreboot.org/c/flashrom/+/39311 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I77def28040fea8d1ecf102463180378f8612b00e Gerrit-Change-Number: 39311 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: 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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2020