[alsa-devel] [PATCH 0/9] ASoC: probe/remove and suspend/resume manual bias level transitions cleanup
The goal of this series is to allow to cut back the number of manual bias level transitions that drivers have to do.
We do have a lot of drivers which manually go to SND_SOC_BIAS_OFF on suspend and go back to SND_SOC_BIAS_STANDBY on resume. Often this is even the only thing done in the suspend and resume handlers. This leads to a lot of drivers with identical suspend and resume code. In this series a new flag called suspend_bias_off is introduced. When this flag is set for a driver the ASoC core will automatically switch to SND_SOC_BIAS_OFF during suspend. This allows to remove the manual transitions (and often the suspend and resume handlers altogether).
Similarly a lot of drivers also go to SND_SOC_BIAS_STANDBY at the end of their probe() callback and a lot of drivers go to SND_SOC_BIAS_OFF at the beginning in their remove() handler. The first one is already unnecessary as things are today since the core will make sure to set the DAPM context to SND_SOC_BIAS_STANDBY when the first snd_soc_dapm_sync() is executed during card instantiation after all components have been probed. To be able to remove the later from individual drivers this series introduces a call to snd_soc_dapm_shutdown() when the card is removed. This not only makes sure that the bias level of all components is at SND_SOC_BIAS_OFF, but also makes sure that all widgets are properly powered down.
This series only converts a small set of drivers for now to keep small and clear and to be able to focus on the core changes, but I do have patches for cleaning up most of the drivers which in total removes over a 1000 lines of redundant code.
The series depends on topic/suspend.
- Lars
Lars-Peter Clausen (9): ASoC: Set card->instantiated to false when removing the card ASoC: Shutdown DAPM contexts when removing a card ASoC: Add support for automatically going to BIAS_OFF on suspend ASoC: Always run default suspend/resume code ASoC: adau1373: Cleanup manual bias level transitions ASoC: adau17x1: Cleanup manual bias level transitions ASoC: adav80x: Cleanup manual bias level transitions ASoC: ssm2518: Cleanup manual bias level transitions ASoC: ssm2602: Cleanup manual bias level transitions
include/sound/soc-dapm.h | 3 ++- include/sound/soc.h | 1 + sound/soc/codecs/adau1373.c | 7 ------- sound/soc/codecs/adau1761.c | 2 +- sound/soc/codecs/adau1781.c | 2 +- sound/soc/codecs/adau17x1.c | 8 -------- sound/soc/codecs/adau17x1.h | 1 - sound/soc/codecs/adav80x.c | 23 ++--------------------- sound/soc/codecs/ssm2518.c | 13 ------------- sound/soc/codecs/ssm2602.c | 24 ++---------------------- sound/soc/soc-core.c | 17 ++++++++++++----- sound/soc/soc-dapm.c | 20 ++++++++++++++++++-- 12 files changed, 39 insertions(+), 82 deletions(-)
Set card->instantiated to false when the card is removed to make sure that operations that expect the card to be fully instantiated do not run anymore during card removal.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/soc-core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index b1327ef..031a19a 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3810,8 +3810,10 @@ EXPORT_SYMBOL_GPL(snd_soc_register_card); */ int snd_soc_unregister_card(struct snd_soc_card *card) { - if (card->instantiated) + if (card->instantiated) { + card->instantiated = false; soc_cleanup_card_resources(card); + } dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
return 0;
Currently when a ASoC sound card is unregistered we leave the individual components in their current state, just call the remove() callback and leave it to the drivers to do the proper shutdown/cleanup.
This patch introduces a call to snd_soc_dapm_shutdown() when removing the card. This will make sure that all DAPM widgets are properly powered down and all DAPM contexts are put at the SND_SOC_BIAS_OFF level. This will ensure that all components are properly powered down when the card is removed.
Since a lot of drivers manually go to SND_SOC_BIAS_OFF in their remove callback this will also allow us to remove a bit of duplicated code.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/soc-core.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 031a19a..42f3690 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3812,6 +3812,7 @@ int snd_soc_unregister_card(struct snd_soc_card *card) { if (card->instantiated) { card->instantiated = false; + snd_soc_dapm_shutdown(card); soc_cleanup_card_resources(card); } dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
There is a substantial amount of drivers that in go to SND_SOC_BIAS_OFF on suspend and go back to SND_SOC_BIAS_SUSPEND on resume (Often this is even the only thing done in the suspend and resume handlers). This patch introduces a new suspend_bias_off flag, which when set by a driver will let the ASoC core automatically put the device's DAPM context at the SND_SOC_BIAS_OFF level during suspend. Once the device is resumed the DAPM context will go back to SND_SOC_BIAS_STANDBY (if the context is idle, otherwise to SND_SOC_BIAS_ON).
This will allow us to remove a fair bit of duplicated code from the drivers.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- include/sound/soc-dapm.h | 3 ++- include/sound/soc.h | 1 + sound/soc/soc-core.c | 1 + sound/soc/soc-dapm.c | 20 ++++++++++++++++++-- 4 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index aac04ff..f955d65 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -587,7 +587,8 @@ struct snd_soc_dapm_context { enum snd_soc_bias_level suspend_bias_level; struct delayed_work delayed_work; unsigned int idle_bias_off:1; /* Use BIAS_OFF instead of STANDBY */ - + /* Go to BIAS_OFF in suspend if the DAPM context is idle */ + unsigned int suspend_bias_off:1; void (*seq_notifier)(struct snd_soc_dapm_context *, enum snd_soc_dapm_type, int);
diff --git a/include/sound/soc.h b/include/sound/soc.h index 29650de..882300d 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -848,6 +848,7 @@ struct snd_soc_codec_driver { int (*set_bias_level)(struct snd_soc_codec *, enum snd_soc_bias_level level); bool idle_bias_off; + bool suspend_bias_off;
void (*seq_notifier)(struct snd_soc_dapm_context *, enum snd_soc_dapm_type, int); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 42f3690..e7c8b11 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -4402,6 +4402,7 @@ int snd_soc_register_codec(struct device *dev, codec->component.ignore_pmdown_time = codec_drv->ignore_pmdown_time; codec->dapm.codec = codec; codec->dapm.idle_bias_off = codec_drv->idle_bias_off; + codec->dapm.suspend_bias_off = codec_drv->suspend_bias_off; if (codec_drv->seq_notifier) codec->dapm.seq_notifier = codec_drv->seq_notifier; if (codec_drv->set_bias_level) diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 177bd86..7efe4fa 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -1683,6 +1683,22 @@ static void dapm_power_one_widget(struct snd_soc_dapm_widget *w, } }
+static bool dapm_idle_bias_off(struct snd_soc_dapm_context *dapm) +{ + if (dapm->idle_bias_off) + return true; + + switch (snd_power_get_state(dapm->card->snd_card)) { + case SNDRV_CTL_POWER_D3hot: + case SNDRV_CTL_POWER_D3cold: + return dapm->suspend_bias_off; + default: + break; + } + + return false; +} + /* * Scan each dapm widget for complete audio path. * A complete path is a route that has valid endpoints i.e.:- @@ -1706,7 +1722,7 @@ static int dapm_power_widgets(struct snd_soc_card *card, int event) trace_snd_soc_dapm_start(card);
list_for_each_entry(d, &card->dapm_list, list) { - if (d->idle_bias_off) + if (dapm_idle_bias_off(d)) d->target_bias_level = SND_SOC_BIAS_OFF; else d->target_bias_level = SND_SOC_BIAS_STANDBY; @@ -1772,7 +1788,7 @@ static int dapm_power_widgets(struct snd_soc_card *card, int event) if (d->target_bias_level > bias) bias = d->target_bias_level; list_for_each_entry(d, &card->dapm_list, list) - if (!d->idle_bias_off) + if (!dapm_idle_bias_off(d)) d->target_bias_level = bias;
trace_snd_soc_dapm_walk_done(card);
We do a bit more than just running the callbacks during suspend and resume these days (e.g. call regcache_mark_dirty() during suspend). But this is only when suspend and resume callbacks are specified for the driver, otherwise nothing is done. This means that drivers which don't want to do anything special during suspend and resume, but still want the standard operations to run, need to provide empty suspend and resume callback functions (rather than no callbacks). This patch updates the suspend and resume code to always run standard sequence regardless of whether suspend and resume handlers are provided.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/soc-core.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index e7c8b11..cf89325 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -637,7 +637,7 @@ int snd_soc_suspend(struct device *dev) list_for_each_entry(codec, &card->codec_dev_list, card_list) { /* If there are paths active then the CODEC will be held with * bias _ON and should not be suspended. */ - if (!codec->suspended && codec->driver->suspend) { + if (!codec->suspended) { switch (codec->dapm.bias_level) { case SND_SOC_BIAS_STANDBY: /* @@ -651,8 +651,10 @@ int snd_soc_suspend(struct device *dev) "ASoC: idle_bias_off CODEC on over suspend\n"); break; } + case SND_SOC_BIAS_OFF: - codec->driver->suspend(codec); + if (codec->driver->suspend) + codec->driver->suspend(codec); codec->suspended = 1; codec->cache_sync = 1; if (codec->component.regmap) @@ -726,11 +728,12 @@ static void soc_resume_deferred(struct work_struct *work) * left with bias OFF or STANDBY and suspended so we must now * resume. Otherwise the suspend was suppressed. */ - if (codec->driver->resume && codec->suspended) { + if (codec->suspended) { switch (codec->dapm.bias_level) { case SND_SOC_BIAS_STANDBY: case SND_SOC_BIAS_OFF: - codec->driver->resume(codec); + if (codec->driver->resume) + codec->driver->resume(codec); codec->suspended = 0; break; default:
The ASoC core now takes care of setting the bias level to SND_SOC_BIAS_OFF when removing the CODEC, no need to do it manually anymore.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/adau1373.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/sound/soc/codecs/adau1373.c b/sound/soc/codecs/adau1373.c index 1947565..7c784ad 100644 --- a/sound/soc/codecs/adau1373.c +++ b/sound/soc/codecs/adau1373.c @@ -1448,12 +1448,6 @@ static int adau1373_set_bias_level(struct snd_soc_codec *codec, return 0; }
-static int adau1373_remove(struct snd_soc_codec *codec) -{ - adau1373_set_bias_level(codec, SND_SOC_BIAS_OFF); - return 0; -} - static int adau1373_resume(struct snd_soc_codec *codec) { struct adau1373 *adau1373 = snd_soc_codec_get_drvdata(codec); @@ -1488,7 +1482,6 @@ static const struct regmap_config adau1373_regmap_config = {
static struct snd_soc_codec_driver adau1373_codec_driver = { .probe = adau1373_probe, - .remove = adau1373_remove, .resume = adau1373_resume, .set_bias_level = adau1373_set_bias_level, .idle_bias_off = true,
Set the CODEC driver's suspend_bias_off flag rather than manually going to SND_SOC_BIAS_OFF in suspend and SND_SOC_BIAS_STANDBY in resume. This makes the code a bit shorter and cleaner.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/adau1761.c | 2 +- sound/soc/codecs/adau1781.c | 2 +- sound/soc/codecs/adau17x1.c | 8 -------- sound/soc/codecs/adau17x1.h | 1 - 4 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/sound/soc/codecs/adau1761.c b/sound/soc/codecs/adau1761.c index 848cab8..5518ebd 100644 --- a/sound/soc/codecs/adau1761.c +++ b/sound/soc/codecs/adau1761.c @@ -714,9 +714,9 @@ static int adau1761_codec_probe(struct snd_soc_codec *codec)
static const struct snd_soc_codec_driver adau1761_codec_driver = { .probe = adau1761_codec_probe, - .suspend = adau17x1_suspend, .resume = adau17x1_resume, .set_bias_level = adau1761_set_bias_level, + .suspend_bias_off = true,
.controls = adau1761_controls, .num_controls = ARRAY_SIZE(adau1761_controls), diff --git a/sound/soc/codecs/adau1781.c b/sound/soc/codecs/adau1781.c index 045a614..e9fc00f 100644 --- a/sound/soc/codecs/adau1781.c +++ b/sound/soc/codecs/adau1781.c @@ -446,9 +446,9 @@ static int adau1781_codec_probe(struct snd_soc_codec *codec)
static const struct snd_soc_codec_driver adau1781_codec_driver = { .probe = adau1781_codec_probe, - .suspend = adau17x1_suspend, .resume = adau17x1_resume, .set_bias_level = adau1781_set_bias_level, + .suspend_bias_off = true,
.controls = adau1781_controls, .num_controls = ARRAY_SIZE(adau1781_controls), diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c index 0b65970..3e16c1c 100644 --- a/sound/soc/codecs/adau17x1.c +++ b/sound/soc/codecs/adau17x1.c @@ -815,13 +815,6 @@ int adau17x1_add_routes(struct snd_soc_codec *codec) } EXPORT_SYMBOL_GPL(adau17x1_add_routes);
-int adau17x1_suspend(struct snd_soc_codec *codec) -{ - codec->driver->set_bias_level(codec, SND_SOC_BIAS_OFF); - return 0; -} -EXPORT_SYMBOL_GPL(adau17x1_suspend); - int adau17x1_resume(struct snd_soc_codec *codec) { struct adau *adau = snd_soc_codec_get_drvdata(codec); @@ -829,7 +822,6 @@ int adau17x1_resume(struct snd_soc_codec *codec) if (adau->switch_mode) adau->switch_mode(codec->dev);
- codec->driver->set_bias_level(codec, SND_SOC_BIAS_STANDBY); regcache_sync(adau->regmap);
return 0; diff --git a/sound/soc/codecs/adau17x1.h b/sound/soc/codecs/adau17x1.h index 3ffabaf..e4a557f 100644 --- a/sound/soc/codecs/adau17x1.h +++ b/sound/soc/codecs/adau17x1.h @@ -52,7 +52,6 @@ int adau17x1_set_micbias_voltage(struct snd_soc_codec *codec, enum adau17x1_micbias_voltage micbias); bool adau17x1_readable_register(struct device *dev, unsigned int reg); bool adau17x1_volatile_register(struct device *dev, unsigned int reg); -int adau17x1_suspend(struct snd_soc_codec *codec); int adau17x1_resume(struct snd_soc_codec *codec);
extern const struct snd_soc_dai_ops adau17x1_dai_ops;
Set the CODEC driver's suspend_bias_off flag rather than manually going to SND_SOC_BIAS_OFF in suspend and SND_SOC_BIAS_STANDBY in resume. This makes the code a bit shorter and cleaner. While we are at it also remove the regcache_cache_only() calls from suspend/resume as there shouldn't be any IO between suspend and resume.
Since the ASoC core now takes care of setting the bias level to SND_SOC_BIAS_OFF when removing the CODEC there is no need to do it manually anymore either.
The manual transition to SND_SOC_BIAS_STANDBY at the end of CODEC probe() can also be removed as the core will automatically do this after the CODEC has been probed.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/adav80x.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/sound/soc/codecs/adav80x.c b/sound/soc/codecs/adav80x.c index c43b93f..ce3cdca 100644 --- a/sound/soc/codecs/adav80x.c +++ b/sound/soc/codecs/adav80x.c @@ -812,42 +812,23 @@ static int adav80x_probe(struct snd_soc_codec *codec) /* Disable DAC zero flag */ regmap_write(adav80x->regmap, ADAV80X_DAC_CTRL3, 0x6);
- return adav80x_set_bias_level(codec, SND_SOC_BIAS_STANDBY); -} - -static int adav80x_suspend(struct snd_soc_codec *codec) -{ - struct adav80x *adav80x = snd_soc_codec_get_drvdata(codec); - int ret; - - ret = adav80x_set_bias_level(codec, SND_SOC_BIAS_OFF); - regcache_cache_only(adav80x->regmap, true); - - return ret; + return 0; }
static int adav80x_resume(struct snd_soc_codec *codec) { struct adav80x *adav80x = snd_soc_codec_get_drvdata(codec);
- regcache_cache_only(adav80x->regmap, false); - adav80x_set_bias_level(codec, SND_SOC_BIAS_STANDBY); regcache_sync(adav80x->regmap);
return 0; }
-static int adav80x_remove(struct snd_soc_codec *codec) -{ - return adav80x_set_bias_level(codec, SND_SOC_BIAS_OFF); -} - static struct snd_soc_codec_driver adav80x_codec_driver = { .probe = adav80x_probe, - .remove = adav80x_remove, - .suspend = adav80x_suspend, .resume = adav80x_resume, .set_bias_level = adav80x_set_bias_level, + .suspend_bias_off = true,
.set_pll = adav80x_set_pll, .set_sysclk = adav80x_set_sysclk,
Since the ASoC core now takes care of setting the bias level to SND_SOC_BIAS_OFF when removing the CODEC there is no need to do it manually anymore either.
The manual transition to SND_SOC_BIAS_OFF at the end of CODEC probe() can also be removed as the CODEC is already in OFF state at this point.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/ssm2518.c | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/sound/soc/codecs/ssm2518.c b/sound/soc/codecs/ssm2518.c index e8680be..67ea55a 100644 --- a/sound/soc/codecs/ssm2518.c +++ b/sound/soc/codecs/ssm2518.c @@ -646,17 +646,6 @@ static struct snd_soc_dai_driver ssm2518_dai = { .ops = &ssm2518_dai_ops, };
-static int ssm2518_probe(struct snd_soc_codec *codec) -{ - return ssm2518_set_bias_level(codec, SND_SOC_BIAS_OFF); -} - -static int ssm2518_remove(struct snd_soc_codec *codec) -{ - ssm2518_set_bias_level(codec, SND_SOC_BIAS_OFF); - return 0; -} - static int ssm2518_set_sysclk(struct snd_soc_codec *codec, int clk_id, int source, unsigned int freq, int dir) { @@ -727,8 +716,6 @@ static int ssm2518_set_sysclk(struct snd_soc_codec *codec, int clk_id, }
static struct snd_soc_codec_driver ssm2518_codec_driver = { - .probe = ssm2518_probe, - .remove = ssm2518_remove, .set_bias_level = ssm2518_set_bias_level, .set_sysclk = ssm2518_set_sysclk, .idle_bias_off = true,
Set the CODEC driver's suspend_bias_off flag rather than manually going to SND_SOC_BIAS_OFF in suspend and SND_SOC_BIAS_STANDBY in resume. This makes the code a bit shorter and cleaner. While we are at it also remove the regcache_cache_only() calls from suspend/resume as there shouldn't be any IO between suspend and resume.
Since the ASoC core now takes care of setting the bias level to SND_SOC_BIAS_OFF when removing the CODEC there is no need to do it manually anymore either.
The manual transition to SND_SOC_BIAS_STANDBY at the end of CODEC probe() can also be removed as the core will automatically do this after the CODEC has been probed.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/ssm2602.c | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/sound/soc/codecs/ssm2602.c b/sound/soc/codecs/ssm2602.c index 484b3bb..0dec1364 100644 --- a/sound/soc/codecs/ssm2602.c +++ b/sound/soc/codecs/ssm2602.c @@ -502,18 +502,11 @@ static struct snd_soc_dai_driver ssm2602_dai = { .symmetric_samplebits = 1, };
-static int ssm2602_suspend(struct snd_soc_codec *codec) -{ - ssm2602_set_bias_level(codec, SND_SOC_BIAS_OFF); - return 0; -} - static int ssm2602_resume(struct snd_soc_codec *codec) { struct ssm2602_priv *ssm2602 = snd_soc_codec_get_drvdata(codec);
regcache_sync(ssm2602->regmap); - ssm2602_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
return 0; } @@ -586,27 +579,14 @@ static int ssm260x_codec_probe(struct snd_soc_codec *codec) break; }
- if (ret) - return ret; - - ssm2602_set_bias_level(codec, SND_SOC_BIAS_STANDBY); - - return 0; -} - -/* remove everything here */ -static int ssm2602_remove(struct snd_soc_codec *codec) -{ - ssm2602_set_bias_level(codec, SND_SOC_BIAS_OFF); - return 0; + return ret; }
static struct snd_soc_codec_driver soc_codec_dev_ssm2602 = { .probe = ssm260x_codec_probe, - .remove = ssm2602_remove, - .suspend = ssm2602_suspend, .resume = ssm2602_resume, .set_bias_level = ssm2602_set_bias_level, + .suspend_bias_off = true,
.controls = ssm260x_snd_controls, .num_controls = ARRAY_SIZE(ssm260x_snd_controls),
On Thu, Sep 04, 2014 at 07:44:03PM +0200, Lars-Peter Clausen wrote:
The goal of this series is to allow to cut back the number of manual bias level transitions that drivers have to do.
Applied all, thanks. Please word wrap *below* 80 columns.
participants (2)
-
Lars-Peter Clausen -
Mark Brown