Peter Marheine has submitted this change. ( https://review.coreboot.org/c/flashrom/+/81606?usp=email )
Change subject: Make sleep threshold for delays configurable
......................................................................
Make sleep threshold for delays configurable
This allows the minimum time that default_delay() will choose to sleep
for instead of polling to be configured at build-time. The default
remains unchanged at 100 milliseconds for now.
The test's correctness has been checked by testing with minimum sleep
time left at its default and set to a non-default value smaller than 100
microseconds (both pass without sleeping, verified with strace) and with
the minimum sleep time set to 0 (causing the test to be skipped). The
configured value from the macro needs to be stored in a const to avoid
-Werror=type-limits errors when configured to be zero.
Change-Id: Ida96e0816ac914ed69d6fd82ad90ebe89cdef1cc
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/81606
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M Makefile
M meson.build
M meson_options.txt
M tests/udelay.c
M udelay.c
M udelay_dos.c
6 files changed, 38 insertions(+), 10 deletions(-)
Approvals:
build bot (Jenkins): Verified
Anastasia Klimchuk: Looks good to me, approved
diff --git a/Makefile b/Makefile
index 9b94e22..97f56b8 100644
--- a/Makefile
+++ b/Makefile
@@ -542,6 +542,11 @@
# Disable wiki printing by default. It is only useful if you have wiki access.
CONFIG_PRINT_WIKI ?= no
+# 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.
+CONFIG_DELAY_MINIMUM_SLEEP_US ?= 100000
+
# Disable all features if CONFIG_NOTHING=yes is given unless CONFIG_EVERYTHING was also set
ifeq ($(CONFIG_NOTHING), yes)
ifeq ($(CONFIG_EVERYTHING), yes)
@@ -587,6 +592,7 @@
endif
FEATURE_FLAGS += -D'CONFIG_DEFAULT_PROGRAMMER_ARGS="$(CONFIG_DEFAULT_PROGRAMMER_ARGS)"'
+FEATURE_FLAGS += -D'CONFIG_DELAY_MINIMUM_SLEEP_US=$(CONFIG_DELAY_MINIMUM_SLEEP_US)'
################################################################################
diff --git a/meson.build b/meson.build
index c4d7717..458b6e8 100644
--- a/meson.build
+++ b/meson.build
@@ -112,6 +112,9 @@
delay_src = files('udelay_dos.c')
endif
srcs += delay_src
+cargs += ['-DCONFIG_DELAY_MINIMUM_SLEEP_US=@0@'.format(
+ get_option('delay_minimum_sleep_us')
+)]
# check for required symbols
if cc.has_function('clock_gettime')
diff --git a/meson_options.txt b/meson_options.txt
index e62deb6..8a04114 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -22,3 +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,
+ 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.')
diff --git a/tests/udelay.c b/tests/udelay.c
index 277a15d..8d24b00 100644
--- a/tests/udelay.c
+++ b/tests/udelay.c
@@ -34,6 +34,8 @@
#endif
}
+static const int64_t min_sleep = CONFIG_DELAY_MINIMUM_SLEEP_US;
+
/*
* A short delay should delay for at least as long as requested,
* and more than 10x as long would be worrisome.
@@ -43,9 +45,21 @@
* test.
*/
void udelay_test_short(void **state) {
+ /*
+ * Delay for 100 microseconds, or short enough that we won't sleep.
+ * It's not useful to test the sleep path because we assume the OS won't
+ * sleep for less time than we ask.
+ */
+ int64_t delay_us = 100;
+ if (delay_us >= min_sleep)
+ delay_us = min_sleep - 1;
+ /* No point in running this test if delay always sleeps. */
+ if (delay_us <= 0)
+ skip();
+
uint64_t start = now_us();
- default_delay(100);
+ default_delay(delay_us);
uint64_t elapsed = now_us() - start;
- assert_in_range(elapsed, 100, 1000);
+ assert_in_range(elapsed, delay_us, 10 * delay_us);
}
diff --git a/udelay.c b/udelay.c
index 453cb34..c43b6e3 100644
--- a/udelay.c
+++ b/udelay.c
@@ -92,14 +92,15 @@
#endif
}
+static const unsigned min_sleep = CONFIG_DELAY_MINIMUM_SLEEP_US;
+
/* Precise delay. */
void default_delay(unsigned int usecs)
{
- /* If the delay is >0.1 s, use internal_sleep because timing does not need to be so precise. */
- if (usecs > 100000) {
- internal_sleep(usecs);
- } else {
+ if (usecs < min_sleep) {
clock_usec_delay(usecs);
+ } else {
+ internal_sleep(usecs);
}
}
diff --git a/udelay_dos.c b/udelay_dos.c
index 61493fc..c3914ea 100644
--- a/udelay_dos.c
+++ b/udelay_dos.c
@@ -153,19 +153,20 @@
usleep(usecs % 1000000);
}
+static const unsigned min_sleep = CONFIG_DELAY_MINIMUM_SLEEP_US;
+
/* Precise delay. */
void default_delay(unsigned int usecs)
{
static bool calibrated = false;
- /* If the delay is >0.1 s, use internal_sleep because timing does not need to be so precise. */
- if (usecs > 100000) {
- internal_sleep(usecs);
- } else {
+ if (usecs < min_sleep) {
if (!calibrated) {
myusec_calibrate_delay();
calibrated = true;
}
myusec_delay(usecs);
+ } else {
+ internal_sleep(usecs);
}
}
--
To view, visit https://review.coreboot.org/c/flashrom/+/81606?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: Ida96e0816ac914ed69d6fd82ad90ebe89cdef1cc
Gerrit-Change-Number: 81606
Gerrit-PatchSet: 6
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Peter Marheine has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82212?usp=email )
Change subject: dos/meson: add a hint for setting sys_root
......................................................................
dos/meson: add a hint for setting sys_root
I found that cross-compiling with GCC 12.2.0 targeting DJGPP from Linux
on x86_64 that meson used my system include directory
(/usr/include/x86_64-linux-gnu/) and pulled in include files that are
incompatible with DJGPP. Setting sys_root prevents meson from assuming
they're compatible between the build and host systems, fixing those
compile-time errors.
TEST=meson setup --cross-file meson_cross/i586_djgpp_dos.txt; ninja
libflashrom.h no longer causes "features.h: No such file or
directory" errors via /usr/include/x86_64-linux-gnu/sys/types.h
Change-Id: Ib9cf70f6f94782c5303fb232aaf4a46192907f66
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/82212
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M meson_cross/i586_djgpp_dos.txt
1 file changed, 6 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Anastasia Klimchuk: Looks good to me, approved
diff --git a/meson_cross/i586_djgpp_dos.txt b/meson_cross/i586_djgpp_dos.txt
index 66d5ed0..3d97aab 100644
--- a/meson_cross/i586_djgpp_dos.txt
+++ b/meson_cross/i586_djgpp_dos.txt
@@ -5,6 +5,11 @@
# Make sure pkg-config can find your self compiles libpci
# or add the path of your libpci.pc as 'pkg_config_libdir'
# under [properies] below.
+#
+# If cross-compiling, you may need to set sys_root in the [properties]
+# section because meson otherwise assumes the same sysroot as the
+# system on which you're building and will get the wrong include files
+# (from /usr/include/x86_64 for example) among other possible issues.
[binaries]
c = 'i586-pc-msdosdjgpp-gcc'
@@ -27,3 +32,4 @@
ich_descriptors_tool = 'disabled'
[properties]
+sys_root = '/usr/local/djgpp'
--
To view, visit https://review.coreboot.org/c/flashrom/+/82212?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: Ib9cf70f6f94782c5303fb232aaf4a46192907f66
Gerrit-Change-Number: 82212
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Marheine <pmarheine(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>
Gerrit-MessageType: merged
Attention is currently required from: Anastasia Klimchuk, J. Neuschäfer, Riku Viitanen, Thomas Heijligen.
Sydney 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:
(1 comment)
File doc/supported_hw/supported_prog/serprog/overview.rst:
https://review.coreboot.org/c/flashrom/+/82229/comment/8df4e581_d6b6fba8 :
PS2, Line 95: etter performance
> I'm not sure it's actually more performant since stacksmashing's fork seems to have made performance […]
I am not aware of any exiting benchmarks. I removed the statement about performance differences until this can be backed by tests/data. Thanks for pointing this out.
--
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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 08 May 2024 16:20:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Riku Viitanen <riku.viitanen(a)protonmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, J. Neuschäfer, Sydney, Thomas Heijligen.
Hello Anastasia Klimchuk, J. Neuschäfer, Riku Viitanen, Thomas Heijligen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/82229?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: Add documentation for pico-serprog
......................................................................
Add documentation for pico-serprog
This commit adds documentation for pico-serprog by stacksmashing:
https://github.com/stacksmashing/pico-serprog
and its fork by Riku_V: https://codeberg.org/Riku_V/pico-serprog
to the serprog overview page.
Change-Id: I457dfec52f89997f64b6c276c50b329359d61b77
Signed-off-by: Funkeleinhorn <git(a)funkeleinhorn.com>
---
M doc/supported_hw/supported_prog/serprog/overview.rst
1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/82229/3
--
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: J. Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sydney <git(a)funkeleinhorn.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Anastasia Klimchuk, J. Neuschäfer, Sydney, Thomas Heijligen.
Riku Viitanen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82229?usp=email )
Change subject: Add documentation for pico-serprog
......................................................................
Patch Set 2:
(1 comment)
File doc/supported_hw/supported_prog/serprog/overview.rst:
https://review.coreboot.org/c/flashrom/+/82229/comment/bef88de4_2c2c51f2 :
PS2, Line 95: etter performance
I'm not sure it's actually more performant since stacksmashing's fork seems to have made performance improvements as well. Has anyone benchmarked these implementations? The PIOs are pretty impressive.
--
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: 2
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: J. Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sydney <git(a)funkeleinhorn.com>
Gerrit-Comment-Date: Wed, 08 May 2024 15:01:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, J. Neuschäfer, Riku Viitanen, Thomas Heijligen.
Sydney has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82229?usp=email )
Change subject: Add documentation for pico-serprog
......................................................................
Patch Set 2:
(3 comments)
Patchset:
PS1:
> Hi, thanks for writing this :)
Thanks for your feedback. I addressed the suggested changes. Please have a look again.
File doc/supported_hw/supported_prog/serprog/overview.rst:
https://review.coreboot.org/c/flashrom/+/82229/comment/74471bfd_3dc92483 :
PS1, Line 90: is an bigger existing
> 1. typo, → "a bigger" […]
Done
https://review.coreboot.org/c/flashrom/+/82229/comment/c04c8cec_17ee5f57 :
PS1, Line 91: Notable differences are:
> To make this paragraph easier to extend, and perhaps also easier to read, I'd reorganize it a bit: […]
I like the suggested structure and restructured the paragraph accordingly. Thanks for the suggestion.
--
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: 2
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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 08 May 2024 13:12:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: J. Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Riku Viitanen, Sydney, Thomas Heijligen.
Hello Anastasia Klimchuk, J. Neuschäfer, Riku Viitanen, Thomas Heijligen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/82229?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: Add documentation for pico-serprog
......................................................................
Add documentation for pico-serprog
This commit adds documentation for pico-serprog by stacksmashing:
https://github.com/stacksmashing/pico-serprog
and its fork by Riku_V: https://codeberg.org/Riku_V/pico-serprog
to the serprog overview page.
Change-Id: I457dfec52f89997f64b6c276c50b329359d61b77
Signed-off-by: Funkeleinhorn <git(a)funkeleinhorn.com>
---
M doc/supported_hw/supported_prog/serprog/overview.rst
1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/82229/2
--
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: 2
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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sydney <git(a)funkeleinhorn.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer.
DZ has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81835?usp=email )
Change subject: flashchips: Add support for MXIC MX25L1636E
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/flashrom/+/81835?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: I415e2d6c89d3d59ba44e22753001c6f69421c39d
Gerrit-Change-Number: 81835
Gerrit-PatchSet: 5
Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 08 May 2024 10:27:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment