Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical build and notification-path issues, plus lifecycle and syscall-access defects, block approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (4)
What changed in this PR
Adds a Zephyr-native queue-based notification mechanism for KPB userspace clients.
Changes:
- Registers generated KPB syscall headers.
- Adds notifier APIs, queue state, and worker-thread handling.
- Adds and enables the
KPB_CLI_Qconfiguration option.
| File | Summary |
|---|---|
zephyr/CMakeLists.txt |
Registers KPB syscall headers. |
src/include/sof/audio/kpb.h |
Adds queue state and notifier API declarations. |
src/audio/kpb.c |
Implements queue processing, syscalls, and lifecycle management. |
src/audio/Kconfig |
Adds and enables queue-based KPB configuration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| static struct k_queue *kpb_q; | ||
| static struct k_thread kpb_thread; | ||
| K_KERNEL_STACK_DEFINE(kpb_stack, 4096); |
There was a problem hiding this comment.
hm, I was assuming that multiple KPB instances don't make sense, but maybe I was wrong. Let me fix that.
There was a problem hiding this comment.
alright, checked that: currently neither the notifier nor the AMS version support multiple KPB instances. The notifier version only supports a single handler per type (as in NOTIFIER_ID_KPB_CLIENT_EVT), and AMS could support routing to multiple handlers in principle, but current clients don't support it either: e.g.
sof/src/samples/audio/detect_test.c
Line 163 in b6c6a05
| #if CONFIG_KPB_CLI_Q | ||
| void kpb_notifier_schedule(struct kpb_client *cli); | ||
| #if defined(__ZEPHYR__) && defined(CONFIG_SOF_FULL_ZEPHYR_APPLICATION) | ||
| #include <sof/compiler_attributes.h> | ||
| __syscall void kpb_notifier_init(struct comp_dev *dev, struct kpb_client *cli); |
KPB currently can use SOF notifications and AMS for triggering, but neither of them is currently accessible from userspace. Use a Zephyr native queue API instead. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
@lgirdwood the API adds 2 functions: |


KPB currently can use SOF notifications and AMS for triggering, but neither of them is currently accessible from userspace. Use a Zephyr native queue API instead.