From: Liam Girdwood <lrg@ti.com>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: alsa-devel@alsa-project.org, Misael Lopez Cruz <misael.lopez@ti.com>
Subject: Re: [PATCH] ASoC: dapm: Fix locking during codec shutdown
Date: Fri, 06 Jul 2012 18:10:13 +0100 [thread overview]
Message-ID: <1341594613.4963.11.camel@odin> (raw)
In-Reply-To: <20120706164344.GH4017@opensource.wolfsonmicro.com>
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
next prev parent reply other threads:[~2012-07-06 17:10 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-06 15:57 [PATCH] ASoC: dapm: Fix locking during codec shutdown Liam Girdwood
2012-07-06 16:02 ` Mark Brown
2012-07-06 16:36 ` Liam Girdwood
2012-07-06 16:43 ` Mark Brown
2012-07-06 17:10 ` Liam Girdwood [this message]
2012-07-06 17:20 ` Mark Brown
2012-07-06 18:03 ` Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1341594613.4963.11.camel@odin \
--to=lrg@ti.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=misael.lopez@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.