Nico Huber merged this change.

View Change

Approvals: Stefan Reinauer: Looks good to me, approved Philippe Mathieu-Daudé: Looks good to me, but someone else must approve David Hendricks: Looks good to me, approved build bot (Jenkins): Verified
udelay: Use clock_gettime() if available and precise

Instead of calibrating our busy loop against a coarse clock, check if
a precise clock is available and loop against that. The former is unre-
liable by definition on any modern system that may dynamically reclock
the processor.

v2: Apparently _POSIX_MONOTONIC_CLOCK being defined only means that
the library knows about CLOCK_MONOTONIC. So check for its support
at runtime and fall back to CLOCK_REALTIME if it's missing.

TEST=Manually added a 10s loop and compared to real time. Run
on Linux RPi3, Linux x86 and my original use case Linux in
VirtualBox (Linux host).

Change-Id: I85ad359823875237ada9cd027af3017d62e9a235
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/19391
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: David Hendricks <david.hendricks@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M Makefile
M udelay.c
2 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 65026da..8b21431 100644
--- a/Makefile
+++ b/Makefile
@@ -1003,6 +1003,9 @@
# We could use PULLED_IN_LIBS, but that would be ugly.
FEATURE_LIBS += $(call debug_shell,grep -q "NEEDLIBZ := yes" .libdeps && printf "%s" "-lz")

+FEATURE_CFLAGS += $(call debug_shell,grep -q "CLOCK_GETTIME := yes" .features && printf "%s" "-D'HAVE_CLOCK_GETTIME=1'")
+FEATURE_LIBS += $(call debug_shell,grep -q "CLOCK_GETTIME := yes" .features && printf "%s" "-lrt")
+
LIBFLASHROM_OBJS = $(CHIP_OBJS) $(PROGRAMMER_OBJS) $(LIB_OBJS)
OBJS = $(CLI_OBJS) $(LIBFLASHROM_OBJS)

@@ -1299,6 +1302,18 @@
endef
export LINUX_I2C_TEST

+define CLOCK_GETTIME_TEST
+#include <time.h>
+
+int main(int argc, char **argv)
+{
+ struct timespec res;
+ clock_gettime(CLOCK_REALTIME, &res);
+ return 0;
+}
+endef
+export CLOCK_GETTIME_TEST
+
features: compiler
@echo "FEATURES := yes" > .features.tmp
ifneq ($(NEED_LIBFTDI), )
@@ -1341,6 +1356,13 @@
@ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \
( echo "found."; echo "UTSNAME := yes" >> .features.tmp ) || \
( echo "not found."; echo "UTSNAME := no" >> .features.tmp ) } 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE)
+ @printf "Checking for clock_gettime support... " | tee -a $(BUILD_DETAILS_FILE)
+ @echo "$$CLOCK_GETTIME_TEST" >.featuretest.c
+ @printf "\nexec: %s\n" "$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -lrt .featuretest.c -o .featuretest$(EXEC_SUFFIX)" >>$(BUILD_DETAILS_FILE)
+ @ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -lrt .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \
+ ( echo "found."; echo "CLOCK_GETTIME := yes" >>.features.tmp ) || \
+ ( echo "not found."; echo "CLOCK_GETTIME := no" >>.features.tmp ) } \
+ 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE)
@$(DIFF) -q .features.tmp .features >/dev/null 2>&1 && rm .features.tmp || mv .features.tmp .features
@rm -f .featuretest.c .featuretest$(EXEC_SUFFIX)

diff --git a/udelay.c b/udelay.c
index 7c6961d..3a44c16 100644
--- a/udelay.c
+++ b/udelay.c
@@ -21,12 +21,63 @@

#ifndef __LIBPAYLOAD__

+#include <stdbool.h>
#include <unistd.h>
+#include <errno.h>
#include <time.h>
#include <sys/time.h>
#include <stdlib.h>
#include <limits.h>
#include "flash.h"
+
+static bool use_clock_gettime = false;
+
+#if HAVE_CLOCK_GETTIME == 1
+
+#ifdef _POSIX_MONOTONIC_CLOCK
+static clockid_t clock_id = CLOCK_MONOTONIC;
+#else
+static clockid_t clock_id = CLOCK_REALTIME;
+#endif
+
+static void clock_usec_delay(int usecs)
+{
+ struct timespec now;
+ clock_gettime(clock_id, &now);
+
+ const long end_nsec = now.tv_nsec + usecs * 1000L;
+ const struct timespec end = {
+ end_nsec / (1000 * 1000 * 1000) + now.tv_sec,
+ end_nsec % (1000 * 1000 * 1000)
+ };
+ do {
+ clock_gettime(clock_id, &now);
+ } while (now.tv_sec < end.tv_sec || (now.tv_sec == end.tv_sec && now.tv_nsec < end.tv_nsec));
+}
+
+static int clock_check_res(void)
+{
+ struct timespec res;
+ if (!clock_getres(clock_id, &res)) {
+ if (res.tv_sec == 0 && res.tv_nsec <= 100) {
+ msg_pinfo("Using clock_gettime for delay loops (clk_id: %d, resolution: %ldns).\n",
+ (int)clock_id, res.tv_nsec);
+ use_clock_gettime = true;
+ return 1;
+ }
+ } else if (clock_id != CLOCK_REALTIME && errno == EINVAL) {
+ /* Try again with CLOCK_REALTIME. */
+ clock_id = CLOCK_REALTIME;
+ return clock_check_res();
+ }
+ return 0;
+}
+#else
+
+static inline void clock_usec_delay(int usecs) {}
+static inline int clock_check_res(void) { return 0; }
+
+#endif /* HAVE_CLOCK_GETTIME == 1 */

/* loops per microsecond */
static unsigned long micro = 1;
@@ -87,6 +138,9 @@

void myusec_calibrate_delay(void)
{
+ if (clock_check_res())
+ return;
+
unsigned long count = 1000;
unsigned long timeusec, resolution;
int i, tries = 0;
@@ -189,6 +243,8 @@
/* If the delay is >1 s, use internal_sleep because timing does not need to be so precise. */
if (usecs > 1000000) {
internal_sleep(usecs);
+ } else if (use_clock_gettime) {
+ clock_usec_delay(usecs);
} else {
myusec_delay(usecs);
}

To view, visit change 19391. To unsubscribe, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-MessageType: merged
Gerrit-Change-Id: I85ad359823875237ada9cd027af3017d62e9a235
Gerrit-Change-Number: 19391
Gerrit-PatchSet: 6
Gerrit-Owner: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Reviewer: Philippe Mathieu-Daudé <f4bug@amsat.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-Reviewer: Stefan Tauner <stefan.tauner@gmx.at>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>