Re: [alsa-devel] [PATCH] ASoC: TWL4030: Add EXT_MUTE gpio to reduce pop-noise effect

On Thursday 25 June 2009 05:25:33 ext Jorge Eduardo Candelaria wrote:

According to TRM, an external FET controlled by a 1.8V output signal can be used to reduce the pop-noise heard when the audio amplifier is switched on.

As a note: the TRM suggests to use GPIO6 of TWL for the external FET control (which is than can be controlled by the HS_POP_SET:EXTMUTE bit).

The signal is controlled by a gpio pin and should be defined in the machine driver. However, the codec driver takes care of enabling and disabling this output during the headset pop attenuation sequence.

If the board does not have an EXT_MUTE, then the machine driver should set the value of ext_mute_gpio to -EINVAL through ramp delay setup data, to bypass execution of the code that manages the gpio line.

The intention of the twl4030_setup_data is broader than that. It is true, that currently it is only used for the Headset click removal.

Also add a delay to let VMID settle in ramp up sequence.

Probably it is a good idea to add this delay. Without it there can be cases, when the first samples would have been played while the VMID is ramping up, making it a bit distorted (or weaker).

Signed-off-by: Jorge Eduardo Candelaria x0107209@ti.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 df42fa2..e850b30 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -27,6 +27,7 @@ #include <linux/i2c.h> #include <linux/platform_device.h> #include <linux/i2c/twl4030.h> +#include <linux/gpio.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -620,6 +621,9 @@ static int handsfreerpga_event(struct snd_soc_dapm_widget *w,

static void headset_ramp(struct snd_soc_codec *codec, int ramp) {

• struct snd_soc_device *socdev = codec->socdev;
• struct twl4030_setup_data *setup = socdev->codec_data;
• unsigned char hs_gain, hs_pop; struct twl4030_priv *twl4030 = codec->private_data; /* Base values for ramp delay calculation: 2^19 - 2^26 */

@@ -629,6 +633,11 @@ static void headset_ramp(struct snd_soc_codec *codec, int ramp) hs_gain = twl4030_read_reg_cache(codec, TWL4030_REG_HS_GAIN_SET); hs_pop = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET);

• /* Enable external mute, this dramatically reduces

• * the pop-noise */
  

• if (setup && gpio_is_valid(setup->ext_mute_gpio))

• gpio_set_value(setup->ext_mute_gpio, 1);
  

• if (ramp) { /* Headset ramp-up according to the TRM */ hs_pop |= TWL4030_VMID_EN;

@@ -636,6 +645,9 @@ static void headset_ramp(struct snd_soc_codec *codec, int ramp) twl4030_write(codec, TWL4030_REG_HS_GAIN_SET, hs_gain); hs_pop |= TWL4030_RAMP_EN; 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);
  

  } else { /* Headset ramp-down _not_ according to
  • the TRM, but in a way that it is working */

@@ -652,6 +664,10 @@ static void headset_ramp(struct snd_soc_codec *codec, int ramp) hs_pop &= ~TWL4030_VMID_EN; twl4030_write(codec, TWL4030_REG_HS_POPN_SET, hs_pop); }

• /* Disable external mute */
• if (setup && gpio_is_valid(setup->ext_mute_gpio))

• gpio_set_value(setup->ext_mute_gpio, 0);
  

}

static int headsetlpga_event(struct snd_soc_dapm_widget *w, @@ -2101,6 +2117,7 @@ static int twl4030_init(struct snd_soc_device *socdev) /* Configuration for headset ramp delay from setup data */ if (setup) { unsigned char hs_pop;

• unsigned int ext_mute;
  
  

  if (setup->sysclk) twl4030->sysclk = setup->sysclk;

@@ -2111,6 +2128,12 @@ static int twl4030_init(struct snd_soc_device *socdev) hs_pop &= ~TWL4030_RAMP_DELAY; hs_pop |= (setup->ramp_delay_value << 2); twl4030_write_reg_cache(codec, TWL4030_REG_HS_POPN_SET, hs_pop);

• if (gpio_is_valid(setup->ext_mute_gpio)) {
  

• 	ext_mute = setup->ext_mute_gpio;
  

• 	BUG_ON(gpio_request(ext_mute, "ext_mute") < 0);
  

Releasing the gpio?

• 	gpio_direction_output(ext_mute, 0);
  

• }
  

  } else { twl4030->sysclk = 26000; }

diff --git a/sound/soc/codecs/twl4030.h b/sound/soc/codecs/twl4030.h index fe5f395..23a75e2 100644 --- a/sound/soc/codecs/twl4030.h +++ b/sound/soc/codecs/twl4030.h @@ -274,6 +274,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 ext_mute_gpio;

};

#endif /* End of __TWL4030_AUDIO_H__ */

I think the HS ext mute should be handled in a different way: There should be a way to use also the HS_POP_SET:EXTMUTE (TWL GPIO6) or external gpio for the extmute. In order to use the TWL GPIO6 for extmute, the PMBR1:GPIO6_PWM0_MUTE should be set to 0x2 initially in TWL, but this is not the job of the codec driver (it is platform specific how this pin is used). Anyways, I think the correct approach should be something like this:

In twl4030.h:

struct twl4030_setup_data { unsigned int ramp_delay_value; unsigned int sysclk; + unsigned int hs_extmute:1; + void (*set_hs_extmute)(int mute); };

Than in the machine driver: a) No extmute possible Just don't set any of these new hs_extmute things.

b) extmute is used through TWL GPIO6 Set the hs_extmute to 1 Set the set_hs_extmute to NULL

c) extmute is used through some other ways Set the hs_extmute to 1

Request the gpio (or whatever you need to control the extmute), also take care of the cleanup. Implement the something like this:

void zoom2_hs_extmute(int mute) { gpio_set_value(GPIO_NUMBER, mute) }

Set the set_hs_extmute to zoom2_hs_extmute

than in the codec driver: If the hs_extmute is 0, than do nothing about the extmute, If only the hs_extmute is set, than set/clear the HS_POP_SET:EXTMUTE bit if both hs_extmute and set_hs_extmute are set, than call the set_hs_extmute function.

What do you think?