June 2025 - flashrom-gerrit - mail.coreboot.org

Change in ...flashrom[master]: Add support for security register and SBULK command on Macronix chips
by vl_sk (Code Review) Jan. 14, 2026

Jan. 14, 2026

vl_sk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/32034 Change subject: Add support for security register and SBULK command on Macronix chips ...................................................................... Add support for security register and SBULK command on Macronix chips Signed-off-by: Vladimir Skokov <vl_sk(a)mail.ru> Change-Id: I6b3d7bd2625dec5fda7b85f421d40c49bbc528a6 --- M chipdrivers.h M flashchips.c M spi.h M spi25_statusreg.c 4 files changed, 78 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/32034/1 diff --git a/chipdrivers.h b/chipdrivers.h index 134ed26..54288cd 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -102,7 +102,9 @@ int spi_prettyprint_status_register_sst25(struct flashctx *flash); int spi_prettyprint_status_register_sst25vf016(struct flashctx *flash); int spi_prettyprint_status_register_sst25vf040b(struct flashctx *flash); +int spi_prettyprint_security_register_mx25(struct flashctx *flash); int spi_disable_blockprotect_sst26_global_unprotect(struct flashctx *flash); +int spi_disable_blockprotect_mx25_global_unprotect(struct flashctx *flash); /* sfdp.c */ int probe_spi_sfdp(struct flashctx *flash); diff --git a/flashchips.c b/flashchips.c index d85ea74..2c662e9 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8555,9 +8555,8 @@ .block_erase = spi_block_erase_c7, } }, - /* TODO: security register and SBLK/SBULK; MX25L12835F: configuration register */ - .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ - .unlock = spi_disable_blockprotect_bp3_srwd, + .printlock = spi_prettyprint_security_register_mx25, + .unlock = spi_disable_blockprotect_mx25_global_unprotect, .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) supported */ .voltage = {2700, 3600}, @@ -8604,9 +8603,8 @@ .block_erase = spi_block_erase_c7, } }, - /* TODO: security register and SBLK/SBULK; MX25L12835F: configuration register */ - .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ - .unlock = spi_disable_blockprotect_bp3_srwd, + .printlock = spi_prettyprint_security_register_mx25, + .unlock = spi_disable_blockprotect_mx25_global_unprotect, .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) supported */ .voltage = {2700, 3600}, @@ -8693,9 +8691,8 @@ .block_erase = spi_block_erase_c7, } }, - /* TODO: security register */ - .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ - .unlock = spi_disable_blockprotect_bp3_srwd, + .printlock = spi_prettyprint_security_register_mx25, + .unlock = spi_disable_blockprotect_mx25_global_unprotect, .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {1650, 2000}, @@ -8734,9 +8731,8 @@ .block_erase = spi_block_erase_c7, } }, - /* TODO: security register */ - .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ - .unlock = spi_disable_blockprotect_bp3_srwd, + .printlock = spi_prettyprint_security_register_mx25, + .unlock = spi_disable_blockprotect_mx25_global_unprotect, .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {1650, 2000}, @@ -8776,9 +8772,8 @@ .block_erase = spi_block_erase_c7, } }, - /* TODO: security register */ - .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ - .unlock = spi_disable_blockprotect_bp3_srwd, + .printlock = spi_prettyprint_security_register_mx25, + .unlock = spi_disable_blockprotect_mx25_global_unprotect, .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {1650, 2000}, @@ -8818,9 +8813,8 @@ .block_erase = spi_block_erase_c7, } }, - /* TODO: security register */ - .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ - .unlock = spi_disable_blockprotect_bp3_srwd, + .printlock = spi_prettyprint_security_register_mx25, + .unlock = spi_disable_blockprotect_mx25_global_unprotect, .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {1650, 2000}, @@ -8858,9 +8852,8 @@ .block_erase = spi_block_erase_c7, } }, - /* TODO: security register */ - .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ - .unlock = spi_disable_blockprotect_bp3_srwd, + .printlock = spi_prettyprint_security_register_mx25, + .unlock = spi_disable_blockprotect_mx25_global_unprotect, .write = spi_chip_write_256, /* Multi I/O supported */ .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {1650, 2000}, @@ -8907,9 +8900,8 @@ .block_erase = spi_block_erase_c7, } }, - /* TODO: security register */ - .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit6 is quad enable */ - .unlock = spi_disable_blockprotect_bp3_srwd, + .printlock = spi_prettyprint_security_register_mx25, + .unlock = spi_disable_blockprotect_mx25_global_unprotect, .write = spi_chip_write_256, /* Multi I/O supported */ .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {1650, 2000}, diff --git a/spi.h b/spi.h index 422e6a0..456d894 100644 --- a/spi.h +++ b/spi.h @@ -170,6 +170,15 @@ From ANY mode (3-bytes or 4-bytes) it works with 4-byte address */ #define JEDEC_BYTE_PROGRAM_4BA 0x12 +/* Read Security Register */ +#define JEDEC_RDSCUR 0x2B +#define JEDEC_RDSCUR_OUTSIZE 0x01 +#define JEDEC_RDSCUR_INSIZE 0x01 + +/* Do GBULK (gang block unlock) */ +#define JEDEC_GBULK 0x98 +#define JEDEC_GBULK_OUTSIZE 0x01 + /* Error codes */ #define SPI_GENERIC_ERROR -1 #define SPI_INVALID_OPCODE -2 diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 4cf7023..4cdf800 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -237,6 +237,28 @@ return spi_disable_blockprotect_generic(flash, 0x7C, 1 << 7, 0, 0xFF); } +int spi_disable_blockprotect_mx25_global_unprotect(struct flashctx *flash) +{ + static const unsigned char cmd[JEDEC_GBULK_OUTSIZE] = { JEDEC_GBULK }; + int ret; + + ret = spi_write_enable(flash); + if (ret) { + msg_cerr("Set WREN failed!\n"); + return -1; + } + + /* Send GBULK cmd to device. */ + ret = spi_send_command(flash, sizeof(cmd), 0, cmd, NULL); + if (ret) { + msg_cerr("GBULK failed!\n"); + return -1; + } + + /* Disable standard blockprotect */ + return spi_disable_blockprotect_bp3_srwd(flash); +} + static void spi_prettyprint_status_register_hex(uint8_t status) { msg_cdbg("Chip status register is 0x%02x.\n", status); @@ -751,3 +773,32 @@ msg_cdbg("Resulting block protection : %s\n", bpt[(status & 0x1c) >> 2]); return 0; } + +/* === Macronix chips with Security Register === */ + +int spi_read_security_register(struct flashctx *flash) +{ + static const unsigned char cmd[JEDEC_RDSCUR_OUTSIZE] = { JEDEC_RDSCUR }; + unsigned char readarr[JEDEC_RDSCUR_INSIZE]; + int ret; + + ret = spi_send_command(flash, sizeof(cmd), sizeof(readarr), cmd, readarr); + if (ret) { + msg_cerr("RDSCUR failed!\n"); + return 0; + } + + return readarr[0]; +} + +int spi_prettyprint_security_register_mx25(struct flashctx *flash) +{ + int security_register = spi_read_security_register(flash); + int wpsel = !!(security_register & (1 << 7)); + + msg_cdbg("Chip security register: 0x%02X\n", security_register); + msg_cdbg("\tWPSEL is %s\n", wpsel ? "ON" : "OFF"); + + /* Print standard blockprotect info */ + return spi_prettyprint_status_register_bp3_srwd(flash); +} \ No newline at end of file -- To view, visit https://review.coreboot.org/c/flashrom/+/32034 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6b3d7bd2625dec5fda7b85f421d40c49bbc528a6 Gerrit-Change-Number: 32034 Gerrit-PatchSet: 1 Gerrit-Owner: vl_sk Gerrit-MessageType: newchange

4 6

Change in flashrom[master]: pcidev: On arm64 we are not getting a valid BAR0 address
by Martijn Berger (Code Review) Sept. 13, 2025

Sept. 13, 2025

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

5 6

Change in flashrom[master]: cli_classic.c: Add runtime measurement
by Simon Buhrow (Code Review) Aug. 20, 2025

Aug. 20, 2025

Simon Buhrow has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/47805 ) Change subject: cli_classic.c: Add runtime measurement ...................................................................... cli_classic.c: Add runtime measurement To get an fast and easy feedback on how parameters effect runtime (e.g. clock setting, polling delays, ...). Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Change-Id: I2238b3f3e6c2ab7745994662a88787fa2e86d480 --- M cli_classic.c 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/47805/1 diff --git a/cli_classic.c b/cli_classic.c index ae7f6ef..0d5381f 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -24,6 +24,7 @@ #include <string.h> #include <stdlib.h> #include <getopt.h> +#include <time.h> #include "flash.h" #include "flashchips.h" #include "fmap.h" @@ -142,6 +143,7 @@ /* Probe for up to eight flash chips. */ struct flashctx flashes[8] = {{0}}; struct flashctx *fill_flash; + struct timespec time_start, time_end; const char *name; int namelen, opt, i, j; int startchip = -1, chipcount = 0, option_index = 0, force = 0, ifd = 0, fmap = 0; @@ -528,6 +530,8 @@ /* FIXME: Delay calibration should happen in programmer code. */ myusec_calibrate_delay(); + timespec_get(&time_start, TIME_UTC); + if (programmer_init(prog, pparam)) { msg_perr("Error: Programmer initialization failed.\n"); ret = 1; @@ -806,6 +810,9 @@ for (i = 0; i < chipcount; i++) free(flashes[i].chip); + timespec_get(&time_end, TIME_UTC); + msg_gdbg("Runtime: %d:%2d min:s\n", (int)((time_end.tv_sec-time_start.tv_sec) / 60), (int)(time_end.tv_sec-time_start.tv_sec) % 60); + layout_cleanup(&include_args); free(filename); free(fmapfile); -- To view, visit https://review.coreboot.org/c/flashrom/+/47805 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2238b3f3e6c2ab7745994662a88787fa2e86d480 Gerrit-Change-Number: 47805 Gerrit-PatchSet: 1 Gerrit-Owner: Simon Buhrow Gerrit-MessageType: newchange

5 11

[XS] Change in flashrom[main]: 82802ab,jedec: Unify use of "shifted"
by Stefan Reinauer (Code Review) July 1, 2025

July 1, 2025

Stefan Reinauer has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/88254?usp=email ) Change subject: 82802ab,jedec: Unify use of "shifted" ...................................................................... 82802ab,jedec: Unify use of "shifted" There are three different ways shifted is defined in the code. This patch unifies to const bool shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED); Change-Id: I4f6cb5f434eabd635a23237a2968ea9d1a649c44 Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> --- M 82802ab.c M jedec.c 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/88254/1 diff --git a/82802ab.c b/82802ab.c index 148042c..9d95c13 100644 --- a/82802ab.c +++ b/82802ab.c @@ -41,7 +41,7 @@ { chipaddr bios = flash->virtual_memory; uint8_t id1, id2, flashcontent1, flashcontent2; - int shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED) ? 1 : 0; + const bool shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED); /* Reset to get a clean state */ chip_writeb(flash, 0xFF, bios); diff --git a/jedec.c b/jedec.c index df0d93a..218ce88 100644 --- a/jedec.c +++ b/jedec.c @@ -285,7 +285,7 @@ static void issuecmd(const struct flashctx *flash, uint8_t op, unsigned int operand) { const chipaddr bios = flash->virtual_memory; - bool shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED); + const bool shifted = (flash->chip->feature_bits & FEATURE_ADDR_SHIFTED); const unsigned int mask = getaddrmask(flash->chip); unsigned int delay_us = (flash->chip->probe_timing == TIMING_ZERO) ? 0 : 10; -- To view, visit https://review.coreboot.org/c/flashrom/+/88254?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4f6cb5f434eabd635a23237a2968ea9d1a649c44 Gerrit-Change-Number: 88254 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org>

1 0

[M] Change in flashrom[main]: probe_flash: Introduce an error code for "other" probing errors
by Stefan Reinauer (Code Review) June 30, 2025

