[PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Peter Ujfalusi]

Hello,

The following series aims to fix the Headset power on/off pop noise problems.

Since the twl4030 codec internal implementation have this order in HW:
DAC -> Analog PGA -> Mixer

While the ASoC framework expects the following order:
DAC -> Mixer -> Analog PGA

I needed to move the APGA control from SND_SOC_DAPM_PGA to be handled with
_MIXER. Than adding a PGA to the headset outputs results correct power
sequences.

This patch also separates the analog and digital loopback paths, so when the
analog loopback is enabled none of the DACs will be powered on, while in case
of digital loopback the corresponding DACs will be powered.

These are internal changes, not visible for the user. I have tested it with my
custom board with the headset soldered to HeadsetL, the pop noise is really small
if any.

The user must also set the correct ramp delay for the given board.
On my board:
MCLK = 38.4MHz
RAMP_DELAY = 2 (109/81/55 ms) works best (meaning 55ms ramp
delay), if it is shorter, than it clicks, if it is longer, than it does not
click, but the beep tone can be observed.

I hope that I have not missed anything with the patches, since these are forward
ported from my working tree to sound-2.6:topic/asoc.

I have applied the patch from Misael on top of the current topic/asoc, so the
head was:
ASoC: TWL4030: Enable/disable voice digital filters

I believe that this should be able to minimize the pop noise on the headset
outputs.

---
Peter Ujfalusi (2):
  ASoC: TWL4030: Change DAPM routings and controls for DACs and PGAs
  ASoC: TWL4030: Move the Headset pop-attenuation code to PGA event

 sound/soc/codecs/twl4030.c |  257 ++++++++++++++++++++++++++++----------------
 1 files changed, 162 insertions(+), 95 deletions(-)

[PATCH 1/2] ASoC: TWL4030: Change DAPM routings and controls for DACs and PGAs

[Peter Ujfalusi]

Restructuring the twl4030 codec's DAPM routing to be able to handle the power
sequences correctly.

The twl4030 codec internal implementation have this order:
DAC -> Analog PGA -> Mixer/Mux

While the ASoC framework expects the following order:
DAC -> Mixer -> Analog PGA

This patch moves the Analog PGA handling from SND_SOC_DAPM_PGA to _MIXER and
adds two levels of mixer to handle the digital and analog loopback
functionality.

Now the analog loopback does not powers on any of the DACs.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/codecs/twl4030.c |  141 ++++++++++++++++++++++----------------------
 1 files changed, 71 insertions(+), 70 deletions(-)

diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
index e4d683d..990aa4a 100644
--- a/sound/soc/codecs/twl4030.c
+++ b/sound/soc/codecs/twl4030.c
@@ -1004,18 +1004,6 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = {
 	SND_SOC_DAPM_DAC("DAC Voice", "Voice Playback",
 			SND_SOC_NOPM, 0, 0),
 
-	/* Analog PGAs */
-	SND_SOC_DAPM_PGA("ARXR1_APGA", TWL4030_REG_ARXR1_APGA_CTL,
-			0, 0, NULL, 0),
-	SND_SOC_DAPM_PGA("ARXL1_APGA", TWL4030_REG_ARXL1_APGA_CTL,
-			0, 0, NULL, 0),
-	SND_SOC_DAPM_PGA("ARXR2_APGA", TWL4030_REG_ARXR2_APGA_CTL,
-			0, 0, NULL, 0),
-	SND_SOC_DAPM_PGA("ARXL2_APGA", TWL4030_REG_ARXL2_APGA_CTL,
-			0, 0, NULL, 0),
-	SND_SOC_DAPM_PGA("VDL_APGA", TWL4030_REG_VDL_APGA_CTL,
-			0, 0, NULL, 0),
-
 	/* Analog bypasses */
 	SND_SOC_DAPM_SWITCH_E("Right1 Analog Loopback", SND_SOC_NOPM, 0, 0,
 			&twl4030_dapm_abypassr1_control, bypass_event,
@@ -1044,16 +1032,29 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = {
 			&twl4030_dapm_dbypassv_control, bypass_event,
 			SND_SOC_DAPM_POST_REG),
 
-	SND_SOC_DAPM_MIXER("Analog R1 Playback Mixer", TWL4030_REG_AVDAC_CTL,
-			0, 0, NULL, 0),
-	SND_SOC_DAPM_MIXER("Analog L1 Playback Mixer", TWL4030_REG_AVDAC_CTL,
-			1, 0, NULL, 0),
-	SND_SOC_DAPM_MIXER("Analog R2 Playback Mixer", TWL4030_REG_AVDAC_CTL,
-			2, 0, NULL, 0),
-	SND_SOC_DAPM_MIXER("Analog L2 Playback Mixer", TWL4030_REG_AVDAC_CTL,
-			3, 0, NULL, 0),
-	SND_SOC_DAPM_MIXER("Analog Voice Playback Mixer", TWL4030_REG_AVDAC_CTL,
-			4, 0, NULL, 0),
+	/* Digital mixers, power control for the physical DACs */
+	SND_SOC_DAPM_MIXER("Digital R1 Playback Mixer",
+			TWL4030_REG_AVDAC_CTL, 0, 0, NULL, 0),
+	SND_SOC_DAPM_MIXER("Digital L1 Playback Mixer",
+			TWL4030_REG_AVDAC_CTL, 1, 0, NULL, 0),
+	SND_SOC_DAPM_MIXER("Digital R2 Playback Mixer",
+			TWL4030_REG_AVDAC_CTL, 2, 0, NULL, 0),
+	SND_SOC_DAPM_MIXER("Digital L2 Playback Mixer",
+			TWL4030_REG_AVDAC_CTL, 3, 0, NULL, 0),
+	SND_SOC_DAPM_MIXER("Digital Voice Playback Mixer",
+			TWL4030_REG_AVDAC_CTL, 4, 0, NULL, 0),
+
+	/* Analog mixers, power control for the physical PGAs */
+	SND_SOC_DAPM_MIXER("Analog R1 Playback Mixer",
+			TWL4030_REG_ARXR1_APGA_CTL, 0, 0, NULL, 0),
+	SND_SOC_DAPM_MIXER("Analog L1 Playback Mixer",
+			TWL4030_REG_ARXL1_APGA_CTL, 0, 0, NULL, 0),
+	SND_SOC_DAPM_MIXER("Analog R2 Playback Mixer",
+			TWL4030_REG_ARXR2_APGA_CTL, 0, 0, NULL, 0),
+	SND_SOC_DAPM_MIXER("Analog L2 Playback Mixer",
+			TWL4030_REG_ARXL2_APGA_CTL, 0, 0, NULL, 0),
+	SND_SOC_DAPM_MIXER("Analog Voice Playback Mixer",
+			TWL4030_REG_VDL_APGA_CTL, 0, 0, NULL, 0),
 
 	/* Output MIXER controls */
 	/* Earpiece */
@@ -1147,60 +1148,60 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = {
 };
 
 static const struct snd_soc_dapm_route intercon[] = {
-	{"Analog L1 Playback Mixer", NULL, "DAC Left1"},
-	{"Analog R1 Playback Mixer", NULL, "DAC Right1"},
-	{"Analog L2 Playback Mixer", NULL, "DAC Left2"},
-	{"Analog R2 Playback Mixer", NULL, "DAC Right2"},
-	{"Analog Voice Playback Mixer", NULL, "DAC Voice"},
-
-	{"ARXL1_APGA", NULL, "Analog L1 Playback Mixer"},
-	{"ARXR1_APGA", NULL, "Analog R1 Playback Mixer"},
-	{"ARXL2_APGA", NULL, "Analog L2 Playback Mixer"},
-	{"ARXR2_APGA", NULL, "Analog R2 Playback Mixer"},
-	{"VDL_APGA", NULL, "Analog Voice Playback Mixer"},
+	{"Digital L1 Playback Mixer", NULL, "DAC Left1"},
+	{"Digital R1 Playback Mixer", NULL, "DAC Right1"},
+	{"Digital L2 Playback Mixer", NULL, "DAC Left2"},
+	{"Digital R2 Playback Mixer", NULL, "DAC Right2"},
+	{"Digital Voice Playback Mixer", NULL, "DAC Voice"},
+
+	{"Analog L1 Playback Mixer", NULL, "Digital L1 Playback Mixer"},
+	{"Analog R1 Playback Mixer", NULL, "Digital R1 Playback Mixer"},
+	{"Analog L2 Playback Mixer", NULL, "Digital L2 Playback Mixer"},
+	{"Analog R2 Playback Mixer", NULL, "Digital R2 Playback Mixer"},
+	{"Analog Voice Playback Mixer", NULL, "Digital Voice Playback Mixer"},
 
 	/* Internal playback routings */
 	/* Earpiece */
-	{"Earpiece Mixer", "Voice", "VDL_APGA"},
-	{"Earpiece Mixer", "AudioL1", "ARXL1_APGA"},
-	{"Earpiece Mixer", "AudioL2", "ARXL2_APGA"},
-	{"Earpiece Mixer", "AudioR1", "ARXR1_APGA"},
+	{"Earpiece Mixer", "Voice", "Analog Voice Playback Mixer"},
+	{"Earpiece Mixer", "AudioL1", "Analog L1 Playback Mixer"},
+	{"Earpiece Mixer", "AudioL2", "Analog L2 Playback Mixer"},
+	{"Earpiece Mixer", "AudioR1", "Analog R1 Playback Mixer"},
 	/* PreDrivL */
-	{"PredriveL Mixer", "Voice", "VDL_APGA"},
-	{"PredriveL Mixer", "AudioL1", "ARXL1_APGA"},
-	{"PredriveL Mixer", "AudioL2", "ARXL2_APGA"},
-	{"PredriveL Mixer", "AudioR2", "ARXR2_APGA"},
+	{"PredriveL Mixer", "Voice", "Analog Voice Playback Mixer"},
+	{"PredriveL Mixer", "AudioL1", "Analog L1 Playback Mixer"},
+	{"PredriveL Mixer", "AudioL2", "Analog L2 Playback Mixer"},
+	{"PredriveL Mixer", "AudioR2", "Analog R2 Playback Mixer"},
 	/* PreDrivR */
-	{"PredriveR Mixer", "Voice", "VDL_APGA"},
-	{"PredriveR Mixer", "AudioR1", "ARXR1_APGA"},
-	{"PredriveR Mixer", "AudioR2", "ARXR2_APGA"},
-	{"PredriveR Mixer", "AudioL2", "ARXL2_APGA"},
+	{"PredriveR Mixer", "Voice", "Analog Voice Playback Mixer"},
+	{"PredriveR Mixer", "AudioR1", "Analog R1 Playback Mixer"},
+	{"PredriveR Mixer", "AudioR2", "Analog R2 Playback Mixer"},
+	{"PredriveR Mixer", "AudioL2", "Analog L2 Playback Mixer"},
 	/* HeadsetL */
-	{"HeadsetL Mixer", "Voice", "VDL_APGA"},
-	{"HeadsetL Mixer", "AudioL1", "ARXL1_APGA"},
-	{"HeadsetL Mixer", "AudioL2", "ARXL2_APGA"},
+	{"HeadsetL Mixer", "Voice", "Analog Voice Playback Mixer"},
+	{"HeadsetL Mixer", "AudioL1", "Analog L1 Playback Mixer"},
+	{"HeadsetL Mixer", "AudioL2", "Analog L2 Playback Mixer"},
 	/* HeadsetR */
-	{"HeadsetR Mixer", "Voice", "VDL_APGA"},
-	{"HeadsetR Mixer", "AudioR1", "ARXR1_APGA"},
-	{"HeadsetR Mixer", "AudioR2", "ARXR2_APGA"},
+	{"HeadsetR Mixer", "Voice", "Analog Voice Playback Mixer"},
+	{"HeadsetR Mixer", "AudioR1", "Analog R1 Playback Mixer"},
+	{"HeadsetR Mixer", "AudioR2", "Analog R2 Playback Mixer"},
 	/* CarkitL */
-	{"CarkitL Mixer", "Voice", "VDL_APGA"},
-	{"CarkitL Mixer", "AudioL1", "ARXL1_APGA"},
-	{"CarkitL Mixer", "AudioL2", "ARXL2_APGA"},
+	{"CarkitL Mixer", "Voice", "Analog Voice Playback Mixer"},
+	{"CarkitL Mixer", "AudioL1", "Analog L1 Playback Mixer"},
+	{"CarkitL Mixer", "AudioL2", "Analog L2 Playback Mixer"},
 	/* CarkitR */
-	{"CarkitR Mixer", "Voice", "VDL_APGA"},
-	{"CarkitR Mixer", "AudioR1", "ARXR1_APGA"},
-	{"CarkitR Mixer", "AudioR2", "ARXR2_APGA"},
+	{"CarkitR Mixer", "Voice", "Analog Voice Playback Mixer"},
+	{"CarkitR Mixer", "AudioR1", "Analog R1 Playback Mixer"},
+	{"CarkitR Mixer", "AudioR2", "Analog R2 Playback Mixer"},
 	/* HandsfreeL */
-	{"HandsfreeL Mux", "Voice", "VDL_APGA"},
-	{"HandsfreeL Mux", "AudioL1", "ARXL1_APGA"},
-	{"HandsfreeL Mux", "AudioL2", "ARXL2_APGA"},
-	{"HandsfreeL Mux", "AudioR2", "ARXR2_APGA"},
+	{"HandsfreeL Mux", "Voice", "Analog Voice Playback Mixer"},
+	{"HandsfreeL Mux", "AudioL1", "Analog L1 Playback Mixer"},
+	{"HandsfreeL Mux", "AudioL2", "Analog L2 Playback Mixer"},
+	{"HandsfreeL Mux", "AudioR2", "Analog R2 Playback Mixer"},
 	/* HandsfreeR */
-	{"HandsfreeR Mux", "Voice", "VDL_APGA"},
-	{"HandsfreeR Mux", "AudioR1", "ARXR1_APGA"},
-	{"HandsfreeR Mux", "AudioR2", "ARXR2_APGA"},
-	{"HandsfreeR Mux", "AudioL2", "ARXL2_APGA"},
+	{"HandsfreeR Mux", "Voice", "Analog Voice Playback Mixer"},
+	{"HandsfreeR Mux", "AudioR1", "Analog R1 Playback Mixer"},
+	{"HandsfreeR Mux", "AudioR2", "Analog R2 Playback Mixer"},
+	{"HandsfreeR Mux", "AudioL2", "Analog L2 Playback Mixer"},
 	/* Vibra */
 	{"Vibra Mux", "AudioL1", "DAC Left1"},
 	{"Vibra Mux", "AudioR1", "DAC Right1"},
@@ -1208,8 +1209,8 @@ static const struct snd_soc_dapm_route intercon[] = {
 	{"Vibra Mux", "AudioR2", "DAC Right2"},
 
 	/* outputs */
-	{"OUTL", NULL, "ARXL2_APGA"},
-	{"OUTR", NULL, "ARXR2_APGA"},
+	{"OUTL", NULL, "Analog L2 Playback Mixer"},
+	{"OUTR", NULL, "Analog R2 Playback Mixer"},
 	{"EARPIECE", NULL, "Earpiece Mixer"},
 	{"PREDRIVEL", NULL, "PredriveL Mixer"},
 	{"PREDRIVER", NULL, "PredriveR Mixer"},
@@ -1273,9 +1274,9 @@ static const struct snd_soc_dapm_route intercon[] = {
 	{"Left Digital Loopback", "Volume", "TX1 Capture Route"},
 	{"Voice Digital Loopback", "Volume", "TX2 Capture Route"},
 
-	{"Analog R2 Playback Mixer", NULL, "Right Digital Loopback"},
-	{"Analog L2 Playback Mixer", NULL, "Left Digital Loopback"},
-	{"Analog Voice Playback Mixer", NULL, "Voice Digital Loopback"},
+	{"Digital R2 Playback Mixer", NULL, "Right Digital Loopback"},
+	{"Digital L2 Playback Mixer", NULL, "Left Digital Loopback"},
+	{"Digital Voice Playback Mixer", NULL, "Voice Digital Loopback"},
 
 };
 
-- 
1.6.3

[PATCH 2/2] ASoC: TWL4030: Move the Headset pop-attenuation code to PGA event

[Peter Ujfalusi]

This patch adds SND_SOC_DAPM_PGA_E to the headset path, which handles
the headset ramp up and down sequences needed for the pop noise
removal.

With this patch the order of the internal components in the twl4030
codec is turned on and off in a correct order.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/codecs/twl4030.c |  116 ++++++++++++++++++++++++++++++++++---------
 1 files changed, 91 insertions(+), 25 deletions(-)

diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
index 990aa4a..afe7879 100644
--- a/sound/soc/codecs/twl4030.c
+++ b/sound/soc/codecs/twl4030.c
@@ -130,6 +130,12 @@ struct twl4030_priv {
 	unsigned int rate;
 	unsigned int sample_bits;
 	unsigned int channels;
+
+	unsigned int sysclk;
+
+	/* Headset output state handling */
+	unsigned int hsl_enabled;
+	unsigned int hsr_enabled;
 };
 
 /*
@@ -564,39 +570,85 @@ static int handsfree_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
-static int headsetl_event(struct snd_soc_dapm_widget *w,
-		struct snd_kcontrol *kcontrol, int event)
+static void headset_ramp(struct snd_soc_codec *codec, int ramp)
 {
 	unsigned char hs_gain, hs_pop;
+	struct twl4030_priv *twl4030 = codec->private_data;
+	/* Base values for ramp delay calculation: 2^19 - 2^26 */
+	unsigned int ramp_base[] = {524288, 1048576, 2097152, 4194304,
+				    8388608, 16777216, 33554432, 67108864};
 
-	/* Save the current volume */
-	hs_gain = twl4030_read_reg_cache(w->codec, TWL4030_REG_HS_GAIN_SET);
-	hs_pop = twl4030_read_reg_cache(w->codec, TWL4030_REG_HS_POPN_SET);
+	hs_gain = twl4030_read_reg_cache(codec, TWL4030_REG_HS_GAIN_SET);
+	hs_pop = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET);
 
-	switch (event) {
-	case SND_SOC_DAPM_POST_PMU:
-		/* Do the anti-pop/bias ramp enable according to the TRM */
+	if (ramp) {
+		/* Headset ramp-up according to the TRM */
 		hs_pop |= TWL4030_VMID_EN;
-		twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop);
-		/* Is this needed? Can we just use whatever gain here? */
-		twl4030_write(w->codec, TWL4030_REG_HS_GAIN_SET,
-				(hs_gain & (~0x0f)) | 0x0a);
+		twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop);
+		twl4030_write(codec, TWL4030_REG_HS_GAIN_SET, hs_gain);
 		hs_pop |= TWL4030_RAMP_EN;
-		twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop);
-
-		/* Restore the original volume */
-		twl4030_write(w->codec, TWL4030_REG_HS_GAIN_SET, hs_gain);
-		break;
-	case SND_SOC_DAPM_POST_PMD:
-		/* Do the anti-pop/bias ramp disable according to the TRM */
+		twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop);
+	} else {
+		/* Headset ramp-down _not_ according to
+		 * the TRM, but in a way that it is working */
 		hs_pop &= ~TWL4030_RAMP_EN;
-		twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop);
+		twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop);
+		/* Wait ramp delay time + 1, so the VMID can settle */
+		mdelay((ramp_base[(hs_pop & TWL4030_RAMP_DELAY) >> 2] /
+			twl4030->sysclk) + 1);
 		/* Bypass the reg_cache to mute the headset */
 		twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE,
 					hs_gain & (~0x0f),
 					TWL4030_REG_HS_GAIN_SET);
