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

[Takashi Iwai <tiwai@suse.de>]

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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel