[RFC/untested] hda-intel: suspend HDA codecs and the PCI device in error cases
diff mbox series

Message ID alpine.DEB.2.20.1902081000370.25485@axis700.grange
State New
Headers show
Series
  • [RFC/untested] hda-intel: suspend HDA codecs and the PCI device in error cases
Related show

Commit Message

Guennadi Liakhovetski Feb. 8, 2019, 10:50 a.m. UTC
From: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

If HDA probing on Intel platforms fails, all the devices, that have
previously been powered up, have to be suspended again.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---

I came across HDA not suspending when no codec driver has successfully 
probed while working on SOF. Then I decided to look how this is handled in 
the mainline Intel HDA code and I think something like this patch is 
needed, however, I don't have a set up to test it. I haven't looked at the 
tegra HDA implementation, possibly they need something similar. Please, 
comment.

 sound/pci/hda/hda_intel.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Takashi Iwai Feb. 8, 2019, 11:06 a.m. UTC | #1
On Fri, 08 Feb 2019 11:50:25 +0100,
Guennadi Liakhovetski wrote:
> 
> From: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> 
> If HDA probing on Intel platforms fails, all the devices, that have
> previously been powered up, have to be suspended again.
> 
> Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> ---
> 
> I came across HDA not suspending when no codec driver has successfully 
> probed while working on SOF. Then I decided to look how this is handled in 
> the mainline Intel HDA code and I think something like this patch is 
> needed, however, I don't have a set up to test it. I haven't looked at the 
> tegra HDA implementation, possibly they need something similar. Please, 
> comment.

This will likely lead to the runtime PM refcount unbalance at
azx_free(), I'm afraid.  chip->running is used as a flag to judge
whether the runtime PM is applied or not, and if you jump to the error
path, the flag won't be set.


thanks,

Takashi

