[PATCH 00/11] ASoC: fsl: Use guard() for mutex & spin locks

[PATCH 00/11] ASoC: fsl: Use guard() for mutex & spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Hi all,

This series converts mutex & spinlock handling in the FSL sound drivers
to use guard() helpers.
The changes are code cleanup only and should have no functional impact.

I have compile-tested all affected files except `mpc5200_dma` and 
`mpc5200_psc_ac97`, as I have not yet found the correct configuration 
needed to enable those drivers.

Best regards,
Phuc

bui duc phuc (11):
  ASoC: fsl_asrc: Use guard() for spin locks
  ASoC: fsl_audmix: Use guard() for spin locks
  ASoC: fsl_easrc: Use guard() for spin locks
  ASoC: fsl_esai: Use guard() for spin locks
  ASoC: fsl_spdif: Use guard() for spin locks
  ASoC: fsl_ssi: Use guard() for mutex locks
  ASoC: fsl_xcvr: Use guard() for spin locks
  ASoC: imx-audio-rpmsg: Use guard() for spin locks
  ASoC: fsl_rpmsg: Use guard() for mutex & spin locks
  ASoC: fsl: mpc5200_dma: Use guard() for spin locks
  ASoC: fsl: mpc5200_psc_ac97: Use guard() for mutex locks

 sound/soc/fsl/fsl_asrc.c         | 10 +----
 sound/soc/fsl/fsl_audmix.c       | 11 ++---
 sound/soc/fsl/fsl_easrc.c        | 36 +++++------------
 sound/soc/fsl/fsl_esai.c         | 16 +++-----
 sound/soc/fsl/fsl_spdif.c        |  8 +---
 sound/soc/fsl/fsl_ssi.c          | 13 ++----
 sound/soc/fsl/fsl_xcvr.c         | 29 ++++++--------
 sound/soc/fsl/imx-audio-rpmsg.c  | 25 ++++++------
 sound/soc/fsl/imx-pcm-rpmsg.c    | 69 ++++++++++++++------------------
 sound/soc/fsl/mpc5200_dma.c      | 56 +++++++++++++-------------
 sound/soc/fsl/mpc5200_psc_ac97.c | 34 ++++++----------
 11 files changed, 121 insertions(+), 186 deletions(-)

-- 
2.43.0

[PATCH 01/11] ASoC: fsl_asrc: Use guard() for spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_asrc.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 5fda9b647c70..0b28bcfa47fe 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -222,10 +222,9 @@ static int fsl_asrc_request_pair(int channels, struct fsl_asrc_pair *pair)
 	enum asrc_pair_index index = ASRC_INVALID_PAIR;
 	struct fsl_asrc *asrc = pair->asrc;
 	struct device *dev = &asrc->pdev->dev;
-	unsigned long lock_flags;
 	int i, ret = 0;
 
-	spin_lock_irqsave(&asrc->lock, lock_flags);
+	guard(spinlock_irqsave)(&asrc->lock);
 
 	for (i = ASRC_PAIR_A; i < ASRC_PAIR_MAX_NUM; i++) {
 		if (asrc->pair[i] != NULL)
@@ -250,8 +249,6 @@ static int fsl_asrc_request_pair(int channels, struct fsl_asrc_pair *pair)
 		pair->index = index;
 	}
 
-	spin_unlock_irqrestore(&asrc->lock, lock_flags);
-
 	return ret;
 }
 
@@ -265,19 +262,16 @@ static void fsl_asrc_release_pair(struct fsl_asrc_pair *pair)
 {
 	struct fsl_asrc *asrc = pair->asrc;
 	enum asrc_pair_index index = pair->index;
-	unsigned long lock_flags;
 
 	/* Make sure the pair is disabled */
 	regmap_update_bits(asrc->regmap, REG_ASRCTR,
 			   ASRCTR_ASRCEi_MASK(index), 0);
 
-	spin_lock_irqsave(&asrc->lock, lock_flags);
+	guard(spinlock_irqsave)(&asrc->lock);
 
 	asrc->channel_avail += pair->channels;
 	asrc->pair[index] = NULL;
 	pair->error = 0;
-
-	spin_unlock_irqrestore(&asrc->lock, lock_flags);
 }
 
 /**
-- 
2.43.0

[PATCH 02/11] ASoC: fsl_audmix: Use guard() for spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_audmix.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c
index 40a3b7432174..066239c64037 100644
--- a/sound/soc/fsl/fsl_audmix.c
+++ b/sound/soc/fsl/fsl_audmix.c
@@ -280,7 +280,6 @@ static int fsl_audmix_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 				  struct snd_soc_dai *dai)
 {
 	struct fsl_audmix *priv = snd_soc_dai_get_drvdata(dai);
-	unsigned long lock_flags;
 
 	/* Capture stream shall not be handled */
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
@@ -290,16 +289,14 @@ static int fsl_audmix_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		spin_lock_irqsave(&priv->lock, lock_flags);
-		priv->tdms |= BIT(dai->driver->id);
-		spin_unlock_irqrestore(&priv->lock, lock_flags);
+		scoped_guard(spinlock_irqsave, &priv->lock)
+			priv->tdms |= BIT(dai->driver->id);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		spin_lock_irqsave(&priv->lock, lock_flags);
-		priv->tdms &= ~BIT(dai->driver->id);
-		spin_unlock_irqrestore(&priv->lock, lock_flags);
+		scoped_guard(spinlock_irqsave, &priv->lock)
+			priv->tdms &= ~BIT(dai->driver->id);
 		break;
 	default:
 		return -EINVAL;
-- 
2.43.0

[PATCH 03/11] ASoC: fsl_easrc: Use guard() for spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_easrc.c | 36 ++++++++++--------------------------
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
index 114a6c0b6b73..edfd943197a0 100644
--- a/sound/soc/fsl/fsl_easrc.c
+++ b/sound/soc/fsl/fsl_easrc.c
@@ -1025,7 +1025,6 @@ static int fsl_easrc_config_context(struct fsl_asrc *easrc, unsigned int ctx_id)
 	struct fsl_easrc_ctx_priv *ctx_priv;
 	struct fsl_asrc_pair *ctx;
 	struct device *dev;
-	unsigned long lock_flags;
 	int ret;
 
 	if (!easrc)
@@ -1053,9 +1052,8 @@ static int fsl_easrc_config_context(struct fsl_asrc *easrc, unsigned int ctx_id)
 	if (ret)
 		return ret;
 
-	spin_lock_irqsave(&easrc->lock, lock_flags);
-	ret = fsl_easrc_config_slot(easrc, ctx->index);
-	spin_unlock_irqrestore(&easrc->lock, lock_flags);
+	scoped_guard(spinlock_irqsave, &easrc->lock)
+		ret = fsl_easrc_config_slot(easrc, ctx->index);
 	if (ret)
 		return ret;
 
@@ -1301,13 +1299,12 @@ static int fsl_easrc_request_context(int channels, struct fsl_asrc_pair *ctx)
 	enum asrc_pair_index index = ASRC_INVALID_PAIR;
 	struct fsl_asrc *easrc = ctx->asrc;
 	struct device *dev;
-	unsigned long lock_flags;
 	int ret = 0;
 	int i;
 
 	dev = &easrc->pdev->dev;
 
-	spin_lock_irqsave(&easrc->lock, lock_flags);
+	guard(spinlock_irqsave)(&easrc->lock);
 
 	for (i = ASRC_PAIR_A; i < EASRC_CTX_MAX_NUM; i++) {
 		if (easrc->pair[i])
@@ -1331,8 +1328,6 @@ static int fsl_easrc_request_context(int channels, struct fsl_asrc_pair *ctx)
 		easrc->channel_avail -= channels;
 	}
 
-	spin_unlock_irqrestore(&easrc->lock, lock_flags);
-
 	return ret;
 }
 
@@ -1343,7 +1338,6 @@ static int fsl_easrc_request_context(int channels, struct fsl_asrc_pair *ctx)
  */
 static void fsl_easrc_release_context(struct fsl_asrc_pair *ctx)
 {
-	unsigned long lock_flags;
 	struct fsl_asrc *easrc;
 
 	if (!ctx)
@@ -1351,14 +1345,12 @@ static void fsl_easrc_release_context(struct fsl_asrc_pair *ctx)
 
 	easrc = ctx->asrc;
 
-	spin_lock_irqsave(&easrc->lock, lock_flags);
+	guard(spinlock_irqsave)(&easrc->lock);
 
 	fsl_easrc_release_slot(easrc, ctx->index);
 
 	easrc->channel_avail += ctx->channels;
 	easrc->pair[ctx->index] = NULL;
-
-	spin_unlock_irqrestore(&easrc->lock, lock_flags);
 }
 
 /*
@@ -2292,15 +2284,13 @@ static int fsl_easrc_runtime_suspend(struct device *dev)
 {
 	struct fsl_asrc *easrc = dev_get_drvdata(dev);
 	struct fsl_easrc_priv *easrc_priv = easrc->private;
-	unsigned long lock_flags;
 
 	regcache_cache_only(easrc->regmap, true);
 
 	clk_disable_unprepare(easrc->mem_clk);
 
-	spin_lock_irqsave(&easrc->lock, lock_flags);
-	easrc_priv->firmware_loaded = 0;
-	spin_unlock_irqrestore(&easrc->lock, lock_flags);
+	scoped_guard(spinlock_irqsave, &easrc->lock)
+		easrc_priv->firmware_loaded = 0;
 
 	return 0;
 }
@@ -2311,7 +2301,6 @@ static int fsl_easrc_runtime_resume(struct device *dev)
 	struct fsl_easrc_priv *easrc_priv = easrc->private;
 	struct fsl_easrc_ctx_priv *ctx_priv;
 	struct fsl_asrc_pair *ctx;
-	unsigned long lock_flags;
 	int ret;
 	int i;
 
@@ -2323,13 +2312,11 @@ static int fsl_easrc_runtime_resume(struct device *dev)
 	regcache_mark_dirty(easrc->regmap);
 	regcache_sync(easrc->regmap);
 
-	spin_lock_irqsave(&easrc->lock, lock_flags);
-	if (easrc_priv->firmware_loaded) {
-		spin_unlock_irqrestore(&easrc->lock, lock_flags);
-		goto skip_load;
+	scoped_guard(spinlock_irqsave, &easrc->lock) {
+		if (easrc_priv->firmware_loaded)
+			return 0;
+		easrc_priv->firmware_loaded = 1;
 	}
-	easrc_priv->firmware_loaded = 1;
-	spin_unlock_irqrestore(&easrc->lock, lock_flags);
 
 	ret = fsl_easrc_get_firmware(easrc);
 	if (ret) {
@@ -2377,9 +2364,6 @@ static int fsl_easrc_runtime_resume(struct device *dev)
 			goto disable_mem_clk;
 	}
 
-skip_load:
-	return 0;
-
 disable_mem_clk:
 	clk_disable_unprepare(easrc->mem_clk);
 	return ret;
-- 
2.43.0

[PATCH 04/11] ASoC: fsl_esai: Use guard() for spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_esai.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index cde0b0c6c1ef..4a530a6c33f0 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -709,10 +709,9 @@ static void fsl_esai_hw_reset(struct work_struct *work)
 {
 	struct fsl_esai *esai_priv = container_of(work, struct fsl_esai, work);
 	bool tx = true, rx = false, enabled[2];
-	unsigned long lock_flags;
 	u32 tfcr, rfcr;
 
-	spin_lock_irqsave(&esai_priv->lock, lock_flags);
+	guard(spinlock_irqsave)(&esai_priv->lock);
 	/* Save the registers */
 	regmap_read(esai_priv->regmap, REG_ESAI_TFCR, &tfcr);
 	regmap_read(esai_priv->regmap, REG_ESAI_RFCR, &rfcr);
@@ -750,8 +749,6 @@ static void fsl_esai_hw_reset(struct work_struct *work)
 		fsl_esai_trigger_start(esai_priv, tx);
 	if (enabled[rx])
 		fsl_esai_trigger_start(esai_priv, rx);
-
-	spin_unlock_irqrestore(&esai_priv->lock, lock_flags);
 }
 
 static int fsl_esai_trigger(struct snd_pcm_substream *substream, int cmd,
@@ -759,7 +756,6 @@ static int fsl_esai_trigger(struct snd_pcm_substream *substream, int cmd,
 {
 	struct fsl_esai *esai_priv = snd_soc_dai_get_drvdata(dai);
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
-	unsigned long lock_flags;
 
 	esai_priv->channels[tx] = substream->runtime->channels;
 
@@ -767,16 +763,14 @@ static int fsl_esai_trigger(struct snd_pcm_substream *substream, int cmd,
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		spin_lock_irqsave(&esai_priv->lock, lock_flags);
-		fsl_esai_trigger_start(esai_priv, tx);
-		spin_unlock_irqrestore(&esai_priv->lock, lock_flags);
+		scoped_guard(spinlock_irqsave, &esai_priv->lock)
+			fsl_esai_trigger_start(esai_priv, tx);
 		break;
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		spin_lock_irqsave(&esai_priv->lock, lock_flags);
-		fsl_esai_trigger_stop(esai_priv, tx);
-		spin_unlock_irqrestore(&esai_priv->lock, lock_flags);
+		scoped_guard(spinlock_irqsave, &esai_priv->lock)
+			fsl_esai_trigger_stop(esai_priv, tx);
 		break;
 	default:
 		return -EINVAL;
-- 
2.43.0

[PATCH 05/11] ASoC: fsl_spdif: Use guard() for spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_spdif.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 1b9be85b34c2..ad1206ed9882 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -853,17 +853,15 @@ static int fsl_spdif_subcode_get(struct snd_kcontrol *kcontrol,
 	struct snd_soc_dai *cpu_dai = snd_kcontrol_chip(kcontrol);
 	struct fsl_spdif_priv *spdif_priv = snd_soc_dai_get_drvdata(cpu_dai);
 	struct spdif_mixer_control *ctrl = &spdif_priv->fsl_spdif_control;
-	unsigned long flags;
 	int ret = -EAGAIN;
 
-	spin_lock_irqsave(&ctrl->ctl_lock, flags);
+	guard(spinlock_irqsave)(&ctrl->ctl_lock);
 	if (ctrl->ready_buf) {
 		int idx = (ctrl->ready_buf - 1) * SPDIF_UBITS_SIZE;
 		memcpy(&ucontrol->value.iec958.subcode[0],
 				&ctrl->subcode[idx], SPDIF_UBITS_SIZE);
 		ret = 0;
 	}
-	spin_unlock_irqrestore(&ctrl->ctl_lock, flags);
 
 	return ret;
 }
@@ -885,17 +883,15 @@ static int fsl_spdif_qget(struct snd_kcontrol *kcontrol,
 	struct snd_soc_dai *cpu_dai = snd_kcontrol_chip(kcontrol);
 	struct fsl_spdif_priv *spdif_priv = snd_soc_dai_get_drvdata(cpu_dai);
 	struct spdif_mixer_control *ctrl = &spdif_priv->fsl_spdif_control;
-	unsigned long flags;
 	int ret = -EAGAIN;
 
-	spin_lock_irqsave(&ctrl->ctl_lock, flags);
+	guard(spinlock_irqsave)(&ctrl->ctl_lock);
 	if (ctrl->ready_buf) {
 		int idx = (ctrl->ready_buf - 1) * SPDIF_QSUB_SIZE;
 		memcpy(&ucontrol->value.bytes.data[0],
 				&ctrl->qsub[idx], SPDIF_QSUB_SIZE);
 		ret = 0;
 	}
-	spin_unlock_irqrestore(&ctrl->ctl_lock, flags);
 
 	return ret;
 }
-- 
2.43.0

[PATCH 06/11] ASoC: fsl_ssi: Use guard() for mutex locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for mutex locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_ssi.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index b2e1da1781ae..dc022976c982 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -1218,13 +1218,13 @@ static void fsl_ssi_ac97_write(struct snd_ac97 *ac97, unsigned short reg,
 	if (reg > 0x7f)
 		return;
 
-	mutex_lock(&fsl_ac97_data->ac97_reg_lock);
+	guard(mutex)(&fsl_ac97_data->ac97_reg_lock);
 
 	ret = clk_prepare_enable(fsl_ac97_data->clk);
 	if (ret) {
 		pr_err("ac97 write clk_prepare_enable failed: %d\n",
 			ret);
-		goto ret_unlock;
+		return;
 	}
 
 	lreg = reg <<  12;
@@ -1238,9 +1238,6 @@ static void fsl_ssi_ac97_write(struct snd_ac97 *ac97, unsigned short reg,
 	udelay(100);
 
 	clk_disable_unprepare(fsl_ac97_data->clk);
-
-ret_unlock:
-	mutex_unlock(&fsl_ac97_data->ac97_reg_lock);
 }
 
 static unsigned short fsl_ssi_ac97_read(struct snd_ac97 *ac97,
@@ -1252,12 +1249,12 @@ static unsigned short fsl_ssi_ac97_read(struct snd_ac97 *ac97,
 	unsigned int lreg;
 	int ret;
 
-	mutex_lock(&fsl_ac97_data->ac97_reg_lock);
+	guard(mutex)(&fsl_ac97_data->ac97_reg_lock);
 
 	ret = clk_prepare_enable(fsl_ac97_data->clk);
 	if (ret) {
 		pr_err("ac97 read clk_prepare_enable failed: %d\n", ret);
-		goto ret_unlock;
+		return val;
 	}
 
 	lreg = (reg & 0x7f) <<  12;
@@ -1272,8 +1269,6 @@ static unsigned short fsl_ssi_ac97_read(struct snd_ac97 *ac97,
 
 	clk_disable_unprepare(fsl_ac97_data->clk);
 
-ret_unlock:
-	mutex_unlock(&fsl_ac97_data->ac97_reg_lock);
 	return val;
 }
 
-- 
2.43.0

[PATCH 07/11] ASoC: fsl_xcvr: Use guard() for spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_xcvr.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index 6677d3bf36ec..41d100500534 100644
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -797,10 +797,9 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 {
 	struct fsl_xcvr *xcvr = snd_soc_dai_get_drvdata(dai);
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
-	unsigned long lock_flags;
 	int ret = 0;
 
-	spin_lock_irqsave(&xcvr->lock, lock_flags);
+	guard(spinlock_irqsave)(&xcvr->lock);
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
@@ -812,7 +811,7 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 					 FSL_XCVR_EXT_CTRL_DPTH_RESET(tx));
 		if (ret < 0) {
 			dev_err(dai->dev, "Failed to set DPATH RESET: %d\n", ret);
-			goto release_lock;
+			return ret;
 		}
 
 		if (tx) {
@@ -824,7 +823,7 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 						   FSL_XCVR_ISR_CMDC_TX_EN);
 				if (ret < 0) {
 					dev_err(dai->dev, "err updating isr %d\n", ret);
-					goto release_lock;
+					return ret;
 				}
 				fallthrough;
 			case FSL_XCVR_MODE_SPDIF:
@@ -833,7 +832,7 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 						      FSL_XCVR_TX_DPTH_CTRL_STRT_DATA_TX);
 				if (ret < 0) {
 					dev_err(dai->dev, "Failed to start DATA_TX: %d\n", ret);
-					goto release_lock;
+					return ret;
 				}
 				break;
 			}
@@ -844,14 +843,14 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 					 FSL_XCVR_EXT_CTRL_DMA_DIS(tx), 0);
 		if (ret < 0) {
 			dev_err(dai->dev, "Failed to enable DMA: %d\n", ret);
-			goto release_lock;
+			return ret;
 		}
 
 		ret = regmap_update_bits(xcvr->regmap, FSL_XCVR_EXT_IER0,
 					 FSL_XCVR_IRQ_EARC_ALL, FSL_XCVR_IRQ_EARC_ALL);
 		if (ret < 0) {
 			dev_err(dai->dev, "Error while setting IER0: %d\n", ret);
-			goto release_lock;
+			return ret;
 		}
 
 		/* clear DPATH RESET */
@@ -860,7 +859,7 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 					 0);
 		if (ret < 0) {
 			dev_err(dai->dev, "Failed to clear DPATH RESET: %d\n", ret);
-			goto release_lock;
+			return ret;
 		}
 
 		break;
@@ -873,14 +872,14 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 					 FSL_XCVR_EXT_CTRL_DMA_DIS(tx));
 		if (ret < 0) {
 			dev_err(dai->dev, "Failed to disable DMA: %d\n", ret);
-			goto release_lock;
+			return ret;
 		}
 
 		ret = regmap_update_bits(xcvr->regmap, FSL_XCVR_EXT_IER0,
 					 FSL_XCVR_IRQ_EARC_ALL, 0);
 		if (ret < 0) {
 			dev_err(dai->dev, "Failed to clear IER0: %d\n", ret);
-			goto release_lock;
+			return ret;
 		}
 
 		if (tx) {
@@ -891,7 +890,7 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 							FSL_XCVR_TX_DPTH_CTRL_STRT_DATA_TX);
 				if (ret < 0) {
 					dev_err(dai->dev, "Failed to stop DATA_TX: %d\n", ret);
-					goto release_lock;
+					return ret;
 				}
 				if (xcvr->soc_data->spdif_only)
 					break;
@@ -905,7 +904,7 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 				if (ret < 0) {
 					dev_err(dai->dev,
 						"Err updating ISR %d\n", ret);
-					goto release_lock;
+					return ret;
 				}
 				break;
 			}
@@ -916,8 +915,6 @@ static int fsl_xcvr_trigger(struct snd_pcm_substream *substream, int cmd,
 		break;
 	}
 
-release_lock:
-	spin_unlock_irqrestore(&xcvr->lock, lock_flags);
 	return ret;
 }
 
@@ -1448,11 +1445,10 @@ static void reset_rx_work(struct work_struct *work)
 {
 	struct fsl_xcvr *xcvr = container_of(work, struct fsl_xcvr, work_rst);
 	struct device *dev = &xcvr->pdev->dev;
-	unsigned long lock_flags;
 	u32 ext_ctrl;
 
 	dev_dbg(dev, "reset rx path\n");
-	spin_lock_irqsave(&xcvr->lock, lock_flags);
+	guard(spinlock_irqsave)(&xcvr->lock);
 	regmap_read(xcvr->regmap, FSL_XCVR_EXT_CTRL, &ext_ctrl);
 
 	if (!(ext_ctrl & FSL_XCVR_EXT_CTRL_DMA_RD_DIS)) {
@@ -1469,7 +1465,6 @@ static void reset_rx_work(struct work_struct *work)
 				   FSL_XCVR_EXT_CTRL_RX_DPTH_RESET,
 				   0);
 	}
-	spin_unlock_irqrestore(&xcvr->lock, lock_flags);
 }
 
 static irqreturn_t irq0_isr(int irq, void *devid)
-- 
2.43.0

[PATCH 08/11] ASoC: imx-audio-rpmsg: Use guard() for spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/imx-audio-rpmsg.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/sound/soc/fsl/imx-audio-rpmsg.c b/sound/soc/fsl/imx-audio-rpmsg.c
index 38aafb8954c7..b55dfbdb4502 100644
--- a/sound/soc/fsl/imx-audio-rpmsg.c
+++ b/sound/soc/fsl/imx-audio-rpmsg.c
@@ -22,7 +22,6 @@ static int imx_audio_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len,
 	struct rpmsg_r_msg *r_msg = (struct rpmsg_r_msg *)data;
 	struct rpmsg_info *info;
 	struct rpmsg_msg *msg;
-	unsigned long flags;
 
 	if (!rpmsg->rpmsg_pdev)
 		return 0;
