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

[Frank Li <Frank.li@oss.nxp.com>]

On Mon, Jun 15, 2026 at 04:38:22PM +0700, phucduc.bui@gmail.com wrote:
> 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>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.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
>
>