+
 		hs_pop &= ~TWL4030_VMID_EN;
-		twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop);
+		twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop);
+	}
+}
+
+static int headsetlpga_event(struct snd_soc_dapm_widget *w,
+		struct snd_kcontrol *kcontrol, int event)
+{
+	struct twl4030_priv *twl4030 = w->codec->private_data;
+
+	switch (event) {
+	case SND_SOC_DAPM_POST_PMU:
+		/* Do the ramp-up only once */
+		if (!twl4030->hsr_enabled)
+			headset_ramp(w->codec, 1);
+
+		twl4030->hsl_enabled = 1;
+		break;
+	case SND_SOC_DAPM_POST_PMD:
+		/* Do the ramp-down only if both headsetL/R is disabled */
+		if (!twl4030->hsr_enabled)
+			headset_ramp(w->codec, 0);
+
+		twl4030->hsl_enabled = 0;
+		break;
+	}
+	return 0;
+}
+
+static int headsetrpga_event(struct snd_soc_dapm_widget *w,
+		struct snd_kcontrol *kcontrol, int event)
+{
+	struct twl4030_priv *twl4030 = w->codec->private_data;
+
+	switch (event) {
+	case SND_SOC_DAPM_POST_PMU:
+		/* Do the ramp-up only once */
+		if (!twl4030->hsl_enabled)
+			headset_ramp(w->codec, 1);
+
+		twl4030->hsr_enabled = 1;
+		break;
+	case SND_SOC_DAPM_POST_PMD:
+		/* Do the ramp-down only if both headsetL/R is disabled */
+		if (!twl4030->hsl_enabled)
+			headset_ramp(w->codec, 0);
+
+		twl4030->hsr_enabled = 0;
 		break;
 	}
 	return 0;
@@ -1069,13 +1121,18 @@ static const struct snd_soc_dapm_widget twl4030_dapm_widgets[] = {
 			&twl4030_dapm_predriver_controls[0],
 			ARRAY_SIZE(twl4030_dapm_predriver_controls)),
 	/* HeadsetL/R */
-	SND_SOC_DAPM_MIXER_E("HeadsetL Mixer", SND_SOC_NOPM, 0, 0,
+	SND_SOC_DAPM_MIXER("HeadsetL Mixer", SND_SOC_NOPM, 0, 0,
 			&twl4030_dapm_hsol_controls[0],
-			ARRAY_SIZE(twl4030_dapm_hsol_controls), headsetl_event,
+			ARRAY_SIZE(twl4030_dapm_hsol_controls)),
+	SND_SOC_DAPM_PGA_E("HeadsetL PGA", SND_SOC_NOPM,
+			0, 0, NULL, 0, headsetlpga_event,
 			SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD),
 	SND_SOC_DAPM_MIXER("HeadsetR Mixer", SND_SOC_NOPM, 0, 0,
 			&twl4030_dapm_hsor_controls[0],
 			ARRAY_SIZE(twl4030_dapm_hsor_controls)),
+	SND_SOC_DAPM_PGA_E("HeadsetR PGA", SND_SOC_NOPM,
+			0, 0, NULL, 0, headsetrpga_event,
+			SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD),
 	/* CarkitL/R */
 	SND_SOC_DAPM_MIXER("CarkitL Mixer", SND_SOC_NOPM, 0, 0,
 			&twl4030_dapm_carkitl_controls[0],
@@ -1180,10 +1237,12 @@ static const struct snd_soc_dapm_route intercon[] = {
 	{"HeadsetL Mixer", "Voice", "Analog Voice Playback Mixer"},
 	{"HeadsetL Mixer", "AudioL1", "Analog L1 Playback Mixer"},
 	{"HeadsetL Mixer", "AudioL2", "Analog L2 Playback Mixer"},
+	{"HeadsetL PGA", NULL, "HeadsetL Mixer"},
 	/* HeadsetR */
 	{"HeadsetR Mixer", "Voice", "Analog Voice Playback Mixer"},
 	{"HeadsetR Mixer", "AudioR1", "Analog R1 Playback Mixer"},
 	{"HeadsetR Mixer", "AudioR2", "Analog R2 Playback Mixer"},
+	{"HeadsetR PGA", NULL, "HeadsetR Mixer"},
 	/* CarkitL */
 	{"CarkitL Mixer", "Voice", "Analog Voice Playback Mixer"},
 	{"CarkitL Mixer", "AudioL1", "Analog L1 Playback Mixer"},
@@ -1214,8 +1273,8 @@ static const struct snd_soc_dapm_route intercon[] = {
 	{"EARPIECE", NULL, "Earpiece Mixer"},
 	{"PREDRIVEL", NULL, "PredriveL Mixer"},
 	{"PREDRIVER", NULL, "PredriveR Mixer"},
-	{"HSOL", NULL, "HeadsetL Mixer"},
-	{"HSOR", NULL, "HeadsetR Mixer"},
+	{"HSOL", NULL, "HeadsetL PGA"},
+	{"HSOR", NULL, "HeadsetR PGA"},
 	{"CARKITL", NULL, "CarkitL Mixer"},
 	{"CARKITR", NULL, "CarkitR Mixer"},
 	{"HFL", NULL, "HandsfreeL Mux"},
@@ -1554,17 +1613,21 @@ static int twl4030_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 		int clk_id, unsigned int freq, int dir)
 {
 	struct snd_soc_codec *codec = codec_dai->codec;
+	struct twl4030_priv *twl4030 = codec->private_data;
 	u8 infreq;
 
 	switch (freq) {
 	case 19200000:
 		infreq = TWL4030_APLL_INFREQ_19200KHZ;
+		twl4030->sysclk = 19200;
 		break;
 	case 26000000:
 		infreq = TWL4030_APLL_INFREQ_26000KHZ;
+		twl4030->sysclk = 26000;
 		break;
 	case 38400000:
 		infreq = TWL4030_APLL_INFREQ_38400KHZ;
+		twl4030->sysclk = 38400;
 		break;
 	default:
 		printk(KERN_ERR "TWL4030 set sysclk: unknown rate %d\n",
@@ -1953,6 +2016,9 @@ static int twl4030_probe(struct platform_device *pdev)
 		kfree(codec);
 		return -ENOMEM;
 	}
+	/* Set default sysclk (used by the headsetl/rpga_event callback for
+	 * pop-attenuation) */
+	twl4030->sysclk = 26000;
 
 	codec->private_data = twl4030;
 	socdev->card->codec = codec;
-- 
1.6.3

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Mark Brown]

On Mon, May 18, 2009 at 04:02:03PM +0300, Peter Ujfalusi wrote:

> The following series aims to fix the Headset power on/off pop noise problems.

Looks OK but given all the to and fro here I'd rather wait for some
testing to make sure it works well on other system.s

> I needed to move the APGA control from SND_SOC_DAPM_PGA to be handled with
> _MIXER. Than adding a PGA to the headset outputs results correct power
> sequences.

TBH it sounds like this is actually a mixer anyway - typically mixers do
have some gain controls associated with them, the sort of PGA DAPM means
is more an output stage driver.  Typically these have much more control
of the gain and only one input and output.

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Aggarwal, Anuj]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Monday, May 18, 2009 6:39 PM
> To: Peter Ujfalusi
> Cc: alsa-devel@alsa-project.org; sakoman@gmail.com; Aggarwal, Anuj;
> getarunks@gmail.com
> Subject: Re: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring
> and Headset pop-attenuation fix
> 
> On Mon, May 18, 2009 at 04:02:03PM +0300, Peter Ujfalusi wrote:
> 
> > The following series aims to fix the Headset power on/off pop noise
> problems.
> 
> Looks OK but given all the to and fro here I'd rather wait for some
> testing to make sure it works well on other system.s
[Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values 0-3 produces minor glitches in the beginning & end, value 4 gave me the best result so far on my EVM, I will try with some other EVMs as well before finalizing it. 5 and others produce a beep in the beginning and end.
Just one question, is there any side-effect of choosing a large ramp delay? For e.g. in my case, it could be 437ms (19.2MHz clock).
> 
> > I needed to move the APGA control from SND_SOC_DAPM_PGA to be
> handled with
> > _MIXER. Than adding a PGA to the headset outputs results correct power
> > sequences.
> 
> TBH it sounds like this is actually a mixer anyway - typically mixers do
> have some gain controls associated with them, the sort of PGA DAPM means
> is more an output stage driver.  Typically these have much more control
> of the gain and only one input and output.

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Lopez Cruz, Misael]

> > > The following series aims to fix the Headset power on/off 
> > > pop noise problems.

> > Looks OK but given all the to and fro here I'd rather wait for some 
> > testing to make sure it works well on other system.s

> [Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values 
> 0-3 produces minor glitches in the beginning & end, value 4 
> gave me the best result so far on my EVM, I will try with 
> some other EVMs as well before finalizing it. 5 and others 
> produce a beep in the beginning and end.
> Just one question, is there any side-effect of choosing a 
> large ramp delay? For e.g. in my case, it could be 437ms 
> (19.2MHz clock).

I tested on SDP3430, the best ramp delay value for my board seems to
be 3. Lower values give glitches and higher values present the beep
sound. In general, it has been reduced.

-Misa

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Peter Ujfalusi]

On Monday 18 May 2009 18:28:52 ext Aggarwal, Anuj wrote:

> [Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values 0-3 produces
> minor glitches in the beginning & end, value 4 gave me the best result so
> far on my EVM, I will try with some other EVMs as well before finalizing
> it. 5 and others produce a beep in the beginning and end. Just one
> question, is there any side-effect of choosing a large ramp delay? For e.g.
> in my case, it could be 437ms (19.2MHz clock).

It is less than half a second ;)
I'm not sure, but we could try later to add a timer to finish up the ramp-
down.
There could be (not foreseen) side effects...
At this point I think this (series) is a good start and it is better than what 
we had.

BTW: I'll wait for the verdict on the series and I will send a fix for the 
Handsfree pop attenuation also.

-- 
Péter

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Aggarwal, Anuj]

I am facing problems on the capture side now, I don't know whether someone has tested it or not.

By default, capture was not working. I have to enable Analog Left Capture 
Route AUXL & Analog Right Capture Route AUXR to enable the capture 
paths for OMAP3 EVM. Even after enabling both the paths, I am not able to
capture audio on both the channels. I am hearing audio only on either of the
channel, the other one is playing only noise for me. (This channel swapping is 
random, sometimes it plays on left and sometimes on right.)

If I disable ' Analog Right Capture Route AUXR', I am not hearing any noise;
audio is playing fine, but again on either of the channels, not consistently on
the left one.

Moreover, in both the cases, I am getting large overrun errors (I am using 
NFS to store the recorded clips, will try with MMC etc. too): 
 
root:~# arecord -f cd -d 30 rec05.dat
Recording WAVE 'rec05.dat' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo
overrun!!! (at least 8424.774 ms long)
overrun!!! (at least 6465.943 ms long)
overrun!!! (at least 3377.716 ms long)
overrun!!! (at least 7982.147 ms long)
overrun!!! (at least 8649.720 ms long)

I didn't face both the problems on my original code base (2.6.29-rc3) and hence
wanted to confirm whether someone else has also faced the same issues. I am
debugging at my end to see what difference is causing the problem but any 
pointers are welcome.

Thanks and Regards,
Anuj Aggarwal

> -----Original Message-----
> From: Lopez Cruz, Misael
> Sent: Tuesday, May 19, 2009 6:18 AM
> To: Aggarwal, Anuj; Mark Brown; Peter Ujfalusi
> Cc: sakoman@gmail.com; alsa-devel@alsa-project.org; getarunks@gmail.com
> Subject: RE: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring
> and Headset pop-attenuation fix
> 
> > > > The following series aims to fix the Headset power on/off
> > > > pop noise problems.
> 
> > > Looks OK but given all the to and fro here I'd rather wait for some
> > > testing to make sure it works well on other system.s
> 
> > [Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values
> > 0-3 produces minor glitches in the beginning & end, value 4
> > gave me the best result so far on my EVM, I will try with
> > some other EVMs as well before finalizing it. 5 and others
> > produce a beep in the beginning and end.
> > Just one question, is there any side-effect of choosing a
> > large ramp delay? For e.g. in my case, it could be 437ms
> > (19.2MHz clock).
> 
> I tested on SDP3430, the best ramp delay value for my board seems to
> be 3. Lower values give glitches and higher values present the beep
> sound. In general, it has been reduced.
> 
> -Misa

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Jarkko Nikula]

On Tue, 19 May 2009 09:46:40 +0300
Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> On Monday 18 May 2009 18:28:52 ext Aggarwal, Anuj wrote:
> 
> > [Aggarwal, Anuj] I tried this on OMAP3 EVM. Ramp delay values 0-3
> > produces minor glitches in the beginning & end, value 4 gave me the
> > best result so far on my EVM, I will try with some other EVMs as
> > well before finalizing it. 5 and others produce a beep in the
> > beginning and end. Just one question, is there any side-effect of
> > choosing a large ramp delay? For e.g. in my case, it could be 437ms
> > (19.2MHz clock).
> 
> It is less than half a second ;)
> I'm not sure, but we could try later to add a timer to finish up the
> ramp- down.
> There could be (not foreseen) side effects...
> At this point I think this (series) is a good start and it is better
> than what we had.
> 
For my Beagle the power-up/-down low-frequency pop goes more silent as
the ramp time increases. Sharp power-down snap is there with '27/20/14
ms' and '55/40/27 ms' but not times above.

Beep sound is here when ramp is '109/81/55 ms' and above. As ramp time
increases, the beep sound frequency goes down. Codec HW issue?

I don't have any objections to these two patches and as the music still
plays fine so probably these can be pushed in safely.


-- 
Jarkko

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Peter Ujfalusi]

On Tuesday 19 May 2009 09:47:13 ext Aggarwal, Anuj wrote:
> I am facing problems on the capture side now, I don't know whether someone
> has tested it or not.
>
> By default, capture was not working. I have to enable Analog Left Capture
> Route AUXL & Analog Right Capture Route AUXR to enable the capture
> paths for OMAP3 EVM. Even after enabling both the paths, I am not able to
> capture audio on both the channels. I am hearing audio only on either of
> the channel, the other one is playing only noise for me. (This channel
> swapping is random, sometimes it plays on left and sometimes on right.)
>
> If I disable ' Analog Right Capture Route AUXR', I am not hearing any
> noise; audio is playing fine, but again on either of the channels, not
> consistently on the left one.
>
> Moreover, in both the cases, I am getting large overrun errors (I am using
> NFS to store the recorded clips, will try with MMC etc. too):
>
> root:~# arecord -f cd -d 30 rec05.dat
> Recording WAVE 'rec05.dat' : Signed 16 bit Little Endian, Rate 44100 Hz,
> Stereo overrun!!! (at least 8424.774 ms long)
> overrun!!! (at least 6465.943 ms long)
> overrun!!! (at least 3377.716 ms long)
> overrun!!! (at least 7982.147 ms long)
> overrun!!! (at least 8649.720 ms long)
>
> I didn't face both the problems on my original code base (2.6.29-rc3) and
> hence wanted to confirm whether someone else has also faced the same
> issues. I am debugging at my end to see what difference is causing the
> problem but any pointers are welcome.

I have sent a patch, which should get the AUXR working again.

>
> Thanks and Regards,
> Anuj Aggarwal
>
-- 
Péter

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Aggarwal, Anuj]

