-
Notifications
You must be signed in to change notification settings - Fork 349
module: add some kconfigs and do some cleanup #10085
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
API_CALL only used to wrap Cadence API calls so move to correct header. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Currently max number of connected sources and sinks is hard coded. Make this Kconfig to support different connections counts and validate the count is within limits. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Currently max blob size is hard coded. make it a kconfig so it can be tuned for needs. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
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 replaces hard-coded module limits and blob size constants with Kconfig-driven values, adds validation for connection counts, and cleans up duplicate macro definitions.
- Introduce
CONFIG_MODULE_MAX_CONNECTIONSandCONFIG_MODULE_MAX_BLOB_SIZEKconfig options - Update array sizes and runtime checks to use the new configurable limits
- Remove
MAX_BLOB_SIZEandMODULE_MAX_SOURCESmacros and relocateAPI_CALLtocadence.h
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/include/sof/audio/module_adapter/module/generic.h | Removed hard-coded size macros and duplicate API_CALL definition |
| src/include/sof/audio/module_adapter/module/cadence.h | Added API_CALL macro |
| src/include/module/module/base.h | Switched sink/source arrays to CONFIG_MODULE_MAX_CONNECTIONS |
| src/audio/module_adapter/module_adapter.c | Added runtime check for exceeding connection limits |
| src/audio/module_adapter/module/generic.c | Replaced MAX_BLOB_SIZE with CONFIG_MODULE_MAX_BLOB_SIZE |
| src/audio/Kconfig | Added MODULE_MAX_CONNECTIONS and MODULE_MAX_BLOB_SIZE entries |
Comments suppressed due to low confidence (3)
src/audio/module_adapter/module_adapter.c:277
- [nitpick] The error message could be clearer by naming both values explicitly (e.g.,
sinks=%u, sources=%u) so it’s unambiguous which counts exceeded the limit.
comp_err(dev, "module_adapter_prepare(): too many connected sinks %d or sources %d!",
src/audio/module_adapter/module_adapter.c:275
- This new validation branch for too many connections should be covered by unit tests to verify that
module_adapter_prepare()fails correctly when exceedingCONFIG_MODULE_MAX_CONNECTIONS.
if (mod->num_of_sources > CONFIG_MODULE_MAX_CONNECTIONS ||
src/include/module/module/base.h:103
- The comment still references the old
MODULE_MAX_SOURCES; update it to mentionCONFIG_MODULE_MAX_CONNECTIONSto keep documentation in sync with the code change.
/* sink and source handlers for the module */
| extern xa_codec_func_t xa_vorbis_dec; | ||
| extern xa_codec_func_t xa_src_pp; | ||
|
|
||
| #define API_CALL(cd, cmd, sub_cmd, value, ret) \ |
Copilot
AI
Jul 1, 2025
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 API_CALL macro is now defined here and was removed from generic.h, but duplicating it in multiple module headers can lead to drift. Consider moving this macro to a common header (e.g., module_api.h) to keep definitions centralized.
Add a descriptive comment and align some doxygen comments. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Add more flexibility to module configuration and a little clean up.