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

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]: flashchips: Mark W25Q256JV_Q as tested for read/write/erase
by Anastasia Klimchuk (Code Review) April 30, 2025

April 30, 2025

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/87459?usp=email ) Change subject: flashchips: Mark W25Q256JV_Q as tested for read/write/erase ...................................................................... flashchips: Mark W25Q256JV_Q as tested for read/write/erase As reported on the mailing list: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/Y27H… Change-Id: I267ee7e86c682626ead2310b920a0e5026982312 Tested-by: Attila Veghelyi <aveghelyi(a)dension.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/87459 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Reviewed-by: attila-v --- M flashchips/winbond.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved attila-v: Looks good to me, but someone else must approve diff --git a/flashchips/winbond.c b/flashchips/winbond.c index ae80b13..957c4b0 100644 --- a/flashchips/winbond.c +++ b/flashchips/winbond.c @@ -546,7 +546,7 @@ /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA | FEATURE_WRSR2, - .tested = TEST_UNTESTED, + .tested = TEST_OK_PREW, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = -- To view, visit https://review.coreboot.org/c/flashrom/+/87459?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: I267ee7e86c682626ead2310b920a0e5026982312 Gerrit-Change-Number: 87459 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in flashrom[main]: Add more delay time for jlink power on feature to stabilize the LDO a...
by attila-v (Code Review) April 29, 2025

April 29, 2025

Attention is currently required from: Anastasia Klimchuk. attila-v has posted comments on this change by attila-v. ( https://review.coreboot.org/c/flashrom/+/86085?usp=email ) Change subject: Add more delay time for jlink power on feature to stabilize the LDO and the decoupling capacitors ...................................................................... Patch Set 2: (1 comment) File jlink_spi.c: https://review.coreboot.org/c/flashrom/+/86085/comment/e77c15d1_246da726?us… : PS2, Line 469: internal_sleep(100000); > I have a question: is this longer delay needed unconditionally (always, for any environment), or onl […] I think it is a safety time for any other LDO-s and decoupling capacitors too. I used only one type (HT7333) and it was definitely needed. -- To view, visit https://review.coreboot.org/c/flashrom/+/86085?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: Ic2dd94e99ac6ffa17a009b8488ce027698ae2c28 Gerrit-Change-Number: 86085 Gerrit-PatchSet: 2 Gerrit-Owner: attila-v Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 29 Apr 2025 16:57:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Add probing v2 which can find all mathching chips
by Anastasia Klimchuk (Code Review) April 28, 2025

April 28, 2025

Attention is currently required from: Dmitry Zhadinets, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email ) Change subject: libflashrom: Add probing v2 which can find all mathching chips ...................................................................... Patch Set 4: (3 comments) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/87341/comment/1f89c545_a671a4e3?us… : PS3, Line 1216: if (ret == 1) { > A comment here is that: probing never returns 1, and v1 probing never returned 1 either. […] I mark this unresolved, because otherwise it is not shown in the latest diffs :( but mostly this is to ask what people think about it. Even if there will be changes to probe_flash, that's not for this commit. File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/87341/comment/4646ffed_6b038800?us… : PS4, Line 312: const char **all_matched_names, > I see from the implementation that the caller is expected to allocate this buffer, but that's not cl […] I initially tried p.2 too! :) but from my memory I got stuck because chip name is const and it did not allow copying into non-const array. And I couldn't pass const array into function if I were to allocate memory for it... it was something like that. The other APIs return the allocated array rather than taking it as param. I can try again, in case I missed something. But possibly that p.2 needs to return array of names (I like more the idea to return the number of matches. as you suggested in other comment) https://review.coreboot.org/c/flashrom/+/87341/comment/cae28c49_4a4384f1?us… : PS4, Line 313: unsigned int *all_matched_count, > I'd say it's more conventional to use negative values for error and positive values for success when […] That's an interesting idea! I also like to have one less parameter. The only thing to double-check: my initial version was keeping the same convention for return codes as it was in probing v1. This idea will change it (especially that 0 no longer indicates success). So you think it's fine? -- To view, visit https://review.coreboot.org/c/flashrom/+/87341?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: Idfcf377a8071e22028ba98515f08495ed2a6e9f0 Gerrit-Change-Number: 87341 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Apr 2025 09:48:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[XS] Change in flashrom[main]: flashchips: Mark W25Q256JV_Q as tested for read/write/erase
by attila-v (Code Review) April 28, 2025

April 28, 2025

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev. attila-v has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87459?usp=email ) Change subject: flashchips: Mark W25Q256JV_Q as tested for read/write/erase ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/87459?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: I267ee7e86c682626ead2310b920a0e5026982312 Gerrit-Change-Number: 87459 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 28 Apr 2025 07:50:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in flashrom[main]: libflashrom: Add probing v2 which can find all mathching chips
by Peter Marheine (Code Review) April 28, 2025

