[PATCH v3 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()

[PATCH v3 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()

[phucduc.bui]

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/

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.

Best regards,
Phuc


bui duc phuc (5):
  ASoC: intel: sst-mfld-platform-pcm: Use __free(kfree) for stream
    pointer
  ASoC: Intel: avs: Use guard() for locking
  ASoC: Intel: avs: Use scoped_guard() for scoped locking
  ASoC: Intel: atom: Use scoped_guard() for scoped locking
  ASoC: Intel: atom: Use guard() for locking

 sound/soc/intel/atom/sst-atom-controls.c     | 66 +++++++++-----------
 sound/soc/intel/atom/sst-mfld-platform-pcm.c | 58 ++++++++---------
 sound/soc/intel/atom/sst/sst_ipc.c           |  4 +-
 sound/soc/intel/atom/sst/sst_pvt.c           |  8 +--
 sound/soc/intel/avs/apl.c                    |  7 +--
 sound/soc/intel/avs/control.c                |  7 +--
 sound/soc/intel/avs/core.c                   |  3 +-
 sound/soc/intel/avs/debug.h                  |  9 +--
 sound/soc/intel/avs/debugfs.c                | 18 +++---
 sound/soc/intel/avs/ipc.c                    | 60 +++++++++---------
 sound/soc/intel/avs/loader.c                 | 14 ++---
 sound/soc/intel/avs/path.c                   | 29 +++------
 sound/soc/intel/avs/utils.c                  | 39 ++++--------
 13 files changed, 128 insertions(+), 194 deletions(-)

-- 
2.43.0

[PATCH v3 1/5] ASoC: intel: sst-mfld-platform-pcm: Use __free(kfree) for stream pointer

[phucduc.bui]

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.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/intel/atom/sst-mfld-platform-pcm.c | 19 +++++++++++++------
 1 file changed, 13 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..47823443354c 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -304,7 +304,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 +330,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 +347,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

Re: [PATCH v3 1/5] ASoC: intel: sst-mfld-platform-pcm: Use __free(kfree) for stream pointer

[Cezary Rojewski]

On 6/23/2026 12:57 PM, phucduc.bui@gmail.com wrote:
> 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.

Hi Phuc,

Thank you for separating the change.  The design of the patchset is off 
though.  __free() is by no means a requirement for the locking update - 
it should be located at the back.  Also, grouping changes targeting the 
same driver makes the entire patchset easier to follow.  Right now we 
have 2 avs patches surrounded by atom ones from either side.

Last but not least, please drop the filename in the commit title.  We do 
not do that here - some filenames are long and the approach leaves 
little space for answering the _what_ question when building a proper 
commit title.  "ASoC: Intel: atom:" is what you want here.

>   sound/soc/intel/atom/sst-mfld-platform-pcm.c | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)

...


> @@ -347,12 +347,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;
> +

The entire construct looks bad, unfortunately.  Guess the reason comes 
from its follow up - the locking update.  The delta (+13, -6) isn't 
exactly a good advertisement for the cleanup either.  Again, I see this 
patch as last in the set.  Should lower the delta too.

>   out_ops:
>   	mutex_unlock(&sst_lock);
> -out_power_up:
> -	kfree(stream);
> +
>   	return ret_val;
>   }
>   

Re: [PATCH v3 1/5] ASoC: intel: sst-mfld-platform-pcm: Use __free(kfree) for stream pointer

[Bui Duc Phuc]

Hi Cezary,

Thank you for the feedback.
I will definitely fix the commit titles and group the patches by driver
(atom and avs) as you suggested in the next version.

Regarding the ordering of this cleanup patch and the locking update,
I wanted to share the rationale behind placing it before the locking patch.

According to cleanup.h guidelines and community best practices (as
also pointed out by Charles in a similar discussion [1]),
mixing guard()/scoped_guard() with traditional goto labels is highly
discouraged and error-prone.

In sst_media_open(), there are currently two goto labels: one for
mutex unlock and one for kfree(stream).
If I introduce the scoped_guard for locking first, that patch would
still be mixed with the remaining goto label for kfree.

To ensure clean, safe, and bisectable patches without mixing guard and goto,
I had to eliminate the kfree error-path first using __free(kfree).
This allows the subsequent locking patch to completely and cleanly replace
the remaining mutex goto pattern with scoped_guard.

Please let me know if this makes sense, or if you still prefer a
different approach to untangle this dependency.

Best regards,
Phuc

[1] https://lore.kernel.org/all/ajUcTfG3vSGz3n3d@opensource.cirrus.com/

On Wed, Jun 24, 2026 at 2:35 AM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
> On 6/23/2026 12:57 PM, phucduc.bui@gmail.com wrote:
> > 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.
>
> Hi Phuc,
>
> Thank you for separating the change.  The design of the patchset is off
> though.  __free() is by no means a requirement for the locking update -
> it should be located at the back.  Also, grouping changes targeting the
> same driver makes the entire patchset easier to follow.  Right now we
> have 2 avs patches surrounded by atom ones from either side.
>
> Last but not least, please drop the filename in the commit title.  We do
> not do that here - some filenames are long and the approach leaves
> little space for answering the _what_ question when building a proper
> commit title.  "ASoC: Intel: atom:" is what you want here.
>
> >   sound/soc/intel/atom/sst-mfld-platform-pcm.c | 19 +++++++++++++------
> >   1 file changed, 13 insertions(+), 6 deletions(-)
>
> ...
>
>
> > @@ -347,12 +347,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;
> > +
>
> The entire construct looks bad, unfortunately.  Guess the reason comes
> from its follow up - the locking update.  The delta (+13, -6) isn't
> exactly a good advertisement for the cleanup either.  Again, I see this
> patch as last in the set.  Should lower the delta too.
>
> >   out_ops:
> >       mutex_unlock(&sst_lock);
> > -out_power_up:
> > -     kfree(stream);
> > +
> >       return ret_val;
> >   }
> >
>

[PATCH v3 2/5] ASoC: Intel: avs: Use guard() for locking

[phucduc.bui]

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>
---
 sound/soc/intel/avs/apl.c     |  7 ++-----
 sound/soc/intel/avs/control.c |  7 ++-----
 sound/soc/intel/avs/core.c    |  3 +--
 sound/soc/intel/avs/debug.h   |  9 ++------
 sound/soc/intel/avs/ipc.c     | 10 +++------
 sound/soc/intel/avs/path.c    | 29 +++++++-------------------
 sound/soc/intel/avs/utils.c   | 39 ++++++++++++-----------------------
 7 files changed, 31 insertions(+), 73 deletions(-)

diff --git a/sound/soc/intel/avs/apl.c b/sound/soc/intel/avs/apl.c
index b922eeaba843..e92a6157fed1 100644
--- a/sound/soc/intel/avs/apl.c
+++ b/sound/soc/intel/avs/apl.c
@@ -190,7 +190,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 +210,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..d309bcfe7f46 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -273,7 +273,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 +283,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..7c5f97b46f0d 100644
--- a/sound/soc/intel/avs/debug.h
+++ b/sound/soc/intel/avs/debug.h
@@ -26,14 +26,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..98403fc08c14 100644
--- a/sound/soc/intel/avs/ipc.c
+++ b/sound/soc/intel/avs/ipc.c
@@ -397,7 +397,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 +412,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 +436,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 +499,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 +520,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..c808fd06c2d9 100644
--- a/sound/soc/intel/avs/path.c
+++ b/sound/soc/intel/avs/path.c
@@ -69,16 +69,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 +1302,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 +1311,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 +1329,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 +1343,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 +1487,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 +1549,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 +1561,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..ca1236731824 100644
--- a/sound/soc/intel/avs/utils.c
+++ b/sound/soc/intel/avs/utils.c
@@ -48,13 +48,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 +61,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 +84,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,50 +160,44 @@ 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;
 
-	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;
 }
 
@@ -214,17 +205,13 @@ 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

Re: [PATCH v3 2/5] ASoC: Intel: avs: Use guard() for locking

[Cezary Rojewski]

On 6/23/2026 12:57 PM, phucduc.bui@gmail.com wrote:
> 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.

Apart from one point below, looks good overall.  What bothers me is lack 
of includes - all these files are missing linux/cleanup.h header despite 
clearly using its members.

>   sound/soc/intel/avs/apl.c     |  7 ++-----
>   sound/soc/intel/avs/control.c |  7 ++-----
>   sound/soc/intel/avs/core.c    |  3 +--
>   sound/soc/intel/avs/debug.h   |  9 ++------
>   sound/soc/intel/avs/ipc.c     | 10 +++------
>   sound/soc/intel/avs/path.c    | 29 +++++++-------------------
>   sound/soc/intel/avs/utils.c   | 39 ++++++++++++-----------------------
>   7 files changed, 31 insertions(+), 73 deletions(-)

...

>   int avs_module_id_alloc(struct avs_dev *adev, u16 module_id)
>   {
>   	int ret, 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);
> +

Drop 'ret' entirely and just 'return ida_alloc_max()'.

>   	return ret;
>   }

Re: [PATCH v3 2/5] ASoC: Intel: avs: Use guard() for locking

[Bui Duc Phuc]

