The regression caused by snd_soc_dapm_enable_pin

[Takashi Iwai]

On Fri, 05 Nov 2021 09:38:41 +0100,
Takashi Iwai wrote:
> 
> On Fri, 05 Nov 2021 09:33:52 +0100,
> Yu-Hsuan Hsu wrote:
> > 
> > Takashi Iwai <tiwai at suse.de> 於 2021年11月5日 週五 下午3:32寫道：
> > >
> > > On Fri, 05 Nov 2021 03:58:46 +0100,
> > > Yu-Hsuan Hsu wrote:
> > > >
> > > > Hi Takashi,
> > > >
> > > > The patch 5af82c81b2c49cfb1cad84d9eb6eab0e3d1c4842(ASoC: DAPM: Fix
> > > > missing kctl change notifications) caused the regression on some
> > > > ChromeBook.
> > > >
> > > > The reason is that some drivers check the return value of
> > > > snd_soc_dapm_enable_pin, like kabylake_ssp0_trigger(Which caused a
> > > > regression). In addition, some SOF drivers may be also affected by
> > > > this change(e.g. sof_sdw_max98373.c). Could you help to fix it?
> > >
> > > Does the patch below fix the problem?
> > >
> > >
> > > Takashi
> > >
> > >
> > > --- a/sound/soc/soc-dapm.c
> > > +++ b/sound/soc/soc-dapm.c
> > > @@ -3589,10 +3589,10 @@ int snd_soc_dapm_put_pin_switch(struct snd_kcontrol *kcontrol,
> > >         const char *pin = (const char *)kcontrol->private_value;
> > >         int ret;
> > >
> > > -       if (ucontrol->value.integer.value[0])
> > > -               ret = snd_soc_dapm_enable_pin(&T->dapm, pin);
> > > -       else
> > > -               ret = snd_soc_dapm_disable_pin(&card->dapm, pin);
> > > +       mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
> > > +
> > > +       ret = snd_soc_dapm_set_pin(dapm, pin, !!ucontrol->value.integer.value[0]);
> > > +       mutex_unlock(&dapm->card->dapm_mutex);
> > >
> > >         snd_soc_dapm_sync(&card->dapm);
> > >         return ret;
> > > @@ -4506,7 +4506,9 @@ EXPORT_SYMBOL_GPL(snd_soc_dapm_stream_stop);
> > >  int snd_soc_dapm_enable_pin_unlocked(struct snd_soc_dapm_context *dapm,
> > >                                    const char *pin)
> > >  {
> > > -       return snd_soc_dapm_set_pin(dapm, pin, 1);
> > > +       int err = snd_soc_dapm_set_pin(dapm, pin, 1);
> > > +
> > > +       return err < 0 ? err : 0;
> > >  }
> > >  EXPORT_SYMBOL_GPL(snd_soc_dapm_enable_pin_unlocked);
> > >
> > > @@ -4527,7 +4529,7 @@ int snd_soc_dapm_enable_pin(struct snd_soc_dapm_context *dapm, const char *pin)
> > >
> > >         mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
> > >
> > > -       ret = snd_soc_dapm_set_pin(dapm, pin, 1);
> > > +       ret = snd_soc_dapm_enable_pin_unlocked(dapm, pin);
> > >
> > >         mutex_unlock(&dapm->card->dapm_mutex);
> > >
> > > @@ -4618,7 +4620,9 @@ EXPORT_SYMBOL_GPL(snd_soc_dapm_force_enable_pin);
> > >  int snd_soc_dapm_disable_pin_unlocked(struct snd_soc_dapm_context *dapm,
> > >                                     const char *pin)
> > >  {
> > > -       return snd_soc_dapm_set_pin(dapm, pin, 0);
> > > +       int err = snd_soc_dapm_set_pin(dapm, pin, 0);
> > > +
> > > +       return err < 0 ? err : 0;
> > >  }
> > >  EXPORT_SYMBOL_GPL(snd_soc_dapm_disable_pin_unlocked);
> > >
> > > @@ -4639,7 +4643,7 @@ int snd_soc_dapm_disable_pin(struct snd_soc_dapm_context *dapm,
> > >
> > >         mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
> > >
> > > -       ret = snd_soc_dapm_set_pin(dapm, pin, 0);
> > > +       ret = snd_soc_dapm_disable_pin_unlocked(dapm, pin);
> > >
> > >         mutex_unlock(&dapm->card->dapm_mutex);
> > >
> > 
> > No, it's not compilable. There is no variable "dapm" in
> > snd_soc_dapm_put_pin_switch.
> 
> Doh, I should have at least build-tested before posting :-<
> 
> > After changing to
> > &card->dapm.card->dapm_mutex, the issue is fixed. Thanks!
> 
> OK, good to hear.  I'm going to submit the proper patch soon later.

I found yet one more place that calls snd_soc_dapm_set_pin().
So it's maybe better to restore the behavior of snd_soc_dapm_set_pin()
like before.

I'll submit the revised patch.


Takashi