[alsa-devel] [PATCH] codecs/tlv320aic23: fix suspend/resume
The PWR register actually can't be restored at resume because its cached value was set to 0xFFFF when suspending and thus the initial state won't be restored. Thus, sound doesn't work after resume (tested on an i.MX27).
To workaround this, this patch saves the PWR value before turning the codec's power off and store this value in the cached registers.
Signed-off-by: Eric Bénard eric@eukrea.com Cc: broonie@opensource.wolfsonmicro.com Cc: anuj.aggarwal@ti.com Cc: lrg@slimlogic.co.uk --- sound/soc/codecs/tlv320aic23.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic23.c b/sound/soc/codecs/tlv320aic23.c index b0bae35..75e320b 100644 --- a/sound/soc/codecs/tlv320aic23.c +++ b/sound/soc/codecs/tlv320aic23.c @@ -614,9 +614,15 @@ static int tlv320aic23_suspend(struct platform_device *pdev, { struct snd_soc_device *socdev = platform_get_drvdata(pdev); struct snd_soc_codec *codec = socdev->card->codec; + u16 tmp;
tlv320aic23_write(codec, TLV320AIC23_ACTIVE, 0x0); + /* backup PWR register */ + tmp = tlv320aic23_read_reg_cache(codec, TLV320AIC23_PWR); + /* turn off the codec */ tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_OFF); + /* restore PWR register in the cache */ + tlv320aic23_write_reg_cache(codec, TLV320AIC23_PWR, tmp);
return 0; } @@ -633,8 +639,6 @@ static int tlv320aic23_resume(struct platform_device *pdev) tlv320aic23_write(codec, reg, val); }
- tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_STANDBY); - return 0; }
On Sat, Jun 19, 2010 at 10:56:54AM +0200, Eric Bénard wrote:
The PWR register actually can't be restored at resume because its cached value was set to 0xFFFF when suspending and thus the initial state won't be restored. Thus, sound doesn't work after resume (tested on an i.MX27).
What is the actual problem here?
- /* backup PWR register */
- tmp = tlv320aic23_read_reg_cache(codec, TLV320AIC23_PWR);
- /* turn off the codec */ tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_OFF);
On power down we put the CODEC into BIAS_OFF...
- tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
...then we bring the power back up on resume. If there's a problem here it looks like the bias management functions need fixing but without knowing what the actual problem with the system is it's not clear what this is supposed to fix.
Le 19/06/2010 12:59, Mark Brown a écrit :
On Sat, Jun 19, 2010 at 10:56:54AM +0200, Eric Bénard wrote:
The PWR register actually can't be restored at resume because its cached value was set to 0xFFFF when suspending and thus the initial state won't be restored. Thus, sound doesn't work after resume (tested on an i.MX27).
What is the actual problem here?
aplay test.wav => sound echo mem > /sys/power/state => system suspended press button to wake up => system resumed aplay test.wav => no sound
at wake up, the codec doesn't produce any sound and checking with a scope shows that the I2S interface (codec is master and thus generates the signals) is not working (no clock produced by the codec).
- /* backup PWR register */
- tmp = tlv320aic23_read_reg_cache(codec, TLV320AIC23_PWR);
- /* turn off the codec */ tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_OFF);
On power down we put the CODEC into BIAS_OFF...
- tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
...then we bring the power back up on resume. If there's a problem here it looks like the bias management functions need fixing but without knowing what the actual problem with the system is it's not clear what this is supposed to fix.
my understanding of the actual code is :
- at suspend : tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_OFF); will set PWR to 0xFFFF (all off) which is the expected behaviour
- at resume : tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_STANDBY); will set PWR to 0xff7f (all off except DAC) as its value is restored from the register cache, which doesn't seems to be the expected behaviour unless I'm one more time mistaken.
You will find hereinafter log added to tlv320aic23_write concerning PWR register and reg value in tlv320aic23_set_bias_level (patch copy and pasted bellow so it may be line wrapped) :
boot : tlv320aic23_set_bias_level : reg is 7 tlv320aic23_write write 047 to register R6
aplay : tlv320aic23_set_bias_level : reg is 5f tlv320aic23_write write 057 to register R6 (so DEVICE + OSC + DAC are on) tlv320aic23_set_bias_level : reg is 47 tlv320aic23_write write 047 to register R6 (so DEVICE + OSC + OUT + DAC are on)
suspend : tlv320aic23_set_bias_level : reg is 5f tlv320aic23_write write ffff to register R6
resume : tlv320aic23_write write ffff to register R6 tlv320aic23_set_bias_level : reg is ff7f tlv320aic23_write write ff7f to register R6
aplay (after resume) : tlv320aic23_set_bias_level : reg is ff7f tlv320aic23_write write ff7f to register R6 (so DEVICE is on but DAC and OSC are off) tlv320aic23_set_bias_level : reg is ff67 tlv320aic23_write write ff67 to register R6 (so DEVICE + OUT + DAC are on but OSC is off)
So do unless I'm mistaken the actual handling of PWR register is wrong as OSC bit stays OFF even after resume.
Eric
--- a/sound/soc/codecs/tlv320aic23.c +++ b/sound/soc/codecs/tlv320aic23.c @@ -95,6 +95,8 @@ static int tlv320aic23_write(struct snd_soc_codec *codec, unsigned int reg, data[1] = value & 0xff;
tlv320aic23_write_reg_cache(codec, reg, value); +printk("%s write %03x to register R%u\n", __func__, + value, reg);
if (codec->hw_write(codec->control_data, data, 2) == 2) return 0; @@ -556,7 +558,7 @@ static int tlv320aic23_set_bias_level(struct snd_soc_codec *codec, enum snd_soc_bias_level level) { u16 reg = tlv320aic23_read_reg_cache(codec, TLV320AIC23_PWR) & 0xff7f; - +printk("%s : reg is %x\n", __FUNCTION__, reg); switch (level) { case SND_SOC_BIAS_ON: /* vref/mid, osc on, dac unmute */
in tlv320aic23_set_bias_level, for the case SND_SOC_BIAS_ON, the comment says "vref/mid, osc on, dac unmute" but the code doesn't clear the corresponding bits, thus when resuming, several bits are not cleared preventing the codec from working.
in tlv320aic23_suspend, clearing the active register is not needed as it will be done by tlv320aic23_set_bias_level, when setting bias to SND_SOC_BIAS_OFF
Signed-off-by: Eric Bénard eric@eukrea.com Cc: broonie@opensource.wolfsonmicro.com Cc: anuj.aggarwal@ti.com Cc: lrg@slimlogic.co.uk --- v2: fix the problem in tlv320aic23_set_bias_level instead of the previous bad workaround in suspend.
sound/soc/codecs/tlv320aic23.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic23.c b/sound/soc/codecs/tlv320aic23.c index b0bae35..0a4b0fe 100644 --- a/sound/soc/codecs/tlv320aic23.c +++ b/sound/soc/codecs/tlv320aic23.c @@ -560,13 +560,16 @@ static int tlv320aic23_set_bias_level(struct snd_soc_codec *codec, switch (level) { case SND_SOC_BIAS_ON: /* vref/mid, osc on, dac unmute */ + reg &= ~(TLV320AIC23_DEVICE_PWR_OFF | TLV320AIC23_OSC_OFF | \ + TLV320AIC23_DAC_OFF); tlv320aic23_write(codec, TLV320AIC23_PWR, reg); break; case SND_SOC_BIAS_PREPARE: break; case SND_SOC_BIAS_STANDBY: /* everything off except vref/vmid, */ - tlv320aic23_write(codec, TLV320AIC23_PWR, reg | 0x0040); + tlv320aic23_write(codec, TLV320AIC23_PWR, reg | \ + TLV320AIC23_CLK_OFF); break; case SND_SOC_BIAS_OFF: /* everything off, dac mute, inactive */ @@ -615,7 +618,6 @@ static int tlv320aic23_suspend(struct platform_device *pdev, struct snd_soc_device *socdev = platform_get_drvdata(pdev); struct snd_soc_codec *codec = socdev->card->codec;
- tlv320aic23_write(codec, TLV320AIC23_ACTIVE, 0x0); tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_OFF);
return 0; @@ -632,7 +634,6 @@ static int tlv320aic23_resume(struct platform_device *pdev) u16 val = tlv320aic23_read_reg_cache(codec, reg); tlv320aic23_write(codec, reg, val); } - tlv320aic23_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
return 0;
On Sat, 2010-06-19 at 19:33 +0200, Eric Bénard wrote:
in tlv320aic23_set_bias_level, for the case SND_SOC_BIAS_ON, the comment says "vref/mid, osc on, dac unmute" but the code doesn't clear the corresponding bits, thus when resuming, several bits are not cleared preventing the codec from working.
in tlv320aic23_suspend, clearing the active register is not needed as it will be done by tlv320aic23_set_bias_level, when setting bias to SND_SOC_BIAS_OFF
Signed-off-by: Eric Bénard eric@eukrea.com Cc: broonie@opensource.wolfsonmicro.com Cc: anuj.aggarwal@ti.com Cc: lrg@slimlogic.co.uk
v2: fix the problem in tlv320aic23_set_bias_level instead of the previous bad workaround in suspend.
sound/soc/codecs/tlv320aic23.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
On Sat, Jun 19, 2010 at 07:33:39PM +0200, Eric Bénard wrote:
in tlv320aic23_set_bias_level, for the case SND_SOC_BIAS_ON, the comment says "vref/mid, osc on, dac unmute" but the code doesn't clear the corresponding bits, thus when resuming, several bits are not cleared preventing the codec from working.
Sorry - I should've registered that this was one that should go through Liam's tree
Acked-by: Mark Brown broonie@opensource.wolfsonmicro.com
On Wed, 2010-06-23 at 00:17 +0100, Mark Brown wrote:
On Sat, Jun 19, 2010 at 07:33:39PM +0200, Eric Bénard wrote:
in tlv320aic23_set_bias_level, for the case SND_SOC_BIAS_ON, the comment says "vref/mid, osc on, dac unmute" but the code doesn't clear the corresponding bits, thus when resuming, several bits are not cleared preventing the codec from working.
Sorry - I should've registered that this was one that should go through Liam's tree
Acked-by: Mark Brown broonie@opensource.wolfsonmicro.com
Applied
Liam
participants (3)
-
Eric Bénard -
Liam Girdwood -
Mark Brown