[alsa-devel] [PATCH 1/2] ALSA: hda - skip runtime PM if async probe is not finished

[Eoff, Ullysses A]

> -----Original Message-----
> From: alsa-devel-bounces at alsa-project.org [mailto:alsa-devel-bounces at alsa-project.org] On Behalf Of Eoff, Ullysses A
> Sent: Tuesday, July 28, 2015 2:38 PM
> To: Takashi Iwai
> Cc: Yang, Libin; alsa-devel at alsa-project.org; Lee, Zhuo-hao
> Subject: Re: [alsa-devel] [PATCH 1/2] ALSA: hda - skip runtime PM if async probe is not finished
> 
> 
> > -----Original Message-----
> > From: alsa-devel-bounces at alsa-project.org [mailto:alsa-devel-bounces at alsa-project.org] On Behalf Of Takashi Iwai
> > Sent: Monday, July 27, 2015 10:39 PM
> > To: Eoff, Ullysses A
> > Cc: Yang, Libin; alsa-devel at alsa-project.org; Lee, Zhuo-hao
> > Subject: Re: [alsa-devel] [PATCH 1/2] ALSA: hda - skip runtime PM if async probe is not finished
> >
> > On Mon, 27 Jul 2015 23:34:53 +0200,
> > U. Artie Eoff wrote:
> > >
> > > Crash can occur if runtime PM is triggered before the async probe
> > > finishes via the azx_firmware_cb when CONFIG_SND_HDA_PATCH_LOADER
> > > is defined.
> > >
> > > Don't execute PM ops if the async probe has not completed yet.
> > >
> > > The probe is finished when chip->running is true.
> > >
> > > Signed-off-by: U. Artie Eoff <ullysses.a.eoff at intel.com>
> > > ---
> > >  sound/pci/hda/hda_intel.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > > index 735bdcb04ce8..b729b25a6ad6 100644
> > > --- a/sound/pci/hda/hda_intel.c
> > > +++ b/sound/pci/hda/hda_intel.c
> > > @@ -1023,7 +1023,7 @@ static int azx_runtime_idle(struct device *dev)
> > >
> > >  	chip = card->private_data;
> > >  	hda = container_of(chip, struct hda_intel, chip);
> > > -	if (chip->disabled || hda->init_failed)
> > > +	if (chip->disabled || hda->init_failed || !chip->running)
> > >  		return 0;
> >
> > For !chip->running, it's better to return -EBUSY.
> >
> > The disabled and init_failed flags are checked in runtime_suspend()
> > and runtime_resume() to skip the whole procedures, too.
> >
> > And I guess the check of chip->running should suffice even without the
> > second patch.  It assures that the bus is powered on.
> >
> 
> Hmm... wait a second.  I think we still need to check bus->chip_init
> as in the second patch.  I just noticed that the async probe
> finishes (i.e. chip->running = 1) before chip init executes (i.e. before
> bus->chip_init is set).  This allows a small window for the issue
> I described in my second patch to occur.
> 
> I already sent another patch to replace these two... but I didn't
> add the check for bus->chip_init in that patch.  I'll create a v2
> for that but will wait for your feedback first.
> 

Oh nevermind... silly me!  I put prints in the wrong place when
testing this theory (at top of the probe cb).  Once I corrected that, I now
see that chip_init=1 happens during the probe cb before running=1
gets set.  I'm still quite new to this driver so bear with me ;-) 
Appologies for the noise.

THEREFORE, my latest patch should be good to go (pending
your feedback of course :-).

Cheers,
U. Artie

> >
> > thanks,
> >
> > Takashi
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel at alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel