Re: [PATCH 2/3] ASoC: dapm - Use card mutex for DAPM ops instead of codec mutex.

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1946 bytes --]

On Fri, Mar 02, 2012 at 06:11:11PM +0000, Liam Girdwood wrote:

Thanks for looking at this stuff, it's needed attention for a long time
but it's never been a practical problem and it's always depressed me too
much every time I've thought about looking at it myself.

> This patch updates the DAPM operations that use the codec mutex to
> now use the card mutex PCM subclass for all DAPM ops.

Hrm, this makes "PCM" look misnamed...  should we have a DAPM context,
or perhaps even just a separate lock for DAPM?  If we can have a
separate lock that'd be good, it's usually much simpler (at least for
me) to reason about multiple locks interacting than nesting on the same
lock but it's not always reasonable to split the locks, like with the
PCM stuff calling back into itself.

It looks like we're also missing a lock in snd_soc_dapm_sync() here, we
need to lock that as well as the updaters otherwise we might change the
lists underneath the DAPM run which doesn't work so well.  I might've
missed that, though (and currently we're not exactly 100% on taking that
lock when we should).  That's why snd_soc_jack_report() has the CODEC
mutex for example.

We'll also need to make sure the locking for the register I/O is sorted,
it's OK for the devices using regmap as that locks for us but currently
anything using the ASoC level cache relies on the CODEC mutex being held
for register I/O.  This isn't quite so risky as it was when we had the
advanced caches since the data structure is just a plain array but
writes may still go AWOL and that'd be a nightmare to debug.  I'm
worrying that right now something is relying on the fact that DAPM takes
the CODEC mutex to protect it.  But probably it's OK to fix that up
separately I think, we've got plenty of problems there and either
killing all the ASoC level caches or adding a new lock at the cache
level seem like the way to go both of which are basically orthogonal to
what's going on here.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]