* [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active
@ 2010-05-25 11:34 Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 1/7] ASoC: TWL4030: Revisit codec defaults Peter Ujfalusi
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 11:34 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, lrg
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(-)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/7] ASoC: TWL4030: Revisit codec defaults
2010-05-25 11:34 [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Peter Ujfalusi
@ 2010-05-25 11:34 ` Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 2/7] ASoC: TWL4030: Remove wrapper for power down Peter Ujfalusi
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 11:34 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, lrg
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) */
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/7] ASoC: TWL4030: Remove wrapper for power down
2010-05-25 11:34 [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 1/7] ASoC: TWL4030: Revisit codec defaults Peter Ujfalusi
@ 2010-05-25 11:34 ` Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 3/7] ASoC: TWL4030: Make offset cancellation path configurable Peter Ujfalusi
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 11:34 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, lrg
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);
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/7] ASoC: TWL4030: Make offset cancellation path configurable
2010-05-25 11:34 [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 1/7] ASoC: TWL4030: Revisit codec defaults Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 2/7] ASoC: TWL4030: Remove wrapper for power down Peter Ujfalusi
@ 2010-05-25 11:34 ` Peter Ujfalusi
2010-05-25 11:59 ` Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 4/7] ASoC: TWL4030: Optimize the power up sequence Peter Ujfalusi
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 11:34 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, lrg
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);
};
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/7] ASoC: TWL4030: Optimize the power up sequence
2010-05-25 11:34 [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Peter Ujfalusi
` (2 preceding siblings ...)
2010-05-25 11:34 ` [PATCH 3/7] ASoC: TWL4030: Make offset cancellation path configurable Peter Ujfalusi
@ 2010-05-25 11:34 ` Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers Peter Ujfalusi
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 11:34 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, lrg
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) {
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers
2010-05-25 11:34 [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Peter Ujfalusi
` (3 preceding siblings ...)
2010-05-25 11:34 ` [PATCH 4/7] ASoC: TWL4030: Optimize the power up sequence Peter Ujfalusi
@ 2010-05-25 11:34 ` Peter Ujfalusi
2010-05-25 11:57 ` Liam Girdwood
2010-05-25 11:34 ` [PATCH 6/7] ASoC: TWL4030: Correct the ARXR2_APGA_CTL chip default Peter Ujfalusi
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 11:34 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, lrg
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);
};
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/7] ASoC: TWL4030: Correct the ARXR2_APGA_CTL chip default
2010-05-25 11:34 [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Peter Ujfalusi
` (4 preceding siblings ...)
2010-05-25 11:34 ` [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers Peter Ujfalusi
@ 2010-05-25 11:34 ` Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 7/7] ASoC: TWL4030: Use BIAS_OFF instead of BIAS_STANDBY, when not in use Peter Ujfalusi
2010-05-25 17:35 ` [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Mark Brown
7 siblings, 0 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 11:34 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, lrg
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;
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 7/7] ASoC: TWL4030: Use BIAS_OFF instead of BIAS_STANDBY, when not in use
2010-05-25 11:34 [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Peter Ujfalusi
` (5 preceding siblings ...)
2010-05-25 11:34 ` [PATCH 6/7] ASoC: TWL4030: Correct the ARXR2_APGA_CTL chip default Peter Ujfalusi
@ 2010-05-25 11:34 ` Peter Ujfalusi
2010-05-25 17:35 ` [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Mark Brown
7 siblings, 0 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 11:34 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie, lrg
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);
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers
2010-05-25 11:34 ` [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers Peter Ujfalusi
@ 2010-05-25 11:57 ` Liam Girdwood
2010-05-25 12:20 ` Peter Ujfalusi
0 siblings, 1 reply; 17+ messages in thread
From: Liam Girdwood @ 2010-05-25 11:57 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: alsa-devel, broonie
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
--
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/7] ASoC: TWL4030: Make offset cancellation path configurable
2010-05-25 11:34 ` [PATCH 3/7] ASoC: TWL4030: Make offset cancellation path configurable Peter Ujfalusi
@ 2010-05-25 11:59 ` Peter Ujfalusi
0 siblings, 0 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 11:59 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie@opensource.wolfsonmicro.com, lrg@slimlogic.co.uk
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.
--
Péter
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers
2010-05-25 11:57 ` Liam Girdwood
@ 2010-05-25 12:20 ` Peter Ujfalusi
2010-05-25 13:09 ` Liam Girdwood
0 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-25 12:20 UTC (permalink / raw)
To: ext Liam Girdwood
Cc: alsa-devel@alsa-project.org, broonie@opensource.wolfsonmicro.com
>
> 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
--
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers
2010-05-25 12:20 ` Peter Ujfalusi
@ 2010-05-25 13:09 ` Liam Girdwood
2010-05-26 6:00 ` Peter Ujfalusi
0 siblings, 1 reply; 17+ messages in thread
From: Liam Girdwood @ 2010-05-25 13:09 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: alsa-devel@alsa-project.org, broonie@opensource.wolfsonmicro.com
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active
2010-05-25 11:34 [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Peter Ujfalusi
` (6 preceding siblings ...)
2010-05-25 11:34 ` [PATCH 7/7] ASoC: TWL4030: Use BIAS_OFF instead of BIAS_STANDBY, when not in use Peter Ujfalusi
@ 2010-05-25 17:35 ` Mark Brown
7 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2010-05-25 17:35 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: alsa-devel, lrg
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers
2010-05-25 13:09 ` Liam Girdwood
@ 2010-05-26 6:00 ` Peter Ujfalusi
2010-05-26 6:28 ` Peter Ujfalusi
0 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-26 6:00 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie@opensource.wolfsonmicro.com, ext Liam Girdwood
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
>
--
Péter
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers
2010-05-26 6:00 ` Peter Ujfalusi
@ 2010-05-26 6:28 ` Peter Ujfalusi
2010-05-26 6:35 ` Mark Brown
0 siblings, 1 reply; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-26 6:28 UTC (permalink / raw)
To: alsa-devel; +Cc: broonie@opensource.wolfsonmicro.com, ext Liam Girdwood
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
--
Péter
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers
2010-05-26 6:28 ` Peter Ujfalusi
@ 2010-05-26 6:35 ` Mark Brown
2010-05-26 7:15 ` Peter Ujfalusi
0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2010-05-26 6:35 UTC (permalink / raw)
To: Peter Ujfalusi; +Cc: alsa-devel, ext Liam Girdwood
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers
2010-05-26 6:35 ` Mark Brown
@ 2010-05-26 7:15 ` Peter Ujfalusi
0 siblings, 0 replies; 17+ messages in thread
From: Peter Ujfalusi @ 2010-05-26 7:15 UTC (permalink / raw)
To: alsa-devel; +Cc: ext Mark Brown, ext Liam Girdwood
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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2010-05-26 7:15 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-25 11:34 [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 1/7] ASoC: TWL4030: Revisit codec defaults Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 2/7] ASoC: TWL4030: Remove wrapper for power down Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 3/7] ASoC: TWL4030: Make offset cancellation path configurable Peter Ujfalusi
2010-05-25 11:59 ` Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 4/7] ASoC: TWL4030: Optimize the power up sequence Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers Peter Ujfalusi
2010-05-25 11:57 ` Liam Girdwood
2010-05-25 12:20 ` Peter Ujfalusi
2010-05-25 13:09 ` Liam Girdwood
2010-05-26 6:00 ` Peter Ujfalusi
2010-05-26 6:28 ` Peter Ujfalusi
2010-05-26 6:35 ` Mark Brown
2010-05-26 7:15 ` Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 6/7] ASoC: TWL4030: Correct the ARXR2_APGA_CTL chip default Peter Ujfalusi
2010-05-25 11:34 ` [PATCH 7/7] ASoC: TWL4030: Use BIAS_OFF instead of BIAS_STANDBY, when not in use Peter Ujfalusi
2010-05-25 17:35 ` [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).