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

Change in flashrom[master]: chipset_enable.c: Add PCI ID for Comet Lake.
by Joshua Wise (Code Review) 29 Aug '20

29 Aug '20

Joshua Wise has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/44921 ) Change subject: chipset_enable.c: Add PCI ID for Comet Lake. ...................................................................... chipset_enable.c: Add PCI ID for Comet Lake. Change-Id: Ic6eec4a650155810dc8430bec5b4f5d3091cf37f --- M chipset_enable.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/44921/1 diff --git a/chipset_enable.c b/chipset_enable.c index 5fc222b..dbb4f7c 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -2068,6 +2068,7 @@ {0x8086, 0xa30d, B_S, NT, "Intel", "HM370", enable_flash_pch300}, {0x8086, 0xa30e, B_S, DEP, "Intel", "CM246", enable_flash_pch300}, {0x8086, 0x3482, B_S, DEP, "Intel", "Ice Lake U Premium", enable_flash_pch300}, + {0x8086, 0x02a4, B_S, NT, "Intel", "Comet Lake", enable_flash_pch300}, #endif {0}, }; -- To view, visit https://review.coreboot.org/c/flashrom/+/44921 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic6eec4a650155810dc8430bec5b4f5d3091cf37f Gerrit-Change-Number: 44921 Gerrit-PatchSet: 1 Gerrit-Owner: Joshua Wise <joshua(a)joshuawise.com> Gerrit-MessageType: newchange

3 5

Change in flashrom[master]: chipset_enable: Mark Intel Q77 as DEP
by Jacob Garber (Code Review) 29 Aug '20

29 Aug '20

Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/44910 ) Change subject: chipset_enable: Mark Intel Q77 as DEP ...................................................................... chipset_enable: Mark Intel Q77 as DEP Tested reading and writing internal flash on Dell Optiplex 9010 SFF. Signed-off-by: Jacob Garber <jgarber1(a)ualberta.ca> Change-Id: I4717959be1b79aa986f1276589d01ce7475bda8f --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/44910/1 diff --git a/chipset_enable.c b/chipset_enable.c index 4772a1c..0810488 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1830,7 +1830,7 @@ {0x8086, 0x1d41, B_FS, DEP, "Intel", "C60x/X79", enable_flash_pch6}, {0x8086, 0x1e44, B_FS, DEP, "Intel", "Z77", enable_flash_pch7}, {0x8086, 0x1e46, B_FS, NT, "Intel", "Z75", enable_flash_pch7}, - {0x8086, 0x1e47, B_FS, NT, "Intel", "Q77", enable_flash_pch7}, + {0x8086, 0x1e47, B_FS, DEP, "Intel", "Q77", enable_flash_pch7}, {0x8086, 0x1e48, B_FS, DEP, "Intel", "Q75", enable_flash_pch7}, {0x8086, 0x1e49, B_FS, DEP, "Intel", "B75", enable_flash_pch7}, {0x8086, 0x1e4a, B_FS, DEP, "Intel", "H77", enable_flash_pch7}, -- To view, visit https://review.coreboot.org/c/flashrom/+/44910 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4717959be1b79aa986f1276589d01ce7475bda8f Gerrit-Change-Number: 44910 Gerrit-PatchSet: 1 Gerrit-Owner: Jacob Garber <jgarber1(a)ualberta.ca> Gerrit-MessageType: newchange

3 3

Change in flashrom[master]: add PCI IDs for additional c620 serials PCH chips
by Jonathan Zhang (Code Review) 27 Aug '20

27 Aug '20

