Re: [PATCH 7/8] ASoC: tlv320aic3x: add initial usage of regulator framework to control avdd_dac
On Fri, Oct 09, 2009 at 07:28:03AM +0300, Eero Nurkkala wrote: On Thu, 2009-10-08 at 18:01 +0200, ext Mark Brown wrote: I'd expect the usage would be that after the audio subsystem has been idle for some configurable period of time the core would bring the audio subsystem down to bias off, at which point supplies could also be switched off. Right. That would also sound like the RST line also needs also be asserted, and then rewriting all register contents upon wakeup? And also redirecting all i2c traffic to the cache instead of any real i2c writes (meanwhile the device is shut down)? Like in tpa6130? Yes. I'd expect that if this is being implemented there'll be some degree of assistance from the core for some of this. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] ASoC: tlv320aic3x: add initial usage of regulator framework to control avdd_dac
From: Eduardo Valentin eduardo.valen...@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.valen...@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.
+*/
+ if (aic3x-regulator) {
+ int err;
+
+ err = regulator_enable(aic3x-regulator);
+ if (err 0)
+ return err;
+ }
+
i2c_set_clientdata(i2c, aic3x);
return aic3x_register(codec);
--
1.6.4.183.g04423
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ASoC: tlv320aic3x: add initial usage of regulator framework to control avdd_dac
On Thu, 2009-10-08 at 13:58 +0200, Valentin Eduardo (Nokia-D/Helsinki)
wrote:
From: Eduardo Valentin eduardo.valen...@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.valen...@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 ;)
+ if (aic3x-regulator) {
+ int err;
+
+ err = regulator_enable(aic3x-regulator);
+ if (err 0)
+ return err;
+ }
+
i2c_set_clientdata(i2c, aic3x);
return aic3x_register(codec);
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ASoC: tlv320aic3x: add initial usage of regulator framework to control avdd_dac
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.valen...@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.valen...@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);
--
Eduardo Valentin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ASoC: tlv320aic3x: add initial usage of regulator framework to control avdd_dac
On Thu, Oct 08, 2009 at 02:58:56PM +0300, Eduardo Valentin wrote:
This patch adds initial usage of regulator framework to control avdd_dac
inside tlv320aic3x ASoC codec driver.
If you're going to do this you should add support for all the supplies
of the device, not just one of them. The extra effort required is low
and it avoids compatibility problems when someone wants to set up other
supplies, causing existing boards to need to specify them.
+ 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);
You shouldn't split error messages over multiple lines like this, it
breaks grepping to try to find where the message came from.
I'd also rather see failure to get the regulator treated as a hard
error. When the regulator API compiled out it is stubbed so that for
simple get/enable/disable/put usage it will return success but do
nothing. If the regulator API is compiled in and we're not able to
acquire regulators there's a good chance that things will break (eg, due
to supplies being turned off because they appear to be unused) so
flagging the error immediately is less likely to result in runtime
fragility.
+ /*
+ * REVISIT: Need to add proper code to put into sleep mode
+ * avdd_dac regulator. For now, just leave it on.
+ */
+ if (aic3x-regulator) {
+ int err;
+
+ err = regulator_enable(aic3x-regulator);
+ if (err 0)
+ return err;
+ }
The best way to handle this is to push the enable/disable into the bias
level configuration so that the regulators are enabled when the chip
goes off-standby and disabled during standby-off. This will have the
same effect for the moment but will mean that we'll be able to add core
support for fully powering down the audio subsystem at runtime in the
future.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ASoC: tlv320aic3x: add initial usage of regulator framework to control avdd_dac
On Thu, Oct 08, 2009 at 03:17:07PM +0300, Eero Nurkkala wrote: 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). The bias is always on when any path through the chip is on, this was fixed in either .31 or .30. 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 ;) I'm not sure what you mean by this? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/8] ASoC: tlv320aic3x: add initial usage of regulator framework to control avdd_dac
Mark Brown wrote: 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). The bias is always on when any path through the chip is on, this was fixed in either .31 or .30. Good! Thanks for the update. 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 ;) I'm not sure what you mean by this? You may power the aic3x from a fixed source, or from multiple sources, with and without any regulator in between. It's up to the HW and HW design. Moreover, you don't _power off_ (turn the regulator off) the analog voltages of aic3x; things won't work. So it's not like a switch everybody may use. Or nothing prevent you from experiencing that... - EEro -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ASoC: tlv320aic3x: add initial usage of regulator framework to control avdd_dac
On 8 Oct 2009, at 16:44, ext-eero.nurkk...@nokia.com wrote: Mark Brown wrote: 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 ;) I'm not sure what you mean by this? You may power the aic3x from a fixed source, or from multiple sources, with and without any regulator in between. It's up to the HW and HW design. The regulator API can cope with all this pretty transparently - if multiple supplies come from the same regulator the API will hide that from the consumer. There is a fixed voltage regulator driver which can be used to represent supplies with no soft control. Moreover, you don't _power off_ (turn the regulator off) the analog voltages of aic3x; things won't work. So it's not like a switch everybody may use. Or nothing prevent you from experiencing that... I'd expect the usage would be that after the audio subsystem has been idle for some configurable period of time the core would bring the audio subsystem down to bias off, at which point supplies could also be switched off. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] ASoC: tlv320aic3x: add initial usage of regulator framework to control avdd_dac
On Thu, 2009-10-08 at 18:01 +0200, ext Mark Brown wrote: On 8 Oct 2009, at 16:44, ext-eero.nurkk...@nokia.com wrote: Mark Brown wrote: 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 ;) I'm not sure what you mean by this? You may power the aic3x from a fixed source, or from multiple sources, with and without any regulator in between. It's up to the HW and HW design. The regulator API can cope with all this pretty transparently - if multiple supplies come from the same regulator the API will hide that from the consumer. There is a fixed voltage regulator driver which can be used to represent supplies with no soft control. Moreover, you don't _power off_ (turn the regulator off) the analog voltages of aic3x; things won't work. So it's not like a switch everybody may use. Or nothing prevent you from experiencing that... I'd expect the usage would be that after the audio subsystem has been idle for some configurable period of time the core would bring the audio subsystem down to bias off, at which point supplies could also be switched off. Right. That would also sound like the RST line also needs also be asserted, and then rewriting all register contents upon wakeup? And also redirecting all i2c traffic to the cache instead of any real i2c writes (meanwhile the device is shut down)? Like in tpa6130? - Eero -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