> -----Original Message-----
> From: Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com]
> Sent: Tuesday, May 19, 2009 1:30 PM
> To: Aggarwal, Anuj
> Cc: Mark Brown; sakoman@gmail.com; alsa-devel@alsa-project.org;
> getarunks@gmail.com; Lopez Cruz, Misael
> Subject: Re: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring
> and Headset pop-attenuation fix
> 
> On Tuesday 19 May 2009 09:47:13 ext Aggarwal, Anuj wrote:
> > I am facing problems on the capture side now, I don't know whether
> someone
> > has tested it or not.
> >
> > By default, capture was not working. I have to enable Analog Left Capture
> > Route AUXL & Analog Right Capture Route AUXR to enable the capture
> > paths for OMAP3 EVM. Even after enabling both the paths, I am not able to
> > capture audio on both the channels. I am hearing audio only on either of
> > the channel, the other one is playing only noise for me. (This channel
> > swapping is random, sometimes it plays on left and sometimes on right.)
> >
> > If I disable ' Analog Right Capture Route AUXR', I am not hearing any
> > noise; audio is playing fine, but again on either of the channels, not
> > consistently on the left one.
> >
> > Moreover, in both the cases, I am getting large overrun errors (I am using
> > NFS to store the recorded clips, will try with MMC etc. too):
> >
> > root:~# arecord -f cd -d 30 rec05.dat
> > Recording WAVE 'rec05.dat' : Signed 16 bit Little Endian, Rate 44100 Hz,
> > Stereo overrun!!! (at least 8424.774 ms long)
> > overrun!!! (at least 6465.943 ms long)
> > overrun!!! (at least 3377.716 ms long)
> > overrun!!! (at least 7982.147 ms long)
> > overrun!!! (at least 8649.720 ms long)
> >
> > I didn't face both the problems on my original code base (2.6.29-rc3) and
> > hence wanted to confirm whether someone else has also faced the same
> > issues. I am debugging at my end to see what difference is causing the
> > problem but any pointers are welcome.
> 
> I have sent a patch, which should get the AUXR working again.
[Aggarwal, Anuj] Yeah, it is working fine now. Thanks for this.
I am still getting underrun errors, looking into this now.
> 
> >
> > Thanks and Regards,
> > Anuj Aggarwal
> >
> --
> Péter

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Mark Brown]

On Tue, May 19, 2009 at 10:33:16AM +0300, Jarkko Nikula wrote:

> I don't have any objections to these two patches and as the music still
> plays fine so probably these can be pushed in safely.

It seems like the feedback has been pretty much OK so it should be OK
apply the patches - Peter?

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Peter Ujfalusi]

On Tuesday 19 May 2009 21:58:06 ext Mark Brown wrote:
> On Tue, May 19, 2009 at 10:33:16AM +0300, Jarkko Nikula wrote:
> > I don't have any objections to these two patches and as the music still
> > plays fine so probably these can be pushed in safely.
>
> It seems like the feedback has been pretty much OK so it should be OK
> apply the patches - Peter?

I also think, that it should be OK to apply this series.

Thanks,
Péter

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Aggarwal, Anuj]

> -----Original Message-----
> From: Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com]
> Sent: Wednesday, May 20, 2009 11:27 AM
> To: ext Mark Brown
> Cc: Jarkko Nikula; Aggarwal, Anuj; sakoman@gmail.com; alsa-devel@alsa-
> project.org; getarunks@gmail.com
> Subject: Re: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring
> and Headset pop-attenuation fix
> 
> On Tuesday 19 May 2009 21:58:06 ext Mark Brown wrote:
> > On Tue, May 19, 2009 at 10:33:16AM +0300, Jarkko Nikula wrote:
> > > I don't have any objections to these two patches and as the music still
> > > plays fine so probably these can be pushed in safely.
> >
> > It seems like the feedback has been pretty much OK so it should be OK
> > apply the patches - Peter?
> 
> I also think, that it should be OK to apply this series.
[Aggarwal, Anuj] Since different platforms are using different ramp values, how should it be handled via the same codec source code? How should this be done, without modifying the code base? Or should it be done every time user wants to play an audio file?
> 
> Thanks,
> Péter

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Mark Brown]

