When being powered on, either initially on probe or when resuming from suspend, the wm8971 configures the device for quick output capacitor charging. Since the charging can take a rather long time (up to multiple seconds) it is done asynchronously without blocking. A delayed work item is run once the charging is finished and the device is switched to the target bias level.

This all done asynchronously to the regular DAPM sequence accessing the same data structures and registers without any looking, which can lead to race conditions. Furthermore this potentially delays the start of stream on the CODEC while the rest of the system is already up and running, meaning the first bytes of audio are lost. It also does no comply with the assumption of the DAPM core that if set_bias_level() returned successfully the device will be at the requested bias level.

This patch slightly refactors things and makes sure that the caps charging is properly integrated into the DAPM sequence. When transitioning from SND_SOC_BIAS_OFF to SND_SOC_BIAS_STANDBY the part will be put into fast charging mode and a work item will be scheduled that puts it back into standby charging once the charging period has elapsed. If a playback or capture stream is started while charging is in progress the driver will now wait in SND_SOC_BIAS_PREPARE until the charging is done. This makes sure that charging is done asynchronously in the background when the chip is idle, but at the same time makes sure that playback/capture is not started before the charging is done.

Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/wm8971.c | 71 +++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 40 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c index 44baacd..4ab034d 100644 --- a/sound/soc/codecs/wm8971.c +++ b/sound/soc/codecs/wm8971.c @@ -34,6 +34,8 @@ /* codec private data */ struct wm8971_priv { unsigned int sysclk; + struct delayed_work charge_work; + struct regmap *regmap; };

/* @@ -550,9 +552,19 @@ static int wm8971_mute(struct snd_soc_dai *dai, int mute) return 0; }

+static void wm8971_charge_work(struct work_struct *work) +{ + struct wm8971_priv *wm8971 = + container_of(work, struct wm8971_priv, charge_work.work); + + /* Set to 500k */ + regmap_update_bits(wm8971->regmap, WM8971_PWR1, 0x0180, 0x0100); +} + static int wm8971_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) { + struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec); u16 pwr_reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e;

switch (level) { @@ -561,15 +573,24 @@ static int wm8971_set_bias_level(struct snd_soc_codec *codec, snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x00c1); break; case SND_SOC_BIAS_PREPARE: + /* Wait until fully charged */ + flush_delayed_work(&wm8971->charge_work); break; case SND_SOC_BIAS_STANDBY: - if (codec->dapm.bias_level == SND_SOC_BIAS_OFF) + if (codec->dapm.bias_level == SND_SOC_BIAS_OFF) { snd_soc_cache_sync(codec); + /* charge output caps - set vmid to 5k for quick power up */ + snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x01c0); + queue_delayed_work(system_power_efficient_wq, + &wm8971->charge_work, msecs_to_jiffies(1000)); + } else { + /* mute dac and set vmid to 500k, enable VREF */ + snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x0140); + }

- /* mute dac and set vmid to 500k, enable VREF */ - snd_soc_write(codec, WM8971_PWR1, pwr_reg | 0x0140); break; case SND_SOC_BIAS_OFF: + cancel_delayed_work_sync(&wm8971->charge_work); snd_soc_write(codec, WM8971_PWR1, 0x0001); break; } @@ -608,15 +629,6 @@ static struct snd_soc_dai_driver wm8971_dai = { .ops = &wm8971_dai_ops, };

-static void wm8971_work(struct work_struct *work) -{ - struct snd_soc_dapm_context *dapm = - container_of(work, struct snd_soc_dapm_context, - delayed_work.work); - struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm); - wm8971_set_bias_level(codec, codec->dapm.bias_level); -} - static int wm8971_suspend(struct snd_soc_codec *codec) { wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF); @@ -625,39 +637,19 @@ static int wm8971_suspend(struct snd_soc_codec *codec)

static int wm8971_resume(struct snd_soc_codec *codec) { - u16 reg; - wm8971_set_bias_level(codec, SND_SOC_BIAS_STANDBY); - - /* charge wm8971 caps */ - if (codec->dapm.suspend_bias_level == SND_SOC_BIAS_ON) { - reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e; - snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0); - codec->dapm.bias_level = SND_SOC_BIAS_ON; - queue_delayed_work(system_power_efficient_wq, - &codec->dapm.delayed_work, - msecs_to_jiffies(1000)); - } - return 0; }

static int wm8971_probe(struct snd_soc_codec *codec) { - int ret = 0; - u16 reg; + struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);

- INIT_DELAYED_WORK(&codec->dapm.delayed_work, wm8971_work); + INIT_DELAYED_WORK(&wm8971->charge_work, wm8971_charge_work);

wm8971_reset(codec);

- /* charge output caps - set vmid to 5k for quick power up */ - reg = snd_soc_read(codec, WM8971_PWR1) & 0xfe3e; - snd_soc_write(codec, WM8971_PWR1, reg | 0x01c0); - codec->dapm.bias_level = SND_SOC_BIAS_STANDBY; - queue_delayed_work(system_power_efficient_wq, - &codec->dapm.delayed_work, - msecs_to_jiffies(1000)); + wm8971_set_bias_level(codec, SND_SOC_BIAS_STANDBY);

/* set the update bits */ snd_soc_update_bits(codec, WM8971_LDAC, 0x0100, 0x0100); @@ -669,7 +661,7 @@ static int wm8971_probe(struct snd_soc_codec *codec) snd_soc_update_bits(codec, WM8971_LINVOL, 0x0100, 0x0100); snd_soc_update_bits(codec, WM8971_RINVOL, 0x0100, 0x0100);

- return ret; + return 0; }

@@ -710,7 +702,6 @@ static int wm8971_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { struct wm8971_priv *wm8971; - struct regmap *regmap; int ret;

wm8971 = devm_kzalloc(&i2c->dev, sizeof(struct wm8971_priv), @@ -718,9 +709,9 @@ static int wm8971_i2c_probe(struct i2c_client *i2c, if (wm8971 == NULL) return -ENOMEM;

- regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap); - if (IS_ERR(regmap)) - return PTR_ERR(regmap); + wm8971->regmap = devm_regmap_init_i2c(i2c, &wm8971_regmap); + if (IS_ERR(wm8971->regmap)) + return PTR_ERR(wm8971->regmap);

i2c_set_clientdata(i2c, wm8971);

When being powered on, either initially on probe or when resuming from suspend, the wm8971 configures the device for quick output capacitor charging. Since the charging can take a rather long time (up to multiple seconds) it is done asynchronously without blocking. A delayed work item is run once the charging is finished and the device is switched to the target bias level.

This all done asynchronously to the regular DAPM sequence accessing the same data structures and registers without any looking, which can lead to race conditions. Furthermore this potentially delays the start of stream on the CODEC while the rest of the system is already up and running, meaning the first bytes of audio are lost. It also does no comply with the assumption of the DAPM core that if set_bias_level() returned successfully the device will be at the requested bias level.

This patch slightly refactors things and makes sure that the caps charging is properly integrated into the DAPM sequence. When transitioning from SND_SOC_BIAS_OFF to SND_SOC_BIAS_STANDBY the part will be put into fast charging mode and a work item will be scheduled that puts it back into standby charging once the charging period has elapsed. If a playback or capture stream is started while charging is in progress the driver will now wait in SND_SOC_BIAS_PREPARE until the charging is done. This makes sure that charging is done asynchronously in the background when the chip is idle, but at the same time makes sure that playback/capture is not started before the charging is done.

Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- sound/soc/codecs/wm8753.c | 54 +++++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/sound/soc/codecs/wm8753.c b/sound/soc/codecs/wm8753.c index 21ca3a9..176fcb1 100644 --- a/sound/soc/codecs/wm8753.c +++ b/sound/soc/codecs/wm8753.c @@ -153,6 +153,7 @@ struct wm8753_priv { unsigned int hifi_fmt;

int dai_func; + struct delayed_work charge_work; };

#define wm8753_reset(c) snd_soc_write(c, WM8753_RESET, 0) @@ -1326,9 +1327,19 @@ static int wm8753_mute(struct snd_soc_dai *dai, int mute) return 0; }

+static void wm8753_charge_work(struct work_struct *work) +{ + struct wm8753_priv *wm8753 = + container_of(work, struct wm8753_priv, charge_work.work); + + /* Set to 500k */ + regmap_update_bits(wm8753->regmap, WM8753_PWR1, 0x0180, 0x0100); +} + static int wm8753_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) { + struct wm8753_priv *wm8753 = snd_soc_codec_get_drvdata(codec); u16 pwr_reg = snd_soc_read(codec, WM8753_PWR1) & 0xfe3e;

switch (level) { @@ -1337,14 +1348,22 @@ static int wm8753_set_bias_level(struct snd_soc_codec *codec, snd_soc_write(codec, WM8753_PWR1, pwr_reg | 0x00c0); break; case SND_SOC_BIAS_PREPARE: - /* set vmid to 5k for quick power up */ - snd_soc_write(codec, WM8753_PWR1, pwr_reg | 0x01c1); + /* Wait until fully charged */ + flush_delayed_work(&wm8753->charge_work); break; case SND_SOC_BIAS_STANDBY: - /* mute dac and set vmid to 500k, enable VREF */ - snd_soc_write(codec, WM8753_PWR1, pwr_reg | 0x0141); + if (codec->dapm.bias_level == SND_SOC_BIAS_OFF) { + /* set vmid to 5k for quick power up */ + snd_soc_write(codec, WM8753_PWR1, pwr_reg | 0x01c1); + schedule_delayed_work(&wm8753->charge_work, + msecs_to_jiffies(caps_charge)); + } else { + /* mute dac and set vmid to 500k, enable VREF */ + snd_soc_write(codec, WM8753_PWR1, pwr_reg | 0x0141); + } break; case SND_SOC_BIAS_OFF: + cancel_delayed_work_sync(&wm8753->charge_work); snd_soc_write(codec, WM8753_PWR1, 0x0001); break; } @@ -1428,15 +1447,6 @@ static struct snd_soc_dai_driver wm8753_dai[] = { }, };

-static void wm8753_work(struct work_struct *work) -{ - struct snd_soc_dapm_context *dapm = - container_of(work, struct snd_soc_dapm_context, - delayed_work.work); - struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm); - wm8753_set_bias_level(codec, dapm->bias_level); -} - static int wm8753_suspend(struct snd_soc_codec *codec) { wm8753_set_bias_level(codec, SND_SOC_BIAS_OFF); @@ -1450,16 +1460,6 @@ static int wm8753_resume(struct snd_soc_codec *codec) regcache_sync(wm8753->regmap);

wm8753_set_bias_level(codec, SND_SOC_BIAS_STANDBY); - - /* charge wm8753 caps */ - if (codec->dapm.suspend_bias_level == SND_SOC_BIAS_ON) { - wm8753_set_bias_level(codec, SND_SOC_BIAS_PREPARE); - codec->dapm.bias_level = SND_SOC_BIAS_ON; - queue_delayed_work(system_power_efficient_wq, - &codec->dapm.delayed_work, - msecs_to_jiffies(caps_charge)); - } - return 0; }

@@ -1468,7 +1468,7 @@ static int wm8753_probe(struct snd_soc_codec *codec) struct wm8753_priv *wm8753 = snd_soc_codec_get_drvdata(codec); int ret;

- INIT_DELAYED_WORK(&codec->dapm.delayed_work, wm8753_work); + INIT_DELAYED_WORK(&wm8753->charge_work, wm8753_charge_work);

ret = wm8753_reset(codec); if (ret < 0) { @@ -1479,11 +1479,6 @@ static int wm8753_probe(struct snd_soc_codec *codec) wm8753_set_bias_level(codec, SND_SOC_BIAS_STANDBY); wm8753->dai_func = 0;

- /* charge output caps */ - wm8753_set_bias_level(codec, SND_SOC_BIAS_PREPARE); - schedule_delayed_work(&codec->dapm.delayed_work, - msecs_to_jiffies(caps_charge)); - /* set the update bits */ snd_soc_update_bits(codec, WM8753_LDAC, 0x0100, 0x0100); snd_soc_update_bits(codec, WM8753_RDAC, 0x0100, 0x0100); @@ -1502,7 +1497,6 @@ static int wm8753_probe(struct snd_soc_codec *codec) /* power down chip */ static int wm8753_remove(struct snd_soc_codec *codec) { - flush_delayed_work(&codec->dapm.delayed_work); wm8753_set_bias_level(codec, SND_SOC_BIAS_OFF);

return 0;

The only two users of the suspend_bias_level field were two rather old drivers which weren't exactly doing things by the book. Those drivers have been updated and field is now unused and can be removed.

Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- include/sound/soc-dapm.h | 1 - sound/soc/soc-core.c | 10 ++-------- 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index 0be5816..2e5f0ce 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -588,7 +588,6 @@ struct snd_soc_dapm_update { /* DAPM context */ struct snd_soc_dapm_context { enum snd_soc_bias_level bias_level; - 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 */ diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 76bfff2..2801578 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -595,15 +595,9 @@ int snd_soc_suspend(struct device *dev) cpu_dai->driver->suspend(cpu_dai); }

- /* close any waiting streams and save state */ - for (i = 0; i < card->num_rtd; i++) { - struct snd_soc_dai **codec_dais = card->rtd[i].codec_dais; + /* close any waiting streams */ + for (i = 0; i < card->num_rtd; i++) flush_delayed_work(&card->rtd[i].delayed_work); - for (j = 0; j < card->rtd[i].num_codecs; j++) { - codec_dais[j]->codec->dapm.suspend_bias_level = - codec_dais[j]->codec->dapm.bias_level; - } - }

for (i = 0; i < card->num_rtd; i++) {

The delayed_work field in the snd_soc_dapm_context struct is now unused and can be removed. Removing it reduces the size of the snd_soc_dapm_context struct by ~50% from 100 bytes to 48 bytes.

Signed-off-by: Lars-Peter Clausen lars@metafoo.de --- include/sound/soc-dapm.h | 1 - 1 file changed, 1 deletion(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index 2e5f0ce..0bc8364 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -588,7 +588,6 @@ struct snd_soc_dapm_update { /* DAPM context */ struct snd_soc_dapm_context { enum snd_soc_bias_level 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;