June 30, 2025

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Stefan Reinauer has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/88008?usp=email ) Change subject: probe_flash: Introduce an error code for "other" probing errors ...................................................................... Patch Set 11: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/88008?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4a271550bea2b36c657c71ce6cb1927082663c3c Gerrit-Change-Number: 88008 Gerrit-PatchSet: 11 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 30 Jun 2025 13:10:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in flashrom[main]: probe_flash: Introduce an error code for "other" probing errors
by Anastasia Klimchuk (Code Review) June 30, 2025

June 30, 2025

Attention is currently required from: Peter Marheine, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/88008?usp=email ) Change subject: probe_flash: Introduce an error code for "other" probing errors ...................................................................... Patch Set 11: (1 comment) Patchset: PS11: I merged CB:88202 and this one needed a rebase after that. I removed the test for v1 since it's now deprecated and usage of it gives a compile error (WAI) Could my dear reviewers approve this patch once again, thank you so much! -- To view, visit https://review.coreboot.org/c/flashrom/+/88008?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4a271550bea2b36c657c71ce6cb1927082663c3c Gerrit-Change-Number: 88008 Gerrit-PatchSet: 11 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 30 Jun 2025 09:10:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in flashrom[main]: probe_flash: Introduce an error code for "other" probing errors
by Anastasia Klimchuk (Code Review) June 30, 2025

