ipc4: update logging backend management #10133
Conversation
tmleman
requested review from
dbaluta,
kv2019i,
lbetlej,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR updates the logging backend management in the IPC4 logging subsystem by migrating from direct backend activation/deactivation to using higher-level logging control functions. The changes align with Zephyr's recommended practices for managing logging backends and provide enhanced control over logging configurations.
- Replaces
log_backend_activate/log_backend_deactivatewithlog_backend_enable/log_backend_disable - Adds log level configuration support through
CONFIG_SOF_LOG_LEVELparameter - Updates header includes to use the logging control API
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Some comments on the why part. Code itself looks good.
| #include <rtos/kernel.h> | ||
| #if !CONFIG_LIBRARY | ||
| #include <zephyr/logging/log_backend.h> | ||
| #include <zephyr/logging/log.h> |
There was a problem hiding this comment.
@tmleman The commit has a certain AI tint to it :) The "why this PR?" was left a bit open still. I guess this is not just refactoring, but starting to use SOF_LOG_LEVEL. Is there any concrete change from this PR? I think we set SOF_LOG_LEVEL for SOF side code already, but does this impact Zephyr side logs?
There was a problem hiding this comment.
@kv2019i I have rephrased the commit description and provided a better justification for these changes.
I guess this is not just refactoring
You're right, it's not just a refactor; it's a fix for an unseen error. And since it's unseen, I didn't mention it ;P
tmleman
force-pushed
the
topic/upstream/pr/ipc4/logging/fix_api_use
branch
from
b059584 to
da1a9d6
Compare
This patch addresses an issue with the logging backend initialization in the ipc4/logging.c file. The previous implementation used log_backend_activate, which does not fully enable the backend for log collection. This was initially masked by the fact that backends are currently autostarted, but disabling backend autostart would reveal that logs are not being collected as expected. By switching to log_backend_enable, the backend is correctly initialized, ensuring logs are collected reliably. The use of CONFIG_SOF_LOG_LEVEL as a parameter does not impact runtime filtering, as it is not enabled, but it aligns with the current logging configuration practices. This update fixes the logging backend initialization issue, ensuring reliable log collection within the SOF firmware, and prepares the system for scenarios where backend autostart might be disabled, thereby addressing a hidden problem in the logging setup. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
tmleman
requested review from
abonislawski,
marcinszkudlinski,
serhiy-katsyuba-intel and
softwarecki
4 participants
This patch addresses an issue with the logging backend initialization in the
ipc4/logging.cfile. The previous implementation usedlog_backend_activate, which does not fully enable the backend for log collection. This was initially masked by the fact that backends are currently autostarted, but disabling backend autostart would reveal that logs are not being collected as expected. By switching tolog_backend_enable, the backend is correctly initialized, ensuring logs are collected reliably.The use of
CONFIG_SOF_LOG_LEVELas a parameter does not impact runtime filtering, as it is not enabled, but it aligns with the current logging configuration practices.This update fixes the logging backend initialization issue, ensuring reliable log collection within the SOF firmware, and prepares the system for scenarios where backend autostart might be disabled, thereby addressing a hidden problem in the logging setup.