Jonathan Zhang has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/44620 ) Change subject: add PCI IDs for additional c620 serials PCH chips ...................................................................... add PCI IDs for additional c620 serials PCH chips Add PCI IDs for C621A, C627A and C629A. Change-Id: I636becd9f08bdf604c6af81ce396049655353b04 Signed-off-by: Jonathan Zhang <jonzhang(a)fb.com> --- M chipset_enable.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/20/44620/1 diff --git a/chipset_enable.c b/chipset_enable.c index d56a547..a9e1e0b 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -2034,6 +2034,9 @@ {0x8086, 0xa1c8, B_S, NT, "Intel", "C620 Series Chipset (QS/PRQ)", enable_flash_c620}, {0x8086, 0xa1c9, B_S, NT, "Intel", "C620 Series Chipset (QS/PRQ)", enable_flash_c620}, {0x8086, 0xa1ca, B_S, NT, "Intel", "C629 Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa1cb, B_S, NT, "Intel", "C621A Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa1cc, B_S, NT, "Intel", "C627A Series Chipset (QS/PRQ)", enable_flash_c620}, + {0x8086, 0xa1cd, B_S, NT, "Intel", "C629A Series Chipset (QS/PRQ)", enable_flash_c620}, {0x8086, 0xa240, B_S, NT, "Intel", "C620 Series Chipset Supersku", enable_flash_c620}, {0x8086, 0xa241, B_S, NT, "Intel", "C620 Series Chipset Supersku", enable_flash_c620}, {0x8086, 0xa242, B_S, NT, "Intel", "C624 Series Chipset Supersku", enable_flash_c620}, -- To view, visit https://review.coreboot.org/c/flashrom/+/44620 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I636becd9f08bdf604c6af81ce396049655353b04 Gerrit-Change-Number: 44620 Gerrit-PatchSet: 1 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-MessageType: newchange

3 5

Change in flashrom[master]: spi: Allow cached ID bytes to be cleared
by Edward O'Callaghan (Code Review) 26 Aug '20

26 Aug '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41399 ) Change subject: spi: Allow cached ID bytes to be cleared ...................................................................... spi: Allow cached ID bytes to be cleared This does two things: 1. Removes the static variables from various ID functions and moves them to a single struct. 2. Adds a function to clear the struct. The idea of the original caching mechanism introduced years ago was simply to speed up probe_flash() by not issuing the same read ID commands dozens or hundreds of times for a single chip. That implementation assumed that we would only call probe_flash() once. However, there are cases when we want to call probe_flash() multiple times, for example, if using an external programmer and using different voltages (for probing the chip). This patch is extremely similar to the original works of: `commit 57b7524b1448189d3630d6c4735e60dbbdf14d51`. Author: David Hendricks <dhendrix(a)chromium.org> && `commit 7f7c711ee1a41649607f81f1533e5135fc1361fc`. Author: David Hendricks <dhendrix(a)chromium.org> BUG=b:15656443 BRANCH=none TEST=none Change-Id: I879cb08dbe66db9ab0c3b8a7f93b04fe1c5980f4 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M spi.h M spi25.c 2 files changed, 50 insertions(+), 26 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/41399/1 diff --git a/spi.h b/spi.h index 3f45038..c1ca46a 100644 --- a/spi.h +++ b/spi.h @@ -192,4 +192,6 @@ #define SPI_FLASHROM_BUG -5 #define SPI_PROGRAMMER_ERROR -6 +void clear_spi_id_cache(void); + #endif /* !__SPI_H__ */ diff --git a/spi25.c b/spi25.c index fb91903..b4fbe6d 100644 --- a/spi25.c +++ b/spi25.c @@ -27,6 +27,26 @@ #include "programmer.h" #include "spi.h" +enum id_type { + RDID, + RDID4, + REMS, + RES2, + RES3, + NUM_ID_TYPES, +}; + +static struct { + int is_cached; + unsigned char bytes[4]; /* enough to hold largest ID type */ +} id_cache[NUM_ID_TYPES]; + +void clear_spi_id_cache(void) +{ + memset(id_cache, 0, sizeof(id_cache)); + return; +} + static int spi_rdid(struct flashctx *flash, unsigned char *readarr, int bytes) { static const unsigned char cmd[JEDEC_RDID_OUTSIZE] = { JEDEC_RDID }; @@ -138,18 +158,20 @@ static int probe_spi_rdid_generic(struct flashctx *flash, int bytes) { const struct flashchip *chip = flash->chip; - unsigned char readarr[4]; uint32_t id1; uint32_t id2; + enum id_type idty = bytes == 3 ? RDID : RDID4; - const int ret = spi_rdid(flash, readarr, bytes); - if (ret == SPI_INVALID_LENGTH) - msg_cinfo("%d byte RDID not supported on this SPI controller\n", bytes); - if (ret) - return 0; + if (!id_cache[idty].is_cached) { + const int ret = spi_rdid(flash, id_cache[idty].bytes, bytes); + if (ret == SPI_INVALID_LENGTH) + msg_cinfo("%d byte RDID not supported on this SPI controller\n", bytes); + if (ret) + return 0; + id_cache[idty].is_cached = 1; + } - rdid_get_ids(readarr, bytes, &id1, &id2); - + rdid_get_ids(id_cache[idty].bytes, bytes, &id1, &id2); return compare_id(chip, id1, id2); } @@ -166,16 +188,16 @@ int probe_spi_rems(struct flashctx *flash) { const struct flashchip *chip = flash->chip; - unsigned char readarr[JEDEC_REMS_INSIZE]; uint32_t id1, id2; - if (spi_rems(flash, readarr)) { - return 0; + if (!id_cache[REMS].is_cached) { + if (spi_rems(flash, id_cache[REMS].bytes)) + return 0; + id_cache[REMS].is_cached = 1; } - id1 = readarr[0]; - id2 = readarr[1]; - + id1 = id_cache[REMS].bytes[0]; + id2 = id_cache[REMS].bytes[1]; return compare_id(chip, id1, id2); } @@ -222,16 +244,16 @@ int probe_spi_res2(struct flashctx *flash) { - unsigned char readarr[2]; uint32_t id1, id2; - if (spi_res(flash, readarr, 2)) { - return 0; + if (!id_cache[RES2].is_cached) { + if (spi_res(flash, id_cache[RES2].bytes, 2)) + return 0; + id_cache[RES2].is_cached = 1; } - id1 = readarr[0]; - id2 = readarr[1]; - + id1 = id_cache[RES2].bytes[0]; + id2 = id_cache[RES2].bytes[1]; msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2); if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id) @@ -242,16 +264,16 @@ int probe_spi_res3(struct flashctx *flash) { - unsigned char readarr[3]; uint32_t id1, id2; - if (spi_res(flash, readarr, 3)) { - return 0; + if (!id_cache[RES3].is_cached) { + if (spi_res(flash, id_cache[RES3].bytes, 3)) + return 0; + id_cache[RES3].is_cached = 1; } - id1 = (readarr[0] << 8) | readarr[1]; - id2 = readarr[2]; - + id1 = (id_cache[RES3].bytes[0] << 8) | id_cache[RES3].bytes[1]; + id2 = id_cache[RES3].bytes[3]; msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2); if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id) -- To view, visit https://review.coreboot.org/c/flashrom/+/41399 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I879cb08dbe66db9ab0c3b8a7f93b04fe1c5980f4 Gerrit-Change-Number: 41399 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 4

Change in flashrom[master]: spi25.c: Factor out rdid_get_ids() and compare_id()
by Edward O'Callaghan (Code Review) 26 Aug '20

