Tracing fixes #4327
Tracing fixes #4327
Conversation
... when there are too many % characters in format string. Spotted by routine valgrind run. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
It's not long and more consistent. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Connect the DMA initialization dots. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Fixes commit 6c14e76 ("trace: Log FW ABI and hash numbers"). In June 2020, PR thesofproject#3195 added a tr_info() "banner" immediately after to make sure tracing works. That commit included a likely misunderstood trace_flush() call immediately after the tr_info(). The very poorly named trace_flush() function sounds like earlier scheduling of something that would the same later anyway but that's absolutely not what it does. Instead it copies pending DMA traces to the shared mailbox. This was most likely why the FW ABI banner is randomly duplicated in the etrace from time to time, see for instance: https://sof-ci.01.org/linuxpr/PR2954/build5823/devicetest/?model=BYT_MB_NOCODEC&testcase=test-speaker https://sof-ci.01.org/softestpr/PR666/build721/devicetest/?model=TGLH_RVP_HDA&testcase=check-sof-logger Note this test failure is unrelated. For now this CI does not read etrace unless there's a failure. In August 2020, commit 67a0a69 ("trace: Trace initial message as error logs") upgraded this banner from tr_info() to tr_err(). That made sure it gets to both the mailbox and the DMA trace but it didn't remove the trace_flush(). Remove it now. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Pure rename, zero functional change. The very poorly named trace_flush() function sounds like earlier scheduling of something that would the same later anyway but that's absolutely not what it does. Instead it copies pending DMA traces to the shared mailbox. As an example, in June 2020, PR thesofproject#3195 commit 6c14e76 ("trace: Log FW ABI and hash numbers") added a tr_info() "banner" immediately after to make sure tracing works. That included a likely misunderstood trace_flush() call immediately after the tr_info(). Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Avoids duplication and potential divergence. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
The DMA trace is optional, the mailbox is not. This lets use the mailbox when initializing the DMA trace. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Avoids crashes when trying to use put_header() early. Learned the hard way because put_header() has also a timestamp argument. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Before: WARN task add 0xbe05ba30 <bad uid ptr 0> After: WARN task add 0xbe05ba30 <bad uid ptr 0x00000000> Signed-off-by: Marc Herbert <marc.herbert@intel.com>
|
Two platforms were unavailable in https://sof-ci.01.org/sofpr/PR4327/build9313/devicetest/ otherwise everything is green. The 2 checkpatch warnings in https://github.com/thesofproject/sof/pull/4327/checks?check_run_id=2789960845 are wrong:
|
marc-hb
requested review from
akloniex,
bkokoszx,
dbaluta,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
| lvl, \ | ||
| comp_class, \ | ||
| params, \ | ||
| n_params, \ |
There was a problem hiding this comment.
While it makes the macro readable, it does not belong to this commit as per the subject and message
There was a problem hiding this comment.
Thanks @ujfalusi for the very thorough review! Technically you're right: this is not just a comment change. In practice this macro parameter name is used only once in the macro definition, must be a compile-time constant and the macro itself is used in only one place in the entire code base right now. So the risk of conflict and possible consequences are either zero or extremely close to it and that's why I chose to save one extra commit for these 4 characters. As you've noticed the rest of the PR is carefully divided in 9 different commits with hopefully good commit messages.
Now if you insist... :-)
There was a problem hiding this comment.
I'm fine - CI has already run. Lets merge.
3 participants
Various fixes and some comments. Main changes:
dma-trace.c: remove misunderstood trace_flush() after FW ABI bannertrace: initialize DMA trace _after_ mailbox