-
Notifications
You must be signed in to change notification settings - Fork 321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trace: Remove ctx dependency from current logging system #8822
Conversation
49fc466
to
32fadb3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the last commit in the series is huge, impossible to review (it seems trivial, but still would be good to at least have a chance), and has a high breakage probability - almost any other PR, that gets merged before this one would break it.
What I personally would do instead, I'd add a new interface (maybe trace_*()
or log_*()
) and gradually move files over to it - one subsystem at a time, one PR for IPC, one PR for schedulers, etc. Then after all are migrated, you just remove the tr_*()
API
This would simplify the merge, i.e. not a big bang and reduced change of conflict races with other PRs. |
32fadb3
to
d72191f
Compare
Can you please try to address at least some of the checkpatch.pl errors: for example this:
I think it is relevant. |
c8bf2a4
to
ac05db0
Compare
@kv2019i @lgirdwood @lyakh @dbaluta , codestyle problem is origin issue, if we want to fix, we can fix with another PR, this PR does not change those code actually. Regarding the fuzzy error, it may related with #8632 patch merge, please notice. |
@dbaluta , could you try with this PR for IPC3 logging? I don't want make pause here due to CNY holiday. |
@LaurentiuM1234 can you please try to see if logging still works after this PR? @btian1 can you please confirm the testing conditions? Is it with XTOS right? |
tested on QM with Zephyr hybrid build (and CS42888 tplg) and all's good. For reference, I'm attaching a snippet of how the logs look like w/ this patch. I'm assuming this behaves the same as XTOS since they use the same infrastructure. |
@LaurentiuM1234 can you post a snippet of the log w/o the patch. It looks like now the component is shown as "Unknown". I wonder if w/o the patch the real component is correctly shown or there is an earlier bug. |
sorry for the long delay. Attaching snippet (8QM, WM8690, SOF main (557e581)): |
@btian1 looks like with your patch we lose ability to get the component for which the log was generated. Is this intended right? If so then maybe we can remove that column from sof-logger output. What's your take on this? |
Thanks, @dbaluta @LaurentiuM1234 , due to UID was removed, then sof-logger unable to find each component name for logging, as you suggested, I removed this part print from sof-logger, please try with full PR with rebuild sof-logger again, it should be fine now with/without this patch, the log should be same. |
Dai class level logging is not used by dai legacy module, remove it. Signed-off-by: Baofeng Tian <[email protected]>
This is part of job for remove ipc3 logging ctx dependency. Signed-off-by: Baofeng Tian <[email protected]>
This is part of remove ipc3 logging context dependency. Signed-off-by: Baofeng Tian <[email protected]>
This interface will be used to replace existing interface in trace header file, compared between this and existing interace, the difference is this interace does not have context dependency. Signed-off-by: Baofeng Tian <[email protected]>
Replace it with new defined logging interface without context dependency. Signed-off-by: Baofeng Tian <[email protected]>
Replace it with new defined logging interface without context dependency. Signed-off-by: Baofeng Tian <[email protected]>
Replace it with new defined logging interface without context dependency. Signed-off-by: Baofeng Tian <[email protected]>
Replace it with new defined logging interface without context dependency. Signed-off-by: Baofeng Tian <[email protected]>
Replace it with new defined logging interface without context dependency. Signed-off-by: Baofeng Tian <[email protected]>
Remove unused trace definition, since it is already replaced by non-zephyr trace definition. Signed-off-by: Baofeng Tian <[email protected]>
Remove context dependency from four atomic trace definition. Signed-off-by: Baofeng Tian <[email protected]>
31d2dc9
to
e89eafc
Compare
Remove it and change source file accordingly. Signed-off-by: Baofeng Tian <[email protected]>
Due to previous change, there is no valid component uuid for sof logger, so component name loopup will return unknown, remove this part. Signed-off-by: Baofeng Tian <[email protected]>
This is a preparation for SOF logging to zephyr interface, IPC3 logging need context support, it is not must, and verified with #8725 , so remove these dependencies and will use another new PR to make SOF with zephyr logging interface.