26 Aug '20

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41398 ) Change subject: spi25.c: Factor out rdid_get_ids() and compare_id() ...................................................................... spi25.c: Factor out rdid_get_ids() and compare_id() This is in preparation for implementing a cache for the probe results of RDID and REMS (3&4-byte variant) commands. The intention is to make probing of SPI rom's slightly faster, a few 10's of ms dependant upon the spi master used. BUG=b:15656443 BRANCH=none TEST=builds Change-Id: I1556e97a7c70425069e3d1dc0d5daf0aeec4e7bf Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M spi25.c 1 file changed, 30 insertions(+), 32 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/41398/1 diff --git a/spi25.c b/spi25.c index 0a939e7..fb91903 100644 --- a/spi25.c +++ b/spi25.c @@ -93,19 +93,9 @@ return spi_send_command(flash, sizeof(cmd), 0, cmd, NULL); } -static int probe_spi_rdid_generic(struct flashctx *flash, int bytes) +static void rdid_get_ids(unsigned char *readarr, int bytes, + uint32_t *id1, uint32_t *id2) { - const struct flashchip *chip = flash->chip; - unsigned char readarr[4]; - uint32_t id1; - uint32_t id2; - - const int ret = spi_rdid(flash, readarr, bytes); - if (ret == SPI_INVALID_LENGTH) - msg_cinfo("%d byte RDID not supported on this SPI controller\n", bytes); - if (ret) - return 0; - if (!oddparity(readarr[0])) msg_cdbg("RDID byte 0 parity violation. "); @@ -115,17 +105,20 @@ if (readarr[0] == 0x7f) { if (!oddparity(readarr[1])) msg_cdbg("RDID byte 1 parity violation. "); - id1 = (readarr[0] << 8) | readarr[1]; - id2 = readarr[2]; + *id1 = (readarr[0] << 8) | readarr[1]; + *id2 = readarr[2]; if (bytes > 3) { - id2 <<= 8; - id2 |= readarr[3]; + *id2 <<= 8; + *id2 |= readarr[3]; } } else { - id1 = readarr[0]; - id2 = (readarr[1] << 8) | readarr[2]; + *id1 = readarr[0]; + *id2 = (readarr[1] << 8) | readarr[2]; } +} +static int compare_id(const struct flashchip *chip, uint32_t id1, uint32_t id2) +{ msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2); if (id1 == chip->manufacture_id && id2 == chip->model_id) @@ -142,6 +135,24 @@ return 0; } +static int probe_spi_rdid_generic(struct flashctx *flash, int bytes) +{ + const struct flashchip *chip = flash->chip; + unsigned char readarr[4]; + uint32_t id1; + uint32_t id2; + + const int ret = spi_rdid(flash, readarr, bytes); + if (ret == SPI_INVALID_LENGTH) + msg_cinfo("%d byte RDID not supported on this SPI controller\n", bytes); + if (ret) + return 0; + + rdid_get_ids(readarr, bytes, &id1, &id2); + + return compare_id(chip, id1, id2); +} + int probe_spi_rdid(struct flashctx *flash) { return probe_spi_rdid_generic(flash, 3); @@ -165,20 +176,7 @@ id1 = readarr[0]; id2 = readarr[1]; - msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2); - - if (id1 == chip->manufacture_id && id2 == chip->model_id) - return 1; - - /* Test if this is a pure vendor match. */ - if (id1 == chip->manufacture_id && GENERIC_DEVICE_ID == chip->model_id) - return 1; - - /* Test if there is any vendor ID. */ - if (GENERIC_MANUF_ID == chip->manufacture_id && id1 != 0xff && id1 != 0x00) - return 1; - - return 0; + return compare_id(chip, id1, id2); } int probe_spi_res1(struct flashctx *flash) -- To view, visit https://review.coreboot.org/c/flashrom/+/41398 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1556e97a7c70425069e3d1dc0d5daf0aeec4e7bf Gerrit-Change-Number: 41398 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 4

Change in flashrom[master]: chipset_enable.c: Add support for Intel C620 Series Chipset SPI Contr...
by Luka Kovacic (Code Review) 25 Aug '20

25 Aug '20

