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

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Peter Marheine (Code Review) 03 Apr '24

03 Apr '24

Attention is currently required from: Anastasia Klimchuk, Brian Norris, Thomas Heijligen. Hello Anastasia Klimchuk, Brian Norris, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81545?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... udelay: only use OS time for delays, except on DOS As proposed on the mailing list ("RFC: remove the calibrated delay loop"), this removes the calibrated delay loop and uses OS-based timing functions for all delays because the calibrated delay loop can delay for shorter times than intended. When sleeping this now uses nanosleep() if available, and otherwise usleep(). When busy-looping, it uses clock_gettime() with CLOCK_MONOTONIC or CLOCK_REALTIME depending on availability, and gettimeofday() otherwise. The calibrated delay loop is retained for DOS only, because timer resolution on DJGPP is only about 50 milliseconds. Since typical delays in flashrom are around 10 microseconds, using OS timing there would regress performance by around 500x. The old implementation is reused with some branches removed based on the knowledge that timer resolution will not be better than about 50 milliseconds. Tested by reading and writing flash on several Intel and AMD systems: * Lenovo P920 (Intel C620, read/verify only) * "nissa" chromebook (Intel Alder Lake-N) * "zork" chromebook (AMD Zen+) Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: I7ac5450d194a475143698d65d64d8bcd2fd25e3f --- M Makefile M include/programmer.h M libflashrom.c M meson.build M tests/meson.build M tests/tests.c M tests/tests.h A tests/udelay.c M udelay.c A udelay_dos.c 10 files changed, 261 insertions(+), 172 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/81545/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?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: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 6 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Peter Marheine (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Anastasia Klimchuk, Brian Norris, Thomas Heijligen. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81545?usp=email ) Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... Patch Set 5: (3 comments) File meson.build: https://review.coreboot.org/c/flashrom/+/81545/comment/18111898_4f216adf : PS4, Line 121: DHAVE_NANOSLEEP=1 > yeah, maybe something similar to the `Makefile.d/clock_gettime_test. […] On reflection, this isn't even required. We previously only avoided using `nanosleep` on DOS, and DOS now has its own version of this file so we can assume this function is always available. Removed this check and the sleep+usleep branch in `internal_sleep`. File udelay.c: https://review.coreboot.org/c/flashrom/+/81545/comment/988fc1ba_e62af74f : PS4, Line 6: * > I think this counts as a significant change, maybe you want to add your copyright here? Done https://review.coreboot.org/c/flashrom/+/81545/comment/0ab1fadd_9bdfed46 : PS4, Line 76: timeusec > I'm gonna say you probably didn't test this part 😜 😬 thanks. I've added a test for basic delay operation and hacked meson.build temporarily to verify that everything builds and the test passes in both cases now. -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?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: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 5 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 02 Apr 2024 23:54:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Peter Marheine (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Thomas Heijligen. Hello Anastasia Klimchuk, Brian Norris, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81545?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... udelay: only use OS time for delays, except on DOS As proposed on the mailing list ("RFC: remove the calibrated delay loop"), this removes the calibrated delay loop and uses OS-based timing functions for all delays because the calibrated delay loop can delay for shorter times than intended. When sleeping this now uses nanosleep() if available, and otherwise usleep(). When busy-looping, it uses clock_gettime() with CLOCK_MONOTONIC or CLOCK_REALTIME depending on availability, and gettimeofday() otherwise. The calibrated delay loop is retained for DOS only, because timer resolution on DJGPP is only about 50 milliseconds. Since typical delays in flashrom are around 10 microseconds, using OS timing there would regress performance by around 500x. The old implementation is reused with some branches removed based on the knowledge that timer resolution will not be better than about 50 milliseconds. Tested by reading and verifying flash on several Intel and AMD systems: * Lenovo P920 (Intel C620) * (more not done yet) Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: I7ac5450d194a475143698d65d64d8bcd2fd25e3f --- M Makefile M include/programmer.h M libflashrom.c M meson.build M tests/meson.build M tests/tests.c M tests/tests.h A tests/udelay.c M udelay.c A udelay_dos.c 10 files changed, 260 insertions(+), 172 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/81545/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?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: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 5 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> 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-MessageType: newpatchset

