November 2019 - flashrom-gerrit

Change in flashrom[master]: cbtable.c: Factor out lb_table_validation logic
by Edward O'Callaghan (Code Review) 28 Nov '19

28 Nov '19

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37239 ) Change subject: cbtable.c: Factor out lb_table_validation logic ...................................................................... cbtable.c: Factor out lb_table_validation logic Write a pure function for the table validation logic, it is easier to unit-test. Change-Id: I07b0f95ec0443fa6a8f54eb93f4a7ea1875cccad Signed-off-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M cbtable.c 1 file changed, 18 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/37239/1 diff --git a/cbtable.c b/cbtable.c index cd0cf15..14e9a85 100644 --- a/cbtable.c +++ b/cbtable.c @@ -150,6 +150,23 @@ return 1; } +static int lb_table_valid(struct lb_header *head, struct lb_record *recs) +{ + if (compute_checksum(recs, head->table_bytes) + != head->table_checksum) { + msg_perr("Bad table checksum: %04x.\n", + head->table_checksum); + return 0; + } + if (count_lb_records(head) != head->table_entries) { + msg_perr("Bad record count: %d.\n", + head->table_entries); + return 0; + } + + return 1; +} + #define for_each_lbrec(head, rec) \ for(rec = (struct lb_record *)(((char *)head) + sizeof(*head)); \ (((char *)rec) < (((char *)head) + sizeof(*head) + head->table_bytes)) && \ @@ -183,17 +200,8 @@ (struct lb_record *)(((char *)base) + addr + sizeof(*head)); if (!lb_header_valid(head, addr)) continue; - if (count_lb_records(head) != head->table_entries) { - msg_perr("Bad record count: %d.\n", - head->table_entries); + if (!lb_table_valid(head, recs)) continue; - } - if (compute_checksum(recs, head->table_bytes) - != head->table_checksum) { - msg_perr("Bad table checksum: %04x.\n", - head->table_checksum); - continue; - } msg_pdbg("Found coreboot table at 0x%08lx.\n", addr); return head; -- To view, visit https://review.coreboot.org/c/flashrom/+/37239 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I07b0f95ec0443fa6a8f54eb93f4a7ea1875cccad Gerrit-Change-Number: 37239 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 3

Change in flashrom[master]: cbtable.c: Factor out lb_header_validation logic
by Edward O'Callaghan (Code Review) 28 Nov '19

28 Nov '19

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37238 ) Change subject: cbtable.c: Factor out lb_header_validation logic ...................................................................... cbtable.c: Factor out lb_header_validation logic Write a pure function for the header validation logic, it is easier to unit-test. Change-Id: Ia288bcbc5c371329952a6efba30ccf0e18965a3d Signed-off-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M cbtable.c 1 file changed, 20 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/37238/1 diff --git a/cbtable.c b/cbtable.c index bdf53ce..cd0cf15 100644 --- a/cbtable.c +++ b/cbtable.c @@ -131,6 +131,25 @@ return (~chksum.word) & 0xFFFF; } +static int lb_header_valid(struct lb_header *head, unsigned long addr) +{ + if (memcmp(head->signature, "LBIO", 4) != 0) + return 0; + msg_pdbg("Found candidate at: %08lx-%08lx\n", + addr, addr + sizeof(*head) + head->table_bytes); + if (head->header_bytes != sizeof(*head)) { + msg_perr("Header bytes of %d are incorrect.\n", + head->header_bytes); + return 0; + } + if (compute_checksum((uint8_t *) head, sizeof(*head)) != 0) { + msg_perr("Bad header checksum.\n"); + return 0; + } + + return 1; +} + #define for_each_lbrec(head, rec) \ for(rec = (struct lb_record *)(((char *)head) + sizeof(*head)); \ (((char *)rec) < (((char *)head) + sizeof(*head) + head->table_bytes)) && \ @@ -162,24 +181,13 @@ (struct lb_header *)(((char *)base) + addr); struct lb_record *recs = (struct lb_record *)(((char *)base) + addr + sizeof(*head)); - if (memcmp(head->signature, "LBIO", 4) != 0) + if (!lb_header_valid(head, addr)) continue; - msg_pdbg("Found candidate at: %08lx-%08lx\n", - addr, addr + head->table_bytes); - if (head->header_bytes != sizeof(*head)) { - msg_perr("Header bytes of %d are incorrect.\n", - head->header_bytes); - continue; - } if (count_lb_records(head) != head->table_entries) { msg_perr("Bad record count: %d.\n", head->table_entries); continue; } - if (compute_checksum((uint8_t *) head, sizeof(*head)) != 0) { - msg_perr("Bad header checksum.\n"); - continue; - } if (compute_checksum(recs, head->table_bytes) != head->table_checksum) { msg_perr("Bad table checksum: %04x.\n", -- To view, visit https://review.coreboot.org/c/flashrom/+/37238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia288bcbc5c371329952a6efba30ccf0e18965a3d Gerrit-Change-Number: 37238 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 4

