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

[Takashi Iwai]

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.

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?)


> > 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.


Takashi