Luka Kovacic has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/44162 ) Change subject: chipset_enable.c: Add support for Intel C620 Series Chipset SPI Controller ...................................................................... chipset_enable.c: Add support for Intel C620 Series Chipset SPI Controller Support for the Intel C620 Series Chipset SPI Controller (rev 04) is added to enable SPI flash access on the following platform: - Intel Xeon D-2187NT Support for this controller was shortly tested on the platform above. The flash is recognized, some regions of the flash are locked. Signed-off-by: Luka Kovacic <luka.kovacic(a)sartura.hr> Tested-by: Jakov Petrina <jakov.petrina(a)sartura.hr> Cc: Luka Perkov <luka.perkov(a)sartura.hr> Change-Id: If39d9bb1acd4029f802a44a2940dd23f66ba09b1 --- M chipset_enable.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/44162/1 diff --git a/chipset_enable.c b/chipset_enable.c index d56a547..4772a1c 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -2023,6 +2023,7 @@ {0x8086, 0xa153, B_S, NT, "Intel", "QM175", enable_flash_pch100}, {0x8086, 0xa154, B_S, NT, "Intel", "CM238", enable_flash_pch100}, {0x8086, 0xa155, B_S, NT, "Intel", "QMU185", enable_flash_pch100}, + {0x8086, 0xa1a4, B_S, DEP, "Intel", "C620 Series Chipset (QS/PRQ)", enable_flash_c620}, {0x8086, 0xa1c0, B_S, NT, "Intel", "C620 Series Chipset (QS/PRQ)", enable_flash_c620}, {0x8086, 0xa1c1, B_S, NT, "Intel", "C621 Series Chipset (QS/PRQ)", enable_flash_c620}, {0x8086, 0xa1c2, B_S, NT, "Intel", "C622 Series Chipset (QS/PRQ)", enable_flash_c620}, -- To view, visit https://review.coreboot.org/c/flashrom/+/44162 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If39d9bb1acd4029f802a44a2940dd23f66ba09b1 Gerrit-Change-Number: 44162 Gerrit-PatchSet: 1 Gerrit-Owner: Luka Kovacic <luka.kovacic(a)sartura.hr> Gerrit-MessageType: newchange

3 4

Change in ...flashrom[master]: dediprog: Use 4BA page program without 0x12
by David Hendricks (Code Review) 25 Aug '20

25 Aug '20

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/30521 Change subject: dediprog: Use 4BA page program without 0x12 ...................................................................... dediprog: Use 4BA page program without 0x12 It's not clear exactly what the difference is between WRITE_MODE_4B_ADDR_256B_PAGE_PGM with and without _0x12 from the SF600 command spec, but the former seems to work more reliably. Tested using an SF600 (FW v7.2.21), IS25LP256D, W25Q256JV, and MX25L25735. Change-Id: I3cb37ec8838f5bb02948ed52b4a2906fa033cf83 Signed-off-by: David Hendricks <david.hendricks(a)gmail.com> --- M dediprog.c 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/30521/1 diff --git a/dediprog.c b/dediprog.c index 2a190d2..303f1d5 100644 --- a/dediprog.c +++ b/dediprog.c @@ -398,7 +398,8 @@ } } else if (dedi_spi_cmd == WRITE_MODE_PAGE_PGM) { if (flash->chip->feature_bits & FEATURE_4BA_WRITE) { - data_packet[3] = WRITE_MODE_4B_ADDR_256B_PAGE_PGM_0x12; +// data_packet[3] = WRITE_MODE_4B_ADDR_256B_PAGE_PGM_0x12; + data_packet[3] = WRITE_MODE_4B_ADDR_256B_PAGE_PGM; data_packet[4] = JEDEC_BYTE_PROGRAM_4BA; use_4ba = true; } else if (flash->in_4ba_mode) { -- To view, visit https://review.coreboot.org/c/flashrom/+/30521 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3cb37ec8838f5bb02948ed52b4a2906fa033cf83 Gerrit-Change-Number: 30521 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-MessageType: newchange

1 1

Change in flashrom[master]: flashrom.c: do not delete programmer arguments
by Nikolai Artemiev (Code Review) 25 Aug '20