On Wed, May 20, 2009 at 01:51:02PM +0530, Aggarwal, Anuj wrote:
> Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com]:
> > On Tuesday 19 May 2009 21:58:06 ext Mark Brown wrote:

> > > It seems like the feedback has been pretty much OK so it should be OK
> > > apply the patches - Peter?
> > I also think, that it should be OK to apply this series.

Applied - thanks for your hard work on this!  

BTW, I don't know if you noticed the DAPM changes I pushed the other
day.  One commit in particular, 452c5eaa0d5162e02ffee742ea17540887bc2904, 
might be interesting for TWL4030 - it changes things so that we're in
BIAS_ON whenever there's a bias path active, not only when there's a DAC
or ADC path.  I've got a feeling some of the pop/click stuff may mean
that's not as useful as it might've been in the past.

> [Aggarwal, Anuj] Since different platforms are using different ramp
> values, how should it be handled via the same codec source code? How
> should this be done, without modifying the code base? Or should it be
> done every time user wants to play an audio file?

Since it's just a change in one parameter the driver should be able to
arrange to get that configuration value provided to it via platform
data.  It's still the same chip and needs the same driver, all that's
changing is configuration information.

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Peter Ujfalusi]

On Wednesday 20 May 2009 11:21:02 ext Aggarwal, Anuj wrote:

