* [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()
@ 2026-06-26 8:28 phucduc.bui
2026-06-26 8:29 ` [PATCH v4 1/5] ASoC: Intel: avs: Use guard() for locking phucduc.bui
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: phucduc.bui @ 2026-06-26 8:28 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Cezary Rojewski
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
Hi all,
This series converts mutex and spinlock handling in Intel ASoC drivers
to use guard()/scoped_guard() helpers.
Most changes are straightforward conversions intended to simplify lock
handling and cleanup paths, with no intended functional changes.
The series has been compile-tested only.
Link v1:
https://lore.kernel.org/all/20260611115901.80438-1-phucduc.bui@gmail.com/
Link v2:
https://lore.kernel.org/all/20260618110649.227062-1-phucduc.bui@gmail.com/
Link v3:
https://lore.kernel.org/all/CAABR9nGB9Z-caMwubqNAY1W9OwM6ZiFWqrqATJP9NtyNBbkL4Q@mail.gmail.com/
Changes in v2:
- Reduce the series from 20 patches to 4 patches.
- Drop smaller conversions that did not provide clear benefits.
Changes in v3:
- Drop the scoped_guard() conversion for the function that still used
a goto-based cleanup path.
- Move the __free(kfree) conversion into a separate patch.
Changes in v4:
- Add cleanup.h include where needed.
- Remove unnecessary variables.
- Reorder patches in the series.
Best regards,
Phuc
bui duc phuc (5):
ASoC: Intel: avs: Use guard() for locking
ASoC: Intel: avs: Use scoped_guard() for scoped locking
ASoC: intel: atom: Use __free(kfree) for stream pointer
ASoC: Intel: atom: Use guard() for locking
ASoC: Intel: atom: Use scoped_guard() for scoped locking
sound/soc/intel/atom/sst-atom-controls.c | 67 +++++++++-----------
sound/soc/intel/atom/sst-mfld-platform-pcm.c | 59 ++++++++---------
sound/soc/intel/atom/sst/sst_ipc.c | 5 +-
sound/soc/intel/atom/sst/sst_pvt.c | 9 ++-
sound/soc/intel/avs/apl.c | 8 +--
sound/soc/intel/avs/control.c | 7 +-
sound/soc/intel/avs/core.c | 4 +-
sound/soc/intel/avs/debug.h | 10 +--
sound/soc/intel/avs/debugfs.c | 19 +++---
sound/soc/intel/avs/ipc.c | 59 ++++++++---------
sound/soc/intel/avs/loader.c | 15 +++--
sound/soc/intel/avs/path.c | 30 +++------
sound/soc/intel/avs/utils.c | 45 +++++--------
13 files changed, 141 insertions(+), 196 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 1/5] ASoC: Intel: avs: Use guard() for locking
2026-06-26 8:28 [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() phucduc.bui
@ 2026-06-26 8:29 ` phucduc.bui
2026-06-26 8:29 ` [PATCH v4 2/5] ASoC: Intel: avs: Use scoped_guard() for scoped locking phucduc.bui
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: phucduc.bui @ 2026-06-26 8:29 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Cezary Rojewski
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
Clean up the code using guard() for spin & mutex locks.
Merely code refactoring, and no behavior change.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
Changes in v4:
- Add cleanup.h include where needed.
sound/soc/intel/avs/apl.c | 8 +++----
sound/soc/intel/avs/control.c | 7 ++----
sound/soc/intel/avs/core.c | 4 ++--
sound/soc/intel/avs/debug.h | 10 +++-----
sound/soc/intel/avs/ipc.c | 11 ++++-----
sound/soc/intel/avs/path.c | 30 +++++++----------------
sound/soc/intel/avs/utils.c | 45 +++++++++++++----------------------
7 files changed, 39 insertions(+), 76 deletions(-)
diff --git a/sound/soc/intel/avs/apl.c b/sound/soc/intel/avs/apl.c
index b922eeaba843..cf600e1c986e 100644
--- a/sound/soc/intel/avs/apl.c
+++ b/sound/soc/intel/avs/apl.c
@@ -6,6 +6,7 @@
// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
//
+#include <linux/cleanup.h>
#include <linux/devcoredump.h>
#include <linux/slab.h>
#include <sound/hdaudio_ext.h>
@@ -190,7 +191,7 @@ static bool avs_apl_lp_streaming(struct avs_dev *adev)
{
struct avs_path *path;
- spin_lock(&adev->path_list_lock);
+ guard(spinlock)(&adev->path_list_lock);
/* Any gateway without buffer allocated in LP area disqualifies D0IX. */
list_for_each_entry(path, &adev->path_list, node) {
struct avs_path_pipeline *ppl;
@@ -210,14 +211,11 @@ static bool avs_apl_lp_streaming(struct avs_dev *adev)
if (cfg->copier.dma_type == INVALID_OBJECT_ID)
continue;
- if (!mod->gtw_attrs.lp_buffer_alloc) {
- spin_unlock(&adev->path_list_lock);
+ if (!mod->gtw_attrs.lp_buffer_alloc)
return false;
- }
}
}
}
- spin_unlock(&adev->path_list_lock);
return true;
}
diff --git a/sound/soc/intel/avs/control.c b/sound/soc/intel/avs/control.c
index a8f05de338e0..370069247a7d 100644
--- a/sound/soc/intel/avs/control.c
+++ b/sound/soc/intel/avs/control.c
@@ -27,7 +27,7 @@ static struct avs_path_module *avs_get_volume_module(struct avs_dev *adev, u32 i
struct avs_path_pipeline *ppl;
struct avs_path_module *mod;
- spin_lock(&adev->path_list_lock);
+ guard(spinlock)(&adev->path_list_lock);
list_for_each_entry(path, &adev->path_list, node) {
list_for_each_entry(ppl, &path->ppl_list, node) {
list_for_each_entry(mod, &ppl->mod_list, node) {
@@ -35,14 +35,11 @@ static struct avs_path_module *avs_get_volume_module(struct avs_dev *adev, u32 i
if ((guid_equal(type, &AVS_PEAKVOL_MOD_UUID) ||
guid_equal(type, &AVS_GAIN_MOD_UUID)) &&
- mod->template->ctl_id == id) {
- spin_unlock(&adev->path_list_lock);
+ mod->template->ctl_id == id)
return mod;
- }
}
}
}
- spin_unlock(&adev->path_list_lock);
return NULL;
}
diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 1a53856c2ffb..2afe59646896 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -14,6 +14,7 @@
// foundation of this driver
//
+#include <linux/cleanup.h>
#include <linux/acpi.h>
#include <linux/module.h>
#include <linux/pci.h>
@@ -273,7 +274,7 @@ static irqreturn_t avs_hda_interrupt(struct hdac_bus *bus)
if (snd_hdac_bus_handle_stream_irq(bus, status, hdac_update_stream))
ret = IRQ_HANDLED;
- spin_lock_irq(&bus->reg_lock);
+ guard(spinlock_irq)(&bus->reg_lock);
/* Clear RIRB interrupt. */
status = snd_hdac_chip_readb(bus, RIRBSTS);
if (status & RIRB_INT_MASK) {
@@ -283,7 +284,6 @@ static irqreturn_t avs_hda_interrupt(struct hdac_bus *bus)
ret = IRQ_HANDLED;
}
- spin_unlock_irq(&bus->reg_lock);
return ret;
}
diff --git a/sound/soc/intel/avs/debug.h b/sound/soc/intel/avs/debug.h
index 94fe8729a5c1..c47fc4e8b02b 100644
--- a/sound/soc/intel/avs/debug.h
+++ b/sound/soc/intel/avs/debug.h
@@ -9,6 +9,7 @@
#ifndef __SOUND_SOC_INTEL_AVS_DEBUG_H
#define __SOUND_SOC_INTEL_AVS_DEBUG_H
+#include <linux/cleanup.h>
#include "messages.h"
#include "registers.h"
@@ -26,14 +27,9 @@ struct avs_dev;
static inline int avs_log_buffer_status_locked(struct avs_dev *adev, union avs_notify_msg *msg)
{
- unsigned long flags;
- int ret;
+ guard(spinlock_irqsave)(&adev->trace_lock);
- spin_lock_irqsave(&adev->trace_lock, flags);
- ret = avs_dsp_op(adev, log_buffer_status, msg);
- spin_unlock_irqrestore(&adev->trace_lock, flags);
-
- return ret;
+ return avs_dsp_op(adev, log_buffer_status, msg);
}
struct avs_apl_log_buffer_layout {
diff --git a/sound/soc/intel/avs/ipc.c b/sound/soc/intel/avs/ipc.c
index c0feb9edd7f6..e99cb703eb7e 100644
--- a/sound/soc/intel/avs/ipc.c
+++ b/sound/soc/intel/avs/ipc.c
@@ -6,6 +6,7 @@
// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
//
+#include <linux/cleanup.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/slab.h>
#include <sound/hdaudio_ext.h>
@@ -397,7 +398,7 @@ static int avs_dsp_do_send_msg(struct avs_dev *adev, struct avs_ipc_msg *request
if (!ipc->ready)
return -EPERM;
- mutex_lock(&ipc->msg_mutex);
+ guard(mutex)(&ipc->msg_mutex);
spin_lock(&ipc->rx_lock);
avs_ipc_msg_init(ipc, reply);
@@ -412,7 +413,7 @@ static int avs_dsp_do_send_msg(struct avs_dev *adev, struct avs_ipc_msg *request
/* Same treatment as on exception, just stack_dump=0. */
avs_dsp_exception_caught(adev, &msg);
}
- goto exit;
+ return ret;
}
ret = ipc->rx.rsp.status;
@@ -436,8 +437,6 @@ static int avs_dsp_do_send_msg(struct avs_dev *adev, struct avs_ipc_msg *request
memcpy(reply->data, ipc->rx.data, reply->size);
}
-exit:
- mutex_unlock(&ipc->msg_mutex);
return ret;
}
@@ -501,7 +500,7 @@ static int avs_dsp_do_send_rom_msg(struct avs_dev *adev, struct avs_ipc_msg *req
struct avs_ipc *ipc = adev->ipc;
int ret;
- mutex_lock(&ipc->msg_mutex);
+ guard(mutex)(&ipc->msg_mutex);
spin_lock(&ipc->rx_lock);
avs_ipc_msg_init(ipc, NULL);
@@ -522,8 +521,6 @@ static int avs_dsp_do_send_rom_msg(struct avs_dev *adev, struct avs_ipc_msg *req
dev_err(adev->dev, "%s (0x%08x 0x%08x) failed: %d\n",
name, request->glb.primary, request->glb.ext.val, ret);
- mutex_unlock(&ipc->msg_mutex);
-
return ret;
}
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c
index 2291f9728a54..213d6ecdd7cc 100644
--- a/sound/soc/intel/avs/path.c
+++ b/sound/soc/intel/avs/path.c
@@ -6,6 +6,7 @@
// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
//
+#include <linux/cleanup.h>
#include <linux/acpi.h>
#include <acpi/nhlt.h>
#include <sound/pcm_params.h>
@@ -69,16 +70,13 @@ avs_path_find_path(struct avs_dev *adev, const char *name, u32 template_id)
if (!template)
return NULL;
- spin_lock(&adev->path_list_lock);
+ guard(spinlock)(&adev->path_list_lock);
/* Only one variant of given path template may be instantiated at a time. */
list_for_each_entry(path, &adev->path_list, node) {
- if (path->template->owner == template) {
- spin_unlock(&adev->path_list_lock);
+ if (path->template->owner == template)
return path;
- }
}
- spin_unlock(&adev->path_list_lock);
return NULL;
}
@@ -1305,7 +1303,7 @@ void avs_path_free(struct avs_path *path)
struct avs_path *cpath, *csave;
struct avs_dev *adev = path->owner;
- mutex_lock(&adev->path_mutex);
+ guard(mutex)(&adev->path_mutex);
/* Free all condpaths this path spawned. */
list_for_each_entry_safe(cpath, csave, &path->source_list, source_node)
@@ -1314,8 +1312,6 @@ void avs_path_free(struct avs_path *path)
avs_condpath_free(path->owner, cpath);
avs_path_free_unlocked(path);
-
- mutex_unlock(&adev->path_mutex);
}
struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id,
@@ -1334,13 +1330,13 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id,
}
/* Serialize path and its components creation. */
- mutex_lock(&adev->path_mutex);
+ guard(mutex)(&adev->path_mutex);
/* Satisfy needs of avs_path_find_tplg(). */
- mutex_lock(&adev->comp_list_mutex);
+ guard(mutex)(&adev->comp_list_mutex);
path = avs_path_create_unlocked(adev, dma_id, variant);
if (IS_ERR(path))
- goto exit;
+ return path;
ret = avs_condpaths_walk_all(adev, path);
if (ret) {
@@ -1348,10 +1344,6 @@ struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id,
path = ERR_PTR(ret);
}
-exit:
- mutex_unlock(&adev->comp_list_mutex);
- mutex_unlock(&adev->path_mutex);
-
return path;
}
@@ -1496,15 +1488,13 @@ static void avs_condpaths_pause(struct avs_dev *adev, struct avs_path *path)
{
struct avs_path *cpath;
- mutex_lock(&adev->path_mutex);
+ guard(mutex)(&adev->path_mutex);
/* If either source or sink stops, so do the attached conditional paths. */
list_for_each_entry(cpath, &path->source_list, source_node)
avs_condpath_pause(adev, cpath);
list_for_each_entry(cpath, &path->sink_list, sink_node)
avs_condpath_pause(adev, cpath);
-
- mutex_unlock(&adev->path_mutex);
}
int avs_path_pause(struct avs_path *path)
@@ -1560,7 +1550,7 @@ static void avs_condpaths_run(struct avs_dev *adev, struct avs_path *path, int t
{
struct avs_path *cpath;
- mutex_lock(&adev->path_mutex);
+ guard(mutex)(&adev->path_mutex);
/* Run conditional paths only if source and sink are both running. */
list_for_each_entry(cpath, &path->source_list, source_node)
@@ -1572,8 +1562,6 @@ static void avs_condpaths_run(struct avs_dev *adev, struct avs_path *path, int t
if (cpath->source->state == AVS_PPL_STATE_RUNNING &&
cpath->sink->state == AVS_PPL_STATE_RUNNING)
avs_condpath_run(adev, cpath, trigger);
-
- mutex_unlock(&adev->path_mutex);
}
int avs_path_run(struct avs_path *path, int trigger)
diff --git a/sound/soc/intel/avs/utils.c b/sound/soc/intel/avs/utils.c
index ee36725ac731..ea14ec173855 100644
--- a/sound/soc/intel/avs/utils.c
+++ b/sound/soc/intel/avs/utils.c
@@ -6,6 +6,7 @@
// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
//
+#include <linux/cleanup.h>
#include <linux/firmware.h>
#include <linux/kfifo.h>
#include <linux/slab.h>
@@ -48,13 +49,12 @@ int avs_get_module_entry(struct avs_dev *adev, const guid_t *uuid, struct avs_mo
{
int idx;
- mutex_lock(&adev->modres_mutex);
+ guard(mutex)(&adev->modres_mutex);
idx = avs_module_entry_index(adev, uuid);
if (idx >= 0)
memcpy(entry, &adev->mods_info->entries[idx], sizeof(*entry));
- mutex_unlock(&adev->modres_mutex);
return (idx < 0) ? idx : 0;
}
@@ -62,13 +62,12 @@ int avs_get_module_id_entry(struct avs_dev *adev, u32 module_id, struct avs_modu
{
int idx;
- mutex_lock(&adev->modres_mutex);
+ guard(mutex)(&adev->modres_mutex);
idx = avs_module_id_entry_index(adev, module_id);
if (idx >= 0)
memcpy(entry, &adev->mods_info->entries[idx], sizeof(*entry));
- mutex_unlock(&adev->modres_mutex);
return (idx < 0) ? idx : 0;
}
@@ -86,13 +85,12 @@ bool avs_is_module_ida_empty(struct avs_dev *adev, u32 module_id)
bool ret = false;
int idx;
- mutex_lock(&adev->modres_mutex);
+ guard(mutex)(&adev->modres_mutex);
idx = avs_module_id_entry_index(adev, module_id);
if (idx >= 0)
ret = ida_is_empty(adev->mod_idas[idx]);
- mutex_unlock(&adev->modres_mutex);
return ret;
}
@@ -163,68 +161,57 @@ int avs_module_info_init(struct avs_dev *adev, bool purge)
if (ret)
return AVS_IPC_RET(ret);
- mutex_lock(&adev->modres_mutex);
+ guard(mutex)(&adev->modres_mutex);
ret = avs_module_ida_alloc(adev, info, purge);
if (ret < 0) {
dev_err(adev->dev, "initialize module idas failed: %d\n", ret);
- goto exit;
+ return ret;
}
/* Refresh current information with newly received table. */
kfree(adev->mods_info);
adev->mods_info = info;
-exit:
- mutex_unlock(&adev->modres_mutex);
return ret;
}
void avs_module_info_free(struct avs_dev *adev)
{
- mutex_lock(&adev->modres_mutex);
+ guard(mutex)(&adev->modres_mutex);
avs_module_ida_destroy(adev);
kfree(adev->mods_info);
adev->mods_info = NULL;
-
- mutex_unlock(&adev->modres_mutex);
}
int avs_module_id_alloc(struct avs_dev *adev, u16 module_id)
{
- int ret, idx, max_id;
+ int idx, max_id;
- mutex_lock(&adev->modres_mutex);
+ guard(mutex)(&adev->modres_mutex);
idx = avs_module_id_entry_index(adev, module_id);
if (idx == -ENOENT) {
dev_err(adev->dev, "invalid module id: %d", module_id);
- ret = -EINVAL;
- goto exit;
+ return -EINVAL;
}
max_id = adev->mods_info->entries[idx].instance_max_count - 1;
- ret = ida_alloc_max(adev->mod_idas[idx], max_id, GFP_KERNEL);
-exit:
- mutex_unlock(&adev->modres_mutex);
- return ret;
+
+ return ida_alloc_max(adev->mod_idas[idx], max_id, GFP_KERNEL);
}
void avs_module_id_free(struct avs_dev *adev, u16 module_id, u8 instance_id)
{
int idx;
- mutex_lock(&adev->modres_mutex);
+ guard(mutex)(&adev->modres_mutex);
idx = avs_module_id_entry_index(adev, module_id);
- if (idx == -ENOENT) {
+ if (idx == -ENOENT)
dev_err(adev->dev, "invalid module id: %d", module_id);
- goto exit;
- }
-
- ida_free(adev->mod_idas[idx], instance_id);
-exit:
- mutex_unlock(&adev->modres_mutex);
+ else
+ ida_free(adev->mod_idas[idx], instance_id);
}
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 2/5] ASoC: Intel: avs: Use scoped_guard() for scoped locking
2026-06-26 8:28 [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() phucduc.bui
2026-06-26 8:29 ` [PATCH v4 1/5] ASoC: Intel: avs: Use guard() for locking phucduc.bui
@ 2026-06-26 8:29 ` phucduc.bui
2026-06-26 8:29 ` [PATCH v4 3/5] ASoC: intel: atom: Use __free(kfree) for stream pointer phucduc.bui
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: phucduc.bui @ 2026-06-26 8:29 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Cezary Rojewski
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
Clean up the code using scoped_guard() for mutex & spin locks.
Merely code refactoring, and no behavior change.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
Changes in v3:
- Drop the scoped_guard() conversion in avs_ipc_wait_busy_completion(),
as the function still uses a goto-based cleanup path.
Changes in v4:
- Add cleanup.h include where needed.
sound/soc/intel/avs/debugfs.c | 19 +++++++------
sound/soc/intel/avs/ipc.c | 50 +++++++++++++++++------------------
sound/soc/intel/avs/loader.c | 15 ++++++-----
3 files changed, 42 insertions(+), 42 deletions(-)
diff --git a/sound/soc/intel/avs/debugfs.c b/sound/soc/intel/avs/debugfs.c
index 701c247227bf..9ab503da3b75 100644
--- a/sound/soc/intel/avs/debugfs.c
+++ b/sound/soc/intel/avs/debugfs.c
@@ -6,6 +6,7 @@
// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
//
+#include <linux/cleanup.h>
#include <linux/debugfs.h>
#include <linux/kfifo.h>
#include <linux/wait.h>
@@ -251,24 +252,22 @@ static int strace_release(struct inode *inode, struct file *file)
union avs_notify_msg msg = AVS_NOTIFICATION(LOG_BUFFER_STATUS);
struct avs_dev *adev = file->private_data;
unsigned long resource_mask;
- unsigned long flags, i;
+ unsigned long i;
u32 num_cores;
resource_mask = adev->logged_resources;
num_cores = adev->hw_cfg.dsp_cores;
- spin_lock_irqsave(&adev->trace_lock, flags);
+ scoped_guard(spinlock_irqsave, &adev->trace_lock) {
+ /* Gather any remaining logs. */
+ for_each_set_bit(i, &resource_mask, num_cores) {
+ msg.log.core = i;
+ avs_dsp_op(adev, log_buffer_status, &msg);
+ }
- /* Gather any remaining logs. */
- for_each_set_bit(i, &resource_mask, num_cores) {
- msg.log.core = i;
- avs_dsp_op(adev, log_buffer_status, &msg);
+ kfifo_free(&adev->trace_fifo);
}
- kfifo_free(&adev->trace_fifo);
-
- spin_unlock_irqrestore(&adev->trace_lock, flags);
-
module_put(adev->dev->driver->owner);
return 0;
}
diff --git a/sound/soc/intel/avs/ipc.c b/sound/soc/intel/avs/ipc.c
index e99cb703eb7e..71e7997e52c2 100644
--- a/sound/soc/intel/avs/ipc.c
+++ b/sound/soc/intel/avs/ipc.c
@@ -100,39 +100,39 @@ static void avs_dsp_recovery(struct avs_dev *adev)
unsigned int core_mask;
int ret;
- mutex_lock(&adev->comp_list_mutex);
- /* disconnect all running streams */
- list_for_each_entry(acomp, &adev->comp_list, node) {
- struct snd_soc_pcm_runtime *rtd;
- struct snd_soc_card *card;
-
- card = acomp->base.card;
- if (!card)
- continue;
-
- for_each_card_rtds(card, rtd) {
- struct snd_pcm *pcm;
- int dir;
-
- pcm = rtd->pcm;
- if (!pcm || rtd->dai_link->no_pcm)
+ scoped_guard(mutex, &adev->comp_list_mutex) {
+ /* disconnect all running streams */
+ list_for_each_entry(acomp, &adev->comp_list, node) {
+ struct snd_soc_pcm_runtime *rtd;
+ struct snd_soc_card *card;
+
+ card = acomp->base.card;
+ if (!card)
continue;
- for_each_pcm_streams(dir) {
- struct snd_pcm_substream *substream;
+ for_each_card_rtds(card, rtd) {
+ struct snd_pcm *pcm;
+ int dir;
- substream = pcm->streams[dir].substream;
- if (!substream || !substream->runtime)
+ pcm = rtd->pcm;
+ if (!pcm || rtd->dai_link->no_pcm)
continue;
- /* No need for _irq() as we are in nonatomic context. */
- snd_pcm_stream_lock(substream);
- snd_pcm_stop(substream, SNDRV_PCM_STATE_DISCONNECTED);
- snd_pcm_stream_unlock(substream);
+ for_each_pcm_streams(dir) {
+ struct snd_pcm_substream *substream;
+
+ substream = pcm->streams[dir].substream;
+ if (!substream || !substream->runtime)
+ continue;
+
+ /* No need for _irq() as we are in nonatomic context. */
+ snd_pcm_stream_lock(substream);
+ snd_pcm_stop(substream, SNDRV_PCM_STATE_DISCONNECTED);
+ snd_pcm_stream_unlock(substream);
+ }
}
}
}
- mutex_unlock(&adev->comp_list_mutex);
/* forcibly shutdown all cores */
core_mask = GENMASK(adev->hw_cfg.dsp_cores - 1, 0);
diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c
index 353e343b1d28..bebdc79ec88e 100644
--- a/sound/soc/intel/avs/loader.c
+++ b/sound/soc/intel/avs/loader.c
@@ -6,6 +6,7 @@
// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
//
+#include <linux/cleanup.h>
#include <linux/firmware.h>
#include <linux/module.h>
#include <linux/slab.h>
@@ -630,15 +631,15 @@ static int avs_load_firmware(struct avs_dev *adev, bool purge)
if (ret)
goto reenable_gating;
- mutex_lock(&adev->comp_list_mutex);
- list_for_each_entry(acomp, &adev->comp_list, node) {
- struct avs_tplg *tplg = acomp->tplg;
+ scoped_guard(mutex, &adev->comp_list_mutex) {
+ list_for_each_entry(acomp, &adev->comp_list, node) {
+ struct avs_tplg *tplg = acomp->tplg;
- ret = avs_dsp_load_libraries(adev, tplg->libs, tplg->num_libs);
- if (ret < 0)
- break;
+ ret = avs_dsp_load_libraries(adev, tplg->libs, tplg->num_libs);
+ if (ret < 0)
+ break;
+ }
}
- mutex_unlock(&adev->comp_list_mutex);
reenable_gating:
avs_hda_l1sen_enable(adev, true);
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 3/5] ASoC: intel: atom: Use __free(kfree) for stream pointer
2026-06-26 8:28 [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() phucduc.bui
2026-06-26 8:29 ` [PATCH v4 1/5] ASoC: Intel: avs: Use guard() for locking phucduc.bui
2026-06-26 8:29 ` [PATCH v4 2/5] ASoC: Intel: avs: Use scoped_guard() for scoped locking phucduc.bui
@ 2026-06-26 8:29 ` phucduc.bui
2026-06-26 8:29 ` [PATCH v4 4/5] ASoC: Intel: atom: Use guard() for locking phucduc.bui
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: phucduc.bui @ 2026-06-26 8:29 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Cezary Rojewski
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
Declare 'stream' with __free(kfree) so it is automatically freed when
leaving scope. This allows direct returns from error paths and removes
the explicit kfree(stream) call.
Set 'stream' to NULL after ownership has been transferred to
runtime->private_data to prevent it from being freed on the success path.
This cleanup is a preparation step for upcoming locking changes.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
Changes in v4:
- Add cleanup.h include where needed.
- update commit message
sound/soc/intel/atom/sst-mfld-platform-pcm.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
index f074af2499c8..67506a718158 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -11,6 +11,7 @@
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/cleanup.h>
#include <linux/slab.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -304,7 +305,7 @@ static int sst_media_open(struct snd_pcm_substream *substream,
{
int ret_val = 0;
struct snd_pcm_runtime *runtime = substream->runtime;
- struct sst_runtime_stream *stream;
+ struct sst_runtime_stream *stream __free(kfree) = NULL;
stream = kzalloc_obj(*stream);
if (!stream)
@@ -330,7 +331,7 @@ static int sst_media_open(struct snd_pcm_substream *substream,
ret_val = power_up_sst(stream);
if (ret_val < 0)
- goto out_power_up;
+ return ret_val;
/*
* Make sure the period to be multiple of 1ms to align the
@@ -347,12 +348,19 @@ static int sst_media_open(struct snd_pcm_substream *substream,
snd_pcm_hw_constraint_step(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_PERIODS, 2);
- return snd_pcm_hw_constraint_integer(runtime,
- SNDRV_PCM_HW_PARAM_PERIODS);
+ ret_val = snd_pcm_hw_constraint_integer(runtime,
+ SNDRV_PCM_HW_PARAM_PERIODS);
+
+ if (ret_val < 0)
+ return ret_val;
+
+ stream = NULL;
+
+ return ret_val;
+
out_ops:
mutex_unlock(&sst_lock);
-out_power_up:
- kfree(stream);
+
return ret_val;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 4/5] ASoC: Intel: atom: Use guard() for locking
2026-06-26 8:28 [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() phucduc.bui
` (2 preceding siblings ...)
2026-06-26 8:29 ` [PATCH v4 3/5] ASoC: intel: atom: Use __free(kfree) for stream pointer phucduc.bui
@ 2026-06-26 8:29 ` phucduc.bui
2026-06-26 8:29 ` [PATCH v4 5/5] ASoC: Intel: atom: Use scoped_guard() for scoped locking phucduc.bui
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: phucduc.bui @ 2026-06-26 8:29 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Cezary Rojewski
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
Clean up the code using guard() for spin & mutex locks.
Merely code refactoring, and no behavior change.
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
Changes in v4:
- Add cleanup.h include where needed.
sound/soc/intel/atom/sst-atom-controls.c | 29 ++++++--------------
sound/soc/intel/atom/sst-mfld-platform-pcm.c | 25 ++++++-----------
sound/soc/intel/atom/sst/sst_ipc.c | 5 ++--
sound/soc/intel/atom/sst/sst_pvt.c | 9 +++---
4 files changed, 23 insertions(+), 45 deletions(-)
diff --git a/sound/soc/intel/atom/sst-atom-controls.c b/sound/soc/intel/atom/sst-atom-controls.c
index 3629ceaaac17..701369349fb6 100644
--- a/sound/soc/intel/atom/sst-atom-controls.c
+++ b/sound/soc/intel/atom/sst-atom-controls.c
@@ -14,6 +14,7 @@
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/cleanup.h>
#include <linux/slab.h>
#include <sound/soc.h>
#include <sound/tlv.h>
@@ -73,14 +74,10 @@ static int sst_fill_and_send_cmd(struct sst_data *drv,
u8 ipc_msg, u8 block, u8 task_id, u8 pipe_id,
void *cmd_data, u16 len)
{
- int ret;
+ guard(mutex)(&drv->lock);
- mutex_lock(&drv->lock);
- ret = sst_fill_and_send_cmd_unlocked(drv, ipc_msg, block,
- task_id, pipe_id, cmd_data, len);
- mutex_unlock(&drv->lock);
-
- return ret;
+ return sst_fill_and_send_cmd_unlocked(drv, ipc_msg, block,
+ task_id, pipe_id, cmd_data, len);
}
/*
@@ -167,7 +164,7 @@ static int sst_slot_get(struct snd_kcontrol *kcontrol,
unsigned int val, mux;
u8 *map = is_tx ? sst_ssp_rx_map : sst_ssp_tx_map;
- mutex_lock(&drv->lock);
+ guard(mutex)(&drv->lock);
val = 1 << ctl_no;
/* search which slot/channel has this bit set - there should be only one */
for (mux = e->max; mux > 0; mux--)
@@ -175,7 +172,6 @@ static int sst_slot_get(struct snd_kcontrol *kcontrol,
break;
ucontrol->value.enumerated.item[0] = mux;
- mutex_unlock(&drv->lock);
dev_dbg(c->dev, "%s - %s map = %#x\n",
is_tx ? "tx channel" : "rx slot",
@@ -235,7 +231,7 @@ static int sst_slot_put(struct snd_kcontrol *kcontrol,
if (mux > e->max - 1)
return -EINVAL;
- mutex_lock(&drv->lock);
+ guard(mutex)(&drv->lock);
/* first clear all registers of this bit */
for (i = 0; i < e->max; i++)
map[i] &= ~val;
@@ -244,7 +240,6 @@ static int sst_slot_put(struct snd_kcontrol *kcontrol,
/* kctl set to 'none' and we reset the bits so send IPC */
ret = sst_check_and_send_slot_map(drv, kcontrol);
- mutex_unlock(&drv->lock);
return ret;
}
@@ -258,7 +253,6 @@ static int sst_slot_put(struct snd_kcontrol *kcontrol,
ret = sst_check_and_send_slot_map(drv, kcontrol);
- mutex_unlock(&drv->lock);
return ret;
}
@@ -354,13 +348,12 @@ static int sst_algo_control_set(struct snd_kcontrol *kcontrol,
struct sst_algo_control *bc = (void *)kcontrol->private_value;
dev_dbg(cmpnt->dev, "control_name=%s\n", kcontrol->id.name);
- mutex_lock(&drv->lock);
+ guard(mutex)(&drv->lock);
switch (bc->type) {
case SST_ALGO_PARAMS:
memcpy(bc->params, ucontrol->value.bytes.data, bc->max);
break;
default:
- mutex_unlock(&drv->lock);
dev_err(cmpnt->dev, "Invalid Input- algo type:%d\n",
bc->type);
return -EINVAL;
@@ -368,7 +361,6 @@ static int sst_algo_control_set(struct snd_kcontrol *kcontrol,
/*if pipe is enabled, need to send the algo params from here*/
if (bc->w && bc->w->power)
ret = sst_send_algo_cmd(drv, bc);
- mutex_unlock(&drv->lock);
return ret;
}
@@ -475,7 +467,7 @@ static int sst_gain_put(struct snd_kcontrol *kcontrol,
struct sst_gain_mixer_control *mc = (void *)kcontrol->private_value;
struct sst_gain_value *gv = mc->gain_val;
- mutex_lock(&drv->lock);
+ guard(mutex)(&drv->lock);
switch (mc->type) {
case SST_GAIN_TLV:
@@ -497,7 +489,6 @@ static int sst_gain_put(struct snd_kcontrol *kcontrol,
break;
default:
- mutex_unlock(&drv->lock);
dev_err(cmpnt->dev, "Invalid Input- gain type:%d\n",
mc->type);
return -EINVAL;
@@ -506,7 +497,6 @@ static int sst_gain_put(struct snd_kcontrol *kcontrol,
if (mc->w && mc->w->power)
ret = sst_send_gain_cmd(drv, gv, mc->task_id,
mc->pipe_id | mc->instance_id, mc->module_id, 0);
- mutex_unlock(&drv->lock);
return ret;
}
@@ -521,10 +511,9 @@ static int sst_send_pipe_module_params(struct snd_soc_dapm_widget *w,
struct sst_data *drv = snd_soc_component_get_drvdata(c);
struct sst_ids *ids = w->priv;
- mutex_lock(&drv->lock);
+ guard(mutex)(&drv->lock);
sst_find_and_send_pipe_algo(drv, w->name, ids);
sst_set_pipe_gain(ids, drv, 0);
- mutex_unlock(&drv->lock);
return 0;
}
diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
index 67506a718158..c757e4dcd7cf 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -33,7 +33,7 @@ int sst_register_dsp(struct sst_device *dev)
return -EINVAL;
if (!try_module_get(dev->dev->driver->owner))
return -ENODEV;
- mutex_lock(&sst_lock);
+ guard(mutex)(&sst_lock);
if (sst) {
dev_err(dev->dev, "we already have a device %s\n", sst->name);
module_put(dev->dev->driver->owner);
@@ -42,7 +42,7 @@ int sst_register_dsp(struct sst_device *dev)
}
dev_dbg(dev->dev, "registering device %s\n", dev->name);
sst = dev;
- mutex_unlock(&sst_lock);
+
return 0;
}
EXPORT_SYMBOL_GPL(sst_register_dsp);
@@ -54,17 +54,15 @@ int sst_unregister_dsp(struct sst_device *dev)
if (dev != sst)
return -EINVAL;
- mutex_lock(&sst_lock);
+ guard(mutex)(&sst_lock);
- if (!sst) {
- mutex_unlock(&sst_lock);
+ if (!sst)
return -EIO;
- }
module_put(sst->dev->driver->owner);
dev_dbg(dev->dev, "unreg %s\n", sst->name);
sst = NULL;
- mutex_unlock(&sst_lock);
+
return 0;
}
EXPORT_SYMBOL_GPL(sst_unregister_dsp);
@@ -104,21 +102,14 @@ static int sst_media_digital_mute(struct snd_soc_dai *dai, int mute, int stream)
void sst_set_stream_status(struct sst_runtime_stream *stream,
int state)
{
- unsigned long flags;
- spin_lock_irqsave(&stream->status_lock, flags);
+ guard(spinlock_irqsave)(&stream->status_lock);
stream->stream_status = state;
- spin_unlock_irqrestore(&stream->status_lock, flags);
}
static inline int sst_get_stream_status(struct sst_runtime_stream *stream)
{
- int state;
- unsigned long flags;
-
- spin_lock_irqsave(&stream->status_lock, flags);
- state = stream->stream_status;
- spin_unlock_irqrestore(&stream->status_lock, flags);
- return state;
+ guard(spinlock_irqsave)(&stream->status_lock);
+ return stream->stream_status;
}
static void sst_fill_alloc_params(struct snd_pcm_substream *substream,
diff --git a/sound/soc/intel/atom/sst/sst_ipc.c b/sound/soc/intel/atom/sst/sst_ipc.c
index 0d5e71e8a5b5..6c19ab63aa4f 100644
--- a/sound/soc/intel/atom/sst/sst_ipc.c
+++ b/sound/soc/intel/atom/sst/sst_ipc.c
@@ -11,6 +11,7 @@
*
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*/
+#include <linux/cleanup.h>
#include <linux/pci.h>
#include <linux/firmware.h>
#include <linux/sched.h>
@@ -180,9 +181,8 @@ void intel_sst_clear_intr_mrfld(struct intel_sst_drv *sst_drv_ctx)
union interrupt_reg_mrfld isr;
union interrupt_reg_mrfld imr;
union ipc_header_mrfld clear_ipc;
- unsigned long irq_flags;
- spin_lock_irqsave(&sst_drv_ctx->ipc_spin_lock, irq_flags);
+ guard(spinlock_irqsave)(&sst_drv_ctx->ipc_spin_lock);
imr.full = sst_shim_read64(sst_drv_ctx->shim, SST_IMRX);
isr.full = sst_shim_read64(sst_drv_ctx->shim, SST_ISRX);
@@ -200,7 +200,6 @@ void intel_sst_clear_intr_mrfld(struct intel_sst_drv *sst_drv_ctx)
/* un mask busy interrupt */
imr.part.busy_interrupt = 0;
sst_shim_write64(sst_drv_ctx->shim, SST_IMRX, imr.full);
- spin_unlock_irqrestore(&sst_drv_ctx->ipc_spin_lock, irq_flags);
}
diff --git a/sound/soc/intel/atom/sst/sst_pvt.c b/sound/soc/intel/atom/sst/sst_pvt.c
index 67b1ab14239f..0b0cfd70efbc 100644
--- a/sound/soc/intel/atom/sst/sst_pvt.c
+++ b/sound/soc/intel/atom/sst/sst_pvt.c
@@ -11,6 +11,7 @@
*
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*/
+#include <linux/cleanup.h>
#include <linux/kobject.h>
#include <linux/pci.h>
#include <linux/fs.h>
@@ -64,9 +65,8 @@ u64 sst_shim_read64(void __iomem *addr, int offset)
void sst_set_fw_state_locked(
struct intel_sst_drv *sst_drv_ctx, int sst_state)
{
- mutex_lock(&sst_drv_ctx->sst_lock);
+ guard(mutex)(&sst_drv_ctx->sst_lock);
sst_drv_ctx->sst_state = sst_state;
- mutex_unlock(&sst_drv_ctx->sst_lock);
}
/*
@@ -302,18 +302,17 @@ int sst_assign_pvt_id(struct intel_sst_drv *drv)
{
int local;
- spin_lock(&drv->block_lock);
+ guard(spinlock)(&drv->block_lock);
/* find first zero index from lsb */
local = ffz(drv->pvt_id);
dev_dbg(drv->dev, "pvt_id assigned --> %d\n", local);
if (local >= SST_MAX_BLOCKS){
- spin_unlock(&drv->block_lock);
dev_err(drv->dev, "PVT _ID error: no free id blocks ");
return -EINVAL;
}
/* toggle the index */
change_bit(local, &drv->pvt_id);
- spin_unlock(&drv->block_lock);
+
return local;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 5/5] ASoC: Intel: atom: Use scoped_guard() for scoped locking
2026-06-26 8:28 [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() phucduc.bui
` (3 preceding siblings ...)
2026-06-26 8:29 ` [PATCH v4 4/5] ASoC: Intel: atom: Use guard() for locking phucduc.bui
@ 2026-06-26 8:29 ` phucduc.bui
2026-06-29 8:29 ` [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() Cezary Rojewski
2026-06-30 11:26 ` Mark Brown
6 siblings, 0 replies; 12+ messages in thread
From: phucduc.bui @ 2026-06-26 8:29 UTC (permalink / raw)
To: Mark Brown, Liam Girdwood, Cezary Rojewski
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
Clean up the code using scoped_guard() for mutex locks.
Merely code refactoring, and no behavior change.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
sound/soc/intel/atom/sst-atom-controls.c | 38 ++++++++++----------
sound/soc/intel/atom/sst-mfld-platform-pcm.c | 20 ++++-------
2 files changed, 27 insertions(+), 31 deletions(-)
diff --git a/sound/soc/intel/atom/sst-atom-controls.c b/sound/soc/intel/atom/sst-atom-controls.c
index 701369349fb6..82df398237da 100644
--- a/sound/soc/intel/atom/sst-atom-controls.c
+++ b/sound/soc/intel/atom/sst-atom-controls.c
@@ -750,27 +750,29 @@ int sst_handle_vb_timer(struct snd_soc_dai *dai, bool enable)
return ret;
}
- mutex_lock(&drv->lock);
- if (enable)
- timer_usage++;
- else
- timer_usage--;
-
- /*
- * Send the command only if this call is the first enable or last
- * disable
- */
- if ((enable && (timer_usage == 1)) ||
- (!enable && (timer_usage == 0))) {
- ret = sst_fill_and_send_cmd_unlocked(drv, SST_IPC_IA_CMD,
- SST_FLAG_BLOCKED, SST_TASK_SBA, 0, &cmd,
- sizeof(cmd.header) + cmd.header.length);
- if (ret && enable) {
+ scoped_guard(mutex, &drv->lock) {
+ if (enable)
+ timer_usage++;
+ else
timer_usage--;
- enable = false;
+
+ /*
+ * Send the command only if this call is the first enable or last
+ * disable
+ */
+ if ((enable && timer_usage == 1) ||
+ (!enable && timer_usage == 0)) {
+ ret = sst_fill_and_send_cmd_unlocked(drv, SST_IPC_IA_CMD,
+ SST_FLAG_BLOCKED,
+ SST_TASK_SBA, 0, &cmd,
+ sizeof(cmd.header) +
+ cmd.header.length);
+ if (ret && enable) {
+ timer_usage--;
+ enable = false;
+ }
}
}
- mutex_unlock(&drv->lock);
if (!enable)
sst->ops->power(sst->dev, false);
diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
index c757e4dcd7cf..9ee4d9926e06 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -304,15 +304,14 @@ static int sst_media_open(struct snd_pcm_substream *substream,
spin_lock_init(&stream->status_lock);
/* get the sst ops */
- mutex_lock(&sst_lock);
- if (!sst ||
- !try_module_get(sst->dev->driver->owner)) {
- dev_err(dai->dev, "no device available to run\n");
- ret_val = -ENODEV;
- goto out_ops;
+ scoped_guard(mutex, &sst_lock) {
+ if (!sst ||
+ !try_module_get(sst->dev->driver->owner)) {
+ dev_err(dai->dev, "no device available to run\n");
+ return -ENODEV;
+ }
+ stream->ops = sst->ops;
}
- stream->ops = sst->ops;
- mutex_unlock(&sst_lock);
stream->stream_info.str_id = 0;
@@ -347,11 +346,6 @@ static int sst_media_open(struct snd_pcm_substream *substream,
stream = NULL;
- return ret_val;
-
-out_ops:
- mutex_unlock(&sst_lock);
-
return ret_val;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()
2026-06-26 8:28 [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() phucduc.bui
` (4 preceding siblings ...)
2026-06-26 8:29 ` [PATCH v4 5/5] ASoC: Intel: atom: Use scoped_guard() for scoped locking phucduc.bui
@ 2026-06-29 8:29 ` Cezary Rojewski
2026-06-29 13:03 ` Bui Duc Phuc
2026-06-30 11:26 ` Mark Brown
6 siblings, 1 reply; 12+ messages in thread
From: Cezary Rojewski @ 2026-06-29 8:29 UTC (permalink / raw)
To: phucduc.bui
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
Mark Brown, Liam Girdwood
On 6/26/2026 10:28 AM, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> Hi all,
>
> This series converts mutex and spinlock handling in Intel ASoC drivers
> to use guard()/scoped_guard() helpers.
>
> Most changes are straightforward conversions intended to simplify lock
> handling and cleanup paths, with no intended functional changes.
>
> The series has been compile-tested only.
The patchset looks very good, for all:
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Can provide Tested-by: tag but not today - tomorrow, as my CI is in the
middle of waking up after a routine maintenance.
Kind regards,
Czarek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()
2026-06-29 8:29 ` [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() Cezary Rojewski
@ 2026-06-29 13:03 ` Bui Duc Phuc
2026-06-29 13:25 ` Cezary Rojewski
0 siblings, 1 reply; 12+ messages in thread
From: Bui Duc Phuc @ 2026-06-29 13:03 UTC (permalink / raw)
To: Cezary Rojewski
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
Mark Brown, Liam Girdwood
On Mon, Jun 29, 2026 at 3:29 PM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
> On 6/26/2026 10:28 AM, phucduc.bui@gmail.com wrote:
> > From: bui duc phuc <phucduc.bui@gmail.com>
> >
> > Hi all,
> >
> > This series converts mutex and spinlock handling in Intel ASoC drivers
> > to use guard()/scoped_guard() helpers.
> >
> > Most changes are straightforward conversions intended to simplify lock
> > handling and cleanup paths, with no intended functional changes.
> >
> > The series has been compile-tested only.
> The patchset looks very good, for all:
>
> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
>
> Can provide Tested-by: tag but not today - tomorrow, as my CI is in the
> middle of waking up after a routine maintenance.
>
>
> Kind regards,
> Czarek
Hi Czarek,
Thanks, I understand. No problem at all.
The only thing I'm still a bit concerned about is splitting the kfree
and guard() changes into two patches.
Since the guard() conversion is mainly there to eliminate the goto
labels, testing them separately might
introduce issues that wouldn't exist when both patches are applied together.
By the way, I'm curious about your CI. What kinds of tests does it cover?
For example, can it detect regressions related to audio functionality
only, or is it also able to validate
audio quality? I imagine things that require someone to actually
listen to the output would be difficult
to automate.
Best regards,
Phuc
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()
2026-06-29 13:03 ` Bui Duc Phuc
@ 2026-06-29 13:25 ` Cezary Rojewski
2026-06-29 14:26 ` Bui Duc Phuc
0 siblings, 1 reply; 12+ messages in thread
From: Cezary Rojewski @ 2026-06-29 13:25 UTC (permalink / raw)
To: Bui Duc Phuc
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
Mark Brown, Liam Girdwood
On 6/29/2026 3:03 PM, Bui Duc Phuc wrote:
> The only thing I'm still a bit concerned about is splitting the kfree
> and guard() changes into two patches.
> Since the guard() conversion is mainly there to eliminate the goto
> labels, testing them separately might
> introduce issues that wouldn't exist when both patches are applied together.
The CI is run for entire patchset.
> By the way, I'm curious about your CI. What kinds of tests does it cover?
> For example, can it detect regressions related to audio functionality
> only, or is it also able to validate
> audio quality? I imagine things that require someone to actually
> listen to the output would be difficult
> to automate.
Cannot share the details but I'm talking about actual long-term-support
CI with large amount of corporate resources invested. A lab, high
number of physical setups with intention to cover every Intel's AudioDSP
architecture version (starting from Haswell) and streaming-interface
type (HDAudio, DMIC etc.).
All functional tests are fully automated - imitate every single scenario
we do expose for our clients, regardless of the OS type. While again, I
cannot list the tests, the basics are quite simple - bucket the
"functions" e.g.: system states (SXes), device states (DXes),
multi-threading, clocking (the list goes on) and do: atomic test and
then a combination for each feature from each bucket (if valid).
Incomplete example for basic playback:
- 1x default endpoint pb before s3
- 1x default endpoint pb during s3
- 1x default endpoint pb after s3
- 1x default endpoint pb before s4
- 1x default endpoint pb during s4
- 1x default endpoint pb after s4
Wait, does that mean the tests go in thousands? Yeah, several thousands.
However, without such investment, people like me would not be able to
propose and provide reliable contributions for Intel and ASoC both.
Kind regards,
Czarek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()
2026-06-29 13:25 ` Cezary Rojewski
@ 2026-06-29 14:26 ` Bui Duc Phuc
2026-06-29 14:51 ` Cezary Rojewski
0 siblings, 1 reply; 12+ messages in thread
From: Bui Duc Phuc @ 2026-06-29 14:26 UTC (permalink / raw)
To: Cezary Rojewski
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
Mark Brown, Liam Girdwood
On Mon, Jun 29, 2026 at 8:25 PM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
>
> The CI is run for entire patchset.
>
> Cannot share the details but I'm talking about actual long-term-support
> CI with large amount of corporate resources invested. A lab, high
> number of physical setups with intention to cover every Intel's AudioDSP
> architecture version (starting from Haswell) and streaming-interface
> type (HDAudio, DMIC etc.).
>
> All functional tests are fully automated - imitate every single scenario
> we do expose for our clients, regardless of the OS type. While again, I
> cannot list the tests, the basics are quite simple - bucket the
> "functions" e.g.: system states (SXes), device states (DXes),
> multi-threading, clocking (the list goes on) and do: atomic test and
> then a combination for each feature from each bucket (if valid).
> Incomplete example for basic playback:
>
> - 1x default endpoint pb before s3
> - 1x default endpoint pb during s3
> - 1x default endpoint pb after s3
> - 1x default endpoint pb before s4
> - 1x default endpoint pb during s4
> - 1x default endpoint pb after s4
>
> Wait, does that mean the tests go in thousands? Yeah, several thousands.
>
> However, without such investment, people like me would not be able to
> propose and provide reliable contributions for Intel and ASoC both.
>
Thanks for the detailed explanation.
That sounds like a very impressive CI infrastructure. Having such a well-funded
and comprehensive validation system is clearly a huge advantage when developing
and reviewing patches. Running thousands of functional scenarios
across different
DSP generations and streaming interfaces certainly gives much higher confidence
in changes.
That said, it also made me think about the `sst_media_open()` case.
Unless I misunderstood something, the CI didn't catch the memory leak
caused by `kfree(stream)` when `snd_pcm_hw_constraint_integer()`
returns an error.
That issue only became apparent while doing the cleanup work.
So perhaps the patch ASoC: intel: atom: Use __free(kfree) for stream pointer
is not merely a cleanup patch after all—it also fixes a real memory leak.
Out of curiosity, has this function been covered by the CI before?
Or is this simply one of those corner cases that are difficult to exercise,
even with such extensive functional testing?
Best regards,
Phuc
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()
2026-06-29 14:26 ` Bui Duc Phuc
@ 2026-06-29 14:51 ` Cezary Rojewski
0 siblings, 0 replies; 12+ messages in thread
From: Cezary Rojewski @ 2026-06-29 14:51 UTC (permalink / raw)
To: Bui Duc Phuc
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel,
Mark Brown, Liam Girdwood
On 6/29/2026 4:26 PM, Bui Duc Phuc wrote:
> On Mon, Jun 29, 2026 at 8:25 PM Cezary Rojewski
> <cezary.rojewski@intel.com> wrote:
>> Cannot share the details but I'm talking about actual long-term-support
>> CI with large amount of corporate resources invested. A lab, high
>> number of physical setups with intention to cover every Intel's AudioDSP
>> architecture version (starting from Haswell) and streaming-interface
>> type (HDAudio, DMIC etc.).
>>
>> All functional tests are fully automated - imitate every single scenario
>> we do expose for our clients, regardless of the OS type. While again, I
>> cannot list the tests, the basics are quite simple - bucket the
>> "functions" e.g.: system states (SXes), device states (DXes),
>> multi-threading, clocking (the list goes on) and do: atomic test and
>> then a combination for each feature from each bucket (if valid).
>> Incomplete example for basic playback:
>>
>> - 1x default endpoint pb before s3
>> - 1x default endpoint pb during s3
>> - 1x default endpoint pb after s3
>> - 1x default endpoint pb before s4
>> - 1x default endpoint pb during s4
>> - 1x default endpoint pb after s4
>>
>> Wait, does that mean the tests go in thousands? Yeah, several thousands.
>>
>> However, without such investment, people like me would not be able to
>> propose and provide reliable contributions for Intel and ASoC both.
>>
>
> Thanks for the detailed explanation.
>
> That sounds like a very impressive CI infrastructure. Having such a well-funded
> and comprehensive validation system is clearly a huge advantage when developing
> and reviewing patches. Running thousands of functional scenarios
> across different
> DSP generations and streaming interfaces certainly gives much higher confidence
> in changes.
>
> That said, it also made me think about the `sst_media_open()` case.
> Unless I misunderstood something, the CI didn't catch the memory leak
> caused by `kfree(stream)` when `snd_pcm_hw_constraint_integer()`
> returns an error.
> That issue only became apparent while doing the cleanup work.
Nah, the CI hasn't been run yet. Maintenance takes a long time :)
Unfortunately for "atom", it's outside of the support window. My
Tested-by: would go for the platforms support by avs (and catpt) driver.
> So perhaps the patch ASoC: intel: atom: Use __free(kfree) for stream pointer
> is not merely a cleanup patch after all—it also fixes a real memory leak.
>
> Out of curiosity, has this function been covered by the CI before?
> Or is this simply one of those corner cases that are difficult to exercise,
> even with such extensive functional testing?
No CI is not 100% bullet-proof window, it never is going to find
everything, that's why you need still us - developers.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()
2026-06-26 8:28 [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() phucduc.bui
` (5 preceding siblings ...)
2026-06-29 8:29 ` [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() Cezary Rojewski
@ 2026-06-30 11:26 ` Mark Brown
6 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2026-06-30 11:26 UTC (permalink / raw)
To: Liam Girdwood, Cezary Rojewski, phucduc.bui
Cc: Jaroslav Kysela, Takashi Iwai, Peter Ujfalusi, Bard Liao,
Kai Vehmanen, Pierre-Louis Bossart, linux-sound, linux-kernel
On Fri, 26 Jun 2026 15:28:59 +0700, phucduc.bui@gmail.com wrote:
> ASoC: Intel: Convert locking to guard()/scoped_guard()
>
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> Hi all,
>
> This series converts mutex and spinlock handling in Intel ASoC drivers
> to use guard()/scoped_guard() helpers.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.3
Thanks!
[1/5] ASoC: Intel: avs: Use guard() for locking
https://git.kernel.org/broonie/sound/c/f16513ffa944
[2/5] ASoC: Intel: avs: Use scoped_guard() for scoped locking
https://git.kernel.org/broonie/sound/c/6aaac9f6baa5
[3/5] ASoC: intel: atom: Use __free(kfree) for stream pointer
https://git.kernel.org/broonie/sound/c/4889a8a73f65
[4/5] ASoC: Intel: atom: Use guard() for locking
https://git.kernel.org/broonie/sound/c/71c0f725d35d
[5/5] ASoC: Intel: atom: Use scoped_guard() for scoped locking
https://git.kernel.org/broonie/sound/c/08d2d5e683e8
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-06-30 15:48 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-26 8:28 [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() phucduc.bui
2026-06-26 8:29 ` [PATCH v4 1/5] ASoC: Intel: avs: Use guard() for locking phucduc.bui
2026-06-26 8:29 ` [PATCH v4 2/5] ASoC: Intel: avs: Use scoped_guard() for scoped locking phucduc.bui
2026-06-26 8:29 ` [PATCH v4 3/5] ASoC: intel: atom: Use __free(kfree) for stream pointer phucduc.bui
2026-06-26 8:29 ` [PATCH v4 4/5] ASoC: Intel: atom: Use guard() for locking phucduc.bui
2026-06-26 8:29 ` [PATCH v4 5/5] ASoC: Intel: atom: Use scoped_guard() for scoped locking phucduc.bui
2026-06-29 8:29 ` [PATCH v4 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard() Cezary Rojewski
2026-06-29 13:03 ` Bui Duc Phuc
2026-06-29 13:25 ` Cezary Rojewski
2026-06-29 14:26 ` Bui Duc Phuc
2026-06-29 14:51 ` Cezary Rojewski
2026-06-30 11:26 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox