All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue
@ 2015-03-30 19:04 Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 2/8] ASoC: wm8971: Integrate capacitor charging into the DAPM sequence Lars-Peter Clausen
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-03-30 19:04 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Charles Keepax, patches, alsa-devel, Lars-Peter Clausen

The delayed work used by the wm8971 driver to manage the caps charging
doesn't have any special requirements that would justify using a custom
workqueue, just use the generic system_power_efficient_wq instead.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/codecs/wm8971.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 39ddb9b..44baacd 100644
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -31,8 +31,6 @@
 
 #define	WM8971_REG_COUNT		43
 
-static struct workqueue_struct *wm8971_workq = NULL;
-
 /* codec private data */
 struct wm8971_priv {
 	unsigned int sysclk;
@@ -636,7 +634,8 @@ static int wm8971_resume(struct snd_soc_codec *codec)
 		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(wm8971_workq, &codec->dapm.delayed_work,
+		queue_delayed_work(system_power_efficient_wq,
+			&codec->dapm.delayed_work,
 			msecs_to_jiffies(1000));
 	}
 
@@ -649,9 +648,6 @@ static int wm8971_probe(struct snd_soc_codec *codec)
 	u16 reg;
 
 	INIT_DELAYED_WORK(&codec->dapm.delayed_work, wm8971_work);
-	wm8971_workq = create_workqueue("wm8971");
-	if (wm8971_workq == NULL)
-		return -ENOMEM;
 
 	wm8971_reset(codec);
 
@@ -659,7 +655,8 @@ static int wm8971_probe(struct snd_soc_codec *codec)
 	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(wm8971_workq, &codec->dapm.delayed_work,
+	queue_delayed_work(system_power_efficient_wq,
+		&codec->dapm.delayed_work,
 		msecs_to_jiffies(1000));
 
 	/* set the update bits */
@@ -681,8 +678,6 @@ static int wm8971_remove(struct snd_soc_codec *codec)
 {
 	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
 
-	if (wm8971_workq)
-		destroy_workqueue(wm8971_workq);
 	return 0;
 }
 
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/8] ASoC: wm8971: Integrate capacitor charging into the DAPM sequence
  2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
@ 2015-03-30 19:04 ` Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 3/8] ASoC: wm8971: Cleanup manual bias level transitions Lars-Peter Clausen
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-03-30 19:04 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Charles Keepax, patches, alsa-devel, Lars-Peter Clausen

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);
 
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/8] ASoC: wm8971: Cleanup manual bias level transitions
  2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 2/8] ASoC: wm8971: Integrate capacitor charging into the DAPM sequence Lars-Peter Clausen
@ 2015-03-30 19:04 ` Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 4/8] ASoC: wm8753: Integrate capacitor charging into the DAPM sequence Lars-Peter Clausen
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-03-30 19:04 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Charles Keepax, patches, alsa-devel, Lars-Peter Clausen

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.

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/wm8971.c | 27 +--------------------------
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/sound/soc/codecs/wm8971.c b/sound/soc/codecs/wm8971.c
index 4ab034d..f9cbabd 100644
--- a/sound/soc/codecs/wm8971.c
+++ b/sound/soc/codecs/wm8971.c
@@ -629,18 +629,6 @@ static struct snd_soc_dai_driver wm8971_dai = {
 	.ops = &wm8971_dai_ops,
 };
 
-static int wm8971_suspend(struct snd_soc_codec *codec)
-{
-	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
-	return 0;
-}
-
-static int wm8971_resume(struct snd_soc_codec *codec)
-{
-	wm8971_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
-	return 0;
-}
-
 static int wm8971_probe(struct snd_soc_codec *codec)
 {
 	struct wm8971_priv *wm8971 = snd_soc_codec_get_drvdata(codec);
@@ -649,8 +637,6 @@ static int wm8971_probe(struct snd_soc_codec *codec)
 
 	wm8971_reset(codec);
 
-	wm8971_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
-
 	/* set the update bits */
 	snd_soc_update_bits(codec, WM8971_LDAC, 0x0100, 0x0100);
 	snd_soc_update_bits(codec, WM8971_RDAC, 0x0100, 0x0100);
@@ -664,21 +650,10 @@ static int wm8971_probe(struct snd_soc_codec *codec)
 	return 0;
 }
 
-
-/* power down chip */
-static int wm8971_remove(struct snd_soc_codec *codec)
-{
-	wm8971_set_bias_level(codec, SND_SOC_BIAS_OFF);
-
-	return 0;
-}
-
 static struct snd_soc_codec_driver soc_codec_dev_wm8971 = {
 	.probe =	wm8971_probe,
-	.remove =	wm8971_remove,
-	.suspend =	wm8971_suspend,
-	.resume =	wm8971_resume,
 	.set_bias_level = wm8971_set_bias_level,
+	.suspend_bias_off = true,
 
 	.controls = wm8971_snd_controls,
 	.num_controls = ARRAY_SIZE(wm8971_snd_controls),
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/8] ASoC: wm8753: Integrate capacitor charging into the DAPM sequence
  2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 2/8] ASoC: wm8971: Integrate capacitor charging into the DAPM sequence Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 3/8] ASoC: wm8971: Cleanup manual bias level transitions Lars-Peter Clausen
@ 2015-03-30 19:04 ` Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 5/8] ASoC: wm8753: Cleanup manual bias level transitions Lars-Peter Clausen
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-03-30 19:04 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Charles Keepax, patches, alsa-devel, Lars-Peter Clausen

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;
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/8] ASoC: wm8753: Cleanup manual bias level transitions
  2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
                   ` (2 preceding siblings ...)
  2015-03-30 19:04 ` [PATCH 4/8] ASoC: wm8753: Integrate capacitor charging into the DAPM sequence Lars-Peter Clausen
@ 2015-03-30 19:04 ` Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 6/8] ASoC: Remove suspend_bias_level from DAPM context struct Lars-Peter Clausen
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-03-30 19:04 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Charles Keepax, patches, alsa-devel, Lars-Peter Clausen

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.

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/wm8753.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/sound/soc/codecs/wm8753.c b/sound/soc/codecs/wm8753.c
index 176fcb1..c50a595 100644
--- a/sound/soc/codecs/wm8753.c
+++ b/sound/soc/codecs/wm8753.c
@@ -1447,19 +1447,12 @@ static struct snd_soc_dai_driver wm8753_dai[] = {
 },
 };
 
-static int wm8753_suspend(struct snd_soc_codec *codec)
-{
-	wm8753_set_bias_level(codec, SND_SOC_BIAS_OFF);
-	return 0;
-}
-
 static int wm8753_resume(struct snd_soc_codec *codec)
 {
 	struct wm8753_priv *wm8753 = snd_soc_codec_get_drvdata(codec);
 
 	regcache_sync(wm8753->regmap);
 
-	wm8753_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 	return 0;
 }
 
@@ -1476,7 +1469,6 @@ static int wm8753_probe(struct snd_soc_codec *codec)
 		return ret;
 	}
 
-	wm8753_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 	wm8753->dai_func = 0;
 
 	/* set the update bits */
@@ -1494,20 +1486,11 @@ static int wm8753_probe(struct snd_soc_codec *codec)
 	return 0;
 }
 
-/* power down chip */
-static int wm8753_remove(struct snd_soc_codec *codec)
-{
-	wm8753_set_bias_level(codec, SND_SOC_BIAS_OFF);
-
-	return 0;
-}
-
 static struct snd_soc_codec_driver soc_codec_dev_wm8753 = {
 	.probe =	wm8753_probe,
-	.remove =	wm8753_remove,
-	.suspend =	wm8753_suspend,
 	.resume =	wm8753_resume,
 	.set_bias_level = wm8753_set_bias_level,
+	.suspend_bias_off = true,
 
 	.controls = wm8753_snd_controls,
 	.num_controls = ARRAY_SIZE(wm8753_snd_controls),
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/8] ASoC: Remove suspend_bias_level from DAPM context struct
  2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
                   ` (3 preceding siblings ...)
  2015-03-30 19:04 ` [PATCH 5/8] ASoC: wm8753: Cleanup manual bias level transitions Lars-Peter Clausen
@ 2015-03-30 19:04 ` Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 7/8] ASoC: wm8350: Move delayed work struct from DAPM context to driver state Lars-Peter Clausen
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-03-30 19:04 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Charles Keepax, patches, alsa-devel, Lars-Peter Clausen

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++) {
 
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/8] ASoC: wm8350: Move delayed work struct from DAPM context to driver state
  2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
                   ` (4 preceding siblings ...)
  2015-03-30 19:04 ` [PATCH 6/8] ASoC: Remove suspend_bias_level from DAPM context struct Lars-Peter Clausen
@ 2015-03-30 19:04 ` Lars-Peter Clausen
  2015-03-30 19:04 ` [PATCH 8/8] ASoC: dapm: Remove delayed_work from dapm context struct Lars-Peter Clausen
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-03-30 19:04 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Charles Keepax, patches, alsa-devel, Lars-Peter Clausen

The wm8350 driver is the last driver that still uses the delayed_work field
from the snd_soc_dapm_context struct. Moving this over to the driver's
private data struct will allow us to remove the field from the DAPM context,
which will drastically reduce its size.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/codecs/wm8350.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/sound/soc/codecs/wm8350.c b/sound/soc/codecs/wm8350.c
index c81a9ea..c65e5a7 100644
--- a/sound/soc/codecs/wm8350.c
+++ b/sound/soc/codecs/wm8350.c
@@ -69,14 +69,14 @@ struct wm8350_data {
 	struct regulator_bulk_data supplies[ARRAY_SIZE(supply_names)];
 	int fll_freq_out;
 	int fll_freq_in;
+	struct delayed_work pga_work;
 };
 
 /*
  * Ramp OUT1 PGA volume to minimise pops at stream startup and shutdown.
  */
-static inline int wm8350_out1_ramp_step(struct snd_soc_codec *codec)
+static inline int wm8350_out1_ramp_step(struct wm8350_data *wm8350_data)
 {
-	struct wm8350_data *wm8350_data = snd_soc_codec_get_drvdata(codec);
 	struct wm8350_output *out1 = &wm8350_data->out1;
 	struct wm8350 *wm8350 = wm8350_data->wm8350;
 	int left_complete = 0, right_complete = 0;
@@ -140,9 +140,8 @@ static inline int wm8350_out1_ramp_step(struct snd_soc_codec *codec)
 /*
  * Ramp OUT2 PGA volume to minimise pops at stream startup and shutdown.
  */
-static inline int wm8350_out2_ramp_step(struct snd_soc_codec *codec)
+static inline int wm8350_out2_ramp_step(struct wm8350_data *wm8350_data)
 {
-	struct wm8350_data *wm8350_data = snd_soc_codec_get_drvdata(codec);
 	struct wm8350_output *out2 = &wm8350_data->out2;
 	struct wm8350 *wm8350 = wm8350_data->wm8350;
 	int left_complete = 0, right_complete = 0;
@@ -210,10 +209,8 @@ static inline int wm8350_out2_ramp_step(struct snd_soc_codec *codec)
  */
 static void wm8350_pga_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);
-	struct wm8350_data *wm8350_data = snd_soc_codec_get_drvdata(codec);
+	struct wm8350_data *wm8350_data =
+		container_of(work, struct wm8350_data, pga_work.work);
 	struct wm8350_output *out1 = &wm8350_data->out1,
 	    *out2 = &wm8350_data->out2;
 	int i, out1_complete, out2_complete;
@@ -226,9 +223,9 @@ static void wm8350_pga_work(struct work_struct *work)
 	for (i = 0; i <= 63; i++) {
 		out1_complete = 1, out2_complete = 1;
 		if (out1->ramp != WM8350_RAMP_NONE)
-			out1_complete = wm8350_out1_ramp_step(codec);
+			out1_complete = wm8350_out1_ramp_step(wm8350_data);
 		if (out2->ramp != WM8350_RAMP_NONE)
-			out2_complete = wm8350_out2_ramp_step(codec);
+			out2_complete = wm8350_out2_ramp_step(wm8350_data);
 
 		/* ramp finished ? */
 		if (out1_complete && out2_complete)
@@ -283,7 +280,7 @@ static int pga_event(struct snd_soc_dapm_widget *w,
 		out->ramp = WM8350_RAMP_UP;
 		out->active = 1;
 
-		schedule_delayed_work(&codec->dapm.delayed_work,
+		schedule_delayed_work(&wm8350_data->pga_work,
 				      msecs_to_jiffies(1));
 		break;
 
@@ -291,7 +288,7 @@ static int pga_event(struct snd_soc_dapm_widget *w,
 		out->ramp = WM8350_RAMP_DOWN;
 		out->active = 0;
 
-		schedule_delayed_work(&codec->dapm.delayed_work,
+		schedule_delayed_work(&wm8350_data->pga_work,
 				      msecs_to_jiffies(1));
 		break;
 	}
@@ -1492,7 +1489,7 @@ static  int wm8350_codec_probe(struct snd_soc_codec *codec)
 	/* Put the codec into reset if it wasn't already */
 	wm8350_clear_bits(wm8350, WM8350_POWER_MGMT_5, WM8350_CODEC_ENA);
 
-	INIT_DELAYED_WORK(&codec->dapm.delayed_work, wm8350_pga_work);
+	INIT_DELAYED_WORK(&priv->pga_work, wm8350_pga_work);
 	INIT_DELAYED_WORK(&priv->hpl.work, wm8350_hpl_work);
 	INIT_DELAYED_WORK(&priv->hpr.work, wm8350_hpr_work);
 
@@ -1578,7 +1575,7 @@ static int  wm8350_codec_remove(struct snd_soc_codec *codec)
 
 	/* if there was any work waiting then we run it now and
 	 * wait for its completion */
-	flush_delayed_work(&codec->dapm.delayed_work);
+	flush_delayed_work(&priv->pga_work);
 
 	wm8350_clear_bits(wm8350, WM8350_POWER_MGMT_5, WM8350_CODEC_ENA);
 
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/8] ASoC: dapm: Remove delayed_work from dapm context struct
  2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
                   ` (5 preceding siblings ...)
  2015-03-30 19:04 ` [PATCH 7/8] ASoC: wm8350: Move delayed work struct from DAPM context to driver state Lars-Peter Clausen
@ 2015-03-30 19:04 ` Lars-Peter Clausen
  2015-04-01 11:52 ` [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Charles Keepax
  2015-04-01 20:28 ` Mark Brown
  8 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2015-03-30 19:04 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Charles Keepax, patches, alsa-devel, Lars-Peter Clausen

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;
-- 
1.8.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue
  2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
                   ` (6 preceding siblings ...)
  2015-03-30 19:04 ` [PATCH 8/8] ASoC: dapm: Remove delayed_work from dapm context struct Lars-Peter Clausen
@ 2015-04-01 11:52 ` Charles Keepax
  2015-04-01 20:28 ` Mark Brown
  8 siblings, 0 replies; 10+ messages in thread
From: Charles Keepax @ 2015-04-01 11:52 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: alsa-devel, Mark Brown, Liam Girdwood, patches

On Mon, Mar 30, 2015 at 09:04:45PM +0200, Lars-Peter Clausen wrote:
> The delayed work used by the wm8971 driver to manage the caps charging
> doesn't have any special requirements that would justify using a custom
> workqueue, just use the generic system_power_efficient_wq instead.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---

Whole series looks good to me:

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue
  2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
                   ` (7 preceding siblings ...)
  2015-04-01 11:52 ` [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Charles Keepax
@ 2015-04-01 20:28 ` Mark Brown
  8 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2015-04-01 20:28 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Charles Keepax, patches, Liam Girdwood, alsa-devel


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

On Mon, Mar 30, 2015 at 09:04:45PM +0200, Lars-Peter Clausen wrote:
> The delayed work used by the wm8971 driver to manage the caps charging
> doesn't have any special requirements that would justify using a custom
> workqueue, just use the generic system_power_efficient_wq instead.

Applied all, thanks.

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

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-04-01 20:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 19:04 [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Lars-Peter Clausen
2015-03-30 19:04 ` [PATCH 2/8] ASoC: wm8971: Integrate capacitor charging into the DAPM sequence Lars-Peter Clausen
2015-03-30 19:04 ` [PATCH 3/8] ASoC: wm8971: Cleanup manual bias level transitions Lars-Peter Clausen
2015-03-30 19:04 ` [PATCH 4/8] ASoC: wm8753: Integrate capacitor charging into the DAPM sequence Lars-Peter Clausen
2015-03-30 19:04 ` [PATCH 5/8] ASoC: wm8753: Cleanup manual bias level transitions Lars-Peter Clausen
2015-03-30 19:04 ` [PATCH 6/8] ASoC: Remove suspend_bias_level from DAPM context struct Lars-Peter Clausen
2015-03-30 19:04 ` [PATCH 7/8] ASoC: wm8350: Move delayed work struct from DAPM context to driver state Lars-Peter Clausen
2015-03-30 19:04 ` [PATCH 8/8] ASoC: dapm: Remove delayed_work from dapm context struct Lars-Peter Clausen
2015-04-01 11:52 ` [PATCH 1/8] ASoC: wm8971: Use system_power_efficient_wq instead of custom workqueue Charles Keepax
2015-04-01 20:28 ` Mark Brown

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.