Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38300 )
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour
As logging is guarded by Kconfig, increase the level from BIOS_SPEW to BIOS_INFO.
The original callsite inside timestamp_add_table_entry() was also called when syncing from timestamps from .bss to CBMEM. We should not reprint the values then.
Change-Id: I72ca4b6a04d8734c141a04e651fc8c23932b1f23 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/lib/timestamp.c 1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/38300/1
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c index 7c7210c..d2206f6 100644 --- a/src/lib/timestamp.c +++ b/src/lib/timestamp.c @@ -130,9 +130,6 @@ tse->entry_id = id; tse->entry_stamp = ts_time - ts_table->base_time;
- if (CONFIG(TIMESTAMPS_ON_CONSOLE)) - printk(BIOS_SPEW, "Timestamp - %s: %llu\n", timestamp_name(id), ts_time); - if (ts_table->num_entries == ts_table->max_entries) printk(BIOS_ERR, "ERROR: Timestamp table full\n"); } @@ -152,6 +149,9 @@ }
timestamp_add_table_entry(ts_table, id, ts_time); + + if (CONFIG(TIMESTAMPS_ON_CONSOLE)) + printk(BIOS_INFO, "Timestamp - %s: %llu\n", timestamp_name(id), ts_time); }
void timestamp_add_now(enum timestamp_id id)
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38300 )
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38300/1/src/lib/timestamp.c File src/lib/timestamp.c:
https://review.coreboot.org/c/coreboot/+/38300/1/src/lib/timestamp.c@131 PS1, Line 131: tse->entry_stamp = ts_time - ts_table->base_time; If I visualize this correctly, timestamps prior to CBMEM coming online will have ts_table->base_time subtracted twice?
https://review.coreboot.org/c/coreboot/+/38300/1/src/lib/timestamp.c@154 PS1, Line 154: printk(BIOS_INFO, "Timestamp - %s: %llu\n", timestamp_name(id), ts_time); If we define this to show raw/absolute timestamp, we could print this in !ts_table case too, and move up a couple lines. It is currently confusing that cbmem -c and cbmem -t will not have same output due to ts_table->base_time.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38300 )
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38300 )
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
Patch Set 1:
(1 comment)
Maybe two commits?
https://review.coreboot.org/c/coreboot/+/38300/1/src/lib/timestamp.c File src/lib/timestamp.c:
https://review.coreboot.org/c/coreboot/+/38300/1/src/lib/timestamp.c@154 PS1, Line 154: printk(BIOS_INFO, "Timestamp - %s: %llu\n", timestamp_name(id), ts_time);
If we define this to show raw/absolute timestamp, we could print this in !ts_table case too, and mov […]
The Kconfig option description needs to be updated accordingly.
Hello Julius Werner, Paul Menzel, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38300
to look at the new patch set (#2).
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour
As logging is guarded by Kconfig, increase the level from BIOS_SPEW to BIOS_INFO.
The original callsite inside timestamp_add_table_entry() was also called when syncing from timestamps from .bss to CBMEM. We should not reprint the values then.
Change-Id: I72ca4b6a04d8734c141a04e651fc8c23932b1f23 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/Kconfig M src/lib/timestamp.c 2 files changed, 4 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/38300/2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38300 )
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38300/1/src/lib/timestamp.c File src/lib/timestamp.c:
https://review.coreboot.org/c/coreboot/+/38300/1/src/lib/timestamp.c@131 PS1, Line 131: tse->entry_stamp = ts_time - ts_table->base_time;
If I visualize this correctly, timestamps prior to CBMEM coming online will have ts_table->base_time […]
Ack
https://review.coreboot.org/c/coreboot/+/38300/1/src/lib/timestamp.c@154 PS1, Line 154: printk(BIOS_INFO, "Timestamp - %s: %llu\n", timestamp_name(id), ts_time);
The Kconfig option description needs to be updated accordingly.
Done
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38300 )
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
Patch Set 2: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38300 )
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38300 )
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
Patch Set 2: Code-Review+1
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38300 )
Change subject: timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour ......................................................................
timestamps: Fix TIMESTAMPS_ON_CONSOLE behaviour
As logging is guarded by Kconfig, increase the level from BIOS_SPEW to BIOS_INFO.
The original callsite inside timestamp_add_table_entry() was also called when syncing from timestamps from .bss to CBMEM. We should not reprint the values then.
Change-Id: I72ca4b6a04d8734c141a04e651fc8c23932b1f23 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38300 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Martin Roth martinroth@google.com Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M src/Kconfig M src/lib/timestamp.c 2 files changed, 4 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved Nico Huber: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve
diff --git a/src/Kconfig b/src/Kconfig index 762cf89..9118914 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -224,7 +224,7 @@ default n depends on COLLECT_TIMESTAMPS help - Print the timestamps to the debug console if enabled at level spew. + Print the timestamps to the debug console if enabled at level info.
config USE_BLOBS bool "Allow use of binary-only repository" diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c index 7c7210c..d2206f6 100644 --- a/src/lib/timestamp.c +++ b/src/lib/timestamp.c @@ -130,9 +130,6 @@ tse->entry_id = id; tse->entry_stamp = ts_time - ts_table->base_time;
- if (CONFIG(TIMESTAMPS_ON_CONSOLE)) - printk(BIOS_SPEW, "Timestamp - %s: %llu\n", timestamp_name(id), ts_time); - if (ts_table->num_entries == ts_table->max_entries) printk(BIOS_ERR, "ERROR: Timestamp table full\n"); } @@ -152,6 +149,9 @@ }
timestamp_add_table_entry(ts_table, id, ts_time); + + if (CONFIG(TIMESTAMPS_ON_CONSOLE)) + printk(BIOS_INFO, "Timestamp - %s: %llu\n", timestamp_name(id), ts_time); }
void timestamp_add_now(enum timestamp_id id)