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

Commit Message

Daniel Mack March 27, 2014, 8:42 p.m. UTC

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 <[e-mail]>
---
 sound/soc/codecs/tas5086.c | 64 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 50 insertions(+), 14 deletions(-)

Comments

Mark Brown March 28, 2014, 11:35 a.m. UTC | #1

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.

Daniel Mack March 28, 2014, 11:40 a.m. UTC | #2

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

Mark Brown March 28, 2014, 11:52 a.m. UTC | #3

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.

Patch

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