On Wed, 27 Jun 2018 11:36:12 +0200,
Chris Wilson wrote:
>
> Quoting Takashi Iwai (2018-06-27 10:10:32)
> > Although snd_hda_power_up() and snd_hda_power_up_pm() may fail, we
> > haven't dealt with the error properly in many places. It's an unusual
> > situation but still possible.
> >
> > This patch spots these places and adds the proper error paths.
> >
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> > @@ -5407,7 +5428,9 @@ static int ca0132_volume_put(struct snd_kcontrol *kcontrol,
> > int dir = get_amp_direction(kcontrol);
> > unsigned long pval;
> >
> > - snd_hda_power_up(codec);
> > + err = snd_hda_power_up(codec);
> > + if (err < 0)
> > + return err;
> > mutex_lock(&codec->control_mutex);
> > pval = kcontrol->private_value;
> > kcontrol->private_value = HDA_COMPOSE_AMP_VAL(shared_nid, ch,
>
> Should this be deferred until pm is next acquired?
> Or are we regarding pm can only fail nonrecoverably?
The power up path is already pm_runtime_get_sync(), so it's a fatal
error. So I suppose that the failure must be very exceptional, and no
need to complicate things too much.
>
> > @@ -5436,6 +5459,7 @@ static int ca0132_alt_volume_put(struct snd_kcontrol *kcontrol,
> > long *valp = ucontrol->value.integer.value;
> > hda_nid_t vnid = 0;
> > int changed = 1;
> > + int err;
> >
> > switch (nid) {
> > case 0x02:
> > @@ -5456,7 +5480,9 @@ static int ca0132_alt_volume_put(struct snd_kcontrol *kcontrol,
> > valp++;
> > }
> >
> > - snd_hda_power_up(codec);
> > + err = snd_hda_power_up(codec);
> > + if (err < 0)
> > + return err;
> > ca0132_alt_dsp_volume_put(codec, vnid);
> > mutex_lock(&codec->control_mutex);
> > changed = snd_hda_mixer_amp_volume_put(kcontrol, ucontrol);
>
> > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> > index 5ad6c7e5f92e..b0d757cf7aab 100644
> > --- a/sound/pci/hda/patch_realtek.c
> > +++ b/sound/pci/hda/patch_realtek.c
> > @@ -3606,7 +3606,8 @@ static void alc269_fixup_mic_mute_hook(void *private_data, int enabled)
> > pinval |= enabled ? AC_PINCTL_VREF_HIZ : AC_PINCTL_VREF_80;
> > if (spec->mute_led_nid) {
> > /* temporarily power up/down for setting VREF */
> > - snd_hda_power_up_pm(codec);
> > + if (snd_hda_power_up_pm(codec) < 0)
> > + return;
> > snd_hda_set_pin_ctl_cache(codec, spec->mute_led_nid, pinval);
> > snd_hda_power_down_pm(codec);
> > }
>
> Similar questions as for deferred application.
>
> I did not find any imbalance introduced and the error is propagated or
> handled,
> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Thanks!
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
