[alsa-devel] [PATCH 1/3] ALSA: hda - Handle error from snd_hda_power_up*()

[Chris Wilson]

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 at suse.de>
> ---
> @@ -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?

> @@ -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 at chris-wilson.co.uk>
-Chris