Attention is currently required from: Bora Guvendik, Furquan Shaikh, Tim Wawrzynczak, Subrata Banik, Angel Pons, Aaron Durbin. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51445 )
Change subject: timestamp: Add new helper functions ......................................................................
Patch Set 8:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51445/comment/056c970b_43a542cb PS3, Line 7: Add helper fucntions
Hi Furquan, […]
Hi Bora,
Not sure if you've heard yet, but Furquan isn't working on coreboot anymore. I believe +Tim is taking over some of his former work, although I'm not sure if he has context on this effort. I can also try to help although I'm not familiar with your platform in particular.
You are right, of course, that the formula should be `base_delta * ts->tick_freq_mhz / base_freq_mhz`, not `base_delta * base_freq_mhz / ts->tick_freq_mhz`.
However, looking at this again with a few months distance, I'm actually not so sure anymore why we even need all this rewind() stuff. Fundamentally, your reason for doing that was just that some of the new timestamps you were trying to add are chronologically *before* the base_time, right? But why is that such a problem? Alternatively, we could just redefine (struct timestamp_entry).entry_stamp from uint64_t to int64_t, and declare that it should be allowed to be negative. Then I don't think you need any API changes here and just need to modify utils/cbmem to be aware of that case, and "rebase" all of those timestamps to the lowest one in the list when displaying them. I think that would require the minimum amount of extra logic in firmware code because you can basically just use timestamp_add() and pass a (potentially) negative number. (You may still need to do frequency conversions, that's where my other suggestion would come in of just rewrite x86's timestamp_get() to immediately convert to microseconds -- all other architecture ports already do it that way anyway, and then we'll never have any of this frequency confusion anymore.)
What do you think?