[PATCH 0/7] ASoC: TWL4030: put the codec to off when not active

[PATCH 0/7] ASoC: TWL4030: put the codec to off when not active

[Peter Ujfalusi]

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(-)

[PATCH 1/7] ASoC: TWL4030: Revisit codec defaults

[Peter Ujfalusi]

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

[PATCH 2/7] ASoC: TWL4030: Remove wrapper for power down

[Peter Ujfalusi]

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

[PATCH 3/7] ASoC: TWL4030: Make offset cancellation path configurable

[Peter Ujfalusi]

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

Re: [PATCH 3/7] ASoC: TWL4030: Make offset cancellation path configurable

[Peter Ujfalusi]

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

[PATCH 4/7] ASoC: TWL4030: Optimize the power up sequence

[Peter Ujfalusi]

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

[PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers

[Peter Ujfalusi]

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

Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers

[Liam Girdwood]

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

Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers

[Peter Ujfalusi]

> 
> 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

Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers

[Liam Girdwood]

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 

Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers

[Peter Ujfalusi]

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

Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers

[Peter Ujfalusi]

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

Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers

[Mark Brown]

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.

Re: [PATCH 5/7] ASoC: TWL4030: Helper to check chip default registers

[Peter Ujfalusi]

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

[PATCH 6/7] ASoC: TWL4030: Correct the ARXR2_APGA_CTL chip default

[Peter Ujfalusi]

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

[PATCH 7/7] ASoC: TWL4030: Use BIAS_OFF instead of BIAS_STANDBY, when not in use

[Peter Ujfalusi]

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

Re: [PATCH 0/7] ASoC: TWL4030: put the codec to off when not active

[Mark Brown]

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.