Attention is currently required from: Boris Mittelberg, Caveh Jalali, Jayvik Desai, Julius Werner, Kapil Porwal, Paul Menzel.
Subrata Banik has posted comments on this change by Jayvik Desai. ( https://review.coreboot.org/c/coreboot/+/85326?usp=email )
Change subject: ec/google/chromeec: Add debug timestamp for host EC commands ......................................................................
Patch Set 11:
(1 comment)
File src/ec/google/chromeec/ec_lpc.c:
https://review.coreboot.org/c/coreboot/+/85326/comment/6fad74b3_f531a5c8?usp... : PS9, Line 286: printk
So would you want to use the same timestamp index for each command? Because I think that would look very confusing, trying to figure out which timestamp came from what command. Isn't a log that can display the command code right next to the time much better for this?
I was thinking we could keep a data structure that maps EC host commands to timestamps. That way, we'd always know which timestamp belongs to which EC host command.
I'm also not sure it's a good idea in general to add Kconfig-conditional timestamps that only turn up in certain debug builds.
We already have a similar feature for FSP. When we build FSP with a specific flag, it adds more timestamps that aren't needed during regular boot.
Here's how it works:
1. When debug Kconfig is disabled, the timestamps look like this:
``` timestamp for foo() ---- 100ms timestamp for foo_1() ---- 300ms ```
So, the total time spent between foo() and foo_1() is 200ms.
2. When debug Kconfig is enabled, the timestamps look like this:
``` timestamp for foo() ---- 100ms timestamp for func_1() ---- 110ms timestamp for func_2() ---- 150ms timestamp for func_3() ---- 210ms timestamp for foo_1() ---- 300ms ```
This gives us a more detailed breakdown of the time spent between foo() and foo_1(), which wasn't possible with the first method.
Since the timestamp system always measures the time between two timestamps, this would shift other times around in unexpected ways when you turn on the option. I think the timestamp system is best for a consistent, high-level overview of which general parts of the boot flow take how much time, and then when drilling down into detail adding extra log messages that can be conditionally enabled is the better tool.