On Wed, Jun 24, 2026 at 2:47 AM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
> On 6/23/2026 12:57 PM, phucduc.bui@gmail.com wrote:
> > 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.
>
> Apart from one point below, looks good overall.  What bothers me is lack
> of includes - all these files are missing linux/cleanup.h header despite
> clearly using its members.
>
> >   sound/soc/intel/avs/apl.c     |  7 ++-----
> >   sound/soc/intel/avs/control.c |  7 ++-----
> >   sound/soc/intel/avs/core.c    |  3 +--
> >   sound/soc/intel/avs/debug.h   |  9 ++------
> >   sound/soc/intel/avs/ipc.c     | 10 +++------
> >   sound/soc/intel/avs/path.c    | 29 +++++++-------------------
> >   sound/soc/intel/avs/utils.c   | 39 ++++++++++++-----------------------
> >   7 files changed, 31 insertions(+), 73 deletions(-)
>
> ...
>
> >   int avs_module_id_alloc(struct avs_dev *adev, u16 module_id)
> >   {
> >       int ret, 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);
> > +
>
> Drop 'ret' entirely and just 'return ida_alloc_max()'.
>
> >       return ret;
> >   }

Thanks for the review.
I'll make that change in the next revision.

[PATCH v3 3/5] ASoC: Intel: avs: Use scoped_guard() for scoped locking

[phucduc.bui]

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.

 sound/soc/intel/avs/debugfs.c | 18 ++++++-------
 sound/soc/intel/avs/ipc.c     | 50 +++++++++++++++++------------------
 sound/soc/intel/avs/loader.c  | 14 +++++-----
 3 files changed, 40 insertions(+), 42 deletions(-)

diff --git a/sound/soc/intel/avs/debugfs.c b/sound/soc/intel/avs/debugfs.c
index 701c247227bf..f398ebd68e13 100644
--- a/sound/soc/intel/avs/debugfs.c
+++ b/sound/soc/intel/avs/debugfs.c
@@ -251,24 +251,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 98403fc08c14..26c02dc4520f 100644
--- a/sound/soc/intel/avs/ipc.c
+++ b/sound/soc/intel/avs/ipc.c
@@ -99,39 +99,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..3b158039acb2 100644
--- a/sound/soc/intel/avs/loader.c
+++ b/sound/soc/intel/avs/loader.c
@@ -630,15 +630,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

[PATCH v3 4/5] ASoC: Intel: atom: Use scoped_guard() for scoped locking

[phucduc.bui]

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 3629ceaaac17..6f3a0646f6a0 100644
--- a/sound/soc/intel/atom/sst-atom-controls.c
+++ b/sound/soc/intel/atom/sst-atom-controls.c
@@ -761,27 +761,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 47823443354c..f679258a030e 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -312,15 +312,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;
 
@@ -355,11 +354,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

[PATCH v3 5/5] ASoC: Intel: atom: Use guard() for locking

[phucduc.bui]

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>
---
 sound/soc/intel/atom/sst-atom-controls.c     | 28 ++++++--------------
 sound/soc/intel/atom/sst-mfld-platform-pcm.c | 25 ++++++-----------
 sound/soc/intel/atom/sst/sst_ipc.c           |  4 +--
 sound/soc/intel/atom/sst/sst_pvt.c           |  8 +++---
 4 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/sound/soc/intel/atom/sst-atom-controls.c b/sound/soc/intel/atom/sst-atom-controls.c
index 6f3a0646f6a0..56e64bacb292 100644
--- a/sound/soc/intel/atom/sst-atom-controls.c
+++ b/sound/soc/intel/atom/sst-atom-controls.c
@@ -73,14 +73,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 +163,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 +171,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 +230,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 +239,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 +252,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 +347,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 +360,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 +466,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 +488,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 +496,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 +510,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 f679258a030e..8275ad6d3126 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -32,7 +32,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);
@@ -41,7 +41,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);
@@ -53,17 +53,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);
@@ -103,21 +101,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..6daebbcdaddb 100644
--- a/sound/soc/intel/atom/sst/sst_ipc.c
+++ b/sound/soc/intel/atom/sst/sst_ipc.c
@@ -180,9 +180,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 +199,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..c872a5958f1d 100644
--- a/sound/soc/intel/atom/sst/sst_pvt.c
+++ b/sound/soc/intel/atom/sst/sst_pvt.c
@@ -64,9 +64,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 +301,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

Re: [PATCH v3 5/5] ASoC: Intel: atom: Use guard() for locking

[Cezary Rojewski]

On 6/23/2026 12:57 PM, phucduc.bui@gmail.com wrote:
> 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.

The change looks good - just dropping by to mention that the cleanup.h 
is missing everywhere : )

Re: [PATCH v3 5/5] ASoC: Intel: atom: Use guard() for locking

[Bui Duc Phuc]

Thanks for the review.

> > Clean up the code using guard() for spin & mutex locks.
> > Merely code refactoring, and no behavior change.
>
> The change looks good - just dropping by to mention that the cleanup.h
> is missing everywhere : )

