[S] Change in flashrom[main]: Make sleep threshold for delays configurable

[Peter Marheine (Code Review)]

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 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/81606
Tested-by: build bot (Jenkins) 
Reviewed-by: Anastasia Klimchuk 
---
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);
    }
 }