[PATCH 1/3 v2] ASoC: ak5386: add regulator consumer support

[PATCH 1/3 v2] ASoC: ak5386: add regulator consumer support

[Daniel Mack]

The chip has two power supplies, VA and VDD. Enable them both as long
as the codec is in use.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 sound/soc/codecs/ak5386.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/sound/soc/codecs/ak5386.c b/sound/soc/codecs/ak5386.c
index 72e953b..a30be5c 100644
--- a/sound/soc/codecs/ak5386.c
+++ b/sound/soc/codecs/ak5386.c
@@ -14,12 +14,18 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/of_device.h>
+#include <linux/regulator/consumer.h>
 #include <sound/soc.h>
 #include <sound/pcm.h>
 #include <sound/initval.h>
 
+static const char const *supply_names[] = {
+	"va", "vd"
+};
+
 struct ak5386_priv {
 	int reset_gpio;
+	struct regulator_bulk_data supplies[ARRAY_SIZE(supply_names)];
 };
 
 static const struct snd_soc_dapm_widget ak5386_dapm_widgets[] = {
@@ -32,7 +38,42 @@ static const struct snd_soc_dapm_route ak5386_dapm_routes[] = {
 	{ "Capture", NULL, "AINR" },
 };
 
+static int ak5386_soc_probe(struct snd_soc_codec *codec)
+{
+	struct ak5386_priv *priv = snd_soc_codec_get_drvdata(codec);
+	return regulator_bulk_enable(ARRAY_SIZE(priv->supplies), priv->supplies);
+}
+
+static int ak5386_soc_remove(struct snd_soc_codec *codec)
+{
+	struct ak5386_priv *priv = snd_soc_codec_get_drvdata(codec);
+	regulator_bulk_disable(ARRAY_SIZE(priv->supplies), priv->supplies);
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int ak5386_soc_suspend(struct snd_soc_codec *codec)
+{
+	struct ak5386_priv *priv = snd_soc_codec_get_drvdata(codec);
+	regulator_bulk_disable(ARRAY_SIZE(priv->supplies), priv->supplies);
+	return 0;
+}
+
+static int ak5386_soc_resume(struct snd_soc_codec *codec)
+{
+	struct ak5386_priv *priv = snd_soc_codec_get_drvdata(codec);
+	return regulator_bulk_enable(ARRAY_SIZE(priv->supplies), priv->supplies);
+}
+#else
+#define ak5386_soc_suspend	NULL
+#define ak5386_soc_resume	NULL
+#endif /* CONFIG_PM */
+
 static struct snd_soc_codec_driver soc_codec_ak5386 = {
+	.probe = ak5386_soc_probe,
+	.remove = ak5386_soc_remove,
+	.suspend = ak5386_soc_suspend,
+	.resume = ak5386_soc_resume,
 	.dapm_widgets = ak5386_dapm_widgets,
 	.num_dapm_widgets = ARRAY_SIZE(ak5386_dapm_widgets),
 	.dapm_routes = ak5386_dapm_routes,
@@ -122,6 +163,7 @@ static int ak5386_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct ak5386_priv *priv;
+	int ret, i;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -130,6 +172,14 @@ static int ak5386_probe(struct platform_device *pdev)
 	priv->reset_gpio = -EINVAL;
 	dev_set_drvdata(dev, priv);
 
+	for (i = 0; i < ARRAY_SIZE(supply_names); i++)
+		priv->supplies[i].supply = supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(priv->supplies),
+				      priv->supplies);
+	if (ret < 0)
+		return ret;
+
 	if (of_match_device(of_match_ptr(ak5386_dt_ids), dev))
 		priv->reset_gpio = of_get_named_gpio(dev->of_node,
 						      "reset-gpio", 0);
-- 
1.8.5.3

[PATCH 2/3 v2] ASoC: ak4104: add regulator consumer support

[Daniel Mack]

The AK4104 has only one power supply, called VDD. Enable it as long as
the codec is in use.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 sound/soc/codecs/ak4104.c | 62 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/ak4104.c b/sound/soc/codecs/ak4104.c
index 10adf25..1fd7f72 100644
--- a/sound/soc/codecs/ak4104.c
+++ b/sound/soc/codecs/ak4104.c
@@ -11,13 +11,14 @@
 
 #include <linux/module.h>
 #include <linux/slab.h>
-#include <sound/core.h>
-#include <sound/soc.h>
-#include <sound/initval.h>
 #include <linux/spi/spi.h>
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
 #include <sound/asoundef.h>
+#include <sound/core.h>
+#include <sound/soc.h>
+#include <sound/initval.h>
 
 /* AK4104 registers addresses */
 #define AK4104_REG_CONTROL1		0x00
@@ -47,6 +48,7 @@
 
 struct ak4104_private {
 	struct regmap *regmap;
+	struct regulator *regulator;
 };
 
 static const struct snd_soc_dapm_widget ak4104_dapm_widgets[] = {
@@ -174,20 +176,30 @@ static int ak4104_probe(struct snd_soc_codec *codec)
 	struct ak4104_private *ak4104 = snd_soc_codec_get_drvdata(codec);
 	int ret;
 
+	ret = regulator_enable(ak4104->regulator);
+	if (ret < 0) {
+		dev_err(codec->dev, "Unable to enable regulator: %d\n", ret);
+		return ret;
+	}
+
 	/* set power-up and non-reset bits */
 	ret = regmap_update_bits(ak4104->regmap, AK4104_REG_CONTROL1,
 				 AK4104_CONTROL1_PW | AK4104_CONTROL1_RSTN,
 				 AK4104_CONTROL1_PW | AK4104_CONTROL1_RSTN);
 	if (ret < 0)
-		return ret;
+		goto exit_disable_regulator;
 
 	/* enable transmitter */
 	ret = regmap_update_bits(ak4104->regmap, AK4104_REG_TX,
 				 AK4104_TX_TXE, AK4104_TX_TXE);
 	if (ret < 0)
-		return ret;
+		goto exit_disable_regulator;
 
 	return 0;
+
+exit_disable_regulator:
+	regulator_disable(ak4104->regulator);
+	return ret;
 }
 
 static int ak4104_remove(struct snd_soc_codec *codec)
@@ -196,13 +208,42 @@ static int ak4104_remove(struct snd_soc_codec *codec)
 
 	regmap_update_bits(ak4104->regmap, AK4104_REG_CONTROL1,
 			   AK4104_CONTROL1_PW | AK4104_CONTROL1_RSTN, 0);
+	regulator_disable(ak4104->regulator);
 
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int ak4104_soc_suspend(struct snd_soc_codec *codec)
+{
+	struct ak4104_private *priv = snd_soc_codec_get_drvdata(codec);
+
+	regulator_disable(priv->regulator);
+
+	return 0;
+}
+
+static int ak4104_soc_resume(struct snd_soc_codec *codec)
+{
+	struct ak4104_private *priv = snd_soc_codec_get_drvdata(codec);
+	int ret;
+
+	ret = regulator_enable(priv->regulator);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+#else
+#define ak4104_soc_suspend	NULL
+#define ak4104_soc_resume	NULL
+#endif /* CONFIG_PM */
+
 static struct snd_soc_codec_driver soc_codec_device_ak4104 = {
-	.probe =	ak4104_probe,
-	.remove =	ak4104_remove,
+	.probe = ak4104_probe,
+	.remove = ak4104_remove,
+	.suspend = ak4104_soc_suspend,
+	.resume = ak4104_soc_resume,
 
 	.dapm_widgets = ak4104_dapm_widgets,
 	.num_dapm_widgets = ARRAY_SIZE(ak4104_dapm_widgets),
@@ -239,6 +280,13 @@ static int ak4104_spi_probe(struct spi_device *spi)
 	if (ak4104 == NULL)
 		return -ENOMEM;
 
+	ak4104->regulator = devm_regulator_get(&spi->dev, "vdd");
+	if (IS_ERR(ak4104->regulator)) {
+		ret = PTR_ERR(ak4104->regulator);
+		dev_err(&spi->dev, "Unable to get Vdd regulator: %d\n", ret);
+		return ret;
+	}
+
 	ak4104->regmap = devm_regmap_init_spi(spi, &ak4104_regmap);
 	if (IS_ERR(ak4104->regmap)) {
 		ret = PTR_ERR(ak4104->regmap);
-- 
1.8.5.3

Re: [PATCH 2/3 v2] ASoC: ak4104: add regulator consumer support

[Mark Brown]

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

On Thu, Mar 27, 2014 at 09:42:15PM +0100, Daniel Mack wrote:
> The AK4104 has only one power supply, called VDD. Enable it as long as
> the codec is in use.

This should update the DT binding too.  Also it seems we still have a
machine driver for Raumfeld using this which doesn't set full
constraints and isn't being updated here so would be broken as far as I
can tell.

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

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

Re: [PATCH 2/3 v2] ASoC: ak4104: add regulator consumer support

[Daniel Mack]

On 03/28/2014 12:23 PM, Mark Brown wrote:
> On Thu, Mar 27, 2014 at 09:42:15PM +0100, Daniel Mack wrote:
>> The AK4104 has only one power supply, called VDD. Enable it as long as
>> the codec is in use.
> 
> This should update the DT binding too.  Also it seems we still have a
> machine driver for Raumfeld using this which doesn't set full
> constraints and isn't being updated here so would be broken as far as I
> can tell.

Yeah, I noticed this one is unfortunately broken since a while already
due to other changes in ASoC. I'm working on fixing this in parallel,
but it's legacy hardware which get less attention than it should. I'll
fix up the constraints along with that.

Want me to resend with a DT bindings fix, or can we do that in 2nd step?


Daniel

Re: [PATCH 2/3 v2] ASoC: ak4104: add regulator consumer support

[Mark Brown]

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

On Fri, Mar 28, 2014 at 12:27:14PM +0100, Daniel Mack wrote:

> Yeah, I noticed this one is unfortunately broken since a while already
> due to other changes in ASoC. I'm working on fixing this in parallel,
> but it's legacy hardware which get less attention than it should. I'll
> fix up the constraints along with that.

> Want me to resend with a DT bindings fix, or can we do that in 2nd step?

Do it in a second step if Raumfeld is already broken anyway, I'll go
ahead and apply.

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

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

[PATCH 3/3 v2] ASoC: tas5086: add regulator consumer support

[Daniel Mack]

The TAS5086 has two power domains, DVDD and AVDD. Enable them both as
long as the codec is in use.

While at it, move the device identification from the i2c probe to the
codec probe, so we do it after the regulators have been enabled.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 sound/soc/codecs/tas5086.c | 64 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 50 insertions(+), 14 deletions(-)

diff --git a/sound/soc/codecs/tas5086.c b/sound/soc/codecs/tas5086.c
index a895a5e..2ee03b6 100644
--- a/sound/soc/codecs/tas5086.c
+++ b/sound/soc/codecs/tas5086.c
@@ -36,6 +36,7 @@
 #include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
@@ -240,6 +241,10 @@ static int tas5086_reg_read(void *context, unsigned int reg,
 	return 0;
 }
 
+static const char const *supply_names[] = {
+	"dvdd", "avdd"
+};
+
 struct tas5086_private {
 	struct regmap	*regmap;
 	unsigned int	mclk, sclk;
@@ -251,6 +256,7 @@ struct tas5086_private {
 	int		rate;
 	/* GPIO driving Reset pin, if any */
 	int		gpio_nreset;
+	struct		regulator_bulk_data supplies[ARRAY_SIZE(supply_names)];
 };
 
 static int tas5086_deemph[] = { 0, 32000, 44100, 48000 };
@@ -773,6 +779,8 @@ static int tas5086_soc_suspend(struct snd_soc_codec *codec)
 	if (ret < 0)
 		return ret;
 
+	regulator_bulk_disable(ARRAY_SIZE(priv->supplies), priv->supplies);
+
 	return 0;
 }
 
@@ -781,6 +789,10 @@ static int tas5086_soc_resume(struct snd_soc_codec *codec)
 	struct tas5086_private *priv = snd_soc_codec_get_drvdata(codec);
 	int ret;
 
+	ret = regulator_bulk_enable(ARRAY_SIZE(priv->supplies), priv->supplies);
+	if (ret < 0)
+		return ret;
+
 	tas5086_reset(priv);
 	regcache_mark_dirty(priv->regmap);
 
@@ -812,6 +824,25 @@ static int tas5086_probe(struct snd_soc_codec *codec)
 	struct tas5086_private *priv = snd_soc_codec_get_drvdata(codec);
 	int i, ret;
 
+	ret = regulator_bulk_enable(ARRAY_SIZE(priv->supplies), priv->supplies);
+	if (ret < 0) {
+		dev_err(codec->dev, "Failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
+	tas5086_reset(priv);
+
+	/* The TAS5086 always returns 0x03 in its TAS5086_DEV_ID register */
+	ret = regmap_read(priv->regmap, TAS5086_DEV_ID, &i);
+	if (ret < 0)
+		goto exit_disable_regulators;
+
+	if (i != 0x3) {
+		dev_err(codec->dev,
+			"Failed to identify TAS5086 codec (got %02x)\n", i);
+		goto exit_disable_regulators;
+	}
+
 	priv->pwm_start_mid_z = 0;
 	priv->charge_period = 1300000; /* hardware default is 1300 ms */
 
@@ -834,14 +865,19 @@ static int tas5086_probe(struct snd_soc_codec *codec)
 
 	ret = tas5086_init(codec->dev, priv);
 	if (ret < 0)
-		return ret;
+		goto exit_disable_regulators;
 
 	/* set master volume to 0 dB */
 	ret = regmap_write(priv->regmap, TAS5086_MASTER_VOL, 0x30);
 	if (ret < 0)
-		return ret;
+		goto exit_disable_regulators;
 
 	return 0;
+
+exit_disable_regulators:
+	regulator_bulk_disable(ARRAY_SIZE(priv->supplies), priv->supplies);
+
+	return ret;
 }
 
 static int tas5086_remove(struct snd_soc_codec *codec)
@@ -852,6 +888,8 @@ static int tas5086_remove(struct snd_soc_codec *codec)
 		/* Set codec to the reset state */
 		gpio_set_value(priv->gpio_nreset, 0);
 
+	regulator_bulk_disable(ARRAY_SIZE(priv->supplies), priv->supplies);
+
 	return 0;
 };
 
@@ -900,6 +938,16 @@ static int tas5086_i2c_probe(struct i2c_client *i2c,
 	if (!priv)
 		return -ENOMEM;
 
+	for (i = 0; i < ARRAY_SIZE(supply_names); i++)
+		priv->supplies[i].supply = supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(priv->supplies),
+				      priv->supplies);
+	if (ret < 0) {
+		dev_err(dev, "Failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
 	priv->regmap = devm_regmap_init(dev, NULL, i2c, &tas5086_regmap);
 	if (IS_ERR(priv->regmap)) {
 		ret = PTR_ERR(priv->regmap);
@@ -919,18 +967,6 @@ static int tas5086_i2c_probe(struct i2c_client *i2c,
 			gpio_nreset = -EINVAL;
 
 	priv->gpio_nreset = gpio_nreset;
-	tas5086_reset(priv);
-
-	/* The TAS5086 always returns 0x03 in its TAS5086_DEV_ID register */
-	ret = regmap_read(priv->regmap, TAS5086_DEV_ID, &i);
-	if (ret < 0)
-		return ret;
-
-	if (i != 0x3) {
-		dev_err(dev,
-			"Failed to identify TAS5086 codec (got %02x)\n", i);
-		return -ENODEV;
-	}
 
 	return snd_soc_register_codec(&i2c->dev, &soc_codec_dev_tas5086,
 		&tas5086_dai, 1);
-- 
1.8.5.3

Re: [PATCH 3/3 v2] ASoC: tas5086: add regulator consumer support

[Mark Brown]

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

On Thu, Mar 27, 2014 at 09:42:16PM +0100, Daniel Mack wrote:
> The TAS5086 has two power domains, DVDD and AVDD. Enable them both as
> long as the codec is in use.

DT binding update for this one as well please.

> While at it, move the device identification from the i2c probe to the
> codec probe, so we do it after the regulators have been enabled.

It'd be better style to enable the regulators during probe to read the
ID register rather than deferring.

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

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

Re: [PATCH 3/3 v2] ASoC: tas5086: add regulator consumer support

[Daniel Mack]

On 03/28/2014 12:35 PM, Mark Brown wrote:
> On Thu, Mar 27, 2014 at 09:42:16PM +0100, Daniel Mack wrote:
>> While at it, move the device identification from the i2c probe to the
>> codec probe, so we do it after the regulators have been enabled.
> 
> It'd be better style to enable the regulators during probe to read the
> ID register rather than deferring.

I had it that way, but that meant I had to introduce device-level pm
functions in parallel to the soc-level ones, in order to really bring
down the supply in suspend, even in setups where the codec is not
actually used.

Nevermind, I can change it back. Will resend the patches for tas5086 and
ak4104.


Thanks,
Daniel

Re: [PATCH 3/3 v2] ASoC: tas5086: add regulator consumer support

[Mark Brown]

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

On Fri, Mar 28, 2014 at 12:40:09PM +0100, Daniel Mack wrote:
> On 03/28/2014 12:35 PM, Mark Brown wrote:

> > It'd be better style to enable the regulators during probe to read the
> > ID register rather than deferring.

> I had it that way, but that meant I had to introduce device-level pm
> functions in parallel to the soc-level ones, in order to really bring
> down the supply in suspend, even in setups where the codec is not
> actually used.

> Nevermind, I can change it back. Will resend the patches for tas5086 and
> ak4104.

The ususal thing is to power on and off during the probe (though this is
mainly for devices which actively manage the regualtor at runtime rather
than just leaving the regulator on while the sound card is active.

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

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

Re: [PATCH 1/3 v2] ASoC: ak5386: add regulator consumer support

[Mark Brown]

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

On Thu, Mar 27, 2014 at 09:42:14PM +0100, Daniel Mack wrote:
> The chip has two power supplies, VA and VDD. Enable them both as long
> as the codec is in use.

Applied, though you need to update the DT bindings too.

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

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