Change in flashrom[master]: Add Manibuilder
by Angel Pons (Code Review) 26 Nov '19

26 Nov '19

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23005 ) Change subject: Add Manibuilder ...................................................................... Patch Set 3: Code-Review+2 > Patch Set 3: Code-Review+2 > > All-in-all this seems like a great step forward for build testing. I looked over the patch chain and it seems OK (nice Make-fu), but unfortunately it failed to run and I couldn't figure out why in the short amount of time I had. > > Some day I'd like to re-write most of the Makefile logic in Go or shell and hopefully make it easier to understand and modify, but that shouldn't block this patch. I was able to run this on poweredgy (my Dell Poweredge R900 space heater) some time ago. -- To view, visit https://review.coreboot.org/c/flashrom/+/23005 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I60863a5c7d70dde71486fccb66cb59b30ba4d982 Gerrit-Change-Number: 23005 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Nov 2019 11:11:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: manibuilder: Add list of tags used for the 1.0.x branch
by David Hendricks (Code Review) 25 Nov '19

25 Nov '19

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/30418 ) Change subject: manibuilder: Add list of tags used for the 1.0.x branch ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/30418 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2bc2e79729016a8f9908f316b051deeb73dc096f Gerrit-Change-Number: 30418 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 25 Nov 2019 04:02:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add Manibuilder
by David Hendricks (Code Review) 25 Nov '19

25 Nov '19

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23005 ) Change subject: Add Manibuilder ...................................................................... Patch Set 3: Code-Review+2 All-in-all this seems like a great step forward for build testing. I looked over the patch chain and it seems OK (nice Make-fu), but unfortunately it failed to run and I couldn't figure out why in the short amount of time I had. Some day I'd like to re-write most of the Makefile logic in Go or shell and hopefully make it easier to understand and modify, but that shouldn't block this patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/23005 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I60863a5c7d70dde71486fccb66cb59b30ba4d982 Gerrit-Change-Number: 23005 Gerrit-PatchSet: 3 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 25 Nov 2019 04:02:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: print.c: Dedup 'test_state_to_text()' logic
by Edward O'Callaghan (Code Review) 17 Nov '19

