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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > ---
> > 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 alsa-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
