[alsa-devel] [PATCHv4 7/7] ASoC: TWL6030: Detect power-up sequence completion
From: Misael Lopez Cruz x0052729@ti.com
When the codec is powered-up through external AUDPWRON line it starts its power-up sequence. The completion of the sequence is signaled through the audio interrupt, and then codec is operational.
If NAUDINT irq is provided, CODEC driver starts a wait_for_completion just after AUDPWRON line transitions from low to high. It's signaled as complete when servicing READYINT interrupt.
If AUDPWRON gpio line is provided but NAUDINT irq is not, then CODEC driver enables READYINT and polls on INTID register. If none of them are provided, then CODEC uses manual power sequences and disables all audio interrupts.
Signed-off-by: Misael Lopez Cruz x0052729@ti.com Signed-off-by: Jorge Eduardo Candelaria jorge.candelaria@ti.com Signed-off-by: Margarita Olaya Cabrera magi.olaya@ti.com --- sound/soc/codecs/twl6030.c | 61 +++++++++++++++++++++++++++++++++++++------ sound/soc/codecs/twl6030.h | 1 + 2 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/sound/soc/codecs/twl6030.c b/sound/soc/codecs/twl6030.c index b8dd5ae..2847f1b 100644 --- a/sound/soc/codecs/twl6030.c +++ b/sound/soc/codecs/twl6030.c @@ -52,6 +52,7 @@ struct twl6030_data { int non_lp; unsigned int sysclk; struct snd_pcm_hw_constraint_list *sysclk_constraints; + struct completion ready; };
/* @@ -372,6 +373,7 @@ static int twl6030_power_mode_event(struct snd_soc_dapm_widget *w, static irqreturn_t twl6030_naudint_handler(int irq, void *data) { struct snd_soc_codec *codec = data; + struct twl6030_data *priv = codec->private_data; u8 intid;
twl_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &intid, TWL6030_REG_INTID); @@ -391,7 +393,7 @@ static irqreturn_t twl6030_naudint_handler(int irq, void *data) dev_alert(codec->dev, "vib drivers over current detection\n"); break; case TWL6030_READYINT: - dev_alert(codec->dev, "codec is ready\n"); + complete(&priv->ready); break; default: dev_err(codec->dev, "unknown audio interrupt %d\n", intid); @@ -626,11 +628,45 @@ static int twl6030_add_widgets(struct snd_soc_codec *codec) return 0; }
+static int twl6030_power_up_completion(struct snd_soc_codec *codec, + int naudint) +{ + struct twl6030_data *priv = codec->private_data; + int time_left; + u8 intid; + + if (naudint) { + /* wait for ready interrupt with 48 ms timeout */ + time_left = wait_for_completion_timeout(&priv->ready, + msecs_to_jiffies(48)); + } else { + /* retry 3 times only */ + for (time_left = 3; time_left > 0; time_left--) { + mdelay(16); + twl_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &intid, + TWL6030_REG_INTID); + if (intid & TWL6030_READYINT) + break; + } + } + + if (!time_left) { + dev_err(codec->dev, "timeout waiting for READYINT\n"); + return -ETIMEDOUT; + } + + priv->codec_powered = 1; + + return 0; +} + static int twl6030_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) { struct twl6030_data *priv = codec->private_data; int audpwron = priv->audpwron; + int naudint = priv->naudint; + int ret;
switch (level) { case SND_SOC_BIAS_ON: @@ -643,8 +679,10 @@ static int twl6030_set_bias_level(struct snd_soc_codec *codec, /* use AUDPWRON line */ gpio_set_value(audpwron, 1);
- /* power-up sequence latency */ - mdelay(16); + /* wait for power-up completion */ + ret = twl6030_power_up_completion(codec, naudint); + if (ret) + return ret;
/* sync registers updated during power-up sequence */ twl6030_read(codec, TWL6030_REG_NCPCTL); @@ -653,12 +691,11 @@ static int twl6030_set_bias_level(struct snd_soc_codec *codec, } else { /* use manual power-up sequence */ twl6030_power_up(codec); + priv->codec_powered = 1; }
/* initialize vdd/vss registers with reg_cache */ twl6030_init_vdd_regs(codec); - - priv->codec_powered = 1; break; case SND_SOC_BIAS_OFF: if (!priv->codec_powered) @@ -1067,6 +1104,7 @@ static int __devinit twl6030_codec_probe(struct platform_device *pdev) mutex_init(&codec->mutex); INIT_LIST_HEAD(&codec->dapm_widgets); INIT_LIST_HEAD(&codec->dapm_paths); + init_completion(&priv->ready);
if (gpio_is_valid(audpwron)) { ret = gpio_request(audpwron, "audpwron"); @@ -1089,10 +1127,15 @@ static int __devinit twl6030_codec_probe(struct platform_device *pdev) if (ret) goto gpio2_err; } else { - dev_warn(codec->dev, - "no naudint irq, audio interrupts disabled\n"); - twl6030_write_reg_cache(codec, TWL6030_REG_INTMR, - TWL6030_ALLINT_MSK); + if (gpio_is_valid(audpwron)) { + /* enable only codec ready interrupt */ + twl6030_write_reg_cache(codec, TWL6030_REG_INTMR, + ~TWL6030_READYMSK & TWL6030_ALLINT_MSK); + } else { + /* no interrupts at all */ + twl6030_write_reg_cache(codec, TWL6030_REG_INTMR, + TWL6030_ALLINT_MSK); + } }
/* init vio registers */ diff --git a/sound/soc/codecs/twl6030.h b/sound/soc/codecs/twl6030.h index 90b8a44..d591be1 100644 --- a/sound/soc/codecs/twl6030.h +++ b/sound/soc/codecs/twl6030.h @@ -79,6 +79,7 @@
/* INTMR (0x04) fields */
+#define TWL6030_READYMSK 0x40 #define TWL6030_ALLINT_MSK 0x7B
/* NCPCTL (0x05) fields */
On Tue, Feb 23, 2010 at 06:10:54PM -0600, Olaya, Margarita wrote:
- if (naudint) {
/* wait for ready interrupt with 48 ms timeout */time_left = wait_for_completion_timeout(&priv->ready,msecs_to_jiffies(48));- } else {
/* retry 3 times only */for (time_left = 3; time_left > 0; time_left--) {mdelay(16);twl_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &intid,TWL6030_REG_INTID);if (intid & TWL6030_READYINT)break;}- }
It strikes me that you could combine these two cases - the wait_for_completion_timeout() will function just as well as a delay. I'd also expect to see an error reported if the device doesn't report as ready one way or another.
-----Original Message----- From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com] Sent: Wednesday, February 24, 2010 7:59 AM To: Olaya, Margarita Cc: alsa-devel@alsa-project.org; linux-omap@vger.kernel.org; lrg@slimlogic.co.uk Subject: Re: [PATCHv4 7/7] ASoC: TWL6030: Detect power-up sequence completion
On Tue, Feb 23, 2010 at 06:10:54PM -0600, Olaya, Margarita wrote:
- if (naudint) {
/* wait for ready interrupt with 48 ms timeout */time_left = wait_for_completion_timeout(&priv->ready,msecs_to_jiffies(48));- } else {
/* retry 3 times only */for (time_left = 3; time_left > 0; time_left--) {mdelay(16);twl_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &intid,TWL6030_REG_INTID);if (intid & TWL6030_READYINT)break;}- }
It strikes me that you could combine these two cases - the wait_for_completion_timeout() will function just as well as a delay. I'd also expect to see an error reported if the device doesn't report as ready one way or another.
It is split to prevent the case of none valid irq line connected, in such case, wait_for_completion won't work.
Regards, Margarita
On 25 Feb 2010, at 00:24, "Olaya, Margarita" magi.olaya@ti.com wrote:
-----Original Message----- From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com] Sent: Wednesday, February 24, 2010 7:59 AM To: Olaya, Margarita Cc: alsa-devel@alsa-project.org; linux-omap@vger.kernel.org; lrg@slimlogic.co.uk Subject: Re: [PATCHv4 7/7] ASoC: TWL6030: Detect power-up sequence completion
On Tue, Feb 23, 2010 at 06:10:54PM -0600, Olaya, Margarita wrote:
- if (naudint) {
/* wait for ready interrupt with 48 ms timeout */time_left = wait_for_completion_timeout(&priv->ready,msecs_to_jiffies(48));- } else {
/* retry 3 times only */for (time_left = 3; time_left > 0; time_left--) {mdelay(16);twl_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &intid,TWL6030_REG_INTID);if (intid & TWL6030_READYINT)break;}- }
It strikes me that you could combine these two cases - the wait_for_completion_timeout() will function just as well as a delay. I'd also expect to see an error reported if the device doesn't report as ready one way or another.
It is split to prevent the case of none valid irq line connected, in such case, wait_for_completion won't work
It will - you can specify a timeout so if the interrupt doesn't happen all that happens is that you delay for the specified timeout.
On Thursday, February 25, 2010 2:42 AM Mark Brown wrote:
On Tue, Feb 23, 2010 at 06:10:54PM -0600, Olaya, Margarita wrote:
- if (naudint) {
/* wait for ready interrupt with 48 ms timeout */time_left = wait_for_completion_timeout(&priv->ready,msecs_to_jiffies(48));
Phoenix manages automatic and manual power on sequences. READYINT indicates the completion of power up sequence, Phoenix audio drives the NAUDINT line low when an interrupt is internally detected, when automatic power on sequence is used, the state of READYINIT is verified through the interrupt handler using wait_for_completion.
- } else {
/* retry 3 times only */for (time_left = 3; time_left > 0; time_left--) { +mdelay(16); + twl_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &intid, + TWL6030_REG_INTID); + if (intid & TWL6030_READYINT)
break;}
When manual power on sequence is used the driver verifies the status of READYINIT by polling.
In both cases if READYINIT is not set before the timeout runs out it means the codec is not powering on and the driver reports an error.
- Margarita
- }
It strikes me that you could combine these two cases - the wait_for_completion_timeout() will function just as well as a delay. I'd also expect to see an error reported if the device doesn't report as ready one way or another.
It is split to prevent the case of none valid irq line connected, in such case, wait_for_completion won't work
It will - you can specify a timeout so if the interrupt doesn't happen all that happens is that you delay for the specified timeout.
On Fri, Feb 26, 2010 at 03:04:22PM -0600, Olaya, Margarita wrote:
In both cases if READYINIT is not set before the timeout runs out it means the codec is not powering on and the driver reports an error.
That's kind of my point - because you're checking for the same status there shouldn't be any need to special case the situation where there's no interrupt, I'd expect to be able to use wait_for_timeout_interruptible() for the delay and have the timeout completion interrupt cause that to be signalled. No *terribly* important but it makes it clearer what's going on.
On Friday, February 26, 2010 3:27 PM Mark Brown wrote:
On Fri, Feb 26, 2010 at 03:04:22PM -0600, Olaya, Margarita wrote:
In both cases if READYINIT is not set before the timeout runs out it means the codec is not powering on and the driver reports an error.
That's kind of my point - because you're checking for the same status there shouldn't be any need to special case the situation where there's no interrupt, I'd expect to be able to use wait_for_timeout_interruptible() for the delay and have the timeout completion interrupt cause that to be signalled. No *terribly* important but it makes it clearer what's going on.
Do you mean something like this? time_left = wait_for_completion_timeout(&priv->ready, msecs_to_jiffies(48)); if(!time_left) { twl_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &intid, TWL6040_REG_INTID); if (!(intid & TWL6040_READYINT)) goto error; }
return 0;
error: dev_err(codec->dev, "timeout waiting for READYINT\n"); return -ETIMEDOUT;
but in this case will it not take unnecessarily 48ms when the interruption line is not valid?
Regards, Margarita
On Fri, Feb 26, 2010 at 06:22:34PM -0600, Olaya, Margarita wrote:
Do you mean something like this? time_left = wait_for_completion_timeout(&priv->ready, msecs_to_jiffies(48)); if(!time_left) { twl_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &intid, TWL6040_REG_INTID); if (!(intid & TWL6040_READYINT)) goto error; }
return 0;
error: dev_err(codec->dev, "timeout waiting for READYINT\n"); return -ETIMEDOUT;
Yes, or wrapped in a for loop with shorter timeouts on the individual waits.
but in this case will it not take unnecessarily 48ms when the interruption line is not valid?
You're always going to get some additional delay when polling unless you busy wait for completion, which obviously has its own problems.
On Monday, March 01, 2010 6:14 AM Mark Brown wrote:
On Fri, Feb 26, 2010 at 06:22:34PM -0600, Olaya, Margarita wrote:
Do you mean something like this? time_left = wait_for_completion_timeout(&priv->ready, msecs_to_jiffies(48)); if(!time_left) { twl_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &intid, TWL6040_REG_INTID); if (!(intid & TWL6040_READYINT)) goto error; }
return 0;
error: dev_err(codec->dev, "timeout waiting for READYINT\n"); return -ETIMEDOUT;
Yes, or wrapped in a for loop with shorter timeouts on the individual waits.
but in this case will it not take unnecessarily 48ms when the interruption line is not valid?
You're always going to get some additional delay when polling unless you busy wait for completion, which obviously has its own problems.
Ok, thanks for the comment I'll re-write the loop for next version of patches
participants (2)
-
Mark Brown -
Olaya, Margarita