From c3db875f3b79e9cb50a6792b64d6cb49c7068075 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Tue, 31 Mar 2026 14:45:40 +0300 Subject: [PATCH 1/2] ipc: ipc4: helper: drop redundant locking in ipc4_search_for_drv() Drop the IRQ disable/enable in ipc4_search_for_drv(). The driver list is only modified at FW boot and when a new driver is registered at runtime via SOF_IPC4_GLB_LOAD_LIBRARY IPC. ipc4_search_for_drv() is only used when processing IPC messages. As IPC processing is serialized, it is not possible for the driver list to be modified concurrently with a call to ipc4_search_for_drv(). Signed-off-by: Kai Vehmanen --- src/ipc/ipc4/helper.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 8065e24e70f6..19a51de12949 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -1142,12 +1142,9 @@ __cold static const struct comp_driver *ipc4_search_for_drv(const void *uuid) struct list_item *clist; const struct comp_driver *drv = NULL; struct comp_driver_info *info; - uint32_t flags; assert_can_be_cold(); - irq_local_disable(flags); - /* search driver list with UUID */ list_for_item(clist, &drivers->list) { info = container_of(clist, struct comp_driver_info, @@ -1162,7 +1159,6 @@ __cold static const struct comp_driver *ipc4_search_for_drv(const void *uuid) } } - irq_local_enable(flags); return drv; } From 4fcddd78ae841fc7015417a3b44a371f28be4453 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Mon, 15 Jun 2026 18:28:49 +0300 Subject: [PATCH 2/2] audio: component: drop redundant locking in driver list The component driver list is only modified at FW boot and at runtime when a library is loaded. At boot, module init runs serially on the primary core (Zephyr SYS_INIT at APPLICATION level, before secondary cores are started; .initcall walked on a single core for XTOS). At runtime, registration happens from the IPC thread, which is serialized with only one command processed at a time. These two phases never overlap, as IPC message processing only begins after boot completes, so the list can never be modified concurrently. The lock was also already incoherent: comp_set_adapter_ops() iterate the list without holding the lock, so it provided no real mutual exclusion. Drop the spinlock from comp_register() and comp_unregister(), and from the UUID search in the IPC3 get_drv() reader. Remove the now-unused lock field from struct comp_driver_list and its initialization. Signed-off-by: Kai Vehmanen --- src/audio/component.c | 14 +++++++------- src/include/sof/audio/component_ext.h | 1 - src/ipc/ipc3/helper.c | 10 ++++------ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/audio/component.c b/src/audio/component.c index da597023cf73..7083fbde1d66 100644 --- a/src/audio/component.c +++ b/src/audio/component.c @@ -45,11 +45,14 @@ DECLARE_TR_CTX(comp_tr, SOF_UUID(component_uuid), LOG_LEVEL_INFO); int comp_register(struct comp_driver_info *drv) { struct comp_driver_list *drivers = comp_drivers_get(); - k_spinlock_key_t key; - key = k_spin_lock(&drivers->lock); + /* + * No locking needed: the driver list is only modified at FW boot, + * where module init runs serially on the primary core, and at + * runtime from the serialized IPC thread (library load). These + * never overlap, so concurrent modification is not possible. + */ list_item_prepend(&drv->list, &drivers->list); - k_spin_unlock(&drivers->lock, key); return 0; } @@ -57,11 +60,9 @@ int comp_register(struct comp_driver_info *drv) void comp_unregister(struct comp_driver_info *drv) { struct comp_driver_list *drivers = comp_drivers_get(); - k_spinlock_key_t key; - key = k_spin_lock(&drivers->lock); + /* see comp_register() on why no locking is needed */ list_item_del(&drv->list); - k_spin_unlock(&drivers->lock, key); } int comp_set_adapter_ops(const struct comp_driver *drv, const struct module_interface *ops) @@ -190,7 +191,6 @@ void sys_comp_init(struct sof *sof) sof->comp_drivers = platform_shared_get(&cd, sizeof(cd)); list_init(&sof->comp_drivers->list); - k_spinlock_init(&sof->comp_drivers->lock); } void comp_get_copy_limits(struct comp_buffer *source, diff --git a/src/include/sof/audio/component_ext.h b/src/include/sof/audio/component_ext.h index d2bbf87a7764..70324175fea8 100644 --- a/src/include/sof/audio/component_ext.h +++ b/src/include/sof/audio/component_ext.h @@ -26,7 +26,6 @@ /** \brief Holds list of registered components' drivers */ struct comp_driver_list { struct list_item list; /**< list of component drivers */ - struct k_spinlock lock; /**< list lock */ }; /** \brief Retrieves the component device buffer list. */ diff --git a/src/ipc/ipc3/helper.c b/src/ipc/ipc3/helper.c index 149ff6008a1a..e65284646079 100644 --- a/src/ipc/ipc3/helper.c +++ b/src/ipc/ipc3/helper.c @@ -81,7 +81,6 @@ static const struct comp_driver *get_drv(struct sof_ipc_comp *comp) struct comp_driver_info *info; struct sof_ipc_comp_ext *comp_ext; uintptr_t offset; - k_spinlock_key_t key; /* do we have extended data ? */ if (!comp->ext_data_length) { @@ -127,9 +126,10 @@ static const struct comp_driver *get_drv(struct sof_ipc_comp *comp) goto out; } - /* search driver list with UUID */ - key = k_spin_lock(&drivers->lock); - + /* + * search driver list with UUID; no locking needed as the driver + * list is only modified at boot and from the serialized IPC thread + */ list_for_item(clist, &drivers->list) { info = container_of(clist, struct comp_driver_info, list); @@ -148,8 +148,6 @@ static const struct comp_driver *get_drv(struct sof_ipc_comp *comp) *(uint32_t *)(&comp_ext->uuid[8]), *(uint32_t *)(&comp_ext->uuid[12])); - k_spin_unlock(&drivers->lock, key); - out: if (drv) tr_dbg(&comp_tr, "get_drv(), found driver type %d, uuid %pU",