On Fri, Nov 13, 2009 at 11:02 PM, Mark Brown broonie@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.