17 Nov '19

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36905 ) Change subject: print.c: Dedup 'test_state_to_text()' logic ...................................................................... print.c: Dedup 'test_state_to_text()' logic Change-Id: I72164323d7ff98fc50cb0c47b69741a4f047e098 Signed-off-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M print.c 1 file changed, 1 insertion(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/36905/1 diff --git a/print.c b/print.c index 6a7ff5d..596fc53 100644 --- a/print.c +++ b/print.c @@ -401,14 +401,7 @@ for (i = 0; i < maxboardlen - strlen(b->name); i++) msg_ginfo(" "); - switch (b->working) { - case OK: msg_ginfo("OK "); break; - case NT: msg_ginfo("NT "); break; - case DEP: msg_ginfo("DEP "); break; - case NA: msg_ginfo("N/A "); break; - case BAD: - default: msg_ginfo("BAD "); break; - } + msg_pinfo(test_state_to_text(b->working)); for (e = board_matches; e->vendor_name != NULL; e++) { if (strcmp(e->vendor_name, b->vendor) -- To view, visit https://review.coreboot.org/c/flashrom/+/36905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I72164323d7ff98fc50cb0c47b69741a4f047e098 Gerrit-Change-Number: 36905 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 5

Change in flashrom[master]: dediprog.c: Add id parameter to dediprog programmer
by Ryan O'Leary (Code Review) 15 Nov '19

15 Nov '19

Ryan O'Leary has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/34160 ) Change subject: dediprog.c: Add id parameter to dediprog programmer ...................................................................... dediprog.c: Add id parameter to dediprog programmer When multiple dediprog programmers are connected, the 'id' parameter allows you to specify which one to use. The id is a string like SF012345 or DP012345. The value is printed on a sticker on the back of the dediprog. This is an improvement over the 'device' parameter which is based on enumeration order and changes when you plug/unplug devices or reboot the machine. To find the id without the sticker, run flashrom with the -V option. This prints the ids as they are enumerated. Alternatively, with dpcmd, you can use the --list-device-id and --fix-device commands to list and write device ids respectively. Note this only supports SF100 at the moment, but SF600 support is possible with more work. Change-Id: I4281213ab02131feb5d47bf66118a001cec0d219 Signed-off-by: Ryan O'Leary <ryanoleary(a)google.com> --- M dediprog.c 1 file changed, 133 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/34160/1 diff --git a/dediprog.c b/dediprog.c index 8029b64..1d10b24 100644 --- a/dediprog.c +++ b/dediprog.c @@ -823,6 +823,46 @@ return 0; } +/* Read the id from the dediprog. This should return the numeric part of the + * serial number found on a sticker on the back of the dediprog. Note this + * number is stored in writable eeprom, so it could get out of sync. Also note, + * this function only supports SF100 at this time, but SF600 support is not too + * much different. + * @return the id on success, -1 on failure + */ +static int dediprog_read_id(void) +{ + int ret; + uint8_t buf[3]; + + ret = libusb_control_transfer(dediprog_handle, REQTYPE_OTHER_IN, + 0x7, /* request */ + 0, /* value */ + 0xEF00, /* index */ + buf, sizeof(buf), + DEFAULT_TIMEOUT); + if (ret != sizeof(buf)) { + msg_perr("Failed to read dediprog id, error %d!\n", ret); + return -1; + } + + return buf[0] << 16 | buf[1] << 8 | buf[2]; +} + +/* Return the prefix to display in front of the id. These two characters also + * appear on the sticker even though they can be derived from the numerical + * part. + * @id a dediprog's id + * @return either "DP" or "SF" + */ +static const char* dediprog_id_prefix(int id) { + /* This is so the id matches dpcmd. */ + if (id < 600000) { + return "DP"; + } + return "SF"; +} + /* * This command presumably sets the voltage for the SF100 itself (not the * SPI flash). Only use this command with firmware older than V6.0.0. Newer @@ -997,9 +1037,11 @@ int dediprog_init(void) { - char *voltage, *device, *spispeed, *target_str; + char *voltage, *id_str, *device, *spispeed, *target_str; int spispeed_idx = 1; int millivolt = 3500; + int id = -1; /* -1 defaults to enumeration order */ + int found_id; long usedevice = 0; long target = FLASH_TYPE_APPLICATION_FLASH_1; int i, ret; @@ -1029,9 +1071,37 @@ msg_pinfo("Setting voltage to %i mV\n", millivolt); } + id_str = extract_programmer_param("id"); + if (id_str) { + char prefix0, prefix1; + const char *prefix; + if (sscanf(id_str, "%c%c%d", &prefix0, &prefix1, &id) != 3) { + msg_perr("Error: Could not parse dediprog 'id'.\n"); + msg_perr("Expected a string like SF012345 or DP012345.\n"); + free(id_str); + return 1; + } + if (id < 0 || id >= 0x1000000) { + msg_perr("Error: id %s is out of range!\n", id_str); + free(id_str); + return 1; + } + prefix = dediprog_id_prefix(id); + if (strlen(prefix) != 2 || prefix[0] != prefix0 || prefix[1] != prefix1) { + msg_perr("Error: %s is an invalid id!\n", id_str); + free(id_str); + return 1; + } + msg_pinfo("Will search for dediprog id %s%06d.\n", dediprog_id_prefix(id), id); + } + free(id_str); + device = extract_programmer_param("device"); if (device) { char *dev_suffix; + if (id != -1) { + msg_perr("Error: Cannot use 'id' and 'device'.\n"); + } errno = 0; usedevice = strtol(device, &dev_suffix, 10); if (errno != 0 || device == dev_suffix) { @@ -1097,25 +1167,69 @@ const uint16_t vid = devs_dediprog[0].vendor_id; const uint16_t pid = devs_dediprog[0].device_id; - dediprog_handle = usb_dev_get_by_vid_pid_number(usb_ctx, vid, pid, (unsigned int) usedevice); - if (!dediprog_handle) { - msg_perr("Could not find a Dediprog programmer on USB.\n"); - libusb_exit(usb_ctx); - return 1; + for (;;) { + /* Notice we can only call dediprog_read_id() after + * libusb_set_configuration() and libusb_claim_interface(). When + * searching by id and either configuration or claim fails + * (usually the device is in use by another instance of + * flashrom), the device is skipped and the next device is + * tried. + */ + dediprog_handle = usb_dev_get_by_vid_pid_number(usb_ctx, vid, pid, (unsigned int) usedevice); + if (!dediprog_handle) { + msg_perr("Could not find a Dediprog programmer on USB.\n"); + libusb_exit(usb_ctx); + return 1; + } + ret = libusb_set_configuration(dediprog_handle, 1); + if (ret != 0) { + msg_perr("Could not set USB device configuration: %i %s\n", + ret, libusb_error_name(ret)); + libusb_close(dediprog_handle); + if (id != -1) { + usedevice++; + continue; + } + libusb_exit(usb_ctx); + return 1; + } + ret = libusb_claim_interface(dediprog_handle, 0); + if (ret < 0) { + msg_perr("Could not claim USB device interface %i: %i %s\n", + 0, ret, libusb_error_name(ret)); + libusb_close(dediprog_handle); + if (id != -1) { + usedevice++; + continue; + } + libusb_exit(usb_ctx); + return 1; + } + + /* Read the dediprog's id. The value is only used (besides + * logging) when searching by id. + */ + found_id = dediprog_read_id(); + if (id != -1) { + if (found_id < 0) { + msg_perr("Could not read id.\n"); + libusb_release_interface(dediprog_handle, 0); + libusb_close(dediprog_handle); + usedevice++; + continue; + } + msg_pinfo("Found dediprog id %s%06d.\n", dediprog_id_prefix(found_id), found_id); + if (found_id != id) { + libusb_release_interface(dediprog_handle, 0); + libusb_close(dediprog_handle); + usedevice++; + continue; + } + } + break; } - ret = libusb_set_configuration(dediprog_handle, 1); - if (ret != 0) { - msg_perr("Could not set USB device configuration: %i %s\n", ret, libusb_error_name(ret)); - libusb_close(dediprog_handle); - libusb_exit(usb_ctx); - return 1; - } - ret = libusb_claim_interface(dediprog_handle, 0); - if (ret < 0) { - msg_perr("Could not claim USB device interface %i: %i %s\n", 0, ret, libusb_error_name(ret)); - libusb_close(dediprog_handle); - libusb_exit(usb_ctx); - return 1; + if (found_id >= 0) { + msg_pinfo("Using dediprog id %s%06d.\n", dediprog_id_prefix(found_id), found_id); } if (register_shutdown(dediprog_shutdown, NULL)) -- To view, visit https://review.coreboot.org/c/flashrom/+/34160 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4281213ab02131feb5d47bf66118a001cec0d219 Gerrit-Change-Number: 34160 Gerrit-PatchSet: 1 Gerrit-Owner: Ryan O'Leary Gerrit-MessageType: newchange

2 11

Change in flashrom[master]: Fix building with GCC 4.9
by Evgeny Zinoviev (Code Review) 14 Nov '19

14 Nov '19

Evgeny Zinoviev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36629 ) Change subject: Fix building with GCC 4.9 ...................................................................... Fix building with GCC 4.9 It doesn't like empty initializers. Change-Id: If2988e60401155f87ee3369c77f00ccf9332012c Signed-off-by: Evgeny Zinoviev <me(a)ch1p.io> --- M atavia.c M it8212.c M pickit2_spi.c M usbblaster_spi.c 4 files changed, 4 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/36629/1 diff --git a/atavia.c b/atavia.c index 80bba58..fdaaa74 100644 --- a/atavia.c +++ b/atavia.c @@ -51,7 +51,7 @@ const struct dev_entry ata_via[] = { {PCI_VENDOR_ID_VIA, 0x3249, DEP, "VIA", "VT6421A"}, - {}, + {0}, }; static void atavia_chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr); diff --git a/it8212.c b/it8212.c index 70b1d9d..ac53a6f 100644 --- a/it8212.c +++ b/it8212.c @@ -26,7 +26,7 @@ const struct dev_entry devs_it8212[] = { {PCI_VENDOR_ID_ITE, 0x8212, NT, "ITE", "8212F PATA RAID"}, - {}, + {0}, }; #define IT8212_MEMMAP_SIZE (128 * 1024) diff --git a/pickit2_spi.c b/pickit2_spi.c index 784dc13..2936ee8 100644 --- a/pickit2_spi.c +++ b/pickit2_spi.c @@ -49,7 +49,7 @@ const struct dev_entry devs_pickit2_spi[] = { {0x04D8, 0x0033, OK, "Microchip", "PICkit 2"}, - {} + {0} }; static libusb_device_handle *pickit2_handle; diff --git a/usbblaster_spi.c b/usbblaster_spi.c index d6b5084..e2a0a4f 100644 --- a/usbblaster_spi.c +++ b/usbblaster_spi.c @@ -48,7 +48,7 @@ const struct dev_entry devs_usbblasterspi[] = { {ALTERA_VID, ALTERA_USBBLASTER_PID, OK, "Altera", "USB-Blaster"}, - {} + {0} }; static const struct spi_master spi_master_usbblaster; -- To view, visit https://review.coreboot.org/c/flashrom/+/36629 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If2988e60401155f87ee3369c77f00ccf9332012c Gerrit-Change-Number: 36629 Gerrit-PatchSet: 1 Gerrit-Owner: Evgeny Zinoviev <me(a)ch1p.io> Gerrit-MessageType: newchange

2 2

Change in flashrom[master]: sb600spi.c: Generalise determin_generation() after Yangtze
by Edward O'Callaghan (Code Review) 14 Nov '19

14 Nov '19

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36426 ) Change subject: sb600spi.c: Generalise determin_generation() after Yangtze ...................................................................... sb600spi.c: Generalise determin_generation() after Yangtze Drop dead USE_YANGTZE_HEURISTICS code. Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Signed-off-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M sb600spi.c 1 file changed, 4 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/36426/1 diff --git a/sb600spi.c b/sb600spi.c index 48baf15..482c2f6 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -118,22 +118,6 @@ } else if (dev->device_id == 0x780e) { /* The PCI ID of the LPC bridge doesn't change between Hudson-2/3/4 and Yangtze (Kabini/Temash) * although they use different SPI interfaces. */ -#ifdef USE_YANGTZE_HEURISTICS - /* This heuristic accesses the SPI interface MMIO BAR at locations beyond those supported by - * Hudson in the hope of getting 0xff readback on older chipsets and non-0xff readback on - * Yangtze (and newer, compatible chipsets). */ - int i; - msg_pdbg("Checking for AMD Yangtze (Kabini/Temash) or later... "); - for (i = 0x20; i <= 0x4f; i++) { - if (mmio_readb(sb600_spibar + i) != 0xff) { - amd_gen = CHIPSET_YANGTZE; - msg_pdbg("found.\n"); - return 0; - } - } - msg_pdbg("not found. Assuming Hudson.\n"); - amd_gen = CHIPSET_HUDSON234; -#else int rev = find_smbus_dev(0x1022, 0x780B); if (rev < 0) return -1; @@ -165,7 +149,6 @@ } -#endif } else msg_pwarn("%s: Unknown LPC device %" PRIx16 ":%" PRIx16 ".\n" "Please report this to flashrom(a)flashrom.org and include this log and\n" @@ -595,7 +578,7 @@ msg_pdbg("SpiRomEnable=%i", (tmp >> 1) & 0x1); if (amd_gen == CHIPSET_SB7XX) msg_pdbg(", AltSpiCSEnable=%i, AbortEnable=%i", tmp & 0x1, (tmp >> 2) & 0x1); - else if (amd_gen == CHIPSET_YANGTZE) + else if (amd_gen >= CHIPSET_YANGTZE) msg_pdbg(", RouteTpm2Sp=%i", (tmp >> 3) & 0x1); tmp = pci_read_byte(dev, 0xba); @@ -631,13 +614,13 @@ */ tmp = mmio_readl(sb600_spibar + 0x00); msg_pdbg("(0x%08" PRIx32 ") SpiArbEnable=%i", tmp, (tmp >> 19) & 0x1); - if (amd_gen == CHIPSET_YANGTZE) + if (amd_gen >= CHIPSET_YANGTZE) msg_pdbg(", IllegalAccess=%i", (tmp >> 21) & 0x1); msg_pdbg(", SpiAccessMacRomEn=%i, SpiHostAccessRomEn=%i, ArbWaitCount=%i", (tmp >> 22) & 0x1, (tmp >> 23) & 0x1, (tmp >> 24) & 0x7); - if (amd_gen != CHIPSET_YANGTZE) + if (amd_gen < CHIPSET_YANGTZE) msg_pdbg(", SpiBridgeDisable=%i", (tmp >> 27) & 0x1); switch (amd_gen) { @@ -710,7 +693,7 @@ return ERROR_FATAL; /* Starting with Yangtze the SPI controller got a different interface with a much bigger buffer. */ - if (amd_gen != CHIPSET_YANGTZE) + if (amd_gen < CHIPSET_YANGTZE) register_spi_master(&spi_master_sb600); else register_spi_master(&spi_master_yangtze); -- To view, visit https://review.coreboot.org/c/flashrom/+/36426 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5aa7370025f5c1af56c6cb96194b6f3007d0ede7 Gerrit-Change-Number: 36426 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 4

Change in flashrom[master]: sb600spi.c: Fold up debug logic into determine_generation()
by Edward O'Callaghan (Code Review) 14 Nov '19

14 Nov '19

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/36425 ) Change subject: sb600spi.c: Fold up debug logic into determine_generation() ...................................................................... sb600spi.c: Fold up debug logic into determine_generation() Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Signed-off-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M sb600spi.c 1 file changed, 12 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/36425/1 diff --git a/sb600spi.c b/sb600spi.c index c89d5a9..48baf15 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -91,7 +91,8 @@ return pci_read_byte(smbus_dev, PCI_REVISION_ID); } -static void determine_generation(struct pci_dev *dev) +/* Determine the chipset's version and identify the respective SMBUS device. */ +static int determine_generation(struct pci_dev *dev) { amd_gen = CHIPSET_AMD_UNKNOWN; msg_pdbg2("Trying to determine the generation of the SPI interface... "); @@ -101,7 +102,7 @@ } else if (dev->device_id == 0x439d) { int rev = find_smbus_dev(0x1002, 0x4385); if (rev < 0) - return; + return -1; if (rev >= 0x39 && rev <= 0x3D) { amd_gen = CHIPSET_SB7XX; msg_pdbg("SB7xx/SP5100 detected.\n"); @@ -127,7 +128,7 @@ if (mmio_readb(sb600_spibar + i) != 0xff) { amd_gen = CHIPSET_YANGTZE; msg_pdbg("found.\n"); - return; + return 0; } } msg_pdbg("not found. Assuming Hudson.\n"); @@ -135,7 +136,7 @@ #else int rev = find_smbus_dev(0x1022, 0x780B); if (rev < 0) - return; + return -1; if (rev >= 0x11 && rev <= 0x15) { amd_gen = CHIPSET_HUDSON234; msg_pdbg("Hudson-2/3/4 detected.\n"); @@ -153,7 +154,7 @@ } else if (dev->device_id == 0x790e) { int rev = find_smbus_dev(0x1022, 0x790B); if (rev < 0) - return; + return -1; if (rev == 0x4a) { amd_gen = CHIPSET_YANGTZE; msg_pdbg("Yangtze detected.\n"); @@ -170,6 +171,11 @@ "Please report this to flashrom(a)flashrom.org and include this log and\n" "the output of lspci -nnvx, thanks!\n", __func__, dev->vendor_id, dev->device_id); + if (amd_gen == CHIPSET_AMD_UNKNOWN) { + msg_perr("Could not determine chipset generation."); + return -1; + } + return 0; } static void reset_internal_fifo_pointer(void) @@ -565,11 +571,8 @@ */ sb600_spibar += tmp & 0xfff; - determine_generation(dev); - if (amd_gen == CHIPSET_AMD_UNKNOWN) { - msg_perr("Could not determine chipset generation."); + if (determine_generation(dev) < 0) return ERROR_NONFATAL; - } /* How to read the following table and similar ones in this file: * "?" means we have no datasheet for this chipset generation or it doesn't have any relevant info. -- To view, visit https://review.coreboot.org/c/flashrom/+/36425 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c722e29b321285bf20fb5ee30c912dcdd83411b Gerrit-Change-Number: 36425 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 5

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2019