On Thu, Oct 08, 2009 at 02:17:07PM +0200, Nurkkala Eero.An (EXT-Offcode/Oulu) wrote:
On Thu, 2009-10-08 at 13:58 +0200, Valentin Eduardo (Nokia-D/Helsinki) wrote:
From: Eduardo Valentin eduardo.valentin@nokia.com
This patch adds initial usage of regulator framework to control avdd_dac inside tlv320aic3x ASoC codec driver.
The refcount to avdd_dac is increased / decreased only during probe and remove. Here it is still needed to implement proper enable/disable regulator depending on chip usage. Now if driver can get regulator for avdd_dac, then it will just let it on on probe and then leave it off on remove.
Signed-off-by: Eduardo Valentin eduardo.valentin@nokia.com
sound/soc/codecs/tlv320aic3x.c | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 3395cf9..82e0a64 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -38,6 +38,7 @@ #include <linux/delay.h> #include <linux/pm.h> #include <linux/i2c.h> +#include <linux/regulator/consumer.h> #include <linux/platform_device.h> #include <sound/core.h> #include <sound/pcm.h> @@ -56,6 +57,7 @@ struct aic3x_priv { struct snd_soc_codec codec; unsigned int sysclk; int master;
- struct regulator *regulator;
};
/* @@ -1286,6 +1288,11 @@ static int aic3x_unregister(struct aic3x_priv *aic3x) snd_soc_unregister_dai(&aic3x_dai); snd_soc_unregister_codec(&aic3x->codec);
- if (aic3x->regulator) {
regulator_disable(aic3x->regulator);
regulator_put(aic3x->regulator);
- }
- kfree(aic3x); aic3x_codec = NULL;
@@ -1320,6 +1327,25 @@ static int aic3x_i2c_probe(struct i2c_client *i2c, codec->control_data = i2c; codec->hw_write = (hw_write_t) i2c_master_send;
- aic3x->regulator = regulator_get(&i2c->dev, "avdd_dac");
- if (IS_ERR(aic3x->regulator)) {
dev_warn(&i2c->dev, "No regulator to supply avdd_dac."
" Assuming always on.\n");
aic3x->regulator = NULL;
- }
- /*
* REVISIT: Need to add proper code to put into sleep mode
* avdd_dac regulator. For now, just leave it on.
*/
Will this ever be revisited =) ? If so, I think there's going to be a jungle in finding the right spots - you need to remember the bypass paths also (bias is not on necessarily). Also, this is regulator thing is highly platform dependent, not aic3x related really at all, so is this the correct place... Just a thought, dont take it too seriously ;)
Heheh.. no I don't take it too seriously, don't worry :-)
Actually I've discussed about it with Peter. Initially I wrote it inside rx51 machine driver. But then, we agreed it is actually a thing which must be controlled by the driver. The same way it is done for TPA for instance.
Of course, the presence of that regulator must not be a blocker for the driver. As you can see, if the regulator can not be queried, the driver will assume that it is always on.
I must agree with you, but would rephrase as: the presence of this regulator is board specific, but controlling when it must be enabled/disabled is a role for the driver, in this case, tlv320aic3x.
What do you think ?
if (aic3x->regulator) {
int err;
err = regulator_enable(aic3x->regulator);
if (err < 0)
return err;
}
i2c_set_clientdata(i2c, aic3x);
return aic3x_register(codec);