> 
>  sound/pci/hda/hda_intel.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index e784130..d3caa37 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -2229,14 +2229,14 @@ static int azx_probe_continue(struct azx *chip)
>  	/* create codec instances */
>  	err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
>  	if (err < 0)
> -		goto out_free;
> +		goto out_power_down;
>  
>  #ifdef CONFIG_SND_HDA_PATCH_LOADER
>  	if (chip->fw) {
>  		err = snd_hda_load_patch(&chip->bus, chip->fw->size,
>  					 chip->fw->data);
>  		if (err < 0)
> -			goto out_free;
> +			goto out_power_down;
>  #ifndef CONFIG_PM
>  		release_firmware(chip->fw); /* no longer needed */
>  		chip->fw = NULL;
> @@ -2246,12 +2246,12 @@ static int azx_probe_continue(struct azx *chip)
>  	if ((probe_only[dev] & 1) == 0) {
>  		err = azx_codec_configure(chip);
>  		if (err < 0)
> -			goto out_free;
> +			goto out_power_down;
>  	}
>  
>  	err = snd_card_register(chip->card);
>  	if (err < 0)
> -		goto out_free;
> +		goto out_power_down;
>  
>  	setup_vga_switcheroo_runtime_pm(chip);
>  
> @@ -2260,6 +2260,7 @@ static int azx_probe_continue(struct azx *chip)
>  
>  	set_default_power_save(chip);
>  
> +out_power_down:
>  	if (azx_has_pm_runtime(chip))
>  		pm_runtime_put_autosuspend(&pci->dev);
>  
> -- 
> 1.9.3
> 
>
Guennadi Liakhovetski Feb. 8, 2019, 11:33 a.m. UTC | #2
Hi Takashi,

On Fri, 8 Feb 2019, Takashi Iwai wrote:

> On Fri, 08 Feb 2019 11:50:25 +0100,
> Guennadi Liakhovetski wrote:
> > 
> > From: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> > 
> > If HDA probing on Intel platforms fails, all the devices, that have
> > previously been powered up, have to be suspended again.
> > 
> > Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> > ---
> > 
> > I came across HDA not suspending when no codec driver has successfully 
> > probed while working on SOF. Then I decided to look how this is handled in 
> > the mainline Intel HDA code and I think something like this patch is 
> > needed, however, I don't have a set up to test it. I haven't looked at the 
> > tegra HDA implementation, possibly they need something similar. Please, 
> > comment.
> 
> This will likely lead to the runtime PM refcount unbalance at
> azx_free(), I'm afraid.  chip->running is used as a flag to judge
> whether the runtime PM is applied or not, and if you jump to the error
> path, the flag won't be set.

Ok, understand, indeed, that would be a problem now. But isn't that also a 
problem, if azx_probe_continue() doesn't run at all before the azx_free()? 
Wouldn't this fix it?

@@ -1300,7 +1300,7 @@ static int azx_free(struct azx *chip)
 	struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
 	struct hdac_bus *bus = azx_bus(chip);
 
-	if (azx_has_pm_runtime(chip) && chip->running)
+	if (azx_has_pm_runtime(chip))
 		pm_runtime_get_noresume(&pci->dev);
 	chip->running = 0;
 

Thanks
Guennadi

> thanks,
> 
> Takashi
> 
> > 
> >  sound/pci/hda/hda_intel.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index e784130..d3caa37 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -2229,14 +2229,14 @@ static int azx_probe_continue(struct azx *chip)
> >  	/* create codec instances */
> >  	err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
> >  	if (err < 0)
> > -		goto out_free;
> > +		goto out_power_down;
> >  
> >  #ifdef CONFIG_SND_HDA_PATCH_LOADER
> >  	if (chip->fw) {
> >  		err = snd_hda_load_patch(&chip->bus, chip->fw->size,
> >  					 chip->fw->data);
> >  		if (err < 0)
> > -			goto out_free;
> > +			goto out_power_down;
> >  #ifndef CONFIG_PM
> >  		release_firmware(chip->fw); /* no longer needed */
> >  		chip->fw = NULL;
> > @@ -2246,12 +2246,12 @@ static int azx_probe_continue(struct azx *chip)
> >  	if ((probe_only[dev] & 1) == 0) {
> >  		err = azx_codec_configure(chip);
> >  		if (err < 0)
> > -			goto out_free;
> > +			goto out_power_down;
> >  	}
> >  
> >  	err = snd_card_register(chip->card);
> >  	if (err < 0)
> > -		goto out_free;
> > +		goto out_power_down;
> >  
> >  	setup_vga_switcheroo_runtime_pm(chip);
> >  
> > @@ -2260,6 +2260,7 @@ static int azx_probe_continue(struct azx *chip)
> >  
> >  	set_default_power_save(chip);
> >  
> > +out_power_down:
> >  	if (azx_has_pm_runtime(chip))
> >  		pm_runtime_put_autosuspend(&pci->dev);
> >  
> > -- 
> > 1.9.3
> > 
> > 
>
Takashi Iwai Feb. 8, 2019, 11:38 a.m. UTC | #3
On Fri, 08 Feb 2019 12:33:14 +0100,
Guennadi Liakhovetski wrote:
> 
> Hi Takashi,
> 
> On Fri, 8 Feb 2019, Takashi Iwai wrote:
> 
> > On Fri, 08 Feb 2019 11:50:25 +0100,
> > Guennadi Liakhovetski wrote:
> > > 
> > > From: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> > > 
> > > If HDA probing on Intel platforms fails, all the devices, that have
> > > previously been powered up, have to be suspended again.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> > > ---
> > > 
> > > I came across HDA not suspending when no codec driver has successfully 
> > > probed while working on SOF. Then I decided to look how this is handled in 
> > > the mainline Intel HDA code and I think something like this patch is 
> > > needed, however, I don't have a set up to test it. I haven't looked at the 
> > > tegra HDA implementation, possibly they need something similar. Please, 
> > > comment.
> > 
> > This will likely lead to the runtime PM refcount unbalance at
> > azx_free(), I'm afraid.  chip->running is used as a flag to judge
> > whether the runtime PM is applied or not, and if you jump to the error
> > path, the flag won't be set.
> 
> Ok, understand, indeed, that would be a problem now. But isn't that also a 
> problem, if azx_probe_continue() doesn't run at all before the azx_free()? 

In that case, it won't go suspend, so it won't need resume at that
point.

> Wouldn't this fix it?

Not really.  The chip->running is also checked in azx_runtime_idle().

Also, what you're trying to change is the behavior of HD-audio
controller, not about codecs.


thanks,

Takashi

> @@ -1300,7 +1300,7 @@ static int azx_free(struct azx *chip)
>  	struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
>  	struct hdac_bus *bus = azx_bus(chip);
>  
> -	if (azx_has_pm_runtime(chip) && chip->running)
> +	if (azx_has_pm_runtime(chip))
>  		pm_runtime_get_noresume(&pci->dev);
>  	chip->running = 0;
>  
> 
> Thanks
> Guennadi
> 
> > thanks,
> > 
> > Takashi
> > 
> > > 
> > >  sound/pci/hda/hda_intel.c | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > > index e784130..d3caa37 100644
> > > --- a/sound/pci/hda/hda_intel.c
> > > +++ b/sound/pci/hda/hda_intel.c
> > > @@ -2229,14 +2229,14 @@ static int azx_probe_continue(struct azx *chip)
> > >  	/* create codec instances */
> > >  	err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
> > >  	if (err < 0)
> > > -		goto out_free;
> > > +		goto out_power_down;
> > >  
> > >  #ifdef CONFIG_SND_HDA_PATCH_LOADER
> > >  	if (chip->fw) {
> > >  		err = snd_hda_load_patch(&chip->bus, chip->fw->size,
> > >  					 chip->fw->data);
> > >  		if (err < 0)
> > > -			goto out_free;
> > > +			goto out_power_down;
> > >  #ifndef CONFIG_PM
> > >  		release_firmware(chip->fw); /* no longer needed */
> > >  		chip->fw = NULL;
> > > @@ -2246,12 +2246,12 @@ static int azx_probe_continue(struct azx *chip)
> > >  	if ((probe_only[dev] & 1) == 0) {
> > >  		err = azx_codec_configure(chip);
> > >  		if (err < 0)
> > > -			goto out_free;
> > > +			goto out_power_down;
> > >  	}
> > >  
> > >  	err = snd_card_register(chip->card);
> > >  	if (err < 0)
> > > -		goto out_free;
> > > +		goto out_power_down;
> > >  
> > >  	setup_vga_switcheroo_runtime_pm(chip);
> > >  
> > > @@ -2260,6 +2260,7 @@ static int azx_probe_continue(struct azx *chip)
> > >  
> > >  	set_default_power_save(chip);
> > >  
> > > +out_power_down:
> > >  	if (azx_has_pm_runtime(chip))
> > >  		pm_runtime_put_autosuspend(&pci->dev);
> > >  
> > > -- 
> > > 1.9.3
> > > 
> > > 
> > 
>
Guennadi Liakhovetski Feb. 8, 2019, 12:14 p.m. UTC | #4
On Fri, 8 Feb 2019, Takashi Iwai wrote:

> On Fri, 08 Feb 2019 12:33:14 +0100,
> Guennadi Liakhovetski wrote:
> > 
> > Hi Takashi,
> > 
> > On Fri, 8 Feb 2019, Takashi Iwai wrote:
> > 
> > > On Fri, 08 Feb 2019 11:50:25 +0100,
> > > Guennadi Liakhovetski wrote:
> > > > 
> > > > From: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> > > > 
> > > > If HDA probing on Intel platforms fails, all the devices, that have
> > > > previously been powered up, have to be suspended again.
> > > > 
> > > > Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> > > > ---
> > > > 
> > > > I came across HDA not suspending when no codec driver has successfully 
> > > > probed while working on SOF. Then I decided to look how this is handled in 
> > > > the mainline Intel HDA code and I think something like this patch is 
> > > > needed, however, I don't have a set up to test it. I haven't looked at the 
> > > > tegra HDA implementation, possibly they need something similar. Please, 
> > > > comment.
> > > 
> > > This will likely lead to the runtime PM refcount unbalance at
> > > azx_free(), I'm afraid.  chip->running is used as a flag to judge
> > > whether the runtime PM is applied or not, and if you jump to the error
> > > path, the flag won't be set.
> > 
> > Ok, understand, indeed, that would be a problem now. But isn't that also a 
> > problem, if azx_probe_continue() doesn't run at all before the azx_free()? 
> 
> In that case, it won't go suspend, so it won't need resume at that
> point.

Mmh, ok, then we have two different things there - see below.

> > Wouldn't this fix it?
> 
> Not really.  The chip->running is also checked in azx_runtime_idle().
> 
> Also, what you're trying to change is the behavior of HD-audio
> controller, not about codecs.

When talking about codecs I meant the path

azx_probe_continue() -> set_default_power_save() -> 
snd_hda_set_power_save() -> codec_set_power_save() -> 
pm_runtime_suspended()

and those codecs are first runtime-enabled via

azx_probe_continue() -> azx_probe_codecs() ->
snd_hda_codec_new() -> snd_hda_codec_device_init() ->
snd_hdac_device_init() -> pm_runtime_get_noresume()

So for symmetry they should also be runtime-disabled if probing doesn't 
complete successfully, right? And currently this isn't the case, so, there 
is a problem?

As for the controller, alright, I guess, we shouldn't touch its runtime PM 
if probe_continue() didn't succeed. So we only have to change the codec 
runtime PM status in case of a failure or have I misunderstood something 
about them and also their status is balanced in such cases?

Thanks
Guennadi

> thanks,
> 
> Takashi
> 
> > @@ -1300,7 +1300,7 @@ static int azx_free(struct azx *chip)
> >  	struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
> >  	struct hdac_bus *bus = azx_bus(chip);
> >  
> > -	if (azx_has_pm_runtime(chip) && chip->running)
> > +	if (azx_has_pm_runtime(chip))
> >  		pm_runtime_get_noresume(&pci->dev);
> >  	chip->running = 0;
> >  
> > 
> > Thanks
> > Guennadi
> > 
> > > thanks,
> > > 
> > > Takashi
> > > 
> > > > 
> > > >  sound/pci/hda/hda_intel.c | 9 +++++----
> > > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > > > index e784130..d3caa37 100644
> > > > --- a/sound/pci/hda/hda_intel.c
> > > > +++ b/sound/pci/hda/hda_intel.c
> > > > @@ -2229,14 +2229,14 @@ static int azx_probe_continue(struct azx *chip)
> > > >  	/* create codec instances */
> > > >  	err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
> > > >  	if (err < 0)
> > > > -		goto out_free;
> > > > +		goto out_power_down;
> > > >  
> > > >  #ifdef CONFIG_SND_HDA_PATCH_LOADER
> > > >  	if (chip->fw) {
> > > >  		err = snd_hda_load_patch(&chip->bus, chip->fw->size,
> > > >  					 chip->fw->data);
> > > >  		if (err < 0)
> > > > -			goto out_free;
> > > > +			goto out_power_down;
> > > >  #ifndef CONFIG_PM
> > > >  		release_firmware(chip->fw); /* no longer needed */
> > > >  		chip->fw = NULL;
> > > > @@ -2246,12 +2246,12 @@ static int azx_probe_continue(struct azx *chip)
> > > >  	if ((probe_only[dev] & 1) == 0) {
> > > >  		err = azx_codec_configure(chip);
> > > >  		if (err < 0)
> > > > -			goto out_free;
> > > > +			goto out_power_down;
> > > >  	}
> > > >  
> > > >  	err = snd_card_register(chip->card);
> > > >  	if (err < 0)
> > > > -		goto out_free;
> > > > +		goto out_power_down;
> > > >  
> > > >  	setup_vga_switcheroo_runtime_pm(chip);
> > > >  
> > > > @@ -2260,6 +2260,7 @@ static int azx_probe_continue(struct azx *chip)
> > > >  
> > > >  	set_default_power_save(chip);
> > > >  
> > > > +out_power_down:
> > > >  	if (azx_has_pm_runtime(chip))
> > > >  		pm_runtime_put_autosuspend(&pci->dev);
> > > >  
> > > > -- 
> > > > 1.9.3
> > > > 
> > > > 
> > > 
> > 
>
Takashi Iwai Feb. 8, 2019, 1:04 p.m. UTC | #5
On Fri, 08 Feb 2019 13:14:06 +0100,
Guennadi Liakhovetski wrote:
> 
> On Fri, 8 Feb 2019, Takashi Iwai wrote:
> 
> > On Fri, 08 Feb 2019 12:33:14 +0100,
> > Guennadi Liakhovetski wrote:
> > > 
> > > Hi Takashi,
> > > 
> > > On Fri, 8 Feb 2019, Takashi Iwai wrote:
> > > 
> > > > On Fri, 08 Feb 2019 11:50:25 +0100,
> > > > Guennadi Liakhovetski wrote:
> > > > > 
> > > > > From: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> > > > > 
> > > > > If HDA probing on Intel platforms fails, all the devices, that have
> > > > > previously been powered up, have to be suspended again.
> > > > > 
> > > > > Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
> > > > > ---
> > > > > 
> > > > > I came across HDA not suspending when no codec driver has successfully 
> > > > > probed while working on SOF. Then I decided to look how this is handled in 
> > > > > the mainline Intel HDA code and I think something like this patch is 
> > > > > needed, however, I don't have a set up to test it. I haven't looked at the 
> > > > > tegra HDA implementation, possibly they need something similar. Please, 
> > > > > comment.
> > > > 
> > > > This will likely lead to the runtime PM refcount unbalance at
> > > > azx_free(), I'm afraid.  chip->running is used as a flag to judge
> > > > whether the runtime PM is applied or not, and if you jump to the error
> > > > path, the flag won't be set.
> > > 
> > > Ok, understand, indeed, that would be a problem now. But isn't that also a 
> > > problem, if azx_probe_continue() doesn't run at all before the azx_free()? 
> > 
> > In that case, it won't go suspend, so it won't need resume at that
> > point.
> 
> Mmh, ok, then we have two different things there - see below.
> 
> > > Wouldn't this fix it?
> > 
> > Not really.  The chip->running is also checked in azx_runtime_idle().
> > 
> > Also, what you're trying to change is the behavior of HD-audio
> > controller, not about codecs.
> 
> When talking about codecs I meant the path
> 
> azx_probe_continue() -> set_default_power_save() -> 
> snd_hda_set_power_save() -> codec_set_power_save() -> 
> pm_runtime_suspended()
> 
> and those codecs are first runtime-enabled via
> 
> azx_probe_continue() -> azx_probe_codecs() ->
> snd_hda_codec_new() -> snd_hda_codec_device_init() ->
> snd_hdac_device_init() -> pm_runtime_get_noresume()
> 
> So for symmetry they should also be runtime-disabled if probing doesn't 
> complete successfully, right? And currently this isn't the case, so, there 
> is a problem?

It's disabled in the counterpart, snd_hdac_device_exit().

> As for the controller, alright, I guess, we shouldn't touch its runtime PM 
> if probe_continue() didn't succeed. So we only have to change the codec 
> runtime PM status in case of a failure or have I misunderstood something 
> about them and also their status is balanced in such cases?

Actually, I don't care much about the power consumption in the driver
failure path as long as it's balanced.  Ideally we should turn off
everything and sets to the minimum, but this is already an abnormal
state, so you can't expect everything perfect in that state.

If there is a scenario that brings the improper PM refcount, we must
fix it, yes.  Or, we can simplify the code a lot, I'd love to apply,
too.  But for other optimization, it's in a much lower priority,
honestly speaking...


thanks,

Takashi


> Thanks
> Guennadi
> 
> > thanks,
> > 
> > Takashi
> > 
> > > @@ -1300,7 +1300,7 @@ static int azx_free(struct azx *chip)
> > >  	struct hda_intel *hda = container_of(chip, struct hda_intel, chip);
> > >  	struct hdac_bus *bus = azx_bus(chip);
> > >  
> > > -	if (azx_has_pm_runtime(chip) && chip->running)
> > > +	if (azx_has_pm_runtime(chip))
> > >  		pm_runtime_get_noresume(&pci->dev);
> > >  	chip->running = 0;
> > >  
> > > 
> > > Thanks
> > > Guennadi
> > > 
> > > > thanks,
> > > > 
> > > > Takashi
> > > > 
> > > > > 
> > > > >  sound/pci/hda/hda_intel.c | 9 +++++----
> > > > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > > > > index e784130..d3caa37 100644
> > > > > --- a/sound/pci/hda/hda_intel.c
> > > > > +++ b/sound/pci/hda/hda_intel.c
> > > > > @@ -2229,14 +2229,14 @@ static int azx_probe_continue(struct azx *chip)
> > > > >  	/* create codec instances */
> > > > >  	err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
> > > > >  	if (err < 0)
> > > > > -		goto out_free;
> > > > > +		goto out_power_down;
> > > > >  
> > > > >  #ifdef CONFIG_SND_HDA_PATCH_LOADER
> > > > >  	if (chip->fw) {
> > > > >  		err = snd_hda_load_patch(&chip->bus, chip->fw->size,
> > > > >  					 chip->fw->data);
> > > > >  		if (err < 0)
> > > > > -			goto out_free;
> > > > > +			goto out_power_down;
> > > > >  #ifndef CONFIG_PM
> > > > >  		release_firmware(chip->fw); /* no longer needed */
> > > > >  		chip->fw = NULL;
> > > > > @@ -2246,12 +2246,12 @@ static int azx_probe_continue(struct azx *chip)
> > > > >  	if ((probe_only[dev] & 1) == 0) {
> > > > >  		err = azx_codec_configure(chip);
> > > > >  		if (err < 0)
> > > > > -			goto out_free;
> > > > > +			goto out_power_down;
> > > > >  	}
> > > > >  
> > > > >  	err = snd_card_register(chip->card);
> > > > >  	if (err < 0)
> > > > > -		goto out_free;
> > > > > +		goto out_power_down;
> > > > >  
> > > > >  	setup_vga_switcheroo_runtime_pm(chip);
> > > > >  
> > > > > @@ -2260,6 +2260,7 @@ static int azx_probe_continue(struct azx *chip)
> > > > >  
> > > > >  	set_default_power_save(chip);
> > > > >  
> > > > > +out_power_down:
> > > > >  	if (azx_has_pm_runtime(chip))
> > > > >  		pm_runtime_put_autosuspend(&pci->dev);
> > > > >  
> > > > > -- 
> > > > > 1.9.3
> > > > > 
> > > > > 
> > > > 
> > > 
> > 
>

Patch
diff mbox series

diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index e784130..d3caa37 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2229,14 +2229,14 @@  static int azx_probe_continue(struct azx *chip)
 	/* create codec instances */
 	err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
 	if (err < 0)
-		goto out_free;
+		goto out_power_down;
 
 #ifdef CONFIG_SND_HDA_PATCH_LOADER
 	if (chip->fw) {
 		err = snd_hda_load_patch(&chip->bus, chip->fw->size,
 					 chip->fw->data);
 		if (err < 0)
-			goto out_free;
+			goto out_power_down;
 #ifndef CONFIG_PM
 		release_firmware(chip->fw); /* no longer needed */
 		chip->fw = NULL;
@@ -2246,12 +2246,12 @@  static int azx_probe_continue(struct azx *chip)
 	if ((probe_only[dev] & 1) == 0) {
 		err = azx_codec_configure(chip);
 		if (err < 0)
-			goto out_free;
+			goto out_power_down;
 	}
 
 	err = snd_card_register(chip->card);
 	if (err < 0)
-		goto out_free;
+		goto out_power_down;
 
 	setup_vga_switcheroo_runtime_pm(chip);
 
@@ -2260,6 +2260,7 @@  static int azx_probe_continue(struct azx *chip)
 
 	set_default_power_save(chip);
 
+out_power_down:
 	if (azx_has_pm_runtime(chip))
 		pm_runtime_put_autosuspend(&pci->dev);