> [Aggarwal, Anuj] Since different platforms are using different ramp values,
> how should it be handled via the same codec source code? How should this be
> done, without modifying the code base? Or should it be done every time user
> wants to play an audio file?

I think we should leave the ramp delay configurable, since it is easier to 
'port' the codec to a new board.
We can add - as Mark already suggested - a platform data parameter for the 
twl4030 codec driver to set the correct ramp delay value for the given board. 
The actual user will still have means to change it, but if the correct ramp 
delay has been found for the board, the pop attenuation would work out of box 
on the boards. 

-- 
Péter

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Peter Ujfalusi]

On Wednesday 20 May 2009 11:51:21 ext Mark Brown wrote:
>
> Applied - thanks for your hard work on this!
>
> BTW, I don't know if you noticed the DAPM changes I pushed the other
> day.  One commit in particular, 452c5eaa0d5162e02ffee742ea17540887bc2904,
> might be interesting for TWL4030 - it changes things so that we're in
> BIAS_ON whenever there's a bias path active, not only when there's a DAC
> or ADC path.  I've got a feeling some of the pop/click stuff may mean
> that's not as useful as it might've been in the past.

I will look at this change.
BTW: I just got the Beagle board, it will take some time to get familiar with 
it, but than I can really use the latest and greatest of ASoC...

