[alsa-devel] [PATCH] soc-core: let suspend/resume not called if the card is not instantiated

[Barry Song]

On Fri, Nov 13, 2009 at 11:02 PM, Mark Brown
<broonie at opensource.wolfsonmicro.com> wrote:
> On Fri, Nov 13, 2009 at 10:56:13PM +0800, Barry Song wrote:
>
>> This patch is to solve a completely different problem with the last
>> one I sent. Its meaning is very simple actually.  While
>
> No, I get that it's fixing a different problem but it's in a similar
> areas and similar issues apply.  Like I say, tweaking the ordering of
> suspend and resume will actually fix the TDM problem anyway.
>
>> card->instantiated is not 1, it means the card(cpu dai/codec dai and
>> related stuff) and the whole links are not built successfully at all.
>> So devices are not even initialized at all. And the audio system
>> doesn't start to work. Since so, suspend/resume should be not needed
>> for the whole system.
>
> That's not the case unfortunately - some or all of the devices may have
> initialised themselves prior to registering with the SoC core and will
> be expecting the SoC core to give them a callback so that they can
> suspend and resume.  At the minute there's basically an assumption that
> the card is going to come up completely and all we're doing here is
> avoiding race conditions during startup (which is realistic for
> production systems, partially constructed cards aren't likely there
> without broken hardware).
The logic seems different: in snd_soc_instantiate_card(), only while
all cpu DAIs and codec DAIs are found and registered in the system
except ac97, their probes will be called. Then card->instantiated
becomes 1.
A case current codes will cause crash is:
For a CPU DAI driver based on platform driver, if arch hasn't defined
the platform device for cpu dai, then cpu dai is not registered and
initialized in the probe() of its platform driver. Even though its
instance is in card->dai_link, it doesn't really enter the system. But
while suspend/resume, its entries will be called too since the call
doesn't check the existence:
static int soc_suspend(struct device *dev)
{
	...
	for (i = 0; i < card->num_links; i++) {
		struct snd_soc_dai  *cpu_dai = card->dai_link[i].cpu_dai;  /* not
registered but in the link!!! */
		if (cpu_dai->suspend && !cpu_dai->ac97_control)
			cpu_dai->suspend(cpu_dai);     /* not initialized but be suspended??? */
		if (platform->suspend)
			platform->suspend(cpu_dai);
	}
       ...
}
It can easily make system crash. Same case for a not-registered codes too.
I think suspend/resume of a DAI can only be called while it really
exists and can be found in the dai_list, at least.
So I add a overall condition for soc_suspend/resume just like
	if (!card->instantiated)
		return 0
has be done in soc_remove(),  soc_poweroff().

>
> With pm_link we'll be able to move some or all of this ordering stuff
> out of the ASoC core and into the device model which will allow things
> to happen either way without two code paths.
>