sof: replace log calls with zephyr logging api #5707
Conversation
aiChaoSONG
requested review from
akloniex,
mrajwa,
libinyang,
bardliao,
marcinszkudlinski,
lyakh,
dbaluta,
cujomalainey,
chiang831,
johnylin76,
singalsu,
lgirdwood,
plbossart,
mmaka1 and
lbetlej
as code owners
|
Looks quite reasonable, but this doesn't work on platforms which haven't migrated to Zephyr yet (we're working on it, it's not done though). So I cannot in good conscience approve this PR right now. Also some constant string (format string) concatenation and extra arguments to give further info in the comp_info and related logs? |
This PR has no impact on existing XTOS implementation, you can keep using XTOS. Once you have done the migration, it should work naturally. If I only do this for Intel platforms, the code will not compile with Zephyr on NXP platforms. |
|
Can one of the admins verify this patch? |
|
@lgirdwood @lyakh @kv2019i ping? |
|
Do I understand it correctly that with this PR SOF traces are redirected to the Zephyr log, so that sof-logger can no longer read them? |
|
@lyakh Yes, exactly, and @keqiaozhang is working on CI to enable cavstool.py script. |
@aiChaoSONG was it cavstool.py that had problems like (1) repeating log fragments, making it unusable in most cases, (2) hitting the CPU-DSP memory window as-fast-as-possible which also sometimes was triggering DSP hardware issues? If that's exactly the tool that had those problems and if those problems are still there, perhaps we first should fix them? I understand that we need to unify with Zephyr to only have one logging infrastructure and to have all the logging available via one interface, but at least now sof-logger is mostly usable and you can always look at etrace for Zephyr specific logs. Unless I'm wrong and those issues are already fixed, switching over would make debugging much more difficult again |
|
@lyakh I only tested lightweight playback and capture cases, and don't see the issues you mentioned, I only see slight log drop in the message. Maybe we can run stress test after this tool enabled in CI. |
@aiChaoSONG @lyakh This is why we need the Kconfig. Default is what we are doing today, Select "y" means use the Zephyr logging. |
aiChaoSONG
force-pushed
the
use_zph_log
branch
2 times, most recently
from
78c206e
to
d4e3eb7
Compare
|
@lgirdwood The config If and only if Zephyr RTOS is used and |
|
Test result with ZEPHYR_LOG=y: https://sof-ci.01.org/sofpr/PR5707/build136/devicetest/ |
|
@lgirdwood Please wait a few hours until the test is done. thanks. now |
The log context in zephyr is per file or module. To use zephyr logging api, LOG_MODULE_REGISTER is used to register a log context, LOG_MODULE_DECLARE is used to refer to the registered context. For function in header file, LOG_MODULE_DECLARE should be used within that function to avoid context collapse, a condition one source file have multiple context registered or declared. Signed-off-by: Chao Song <[email protected]>
|
@lgirdwood @kv2019i @lyakh Thanks for the review. Test Summary:
|
|
At first sight this looks like the most likely suspect for these brand new (and non-Zephyr) hard kernel lockups: |
The log context in zephyr is per file or module.
To use zephyr logging api, LOG_MODULE_REGISTER is used
to register a log context, LOG_MODULE_DECLARE is used
to refer to the registered context.
For function in header file, LOG_MODULE_DECLARE should
be used within that function to avoid context collapse,
a condition one source file have multiple context
registered or declared.
Signed-off-by: Chao Song [email protected]
This PR is moved from #5411, because there are a lot of reviews that not valid anymore, move it here to avoid misleading.