[alsa-devel] [PATCH 00/15] Update locking for snd_soc_dapm_xxxx_pin functions
This patchset updates the locking around the snd_soc_dapm_xxxx_pin functions. First we add a locked version of the functions and add locking to the standard call, this also requires us to remove the few places where these functions are externally locked. Then we patch usages of the functions that look like they are expecting to do an atomic update to do so.
Thanks, Charles
Charles Keepax (15): Input - arizona-haptics: Fix double lock of dapm_mutex ASoC: dapm: Add locked version of snd_soc_dapm_xxxx_pin functions ASoC: adav80x: Update locking around use of DAPM pin API ASoC: wm5100: Update locking around use of DAPM pin API ASoC: wm8962: Update locking around use of DAPM pin API ASoC: wm8994: Update locking around use of DAPM pin API ASoC: wm8996: Update locking around use of DAPM pin API ASoC: mfld_machine: Update locking around use of DAPM pin API ASoC: ams-delta: Update locking around use of DAPM pin API ASoC: omap: n810: Update locking around use of DAPM pin API ASoC: omap: rx51: Update locking around use of DAPM pin API ASoC: pxa: corgi: Update locking around use of DAPM pin API ASoC: pxa: magician: Update locking around use of DAPM pin API ASoC: pxa: spitz: Update locking around use of DAPM pin API ASoC: pxa: tosa: Update locking around use of DAPM pin API
drivers/extcon/extcon-arizona.c | 12 --- drivers/input/misc/arizona-haptics.c | 19 ----- include/sound/soc-dapm.h | 8 ++ sound/soc/codecs/adav80x.c | 14 +++- sound/soc/codecs/wm5100.c | 9 ++- sound/soc/codecs/wm8962.c | 12 ++- sound/soc/codecs/wm8994.c | 40 ++++++---- sound/soc/codecs/wm8996.c | 8 ++- sound/soc/intel/mfld_machine.c | 43 ++++++++---- sound/soc/omap/ams-delta.c | 38 ++++++---- sound/soc/omap/n810.c | 20 +++-- sound/soc/omap/rx51.c | 20 +++-- sound/soc/pxa/corgi.c | 40 ++++++----- sound/soc/pxa/magician.c | 20 +++-- sound/soc/pxa/spitz.c | 49 +++++++------ sound/soc/pxa/tosa.c | 26 ++++--- sound/soc/soc-dapm.c | 133 +++++++++++++++++++++++++++++++--- 17 files changed, 338 insertions(+), 173 deletions(-)
snd_soc_dapm_sync takes the dapm_mutex internally, but we currently take it externally as well. This patch fixes this.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- drivers/input/misc/arizona-haptics.c | 11 ++++------- 1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/input/misc/arizona-haptics.c b/drivers/input/misc/arizona-haptics.c index 7a04f54..e7e12a5 100644 --- a/drivers/input/misc/arizona-haptics.c +++ b/drivers/input/misc/arizona-haptics.c @@ -77,16 +77,14 @@ static void arizona_haptics_work(struct work_struct *work) return; }
+ mutex_unlock(dapm_mutex); + ret = snd_soc_dapm_sync(arizona->dapm); if (ret != 0) { dev_err(arizona->dev, "Failed to sync DAPM: %d\n", ret); - mutex_unlock(dapm_mutex); return; } - - mutex_unlock(dapm_mutex); - } else { /* This disable sequence will be a noop if already enabled */ mutex_lock_nested(dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME); @@ -99,16 +97,15 @@ static void arizona_haptics_work(struct work_struct *work) return; }
+ mutex_unlock(dapm_mutex); + ret = snd_soc_dapm_sync(arizona->dapm); if (ret != 0) { dev_err(arizona->dev, "Failed to sync DAPM: %d\n", ret); - mutex_unlock(dapm_mutex); return; }
- mutex_unlock(dapm_mutex); - ret = regmap_update_bits(arizona->regmap, ARIZONA_HAPTICS_CONTROL_1, ARIZONA_HAP_CTRL_MASK,
HI Charles,
On Mon, Feb 17, 2014 at 04:51:29PM +0000, Charles Keepax wrote:
snd_soc_dapm_sync takes the dapm_mutex internally, but we currently take it externally as well. This patch fixes this.
Hmm, from the first glance this needs to go into current release, however it seems that it has been broken by a73fb2df01866b772a48fab93401fe3edbe0b38d 2 years ago so nobody cares...
Looking at the series I am not sure that this is right direction. You actually do want unlocked versions of snd_soc_dapm_* functions so that callers can take dapm mutex and then perform sequence of operations. The currect callers that were taking the mutex should probbaly be changed so that they do not drop it until they called unlocked version of snd_soc_dapm_sync().
Thanks.
On Mon, Feb 17, 2014 at 11:20:26AM -0800, Dmitry Torokhov wrote:
On Mon, Feb 17, 2014 at 04:51:29PM +0000, Charles Keepax wrote:
snd_soc_dapm_sync takes the dapm_mutex internally, but we currently take it externally as well. This patch fixes this.
Hmm, from the first glance this needs to go into current release, however it seems that it has been broken by a73fb2df01866b772a48fab93401fe3edbe0b38d 2 years ago so nobody cares...
Please include plain text versions of commit message descriptions, in this case you're referring to "ASoC: dapm: Use DAPM mutex for DAPM ops instead of codec mutex". I expect nobody noticed in part because a lot of use has been out of tree on older kernels and in part because the issue is a race - there's a good chance things will work just fine without the locking.
Looking at the series I am not sure that this is right direction. You actually do want unlocked versions of snd_soc_dapm_* functions so that callers can take dapm mutex and then perform sequence of operations. The currect callers that were taking the mutex should probbaly be changed so that they do not drop it until they called unlocked version of snd_soc_dapm_sync().
That's all more work than is in scope for a bug fix though.
On Mon, Feb 17, 2014 at 11:20:26AM -0800, Dmitry Torokhov wrote:
HI Charles,
On Mon, Feb 17, 2014 at 04:51:29PM +0000, Charles Keepax wrote:
snd_soc_dapm_sync takes the dapm_mutex internally, but we currently take it externally as well. This patch fixes this.
Hmm, from the first glance this needs to go into current release, however it seems that it has been broken by a73fb2df01866b772a48fab93401fe3edbe0b38d 2 years ago so nobody cares...
Looking at the series I am not sure that this is right direction. You actually do want unlocked versions of snd_soc_dapm_* functions so that callers can take dapm mutex and then perform sequence of operations. The currect callers that were taking the mutex should probbaly be changed so that they do not drop it until they called unlocked version of snd_soc_dapm_sync().
Having looked over this again I would prefer to go with the current fix. It means the fix can be sent to the stable trees and the haptics driver has no requirement for multiple pin updates to be done atomically, so there is not a huge amount to be gained from bundling things under an external lock.
Thanks, Charles
The snd_soc_dapm_xxxx_pin all require the dapm_mutex to be held when they are called as they edit the dirty list, however very few of the callers do so.
This patch adds locked versions of all the functions replacing the existing implementations with one that holds the lock internally. We also fix up the places where the lock was actually held on the caller side.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- drivers/extcon/extcon-arizona.c | 12 --- drivers/input/misc/arizona-haptics.c | 16 ---- include/sound/soc-dapm.h | 8 ++ sound/soc/soc-dapm.c | 133 +++++++++++++++++++++++++++++++--- 4 files changed, 131 insertions(+), 38 deletions(-)
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c index c20602f..98a14f6 100644 --- a/drivers/extcon/extcon-arizona.c +++ b/drivers/extcon/extcon-arizona.c @@ -222,27 +222,19 @@ static void arizona_extcon_pulse_micbias(struct arizona_extcon_info *info) struct snd_soc_dapm_context *dapm = arizona->dapm; int ret;
- mutex_lock(&dapm->card->dapm_mutex); - ret = snd_soc_dapm_force_enable_pin(dapm, widget); if (ret != 0) dev_warn(arizona->dev, "Failed to enable %s: %d\n", widget, ret);
- mutex_unlock(&dapm->card->dapm_mutex); - snd_soc_dapm_sync(dapm);
if (!arizona->pdata.micd_force_micbias) { - mutex_lock(&dapm->card->dapm_mutex); - ret = snd_soc_dapm_disable_pin(arizona->dapm, widget); if (ret != 0) dev_warn(arizona->dev, "Failed to disable %s: %d\n", widget, ret);
- mutex_unlock(&dapm->card->dapm_mutex); - snd_soc_dapm_sync(dapm); } } @@ -304,16 +296,12 @@ static void arizona_stop_mic(struct arizona_extcon_info *info) ARIZONA_MICD_ENA, 0, &change);
- mutex_lock(&dapm->card->dapm_mutex); - ret = snd_soc_dapm_disable_pin(dapm, widget); if (ret != 0) dev_warn(arizona->dev, "Failed to disable %s: %d\n", widget, ret);
- mutex_unlock(&dapm->card->dapm_mutex); - snd_soc_dapm_sync(dapm);
if (info->micd_reva) { diff --git a/drivers/input/misc/arizona-haptics.c b/drivers/input/misc/arizona-haptics.c index e7e12a5..ef2e281 100644 --- a/drivers/input/misc/arizona-haptics.c +++ b/drivers/input/misc/arizona-haptics.c @@ -37,7 +37,6 @@ static void arizona_haptics_work(struct work_struct *work) struct arizona_haptics, work); struct arizona *arizona = haptics->arizona; - struct mutex *dapm_mutex = &arizona->dapm->card->dapm_mutex; int ret;
if (!haptics->arizona->dapm) { @@ -67,18 +66,13 @@ static void arizona_haptics_work(struct work_struct *work) return; }
- mutex_lock_nested(dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME); - ret = snd_soc_dapm_enable_pin(arizona->dapm, "HAPTICS"); if (ret != 0) { dev_err(arizona->dev, "Failed to start HAPTICS: %d\n", ret); - mutex_unlock(dapm_mutex); return; }
- mutex_unlock(dapm_mutex); - ret = snd_soc_dapm_sync(arizona->dapm); if (ret != 0) { dev_err(arizona->dev, "Failed to sync DAPM: %d\n", @@ -87,18 +81,13 @@ static void arizona_haptics_work(struct work_struct *work) } } else { /* This disable sequence will be a noop if already enabled */ - mutex_lock_nested(dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME); - ret = snd_soc_dapm_disable_pin(arizona->dapm, "HAPTICS"); if (ret != 0) { dev_err(arizona->dev, "Failed to disable HAPTICS: %d\n", ret); - mutex_unlock(dapm_mutex); return; }
- mutex_unlock(dapm_mutex); - ret = snd_soc_dapm_sync(arizona->dapm); if (ret != 0) { dev_err(arizona->dev, "Failed to sync DAPM: %d\n", @@ -152,16 +141,11 @@ static int arizona_haptics_play(struct input_dev *input, void *data, static void arizona_haptics_close(struct input_dev *input) { struct arizona_haptics *haptics = input_get_drvdata(input); - struct mutex *dapm_mutex = &haptics->arizona->dapm->card->dapm_mutex;
cancel_work_sync(&haptics->work);
- mutex_lock_nested(dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME); - if (haptics->arizona->dapm) snd_soc_dapm_disable_pin(haptics->arizona->dapm, "HAPTICS"); - - mutex_unlock(dapm_mutex); }
static int arizona_haptics_probe(struct platform_device *pdev) diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index 68d92e3..bc8f6cd 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -449,14 +449,22 @@ void snd_soc_dapm_debugfs_init(struct snd_soc_dapm_context *dapm, /* dapm audio pin control and status */ int snd_soc_dapm_enable_pin(struct snd_soc_dapm_context *dapm, const char *pin); +int snd_soc_dapm_enable_pin_locked(struct snd_soc_dapm_context *dapm, + const char *pin); int snd_soc_dapm_disable_pin(struct snd_soc_dapm_context *dapm, const char *pin); +int snd_soc_dapm_disable_pin_locked(struct snd_soc_dapm_context *dapm, + const char *pin); int snd_soc_dapm_nc_pin(struct snd_soc_dapm_context *dapm, const char *pin); +int snd_soc_dapm_nc_pin_locked(struct snd_soc_dapm_context *dapm, + const char *pin); int snd_soc_dapm_get_pin_status(struct snd_soc_dapm_context *dapm, const char *pin); int snd_soc_dapm_sync(struct snd_soc_dapm_context *dapm); int snd_soc_dapm_force_enable_pin(struct snd_soc_dapm_context *dapm, const char *pin); +int snd_soc_dapm_force_enable_pin_locked(struct snd_soc_dapm_context *dapm, + const char *pin); int snd_soc_dapm_ignore_suspend(struct snd_soc_dapm_context *dapm, const char *pin); void snd_soc_dapm_auto_nc_codec_pins(struct snd_soc_codec *codec); diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 2c2d751..fc5094b 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3211,15 +3211,11 @@ int snd_soc_dapm_put_pin_switch(struct snd_kcontrol *kcontrol, struct snd_soc_card *card = snd_kcontrol_chip(kcontrol); const char *pin = (const char *)kcontrol->private_value;
- mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME); - if (ucontrol->value.integer.value[0]) snd_soc_dapm_enable_pin(&card->dapm, pin); else snd_soc_dapm_disable_pin(&card->dapm, pin);
- mutex_unlock(&card->dapm_mutex); - snd_soc_dapm_sync(&card->dapm); return 0; } @@ -3768,23 +3764,52 @@ void snd_soc_dapm_stream_event(struct snd_soc_pcm_runtime *rtd, int stream, }
/** + * snd_soc_dapm_enable_pin_locked - enable pin. + * @dapm: DAPM context + * @pin: pin name + * + * Enables input/output pin and its parents or children widgets iff there is + * a valid audio route and active audio stream. + * + * Requires external locking. + * + * NOTE: snd_soc_dapm_sync() needs to be called after this for DAPM to + * do any widget power switching. + */ +int snd_soc_dapm_enable_pin_locked(struct snd_soc_dapm_context *dapm, + const char *pin) +{ + return snd_soc_dapm_set_pin(dapm, pin, 1); +} +EXPORT_SYMBOL_GPL(snd_soc_dapm_enable_pin_locked); + +/** * snd_soc_dapm_enable_pin - enable pin. * @dapm: DAPM context * @pin: pin name * * Enables input/output pin and its parents or children widgets iff there is * a valid audio route and active audio stream. + * * NOTE: snd_soc_dapm_sync() needs to be called after this for DAPM to * do any widget power switching. */ int snd_soc_dapm_enable_pin(struct snd_soc_dapm_context *dapm, const char *pin) { - return snd_soc_dapm_set_pin(dapm, pin, 1); + int ret; + + mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME); + + ret = snd_soc_dapm_set_pin(dapm, pin, 1); + + mutex_unlock(&dapm->card->dapm_mutex); + + return ret; } EXPORT_SYMBOL_GPL(snd_soc_dapm_enable_pin);
/** - * snd_soc_dapm_force_enable_pin - force a pin to be enabled + * snd_soc_dapm_force_enable_pin_locked - force a pin to be enabled * @dapm: DAPM context * @pin: pin name * @@ -3792,11 +3817,13 @@ EXPORT_SYMBOL_GPL(snd_soc_dapm_enable_pin); * intended for use with microphone bias supplies used in microphone * jack detection. * + * Requires external locking. + * * NOTE: snd_soc_dapm_sync() needs to be called after this for DAPM to * do any widget power switching. */ -int snd_soc_dapm_force_enable_pin(struct snd_soc_dapm_context *dapm, - const char *pin) +int snd_soc_dapm_force_enable_pin_locked(struct snd_soc_dapm_context *dapm, + const char *pin) { struct snd_soc_dapm_widget *w = dapm_find_widget(dapm, pin, true);
@@ -3812,25 +3839,103 @@ int snd_soc_dapm_force_enable_pin(struct snd_soc_dapm_context *dapm,
return 0; } +EXPORT_SYMBOL_GPL(snd_soc_dapm_force_enable_pin_locked); + +/** + * snd_soc_dapm_force_enable_pin - force a pin to be enabled + * @dapm: DAPM context + * @pin: pin name + * + * Enables input/output pin regardless of any other state. This is + * intended for use with microphone bias supplies used in microphone + * jack detection. + * + * NOTE: snd_soc_dapm_sync() needs to be called after this for DAPM to + * do any widget power switching. + */ +int snd_soc_dapm_force_enable_pin(struct snd_soc_dapm_context *dapm, + const char *pin) +{ + int ret; + + mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME); + + ret = snd_soc_dapm_force_enable_pin_locked(dapm, pin); + + mutex_unlock(&dapm->card->dapm_mutex); + + return ret; +} EXPORT_SYMBOL_GPL(snd_soc_dapm_force_enable_pin);
/** + * snd_soc_dapm_disable_pin_locked - disable pin. + * @dapm: DAPM context + * @pin: pin name + * + * Disables input/output pin and its parents or children widgets. + * + * Requires external locking. + * + * NOTE: snd_soc_dapm_sync() needs to be called after this for DAPM to + * do any widget power switching. + */ +int snd_soc_dapm_disable_pin_locked(struct snd_soc_dapm_context *dapm, + const char *pin) +{ + return snd_soc_dapm_set_pin(dapm, pin, 0); +} +EXPORT_SYMBOL_GPL(snd_soc_dapm_disable_pin_locked); + +/** * snd_soc_dapm_disable_pin - disable pin. * @dapm: DAPM context * @pin: pin name * * Disables input/output pin and its parents or children widgets. + * * NOTE: snd_soc_dapm_sync() needs to be called after this for DAPM to * do any widget power switching. */ int snd_soc_dapm_disable_pin(struct snd_soc_dapm_context *dapm, const char *pin) { - return snd_soc_dapm_set_pin(dapm, pin, 0); + int ret; + + mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME); + + ret = snd_soc_dapm_set_pin(dapm, pin, 0); + + mutex_unlock(&dapm->card->dapm_mutex); + + return ret; } EXPORT_SYMBOL_GPL(snd_soc_dapm_disable_pin);
/** + * snd_soc_dapm_nc_pin_locked - permanently disable pin. + * @dapm: DAPM context + * @pin: pin name + * + * Marks the specified pin as being not connected, disabling it along + * any parent or child widgets. At present this is identical to + * snd_soc_dapm_disable_pin() but in future it will be extended to do + * additional things such as disabling controls which only affect + * paths through the pin. + * + * Requires external locking. + * + * NOTE: snd_soc_dapm_sync() needs to be called after this for DAPM to + * do any widget power switching. + */ +int snd_soc_dapm_nc_pin_locked(struct snd_soc_dapm_context *dapm, + const char *pin) +{ + return snd_soc_dapm_set_pin(dapm, pin, 0); +} +EXPORT_SYMBOL_GPL(snd_soc_dapm_nc_pin_locked); + +/** * snd_soc_dapm_nc_pin - permanently disable pin. * @dapm: DAPM context * @pin: pin name @@ -3846,7 +3951,15 @@ EXPORT_SYMBOL_GPL(snd_soc_dapm_disable_pin); */ int snd_soc_dapm_nc_pin(struct snd_soc_dapm_context *dapm, const char *pin) { - return snd_soc_dapm_set_pin(dapm, pin, 0); + int ret; + + mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME); + + ret = snd_soc_dapm_set_pin(dapm, pin, 0); + + mutex_unlock(&dapm->card->dapm_mutex); + + return ret; } EXPORT_SYMBOL_GPL(snd_soc_dapm_nc_pin);
[...]
+int snd_soc_dapm_enable_pin_locked(struct snd_soc_dapm_context *dapm,
const char *pin)+{
- return snd_soc_dapm_set_pin(dapm, pin, 1);
+} +EXPORT_SYMBOL_GPL(snd_soc_dapm_enable_pin_locked);
+/**
- snd_soc_dapm_enable_pin - enable pin.
- @dapm: DAPM context
- @pin: pin name
- Enables input/output pin and its parents or children widgets iff there is
- a valid audio route and active audio stream.
*/ int snd_soc_dapm_enable_pin(struct snd_soc_dapm_context *dapm, const char *pin) {
- NOTE: snd_soc_dapm_sync() needs to be called after this for DAPM to
- do any widget power switching.
- return snd_soc_dapm_set_pin(dapm, pin, 1);
- int ret;
- mutex_lock_nested(&dapm->card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
- ret = snd_soc_dapm_set_pin(dapm, pin, 1);
- mutex_unlock(&dapm->card->dapm_mutex);
- return ret; }
Hm, this is a bit confusing. For other functions in ASoC when we have a pair of functions with the _locked suffix and without the one with _locked takes the lock, the other one doesn't. E.g. snd_soc_update_bits_locked. Here we do it the other way around.
- Lars
On Mon, Feb 17, 2014 at 06:06:03PM +0100, Lars-Peter Clausen wrote:
Hm, this is a bit confusing. For other functions in ASoC when we have a pair of functions with the _locked suffix and without the one with _locked takes the lock, the other one doesn't. E.g. snd_soc_update_bits_locked. Here we do it the other way around.
Yes, this is definitely confusing. The existing naming might not be the best but making things inconsistent isn't going to help the situation either.
On Tue, Feb 18, 2014 at 09:30:30AM +0900, Mark Brown wrote:
On Mon, Feb 17, 2014 at 06:06:03PM +0100, Lars-Peter Clausen wrote:
Hm, this is a bit confusing. For other functions in ASoC when we have a pair of functions with the _locked suffix and without the one with _locked takes the lock, the other one doesn't. E.g. snd_soc_update_bits_locked. Here we do it the other way around.
Yes, this is definitely confusing. The existing naming might not be the best but making things inconsistent isn't going to help the situation either.
Oops... sorry that was not intentional I will respin to correct this.
Thanks, Charles
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/adav80x.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/adav80x.c b/sound/soc/codecs/adav80x.c index f78b27a..aacb2c03 100644 --- a/sound/soc/codecs/adav80x.c +++ b/sound/soc/codecs/adav80x.c @@ -600,15 +600,21 @@ static int adav80x_set_sysclk(struct snd_soc_codec *codec, adav80x->sysclk_pd[clk_id] = false; }
+ mutex_lock(&codec->dapm.card->dapm_mutex); + if (adav80x->sysclk_pd[0]) - snd_soc_dapm_disable_pin(&codec->dapm, "PLL1"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, "PLL1"); else - snd_soc_dapm_force_enable_pin(&codec->dapm, "PLL1"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, + "PLL1");
if (adav80x->sysclk_pd[1] || adav80x->sysclk_pd[2]) - snd_soc_dapm_disable_pin(&codec->dapm, "PLL2"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, "PLL2"); else - snd_soc_dapm_force_enable_pin(&codec->dapm, "PLL2"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, + "PLL2"); + + mutex_unlock(&codec->dapm.card->dapm_mutex);
snd_soc_dapm_sync(&codec->dapm); }
On Mon, Feb 17, 2014 at 04:51:31PM +0000, Charles Keepax wrote:
mutex_lock(&codec->dapm.card->dapm_mutex);
I think this needs a helper that just takes the CODEC as an argument and does the lookup - a static inline in the header would be fine. The big long dereference is ugly for something people should be doing and will be annoying if we want to refactor in the future.
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/wm5100.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wm5100.c b/sound/soc/codecs/wm5100.c index 4e3e31a..c69992b 100644 --- a/sound/soc/codecs/wm5100.c +++ b/sound/soc/codecs/wm5100.c @@ -2117,8 +2117,13 @@ int wm5100_detect(struct snd_soc_codec *codec, struct snd_soc_jack *jack) WM5100_ACCDET_RATE_MASK);
/* We need the charge pump to power MICBIAS */ - snd_soc_dapm_force_enable_pin(&codec->dapm, "CP2"); - snd_soc_dapm_force_enable_pin(&codec->dapm, "SYSCLK"); + mutex_lock(&codec->dapm.card->dapm_mutex); + + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "CP2"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "SYSCLK"); + + mutex_unlock(&codec->dapm.card->dapm_mutex); + snd_soc_dapm_sync(&codec->dapm);
/* We start off just enabling microphone detection - even a
On Mon, Feb 17, 2014 at 04:51:32PM +0000, Charles Keepax wrote:
snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "CP2");snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "SYSCLK");mutex_unlock(&codec->dapm.card->dapm_mutex);- snd_soc_dapm_sync(&codec->dapm);
With all these patches it seems weird that we have to drop the lock to do the sync which will immediately retake it. It's not broken but it looks off - it would be better to have a version of _sync() that we can call within the lock.
Regarding the naming issue that Lars mentioned I think the current operations are probably fine but calling them _unlocked() meaning they don't do any locking (as distinct from the existing _locked() which take locks) might be OK.
On Tue, Feb 18, 2014 at 09:47:29AM +0900, Mark Brown wrote:
On Mon, Feb 17, 2014 at 04:51:32PM +0000, Charles Keepax wrote:
snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "CP2");snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "SYSCLK");mutex_unlock(&codec->dapm.card->dapm_mutex);- snd_soc_dapm_sync(&codec->dapm);
With all these patches it seems weird that we have to drop the lock to do the sync which will immediately retake it. It's not broken but it looks off - it would be better to have a version of _sync() that we can call within the lock.
No problem to add a version of sync that can be called from within the lock, should help out with Dimtry's comments as well.
Regarding the naming issue that Lars mentioned I think the current operations are probably fine but calling them _unlocked() meaning they don't do any locking (as distinct from the existing _locked() which take locks) might be OK.
Yeah that would be good, since my original aim here was to avoid updating every single usage of these functions.
Thanks, Charles
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/wm8962.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c index c06bb50..535a7dc 100644 --- a/sound/soc/codecs/wm8962.c +++ b/sound/soc/codecs/wm8962.c @@ -3126,14 +3126,18 @@ int wm8962_mic_detect(struct snd_soc_codec *codec, struct snd_soc_jack *jack) snd_soc_jack_report(wm8962->jack, 0, SND_JACK_MICROPHONE | SND_JACK_BTN_0);
+ mutex_lock(&codec->dapm.card->dapm_mutex); + if (jack) { - snd_soc_dapm_force_enable_pin(&codec->dapm, "SYSCLK"); - snd_soc_dapm_force_enable_pin(&codec->dapm, "MICBIAS"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "SYSCLK"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "MICBIAS"); } else { - snd_soc_dapm_disable_pin(&codec->dapm, "SYSCLK"); - snd_soc_dapm_disable_pin(&codec->dapm, "MICBIAS"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, "SYSCLK"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, "MICBIAS"); }
+ mutex_unlock(&codec->dapm.card->dapm_mutex); + return 0; } EXPORT_SYMBOL_GPL(wm8962_mic_detect);
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/wm8994.c | 40 ++++++++++++++++++++++++---------------- 1 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c index b9be9cb..4788c4e 100644 --- a/sound/soc/codecs/wm8994.c +++ b/sound/soc/codecs/wm8994.c @@ -2552,38 +2552,46 @@ int wm8994_vmid_mode(struct snd_soc_codec *codec, enum wm8994_vmid_mode mode)
switch (mode) { case WM8994_VMID_NORMAL: + mutex_lock(&codec->dapm.card->dapm_mutex); + if (wm8994->hubs.lineout1_se) { - snd_soc_dapm_disable_pin(&codec->dapm, - "LINEOUT1N Driver"); - snd_soc_dapm_disable_pin(&codec->dapm, - "LINEOUT1P Driver"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, + "LINEOUT1N Driver"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, + "LINEOUT1P Driver"); } if (wm8994->hubs.lineout2_se) { - snd_soc_dapm_disable_pin(&codec->dapm, - "LINEOUT2N Driver"); - snd_soc_dapm_disable_pin(&codec->dapm, - "LINEOUT2P Driver"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, + "LINEOUT2N Driver"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, + "LINEOUT2P Driver"); }
+ mutex_unlock(&codec->dapm.card->dapm_mutex); + /* Do the sync with the old mode to allow it to clean up */ snd_soc_dapm_sync(&codec->dapm); wm8994->vmid_mode = mode; break;
case WM8994_VMID_FORCE: + mutex_lock(&codec->dapm.card->dapm_mutex); + if (wm8994->hubs.lineout1_se) { - snd_soc_dapm_force_enable_pin(&codec->dapm, - "LINEOUT1N Driver"); - snd_soc_dapm_force_enable_pin(&codec->dapm, - "LINEOUT1P Driver"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, + "LINEOUT1N Driver"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, + "LINEOUT1P Driver"); } if (wm8994->hubs.lineout2_se) { - snd_soc_dapm_force_enable_pin(&codec->dapm, - "LINEOUT2N Driver"); - snd_soc_dapm_force_enable_pin(&codec->dapm, - "LINEOUT2P Driver"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, + "LINEOUT2N Driver"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, + "LINEOUT2P Driver"); }
+ mutex_unlock(&codec->dapm.card->dapm_mutex); + wm8994->vmid_mode = mode; snd_soc_dapm_sync(&codec->dapm); break;
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/codecs/wm8996.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/wm8996.c b/sound/soc/codecs/wm8996.c index 1a7655b..a391128 100644 --- a/sound/soc/codecs/wm8996.c +++ b/sound/soc/codecs/wm8996.c @@ -2267,8 +2267,12 @@ int wm8996_detect(struct snd_soc_codec *codec, struct snd_soc_jack *jack, WM8996_MICB2_DISCH, 0);
/* LDO2 powers the microphones, SYSCLK clocks detection */ - snd_soc_dapm_force_enable_pin(&codec->dapm, "LDO2"); - snd_soc_dapm_force_enable_pin(&codec->dapm, "SYSCLK"); + mutex_lock(&codec->dapm.card->dapm_mutex); + + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "LDO2"); + snd_soc_dapm_force_enable_pin_locked(&codec->dapm, "SYSCLK"); + + mutex_unlock(&codec->dapm.card->dapm_mutex);
/* We start off just enabling microphone detection - even a * plain headphone will trigger detection.
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/intel/mfld_machine.c | 43 +++++++++++++++++++++++++++------------- 1 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/sound/soc/intel/mfld_machine.c b/sound/soc/intel/mfld_machine.c index d3d4c32..4b60542 100644 --- a/sound/soc/intel/mfld_machine.c +++ b/sound/soc/intel/mfld_machine.c @@ -105,15 +105,20 @@ static int headset_set_switch(struct snd_kcontrol *kcontrol, if (ucontrol->value.integer.value[0] == hs_switch) return 0;
+ mutex_lock(&codec->dapm.card->dapm_mutex); + if (ucontrol->value.integer.value[0]) { pr_debug("hs_set HS path\n"); - snd_soc_dapm_enable_pin(&codec->dapm, "Headphones"); - snd_soc_dapm_disable_pin(&codec->dapm, "EPOUT"); + snd_soc_dapm_enable_pin_locked(&codec->dapm, "Headphones"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, "EPOUT"); } else { pr_debug("hs_set EP path\n"); - snd_soc_dapm_disable_pin(&codec->dapm, "Headphones"); - snd_soc_dapm_enable_pin(&codec->dapm, "EPOUT"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, "Headphones"); + snd_soc_dapm_enable_pin_locked(&codec->dapm, "EPOUT"); } + + mutex_unlock(&codec->dapm.card->dapm_mutex); + snd_soc_dapm_sync(&codec->dapm); hs_switch = ucontrol->value.integer.value[0];
@@ -122,19 +127,23 @@ static int headset_set_switch(struct snd_kcontrol *kcontrol,
static void lo_enable_out_pins(struct snd_soc_codec *codec) { - snd_soc_dapm_enable_pin(&codec->dapm, "IHFOUTL"); - snd_soc_dapm_enable_pin(&codec->dapm, "IHFOUTR"); - snd_soc_dapm_enable_pin(&codec->dapm, "LINEOUTL"); - snd_soc_dapm_enable_pin(&codec->dapm, "LINEOUTR"); - snd_soc_dapm_enable_pin(&codec->dapm, "VIB1OUT"); - snd_soc_dapm_enable_pin(&codec->dapm, "VIB2OUT"); + mutex_lock(&codec->dapm.card->dapm_mutex); + + snd_soc_dapm_enable_pin_locked(&codec->dapm, "IHFOUTL"); + snd_soc_dapm_enable_pin_locked(&codec->dapm, "IHFOUTR"); + snd_soc_dapm_enable_pin_locked(&codec->dapm, "LINEOUTL"); + snd_soc_dapm_enable_pin_locked(&codec->dapm, "LINEOUTR"); + snd_soc_dapm_enable_pin_locked(&codec->dapm, "VIB1OUT"); + snd_soc_dapm_enable_pin_locked(&codec->dapm, "VIB2OUT"); if (hs_switch) { - snd_soc_dapm_enable_pin(&codec->dapm, "Headphones"); - snd_soc_dapm_disable_pin(&codec->dapm, "EPOUT"); + snd_soc_dapm_enable_pin_locked(&codec->dapm, "Headphones"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, "EPOUT"); } else { - snd_soc_dapm_disable_pin(&codec->dapm, "Headphones"); - snd_soc_dapm_enable_pin(&codec->dapm, "EPOUT"); + snd_soc_dapm_disable_pin_locked(&codec->dapm, "Headphones"); + snd_soc_dapm_enable_pin_locked(&codec->dapm, "EPOUT"); } + + mutex_unlock(&codec->dapm.card->dapm_mutex); }
static int lo_get_switch(struct snd_kcontrol *kcontrol, @@ -156,6 +165,9 @@ static int lo_set_switch(struct snd_kcontrol *kcontrol, * pins and then disable pins not required */ lo_enable_out_pins(codec); + + mutex_lock(&codec->dapm.card->dapm_mutex); + switch (ucontrol->value.integer.value[0]) { case 0: pr_debug("set vibra path\n"); @@ -185,6 +197,9 @@ static int lo_set_switch(struct snd_kcontrol *kcontrol, snd_soc_update_bits(codec, SN95031_LOCTL, 0x66, 0x66); break; } + + mutex_unlock(&codec->dapm.card->dapm_mutex); + snd_soc_dapm_sync(&codec->dapm); lo_dac = ucontrol->value.integer.value[0]; return 0;
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/omap/ams-delta.c | 38 +++++++++++++++++++++++--------------- 1 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c index 6294464..73deddf 100644 --- a/sound/soc/omap/ams-delta.c +++ b/sound/soc/omap/ams-delta.c @@ -113,46 +113,52 @@ static int ams_delta_set_audio_mode(struct snd_kcontrol *kcontrol,
/* Setup pins after corresponding bits if changed */ pin = !!(pins & (1 << AMS_DELTA_MOUTHPIECE)); + + mutex_lock(&dapm->card->dapm_mutex); + if (pin != snd_soc_dapm_get_pin_status(dapm, "Mouthpiece")) { changed = 1; if (pin) - snd_soc_dapm_enable_pin(dapm, "Mouthpiece"); + snd_soc_dapm_enable_pin_locked(dapm, "Mouthpiece"); else - snd_soc_dapm_disable_pin(dapm, "Mouthpiece"); + snd_soc_dapm_disable_pin_locked(dapm, "Mouthpiece"); } pin = !!(pins & (1 << AMS_DELTA_EARPIECE)); if (pin != snd_soc_dapm_get_pin_status(dapm, "Earpiece")) { changed = 1; if (pin) - snd_soc_dapm_enable_pin(dapm, "Earpiece"); + snd_soc_dapm_enable_pin_locked(dapm, "Earpiece"); else - snd_soc_dapm_disable_pin(dapm, "Earpiece"); + snd_soc_dapm_disable_pin_locked(dapm, "Earpiece"); } pin = !!(pins & (1 << AMS_DELTA_MICROPHONE)); if (pin != snd_soc_dapm_get_pin_status(dapm, "Microphone")) { changed = 1; if (pin) - snd_soc_dapm_enable_pin(dapm, "Microphone"); + snd_soc_dapm_enable_pin_locked(dapm, "Microphone"); else - snd_soc_dapm_disable_pin(dapm, "Microphone"); + snd_soc_dapm_disable_pin_locked(dapm, "Microphone"); } pin = !!(pins & (1 << AMS_DELTA_SPEAKER)); if (pin != snd_soc_dapm_get_pin_status(dapm, "Speaker")) { changed = 1; if (pin) - snd_soc_dapm_enable_pin(dapm, "Speaker"); + snd_soc_dapm_enable_pin_locked(dapm, "Speaker"); else - snd_soc_dapm_disable_pin(dapm, "Speaker"); + snd_soc_dapm_disable_pin_locked(dapm, "Speaker"); } pin = !!(pins & (1 << AMS_DELTA_AGC)); if (pin != ams_delta_audio_agc) { ams_delta_audio_agc = pin; changed = 1; if (pin) - snd_soc_dapm_enable_pin(dapm, "AGCIN"); + snd_soc_dapm_enable_pin_locked(dapm, "AGCIN"); else - snd_soc_dapm_disable_pin(dapm, "AGCIN"); + snd_soc_dapm_disable_pin_locked(dapm, "AGCIN"); } + + mutex_unlock(&dapm->card->dapm_mutex); + if (changed) snd_soc_dapm_sync(dapm);
@@ -315,11 +321,13 @@ static void cx81801_close(struct tty_struct *tty) v253_ops.close(tty);
/* Revert back to default audio input/output constellation */ - snd_soc_dapm_disable_pin(dapm, "Mouthpiece"); - snd_soc_dapm_enable_pin(dapm, "Earpiece"); - snd_soc_dapm_enable_pin(dapm, "Microphone"); - snd_soc_dapm_disable_pin(dapm, "Speaker"); - snd_soc_dapm_disable_pin(dapm, "AGCIN"); + mutex_lock(&dapm->card->dapm_mutex); + snd_soc_dapm_disable_pin_locked(dapm, "Mouthpiece"); + snd_soc_dapm_enable_pin_locked(dapm, "Earpiece"); + snd_soc_dapm_enable_pin_locked(dapm, "Microphone"); + snd_soc_dapm_disable_pin_locked(dapm, "Speaker"); + snd_soc_dapm_disable_pin_locked(dapm, "AGCIN"); + mutex_unlock(&dapm->card->dapm_mutex); snd_soc_dapm_sync(dapm); }
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/omap/n810.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/sound/soc/omap/n810.c b/sound/soc/omap/n810.c index 3fde9e4..b3f59a5 100644 --- a/sound/soc/omap/n810.c +++ b/sound/soc/omap/n810.c @@ -68,24 +68,28 @@ static void n810_ext_control(struct snd_soc_dapm_context *dapm) break; }
+ mutex_lock(&dapm->card->dapm_mutex); + if (n810_spk_func) - snd_soc_dapm_enable_pin(dapm, "Ext Spk"); + snd_soc_dapm_enable_pin_locked(dapm, "Ext Spk"); else - snd_soc_dapm_disable_pin(dapm, "Ext Spk"); + snd_soc_dapm_disable_pin_locked(dapm, "Ext Spk");
if (hp) - snd_soc_dapm_enable_pin(dapm, "Headphone Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Headphone Jack"); else - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); if (line1l) - snd_soc_dapm_enable_pin(dapm, "LINE1L"); + snd_soc_dapm_enable_pin_locked(dapm, "LINE1L"); else - snd_soc_dapm_disable_pin(dapm, "LINE1L"); + snd_soc_dapm_disable_pin_locked(dapm, "LINE1L");
if (n810_dmic_func) - snd_soc_dapm_enable_pin(dapm, "DMic"); + snd_soc_dapm_enable_pin_locked(dapm, "DMic"); else - snd_soc_dapm_disable_pin(dapm, "DMic"); + snd_soc_dapm_disable_pin_locked(dapm, "DMic"); + + mutex_unlock(&dapm->card->dapm_mutex);
snd_soc_dapm_sync(dapm); }
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/omap/rx51.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c index 611179c..51f1ae6 100644 --- a/sound/soc/omap/rx51.c +++ b/sound/soc/omap/rx51.c @@ -74,22 +74,26 @@ static void rx51_ext_control(struct snd_soc_dapm_context *dapm) break; }
+ mutex_lock(&dapm->card->dapm_mutex); + if (rx51_spk_func) - snd_soc_dapm_enable_pin(dapm, "Ext Spk"); + snd_soc_dapm_enable_pin_locked(dapm, "Ext Spk"); else - snd_soc_dapm_disable_pin(dapm, "Ext Spk"); + snd_soc_dapm_disable_pin_locked(dapm, "Ext Spk"); if (rx51_dmic_func) - snd_soc_dapm_enable_pin(dapm, "DMic"); + snd_soc_dapm_enable_pin_locked(dapm, "DMic"); else - snd_soc_dapm_disable_pin(dapm, "DMic"); + snd_soc_dapm_disable_pin_locked(dapm, "DMic"); if (hp) - snd_soc_dapm_enable_pin(dapm, "Headphone Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Headphone Jack"); else - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); if (hs) - snd_soc_dapm_enable_pin(dapm, "HS Mic"); + snd_soc_dapm_enable_pin_locked(dapm, "HS Mic"); else - snd_soc_dapm_disable_pin(dapm, "HS Mic"); + snd_soc_dapm_disable_pin_locked(dapm, "HS Mic"); + + mutex_unlock(&dapm->card->dapm_mutex);
gpio_set_value(RX51_TVOUT_SEL_GPIO, tvout);
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/pxa/corgi.c | 40 ++++++++++++++++++++++------------------ 1 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/sound/soc/pxa/corgi.c b/sound/soc/pxa/corgi.c index 1853d41..d29cd0d 100644 --- a/sound/soc/pxa/corgi.c +++ b/sound/soc/pxa/corgi.c @@ -47,48 +47,52 @@ static int corgi_spk_func;
static void corgi_ext_control(struct snd_soc_dapm_context *dapm) { + mutex_lock(&dapm->card->dapm_mutex); + /* set up jack connection */ switch (corgi_jack_func) { case CORGI_HP: /* set = unmute headphone */ gpio_set_value(CORGI_GPIO_MUTE_L, 1); gpio_set_value(CORGI_GPIO_MUTE_R, 1); - snd_soc_dapm_disable_pin(dapm, "Mic Jack"); - snd_soc_dapm_disable_pin(dapm, "Line Jack"); - snd_soc_dapm_enable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_disable_pin(dapm, "Headset Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Mic Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Line Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Jack"); break; case CORGI_MIC: /* reset = mute headphone */ gpio_set_value(CORGI_GPIO_MUTE_L, 0); gpio_set_value(CORGI_GPIO_MUTE_R, 0); - snd_soc_dapm_enable_pin(dapm, "Mic Jack"); - snd_soc_dapm_disable_pin(dapm, "Line Jack"); - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_disable_pin(dapm, "Headset Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Mic Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Line Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Jack"); break; case CORGI_LINE: gpio_set_value(CORGI_GPIO_MUTE_L, 0); gpio_set_value(CORGI_GPIO_MUTE_R, 0); - snd_soc_dapm_disable_pin(dapm, "Mic Jack"); - snd_soc_dapm_enable_pin(dapm, "Line Jack"); - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_disable_pin(dapm, "Headset Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Mic Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Line Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Jack"); break; case CORGI_HEADSET: gpio_set_value(CORGI_GPIO_MUTE_L, 0); gpio_set_value(CORGI_GPIO_MUTE_R, 1); - snd_soc_dapm_enable_pin(dapm, "Mic Jack"); - snd_soc_dapm_disable_pin(dapm, "Line Jack"); - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_enable_pin(dapm, "Headset Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Mic Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Line Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Headset Jack"); break; }
if (corgi_spk_func == CORGI_SPK_ON) - snd_soc_dapm_enable_pin(dapm, "Ext Spk"); + snd_soc_dapm_enable_pin_locked(dapm, "Ext Spk"); else - snd_soc_dapm_disable_pin(dapm, "Ext Spk"); + snd_soc_dapm_disable_pin_locked(dapm, "Ext Spk"); + + mutex_unlock(&dapm->card->dapm_mutex);
/* signal a DAPM event */ snd_soc_dapm_sync(dapm);
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/pxa/magician.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/sound/soc/pxa/magician.c b/sound/soc/pxa/magician.c index aace19e..b5f0e00 100644 --- a/sound/soc/pxa/magician.c +++ b/sound/soc/pxa/magician.c @@ -45,26 +45,30 @@ static void magician_ext_control(struct snd_soc_codec *codec) { struct snd_soc_dapm_context *dapm = &codec->dapm;
+ mutex_lock(&dapm->card->dapm_mutex); + if (magician_spk_switch) - snd_soc_dapm_enable_pin(dapm, "Speaker"); + snd_soc_dapm_enable_pin_locked(dapm, "Speaker"); else - snd_soc_dapm_disable_pin(dapm, "Speaker"); + snd_soc_dapm_disable_pin_locked(dapm, "Speaker"); if (magician_hp_switch) - snd_soc_dapm_enable_pin(dapm, "Headphone Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Headphone Jack"); else - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack");
switch (magician_in_sel) { case MAGICIAN_MIC: - snd_soc_dapm_disable_pin(dapm, "Headset Mic"); - snd_soc_dapm_enable_pin(dapm, "Call Mic"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Mic"); + snd_soc_dapm_enable_pin_locked(dapm, "Call Mic"); break; case MAGICIAN_MIC_EXT: - snd_soc_dapm_disable_pin(dapm, "Call Mic"); - snd_soc_dapm_enable_pin(dapm, "Headset Mic"); + snd_soc_dapm_disable_pin_locked(dapm, "Call Mic"); + snd_soc_dapm_enable_pin_locked(dapm, "Headset Mic"); break; }
+ mutex_unlock(&dapm->card->dapm_mutex); + snd_soc_dapm_sync(dapm); }
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/pxa/spitz.c | 49 +++++++++++++++++++++++++++---------------------- 1 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/sound/soc/pxa/spitz.c b/sound/soc/pxa/spitz.c index fc052d8..e755919 100644 --- a/sound/soc/pxa/spitz.c +++ b/sound/soc/pxa/spitz.c @@ -46,60 +46,65 @@ static int spitz_mic_gpio;
static void spitz_ext_control(struct snd_soc_dapm_context *dapm) { + mutex_lock(&dapm->card->dapm_mutex); + if (spitz_spk_func == SPITZ_SPK_ON) - snd_soc_dapm_enable_pin(dapm, "Ext Spk"); + snd_soc_dapm_enable_pin_locked(dapm, "Ext Spk"); else - snd_soc_dapm_disable_pin(dapm, "Ext Spk"); + snd_soc_dapm_disable_pin_locked(dapm, "Ext Spk");
/* set up jack connection */ switch (spitz_jack_func) { case SPITZ_HP: /* enable and unmute hp jack, disable mic bias */ - snd_soc_dapm_disable_pin(dapm, "Headset Jack"); - snd_soc_dapm_disable_pin(dapm, "Mic Jack"); - snd_soc_dapm_disable_pin(dapm, "Line Jack"); - snd_soc_dapm_enable_pin(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Mic Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Line Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Headphone Jack"); gpio_set_value(SPITZ_GPIO_MUTE_L, 1); gpio_set_value(SPITZ_GPIO_MUTE_R, 1); break; case SPITZ_MIC: /* enable mic jack and bias, mute hp */ - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_disable_pin(dapm, "Headset Jack"); - snd_soc_dapm_disable_pin(dapm, "Line Jack"); - snd_soc_dapm_enable_pin(dapm, "Mic Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Line Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Mic Jack"); gpio_set_value(SPITZ_GPIO_MUTE_L, 0); gpio_set_value(SPITZ_GPIO_MUTE_R, 0); break; case SPITZ_LINE: /* enable line jack, disable mic bias and mute hp */ - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_disable_pin(dapm, "Headset Jack"); - snd_soc_dapm_disable_pin(dapm, "Mic Jack"); - snd_soc_dapm_enable_pin(dapm, "Line Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Mic Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Line Jack"); gpio_set_value(SPITZ_GPIO_MUTE_L, 0); gpio_set_value(SPITZ_GPIO_MUTE_R, 0); break; case SPITZ_HEADSET: /* enable and unmute headset jack enable mic bias, mute L hp */ - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_enable_pin(dapm, "Mic Jack"); - snd_soc_dapm_disable_pin(dapm, "Line Jack"); - snd_soc_dapm_enable_pin(dapm, "Headset Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Mic Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Line Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Headset Jack"); gpio_set_value(SPITZ_GPIO_MUTE_L, 0); gpio_set_value(SPITZ_GPIO_MUTE_R, 1); break; case SPITZ_HP_OFF:
/* jack removed, everything off */ - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_disable_pin(dapm, "Headset Jack"); - snd_soc_dapm_disable_pin(dapm, "Mic Jack"); - snd_soc_dapm_disable_pin(dapm, "Line Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Mic Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Line Jack"); gpio_set_value(SPITZ_GPIO_MUTE_L, 0); gpio_set_value(SPITZ_GPIO_MUTE_R, 0); break; } + + mutex_unlock(&dapm->card->dapm_mutex); + snd_soc_dapm_sync(dapm); }
The pin updates in this driver look like they are intended to be done atomically, update to do so.
Signed-off-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com --- sound/soc/pxa/tosa.c | 26 +++++++++++++++----------- 1 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/sound/soc/pxa/tosa.c b/sound/soc/pxa/tosa.c index 1d9c2ed..64f9620 100644 --- a/sound/soc/pxa/tosa.c +++ b/sound/soc/pxa/tosa.c @@ -48,29 +48,33 @@ static void tosa_ext_control(struct snd_soc_codec *codec) { struct snd_soc_dapm_context *dapm = &codec->dapm;
+ mutex_lock(&dapm->card->dapm_mutex); + /* set up jack connection */ switch (tosa_jack_func) { case TOSA_HP: - snd_soc_dapm_disable_pin(dapm, "Mic (Internal)"); - snd_soc_dapm_enable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_disable_pin(dapm, "Headset Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Mic (Internal)"); + snd_soc_dapm_enable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Jack"); break; case TOSA_MIC_INT: - snd_soc_dapm_enable_pin(dapm, "Mic (Internal)"); - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_disable_pin(dapm, "Headset Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Mic (Internal)"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Headset Jack"); break; case TOSA_HEADSET: - snd_soc_dapm_disable_pin(dapm, "Mic (Internal)"); - snd_soc_dapm_disable_pin(dapm, "Headphone Jack"); - snd_soc_dapm_enable_pin(dapm, "Headset Jack"); + snd_soc_dapm_disable_pin_locked(dapm, "Mic (Internal)"); + snd_soc_dapm_disable_pin_locked(dapm, "Headphone Jack"); + snd_soc_dapm_enable_pin_locked(dapm, "Headset Jack"); break; }
if (tosa_spk_func == TOSA_SPK_ON) - snd_soc_dapm_enable_pin(dapm, "Speaker"); + snd_soc_dapm_enable_pin_locked(dapm, "Speaker"); else - snd_soc_dapm_disable_pin(dapm, "Speaker"); + snd_soc_dapm_disable_pin_locked(dapm, "Speaker"); + + mutex_unlock(&dapm->card->dapm_mutex);
snd_soc_dapm_sync(dapm); }
participants (4)
-
Charles Keepax -
Dmitry Torokhov -
Lars-Peter Clausen -
Mark Brown