25 Aug '20

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/44562 ) Change subject: flashrom.c: do not delete programmer arguments ...................................................................... flashrom.c: do not delete programmer arguments Make extract_param() leave parameters in tact rather than deleting them as they are retrieved. Deleting them is undesierable as makes the function more complex and requires that extract_programmer_param() does not get called for the same parameter multiple times, which can be difficult to gaurentee for some programmer initialization cases. Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c 1 file changed, 0 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/44562/1 diff --git a/flashrom.c b/flashrom.c index 4a30986..f3f0b46 100644 --- a/flashrom.c +++ b/flashrom.c @@ -750,11 +750,6 @@ } strncpy(opt, opt_pos, optlen); opt[optlen] = '\0'; - rest = opt_pos + optlen; - /* Skip all delimiters after the current parameter. */ - rest += strspn(rest, delim); - memmove(param_pos, rest, strlen(rest) + 1); - /* We could shrink haystack, but the effort is not worth it. */ } return opt; -- To view, visit https://review.coreboot.org/c/flashrom/+/44562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b1ec2d273c723fef8908e4ddf13b3079924f0a7 Gerrit-Change-Number: 44562 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

3 10

Change in flashrom[master]: Makefile: Fix building on AArch64 NixOS
by Pyry Kontio (Code Review) 24 Aug '20

24 Aug '20

Pyry Kontio has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/43140 ) Change subject: Makefile: Fix building on AArch64 NixOS ...................................................................... Makefile: Fix building on AArch64 NixOS Signed-off-by: Pyry Kontio <pyry.kontio(a)drasa.eu> Change-Id: Iaa4477a71e758cf9ecad2c22f3b77bc6508a3510 --- M Makefile 1 file changed, 8 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/43140/1 diff --git a/Makefile b/Makefile index 803529f..6a2abd1 100644 --- a/Makefile +++ b/Makefile @@ -106,7 +106,8 @@ # IMPORTANT: The following line must be placed before TARGET_OS is ever used # (of course), but should come after any lines setting CC because the line # below uses CC itself. -override TARGET_OS := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E os.h 2>/dev/null | grep -v '^\#' | grep '"' | cut -f 2 -d'"')) +override TARGET_OS := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E os.h 2>/dev/null \ + | grep -v '^\#' | grep '"' | tail -1 | cut -f 2 -d'"')) ifeq ($(TARGET_OS), Darwin) override CPPFLAGS += -I/opt/local/include -I/usr/local/include @@ -460,8 +461,10 @@ # IMPORTANT: The following line must be placed before ARCH is ever used # (of course), but should come after any lines setting CC because the line # below uses CC itself. -override ARCH := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E archtest.c 2>/dev/null | grep -v '^\#' | grep '"' | cut -f 2 -d'"')) -override ENDIAN := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E endiantest.c 2>/dev/null | grep -v '^\#')) +override ARCH := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E archtest.c 2>/dev/null \ + | grep -v '^\#' | grep '"' | tail -1 | cut -f 2 -d'"')) +override ENDIAN := $(strip $(call debug_shell,$(CC) $(CPPFLAGS) -E endiantest.c 2>/dev/null \ + | grep -v '^\#')) # Disable the internal programmer on unsupported architectures (everything but x86 and mipsel) ifneq ($(ARCH)-little, $(filter $(ARCH),x86 mips)-$(ENDIAN)) @@ -1253,12 +1256,12 @@ @printf "Target arch is " @# FreeBSD wc will output extraneous whitespace. @echo $(ARCH)|wc -w|grep -q '^[[:blank:]]*1[[:blank:]]*$$' || \ - ( echo "unknown. Aborting."; exit 1) + ( echo "unknown (\"$(ARCH)\"). Aborting."; exit 1) @printf "%s\n" '$(ARCH)' @printf "Target OS is " @# FreeBSD wc will output extraneous whitespace. @echo $(TARGET_OS)|wc -w|grep -q '^[[:blank:]]*1[[:blank:]]*$$' || \ - ( echo "unknown. Aborting."; exit 1) + ( echo "unknown (\"$(TARGET_OS)\"). Aborting."; exit 1) @printf "%s\n" '$(TARGET_OS)' ifeq ($(TARGET_OS), libpayload) @$(CC) --version 2>&1 | grep -q coreboot || \ -- To view, visit https://review.coreboot.org/c/flashrom/+/43140 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iaa4477a71e758cf9ecad2c22f3b77bc6508a3510 Gerrit-Change-Number: 43140 Gerrit-PatchSet: 1 Gerrit-Owner: Pyry Kontio <pyry.kontio(a)drasa.eu> Gerrit-MessageType: newchange

