ipc4: helper: remove false warning#10605
Conversation
wjablon1
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
|
Nice fix. The previous behavior was indeed confusing because the warning appeared in the logs before the firmware actually checked whether the module was in the loadable set. |
There was a problem hiding this comment.
Pull request overview
This PR removes a false warning emitted when checking whether an IPC4 component driver is already registered, by splitting the lookup into a non-logging search helper and a logging wrapper.
Changes:
- Refactored driver lookup into
ipc4_search_for_drv()(no warning) andipc4_get_drv()(warns if not found). - Updated the “already registered components” check to use the non-logging search helper.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/ipc/ipc4/helper.c
Outdated
| /* Check already registered components */ | ||
| drv = ipc4_get_drv(&mod->uuid); | ||
| drv = ipc4_search_for_drv(&mod->uuid); | ||
|
|
There was a problem hiding this comment.
ipc4_get_comp_drv() now uses ipc4_search_for_drv() for the initial lookup unconditionally. When CONFIG_LIBRARY_MANAGER is disabled (or for non-loadable/base modules where no registration attempt follows), a missing driver will now fail silently because ipc4_get_comp_drv()/comp_new_ipc4() return NULL without logging. Consider using ipc4_search_for_drv() only in the pre-registration check under #if CONFIG_LIBRARY_MANAGER, and keep ipc4_get_drv() (or add an explicit tr_err/tr_warn) for the non-library-manager path so missing base drivers still produce a diagnostic.
| __cold static const struct comp_driver *ipc4_get_drv(const void *uuid) | ||
| { | ||
| const struct comp_driver *drv = ipc4_search_for_drv(uuid); | ||
| const struct sof_uuid *const __maybe_unused sof_uuid = (const struct sof_uuid *)uuid; |
There was a problem hiding this comment.
please call assert_can_be_cold() here too, since this function is also called from ipc4_chain_manager_create()
src/ipc/ipc4/helper.c
Outdated
| const struct sof_uuid *const __maybe_unused sof_uuid = (const struct sof_uuid *)uuid; | ||
|
|
||
| if (!drv) | ||
| tr_warn(&comp_tr, |
There was a problem hiding this comment.
now this is a critical failure, so we should change it to tr_err()
wjablon1
force-pushed
the
ipc4_wrn_fix
branch
2 times, most recently
from
d9bf678 to
a66bf43
Compare
Function get_driver is once called just for checking if a component is already registered prior to registration. This leads to a false warning. This change fixes this by adding a separate function that does not issue the warning and replacing the warning with error. Signed-off-by: Wojciech Jablonski <wojciech.jablonski@intel.com>
Function get_driver is once called just for checking if a component is already registered prior to registration. This leads to a false warning. This change fixes this by adding a separate function that does not issue the warning.