@@ -37,21 +36,21 @@ static int imx_audio_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len,
 		/* TYPE C is notification from M core */
 		switch (r_msg->header.cmd) {
 		case TX_PERIOD_DONE:
-			spin_lock_irqsave(&info->lock[TX], flags);
-			msg = &info->msg[TX_PERIOD_DONE + MSG_TYPE_A_NUM];
-			msg->r_msg.param.buffer_tail =
-						r_msg->param.buffer_tail;
-			msg->r_msg.param.buffer_tail %= info->num_period[TX];
-			spin_unlock_irqrestore(&info->lock[TX], flags);
+			scoped_guard(spinlock_irqsave, &info->lock[TX]) {
+				msg = &info->msg[TX_PERIOD_DONE + MSG_TYPE_A_NUM];
+				msg->r_msg.param.buffer_tail =
+							r_msg->param.buffer_tail;
+				msg->r_msg.param.buffer_tail %= info->num_period[TX];
+			}
 			info->callback[TX](info->callback_param[TX]);
 			break;
 		case RX_PERIOD_DONE:
-			spin_lock_irqsave(&info->lock[RX], flags);
-			msg = &info->msg[RX_PERIOD_DONE + MSG_TYPE_A_NUM];
-			msg->r_msg.param.buffer_tail =
-						r_msg->param.buffer_tail;
-			msg->r_msg.param.buffer_tail %= info->num_period[1];
-			spin_unlock_irqrestore(&info->lock[RX], flags);
+			scoped_guard(spinlock_irqsave, &info->lock[RX]) {
+				msg = &info->msg[RX_PERIOD_DONE + MSG_TYPE_A_NUM];
+				msg->r_msg.param.buffer_tail =
+							r_msg->param.buffer_tail;
+				msg->r_msg.param.buffer_tail %= info->num_period[1];
+			}
 			info->callback[RX](info->callback_param[RX]);
 			break;
 		default:
-- 
2.43.0

[PATCH 09/11] ASoC: fsl_rpmsg: Use guard() for mutex & spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for mutex & spin locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/imx-pcm-rpmsg.c | 69 +++++++++++++++--------------------
 1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/sound/soc/fsl/imx-pcm-rpmsg.c b/sound/soc/fsl/imx-pcm-rpmsg.c
index 031e5272215d..7210393dfa5d 100644
--- a/sound/soc/fsl/imx-pcm-rpmsg.c
+++ b/sound/soc/fsl/imx-pcm-rpmsg.c
@@ -39,10 +39,9 @@ static int imx_rpmsg_pcm_send_message(struct rpmsg_msg *msg,
 	struct rpmsg_device *rpdev = info->rpdev;
 	int ret = 0;
 
-	mutex_lock(&info->msg_lock);
+	guard(mutex)(&info->msg_lock);
 	if (!rpdev) {
 		dev_err(info->dev, "rpmsg channel not ready\n");
-		mutex_unlock(&info->msg_lock);
 		return -EINVAL;
 	}
 
@@ -55,15 +54,12 @@ static int imx_rpmsg_pcm_send_message(struct rpmsg_msg *msg,
 			 sizeof(struct rpmsg_s_msg));
 	if (ret) {
 		dev_err(&rpdev->dev, "rpmsg_send failed: %d\n", ret);
-		mutex_unlock(&info->msg_lock);
 		return ret;
 	}
 
 	/* No receive msg for TYPE_C command */
-	if (msg->s_msg.header.type == MSG_TYPE_C) {
-		mutex_unlock(&info->msg_lock);
+	if (msg->s_msg.header.type == MSG_TYPE_C)
 		return 0;
-	}
 
 	/* wait response from rpmsg */
 	ret = wait_for_completion_timeout(&info->cmd_complete,
@@ -71,7 +67,6 @@ static int imx_rpmsg_pcm_send_message(struct rpmsg_msg *msg,
 	if (!ret) {
 		dev_err(&rpdev->dev, "rpmsg_send cmd %d timeout!\n",
 			msg->s_msg.header.cmd);
-		mutex_unlock(&info->msg_lock);
 		return -ETIMEDOUT;
 	}
 
@@ -100,8 +95,6 @@ static int imx_rpmsg_pcm_send_message(struct rpmsg_msg *msg,
 	dev_dbg(&rpdev->dev, "cmd:%d, resp %d\n", msg->s_msg.header.cmd,
 		info->r_msg.param.resp);
 
-	mutex_unlock(&info->msg_lock);
-
 	return 0;
 }
 
@@ -109,14 +102,13 @@ static int imx_rpmsg_insert_workqueue(struct snd_pcm_substream *substream,
 				      struct rpmsg_msg *msg,
 				      struct rpmsg_info *info)
 {
-	unsigned long flags;
 	int ret = 0;
 
 	/*
 	 * Queue the work to workqueue.
 	 * If the queue is full, drop the message.
 	 */
-	spin_lock_irqsave(&info->wq_lock, flags);
+	guard(spinlock_irqsave)(&info->wq_lock);
 	if (info->work_write_index != info->work_read_index) {
 		int index = info->work_write_index;
 
@@ -130,7 +122,6 @@ static int imx_rpmsg_insert_workqueue(struct snd_pcm_substream *substream,
 		info->msg_drop_count[substream->stream]++;
 		ret = -EPIPE;
 	}
-	spin_unlock_irqrestore(&info->wq_lock, flags);
 
 	return ret;
 }
@@ -523,7 +514,6 @@ static int imx_rpmsg_pcm_ack(struct snd_soc_component *component,
 	snd_pcm_sframes_t avail;
 	struct timer_list *timer;
 	struct rpmsg_msg *msg;
-	unsigned long flags;
 	int buffer_tail = 0;
 	int written_num;
 
@@ -553,11 +543,11 @@ static int imx_rpmsg_pcm_ack(struct snd_soc_component *component,
 		msg->s_msg.param.buffer_tail = buffer_tail;
 
 		/* The notification message is updated to latest */
-		spin_lock_irqsave(&info->lock[substream->stream], flags);
-		memcpy(&info->notify[substream->stream], msg,
-		       sizeof(struct rpmsg_s_msg));
-		info->notify_updated[substream->stream] = true;
-		spin_unlock_irqrestore(&info->lock[substream->stream], flags);
+		scoped_guard(spinlock_irqsave, &info->lock[substream->stream]) {
+			memcpy(&info->notify[substream->stream], msg,
+			       sizeof(struct rpmsg_s_msg));
+			info->notify_updated[substream->stream] = true;
+		}
 
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 			avail = snd_pcm_playback_hw_avail(runtime);
@@ -640,7 +630,7 @@ static void imx_rpmsg_pcm_work(struct work_struct *work)
 	bool is_notification = false;
 	struct rpmsg_info *info;
 	struct rpmsg_msg msg;
-	unsigned long flags;
+	bool updated;
 
 	work_of_rpmsg = container_of(work, struct work_of_rpmsg, work);
 	info = work_of_rpmsg->info;
@@ -651,25 +641,26 @@ static void imx_rpmsg_pcm_work(struct work_struct *work)
 	 * enough data in M core side, need to let M core know
 	 * data is updated immediately.
 	 */
-	spin_lock_irqsave(&info->lock[TX], flags);
-	if (info->notify_updated[TX]) {
-		memcpy(&msg, &info->notify[TX], sizeof(struct rpmsg_s_msg));
-		info->notify_updated[TX] = false;
-		spin_unlock_irqrestore(&info->lock[TX], flags);
-		info->send_message(&msg, info);
-	} else {
-		spin_unlock_irqrestore(&info->lock[TX], flags);
+	scoped_guard(spinlock_irqsave, &info->lock[TX]) {
+		updated = info->notify_updated[TX];
+		if (updated) {
+			memcpy(&msg, &info->notify[TX], sizeof(struct rpmsg_s_msg));
+			info->notify_updated[TX] = false;
+		}
 	}
-
-	spin_lock_irqsave(&info->lock[RX], flags);
-	if (info->notify_updated[RX]) {
-		memcpy(&msg, &info->notify[RX], sizeof(struct rpmsg_s_msg));
-		info->notify_updated[RX] = false;
-		spin_unlock_irqrestore(&info->lock[RX], flags);
+	if (updated)
 		info->send_message(&msg, info);
-	} else {
-		spin_unlock_irqrestore(&info->lock[RX], flags);
+
+	scoped_guard(spinlock_irqsave, &info->lock[RX]) {
+		updated = info->notify_updated[RX];
+		if (updated) {
+			memcpy(&msg, &info->notify[RX], sizeof(struct rpmsg_s_msg));
+			info->notify_updated[RX] = false;
+		}
 	}
+	if (updated)
+		info->send_message(&msg, info);
+
 
 	/* Skip the notification message for it has been processed above */
 	if (work_of_rpmsg->msg.s_msg.header.type == MSG_TYPE_C &&
@@ -681,10 +672,10 @@ static void imx_rpmsg_pcm_work(struct work_struct *work)
 		info->send_message(&work_of_rpmsg->msg, info);
 
 	/* update read index */
-	spin_lock_irqsave(&info->wq_lock, flags);
-	info->work_read_index++;
-	info->work_read_index %= WORK_MAX_NUM;
-	spin_unlock_irqrestore(&info->wq_lock, flags);
+	scoped_guard(spinlock_irqsave, &info->wq_lock) {
+		info->work_read_index++;
+		info->work_read_index %= WORK_MAX_NUM;
+	}
 }
 
 static int imx_rpmsg_pcm_probe(struct platform_device *pdev)
-- 
2.43.0

[PATCH 10/11] ASoC: fsl: mpc5200_dma: Use guard() for spin locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/mpc5200_dma.c | 56 ++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
index 56e2cf2f727b..bfedb2dea0b3 100644
--- a/sound/soc/fsl/mpc5200_dma.c
+++ b/sound/soc/fsl/mpc5200_dma.c
@@ -77,18 +77,20 @@ static irqreturn_t psc_dma_bcom_irq(int irq, void *_psc_dma_stream)
 {
 	struct psc_dma_stream *s = _psc_dma_stream;
 
-	spin_lock(&s->psc_dma->lock);
-	/* For each finished period, dequeue the completed period buffer
-	 * and enqueue a new one in it's place. */
-	while (bcom_buffer_done(s->bcom_task)) {
-		bcom_retrieve_buffer(s->bcom_task, NULL, NULL);
+	scoped_guard(spinlock, &s->psc_dma->lock) {
+		/*
+		 * For each finished period, dequeue the completed period buffer
+		 * and enqueue a new one in its place
+		 */
+		while (bcom_buffer_done(s->bcom_task)) {
+			bcom_retrieve_buffer(s->bcom_task, NULL, NULL);
 
-		s->period_current = (s->period_current+1) % s->runtime->periods;
-		s->period_count++;
+			s->period_current = (s->period_current+1) % s->runtime->periods;
+			s->period_count++;
 
-		psc_dma_bcom_enqueue_next_buffer(s);
+			psc_dma_bcom_enqueue_next_buffer(s);
+		}
 	}
-	spin_unlock(&s->psc_dma->lock);
 
 	/* If the stream is active, then also inform the PCM middle layer
 	 * of the period finished event. */
@@ -116,7 +118,6 @@ static int psc_dma_trigger(struct snd_soc_component *component,
 	struct psc_dma_stream *s = to_psc_dma_stream(substream, psc_dma);
 	struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
 	u16 imr;
-	unsigned long flags;
 	int i;
 
 	switch (cmd) {
@@ -135,19 +136,18 @@ static int psc_dma_trigger(struct snd_soc_component *component,
 		/* Fill up the bestcomm bd queue and enable DMA.
 		 * This will begin filling the PSC's fifo.
 		 */
-		spin_lock_irqsave(&psc_dma->lock, flags);
-
-		if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
-			bcom_gen_bd_rx_reset(s->bcom_task);
-		else
-			bcom_gen_bd_tx_reset(s->bcom_task);
+		scoped_guard(spinlock_irqsave, &psc_dma->lock) {
+			if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
+				bcom_gen_bd_rx_reset(s->bcom_task);
+			else
+				bcom_gen_bd_tx_reset(s->bcom_task);
 
-		for (i = 0; i < runtime->periods; i++)
-			if (!bcom_queue_full(s->bcom_task))
-				psc_dma_bcom_enqueue_next_buffer(s);
+			for (i = 0; i < runtime->periods; i++)
+				if (!bcom_queue_full(s->bcom_task))
+					psc_dma_bcom_enqueue_next_buffer(s);
 
-		bcom_enable(s->bcom_task);
-		spin_unlock_irqrestore(&psc_dma->lock, flags);
+			bcom_enable(s->bcom_task);
+		}
 
 		out_8(&regs->command, MPC52xx_PSC_RST_ERR_STAT);
 
@@ -158,13 +158,13 @@ static int psc_dma_trigger(struct snd_soc_component *component,
 			substream->pstr->stream, s->period_count);
 		s->active = 0;
 
-		spin_lock_irqsave(&psc_dma->lock, flags);
-		bcom_disable(s->bcom_task);
-		if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
-			bcom_gen_bd_rx_reset(s->bcom_task);
-		else
-			bcom_gen_bd_tx_reset(s->bcom_task);
-		spin_unlock_irqrestore(&psc_dma->lock, flags);
+		scoped_guard(spinlock_irqsave, &psc_dma->lock) {
+			bcom_disable(s->bcom_task);
+			if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
+				bcom_gen_bd_rx_reset(s->bcom_task);
+			else
+				bcom_gen_bd_tx_reset(s->bcom_task);
+		}
 
 		break;
 
-- 
2.43.0

[PATCH 11/11] ASoC: fsl: mpc5200_psc_ac97: Use guard() for mutex locks

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Clean up the code using guard() for mutex locks.
Merely code refactoring, and no behavior change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/mpc5200_psc_ac97.c | 34 +++++++++++---------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_psc_ac97.c
index 8554fb690772..80666da923d0 100644
--- a/sound/soc/fsl/mpc5200_psc_ac97.c
+++ b/sound/soc/fsl/mpc5200_psc_ac97.c
@@ -31,14 +31,13 @@ static unsigned short psc_ac97_read(struct snd_ac97 *ac97, unsigned short reg)
 	int status;
 	unsigned int val;
 
-	mutex_lock(&psc_dma->mutex);
+	guard(mutex)(&psc_dma->mutex);
 
 	/* Wait for command send status zero = ready */
 	status = spin_event_timeout(!(in_be16(&psc_dma->psc_regs->sr_csr.status) &
 				MPC52xx_PSC_SR_CMDSEND), 100, 0);
 	if (status == 0) {
 		pr_err("timeout on ac97 bus (rdy)\n");
-		mutex_unlock(&psc_dma->mutex);
 		return -ENODEV;
 	}
 
@@ -54,19 +53,16 @@ static unsigned short psc_ac97_read(struct snd_ac97 *ac97, unsigned short reg)
 	if (status == 0) {
 		pr_err("timeout on ac97 read (val) %x\n",
 				in_be16(&psc_dma->psc_regs->sr_csr.status));
-		mutex_unlock(&psc_dma->mutex);
 		return -ENODEV;
 	}
 	/* Get the data */
 	val = in_be32(&psc_dma->psc_regs->ac97_data);
 	if (((val >> 24) & 0x7f) != reg) {
 		pr_err("reg echo error on ac97 read\n");
-		mutex_unlock(&psc_dma->mutex);
 		return -ENODEV;
 	}
 	val = (val >> 8) & 0xffff;
 
-	mutex_unlock(&psc_dma->mutex);
 	return (unsigned short) val;
 }
 
@@ -75,52 +71,46 @@ static void psc_ac97_write(struct snd_ac97 *ac97,
 {
 	int status;
 
-	mutex_lock(&psc_dma->mutex);
+	guard(mutex)(&psc_dma->mutex);
 
 	/* Wait for command status zero = ready */
 	status = spin_event_timeout(!(in_be16(&psc_dma->psc_regs->sr_csr.status) &
 				MPC52xx_PSC_SR_CMDSEND), 100, 0);
 	if (status == 0) {
 		pr_err("timeout on ac97 bus (write)\n");
-		goto out;
+		return;
 	}
 	/* Write data */
 	out_be32(&psc_dma->psc_regs->ac97_cmd,
 			((reg & 0x7f) << 24) | (val << 8));
-
- out:
-	mutex_unlock(&psc_dma->mutex);
 }
 
 static void psc_ac97_warm_reset(struct snd_ac97 *ac97)
 {
 	struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
 
-	mutex_lock(&psc_dma->mutex);
+	guard(mutex)(&psc_dma->mutex);
 
 	out_be32(&regs->sicr, psc_dma->sicr | MPC52xx_PSC_SICR_AWR);
 	udelay(3);
 	out_be32(&regs->sicr, psc_dma->sicr);
-
-	mutex_unlock(&psc_dma->mutex);
 }
 
 static void psc_ac97_cold_reset(struct snd_ac97 *ac97)
 {
 	struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
 
-	mutex_lock(&psc_dma->mutex);
-	dev_dbg(psc_dma->dev, "cold reset\n");
+	scoped_guard(mutex_lock, &psc_dma->mutex) {
+		dev_dbg(psc_dma->dev, "cold reset\n");
 
-	mpc5200_psc_ac97_gpio_reset(psc_dma->id);
+		mpc5200_psc_ac97_gpio_reset(psc_dma->id);
 
-	/* Notify the PSC that a reset has occurred */
-	out_be32(&regs->sicr, psc_dma->sicr | MPC52xx_PSC_SICR_ACRB);
+		/* Notify the PSC that a reset has occurred */
+		out_be32(&regs->sicr, psc_dma->sicr | MPC52xx_PSC_SICR_ACRB);
 
-	/* Re-enable RX and TX */
-	out_8(&regs->command, MPC52xx_PSC_TX_ENABLE | MPC52xx_PSC_RX_ENABLE);
-
-	mutex_unlock(&psc_dma->mutex);
+		/* Re-enable RX and TX */
+		out_8(&regs->command, MPC52xx_PSC_TX_ENABLE | MPC52xx_PSC_RX_ENABLE);
+	}
 
 	usleep_range(1000, 2000);
 	psc_ac97_warm_reset(ac97);
-- 
2.43.0

Re: [PATCH 03/11] ASoC: fsl_easrc: Use guard() for spin locks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The success path of `fsl_easrc_runtime_resume` falls through to the error cleanup label, incorrectly disabling the device clock on success.
- [Medium] Mixing goto-based error handling with scope-based cleanup helpers (`scoped_guard`) in the same function violates subsystem rules.

Pre-existing issues:
- [High] The `firmware_loaded` flag is prematurely set to 1 before firmware loading succeeds, leading to a NULL pointer dereference on subsequent resumes if loading fails.
--

commit 4528b201cb9e89015d267dc100f8b7c044c41e30
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: fsl_easrc: Use guard() for spin locks

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

> diff --git a/sound/soc/fsl/fsl_easrc.c b/sound/soc/fsl/fsl_easrc.c
> index 114a6c0b6b730..edfd943197a00 100644
> --- a/sound/soc/fsl/fsl_easrc.c
> +++ b/sound/soc/fsl/fsl_easrc.c
[ ... ]
> @@ -2323,13 +2312,11 @@ static int fsl_easrc_runtime_resume(struct device *dev)
>  	regcache_mark_dirty(easrc->regmap);
>  	regcache_sync(easrc->regmap);
>  
> -	spin_lock_irqsave(&easrc->lock, lock_flags);
> -	if (easrc_priv->firmware_loaded) {
> -		spin_unlock_irqrestore(&easrc->lock, lock_flags);
> -		goto skip_load;
> +	scoped_guard(spinlock_irqsave, &easrc->lock) {
> +		if (easrc_priv->firmware_loaded)
> +			return 0;
> +		easrc_priv->firmware_loaded = 1;

[Severity: High]
This is a pre-existing issue, but does setting firmware_loaded to 1 here cause
a NULL pointer dereference on subsequent resumes if fsl_easrc_get_firmware()
fails?

If the firmware load fails, it returns an error but leaves firmware_loaded
set to 1.

On the next resume attempt, this function will see firmware_loaded is 1, return
success immediately, and bypass initialization. This causes
fsl_easrc_prefilter_config() to dereference a NULL
easrc_priv->firmware_hdr->prefil_scen pointer.

>  	}
> -	easrc_priv->firmware_loaded = 1;
> -	spin_unlock_irqrestore(&easrc->lock, lock_flags);
>  
>  	ret = fsl_easrc_get_firmware(easrc);
>  	if (ret) {
>  		dev_err(dev, "failed to get firmware\n");
>  		goto disable_mem_clk;

[Severity: Medium]
Does mixing goto-based error handling with scope-based cleanup helpers like
scoped_guard() in the same function violate the cleanup subsystem rules?

The expectation in include/linux/cleanup.h is that functions convert all
resources to scope-based cleanup or none of them.

[ ... ]
> @@ -2377,9 +2364,6 @@ static int fsl_easrc_runtime_resume(struct device *dev)
>  			goto disable_mem_clk;
>  	}
>  
> -skip_load:
> -	return 0;
> -
>  disable_mem_clk:
>  	clk_disable_unprepare(easrc->mem_clk);
>  	return ret;

[Severity: High]
Since the return 0 on the success path was removed, does the success path in
fsl_easrc_runtime_resume() now fall through into the disable_mem_clk label?

It appears this will incorrectly disable the device clock when the firmware
initialization successfully completes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132639.78086-1-phucduc.bui@gmail.com?part=3

Re: [PATCH 07/11] ASoC: fsl_xcvr: Use guard() for spin locks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The work struct `xcvr->work_rst` can be re-scheduled by the IRQ handler after it is cancelled in `fsl_xcvr_remove()`, leading to a Use-After-Free.
--

commit cd76e5a17f7950836eed0759c4053e72e241e545
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: fsl_xcvr: Use guard() for spin locks
    
Clean up the code using guard() for spin locks. Merely code refactoring,
and no behavior change.

> diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
> index 6677d3bf36ec2..41d100500534f 100644
> --- a/sound/soc/fsl/fsl_xcvr.c
> +++ b/sound/soc/fsl/fsl_xcvr.c

[ ... ]

> @@ -1448,11 +1445,10 @@ static void reset_rx_work(struct work_struct *work)

[Severity: High]
This is a pre-existing issue, but could this work struct be rescheduled by an
interrupt after it has been cancelled during removal, leading to a
use-after-free?

In fsl_xcvr_remove(), the driver calls cancel_work_sync(&xcvr->work_rst),
but hardware interrupts are not explicitly disabled before devres cleans
up. Because irq0_isr() was registered with devm_request_irq(), the interrupt
handler remains active after fsl_xcvr_remove() returns.

If an interrupt fires in this window, irq0_isr() can call:

    if (isr & (FSL_XCVR_IRQ_PREAMBLE_MISMATCH |
               ...)) {
        schedule_work(&xcvr->work_rst);
    }

This queues reset_rx_work() on a device structure (xcvr) that devres will soon
free, resulting in a use-after-free when the work queue executes.

>  {
>  	struct fsl_xcvr *xcvr = container_of(work, struct fsl_xcvr, work_rst);
>  	struct device *dev = &xcvr->pdev->dev;
> -	unsigned long lock_flags;
>  	u32 ext_ctrl;
>  
>  	dev_dbg(dev, "reset rx path\n");
> -	spin_lock_irqsave(&xcvr->lock, lock_flags);
> +	guard(spinlock_irqsave)(&xcvr->lock);
>  	regmap_read(xcvr->regmap, FSL_XCVR_EXT_CTRL, &ext_ctrl);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132639.78086-1-phucduc.bui@gmail.com?part=7

Re: [PATCH 05/11] ASoC: fsl_spdif: Use guard() for spin locks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The Q-subcode interrupt handler incorrectly writes its data into the `subcode` (U-subcode) array, leaving `qsub` empty and corrupting U-subcode data.
- [High] The `ctl_lock` spinlock does not protect against concurrent modification by the interrupt handler on SMP systems, leading to a critical data race and severe out-of-bounds read.
--

commit 9c302e33fc7af448093d631b3d2663e0de990905
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: fsl_spdif: Use guard() for spin locks

Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

> diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
> index 1b9be85b34c24..ad1206ed98828 100644
> --- a/sound/soc/fsl/fsl_spdif.c
> +++ b/sound/soc/fsl/fsl_spdif.c
> @@ -853,17 +853,15 @@ static int fsl_spdif_subcode_get(struct snd_kcontrol *kcontrol,
>  	struct snd_soc_dai *cpu_dai = snd_kcontrol_chip(kcontrol);
>  	struct fsl_spdif_priv *spdif_priv = snd_soc_dai_get_drvdata(cpu_dai);
>  	struct spdif_mixer_control *ctrl = &spdif_priv->fsl_spdif_control;
> -	unsigned long flags;
>  	int ret = -EAGAIN;
>  
> -	spin_lock_irqsave(&ctrl->ctl_lock, flags);
> +	guard(spinlock_irqsave)(&ctrl->ctl_lock);
>  	if (ctrl->ready_buf) {
>  		int idx = (ctrl->ready_buf - 1) * SPDIF_UBITS_SIZE;
>  		memcpy(&ucontrol->value.iec958.subcode[0],
>  				&ctrl->subcode[idx], SPDIF_UBITS_SIZE);
>  		ret = 0;
>  	}

[Severity: High]
This is a pre-existing issue, but does ctl_lock actually protect against
concurrent modification by the interrupt handler on SMP systems?

The ctl_lock is acquired here by readers in fsl_spdif_subcode_get() and
fsl_spdif_qget(). While this disables local interrupts, the interrupt
handler (spdif_isr and helpers like spdif_irq_uq_err) can execute
concurrently on another CPU in an SMP system because it does not acquire
ctl_lock.

If the ISR clears ctrl->ready_buf to 0 between the if (ctrl->ready_buf)
check and the calculation of idx, the compiler re-evaluation of
ready_buf could cause idx to evaluate to -96. This would trigger an
out-of-bounds read in memcpy, leaking arbitrary kernel heap memory to
userspace:

sound/soc/fsl/fsl_spdif.c:spdif_irq_uq_err() {
    ...
	/* Drop this U/Q buffer */
	ctrl->ready_buf = 0;
	ctrl->upos = 0;
	ctrl->qpos = 0;
    ...
}

[ ... ]

> @@ -885,17 +883,15 @@ static int fsl_spdif_qget(struct snd_kcontrol *kcontrol,
>  	struct snd_soc_dai *cpu_dai = snd_kcontrol_chip(kcontrol);
>  	struct fsl_spdif_priv *spdif_priv = snd_soc_dai_get_drvdata(cpu_dai);
>  	struct spdif_mixer_control *ctrl = &spdif_priv->fsl_spdif_control;
> -	unsigned long flags;
>  	int ret = -EAGAIN;
>  
> -	spin_lock_irqsave(&ctrl->ctl_lock, flags);
> +	guard(spinlock_irqsave)(&ctrl->ctl_lock);
>  	if (ctrl->ready_buf) {
>  		int idx = (ctrl->ready_buf - 1) * SPDIF_QSUB_SIZE;
>  		memcpy(&ucontrol->value.bytes.data[0],
>  				&ctrl->qsub[idx], SPDIF_QSUB_SIZE);

[Severity: High]
This isn't a bug introduced by this patch, but does the Q-subcode array
actually contain valid data here?

Looking at the Q-subcode interrupt handler spdif_irq_uqrx_full(), it
unconditionally writes its data into the subcode (U-subcode) array, even
when processing a Q-subcode interrupt:

sound/soc/fsl/fsl_spdif.c:spdif_irq_uqrx_full() {
    ...
	ctrl->subcode[*pos++] = val >> 16;
	ctrl->subcode[*pos++] = val >> 8;
	ctrl->subcode[*pos++] = val;
    ...
}

Because qpos increments up to 24 independently of upos, this overwrites
the first 24 bytes of the U-channel buffer (ctrl->subcode) and leaves
qsub empty. As a result, fsl_spdif_qget() will return uninitialized or
zeroed memory from ctrl->qsub, and the U-subcode data returned to
userspace is corrupted.

>  		ret = 0;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132639.78086-1-phucduc.bui@gmail.com?part=5

Re: [PATCH 01/11] ASoC: fsl_asrc: Use guard() for spin locks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unvalidated cleanup in fsl_asrc_release_pair() unconditionally releases a pair without an ownership check, causing hardware state corruption and channel count tracking errors if fsl_asrc_request_pair() fails.
--

commit f44cdf6b851bca6e946cdc01dc94cd1b9c6b9570
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: fsl_asrc: Use guard() for spin locks

Clean up the code using guard() for spin locks. Merely code refactoring, 
and no behavior change.

> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 5fda9b647c704..0b28bcfa47fed 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c

[ ... ]

> @@ -265,19 +262,16 @@ static void fsl_asrc_release_pair(struct fsl_asrc_pair *pair)
>  {
>  	struct fsl_asrc *asrc = pair->asrc;
>  	enum asrc_pair_index index = pair->index;

[Severity: High]
This is a pre-existing issue, but does fsl_asrc_release_pair() unconditionally 
trust the pair's index without verifying ownership?

If fsl_asrc_request_pair() fails (for example, due to insufficient channels), 
it returns an error but leaves pair->index and pair->channels unmodified. 

The ALSA core handles this configuration failure by invoking the hw_free 
callback on the error path:

sound/soc/fsl/fsl_asrc.c:fsl_asrc_dai_hw_free() {
    ...
	if (pair)
		fsl_asrc_release_pair(pair);
    ...
}

Since ownership isn't verified here, fsl_asrc_release_pair() blindly uses the 
stale pair->index, forcefully disables the hardware register for that pair, and 
adds the stale pair->channels back to asrc->channel_avail.

Could this abruptly destroy the active audio session of another process that 
owns the pair, and artificially inflate the available channel count?

> -	unsigned long lock_flags;
>  
>  	/* Make sure the pair is disabled */
>  	regmap_update_bits(asrc->regmap, REG_ASRCTR,
>  			   ASRCTR_ASRCEi_MASK(index), 0);
>  
> -	spin_lock_irqsave(&asrc->lock, lock_flags);
> +	guard(spinlock_irqsave)(&asrc->lock);
>  
>  	asrc->channel_avail += pair->channels;
>  	asrc->pair[index] = NULL;
>  	pair->error = 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132639.78086-1-phucduc.bui@gmail.com?part=1

Re: [PATCH 09/11] ASoC: fsl_rpmsg: Use guard() for mutex & spin locks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Non-synchronous timer cancellation in teardown paths (`imx_rpmsg_pcm_close` and `imx_rpmsg_terminate_all`) can lead to use-after-free, queueing work on a destroyed workqueue, and sending messages after stream closure.
--

commit ad07813ef8b508a94e71b5247875307259ab8409
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: fsl_rpmsg: Use guard() for mutex & spin locks

This commit cleans up the code by using guard() for mutex and spin locks.
This is intended to be a refactoring with no behavior change.

> diff --git a/sound/soc/fsl/imx-pcm-rpmsg.c b/sound/soc/fsl/imx-pcm-rpmsg.c
> index 031e5272215d4..7210393dfa5d9 100644
> --- a/sound/soc/fsl/imx-pcm-rpmsg.c
> +++ b/sound/soc/fsl/imx-pcm-rpmsg.c
> @@ -39,10 +39,9 @@ static int imx_rpmsg_pcm_send_message(struct rpmsg_msg *msg,
>  	struct rpmsg_device *rpdev = info->rpdev;
>  	int ret = 0;
>  
> -	mutex_lock(&info->msg_lock);
> +	guard(mutex)(&info->msg_lock);
>  	if (!rpdev) {
>  		dev_err(info->dev, "rpmsg channel not ready\n");
> -		mutex_unlock(&info->msg_lock);
>  		return -EINVAL;
>  	}
>  

[ ... ]

[Severity: High]
This is a pre-existing issue, but the driver uses timer_delete() instead of
timer_delete_sync() or timer_shutdown_sync() during teardown paths.

In imx_rpmsg_pcm_close():

imx_rpmsg_pcm_close() {
        ...
        /* Flush work in workqueue to make TX_CLOSE is the last message */
        flush_workqueue(info->rpmsg_wq);
        ...
        info->send_message(msg, info);

        timer_delete(&info->stream_timer[substream->stream].timer);
        ...
}

Because timer_delete() is called after flush_workqueue(), could a concurrently
firing timer insert a new TX_PERIOD_DONE work into the workqueue after the
workqueue is flushed and the close message has been sent?

Furthermore, during device unbind or module removal, the workqueue is
destroyed and the info struct is freed.

If the timer callback fires concurrently during teardown, would it attempt to
queue work on the destroyed workqueue and access freed memory, resulting in a
use-after-free?

A similar pattern exists in imx_rpmsg_terminate_all():

imx_rpmsg_terminate_all() {
        ...
        timer_delete(&info->stream_timer[substream->stream].timer);

        return imx_rpmsg_insert_workqueue(substream, msg, info);
}

Does this non-synchronous cancellation allow out-of-order PERIOD_DONE
messages to be sent after the terminate message?

Should timer_delete_sync() be used in these paths to ensure the callback
is completely finished before proceeding?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132639.78086-1-phucduc.bui@gmail.com?part=9

Re: [PATCH 11/11] ASoC: fsl: mpc5200_psc_ac97: Use guard() for mutex locks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Build failure due to invalid guard class name `mutex_lock` in `scoped_guard()`.
--

commit 1d8ae1620f1469808672c10039fce92329ce78b6
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: fsl: mpc5200_psc_ac97: Use guard() for mutex locks

This commit cleans up the MPC5200 PSC AC97 driver by replacing manual
mutex lock and unlock calls with the scope-based guard() and scoped_guard()
macros to simplify execution paths.

> diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_psc_ac97.c
> index 8554fb690772c..80666da923d02 100644
> --- a/sound/soc/fsl/mpc5200_psc_ac97.c
> +++ b/sound/soc/fsl/mpc5200_psc_ac97.c

[ ... ]

> @@ -83,16 +82,15 @@ static void psc_ac97_cold_reset(struct snd_ac97 *ac97)
>  {
>  	struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
>  
> -	mutex_lock(&psc_dma->mutex);
> -	dev_dbg(psc_dma->dev, "cold reset\n");
> +	scoped_guard(mutex_lock, &psc_dma->mutex) {

[Severity: Low]
Does this code compile successfully? The scoped_guard() macro expects the
lock class name as its first argument, which for a standard mutex should be
"mutex", not "mutex_lock".

Passing "mutex_lock" here will likely result in a build failure due to an
undefined guard class type.

> +		dev_dbg(psc_dma->dev, "cold reset\n");
>  
> -	mpc5200_psc_ac97_gpio_reset(psc_dma->id);
> +		mpc5200_psc_ac97_gpio_reset(psc_dma->id);
>  
> -	/* Notify the PSC that a reset has occurred */
> -	out_be32(&regs->sicr, psc_dma->sicr | MPC52xx_PSC_SICR_ACRB);
> +		/* Notify the PSC that a reset has occurred */
> +		out_be32(&regs->sicr, psc_dma->sicr | MPC52xx_PSC_SICR_ACRB);
>  
> -	/* Re-enable RX and TX */
> -	out_8(&regs->command, MPC52xx_PSC_TX_ENABLE | MPC52xx_PSC_RX_ENABLE);
> -
> -	mutex_unlock(&psc_dma->mutex);
> +		/* Re-enable RX and TX */
> +		out_8(&regs->command, MPC52xx_PSC_TX_ENABLE | MPC52xx_PSC_RX_ENABLE);
> +	}
>  
>  	usleep_range(1000, 2000);
>  	psc_ac97_warm_reset(ac97);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132639.78086-1-phucduc.bui@gmail.com?part=11

Re: [PATCH 10/11] ASoC: fsl: mpc5200_dma: Use guard() for spin locks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent ALSA triggers for playback and capture substreams can race when updating the shared `regs->isr_imr.imr` hardware register, potentially dropping interrupt enable bits.
--

commit 41f1f50873252390b25ba84ac77438b563c3e7ea
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: fsl: mpc5200_dma: Use guard() for spin locks

This commit cleans up the mpc5200_dma code by using scoped_guard() for spin
locks. This is intended to be purely a code refactoring with no behavior
change.

> diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
> index 56e2cf2f727ba..bfedb2dea0b32 100644
> --- a/sound/soc/fsl/mpc5200_dma.c
> +++ b/sound/soc/fsl/mpc5200_dma.c

[ ... ]

> @@ -158,13 +158,13 @@ static int psc_dma_trigger(struct snd_soc_component *component,
>  			substream->pstr->stream, s->period_count);
>  		s->active = 0;
>  
> -		spin_lock_irqsave(&psc_dma->lock, flags);
> -		bcom_disable(s->bcom_task);
> -		if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
> -			bcom_gen_bd_rx_reset(s->bcom_task);
> -		else
> -			bcom_gen_bd_tx_reset(s->bcom_task);
> -		spin_unlock_irqrestore(&psc_dma->lock, flags);
> +		scoped_guard(spinlock_irqsave, &psc_dma->lock) {
> +			bcom_disable(s->bcom_task);
> +			if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)
> +				bcom_gen_bd_rx_reset(s->bcom_task);
> +			else
> +				bcom_gen_bd_tx_reset(s->bcom_task);
> +		}
>  
>  		break;
>  

[Severity: High]
This is a pre-existing issue, but can concurrent ALSA triggers for playback
and capture substreams race when updating the shared regs->isr_imr.imr hardware
register?

Looking down at the end of psc_dma_trigger(), the interrupt mask is updated
entirely outside of the psc_dma->lock:

    /* Update interrupt enable settings */
    imr = 0;
    if (psc_dma->playback.active)
        imr |= MPC52xx_PSC_IMR_TXEMP;
    if (psc_dma->capture.active)
        imr |= MPC52xx_PSC_IMR_ORERR;
    out_be16(&regs->isr_imr.imr, psc_dma->imr | imr);

If CPU1 triggers playback and CPU2 triggers capture simultaneously, could
they both read stale values for the other stream's active flag?

If so, one of the concurrent writes to regs->isr_imr.imr would overwrite
the other, potentially dropping either the MPC52xx_PSC_IMR_TXEMP or
MPC52xx_PSC_IMR_ORERR bit. This could leave the corresponding stream without
interrupts and stall the audio stream indefinitely.

Since this patch is refactoring the locking in this function, would it make
sense to expand the scope of the new scoped_guard to cover the s->active
updates and this hardware register write?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132639.78086-1-phucduc.bui@gmail.com?part=10

Re: [PATCH 08/11] ASoC: imx-audio-rpmsg: Use guard() for spin locks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

Pre-existing issues:
- [High] The rpmsg callback `imx_audio_rpmsg_cb` can be invoked before probe initialization completes, leading to a NULL pointer dereference on the `rpmsg` variable.
- [High] Spurious or early messages from the M core cause a NULL pointer dereference on the `info` pointer.
- [High] Spurious or early `TX_PERIOD_DONE` / `RX_PERIOD_DONE` messages cause a divide-by-zero panic due to an uninitialized `num_period` divisor.
- [High] Uninitialized callback execution upon receiving early messages causes a kernel crash.
- [High] The callback `imx_audio_rpmsg_cb` can execute concurrently with or after driver removal, resulting in a Use-After-Free of the `info` and `rpmsg_pdev` structures.
- [High] Platform devices are leaked on the error paths of `imx_audio_rpmsg_probe()` due to missing cleanup.
- [Medium] The callback trusts the length of the incoming rpmsg payload without validation, leading to out-of-bounds reads.
--

commit b1578c7b9299ee080ea08b2217771219cd8e93c4
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: imx-audio-rpmsg: Use guard() for spin locks
    
Clean up the code using guard() for spin locks.
Merely code refactoring, and no behavior change.

> diff --git a/sound/soc/fsl/imx-audio-rpmsg.c b/sound/soc/fsl/imx-audio-rpmsg.c
> index 38aafb8954c76..b55dfbdb45026 100644
> --- a/sound/soc/fsl/imx-audio-rpmsg.c
> +++ b/sound/soc/fsl/imx-audio-rpmsg.c
> @@ -22,7 +22,6 @@ static int imx_audio_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len,
>  	struct rpmsg_r_msg *r_msg = (struct rpmsg_r_msg *)data;
>  	struct rpmsg_info *info;
>  	struct rpmsg_msg *msg;
> -	unsigned long flags;
>  
>  	if (!rpmsg->rpmsg_pdev)

[Severity: High]
This isn't a bug introduced by this patch, but could this lead to a NULL
pointer dereference?

The rpmsg endpoint is created and the callback registered before the driver's
probe function executes. If the remote core sends an early message, this
callback could run while rpmsg is NULL because dev_set_drvdata() hasn't
happened yet.

Additionally, if imx_audio_rpmsg_remove() unregisters the platform device while
the rpmsg endpoint is still active, could an incoming message execute this
callback and dereference freed memory for rpmsg_pdev and info?

>  		return 0;

[ ... ]

> @@ -37,21 +36,21 @@ static int imx_audio_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len,
>  		/* TYPE C is notification from M core */
>  		switch (r_msg->header.cmd) {
>  		case TX_PERIOD_DONE:
> -			spin_lock_irqsave(&info->lock[TX], flags);
> -			msg = &info->msg[TX_PERIOD_DONE + MSG_TYPE_A_NUM];
> -			msg->r_msg.param.buffer_tail =
> -						r_msg->param.buffer_tail;
> -			msg->r_msg.param.buffer_tail %= info->num_period[TX];
> -			spin_unlock_irqrestore(&info->lock[TX], flags);
> +			scoped_guard(spinlock_irqsave, &info->lock[TX]) {

[Severity: High]
This is a pre-existing issue, but can spurious or early messages cause a NULL
pointer dereference here?

If a message arrives before the child probe completes initialization,
platform_get_drvdata() could return NULL for info, causing scoped_guard() to
dereference a NULL pointer when accessing &info->lock[TX].

> +				msg = &info->msg[TX_PERIOD_DONE + MSG_TYPE_A_NUM];
> +				msg->r_msg.param.buffer_tail =
> +							r_msg->param.buffer_tail;
> +				msg->r_msg.param.buffer_tail %= info->num_period[TX];

[Severity: High]
This isn't a bug introduced by this patch, but does this modulo operation
risk a division-by-zero panic?

If a spurious or early TX_PERIOD_DONE message arrives before the ALSA stream
is prepared, info->num_period[TX] will still be initialized to 0.

> +			}
>  			info->callback[TX](info->callback_param[TX]);

[Severity: High]
This is a pre-existing issue, but could this unconditionally execute an
uninitialized function pointer?

If an early message arrives before the ALSA stream starts, info->callback[TX]
might be NULL, triggering a kernel panic.


[ ... ]


There are a few other pre-existing issues in this file that might be worth
looking into:

[Severity: Medium]
This is a pre-existing issue, but does the callback trust the length of the
incoming rpmsg payload without validation?

In imx_audio_rpmsg_cb() for MSG_TYPE_B:

	memcpy(&info->r_msg, r_msg, sizeof(struct rpmsg_r_msg));

The callback casts data to struct rpmsg_r_msg * but doesn't validate that
len >= sizeof(struct rpmsg_r_msg). If the remote peer sends a smaller payload,
this results in an out-of-bounds read from the IPC buffer.

[Severity: High]
This isn't a bug introduced by this patch, but are platform devices leaked on
the error paths of imx_audio_rpmsg_probe()?

If platform_device_register_data() for data->rpmsg_pdev fails, the code records
the error but unconditionally proceeds to register data->card_pdev. If the
latter succeeds, the function returns the error from the first failure, leaving
card_pdev permanently registered. Conversely, if rpmsg_pdev succeeds but
card_pdev fails, it returns without unregistering rpmsg_pdev.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612132639.78086-1-phucduc.bui@gmail.com?part=8

Re: [PATCH 11/11] ASoC: fsl: mpc5200_psc_ac97: Use guard() for mutex locks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

On Fri, Jun 12, 2026 at 08:26:39PM +0700, phucduc.bui@gmail.com wrote:

> -	dev_dbg(psc_dma->dev, "cold reset\n");
> +	scoped_guard(mutex_lock, &psc_dma->mutex) {
> +		dev_dbg(psc_dma->dev, "cold reset\n");

mutex_lock not mutex?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 11/11] ASoC: fsl: mpc5200_psc_ac97: Use guard() for mutex locks

[Bui Duc Phuc]

Hi Mark,

Thank you for the review.

>
> > -     dev_dbg(psc_dma->dev, "cold reset\n");
> > +     scoped_guard(mutex_lock, &psc_dma->mutex) {
> > +             dev_dbg(psc_dma->dev, "cold reset\n");
>
> mutex_lock not mutex?

Sorry for this oversight.

As mentioned in the cover letter, all files were compile-tested except
mpc5200_dma and mpc5200_psc_ac97.
I'll take a closer look next week and send a v2.

Best regards,
Phuc