4 17

Change in flashrom[master]: udelay.c: Enable providing delay calibration value through CLI
by Hrvoje Čavrak (Code Review) 21 Aug '20

21 Aug '20

Hrvoje Čavrak has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/44517 ) Change subject: udelay.c: Enable providing delay calibration value through CLI ...................................................................... udelay.c: Enable providing delay calibration value through CLI This commit implements --get-loop-delay/--set-loop-delay options to retrieve and provide the calibration loop delay through the command line. The delay calibration procedure can take up to few seconds, which adds up when executed a number of times consecutively. Therefore, this provides a means to execute it only once, and then re-use the calibrated value on subsequent runs. Also, it implements a sanity check, expecting the value to land within 10% of the theoretical delay expectance, otherwise it ignores the provided value and runs the calibration procedure instead. Signed-off-by: Hrvoje Cavrak <hrvoje(a)hrvoje.org> Change-Id: I7c9a491c5cb5fb3af56a691a0c6af9b53dcff550 --- M cli_classic.c M flashrom.8.tmpl M libflashrom.c M programmer.h M udelay.c 5 files changed, 60 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/44517/1 diff --git a/cli_classic.c b/cli_classic.c index 967ff50..b023c45 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -63,6 +63,8 @@ " -o | --output <logfile> log output to <logfile>\n" " --flash-contents <ref-file> assume flash contents to be <ref-file>\n" " -L | --list-supported print supported devices\n" + " --set-loop-delay <value> set loop delay calibration manually\n" + " --get-loop-delay retrieve loop delay calibration\n" #if CONFIG_PRINT_WIKI == 1 " -z | --list-supported-wiki print supported devices in wiki syntax\n" #endif @@ -118,6 +120,7 @@ int flash_name = 0, flash_size = 0; int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0; int dont_verify_it = 0, dont_verify_all = 0, list_supported = 0, operation_specified = 0; + unsigned long provided_delay = 0; struct flashrom_layout *layout = NULL; enum programmer prog = PROGRAMMER_INVALID; enum { @@ -127,6 +130,8 @@ OPTION_FLASH_CONTENTS, OPTION_FLASH_NAME, OPTION_FLASH_SIZE, + OPTION_GET_LOOP_DELAY, + OPTION_SET_LOOP_DELAY, }; int ret = 0; @@ -156,6 +161,8 @@ {"help", 0, NULL, 'h'}, {"version", 0, NULL, 'R'}, {"output", 1, NULL, 'o'}, + {"get-loop-delay", 0, NULL, OPTION_GET_LOOP_DELAY}, + {"set-loop-delay", 1, NULL, OPTION_SET_LOOP_DELAY}, {NULL, 0, NULL, 0}, }; @@ -365,6 +372,14 @@ } #endif /* STANDALONE */ break; + case OPTION_GET_LOOP_DELAY: + /* Make it easy for scripts to parse this by omitting anything else */ + msg_pinfo("%lu\n", get_calibration_value()); + exit(0); + break; + case OPTION_SET_LOOP_DELAY: + provided_delay = strtoul(strdup(optarg), NULL, 0); + break; default: cli_classic_abort_usage(NULL); break; @@ -457,7 +472,10 @@ } /* FIXME: Delay calibration should happen in programmer code. */ - myusec_calibrate_delay(); + if (provided_delay) + set_external_calibration(provided_delay); + else + myusec_calibrate_delay(0); if (programmer_init(prog, pparam)) { msg_perr("Error: Programmer initialization failed.\n"); diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index db50d59..4aada91 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -50,7 +50,7 @@ [\fB\-E\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>] [(\fB\-l\fR <file>|\fB\-\-ifd|\fB \-\-fmap\fR|\fB\-\-fmap-file\fR <file>) [\fB\-i\fR <image>]] [\fB\-n\fR] [\fB\-N\fR] [\fB\-f\fR])] - [\fB\-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>] + [\fB\-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>] [\fB-d\fR get | <value>] .SH DESCRIPTION .B flashrom is a utility for detecting, reading, writing, verifying and erasing flash @@ -364,6 +364,17 @@ .TP .B "\-R, \-\-version" Show version information and exit. +.TP +.B "\-\-get\-loop\-delay" +Retrieve the loop calibration delay value or provide one externally. Calibrate +delay procedure can take a few seconds and for repeated execution on a large +number of hosts it can provide a significant overhead. By getting the value, +it can later be provided by \-\-set\-loop\-delay +.TP +.B "\-\-set\-loop\-delay <value>" +This argument makes it possible to provide the calibration delay through the +command line option. + .SH PROGRAMMER-SPECIFIC INFORMATION Some programmer drivers accept further parameters to set programmer-specific parameters. These parameters are separated from the programmer name by a diff --git a/libflashrom.c b/libflashrom.c index ae2d33d..92e2527 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -51,7 +51,7 @@ { if (perform_selfcheck && selfcheck()) return 1; - myusec_calibrate_delay(); + myusec_calibrate_delay(0); return 0; } diff --git a/programmer.h b/programmer.h index c5cab18..e0925e2 100644 --- a/programmer.h +++ b/programmer.h @@ -280,9 +280,11 @@ /* udelay.c */ void myusec_delay(unsigned int usecs); -void myusec_calibrate_delay(void); +void myusec_calibrate_delay(int force); void internal_sleep(unsigned int usecs); void internal_delay(unsigned int usecs); +unsigned long get_calibration_value(void); +void set_external_calibration(unsigned long external_micro); #if CONFIG_INTERNAL == 1 /* board_enable.c */ diff --git a/udelay.c b/udelay.c index 6c0efc4..698f1ca 100644 --- a/udelay.c +++ b/udelay.c @@ -133,9 +133,9 @@ return timeusec; } -void myusec_calibrate_delay(void) +void myusec_calibrate_delay(int force) { - if (clock_check_res()) + if (clock_check_res() && !force) return; unsigned long count = 1000; @@ -221,6 +221,28 @@ msg_pinfo("OK.\n"); } +void set_external_calibration(unsigned long external_micro) +{ + micro = external_micro; + unsigned long elapsed = measure_delay(100000); + /* Do a sanity check if the provided parameter makes loops fall + * within 10% of the desired value. Otherwise, ignore provided value + * and recalibrate. + */ + if (elapsed > 90000 && elapsed < 110000) + msg_pinfo("Provided delay is acceptable!\n"); + else { + msg_pinfo("Elapsed: %lu, provided delay is NOT acceptable!\n", elapsed); + myusec_calibrate_delay(1); /* Ignore using the system clock for loop timing */ + } +} + +unsigned long get_calibration_value(void) +{ + myusec_calibrate_delay(1); /* Ignore using the system clock for loop timing */ + return micro; +} + /* Not very precise sleep. */ void internal_sleep(unsigned int usecs) { @@ -250,7 +272,7 @@ #else #include <libpayload.h> -void myusec_calibrate_delay(void) +void myusec_calibrate_delay(int force) { get_cpu_speed(); } -- To view, visit https://review.coreboot.org/c/flashrom/+/44517 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7c9a491c5cb5fb3af56a691a0c6af9b53dcff550 Gerrit-Change-Number: 44517 Gerrit-PatchSet: 1 Gerrit-Owner: Hrvoje Čavrak <github(a)hrvoje.org> Gerrit-MessageType: newchange

2 7

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2020