April 28, 2025

Attention is currently required from: Anastasia Klimchuk, Dmitry Zhadinets. Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email ) Change subject: libflashrom: Add probing v2 which can find all mathching chips ...................................................................... Patch Set 4: (2 comments) File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/87341/comment/7c246aab_9dd4cb2c?us… : PS4, Line 312: const char **all_matched_names, I see from the implementation that the caller is expected to allocate this buffer, but that's not clear from the documentation here and there's currently no guarantee that the buffer is large enough for all the detected chips. Either: 1. add a `size_t` parameter so the caller can indicate how large the buffer is and make the implementation avoid writing more than that many names (while still counting matches if more are found) 2. dynamically allocate the buffer so the caller doesn't need to allocate it and document that it needs to be freed with `flashrom_data_free`. 3. Instead take a callback which the name of matched chips get passed to, allowing the caller to buffer names (or not) as desired (kind of an iterator-based solution). I'm more inclined toward the second option, since a few allocations shouldn't matter and then a caller never needs to probe multiple times if they discover their buffer was too small. https://review.coreboot.org/c/flashrom/+/87341/comment/959a938c_66a60e55?us… : PS4, Line 313: unsigned int *all_matched_count, I'd say it's more conventional to use negative values for error and positive values for success when the logical return value is an int, so you could eliminate this parameter and redefine the return value to be the number of matched chips (if zero of more) or a negative error code. -- To view, visit https://review.coreboot.org/c/flashrom/+/87341?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: Idfcf377a8071e22028ba98515f08495ed2a6e9f0 Gerrit-Change-Number: 87341 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Apr 2025 00:10:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: cli: Use flashrom_supported_programmers API in cli
by Peter Marheine (Code Review) April 27, 2025

April 27, 2025

Attention is currently required from: Dmitry Zhadinets. Peter Marheine has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/87195?usp=email ) Change subject: cli: Use flashrom_supported_programmers API in cli ...................................................................... Patch Set 2: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/87195/comment/257270a7_79893248?us… : PS2, Line 1562: return; This error path should probably print a message, since silently doing nothing could be confusing. -- To view, visit https://review.coreboot.org/c/flashrom/+/87195?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: I86ac83ee0ef4e850e69aa4e7b607e28ebab9d3d5 Gerrit-Change-Number: 87195 Gerrit-PatchSet: 2 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> 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> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Comment-Date: Sun, 27 Apr 2025 23:46:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in flashrom[main]: flashchips: Mark W25Q256JV_Q as tested for read/write/erase
by Stefan Reinauer (Code Review) April 26, 2025

April 26, 2025

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, attila-v. Stefan Reinauer has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87459?usp=email ) Change subject: flashchips: Mark W25Q256JV_Q as tested for read/write/erase ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/87459?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: I267ee7e86c682626ead2310b920a0e5026982312 Gerrit-Change-Number: 87459 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: attila-v Gerrit-Comment-Date: Sat, 26 Apr 2025 18:25:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: flashchips: Mark W25Q256JV_Q as tested for read/write/erase
by Anastasia Klimchuk (Code Review) April 26, 2025

April 26, 2025

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/87459?usp=email ) Change subject: flashchips: Mark W25Q256JV_Q as tested for read/write/erase ...................................................................... flashchips: Mark W25Q256JV_Q as tested for read/write/erase As reported on the mailing list: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/Y27H… Change-Id: I267ee7e86c682626ead2310b920a0e5026982312 Tested-by: Attila Veghelyi <aveghelyi(a)dension.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M flashchips/winbond.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/87459/1 diff --git a/flashchips/winbond.c b/flashchips/winbond.c index ae80b13..957c4b0 100644 --- a/flashchips/winbond.c +++ b/flashchips/winbond.c @@ -546,7 +546,7 @@ /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA | FEATURE_WRSR2, - .tested = TEST_UNTESTED, + .tested = TEST_OK_PREW, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = -- To view, visit https://review.coreboot.org/c/flashrom/+/87459?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: I267ee7e86c682626ead2310b920a0e5026982312 Gerrit-Change-Number: 87459 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>

1 0