I noticed that both spinlock.h and mutex.h already pull in cleanup.h,
so the guard()/scoped_guard() macros are available without an explicit include.
Could you clarify the reasoning for adding #include <linux/cleanup.h>
explicitly here?

Re: [PATCH v3 5/5] ASoC: Intel: atom: Use guard() for locking

[Cezary Rojewski]

On 6/24/2026 10:07 AM, Bui Duc Phuc wrote:
> Thanks for the review.
> 
>>> Clean up the code using guard() for spin & mutex locks.
>>> Merely code refactoring, and no behavior change.
>>
>> The change looks good - just dropping by to mention that the cleanup.h
>> is missing everywhere : )
> 
> I noticed that both spinlock.h and mutex.h already pull in cleanup.h,
> so the guard()/scoped_guard() macros are available without an explicit include.
> Could you clarify the reasoning for adding #include <linux/cleanup.h>
> explicitly here?

Follow the include-what-you-use (IWYU) approach.  While the approach 
could be tailored per-header, in general it makes any future job of 
cleaning up the headers easier.

Re: [PATCH v3 5/5] ASoC: Intel: atom: Use guard() for locking

[Bui Duc Phuc]

On Wed, Jun 24, 2026 at 3:54 PM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
> On 6/24/2026 10:07 AM, Bui Duc Phuc wrote:
> > Thanks for the review.
> >
> >>> Clean up the code using guard() for spin & mutex locks.
> >>> Merely code refactoring, and no behavior change.
> >>
> >> The change looks good - just dropping by to mention that the cleanup.h
> >> is missing everywhere : )
> >
> > I noticed that both spinlock.h and mutex.h already pull in cleanup.h,
> > so the guard()/scoped_guard() macros are available without an explicit include.
> > Could you clarify the reasoning for adding #include <linux/cleanup.h>
> > explicitly here?
>
> Follow the include-what-you-use (IWYU) approach.  While the approach
> could be tailored per-header, in general it makes any future job of
> cleaning up the headers easier.

Thanks for the clarification.
I will explicitly add #include <linux/cleanup.h> to follow the IWYU
principle in the next version.

Re: [PATCH v3 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()

[Cezary Rojewski]

On 6/23/2026 12:57 PM, 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.

...

> bui duc phuc (5):
>    ASoC: intel: sst-mfld-platform-pcm: Use __free(kfree) for stream
>      pointer
>    ASoC: Intel: avs: Use guard() for locking
>    ASoC: Intel: avs: Use scoped_guard() for scoped locking
>    ASoC: Intel: atom: Use scoped_guard() for scoped locking
>    ASoC: Intel: atom: Use guard() for locking

The order is off just by looking at it.  On top of what I mentioned in 
1/5, please fix the ordering so that both avs and atom drivers receive 
updates in the same manner.

TLDR:

1. ASoC: Intel: avs: Use guard() for locking
2. ASoC: Intel: avs: Use scoped_guard() for scoped locking
3. ASoC: Intel: atom: Use guard() for locking
4. ASoC: Intel: atom: Use scoped_guard() for scoped locking
5. ASoC: Intel: atom: Use __free(kfree) for stream pointer

Kind regards,
Czarek

Re: [PATCH v3 0/5] ASoC: Intel: Convert locking to guard()/scoped_guard()

[Bui Duc Phuc]

Hi Crazek

>
> The order is off just by looking at it.  On top of what I mentioned in
> 1/5, please fix the ordering so that both avs and atom drivers receive
> updates in the same manner.
>
> TLDR:
>
> 1. ASoC: Intel: avs: Use guard() for locking
> 2. ASoC: Intel: avs: Use scoped_guard() for scoped locking
> 3. ASoC: Intel: atom: Use guard() for locking
> 4. ASoC: Intel: atom: Use scoped_guard() for scoped locking
> 5. ASoC: Intel: atom: Use __free(kfree) for stream pointer
>

As mentioned in my reply to patch 1/5, we cannot place the __free(kfree)
patch at the very end (as patch 5) because the locking updates in atom
still rely on the goto labels for kfree. Moving __free(kfree) after the locking
patches would force us to temporarily mix guard/scoped_guard with goto,
which is highly discouraged.

To keep the driver groups consistent while respecting this dependency,
I can reorder the patchset in one of the following ways:

Option 1:

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

Option 2 (Most convenient for me):

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 scoped_guard() for scoped locking

ASoC: Intel: atom: Use guard() for locking

The reason Option 2 is much more convenient for me is that atom has very few
scoped_guard() instances, making it very easy to separate them first
using git add -p.
On the other hand, guard() instances are quite numerous,
so leaving them for the final patch makes the staging process much smoother.

Please let me know which option you prefer, and I will roll out v4 accordingly.

Regards,
Phuc