May 2024 - flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[main]: flashchips: Add support for GigaDevice GD25LR256E, GD251R512ME
by Anastasia Klimchuk (Code Review) 11 May '24

11 May '24

Attention is currently required from: Angel Pons, Martin L Roth, Martin Roth, Peter Marheine, Raj Astekar, Ravishankar Sarawadi, Wonkyu Kim. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58025?usp=email ) Change subject: flashchips: Add support for GigaDevice GD25LR256E, GD251R512ME ...................................................................... Patch Set 8: (1 comment) Patchset: PS8: > Does this change need anything else? There was interest in support for this chip on the mailing list […] I read through the comments thread to recall what the patch have been waiting for. The reason why it is hanging is, there are no datasheets available, so there is no way to check the chip definitions by datasheets (which is normal process). From the earlier comments, it seems the datasheets were publicly available in 2021 when the patch was pushed for review. But today not available anymore :( I can believe the chip is working, there are few people in comment thread who tested it successfully. As a first step, let's try to get datasheets, I will post to the thread on the mailing list. Also in any case, patch will need to be rebased, it's base on almost a year ago code. -- To view, visit https://review.coreboot.org/c/flashrom/+/58025?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I2fe6bc1219cd1ee19b93caabab69de938cfc44b0 Gerrit-Change-Number: 58025 Gerrit-PatchSet: 8 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Sat, 11 May 2024 09:35:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: MAINTAINERS: add Peter Marheine for build system
by Anastasia Klimchuk (Code Review) 11 May '24

11 May '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82238?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: MAINTAINERS: add Peter Marheine for build system ...................................................................... MAINTAINERS: add Peter Marheine for build system Change-Id: Ibae0c006b293dad85a9571ec8e7081a6396bc7ce Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82238 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M MAINTAINERS 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified diff --git a/MAINTAINERS b/MAINTAINERS index 97b6038..f4e0c9a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -78,6 +78,7 @@ BUILD SYSTEM M: Thomas Heijligen <src(a)posteo.de> +M: Peter Marheine <pmarheine(a)chromium.org> S: Maintained F: Makefile* F: meson* -- To view, visit https://review.coreboot.org/c/flashrom/+/82238?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibae0c006b293dad85a9571ec8e7081a6396bc7ce Gerrit-Change-Number: 82238 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: Add documentation for pico-serprog
by J. Neuschäfer (Code Review) 10 May '24

10 May '24

Attention is currently required from: Riku Viitanen, Sydney, Thomas Heijligen. J. Neuschäfer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82229?usp=email ) Change subject: Add documentation for pico-serprog ...................................................................... Patch Set 3: (4 comments) Patchset: PS3: Some comments on wording File doc/supported_hw/supported_prog/serprog/overview.rst: https://review.coreboot.org/c/flashrom/+/82229/comment/98f6d4c2_c15fc6de : PS2, Line 95: etter performance > I am not aware of any exiting benchmarks. […] I did the performance improvements and I did some very rudimentary benchmarking. It's in this commit message: https://github.com/stacksmashing/pico-serprog/commit/ed2520ccb39965deb20d6b… Total payload throughput gets to about 5 Mbit/s, and I think I'm limited by USB (12 Mbit/s nominal for Full Speed, and USB protocol overhead that I don't fully understand). I haven't benchmarked your implementation. File doc/supported_hw/supported_prog/serprog/overview.rst: https://review.coreboot.org/c/flashrom/+/82229/comment/2d6095bc_5d967f5c : PS3, Line 94: over PIO "over" can be misunderstood. "instead of" is less ambiguous https://review.coreboot.org/c/flashrom/+/82229/comment/cacc5838_eee2ce2d : PS3, Line 95: pinouts but implements USB descriptors allowing custom udev-rules. : Nothing wrong with what this _says_, but what it _implies_ is a bit confusing. Because these two things are in the same sentence, it seems to imply that arbitrary pinouts are sacrified for gaining custom USB descriptors, but really these are two unrelated features, AFAIUI: 1. Hardware SPI is used, therefore the pins can't be moved around arbitrarily 2. Tinyusb used used directly instead of going through the pico-stdio library's USB code. That is what allows having custom (libreboot/pico-serprog) instead of generic (raspberry pi pico) descriptors. The USB device has descriptors in any case. -- To view, visit https://review.coreboot.org/c/flashrom/+/82229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I457dfec52f89997f64b6c276c50b329359d61b77 Gerrit-Change-Number: 82229 Gerrit-PatchSet: 3 Gerrit-Owner: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: J. Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Riku Viitanen <riku.viitanen(a)protonmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Riku Viitanen <riku.viitanen(a)protonmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Sydney <git(a)funkeleinhorn.com> Gerrit-Comment-Date: Fri, 10 May 2024 14:38:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Riku Viitanen <riku.viitanen(a)protonmail.com> Comment-In-Reply-To: Sydney <git(a)funkeleinhorn.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: Add documentation for pico-serprog
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: J. Neuschäfer, Riku Viitanen, Sydney, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82229?usp=email ) Change subject: Add documentation for pico-serprog ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS3: Thank you for the patch! I approve but I also wanted to check with other reviewers that they approve. Please give your +1s as an indication that you agree that your comments have been resolved. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/82229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I457dfec52f89997f64b6c276c50b329359d61b77 Gerrit-Change-Number: 82229 Gerrit-PatchSet: 3 Gerrit-Owner: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: J. Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Riku Viitanen <riku.viitanen(a)protonmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Riku Viitanen <riku.viitanen(a)protonmail.com> Gerrit-Attention: J. Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Sydney <git(a)funkeleinhorn.com> Gerrit-Comment-Date: Fri, 10 May 2024 13:06:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: doc: Fix index files in Supported HW section
by Sydney (Code Review) 10 May '24

10 May '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Thomas Heijligen. Sydney has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82196?usp=email ) Change subject: doc: Fix index files in Supported HW section ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/82196?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3 Gerrit-Change-Number: 82196 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 10 May 2024 11:27:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: doc: Add doc for supported flash chips
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Alexander Goncharov, Peter Marheine, Thomas Heijligen, Victor Lim. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82197?usp=email ) Change subject: doc: Add doc for supported flash chips ...................................................................... Patch Set 2: (1 comment) File doc/supported_hw/supported_flashchips.rst: https://review.coreboot.org/c/flashrom/+/82197/comment/e204da35_4be95fc6 : PS1, Line 9: If you want to check whether a flash chip is supported in the given release, you can rebase your local > `flashrom -L` also prints out all supported chips, which is probably more accessible to users who al […] I completely forgot about it, but it's super useful. Not only in this patch, but in all the other patches in the chain. And I have discovered there is one more branch of code which prints everything! print_wiki definitely can retire with a peace of mind. -- To view, visit https://review.coreboot.org/c/flashrom/+/82197?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I05fb60a4caf2cfb30586fa482687b10638996395 Gerrit-Change-Number: 82197 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 10 May 2024 10:44:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: doc: Fix index files in Supported HW section
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Peter Marheine, Sydney, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82196?usp=email ) Change subject: doc: Fix index files in Supported HW section ...................................................................... Patch Set 2: (2 comments) Patchset: PS1: > Thanks for requesting my review on this. […] Thanks! I uploaded new version after Peter's comment, to achieve the same effect but in more correct way. There are three more patches in the chain, if you are interested, you are very welcome to have a look and review! Gerrit works in a way that you can add comments and vote on any patch, even if you are not explicitly added to it (Gerrit will add you). File doc/supported_hw/index.rst: https://review.coreboot.org/c/flashrom/+/82196/comment/cea7ab84_790e6bf8 : PS1, Line 6: :maxdepth: 3 > Couldn't we just change maxdepth to have fewer nested levels? It seems error-prone to set this `hidd […] Oh somehow I was reading documentation for toctree directive and misunderstood it. Yes you idea is great, and it works! I updated the rest of the chain as well. -- To view, visit https://review.coreboot.org/c/flashrom/+/82196?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3 Gerrit-Change-Number: 82196 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Sydney <git(a)funkeleinhorn.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 10 May 2024 10:42:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sydney <git(a)funkeleinhorn.com> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashrom: Change chip unlock error to warning
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82175?usp=email ) Change subject: flashrom: Change chip unlock error to warning ...................................................................... flashrom: Change chip unlock error to warning Failing to disable WP before write/erase doesn't necessarily indicate an error and flashrom doesn't treat it as such. Print a warning instead on an error. BUG=b:336220545 BRANCH=none TEST=build Change-Id: I14c3b55e387443909ca1efab2fc1901f87dd66d6 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82175 Reviewed-by: Hsuan-ting Chen <roccochen(a)google.com> Reviewed-by: Brian Norris <briannorris(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashrom.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Nikolai Artemiev: Verified Brian Norris: Looks good to me, but someone else must approve Hsuan-ting Chen: Looks good to me, but someone else must approve build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 6b15ee5..a365303 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2092,7 +2092,7 @@ warn_out: if (ret) - msg_cerr("Failed to unlock flash status reg with wp support.\n"); + msg_cwarn("Failed to unlock flash status reg with wp support.\n"); return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/82175?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I14c3b55e387443909ca1efab2fc1901f87dd66d6 Gerrit-Change-Number: 82175 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: flashrom: Don't throw around "delay 1 second" so lightly
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email ) Change subject: flashrom: Don't throw around "delay 1 second" so lightly ...................................................................... flashrom: Don't throw around "delay 1 second" so lightly Waiting a full second is a very long time, especially when default_delay() chooses to busy-loop. This code has been around for a decade, with vague references to user reports: commit 8ab49e72af8465d4527de2ec37b22cd44f7a1169 Date: Wed Aug 19 13:55:34 2009 +0000 Disallow erase/write for known bad chips so people won't try without a clear understanding Still, this logic does not belong in the high-level library logic, used by all programmers and all chips. If there is a timing issue, it should either be encoded in the appropriate programmer or flashchip timing. However, we don't really know what chips were in use, as the above commit doesn't have any links to reports. So in a feeble attempt at avoiding breaking users here, we also surmise that... * SPI chips weren't all that common in 2009; * I'm mostly motivated by flashrom performance on Chromebooks, were SPI chips (and linux_mtd / BUS_PROG) are the rule; and * SPI chips have precise timing requirements and an appropriate BUSY status. So we guess that this "calm down" magic delay wouldn't be necessary there. Thus, we allow this magic delay only on non-SPI (and non-BUG_PROG, used by linux_mtd for one) buses as a compromise. Now, this change has some (hopefully [1] tiny) chance of regression, so we have the following considerations: 1. emergency_help_message() already provides documentation on how to contact support, in case we need to handle any user-reported regressions. 2. If there is any regression here, it's only in the --verify code; so we can always provide workarounds for testing this, to determine whether this change may have been at fault. For example, something like: flashrom --write /my/new/image.bin --noverify sleep 1 flashrom --read /tmp/bar.bin cmp /my/new/image.bin /tmp/bar.bin If such problems occur, we can collect system/programmer/chip info to try to encode a more targeted delay into the appropriate chip/programmer implementation, and avoid penalizing the entire project like this. 3. We already have (embedded in erase_write()) erase verification that performs no such delay. So depending on the type of timing error that this delay was attempting to cover, we may have some proof that this delay is no longer necessary (or at least, that whatever systems were needing this delay in the first place are no longer caring about flashrom). 4. We've retained the delay for buses that were likely common in 2009 (per the above "feeble attempt"). NB: I avoid using the BUS_NONSPI macro, because I want to exclude any future buses from this workaround, even in the event that the BUS_NONSPI category grows in the future. [1] Famous last words. BUG=b:325539928 TEST=`flashrom_tester --flashrom_binary=$(which flashrom) \ internal Erase_and_Write Fail_to_verify`, TEST=`vpd -i RW_VPD -s foo=bar; vpd -i RW_VPD -l; \ vpd -i RW_VPD -d foo; vpd -i RW_VPD -l` TEST=`elogtool list; elogtool add 0xa7; elogtool list` on (at least) 2 systems: #1: Kukui/Kakadu rev2 - MTD programmer / kernel 5.10.215-24542-g0515a679eb42 / CrOS ~ 15857.0.0 #2: Zork/Dirinboz rev2 - chip name: vendor="Winbond" name="W25Q128.JW.DTR" / BIOS: Google_Dirinboz.13434.688.0 / kernel 5.4.267-21940-g67f70e251a74 / CrOS ~ 15753.43.0 Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Signed-off-by: Brian Norris <briannorris(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/80807 Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashrom.c 1 file changed, 25 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve diff --git a/flashrom.c b/flashrom.c index 630c69d..6b15ee5 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2330,8 +2330,31 @@ if (verify && !all_skipped) { msg_cinfo("Verifying flash... "); - /* Work around chips which need some time to calm down. */ - programmer_delay(flashctx, 1000*1000); + /* + * Work around chips which "need some time to calm down." + * + * Frankly, it's not 100% clear why this delay is here at all, + * except for a terse message from 2009 of "a few reports where + * verify directly after erase had unpleasant side effects like + * corrupting flash or at least getting incorrect verify + * results". Ideally, if there were a few known problematic + * chips or programmers, we could add quirks flags for those + * specific implementations without penalizing all other + * flashrom users. But alas, we don't know which systems + * experienced those issues. + * + * Out of an extreme abundance of caution, we retain this + * delay, but only for a few non-SPI bus types that were the + * likely prevalent targets at the time. This is a complete + * guess, which conveniently avoids wasting time on common + * BUS_SPI and BUS_PROG systems. + * + * Background thread: + * Subject: RFC: removing 1 second verification delay + * https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/SFV3… + */ + if (flashctx->chip->bustype & (BUS_PARALLEL | BUS_LPC | BUS_FWH)) + programmer_delay(flashctx, 1000*1000); if (verify_all) combine_image_by_layout(flashctx, newcontents, oldcontents); -- To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Gerrit-Change-Number: 80807 Gerrit-PatchSet: 7 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: doc: Add doc for supported boards and laptops
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/82271?usp=email ) Change subject: doc: Add doc for supported boards and laptops ...................................................................... doc: Add doc for supported boards and laptops Change-Id: Iaae05ccd138fd8f7760823f867f3c7799018dc2e Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/supported_hw/index.rst A doc/supported_hw/supported_boards.rst 2 files changed, 20 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/71/82271/1 diff --git a/doc/supported_hw/index.rst b/doc/supported_hw/index.rst index ba77132..eebf394 100644 --- a/doc/supported_hw/index.rst +++ b/doc/supported_hw/index.rst @@ -8,3 +8,4 @@ supported_flashchips supported_prog/index supported_chipsets + supported_boards diff --git a/doc/supported_hw/supported_boards.rst b/doc/supported_hw/supported_boards.rst new file mode 100644 index 0000000..91b2899 --- /dev/null +++ b/doc/supported_hw/supported_boards.rst @@ -0,0 +1,19 @@ +======================== +Supported boards/laptops +======================== + +To see the list of all supported boards or laptops, check either ``struct board_info boards_known[]`` or ``struct board_info laptops_known[]`` +in the ``known_boards.c`` file in the source tree. + +If you have a flashrom repo cloned locally, you can look at the file in your repo, alternatively inspect the file +`on the web UI of our GitHub mirror <https://github.com/flashrom/flashrom/blob/main/known_boards.c#L29>`_. + +If you can run flashrom locally, the command ``flashrom -L`` prints the list of all supported boards and laptops +(see :doc:`/classic_cli_manpage` for more details on command line options). The output of this command is long, so you might +want to save it to file or grep. + +Each board entry is described by the ``struct board_info`` in ``include/programmer.h`` which you can inspect in the same way, either in the local source tree or +`in the GitHub web UI <https://github.com/flashrom/flashrom/blob/main/include/programmer.h#L207>`_. + +Note the ``enum test_state status`` of the board. ``OK`` means board is tested, ``NT`` means not tested, to see all possible +test states check the ``enum test_state`` definition in ``include/flash.h``. -- To view, visit https://review.coreboot.org/c/flashrom/+/82271?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iaae05ccd138fd8f7760823f867f3c7799018dc2e Gerrit-Change-Number: 82271 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2024