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

On Wed, 27 Jun 2018 18:12:06 +0200, Ville Syrjälä wrote:

On Wed, Jun 27, 2018 at 11:10:32AM +0200, Takashi Iwai wrote:

...

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

sound/pci/hda/hda_beep.c | 3 +- sound/pci/hda/hda_codec.c | 4 ++- sound/pci/hda/hda_controller.c | 4 ++- sound/pci/hda/hda_proc.c | 3 +- sound/pci/hda/hda_sysfs.c | 4 ++- sound/pci/hda/patch_ca0132.c | 53 +++++++++++++++++++++++++++------- sound/pci/hda/patch_realtek.c | 3 +- 7 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/sound/pci/hda/hda_beep.c b/sound/pci/hda/hda_beep.c index 066b5b59c4d7..e9d5fbd6c13a 100644 --- a/sound/pci/hda/hda_beep.c +++ b/sound/pci/hda/hda_beep.c @@ -26,7 +26,8 @@ static void generate_tone(struct hda_beep *beep, int tone) struct hda_codec *codec = beep->codec;

if (tone && !beep->playing) {

• snd_hda_power_up(codec);
  

• if (snd_hda_power_up(codec) < 0)
  

• 	return;
  

  if (beep->power_hook) beep->power_hook(beep, true); beep->playing = 1;

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 20a171ac4bb2..44165f3e344e 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -65,7 +65,9 @@ static int codec_exec_verb(struct hdac_device *dev, unsigned int cmd, return -1;

again:

• snd_hda_power_up_pm(codec);

• err = snd_hda_power_up_pm(codec);
• if (err < 0)

• return err;
  

  mutex_lock(&bus->core.cmd_mutex); if (flags & HDA_RW_NO_RESPONSE_FALLBACK) bus->no_response_fallback = 1;

diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c index a12e594d4e3b..4273be1f3eaa 100644 --- a/sound/pci/hda/hda_controller.c +++ b/sound/pci/hda/hda_controller.c @@ -645,7 +645,9 @@ static int azx_pcm_open(struct snd_pcm_substream *substream) buff_step); snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES, buff_step);

• snd_hda_power_up(apcm->codec);

• err = snd_hda_power_up(apcm->codec);
• if (err < 0)

• return err;
  

Missing azx_release_device() here?

Yes, and even worse, it skips the mutex unlock :-< Let's scratch this patch.

thanks,

Takashi