1 0

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Brian Norris (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Thomas Heijligen. Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81545?usp=email ) Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: I meant to say: I'm happy to see this change in general. Aside from the relatively small review comments, this looks good to me -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?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: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 4 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> 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: Tue, 02 Apr 2024 21:22:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Brian Norris (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Thomas Heijligen. Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81545?usp=email ) Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... Patch Set 4: (2 comments) File meson.build: https://review.coreboot.org/c/flashrom/+/81545/comment/83756100_30849eb3 : PS4, Line 121: DHAVE_NANOSLEEP=1 > I didn't find this to be ever defined in Makefile, is it right? I thought there would be something i […] yeah, maybe something similar to the `Makefile.d/clock_gettime_test.c` test? frankly, it's easy to get lazy and incomplete [*] when there are two build systems, for some reason... [*] speaking from experience File udelay.c: https://review.coreboot.org/c/flashrom/+/81545/comment/47ccbdf1_dd3d0200 : PS4, Line 76: timeusec I'm gonna say you probably didn't test this part 😜 -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?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: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 4 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> 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: Tue, 02 Apr 2024 21:21:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Anastasia Klimchuk (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Brian Norris, Peter Marheine, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81545?usp=email ) Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... Patch Set 4: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/81545/comment/77d2ad59_24d5233c : PS4, Line 25: For the patch like this, it would be useful to know what was tested, if you could add testing info here, at least from yourself, it would be great! (whenever you will have such info, I understand it might be ready a bit later). Patchset: PS4: Thank you so much for the patch! File meson.build: https://review.coreboot.org/c/flashrom/+/81545/comment/db72f8ed_b0a68bf9 : PS4, Line 121: DHAVE_NANOSLEEP=1 I didn't find this to be ever defined in Makefile, is it right? I thought there would be something in Makefile too? File udelay.c: https://review.coreboot.org/c/flashrom/+/81545/comment/fc39ae6f_9e6f8054 : PS4, Line 6: * I think this counts as a significant change, maybe you want to add your copyright here? -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?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: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 4 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 02 Apr 2024 08:34:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: serprog: Add SPI Mode and CS Mode commands
by Anastasia Klimchuk (Code Review) 02 Apr '24

02 Apr '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/81428?usp=email ) Change subject: serprog: Add SPI Mode and CS Mode commands ...................................................................... serprog: Add SPI Mode and CS Mode commands This commit adds two new commands to the serprog protocol which allow more fine grained control over the SPI bus. This enables more applications over serprog like e.g. flashing AVR microcontrollers. This can be tried with my forks of pico-serprog: https://github.com/funkeleinhorn/pico-serprog/tree/spimode and avrdude: https://github.com/funkeleinhorn/avrgirl/tree/serprog-programmer I announced this change in flashrom and flashprog IRC channels and got overall positive feedback in the flashprog channel. The same changes will be sent to flashprog to prevent diverging specs. Change-Id: Idb5a9a3710fede322def5191d68b7fba0e135292 Signed-off-by: Funkeleinhorn <git(a)funkeleinhorn.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/81428 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M Documentation/serprog-protocol.txt 1 file changed, 17 insertions(+), 0 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified diff --git a/Documentation/serprog-protocol.txt b/Documentation/serprog-protocol.txt index 3d4aa1a..d001ceb 100644 --- a/Documentation/serprog-protocol.txt +++ b/Documentation/serprog-protocol.txt @@ -36,6 +36,8 @@ 0x14 Set SPI clock frequency in Hz 32-bit requested frequency ACK + 32-bit set frequency / NAK 0x15 Toggle flash chip pin drivers 8-bit (0 disable, else enable) ACK / NAK 0x16 Set SPI Chip Select 8-bit ACK / NAK +0x17 Set SPI Mode 8-bit ACK / NAK +0x18 Set CS Mode 8-bit ACK / NAK 0x?? unimplemented command - invalid. @@ -93,6 +95,21 @@ 0x16 (S_SPI_CS): Set which SPI Chip Select pin to use. This operation is immediate, meaning it doesn't use the operation buffer. + 0x17 (S_SPI_MODE): + Set which SPI Mode to use for 0x13 O_SPIOP commands. + This operation is immediate, meaning it doesn't use the operation buffer. + The current defined modes are: + 0x00: SPI Half Duplex (default) + 0x01: SPI Full Duplex + 0x18 (S_CS_MODE): + Set which CS Mode to use. The CS Mode determines the CS behaviour. + This allows manual control over the CS. + This operation is immediate, meaning it doesn't use the operation buffer. + The current defined modes are: + 0x00: CS Auto Mode. The CS gets selected before 0x13 O_SPIOP commands and + deselected afterwards. (default) + 0x01: CS Selected. The CS will be selected until another mode is set. + 0x02: CS Deselected. The CS will be deselected until another mode is set. About mandatory commands: The only truly mandatory commands for any device are 0x00, 0x01, 0x02 and 0x10, but one can't really do anything with these commands. -- To view, visit https://review.coreboot.org/c/flashrom/+/81428?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: Idb5a9a3710fede322def5191d68b7fba0e135292 Gerrit-Change-Number: 81428 Gerrit-PatchSet: 2 Gerrit-Owner: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: serprog: Add SPI Mode and CS Mode commands
by Anastasia Klimchuk (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Funkeleinhorn, Urja Rannikko. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81428?usp=email ) Change subject: serprog: Add SPI Mode and CS Mode commands ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I forgot to reply to: > I didn't know about the Discord There is a doc with all various way to talk to people: https://flashrom.org/contact.html you are welcome! -- To view, visit https://review.coreboot.org/c/flashrom/+/81428?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: Idb5a9a3710fede322def5191d68b7fba0e135292 Gerrit-Change-Number: 81428 Gerrit-PatchSet: 1 Gerrit-Owner: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Urja Rannikko <urjaman(a)gmail.com> Gerrit-Attention: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 02:51:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Funkeleinhorn <git(a)funkeleinhorn.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: udelay: clock_getres() does not provide clock_gettime() resolution
by Peter Marheine (Code Review) 02 Apr '24

