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

From: Bui Duc Phuc

Date: Tue Jun 30 2026 - 05:24:00 EST

On Tue, Jun 30, 2026 at 2:28 PM Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
>
> Hi,
>
> On Tue, 30 Jun 2026 13:34:31 +0700
> phucduc.bui@xxxxxxxxx wrote:
>
> > From: bui duc phuc <phucduc.bui@xxxxxxxxx>
> >
> > Clean up the code using guard() for mutex locks.
> > Merely code refactoring, and no behavior change.
> >
> > Signed-off-by: bui duc phuc <phucduc.bui@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxx>
> >
> > +#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é
>

Hi Hervé,

Thank you for the very detailed review.
I hadn't realized I had missed so many issues.
I'll fix them all and include the updates in v2.

Best regards,
Phuc