[alsa-devel] [PATCH] ASoC: Intel: fix broadwell module removing failed issue

[Jie, Yang]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Friday, May 29, 2015 6:52 PM
> To: Jie, Yang
> Cc: Girdwood, Liam R; broonie at kernel.org; alsa-devel at alsa-project.org
> Subject: Re: [PATCH] ASoC: Intel: fix broadwell module removing failed issue
> 
> At Fri, 29 May 2015 07:46:37 +0000,
> Jie, Yang wrote:
> >
> > > -----Original Message-----
> > > From: Girdwood, Liam R
> > > Sent: Thursday, May 28, 2015 11:45 PM
> > > To: Takashi Iwai
> > > Cc: Jie, Yang; broonie at kernel.org; alsa-devel at alsa-project.org
> > > Subject: Re: [PATCH] ASoC: Intel: fix broadwell module removing
> > > failed issue
> > >
> > > On Thu, 2015-05-28 at 17:09 +0200, Takashi Iwai wrote:
> > > > At Thu, 28 May 2015 14:14:18 +0800, Jie Yang wrote:
> > > > >
> > > > > From: Liam Girdwood <liam.r.girdwood at linux.intel.com>
> > > > >
> > > > > In haswell-pcm module unloading, we can't free runtime modules
> > > > > directly, for they may be already freed in runtime suspend.
> > > > >
> > > > > Here add executing suspend call to unload runtime modules, only
> > > > > for status not equal to RPM_SUSPEND, to fix broadwell module
> > > > > removing failed issue.
> > > >
> > > > What if a kernel is built without PM support?  (Practically seen,
> > > > it's never any serious problem, though.)
> >
> > I think or kernel built without PM, empty pm_runtime_xxx()s are called
> > and no problems for that.
> 
> Well, but what about the modules?  Where will they be freed?  Your patch
> description sounds as if they are freed in the runtime suspend path.

The freeing of modules is actually done in fw_unload() stage.

> 
> Also, I'm not quite certain whether it's allowed to do runtime suspend at
> driver free.  Usually it's even forcibly powered up, as we can't leave the
> device as suspended state after free (who can wake up rightly if no driver is
> bound?)
 
You are right. Will change it to that descripted below.

> 
> 
> > > Keyon, it sounds like you will still need hsw_pcm_free_modules() and
> > > should call it after the PM put/disable on module remove. You will
> > > need to add some code too that will check the memory state prior to
> > > doing any unloading though....
> >
> > Yes, I plan to add both check before runtime_module_free() and set
> > pcm_data->runtime to NULL after that. At the same time, make sure
> > hsw_pcm_free_modules() is called before fw_unload() called.
> >
> > With those implemented, suppose the issue can be fixed neatly.
> >
> > I will test it and submit then.
> 
> Yes, this looks like the missing piece.
 
OK, then I will follow this.

Hi Mark, can you help revert these 2 commits? Sorry for the inconvenience
caused.

ASoC: Intel: fix broadwell module removing failed issue
ASoC: Intel: remove unused function hsw_pcm_free_modules()

~Keyon

> 
> 
> Takashi