02 Apr '24

Attention is currently required from: Anastasia Klimchuk, Brian Norris, Chen, Nikolai Artemiev, Stefan Reinauer. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80806?usp=email ) Change subject: udelay: clock_getres() does not provide clock_gettime() resolution ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/80806/comment/7f4040c3_29a8c675 : PS2, Line 10: Linux makes this clearer > I've subscribed now, but there's no way to reply to messages that predate my subscription. […] After looking at what kinds of delays things use in reality, I decided realtime priority wasn't necessary anyway. If there's insufficient agreement on how long we can reasonably sleep, that's why it's nice to make it configurable at build-time. The CL resulting from that proposal is https://review.coreboot.org/c/flashrom/+/81545 -- To view, visit https://review.coreboot.org/c/flashrom/+/80806?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: Icdc6e184eacb14d3bd27029e3fc75be4431d82b4 Gerrit-Change-Number: 80806 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Chen <c(a)jia.je> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> 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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Chen <c(a)jia.je> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 02:22:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: reduce DELAY_MINIMUM_SLEEP_US to 100 by default
by Peter Marheine (Code Review) 02 Apr '24

02 Apr '24

Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/81608?usp=email ) Change subject: reduce DELAY_MINIMUM_SLEEP_US to 100 by default ...................................................................... reduce DELAY_MINIMUM_SLEEP_US to 100 by default This makes flashrom sleep more eagerly rather than busy-waiting, observing that most delays in flashrom are either less than 100 microseconds (barely enough time to get any work done, even on a fast machine) or much more than 1 millisecond (very wasteful to busy-loop). Since we believe most systems offer good timer resolution that should provide sleep latency on the order of 100 microseconds, this is a reasonable default. For DOS, the default is set to 50ms because the best available timing source on DOS only ticks at about 20 Hz. Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: I0f431d240c670446218b14811ef62a34e4c83da2 --- M Makefile M meson_cross/i586_djgpp_dos.txt M meson_options.txt 3 files changed, 4 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/81608/1 diff --git a/Makefile b/Makefile index 09b31b3..71a7490 100644 --- a/Makefile +++ b/Makefile @@ -542,7 +542,7 @@ # Disable wiki printing by default. It is only useful if you have wiki access. CONFIG_PRINT_WIKI ?= no -CONFIG_DELAY_MINIMUM_SLEEP_US ?= 100000 +CONFIG_DELAY_MINIMUM_SLEEP_US ?= 100 # Disable all features if CONFIG_NOTHING=yes is given unless CONFIG_EVERYTHING was also set ifeq ($(CONFIG_NOTHING), yes) diff --git a/meson_cross/i586_djgpp_dos.txt b/meson_cross/i586_djgpp_dos.txt index 66d5ed0..4a96e2e 100644 --- a/meson_cross/i586_djgpp_dos.txt +++ b/meson_cross/i586_djgpp_dos.txt @@ -25,5 +25,7 @@ [project options] tests = 'disabled' ich_descriptors_tool = 'disabled' +# DOS time resolution is only about 50ms +delay_minimum_sleep_us = 50000 [properties] diff --git a/meson_options.txt b/meson_options.txt index 8a04114..f5e2800 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -22,6 +22,6 @@ option('documentation', type : 'feature', value : 'auto', description : 'build the html documentation') option('ni845x_search_path', type : 'string', value : 'C:\Program Files (x86)\National Instruments\Ni-845x\MS Visual C', description : 'Path to search for the proprietary ni845x library and header (32-bit Windows only)') -option('delay_minimum_sleep_us', type : 'integer', min : 0, value : 100000, +option('delay_minimum_sleep_us', type : 'integer', min : 0, value : 100, description : 'Minimum time in microseconds to suspend execution for (rather than polling) when a delay is required.' + ' Larger values may perform better on machines with low timer resolution, at the cost of increased power.') -- To view, visit https://review.coreboot.org/c/flashrom/+/81608?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: I0f431d240c670446218b14811ef62a34e4c83da2 Gerrit-Change-Number: 81608 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2024