June 30, 2025

Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Stefan Reinauer. Hello Peter Marheine, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/88008?usp=email to look at the new patch set (#11). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: probe_flash: Introduce an error code for "other" probing errors ...................................................................... probe_flash: Introduce an error code for "other" probing errors Previously probe_flash had the same return code for the case when no chips were matched, and when some other error happened during probing. However these are two different scenarios and it is useful for the caller to distinguish between them. In fact, the caller (libflashrom it is) wanted to distinguish between "no chips found" and "some other probing error" from the very beginning. libflashrom probe API documented returning special error code for "other error". However it was not possible to know when "other error" happened because probe_flash never returned that back, it could only say "no matched chips found". This patch introduces -2 as "other error" code from probe_flash, while -1 remains as "no chips found". Both libflashrom probe APIs v1 and v2 are now handling "other error" from probe_flash and return it to the API callers as was promised in the documentation. This also adds a unit test for error code propagation for "no chips found" error. Change-Id: I4a271550bea2b36c657c71ce6cb1927082663c3c Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M cli_classic.c M flashrom.c M include/flash.h M include/libflashrom.h M libflashrom.c M tests/libflashrom.c M tests/tests.c M tests/tests.h 8 files changed, 98 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/88008/11 -- To view, visit https://review.coreboot.org/c/flashrom/+/88008?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4a271550bea2b36c657c71ce6cb1927082663c3c Gerrit-Change-Number: 88008 Gerrit-PatchSet: 11 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>

1 0

[M] Change in flashrom[main]: probe_flash: Introduce an error code for "other" probing errors
by Anastasia Klimchuk (Code Review) June 30, 2025

June 30, 2025

Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Stefan Reinauer. Hello Peter Marheine, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/88008?usp=email to look at the new patch set (#10). The following approvals got outdated and were removed: Code-Review+2 by Peter Marheine, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: probe_flash: Introduce an error code for "other" probing errors ...................................................................... probe_flash: Introduce an error code for "other" probing errors Previously probe_flash had the same return code for the case when no chips were matched, and when some other error happened during probing. However these are two different scenarios and it is useful for the caller to distinguish between them. In fact, the caller (libflashrom it is) wanted to distinguish between "no chips found" and "some other probing error" from the very beginning. libflashrom probe API documented returning special error code for "other error". However it was not possible to know when "other error" happened because probe_flash never returned that back, it could only say "no matched chips found". This patch introduces -2 as "other error" code from probe_flash, while -1 remains as "no chips found". Both libflashrom probe APIs v1 and v2 are now handling "other error" from probe_flash and return it to the API callers as was promised in the documentation. This also adds a unit test for error code propagation for "no chips found" error. Change-Id: I4a271550bea2b36c657c71ce6cb1927082663c3c Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M cli_classic.c M flashrom.c M include/flash.h M include/libflashrom.h M libflashrom.c M tests/libflashrom.c M tests/tests.c M tests/tests.h 8 files changed, 99 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/88008/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/88008?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4a271550bea2b36c657c71ce6cb1927082663c3c Gerrit-Change-Number: 88008 Gerrit-PatchSet: 10 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Deprecate probing v1 API
by Anastasia Klimchuk (Code Review) June 30, 2025

June 30, 2025

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/88202?usp=email ) Change subject: libflashrom: Deprecate probing v1 API ...................................................................... libflashrom: Deprecate probing v1 API flashrom_flash_probe marked as deprecated and existing tests are updated to use probing v2 API Change-Id: I88f78ac0c93ce99a555b42f87aa0a695089e0b3f Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/88202 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M doc/release_notes/devel.rst M include/libflashrom.h M tests/ch341a_spi.c M tests/dummyflasher.c M tests/lifecycle.c M tests/lifecycle.h M tests/linux_mtd.c M tests/linux_spi.c M tests/spidriver.c 9 files changed, 33 insertions(+), 41 deletions(-) Approvals: Peter Marheine: Looks good to me, approved build bot (Jenkins): Verified diff --git a/doc/release_notes/devel.rst b/doc/release_notes/devel.rst index ef8364d..12d66a1 100644 --- a/doc/release_notes/devel.rst +++ b/doc/release_notes/devel.rst @@ -96,8 +96,17 @@ ``flashrom_flash_probe_v2`` continues to support an optional parameter ``chip_name`` if the caller want to probe for only one specific chip with given name. -Old API ``flashrom_flash_probe`` which stops probing if more than one chip entry matches -continues to work as before. +Command line interface is now using ``flashrom_flash_probe_v2``. + +Old API ``flashrom_flash_probe`` is deprecated. It stays available "as is" for now, +however will be removed at some point in future. Users should switch to v2. + +The main reason for deprecation is that it was returning incomplete information in +case when multiple matching chips were found during probing. Specifically, the only +info returned was error code of "multiple chips match" with no details of which are +the matches and how many. This left the caller unable to proceed further. Consequently, +even flashrom's own command line interface was unable to use old libflashrom probing API, +and had to maintain separate logic for probing. New API to get list of supported programmers -------------------------------------------- diff --git a/include/libflashrom.h b/include/libflashrom.h index ca9a5e3..0b3a5b5 100644 --- a/include/libflashrom.h +++ b/include/libflashrom.h @@ -261,6 +261,8 @@ */ /** + * @deprecated Use flashrom_flash_probe_v2 instead + * * @brief Probe for a flash chip. * * Probes for a flash chip and returns a flash context, that can be used @@ -278,7 +280,8 @@ * 2 if no chip was found, * or 1 on any other error. */ -int flashrom_flash_probe(struct flashrom_flashctx **flashctx, const struct flashrom_programmer *flashprog, const char *chip_name); +int flashrom_flash_probe(struct flashrom_flashctx **flashctx, const struct flashrom_programmer *flashprog, const char *chip_name) +__attribute__((deprecated("Use flashrom_flash_probe_v2 instead"))); /** * @brief Probe for a flash chip, v2 diff --git a/tests/ch341a_spi.c b/tests/ch341a_spi.c index 50b3d11..13fa1a2 100644 --- a/tests/ch341a_spi.c +++ b/tests/ch341a_spi.c @@ -129,7 +129,9 @@ .fallback_open_state = &ch341a_spi_fallback_open_state, }; - run_probe_lifecycle(state, &ch341a_spi_io, &programmer_ch341a_spi, "", "W25Q128.V"); + const char *expected_matched_names[1] = {"W25Q128.V"}; + run_probe_v2_lifecycle(state, &ch341a_spi_io, &programmer_ch341a_spi, "", "W25Q128.V", + expected_matched_names, 1); } #else diff --git a/tests/dummyflasher.c b/tests/dummyflasher.c index 0faecb9..7b98f61 100644 --- a/tests/dummyflasher.c +++ b/tests/dummyflasher.c @@ -39,7 +39,9 @@ .fallback_open_state = &dummy_fallback_open_state, }; - run_probe_lifecycle(state, &dummy_io, &programmer_dummy, "bus=spi,emulate=W25Q128FV", "W25Q128.V"); + const char *expected_matched_names[1] = {"W25Q128.V"}; + run_probe_v2_lifecycle(state, &dummy_io, &programmer_dummy, "bus=spi,emulate=W25Q128FV", "W25Q128.V", + expected_matched_names, 1); } void dummy_probe_v2_one_match_for_W25Q128FV(void **state) @@ -103,7 +105,9 @@ .fallback_open_state = &dummy_fallback_open_state, }; - run_probe_lifecycle(state, &dummy_io, &programmer_dummy, "size=8388608,emulate=VARIABLE_SIZE", "Opaque flash chip"); + const char *expected_matched_names[1] = {"Opaque flash chip"}; + run_probe_v2_lifecycle(state, &dummy_io, &programmer_dummy, "size=8388608,emulate=VARIABLE_SIZE", "Opaque flash chip", + expected_matched_names, 1); } void dummy_init_fails_unhandled_param_test_success(void **state) diff --git a/tests/lifecycle.c b/tests/lifecycle.c index a92e93e..555be78 100644 --- a/tests/lifecycle.c +++ b/tests/lifecycle.c @@ -15,25 +15,6 @@ #include "lifecycle.h" -static void probe_chip(const struct programmer_entry *prog, - struct flashrom_programmer *flashprog, - const char *const chip_name, - const char **expected_matched_names, /* unused in probe v1 */ - unsigned int expected_matched_count /* unused in probe v1 */) -{ - struct flashrom_flashctx *flashctx; - - printf("Testing flashrom_flash_probe for programmer=%s, chip=%s ... \n", prog->name, chip_name); - - assert_int_equal(0, flashrom_flash_probe(&flashctx, flashprog, chip_name)); - if (chip_name) - assert_int_equal(0, strcmp(chip_name, flashctx->chip->name)); - - printf("... flashrom_flash_probe for programmer=%s successful\n", prog->name); - - flashrom_flash_release(flashctx); /* cleanup */ -} - static void probe_chip_v2(const struct programmer_entry *prog, struct flashrom_programmer *flashprog, const char *const chip_name, @@ -103,16 +84,6 @@ NULL /* action */); } -void run_probe_lifecycle(void **state, const struct io_mock *io, - const struct programmer_entry *prog, const char *param, const char *const chip_name) -{ - /* Each probe lifecycle should run independently, without cache. */ - clear_spi_id_cache(); - run_lifecycle(state, io, prog, param, chip_name, - NULL /* expected_matched_names, */, 0 /* expected_matched_count, */, - &probe_chip); -} - void run_probe_v2_lifecycle(void **state, const struct io_mock *io, const struct programmer_entry *prog, const char *param, const char *const chip_name, diff --git a/tests/lifecycle.h b/tests/lifecycle.h index 1565a69..d0ba179 100644 --- a/tests/lifecycle.h +++ b/tests/lifecycle.h @@ -31,9 +31,6 @@ void run_basic_lifecycle(void **state, const struct io_mock *io, const struct programmer_entry *prog, const char *param); -void run_probe_lifecycle(void **state, const struct io_mock *io, - const struct programmer_entry *prog, const char *param, const char *const chip_name); - void run_probe_v2_lifecycle(void **state, const struct io_mock *io, const struct programmer_entry *prog, const char *param, const char *const chip_name, diff --git a/tests/linux_mtd.c b/tests/linux_mtd.c index 0cee785..bab6ee6 100644 --- a/tests/linux_mtd.c +++ b/tests/linux_mtd.c @@ -89,7 +89,9 @@ .fallback_open_state = &linux_mtd_fallback_open_state, }; - run_probe_lifecycle(state, &linux_mtd_io, &programmer_linux_mtd, "", "Opaque flash chip"); + const char *expected_matched_names[1] = {"Opaque flash chip"}; + run_probe_v2_lifecycle(state, &linux_mtd_io, &programmer_linux_mtd, "", "Opaque flash chip", + expected_matched_names, 1); } #else SKIP_TEST(linux_mtd_probe_lifecycle_test_success) diff --git a/tests/linux_spi.c b/tests/linux_spi.c index 1b653fe..d2d1414 100644 --- a/tests/linux_spi.c +++ b/tests/linux_spi.c @@ -65,7 +65,9 @@ .fallback_open_state = &linux_spi_fallback_open_state, }; - run_probe_lifecycle(state, &linux_spi_io, &programmer_linux_spi, "dev=/dev/null", "W25Q128.V"); + const char *expected_matched_names[1] = {"W25Q128.V"}; + run_probe_v2_lifecycle(state, &linux_spi_io, &programmer_linux_spi, "dev=/dev/null", "W25Q128.V", + expected_matched_names, 1); } #else SKIP_TEST(linux_spi_probe_lifecycle_test_success) diff --git a/tests/spidriver.c b/tests/spidriver.c index 597895a..4b5e9cc 100644 --- a/tests/spidriver.c +++ b/tests/spidriver.c @@ -212,7 +212,9 @@ .fallback_open_state = &spidriver_fallback_open_state, }; - run_probe_lifecycle(state, &spidriver_io, &programmer_spidriver, "dev=/dev/null", "W25Q128.V"); + const char *expected_matched_names[1] = {"W25Q128.V"}; + run_probe_v2_lifecycle(state, &spidriver_io, &programmer_spidriver, "dev=/dev/null", "W25Q128.V", + expected_matched_names, 1); } #else SKIP_TEST(spidriver_probe_lifecycle_test_success) -- To view, visit https://review.coreboot.org/c/flashrom/+/88202?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I88f78ac0c93ce99a555b42f87aa0a695089e0b3f Gerrit-Change-Number: 88202 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in flashrom[main]: cli_classic.c: Reimplement parse_wp_range
by Anastasia Klimchuk (Code Review) June 29, 2025

June 29, 2025

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/88162?usp=email ) Change subject: cli_classic.c: Reimplement parse_wp_range ...................................................................... cli_classic.c: Reimplement parse_wp_range Current method modifies the optarg string and causes it to not get printed in future debug log. For example writing the log to a file with -o will show "Command line" without the size parameter of the range. Tested by logging the output and reading the log: ./flashrom -p linux_spi:dev=/dev/spidev0.0 -c "W25Q64JV-.Q" \ --wp-range 0x0,0x00001000 -o logfile && grep 'Command line' logfile Change-Id: I77acd49a5fa17a0af69b4fb1371a131a5249d3dc Signed-off-by: Nikola Z. Ivanov <zlatistiv(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/88162 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M cli_classic.c 1 file changed, 8 insertions(+), 11 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified diff --git a/cli_classic.c b/cli_classic.c index 8b6474b..870f8da 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -219,26 +219,23 @@ static int parse_wp_range(unsigned int *start, unsigned int *len) { - char *endptr = NULL, *token = NULL; + char *delim = NULL; if (!optarg) { msg_gerr("Error: No wp-range values provided\n"); return -1; } - token = strtok(optarg, ","); - if (!token) { - msg_gerr("Error: Invalid wp-range argument format\n"); + if ((delim = strchr(optarg, ',')) != strrchr(optarg, ',') || + delim == NULL || + delim == optarg || + *(delim + 1) == '\0') { + msg_gerr("Error: Invalid wp-range argument format. Valid format is --wp-range <start>,<end>\n"); return -1; } - *start = strtoul(token, &endptr, 0); - token = strtok(NULL, ","); - if (!token) { - msg_gerr("Error: Invalid wp-range argument format\n"); - return -1; - } - *len = strtoul(token, &endptr, 0); + *start = strtoul(optarg, NULL, 0); + *len = strtoul(delim+1, NULL, 0); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/88162?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I77acd49a5fa17a0af69b4fb1371a131a5249d3dc Gerrit-Change-Number: 88162 Gerrit-PatchSet: 4 Gerrit-Owner: Nikola <zlatistiv(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0