-
Notifications
You must be signed in to change notification settings - Fork 349
lib_manager: modules: Set of improvements for loadable module #10098
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
Conversation
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.
Pull Request Overview
This PR refactors how loadable modules are initialized and started in the library manager, adds tighter ID masking, and extends adapter creation to accept private data pointers.
- Unified module allocation and system agent invocation into
lib_manager_module_create - Introduced
module_adapter_new_extto pass module-specific private data - Changed entry-point parameters to
uintptr_tand updated system agent function signatures
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/library_manager/lib_manager.c | Moved allocation and agent calls into lib_manager_module_create, updated free logic and signatures |
| src/include/sof/lib_manager.h | Added missing headers and refined LIB_MANAGER_GET_LIB_ID |
| src/include/sof/audio/module_adapter/module/generic.h | Declared module_adapter_new_ext |
| src/include/sof/audio/module_adapter/library/native_system_agent.h | Updated native_system_agent_start signature |
| src/include/sof/audio/module_adapter/iadk/system_agent.h | Updated system_agent_start signature |
| src/audio/module_adapter/module_adapter.c | Added module_adapter_new_ext implementation |
| src/audio/module_adapter/module/modules.c | Removed old modules_init allocation logic |
| src/audio/module_adapter/library/native_system_agent.c | Updated return handling and interface extraction |
| src/audio/module_adapter/iadk/system_agent.cpp | Changed entry-point type to uintptr_t |
Comments suppressed due to low confidence (2)
src/library_manager/lib_manager.c:333
- Add a
@returndescription to this doc block explaining the returned base address or error conditions when allocation fails.
* param[in] ipc_specific_config - ipc4 base configuration
src/include/sof/audio/module_adapter/module/generic.h:212
- The header lacks a comment for
module_adapter_new_ext; please document themod_privparameter and its effect on the module adapter.
struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
|
SOFCI TEST |
|
SOFCI TEST |
lgirdwood
left a comment
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.
Just one open for me to differentiate the new call from the existing call in the doxygen.
src/include/sof/audio/module_adapter/library/native_system_agent.h
Outdated
Show resolved
Hide resolved
a512d17 to
03306c5
Compare
|
Two CI failures on PTL https://sof-ci.01.org/sofpr/PR10098/build14005/devicetest/index.html - I don't understand how this can be related, but of those two the former I don't remember seeing recently. |
Will run again, to rule out - cant see how related to this PR. |
|
SOFCI TEST |
lyakh
left a comment
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.
I think we should never cast-away const. ("never" is a strong word, in reality I mean "except possibly in very-very few exception cases"). It would be good to have a static code analysis tool to have capture such attempts...
Change the signature of the native_system_agent_start function to match with the system_agent_start function. The function returns an error code and a pointer to the module interface is returned via the parameter. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Change type of the entry_point parameter in the native_system_agent_start and system_agent_start functions to uintptr_t which allows safe pointer passing. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Add missing include headers for list and component. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Create a function module_adapter_new_ext that allows to assign a pointer to the private data of the module for which the adapter is being created. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Add a bitmask to the LIB_MANAGER_GET_LIB_ID macro to discard the instance identifier placed on the high order bits of the component id. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
…unctions Move module allocation and system agent call from modules_init to lib_manager_module_create to unify the module loading process. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Move module allocation and system agent call from modules_init to lib_manager_module_create to unify the module loading process.
Add a bitmask to the LIB_MANAGER_GET_LIB_ID macro to discard the instance identifier placed on the high order bits of the component id.
Create a function
module_adapter_new_extthat allows to assign a pointer to the private data of the module for which the adapter is being created.Add missing include headers for list and component in lib_manager.
Change type of the
entry_pointparameter in thenative_system_agent_startandsystem_agent_startfunctions to uintptr_t which allows safe pointer passing.Change signature of the
native_system_agent_startfunction to match with thesystem_agent_startfunction. The function returns an error code and a pointer to the module interface is returned via the parameter.