> > [Aggarwal, Anuj] Since different platforms are using different ramp
> > values, how should it be handled via the same codec source code? How
> > should this be done, without modifying the code base? Or should it be
> > done every time user wants to play an audio file?
>
> Since it's just a change in one parameter the driver should be able to
> arrange to get that configuration value provided to it via platform
> data.  It's still the same chip and needs the same driver, all that's
> changing is configuration information.

As for now, storing the mixer settings and restoring those at boot time would 
be the way.
I will look at the possibility to add platform data parameter to twl4030 codec 
to set the board specific initial ramp delay value.

-- 
Péter

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Aggarwal, Anuj]

> -----Original Message-----
> From: Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com]
> Sent: Wednesday, May 20, 2009 2:43 PM
> To: alsa-devel@alsa-project.org
> Cc: ext Mark Brown; Aggarwal, Anuj; sakoman@gmail.com;
> getarunks@gmail.com
> Subject: Re: [alsa-devel] [PATCH 0/2] ASoC: TWL4030: DAPM restructuring
> and Headset pop-attenuation fix
> 
> On Wednesday 20 May 2009 11:51:21 ext Mark Brown wrote:
> >
> > Applied - thanks for your hard work on this!
> >
> > BTW, I don't know if you noticed the DAPM changes I pushed the other
> > day.  One commit in particular,
> 452c5eaa0d5162e02ffee742ea17540887bc2904,
> > might be interesting for TWL4030 - it changes things so that we're in
> > BIAS_ON whenever there's a bias path active, not only when there's a DAC
> > or ADC path.  I've got a feeling some of the pop/click stuff may mean
> > that's not as useful as it might've been in the past.
> 
> I will look at this change.
> BTW: I just got the Beagle board, it will take some time to get familiar with
> it, but than I can really use the latest and greatest of ASoC...
> 
> > > [Aggarwal, Anuj] Since different platforms are using different ramp
> > > values, how should it be handled via the same codec source code? How
> > > should this be done, without modifying the code base? Or should it be
> > > done every time user wants to play an audio file?
> >
> > Since it's just a change in one parameter the driver should be able to
> > arrange to get that configuration value provided to it via platform
> > data.  It's still the same chip and needs the same driver, all that's
> > changing is configuration information.
> 
> As for now, storing the mixer settings and restoring those at boot time would
> be the way.
> I will look at the possibility to add platform data parameter to twl4030 codec
> to set the board specific initial ramp delay value.
[Aggarwal, Anuj] I tried to look into the soc-dapm.h for the appropriate platform/machine-domain specific widget which could do the needful but could not find any. What is the correct way of handling this? I want to pass the correct ramp value for my platform.
> 
> --
> Péter

Re: [PATCH 0/2] ASoC: TWL4030: DAPM restructuring and Headset pop-attenuation fix

[Mark Brown]

On Thu, May 21, 2009 at 04:17:45PM +0530, Aggarwal, Anuj wrote:

> [Aggarwal, Anuj] I tried to look into the soc-dapm.h for the
> appropriate platform/machine-domain specific widget which could do the
> needful but could not find any. What is the correct way of handling
> this? I want to pass the correct ramp value for my platform.

Provide standard platform data for the twl4030, which would need the
driver patching to use that (and hopefully converted to the standard
device model).  Or for the time being you can do a register write in the
DAI init function, but the way forward is the platform data.