[alsa-devel] [PATCH] ASoC: TWL4030: Add EXT_MUTE gpio to reduce pop-noise effect
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.
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.
Also add a delay to let VMID settle in ramp up sequence.
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); + 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__ */
On Wed, 24 Jun 2009 21:25:33 -0500 Jorge Eduardo Candelaria x0107209@ti.com 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.
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.
Two cents side comment. Are there any reason why the mute FET can not be driven on thus grounding the output pin always when output is not active? I know this patch follows the TPS65950 TRM but IMO simplier and cleanear would be consider this FET like an amplifier (with inverted control) and define and control it only from machine driver.
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] /
} else { /* Headset ramp-down _not_ according totwl4030->sysclk) + 1);
- 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?
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).
TRM does suggest the use of TWL GPIO6, however, there may be cases (actually, there ARE cases) in which the FET control is assigned to another gpio.
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).
I agree. I was originally thinking of using the gpio library for either TWL GPIO6 or an external line. However, it should be better to follow the TRM approach, along with support for the special cases in which an external gpio is used for the extmute.
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?
This is a much better approach. I was having problems finding a way to keep the gpio calls away from the codec driver, but this helps a lot, thanks. I will try it out and test both cases:
- GPIO6 of TWL - external gpio line.
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).
You mention this is platform specific, but shouldn't twl4030 registers be accesed only by twl4030 drivers?
What if codec driver sets the register only when instructed by the machine driver. Something like:
if (setup->hs_extmute && !(setup->set_hs_extmute)) twl4030_i2c_write_u8(TWL4030_MODULE_INTBR, 0x02 << 2, TWL4030_REG_PMBR1);
But it still looks like it should'nt be in the codec driver. Any idea on how to do this?
participants (4)
-
Candelaria Villareal, Jorge
-
Jarkko Nikula
-
Jorge Eduardo Candelaria
-
Peter Ujfalusi