[alsa-devel] [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active
Hello,
The following series at the end will let the twl4030 codec to use BIAS_OFF instead of BIAS_STANDBY. The difference in power consumption is about 0.5mA.
To achieve this: - The regcache has been reseted to codec default - The codec initialization has been optimized, it is no longer writes all 73 registers at startup, but only modifies few selected one. - The power related code has been cleaned up, and optimized - Support added for machine drivers to select the offset cancellation path - debug support for checking the codec default registers (machine drivers can ask for checking, but shall be disabled in production).
I guess that's it. The driver has been tested on a custom board with twl5031. It passed all of our internal test cases covering much of the codec features.
--- Peter Ujfalusi (7): ASoC: TWL4030: Revisit codec defaults ASoC: TWL4030: Remove wrapper for power down ASoC: TWL4030: Make offset cancellation path configurable ASoC: TWL4030: Optimize the power up sequence ASoC: TWL4030: Helper to check chip default registers ASoC: TWL4030: Correct the ARXR2_APGA_CTL chip default ASoC: TWL4030: Use BIAS_OFF instead of BIAS_STANDBY, when not in use
sound/soc/codecs/twl4030.c | 286 ++++++++++++++++++++++++-------------------- sound/soc/codecs/twl4030.h | 2 + 2 files changed, 157 insertions(+), 131 deletions(-)
Reset most of the codec registers to their chip reset value.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 44 ++++++++++++++++++++++---------------------- 1 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 6a34f56..9a3e999 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -42,7 +42,7 @@ */ static const u8 twl4030_reg[TWL4030_CACHEREGNUM] = { 0x00, /* this register not used */ - 0x91, /* REG_CODEC_MODE (0x1) */ + 0x00, /* REG_CODEC_MODE (0x1) */ 0xc3, /* REG_OPTION (0x2) */ 0x00, /* REG_UNKNOWN (0x3) */ 0x00, /* REG_MICBIAS_CTL (0x4) */ @@ -51,28 +51,28 @@ static const u8 twl4030_reg[TWL4030_CACHEREGNUM] = { 0x00, /* REG_AVADC_CTL (0x7) */ 0x00, /* REG_ADCMICSEL (0x8) */ 0x00, /* REG_DIGMIXING (0x9) */ - 0x0c, /* REG_ATXL1PGA (0xA) */ - 0x0c, /* REG_ATXR1PGA (0xB) */ - 0x00, /* REG_AVTXL2PGA (0xC) */ - 0x00, /* REG_AVTXR2PGA (0xD) */ + 0x0f, /* REG_ATXL1PGA (0xA) */ + 0x0f, /* REG_ATXR1PGA (0xB) */ + 0x0f, /* REG_AVTXL2PGA (0xC) */ + 0x0f, /* REG_AVTXR2PGA (0xD) */ 0x00, /* REG_AUDIO_IF (0xE) */ 0x00, /* REG_VOICE_IF (0xF) */ - 0x00, /* REG_ARXR1PGA (0x10) */ - 0x00, /* REG_ARXL1PGA (0x11) */ - 0x6c, /* REG_ARXR2PGA (0x12) */ - 0x6c, /* REG_ARXL2PGA (0x13) */ - 0x00, /* REG_VRXPGA (0x14) */ + 0x3f, /* REG_ARXR1PGA (0x10) */ + 0x3f, /* REG_ARXL1PGA (0x11) */ + 0x3f, /* REG_ARXR2PGA (0x12) */ + 0x3f, /* REG_ARXL2PGA (0x13) */ + 0x25, /* REG_VRXPGA (0x14) */ 0x00, /* REG_VSTPGA (0x15) */ 0x00, /* REG_VRX2ARXPGA (0x16) */ 0x00, /* REG_AVDAC_CTL (0x17) */ 0x00, /* REG_ARX2VTXPGA (0x18) */ - 0x00, /* REG_ARXL1_APGA_CTL (0x19) */ - 0x00, /* REG_ARXR1_APGA_CTL (0x1A) */ - 0x4a, /* REG_ARXL2_APGA_CTL (0x1B) */ - 0x4a, /* REG_ARXR2_APGA_CTL (0x1C) */ + 0x32, /* REG_ARXL1_APGA_CTL (0x19) */ + 0x32, /* REG_ARXR1_APGA_CTL (0x1A) */ + 0x32, /* REG_ARXL2_APGA_CTL (0x1B) */ + 0x32, /* REG_ARXR2_APGA_CTL (0x1C) */ 0x00, /* REG_ATX2ARXPGA (0x1D) */ 0x00, /* REG_BT_IF (0x1E) */ - 0x00, /* REG_BTPGA (0x1F) */ + 0x55, /* REG_BTPGA (0x1F) */ 0x00, /* REG_BTSTPGA (0x20) */ 0x00, /* REG_EAR_CTL (0x21) */ 0x00, /* REG_HS_SEL (0x22) */ @@ -84,32 +84,32 @@ static const u8 twl4030_reg[TWL4030_CACHEREGNUM] = { 0x00, /* REG_PRECKR_CTL (0x28) */ 0x00, /* REG_HFL_CTL (0x29) */ 0x00, /* REG_HFR_CTL (0x2A) */ - 0x00, /* REG_ALC_CTL (0x2B) */ + 0x05, /* REG_ALC_CTL (0x2B) */ 0x00, /* REG_ALC_SET1 (0x2C) */ 0x00, /* REG_ALC_SET2 (0x2D) */ 0x00, /* REG_BOOST_CTL (0x2E) */ 0x00, /* REG_SOFTVOL_CTL (0x2F) */ - 0x00, /* REG_DTMF_FREQSEL (0x30) */ + 0x13, /* REG_DTMF_FREQSEL (0x30) */ 0x00, /* REG_DTMF_TONEXT1H (0x31) */ 0x00, /* REG_DTMF_TONEXT1L (0x32) */ 0x00, /* REG_DTMF_TONEXT2H (0x33) */ 0x00, /* REG_DTMF_TONEXT2L (0x34) */ - 0x00, /* REG_DTMF_TONOFF (0x35) */ - 0x00, /* REG_DTMF_WANONOFF (0x36) */ + 0x79, /* REG_DTMF_TONOFF (0x35) */ + 0x11, /* REG_DTMF_WANONOFF (0x36) */ 0x00, /* REG_I2S_RX_SCRAMBLE_H (0x37) */ 0x00, /* REG_I2S_RX_SCRAMBLE_M (0x38) */ 0x00, /* REG_I2S_RX_SCRAMBLE_L (0x39) */ 0x06, /* REG_APLL_CTL (0x3A) */ 0x00, /* REG_DTMF_CTL (0x3B) */ - 0x00, /* REG_DTMF_PGA_CTL2 (0x3C) */ - 0x00, /* REG_DTMF_PGA_CTL1 (0x3D) */ + 0x44, /* REG_DTMF_PGA_CTL2 (0x3C) */ + 0x69, /* REG_DTMF_PGA_CTL1 (0x3D) */ 0x00, /* REG_MISC_SET_1 (0x3E) */ 0x00, /* REG_PCMBTMUX (0x3F) */ 0x00, /* not used (0x40) */ 0x00, /* not used (0x41) */ 0x00, /* not used (0x42) */ 0x00, /* REG_RX_PATH_SEL (0x43) */ - 0x00, /* REG_VDL_APGA_CTL (0x44) */ + 0x32, /* REG_VDL_APGA_CTL (0x44) */ 0x00, /* REG_VIBRA_CTL (0x45) */ 0x00, /* REG_VIBRA_SET (0x46) */ 0x00, /* REG_VIBRA_PWM_SET (0x47) */
There is no need for the power down wrapper.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 13 ++----------- 1 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 9a3e999..1e0aba5 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -319,15 +319,6 @@ static void twl4030_power_up(struct snd_soc_codec *codec) twl4030_codec_enable(codec, 1); }
-/* - * Unconditional power down - */ -static void twl4030_power_down(struct snd_soc_codec *codec) -{ - /* power down */ - twl4030_codec_enable(codec, 0); -} - /* Earpiece */ static const struct snd_kcontrol_new twl4030_dapm_earpiece_controls[] = { SOC_DAPM_SINGLE("Voice", TWL4030_REG_EAR_CTL, 0, 1, 0), @@ -1607,7 +1598,7 @@ static int twl4030_set_bias_level(struct snd_soc_codec *codec, twl4030_power_up(codec); break; case SND_SOC_BIAS_OFF: - twl4030_power_down(codec); + twl4030_codec_enable(codec, 0); break; } codec->bias_level = level; @@ -2321,7 +2312,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev) return 0;
error_codec: - twl4030_power_down(codec); + twl4030_codec_enable(codec, 0); kfree(codec->reg_cache); error_cache: kfree(twl4030);
Add means for machine drivers to select the path for offset cancellation. Reset the reg cache value to the chip reset value at the same time.
Machine drivers can specify which path need to be used for offset cancellation via the twl4030_setup.offset_cncl_path. For paths use the defines from include/linux/mfd/twl4030-codec.h: TWL4030_OFFSET_CNCL_SEL_*
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 6 +++++- sound/soc/codecs/twl4030.h | 1 + 2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 1e0aba5..2855771 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -46,7 +46,7 @@ static const u8 twl4030_reg[TWL4030_CACHEREGNUM] = { 0xc3, /* REG_OPTION (0x2) */ 0x00, /* REG_UNKNOWN (0x3) */ 0x00, /* REG_MICBIAS_CTL (0x4) */ - 0x20, /* REG_ANAMICL (0x5) */ + 0x00, /* REG_ANAMICL (0x5) */ 0x00, /* REG_ANAMICR (0x6) */ 0x00, /* REG_AVADC_CTL (0x7) */ 0x00, /* REG_ADCMICSEL (0x8) */ @@ -281,6 +281,8 @@ static void twl4030_apll_enable(struct snd_soc_codec *codec, int enable)
static void twl4030_power_up(struct snd_soc_codec *codec) { + struct snd_soc_device *socdev = codec->socdev; + struct twl4030_setup_data *setup = socdev->codec_data; struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); u8 anamicl, regmisc1, byte; int i = 0; @@ -293,6 +295,8 @@ static void twl4030_power_up(struct snd_soc_codec *codec)
/* initiate offset cancellation */ anamicl = twl4030_read_reg_cache(codec, TWL4030_REG_ANAMICL); + anamicl &= ~TWL4030_OFFSET_CNCL_SEL; + anamicl |= (setup->offset_cncl_path << 5); twl4030_write(codec, TWL4030_REG_ANAMICL, anamicl | TWL4030_CNCL_OFFSET_START);
diff --git a/sound/soc/codecs/twl4030.h b/sound/soc/codecs/twl4030.h index f206d24..c98e303 100644 --- a/sound/soc/codecs/twl4030.h +++ b/sound/soc/codecs/twl4030.h @@ -42,6 +42,7 @@ extern struct snd_soc_codec_device soc_codec_dev_twl4030; struct twl4030_setup_data { unsigned int ramp_delay_value; unsigned int sysclk; + unsigned int offset_cncl_path; unsigned int hs_extmute:1; void (*set_hs_extmute)(int mute); };
Hi,
resending, because:
On Tuesday 25 May 2010 14:34:04 Ujfalusi Peter (Nokia-D/Tampere) wrote:
For paths use the defines from include/linux/mfd/twl4030-codec.h: TWL4030_OFFSET_CNCL_SEL_*
...
/* initiate offset cancellation */ anamicl = twl4030_read_reg_cache(codec, TWL4030_REG_ANAMICL);
- anamicl &= ~TWL4030_OFFSET_CNCL_SEL;
- anamicl |= (setup->offset_cncl_path << 5);
No need for the shift, it seams to be left from some older test code.
Since the reg cache now contains the chip default values for all registers (REG_OPTION is reset to it's default within this patch), there is no longer need to rewrite _all_ registers. Initialize only few selected registers.
According to the latest information, the offset cancellation need to be done only once, when the codec is powered on, so there is no need for the power up wrapper.
Move all chip initialization code under chip_init, and do it when the codec is initialized.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 133 ++++++++++++++++++++------------------------ 1 files changed, 60 insertions(+), 73 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 2855771..08f24de 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -43,7 +43,7 @@ static const u8 twl4030_reg[TWL4030_CACHEREGNUM] = { 0x00, /* this register not used */ 0x00, /* REG_CODEC_MODE (0x1) */ - 0xc3, /* REG_OPTION (0x2) */ + 0x00, /* REG_OPTION (0x2) */ 0x00, /* REG_UNKNOWN (0x3) */ 0x00, /* REG_MICBIAS_CTL (0x4) */ 0x00, /* REG_ANAMICL (0x5) */ @@ -243,62 +243,52 @@ static void twl4030_codec_enable(struct snd_soc_codec *codec, int enable) udelay(10); }
-static void twl4030_init_chip(struct snd_soc_codec *codec) -{ - u8 *cache = codec->reg_cache; - int i; - - /* clear CODECPDZ prior to setting register defaults */ - twl4030_codec_enable(codec, 0); - - /* set all audio section registers to reasonable defaults */ - for (i = TWL4030_REG_OPTION; i <= TWL4030_REG_MISC_SET_2; i++) - if (i != TWL4030_REG_APLL_CTL) - twl4030_write(codec, i, cache[i]); - -} - -static void twl4030_apll_enable(struct snd_soc_codec *codec, int enable) +static void twl4030_init_chip(struct platform_device *pdev) { + struct snd_soc_device *socdev = platform_get_drvdata(pdev); + struct twl4030_setup_data *setup = socdev->codec_data; + struct snd_soc_codec *codec = socdev->card->codec; struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); - int status = -1; + u8 reg, byte; + int i = 0;
- if (enable) { - twl4030->apll_enabled++; - if (twl4030->apll_enabled == 1) - status = twl4030_codec_enable_resource( - TWL4030_CODEC_RES_APLL); - } else { - twl4030->apll_enabled--; - if (!twl4030->apll_enabled) - status = twl4030_codec_disable_resource( - TWL4030_CODEC_RES_APLL); - } + /* Refresh APLL_CTL register from HW */ + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &byte, + TWL4030_REG_APLL_CTL); + twl4030_write_reg_cache(codec, TWL4030_REG_APLL_CTL, byte);
- if (status >= 0) - twl4030_write_reg_cache(codec, TWL4030_REG_APLL_CTL, status); -} + /* anti-pop when changing analog gain */ + reg = twl4030_read_reg_cache(codec, TWL4030_REG_MISC_SET_1); + twl4030_write(codec, TWL4030_REG_MISC_SET_1, + reg | TWL4030_SMOOTH_ANAVOL_EN);
-static void twl4030_power_up(struct snd_soc_codec *codec) -{ - struct snd_soc_device *socdev = codec->socdev; - struct twl4030_setup_data *setup = socdev->codec_data; - struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); - u8 anamicl, regmisc1, byte; - int i = 0; + twl4030_write(codec, TWL4030_REG_OPTION, + TWL4030_ATXL1_EN | TWL4030_ATXR1_EN | + TWL4030_ARXL2_EN | TWL4030_ARXR2_EN);
- if (twl4030->codec_powered) + /* Machine dependent setup */ + if (!setup) return;
- /* set CODECPDZ to turn on codec */ - twl4030_codec_enable(codec, 1); + /* Configuration for headset ramp delay from setup data */ + if (setup->sysclk != twl4030->sysclk) + dev_warn(codec->dev, + "Mismatch in APLL mclk: %u (configured: %u)\n", + setup->sysclk, twl4030->sysclk); + + reg = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET); + reg &= ~TWL4030_RAMP_DELAY; + reg |= (setup->ramp_delay_value << 2); + twl4030_write_reg_cache(codec, TWL4030_REG_HS_POPN_SET, reg);
/* initiate offset cancellation */ - anamicl = twl4030_read_reg_cache(codec, TWL4030_REG_ANAMICL); - anamicl &= ~TWL4030_OFFSET_CNCL_SEL; - anamicl |= (setup->offset_cncl_path << 5); + twl4030_codec_enable(codec, 1); + + reg = twl4030_read_reg_cache(codec, TWL4030_REG_ANAMICL); + reg &= ~TWL4030_OFFSET_CNCL_SEL; + reg |= setup->offset_cncl_path; twl4030_write(codec, TWL4030_REG_ANAMICL, - anamicl | TWL4030_CNCL_OFFSET_START); + reg | TWL4030_CNCL_OFFSET_START);
/* wait for offset cancellation to complete */ do { @@ -313,14 +303,28 @@ static void twl4030_power_up(struct snd_soc_codec *codec) /* Make sure that the reg_cache has the same value as the HW */ twl4030_write_reg_cache(codec, TWL4030_REG_ANAMICL, byte);
- /* anti-pop when changing analog gain */ - regmisc1 = twl4030_read_reg_cache(codec, TWL4030_REG_MISC_SET_1); - twl4030_write(codec, TWL4030_REG_MISC_SET_1, - regmisc1 | TWL4030_SMOOTH_ANAVOL_EN); - - /* toggle CODECPDZ as per TRM */ twl4030_codec_enable(codec, 0); - twl4030_codec_enable(codec, 1); +} + +static void twl4030_apll_enable(struct snd_soc_codec *codec, int enable) +{ + struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); + int status = -1; + + if (enable) { + twl4030->apll_enabled++; + if (twl4030->apll_enabled == 1) + status = twl4030_codec_enable_resource( + TWL4030_CODEC_RES_APLL); + } else { + twl4030->apll_enabled--; + if (!twl4030->apll_enabled) + status = twl4030_codec_disable_resource( + TWL4030_CODEC_RES_APLL); + } + + if (status >= 0) + twl4030_write_reg_cache(codec, TWL4030_REG_APLL_CTL, status); }
/* Earpiece */ @@ -1599,7 +1603,7 @@ static int twl4030_set_bias_level(struct snd_soc_codec *codec, break; case SND_SOC_BIAS_STANDBY: if (codec->bias_level == SND_SOC_BIAS_OFF) - twl4030_power_up(codec); + twl4030_codec_enable(codec, 1); break; case SND_SOC_BIAS_OFF: twl4030_codec_enable(codec, 0); @@ -2196,31 +2200,16 @@ static struct snd_soc_codec *twl4030_codec; static int twl4030_soc_probe(struct platform_device *pdev) { struct snd_soc_device *socdev = platform_get_drvdata(pdev); - struct twl4030_setup_data *setup = socdev->codec_data; struct snd_soc_codec *codec; - struct twl4030_priv *twl4030; int ret;
BUG_ON(!twl4030_codec);
codec = twl4030_codec; - twl4030 = snd_soc_codec_get_drvdata(codec); socdev->card->codec = codec;
- /* Configuration for headset ramp delay from setup data */ - if (setup) { - unsigned char hs_pop; - - if (setup->sysclk != twl4030->sysclk) - dev_warn(&pdev->dev, - "Mismatch in APLL mclk: %u (configured: %u)\n", - setup->sysclk, twl4030->sysclk); - - hs_pop = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET); - hs_pop &= ~TWL4030_RAMP_DELAY; - hs_pop |= (setup->ramp_delay_value << 2); - twl4030_write_reg_cache(codec, TWL4030_REG_HS_POPN_SET, hs_pop); - } + twl4030_init_chip(pdev); + twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
/* register pcms */ ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1); @@ -2296,9 +2285,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev)
/* Set the defaults, and power up the codec */ twl4030->sysclk = twl4030_codec_get_mclk() / 1000; - twl4030_init_chip(codec); codec->bias_level = SND_SOC_BIAS_OFF; - twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
ret = snd_soc_register_codec(codec); if (ret != 0) {
Since the twl4030 codec driver supports different version of the PM chip, a helper function can come handy, which can check the driver's default versus the chip values.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 23 +++++++++++++++++++++++ sound/soc/codecs/twl4030.h | 1 + 2 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 08f24de..4220c8d 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -243,6 +243,25 @@ static void twl4030_codec_enable(struct snd_soc_codec *codec, int enable) udelay(10); }
+static inline void twl4030_check_defaults(struct snd_soc_codec *codec) +{ + int i, difference = 0; + u8 val; + + dev_info(codec->dev, "Checking TWL audio default configuration\n"); + for (i = 1; i <= TWL4030_REG_MISC_SET_2; i++) { + twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &val, i); + if (val != twl4030_reg[i]) { + difference++; + dev_info(codec->dev, + "Reg 0x%02x: chip: 0x%02x driver: 0x%02x\n", + i, val, twl4030_reg[i]); + } + } + dev_info(codec->dev, "Found %d non maching registers. %s\n", + difference, difference ? "Not OK" : "OK"); +} + static void twl4030_init_chip(struct platform_device *pdev) { struct snd_soc_device *socdev = platform_get_drvdata(pdev); @@ -252,6 +271,10 @@ static void twl4030_init_chip(struct platform_device *pdev) u8 reg, byte; int i = 0;
+ /* Check defaults, if instructed before anythiing else */ + if (setup && setup->check_defaults) + twl4030_check_defaults(codec); + /* Refresh APLL_CTL register from HW */ twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &byte, TWL4030_REG_APLL_CTL); diff --git a/sound/soc/codecs/twl4030.h b/sound/soc/codecs/twl4030.h index c98e303..c22542c 100644 --- a/sound/soc/codecs/twl4030.h +++ b/sound/soc/codecs/twl4030.h @@ -43,6 +43,7 @@ struct twl4030_setup_data { unsigned int ramp_delay_value; unsigned int sysclk; unsigned int offset_cncl_path; + unsigned int check_defaults:1; unsigned int hs_extmute:1; void (*set_hs_extmute)(int mute); };
On Tue, 2010-05-25 at 14:34 +0300, Peter Ujfalusi wrote:
Since the twl4030 codec driver supports different version of the PM chip, a helper function can come handy, which can check the driver's default versus the chip values.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
sound/soc/codecs/twl4030.c | 23 +++++++++++++++++++++++ sound/soc/codecs/twl4030.h | 1 + 2 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 08f24de..4220c8d 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -243,6 +243,25 @@ static void twl4030_codec_enable(struct snd_soc_codec *codec, int enable) udelay(10); }
+static inline void twl4030_check_defaults(struct snd_soc_codec *codec) +{
- int i, difference = 0;
- u8 val;
- dev_info(codec->dev, "Checking TWL audio default configuration\n");
- for (i = 1; i <= TWL4030_REG_MISC_SET_2; i++) {
twl4030_i2c_read_u8(TWL4030_MODULE_AUDIO_VOICE, &val, i);if (val != twl4030_reg[i]) {difference++;dev_info(codec->dev,"Reg 0x%02x: chip: 0x%02x driver: 0x%02x\n",i, val, twl4030_reg[i]);}- }
- dev_info(codec->dev, "Found %d non maching registers. %s\n",
matching
difference, difference ? "Not OK" : "OK");+}
static void twl4030_init_chip(struct platform_device *pdev) { struct snd_soc_device *socdev = platform_get_drvdata(pdev); @@ -252,6 +271,10 @@ static void twl4030_init_chip(struct platform_device *pdev) u8 reg, byte; int i = 0;
- /* Check defaults, if instructed before anythiing else */
- if (setup && setup->check_defaults)
twl4030_check_defaults(codec);
Is this purely for information/debug purposes ?
Why do we need to check default vales at init(). Is there another driver changing the audio codec registers ?
Thanks
Liam
Is this purely for information/debug purposes ?
Yes, the driver support twl4030, twl5030, twl5031, tps*something* chips. If someone, who have access to those chips, and in doubt, can check it.
Why do we need to check default vales at init(). Is there another driver changing the audio codec registers ?
This driver is going to change it. It is also possible that the bootloader changed them (startup tone?). So it is not really bulletproof, but at least it helped me to find out the the ARXR2_APGA_CTL register does not have the reset value, which it supposed to have.
Well, I can remove it, but I thought that it is a nice touch ;)
Thanks
Liam
On Tue, 2010-05-25 at 15:20 +0300, Peter Ujfalusi wrote:
Is this purely for information/debug purposes ?
Yes, the driver support twl4030, twl5030, twl5031, tps*something* chips. If someone, who have access to those chips, and in doubt, can check it.
So it's more a debug aid.
Why do we need to check default vales at init(). Is there another driver changing the audio codec registers ?
This driver is going to change it. It is also possible that the bootloader changed them (startup tone?).
Yeah, that's what I thought. In the past I've always forced the CODEC registers to their default values during probe(). Very handy if the driver is a module and you are recovering from a buggy state.
So it is not really bulletproof, but at least it helped me to find out the the ARXR2_APGA_CTL register does not have the reset value, which it supposed to have.
Well, I can remove it, but I thought that it is a nice touch ;)
It's nice ;) But maybe we should reset the default values at probe() to be sure.
Thanks
Liam
Hi,
You and Mark has a valid point...
On Tuesday 25 May 2010 16:09:56 ext Liam Girdwood wrote:
On Tue, 2010-05-25 at 15:20 +0300, Peter Ujfalusi wrote:
Is this purely for information/debug purposes ?
Yes, the driver support twl4030, twl5030, twl5031, tps*something* chips. If someone, who have access to those chips, and in doubt, can check it.
So it's more a debug aid.
I'll change the dev_info to dev_dbg to reflect that correctly.
Why do we need to check default vales at init(). Is there another driver changing the audio codec registers ?
This driver is going to change it. It is also possible that the bootloader changed them (startup tone?).
Yeah, that's what I thought. In the past I've always forced the CODEC registers to their default values during probe(). Very handy if the driver is a module and you are recovering from a buggy state.
Yeah, during development this is really handy.
So it is not really bulletproof, but at least it helped me to find out the the ARXR2_APGA_CTL register does not have the reset value, which it supposed to have.
Well, I can remove it, but I thought that it is a nice touch ;)
It's nice ;) But maybe we should reset the default values at probe() to be sure.
I did run some tests. The codec registers are in reset state whenever the device boots (either power on, or reboot). In our setup the codec is built in the kernel. I have measured the time needed to execute the twl4030_init_chip with and without rewriting the codec registers (71 register writes): No reset_registers: ~51ms reset registers: ~71ms
I need to optimize for module loading time, and ~20ms extra is quite big.
Can we make a compromise? I propose to have twl4030_setup_data.reset_registers, if it is set by the machine driver, than we are going to reset the registers, if it is not set, than we skip the restore part (not writing the 71 registers). So during development, or if one have the codec as module, the machine can set this, so the registers will be restored, but if the testing shows that there is no need to do that, than we can speed up the module probe.
What do you think?
Thanks
Liam
On Wednesday 26 May 2010 09:00:35 Ujfalusi Peter (Nokia-D/Tampere) wrote:
...
I did run some tests. The codec registers are in reset state whenever the device boots (either power on, or reboot). In our setup the codec is built in the kernel. I have measured the time needed to execute the twl4030_init_chip with and without rewriting the codec registers (71 register writes): No reset_registers: ~51ms reset registers: ~71ms
I need to optimize for module loading time, and ~20ms extra is quite big.
Can we make a compromise? I propose to have twl4030_setup_data.reset_registers, if it is set by the machine driver, than we are going to reset the registers, if it is not set, than we skip the restore part (not writing the 71 registers). So during development, or if one have the codec as module, the machine can set this, so the registers will be restored, but if the testing shows that there is no need to do that, than we can speed up the module probe.
What do you think?
Better thing to do is: restore the registers in these cases: if (!setup || (setup && setup->reset_registers))
So if the machine does not provide setup data, than we can assume, than no one taken a time to tune the platform, so we need to restore to be on the safe side.
What do you think?
Thanks
Liam
On Wed, May 26, 2010 at 09:28:49AM +0300, Peter Ujfalusi wrote:
Better thing to do is: restore the registers in these cases: if (!setup || (setup && setup->reset_registers))
So if the machine does not provide setup data, than we can assume, than no one taken a time to tune the platform, so we need to restore to be on the safe side.
What do you think?
This sounds like a sensible optimisation. May also be an idea to explicitly do a reset of the registers on unload - I guess nobody will ever do that if they're not developing, and that'd probably mean that many of the people who might actually need to reset the registers purely for development will get the benefit of it without the risk of leaving the option on in production by mistake.
I can't remember if you're doing this already (and don't even know if it's possible with the hardware) but if you can do a bulk read of the registers rather than reading register by register then the overhead from the I/O should be noticably reduced if you do do the reset.
On Wednesday 26 May 2010 09:35:00 ext Mark Brown wrote:
On Wed, May 26, 2010 at 09:28:49AM +0300, Peter Ujfalusi wrote:
Better thing to do is: restore the registers in these cases: if (!setup || (setup && setup->reset_registers))
So if the machine does not provide setup data, than we can assume, than no one taken a time to tune the platform, so we need to restore to be on the safe side.
What do you think?
This sounds like a sensible optimisation. May also be an idea to explicitly do a reset of the registers on unload - I guess nobody will ever do that if they're not developing, and that'd probably mean that many of the people who might actually need to reset the registers purely for development will get the benefit of it without the risk of leaving the option on in production by mistake.
I can add the restore also to the unload path, good idea.
I can't remember if you're doing this already (and don't even know if it's possible with the hardware) but if you can do a bulk read of the registers rather than reading register by register then the overhead from the I/O should be noticably reduced if you do do the reset.
No I'm not doing that, but at least the twl core (and HW) supports burst write, and burst read as well. To add the burst restore support needs a bit bigger change here, and there in the codec driver from the first look.
For now, I'm planning to put back the 'old' for (...) style of register restore. Is it OK if I add the burst support later, not in this series?
Also is it OK, if I add the restore support as a new patch in the series (so the restore will be gone for 3 commits)?
Thanks, Péter
It seams at least on twl5031 that the ARXR2_APGA_CTL register does not have the same default value as it is written in the TRM. Since the codec part of the PM chip has not been actually changed according to TI, assuming, that all version has the same problem, so writing there the TRM value.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 4220c8d..72a19ce 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -289,6 +289,9 @@ static void twl4030_init_chip(struct platform_device *pdev) TWL4030_ATXL1_EN | TWL4030_ATXR1_EN | TWL4030_ARXL2_EN | TWL4030_ARXR2_EN);
+ /* REG_ARXR2_APGA_CTL reset according to the TRM: 0dB, DA_EN */ + twl4030_write(codec, TWL4030_REG_ARXR2_APGA_CTL, 0x32); + /* Machine dependent setup */ if (!setup) return;
Restructure the codec power code in order to be able to hit off when the codec is not in use.
Since the audio registers are accessible while the codec is powered down, there is no need for additional safety mechanism.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/twl4030.c | 86 ++++++++++++++++++++++++++------------------ 1 files changed, 51 insertions(+), 35 deletions(-)
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index 72a19ce..79d1cf9 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -1818,13 +1818,6 @@ static int twl4030_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- if (mode != old_mode) { - /* change rate and set CODECPDZ */ - twl4030_codec_enable(codec, 0); - twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode); - twl4030_codec_enable(codec, 1); - } - /* sample size */ old_format = twl4030_read_reg_cache(codec, TWL4030_REG_AUDIO_IF); format = old_format; @@ -1842,16 +1835,20 @@ static int twl4030_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- if (format != old_format) { - - /* clear CODECPDZ before changing format (codec requirement) */ - twl4030_codec_enable(codec, 0); - - /* change format */ - twl4030_write(codec, TWL4030_REG_AUDIO_IF, format); - - /* set CODECPDZ afterwards */ - twl4030_codec_enable(codec, 1); + if (format != old_format || mode != old_mode) { + if (twl4030->codec_powered) { + /* + * If the codec is powered, than we need to toggle the + * codec power. + */ + twl4030_codec_enable(codec, 0); + twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode); + twl4030_write(codec, TWL4030_REG_AUDIO_IF, format); + twl4030_codec_enable(codec, 1); + } else { + twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode); + twl4030_write(codec, TWL4030_REG_AUDIO_IF, format); + } }
/* Store the important parameters for the DAI configuration and set @@ -1901,6 +1898,7 @@ static int twl4030_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) { struct snd_soc_codec *codec = codec_dai->codec; + struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); u8 old_format, format;
/* get format */ @@ -1935,15 +1933,17 @@ static int twl4030_set_dai_fmt(struct snd_soc_dai *codec_dai, }
if (format != old_format) { - - /* clear CODECPDZ before changing format (codec requirement) */ - twl4030_codec_enable(codec, 0); - - /* change format */ - twl4030_write(codec, TWL4030_REG_AUDIO_IF, format); - - /* set CODECPDZ afterwards */ - twl4030_codec_enable(codec, 1); + if (twl4030->codec_powered) { + /* + * If the codec is powered, than we need to toggle the + * codec power. + */ + twl4030_codec_enable(codec, 0); + twl4030_write(codec, TWL4030_REG_AUDIO_IF, format); + twl4030_codec_enable(codec, 1); + } else { + twl4030_write(codec, TWL4030_REG_AUDIO_IF, format); + } }
return 0; @@ -2035,6 +2035,7 @@ static int twl4030_voice_hw_params(struct snd_pcm_substream *substream, struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_device *socdev = rtd->socdev; struct snd_soc_codec *codec = socdev->card->codec; + struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); u8 old_mode, mode;
/* Enable voice digital filters */ @@ -2059,10 +2060,17 @@ static int twl4030_voice_hw_params(struct snd_pcm_substream *substream, }
if (mode != old_mode) { - /* change rate and set CODECPDZ */ - twl4030_codec_enable(codec, 0); - twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode); - twl4030_codec_enable(codec, 1); + if (twl4030->codec_powered) { + /* + * If the codec is powered, than we need to toggle the + * codec power. + */ + twl4030_codec_enable(codec, 0); + twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode); + twl4030_codec_enable(codec, 1); + } else { + twl4030_write(codec, TWL4030_REG_CODEC_MODE, mode); + } }
return 0; @@ -2092,6 +2100,7 @@ static int twl4030_voice_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) { struct snd_soc_codec *codec = codec_dai->codec; + struct twl4030_priv *twl4030 = snd_soc_codec_get_drvdata(codec); u8 old_format, format;
/* get format */ @@ -2123,10 +2132,17 @@ static int twl4030_voice_set_dai_fmt(struct snd_soc_dai *codec_dai, }
if (format != old_format) { - /* change format and set CODECPDZ */ - twl4030_codec_enable(codec, 0); - twl4030_write(codec, TWL4030_REG_VOICE_IF, format); - twl4030_codec_enable(codec, 1); + if (twl4030->codec_powered) { + /* + * If the codec is powered, than we need to toggle the + * codec power. + */ + twl4030_codec_enable(codec, 0); + twl4030_write(codec, TWL4030_REG_VOICE_IF, format); + twl4030_codec_enable(codec, 1); + } else { + twl4030_write(codec, TWL4030_REG_VOICE_IF, format); + } }
return 0; @@ -2235,7 +2251,6 @@ static int twl4030_soc_probe(struct platform_device *pdev) socdev->card->codec = codec;
twl4030_init_chip(pdev); - twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
/* register pcms */ ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1); @@ -2296,6 +2311,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev) codec->read = twl4030_read_reg_cache; codec->write = twl4030_write; codec->set_bias_level = twl4030_set_bias_level; + codec->idle_bias_off = 1; codec->dai = twl4030_dai; codec->num_dai = ARRAY_SIZE(twl4030_dai); codec->reg_cache_size = sizeof(twl4030_reg);
On Tue, May 25, 2010 at 02:34:01PM +0300, Peter Ujfalusi wrote:
The following series at the end will let the twl4030 codec to use BIAS_OFF instead of BIAS_STANDBY.
All
Acked-by: Mark Brown broonie@opensource.wolfsonmicro.com
but I do agree with Liam's comment about forcing the register values into the expected state rather than just warning at startup.
The difference in power consumption is about 0.5mA.
Nice.
participants (3)
-
Liam Girdwood -
Mark Brown -
Peter Ujfalusi