| Message ID | 20180627091034.22724-2-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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@chris-wilson.co.uk> -Chris
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@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? 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@chris-wilson.co.uk> Thanks! Takashi
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@suse.de> Verdict from CI, https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_2430/issues.html is that this one causes a bunch of pm fallout. Do you mind doing a quick revert? Or working with our CI to find the bad chunk? On a second look, > @@ -7310,7 +7339,8 @@ static void ca0132_free(struct hda_codec *codec) > struct ca0132_spec *spec = codec->spec; > > cancel_delayed_work_sync(&spec->unsol_hp_work); > - snd_hda_power_up(codec); > + if (snd_hda_power_up(codec) < 0) > + goto skip_shutdown; > switch (spec->quirk) { > case QUIRK_SBZ: > sbz_exit_chip(codec); > @@ -7326,6 +7356,7 @@ static void ca0132_free(struct hda_codec *codec) > break; > } > snd_hda_power_down(codec); > + skip_shutdown: > if (spec->mem_base) > iounmap(spec->mem_base); > kfree(spec->spec_init_verbs); would seem to be the only chunk more complicated than the rest. -Chris
On Wed, 27 Jun 2018 17:54:31 +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@suse.de> > > Verdict from CI, > > https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_2430/issues.html > > is that this one causes a bunch of pm fallout. > > Do you mind doing a quick revert? Or working with our CI to find the bad > chunk? Hrm, it doesn't look good -- I revert the branch merge now. Thanks for the quick heads up. Takashi > > On a second look, > > > @@ -7310,7 +7339,8 @@ static void ca0132_free(struct hda_codec *codec) > > struct ca0132_spec *spec = codec->spec; > > > > cancel_delayed_work_sync(&spec->unsol_hp_work); > > - snd_hda_power_up(codec); > > + if (snd_hda_power_up(codec) < 0) > > + goto skip_shutdown; > > switch (spec->quirk) { > > case QUIRK_SBZ: > > sbz_exit_chip(codec); > > @@ -7326,6 +7356,7 @@ static void ca0132_free(struct hda_codec *codec) > > break; > > } > > snd_hda_power_down(codec); > > + skip_shutdown: > > if (spec->mem_base) > > iounmap(spec->mem_base); > > kfree(spec->spec_init_verbs); > > would seem to be the only chunk more complicated than the rest. > -Chris >
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? > if (hinfo->ops.open) > err = hinfo->ops.open(hinfo, apcm->codec, substream); > else
On Wed, 27 Jun 2018 18:09:35 +0200, Takashi Iwai wrote: > > On Wed, 27 Jun 2018 17:54:31 +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@suse.de> > > > > Verdict from CI, > > > > https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_2430/issues.html > > > > is that this one causes a bunch of pm fallout. > > > > Do you mind doing a quick revert? Or working with our CI to find the bad > > chunk? > > Hrm, it doesn't look good -- I revert the branch merge now. > Thanks for the quick heads up. After a deeper look, I found that it's an error -EACCES from pm_runtime_get_sync(). Actually it's no real error but indicates that the runtime PM is disabled. That's the reason it broke things easily... That is, dealing a negative code always as a fatal error is simply wrong. We may filter out -EACCES, but I think we need more careful checks. So the patch isn't worth, so far. thanks, Takashi
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
Hi Takashi,
I love your patch! Perhaps something to improve:
url: https://github.com/0day-ci/linux/commits/Takashi-Iwai/ALSA-hda-Handle-error-from-snd_hda_power_up/20180627-171524
base: https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
smatch warnings:
sound/pci/hda/hda_controller.c:688 azx_pcm_open() warn: inconsistent returns 'mutex:&chip->open_mutex'.
Locked on: line 650
Unlocked on: line 681
# https://github.com/0day-ci/linux/commit/2e036d491f4b7a4a100b2e1cd7056fac63868245
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 2e036d491f4b7a4a100b2e1cd7056fac63868245
vim +688 sound/pci/hda/hda_controller.c
05e84878 Dylan Reid 2014-02-28 591
05e84878 Dylan Reid 2014-02-28 592 static int azx_pcm_open(struct snd_pcm_substream *substream)
05e84878 Dylan Reid 2014-02-28 593 {
05e84878 Dylan Reid 2014-02-28 594 struct azx_pcm *apcm = snd_pcm_substream_chip(substream);
820cc6cf Takashi Iwai 2015-02-20 595 struct hda_pcm_stream *hinfo = to_hda_pcm_stream(substream);
05e84878 Dylan Reid 2014-02-28 596 struct azx *chip = apcm->chip;
05e84878 Dylan Reid 2014-02-28 597 struct azx_dev *azx_dev;
05e84878 Dylan Reid 2014-02-28 598 struct snd_pcm_runtime *runtime = substream->runtime;
05e84878 Dylan Reid 2014-02-28 599 int err;
05e84878 Dylan Reid 2014-02-28 600 int buff_step;
05e84878 Dylan Reid 2014-02-28 601
9a6246ff Takashi Iwai 2015-02-27 602 snd_hda_codec_pcm_get(apcm->info);
05e84878 Dylan Reid 2014-02-28 603 mutex_lock(&chip->open_mutex);
05e84878 Dylan Reid 2014-02-28 604 azx_dev = azx_assign_device(chip, substream);
18486508 Libin Yang 2015-05-12 605 trace_azx_pcm_open(chip, azx_dev);
05e84878 Dylan Reid 2014-02-28 606 if (azx_dev == NULL) {
61ca4107 Takashi Iwai 2015-02-27 607 err = -EBUSY;
61ca4107 Takashi Iwai 2015-02-27 608 goto unlock;
05e84878 Dylan Reid 2014-02-28 609 }
ccc98865 Takashi Iwai 2015-04-14 610 runtime->private_data = azx_dev;
50279d9b Guneshwor Singh 2016-08-04 611
50279d9b Guneshwor Singh 2016-08-04 612 if (chip->gts_present)
50279d9b Guneshwor Singh 2016-08-04 613 azx_pcm_hw.info = azx_pcm_hw.info |
50279d9b Guneshwor Singh 2016-08-04 614 SNDRV_PCM_INFO_HAS_LINK_SYNCHRONIZED_ATIME;
50279d9b Guneshwor Singh 2016-08-04 615
05e84878 Dylan Reid 2014-02-28 616 runtime->hw = azx_pcm_hw;
05e84878 Dylan Reid 2014-02-28 617 runtime->hw.channels_min = hinfo->channels_min;
05e84878 Dylan Reid 2014-02-28 618 runtime->hw.channels_max = hinfo->channels_max;
05e84878 Dylan Reid 2014-02-28 619 runtime->hw.formats = hinfo->formats;
05e84878 Dylan Reid 2014-02-28 620 runtime->hw.rates = hinfo->rates;
05e84878 Dylan Reid 2014-02-28 621 snd_pcm_limit_hw_rates(runtime);
05e84878 Dylan Reid 2014-02-28 622 snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
05e84878 Dylan Reid 2014-02-28 623
05e84878 Dylan Reid 2014-02-28 624 /* avoid wrap-around with wall-clock */
05e84878 Dylan Reid 2014-02-28 625 snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_TIME,
05e84878 Dylan Reid 2014-02-28 626 20,
05e84878 Dylan Reid 2014-02-28 627 178000000);
05e84878 Dylan Reid 2014-02-28 628
05e84878 Dylan Reid 2014-02-28 629 if (chip->align_buffer_size)
05e84878 Dylan Reid 2014-02-28 630 /* constrain buffer sizes to be multiple of 128
05e84878 Dylan Reid 2014-02-28 631 bytes. This is more efficient in terms of memory
05e84878 Dylan Reid 2014-02-28 632 access but isn't required by the HDA spec and
05e84878 Dylan Reid 2014-02-28 633 prevents users from specifying exact period/buffer
05e84878 Dylan Reid 2014-02-28 634 sizes. For example for 44.1kHz, a period size set
05e84878 Dylan Reid 2014-02-28 635 to 20ms will be rounded to 19.59ms. */
05e84878 Dylan Reid 2014-02-28 636 buff_step = 128;
05e84878 Dylan Reid 2014-02-28 637 else
05e84878 Dylan Reid 2014-02-28 638 /* Don't enforce steps on buffer sizes, still need to
05e84878 Dylan Reid 2014-02-28 639 be multiple of 4 bytes (HDA spec). Tested on Intel
05e84878 Dylan Reid 2014-02-28 640 HDA controllers, may not work on all devices where
05e84878 Dylan Reid 2014-02-28 641 option needs to be disabled */
05e84878 Dylan Reid 2014-02-28 642 buff_step = 4;
05e84878 Dylan Reid 2014-02-28 643
05e84878 Dylan Reid 2014-02-28 644 snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_BYTES,
05e84878 Dylan Reid 2014-02-28 645 buff_step);
05e84878 Dylan Reid 2014-02-28 646 snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES,
05e84878 Dylan Reid 2014-02-28 647 buff_step);
2e036d49 Takashi Iwai 2018-06-27 648 err = snd_hda_power_up(apcm->codec);
2e036d49 Takashi Iwai 2018-06-27 649 if (err < 0)
2e036d49 Takashi Iwai 2018-06-27 650 return err;
^^^^^^^^^^
Needs to be a "goto unlock;"
61ca4107 Takashi Iwai 2015-02-27 651 if (hinfo->ops.open)
05e84878 Dylan Reid 2014-02-28 652 err = hinfo->ops.open(hinfo, apcm->codec, substream);
61ca4107 Takashi Iwai 2015-02-27 653 else
61ca4107 Takashi Iwai 2015-02-27 654 err = -ENODEV;
05e84878 Dylan Reid 2014-02-28 655 if (err < 0) {
05e84878 Dylan Reid 2014-02-28 656 azx_release_device(azx_dev);
61ca4107 Takashi Iwai 2015-02-27 657 goto powerdown;
05e84878 Dylan Reid 2014-02-28 658 }
05e84878 Dylan Reid 2014-02-28 659 snd_pcm_limit_hw_rates(runtime);
05e84878 Dylan Reid 2014-02-28 660 /* sanity check */
05e84878 Dylan Reid 2014-02-28 661 if (snd_BUG_ON(!runtime->hw.channels_min) ||
05e84878 Dylan Reid 2014-02-28 662 snd_BUG_ON(!runtime->hw.channels_max) ||
05e84878 Dylan Reid 2014-02-28 663 snd_BUG_ON(!runtime->hw.formats) ||
05e84878 Dylan Reid 2014-02-28 664 snd_BUG_ON(!runtime->hw.rates)) {
05e84878 Dylan Reid 2014-02-28 665 azx_release_device(azx_dev);
61ca4107 Takashi Iwai 2015-02-27 666 if (hinfo->ops.close)
05e84878 Dylan Reid 2014-02-28 667 hinfo->ops.close(hinfo, apcm->codec, substream);
61ca4107 Takashi Iwai 2015-02-27 668 err = -EINVAL;
61ca4107 Takashi Iwai 2015-02-27 669 goto powerdown;
05e84878 Dylan Reid 2014-02-28 670 }
05e84878 Dylan Reid 2014-02-28 671
9e94df3a Pierre-Louis Bossart 2015-02-13 672 /* disable LINK_ATIME timestamps for capture streams
05e84878 Dylan Reid 2014-02-28 673 until we figure out how to handle digital inputs */
9e94df3a Pierre-Louis Bossart 2015-02-13 674 if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
9e94df3a Pierre-Louis Bossart 2015-02-13 675 runtime->hw.info &= ~SNDRV_PCM_INFO_HAS_WALL_CLOCK; /* legacy */
9e94df3a Pierre-Louis Bossart 2015-02-13 676 runtime->hw.info &= ~SNDRV_PCM_INFO_HAS_LINK_ATIME;
9e94df3a Pierre-Louis Bossart 2015-02-13 677 }
05e84878 Dylan Reid 2014-02-28 678
05e84878 Dylan Reid 2014-02-28 679 snd_pcm_set_sync(substream);
05e84878 Dylan Reid 2014-02-28 680 mutex_unlock(&chip->open_mutex);
05e84878 Dylan Reid 2014-02-28 681 return 0;
61ca4107 Takashi Iwai 2015-02-27 682
61ca4107 Takashi Iwai 2015-02-27 683 powerdown:
61ca4107 Takashi Iwai 2015-02-27 684 snd_hda_power_down(apcm->codec);
61ca4107 Takashi Iwai 2015-02-27 685 unlock:
61ca4107 Takashi Iwai 2015-02-27 686 mutex_unlock(&chip->open_mutex);
9a6246ff Takashi Iwai 2015-02-27 687 snd_hda_codec_pcm_put(apcm->info);
61ca4107 Takashi Iwai 2015-02-27 @688 return err;
05e84878 Dylan Reid 2014-02-28 689 }
05e84878 Dylan Reid 2014-02-28 690
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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; if (hinfo->ops.open) err = hinfo->ops.open(hinfo, apcm->codec, substream); else diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index c6b778b2580c..4206749d5130 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -760,7 +760,8 @@ static void print_codec_info(struct snd_info_entry *entry, fg = codec->core.afg; if (!fg) return; - snd_hda_power_up(codec); + if (snd_hda_power_up(codec) < 0) + return; snd_iprintf(buffer, "Default PCM:\n"); print_pcm_caps(buffer, codec, fg); snd_iprintf(buffer, "Default Amp-In caps: "); diff --git a/sound/pci/hda/hda_sysfs.c b/sound/pci/hda/hda_sysfs.c index 6ec79c58d48d..d17a808e72d2 100644 --- a/sound/pci/hda/hda_sysfs.c +++ b/sound/pci/hda/hda_sysfs.c @@ -130,7 +130,9 @@ static int reconfig_codec(struct hda_codec *codec) { int err; - snd_hda_power_up(codec); + err = snd_hda_power_up(codec); + if (err < 0) + return err; codec_info(codec, "hda-codec: reconfiguring\n"); err = snd_hda_codec_reset(codec); if (err < 0) { diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c index 4ff5320378e2..3b58f6032250 100644 --- a/sound/pci/hda/patch_ca0132.c +++ b/sound/pci/hda/patch_ca0132.c @@ -3575,12 +3575,15 @@ static int tuning_ctl_set(struct hda_codec *codec, hda_nid_t nid, unsigned int *lookup, int idx) { int i = 0; + int err; for (i = 0; i < TUNING_CTLS_COUNT; i++) if (nid == ca0132_tuning_ctls[i].nid) break; - snd_hda_power_up(codec); + err = snd_hda_power_up(codec); + if (err < 0) + return err; dspio_set_param(codec, ca0132_tuning_ctls[i].mid, 0x20, ca0132_tuning_ctls[i].req, &(lookup[idx]), sizeof(unsigned int)); @@ -3801,7 +3804,9 @@ static int ca0132_select_out(struct hda_codec *codec) codec_dbg(codec, "ca0132_select_out\n"); - snd_hda_power_up_pm(codec); + err = snd_hda_power_up_pm(codec); + if (err < 0) + return err; auto_jack = spec->vnode_lswitch[VNID_HP_ASEL - VNODE_START_NID]; @@ -3914,7 +3919,9 @@ static int ca0132_alt_select_out(struct hda_codec *codec) codec_dbg(codec, "%s\n", __func__); - snd_hda_power_up_pm(codec); + err = snd_hda_power_up_pm(codec); + if (err < 0) + return err; auto_jack = spec->vnode_lswitch[VNID_HP_ASEL - VNODE_START_NID]; @@ -4225,10 +4232,13 @@ static int ca0132_select_mic(struct hda_codec *codec) struct ca0132_spec *spec = codec->spec; int jack_present; int auto_jack; + int err; codec_dbg(codec, "ca0132_select_mic\n"); - snd_hda_power_up_pm(codec); + err = snd_hda_power_up_pm(codec); + if (err < 0) + return err; auto_jack = spec->vnode_lswitch[VNID_AMIC1_ASEL - VNODE_START_NID]; @@ -4276,10 +4286,13 @@ static int ca0132_alt_select_in(struct hda_codec *codec) { struct ca0132_spec *spec = codec->spec; unsigned int tmp; + int err; codec_dbg(codec, "%s\n", __func__); - snd_hda_power_up_pm(codec); + err = snd_hda_power_up_pm(codec); + if (err < 0) + return err; chipio_set_stream_control(codec, 0x03, 0); chipio_set_stream_control(codec, 0x04, 0); @@ -4701,6 +4714,8 @@ static int ca0132_alt_slider_ctl_set(struct hda_codec *codec, hda_nid_t nid, { int i = 0; unsigned int y; + int err; + /* * For X_BASS, req 2 is actually crossover freq instead of * effect level @@ -4710,7 +4725,9 @@ static int ca0132_alt_slider_ctl_set(struct hda_codec *codec, hda_nid_t nid, else y = 1; - snd_hda_power_up(codec); + err = snd_hda_power_up(codec); + if (err < 0) + return err; if (nid == XBASS_XOVER) { for (i = 0; i < OUT_EFFECTS_COUNT; i++) if (ca0132_effects[i].nid == X_BASS) @@ -5221,11 +5238,14 @@ static int ca0132_switch_put(struct snd_kcontrol *kcontrol, int ch = get_amp_channels(kcontrol); long *valp = ucontrol->value.integer.value; int changed = 1; + int err; codec_dbg(codec, "ca0132_switch_put: nid=0x%x, val=%ld\n", nid, *valp); - snd_hda_power_up(codec); + err = snd_hda_power_up(codec); + if (err < 0) + return err; /* vnode */ if ((nid >= VNODE_START_NID) && (nid < VNODE_END_NID)) { if (ch & 1) { @@ -5390,6 +5410,7 @@ static int ca0132_volume_put(struct snd_kcontrol *kcontrol, hda_nid_t shared_nid = 0; bool effective; int changed = 1; + int err; /* store the left and right volume */ if (ch & 1) { @@ -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, @@ -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); @@ -7190,6 +7216,7 @@ static int ca0132_init(struct hda_codec *codec) struct auto_pin_cfg *cfg = &spec->autocfg; int i; bool dsp_loaded; + int err; /* * If the DSP is already downloaded, and init has been entered again, @@ -7220,7 +7247,9 @@ static int ca0132_init(struct hda_codec *codec) if (spec->quirk == QUIRK_SBZ) sbz_region2_startup(codec); - snd_hda_power_up_pm(codec); + err = snd_hda_power_up_pm(codec); + if (err < 0) + return err; ca0132_init_unsol(codec); ca0132_init_params(codec); @@ -7310,7 +7339,8 @@ static void ca0132_free(struct hda_codec *codec) struct ca0132_spec *spec = codec->spec; cancel_delayed_work_sync(&spec->unsol_hp_work); - snd_hda_power_up(codec); + if (snd_hda_power_up(codec) < 0) + goto skip_shutdown; switch (spec->quirk) { case QUIRK_SBZ: sbz_exit_chip(codec); @@ -7326,6 +7356,7 @@ static void ca0132_free(struct hda_codec *codec) break; } snd_hda_power_down(codec); + skip_shutdown: if (spec->mem_base) iounmap(spec->mem_base); kfree(spec->spec_init_verbs); 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); }
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(-)