[PATCH 1/4] clock: Change cyc2ns return type to uint64_t

[PATCH 1/4] clock: Change cyc2ns return type to uint64_t

[Jules Maselbas]

Using an uint32_t to count nanosec will overflow every ~4sec, this means
that if get_time_ns is not called often enough the time keeping will be
wrong. By changing the return type to uint64_t doesn't fix the underlying
overflow issue but it will take more than 500 years to happen and I think
it's safe to assume this won't be an issue.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 include/clock.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/clock.h b/include/clock.h
index d681bf630..e6197e7eb 100644
--- a/include/clock.h
+++ b/include/clock.h
@@ -17,7 +17,7 @@ struct clocksource {
 	int		(*init)(struct clocksource*);
 };
 
-static inline uint32_t cyc2ns(struct clocksource *cs, uint64_t cycles)
+static inline uint64_t cyc2ns(struct clocksource *cs, uint64_t cycles)
 {
         uint64_t ret = cycles;
         ret = (ret * cs->mult) >> cs->shift;
-- 
2.17.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/4] clock: Update cs->cycle_last when switching clock

[Jules Maselbas]

If clocksource is free-running it might have been running for a while
before barebox started, we only care about the time spent in barebox.
We can discard the cycles accumulated before switching clocks by
updating cs->cycle_last with the cycles read for the new clock right
before switching.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 common/clock.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/common/clock.c b/common/clock.c
index 58c2964b1..e30c81a37 100644
--- a/common/clock.c
+++ b/common/clock.c
@@ -222,6 +222,12 @@ int init_clock(struct clocksource *cs)
 			return ret;
 	}
 
+	/*
+	 * If clocksource is freerunning it might have been running for a while
+	 * before barebox started, we only care about the time spent in barebox
+	 * thus we must discard the clocksource cycles up to this exact moment:
+	 */
+	cs->cycle_last = cs->read() & cs->mask;
 	current_clock = cs;
 	time_beginning = get_time_ns();
 
-- 
2.17.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/4] clock: Remove time_beginning

[Jules Maselbas]

The output of the `demsg -t` command will have broken timestamps for
every log entries written (in the log) before switching clock.

The function get_time_ns() is monotonic, thus setting time_beginning
with get_time_ns() when switching clocksource will always result in
time_beginning being greater than every timestamp set before.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 common/clock.c          | 7 -------
 common/console_common.c | 2 +-
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/common/clock.c b/common/clock.c
index e30c81a37..4dd9377de 100644
--- a/common/clock.c
+++ b/common/clock.c
@@ -17,12 +17,6 @@
 
 static uint64_t time_ns;
 
-/*
- * The first timestamp when the clocksource is registered.
- * Useful for measuring the time spent in barebox.
- */
-uint64_t time_beginning;
-
 static uint64_t dummy_read(void)
 {
 	static uint64_t dummy_counter;
@@ -229,7 +223,6 @@ int init_clock(struct clocksource *cs)
 	 */
 	cs->cycle_last = cs->read() & cs->mask;
 	current_clock = cs;
-	time_beginning = get_time_ns();
 
 	return 0;
 }
diff --git a/common/console_common.c b/common/console_common.c
index 48590c522..7e9973ed8 100644
--- a/common/console_common.c
+++ b/common/console_common.c
@@ -188,7 +188,7 @@ void log_print(unsigned flags, unsigned levels)
 	unsigned long last = 0;
 
 	list_for_each_entry(log, &barebox_logbuf, list) {
-		uint64_t diff = log->timestamp - time_beginning;
+		uint64_t diff = log->timestamp;
 		unsigned long difful;
 
 		if (levels && !(levels & (1 << log->level)))
-- 
2.17.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/4] common: common_console: Rework log_print

[Jules Maselbas]

Since time_beginning has been removed there is no reason to use a
variables called `diff` or `diff_ul`, there are respectively renamed
to `time_ns` and `time`.
Also remove the test for null timestamp.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 common/console_common.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/common/console_common.c b/common/console_common.c
index 7e9973ed8..1e4bf87b1 100644
--- a/common/console_common.c
+++ b/common/console_common.c
@@ -188,8 +188,8 @@ void log_print(unsigned flags, unsigned levels)
 	unsigned long last = 0;
 
 	list_for_each_entry(log, &barebox_logbuf, list) {
-		uint64_t diff = log->timestamp;
-		unsigned long difful;
+		uint64_t time_ns = log->timestamp;
+		unsigned long time;
 
 		if (levels && !(levels & (1 << log->level)))
 			continue;
@@ -201,21 +201,19 @@ void log_print(unsigned flags, unsigned levels)
 		if (flags & BAREBOX_LOG_PRINT_RAW)
 			printf("<%i>", log->level);
 
-		do_div(diff, 1000);
-		difful = diff;
-
-		if (!log->timestamp)
-			difful = 0;
+		/* convert ns to us */
+		do_div(time_ns, 1000);
+		time = time_ns;
 
 		if (flags & (BAREBOX_LOG_PRINT_TIME | BAREBOX_LOG_DIFF_TIME))
 			printf("[");
 
 		if (flags & BAREBOX_LOG_PRINT_TIME)
-			printf("%10luus", difful);
+			printf("%10luus", time);
 
 		if (flags & BAREBOX_LOG_DIFF_TIME) {
-			printf(" < %10luus", difful - last);
-			last = difful;
+			printf(" < %10luus", time - last);
+			last = time;
 		}
 
 		if (flags & (BAREBOX_LOG_PRINT_TIME | BAREBOX_LOG_DIFF_TIME))
-- 
2.17.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox