Re: [PATCH 09/27] ASoC: codecs: idt821034: Use guard() for mutex locks

[Herve Codina <herve.codina@bootlin.com>]

Hi,

On Tue, 30 Jun 2026 13:34:31 +0700
phucduc.bui@gmail.com wrote:

> 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/codecs/idt821034.c | 121 +++++++++++++++--------------------
>  1 file changed, 51 insertions(+), 70 deletions(-)
> 
> diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
> index 084090ccef77..078de6c9c395 100644
> --- a/sound/soc/codecs/idt821034.c
> +++ b/sound/soc/codecs/idt821034.c
> @@ -6,6 +6,7 @@
>  //
>  // Author: Herve Codina <herve.codina@bootlin.com>
>  
> +#include <linux/cleanup.h>
>  #include <linux/bitrev.h>
>  #include <linux/gpio/driver.h>
>  #include <linux/module.h>

Alphabetic order. Move <linux/cleanup.h> after <linux/bitrev.h>.

...

> @@ -456,7 +457,7 @@ static int idt821034_kctrl_gain_put(struct snd_kcontrol *kcontrol,
>  
>  	ch = IDT821034_ID_GET_CHAN(mc->reg);
>  
> -	mutex_lock(&idt821034->mutex);
> +	guard(mutex)(&idt821034->mutex);
>  
>  	if (IDT821034_ID_IS_OUT(mc->reg)) {
>  		amp = &idt821034->amps.ch[ch].amp_out;
> @@ -466,21 +467,18 @@ static int idt821034_kctrl_gain_put(struct snd_kcontrol *kcontrol,
>  		gain_type = IDT821034_GAIN_TX;
>  	}
>  
> -	if (amp->gain == val) {
> -		ret = 0;
> -		goto end;
> -	}
> +	if (amp->gain == val)
> +		return 0;
>  
>  	if (!amp->is_muted) {
>  		ret = idt821034_set_gain_channel(idt821034, ch, gain_type, val);
>  		if (ret)
> -			goto end;
> +			return ret;
>  	}
>  
>  	amp->gain = val;
>  	ret = 1; /* The value changed */
> -end:
> -	mutex_unlock(&idt821034->mutex);
> +
>  	return ret;

Instead of
	ret = 1; /* The value changed */
	return ret;

Call directly
	return 1; /* The value changed */

>  }

...
> @@ -521,7 +519,7 @@ static int idt821034_kctrl_mute_put(struct snd_kcontrol *kcontrol,
>  	ch = IDT821034_ID_GET_CHAN(id);
>  	is_mute = !ucontrol->value.integer.value[0];
>  
> -	mutex_lock(&idt821034->mutex);
> +	guard(mutex)(&idt821034->mutex);
>  
>  	if (IDT821034_ID_IS_OUT(id)) {
>  		amp = &idt821034->amps.ch[ch].amp_out;
> @@ -531,20 +529,17 @@ static int idt821034_kctrl_mute_put(struct snd_kcontrol *kcontrol,
>  		gain_type = IDT821034_GAIN_TX;
>  	}
>  
> -	if (amp->is_muted == is_mute) {
> -		ret = 0;
> -		goto end;
> -	}
> +	if (amp->is_muted == is_mute)
> +		return 0;
>  
>  	ret = idt821034_set_gain_channel(idt821034, ch, gain_type,
>  					 is_mute ? 0 : amp->gain);
>  	if (ret)
> -		goto end;
> +		return ret;
>  
>  	amp->is_muted = is_mute;
>  	ret = 1; /* The value changed */
> -end:
> -	mutex_unlock(&idt821034->mutex);
> +
>  	return ret;

Instead of
	ret = 1; /* The value changed */
	return ret;

Call directly
	return 1; /* The value changed */

>  }
>  
> @@ -629,7 +624,7 @@ static int idt821034_power_event(struct snd_soc_dapm_widget *w,
>  	ch = IDT821034_ID_GET_CHAN(id);
>  	mask = IDT821034_ID_IS_OUT(id) ? IDT821034_CONF_PWRUP_RX : IDT821034_CONF_PWRUP_TX;
>  
> -	mutex_lock(&idt821034->mutex);
> +	guard(mutex)(&idt821034->mutex);
>  
>  	power = idt821034_get_channel_power(idt821034, ch);
>  	if (SND_SOC_DAPM_EVENT_ON(event))
> @@ -638,8 +633,6 @@ static int idt821034_power_event(struct snd_soc_dapm_widget *w,
>  		power &= ~mask;
>  	ret = idt821034_set_channel_power(idt821034, ch, power);
>  
> -	mutex_unlock(&idt821034->mutex);
> -
>  	return ret;

Instead of
	ret = idt821034_set_channel_power(idt821034, ch, power);
	return ret;

return directly:

	return idt821034_set_channel_power(idt821034, ch, power);

and remove the 'ret' variable (no more used)

>  }
>  

...
> @@ -771,7 +764,7 @@ static int idt821034_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>  	u8 conf;
>  	int ret;
>  
> -	mutex_lock(&idt821034->mutex);
> +	guard(mutex)(&idt821034->mutex);
>  
>  	conf = idt821034_get_codec_conf(idt821034);
>  
> @@ -785,12 +778,10 @@ static int idt821034_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>  	default:
>  		dev_err(dai->dev, "Unsupported DAI format 0x%x\n",
>  			fmt & SND_SOC_DAIFMT_FORMAT_MASK);
> -		ret = -EINVAL;
> -		goto end;
> +		return -EINVAL;
>  	}
>  	ret = idt821034_set_codec_conf(idt821034, conf);
> -end:
> -	mutex_unlock(&idt821034->mutex);
> +
>  	return ret;

Instead of
	ret = idt821034_set_codec_conf(idt821034, conf);
	return ret;

return directly:
	return idt821034_set_codec_conf(idt821034, conf);

and remove the 'ret' variable.

>  }
>  
> @@ -802,7 +793,7 @@ static int idt821034_dai_hw_params(struct snd_pcm_substream *substream,
>  	u8 conf;
>  	int ret;
>  
> -	mutex_lock(&idt821034->mutex);
> +	guard(mutex)(&idt821034->mutex);
>  
>  	conf = idt821034_get_codec_conf(idt821034);
>  
> @@ -816,12 +807,10 @@ static int idt821034_dai_hw_params(struct snd_pcm_substream *substream,
>  	default:
>  		dev_err(dai->dev, "Unsupported PCM format 0x%x\n",
>  			params_format(params));
> -		ret = -EINVAL;
> -		goto end;
> +		return -EINVAL;
>  	}
>  	ret = idt821034_set_codec_conf(idt821034, conf);
> -end:
> -	mutex_unlock(&idt821034->mutex);
> +
>  	return ret;

Idem here:
	return idt821034_set_codec_conf(idt821034, conf);

and remove 'ret'.


>  }
>  
> @@ -897,11 +886,11 @@ static int idt821034_reset_audio(struct idt821034 *idt821034)
>  	int ret;
>  	u8 i;
>  
> -	mutex_lock(&idt821034->mutex);
> +	guard(mutex)(&idt821034->mutex);
>  
>  	ret = idt821034_set_codec_conf(idt821034, 0);
>  	if (ret)
> -		goto end;
> +		return ret;
>  
>  	for (i = 0; i < IDT821034_NB_CHANNEL; i++) {
>  		idt821034->amps.ch[i].amp_out.gain = IDT821034_GAIN_OUT_INIT_RAW;
> @@ -909,23 +898,22 @@ static int idt821034_reset_audio(struct idt821034 *idt821034)
>  		ret = idt821034_set_gain_channel(idt821034, i, IDT821034_GAIN_RX,
>  						 idt821034->amps.ch[i].amp_out.gain);
>  		if (ret)
> -			goto end;
> +			return ret;
>  
>  		idt821034->amps.ch[i].amp_in.gain = IDT821034_GAIN_IN_INIT_RAW;
>  		idt821034->amps.ch[i].amp_in.is_muted = false;
>  		ret = idt821034_set_gain_channel(idt821034, i, IDT821034_GAIN_TX,
>  						 idt821034->amps.ch[i].amp_in.gain);
>  		if (ret)
> -			goto end;
> +			return ret;
>  
>  		ret = idt821034_set_channel_power(idt821034, i, 0);
>  		if (ret)
> -			goto end;
> +			return ret;
>  	}
>  
>  	ret = 0;
> -end:
> -	mutex_unlock(&idt821034->mutex);
> +
>  	return ret;

Instead of
	ret = 0;
	return ret;

return directly
	return 0;

>  }
>  
...

>  
> @@ -1079,23 +1061,22 @@ static int idt821034_reset_gpio(struct idt821034 *idt821034)
>  	int ret;
>  	u8 i;
>  
> -	mutex_lock(&idt821034->mutex);
> +	guard(mutex)(&idt821034->mutex);
>  
>  	/* IO0 and IO1 as input for all channels and output IO set to 0 */
>  	for (i = 0; i < IDT821034_NB_CHANNEL; i++) {
>  		ret = idt821034_set_slic_conf(idt821034, i,
>  					      IDT821034_SLIC_IO1_IN | IDT821034_SLIC_IO0_IN);
>  		if (ret)
> -			goto end;
> +			return ret;
>  
>  		ret = idt821034_write_slic_raw(idt821034, i, 0);
>  		if (ret)
> -			goto end;
> +			return ret;
>  
>  	}
>  	ret = 0;
> -end:
> -	mutex_unlock(&idt821034->mutex);
> +
>  	return ret;

return 0;

>  }
>  

Best regards,
Hervé