On Fri, 2012-07-06 at 17:43 +0100, Mark Brown wrote:
On Fri, Jul 06, 2012 at 05:36:15PM +0100, Liam Girdwood wrote:
I don't think there is anything explicitly stopping a power off from happening during a stream shutdown atm without this patch. It was reported to me that this would fail 1 in 20 times on a certain device and that the above patch fixed the issue (1200 tests completed OK after the patch).
I do think we need this patch atm since we are accessing the same resources as the stream event.
So how is suspend and resume safe, for example? It's the same general path through the PM core.
suspend and resume are safe as they call stream_event() that holds the DAPM mutex.
Fwiw, I've searched my email and found Misa's analysis here :-
<device is preparing to reboot> soc_dapm_shutdown_codec() : Insert widget A, B, C, ... in "down_list" soc_dapm_shutdown_codec() : Run power sequence via dapm_seq_run() dapm_seq_run(): Move widget A, B, C, ... from "down_list" to "pending" list for coalesced changes dapm_seq_run_coalesced(): Iterating over A
<system sound indicating device reboot closes> dapm_power_widgets() : Run a STREAM_STOP event a inserts widget B in a different "down_list" (whose next element is not C, but other widget X with diff reg address)
dapm_seq_run_coalesced() : Iterates over B and expected C next, but got the widget X added by dapm_power_widgets() above dapm_seq_run_coalesced() : Widget X doesn't share same reg address as widget A, B, ... hence BUG().
dapm_power_widgets() uses card's power_mutex, but soc_dapm_shutdown_codec() does not, so above scenario is possible. IMO, just protecting codec shutdown with card's power_mutex should suffice as in attached patch.
Liam