[alsa-devel] [PATCH 1/7] ASoC: WM8903: Fix mic detection register definitions
* There is no hysteresis enable field in the current datasheet. * Mic detection threshold field is only 2 bits wide.
Signed-off-by: Stephen Warren swarren@nvidia.com --- include/sound/wm8903.h | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/include/sound/wm8903.h b/include/sound/wm8903.h index 86172cf..cf7ccb7 100644 --- a/include/sound/wm8903.h +++ b/include/sound/wm8903.h @@ -17,13 +17,9 @@ /* * R6 (0x06) - Mic Bias Control 0 */ -#define WM8903_MICDET_HYST_ENA 0x0080 /* MICDET_HYST_ENA */ -#define WM8903_MICDET_HYST_ENA_MASK 0x0080 /* MICDET_HYST_ENA */ -#define WM8903_MICDET_HYST_ENA_SHIFT 7 /* MICDET_HYST_ENA */ -#define WM8903_MICDET_HYST_ENA_WIDTH 1 /* MICDET_HYST_ENA */ -#define WM8903_MICDET_THR_MASK 0x0070 /* MICDET_THR - [6:4] */ -#define WM8903_MICDET_THR_SHIFT 4 /* MICDET_THR - [6:4] */ -#define WM8903_MICDET_THR_WIDTH 3 /* MICDET_THR - [6:4] */ +#define WM8903_MICDET_THR_MASK 0x0030 /* MICDET_THR - [5:4] */ +#define WM8903_MICDET_THR_SHIFT 4 /* MICDET_THR - [5:4] */ +#define WM8903_MICDET_THR_WIDTH 2 /* MICDET_THR - [5:4] */ #define WM8903_MICSHORT_THR_MASK 0x000C /* MICSHORT_THR - [3:2] */ #define WM8903_MICSHORT_THR_SHIFT 2 /* MICSHORT_THR - [3:2] */ #define WM8903_MICSHORT_THR_WIDTH 2 /* MICSHORT_THR - [3:2] */
The mic detection HW should be enabled when either mic or short detection is required, not when only both are required.
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/codecs/wm8903.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c index 2f91227..a6ca1d4 100644 --- a/sound/soc/codecs/wm8903.c +++ b/sound/soc/codecs/wm8903.c @@ -1610,7 +1610,7 @@ int wm8903_mic_detect(struct snd_soc_codec *codec, struct snd_soc_jack *jack, WM8903_MICDET_EINT | WM8903_MICSHRT_EINT, irq_mask);
- if (det && shrt) { + if (det || shrt) { /* Enable mic detection, this may not have been set through * platform data (eg, if the defaults are OK). */ snd_soc_update_bits(codec, WM8903_WRITE_SEQUENCER_0,
The WM8903 mic detection circuit will interrupt when: a) It is first enabled, and a mic is detected as present. b) Whenever a mic is plugged in or removed.
The current code makes it easy to miss case (a) by misconfiguring micdet_cfg via platform data.
Specifically, if WM8903_MICDET_ENA and WM8903_MICBIAS_ENA are both part of micdet_cfg, and a mic is plugged in when the driver initializes, the interrupt will fire almost immediately. However, at that point, the micdet interrupt is probably still masked, and hence wm8903_irq does not handle it. However, the act of reading the interrupt status register in the ISR clears the micdet interrupt status, so it is lost.
To avoid this, refuse to write any fields from micdet_cfg to the register except those simply configuring detection thresholds; actual enabling of mic detecion should always be implemented by calling wm8903_mic_detect().
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/codecs/wm8903.c | 9 +++------ 1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c index a6ca1d4..f1a4665 100644 --- a/sound/soc/codecs/wm8903.c +++ b/sound/soc/codecs/wm8903.c @@ -1918,12 +1918,9 @@ static int wm8903_probe(struct snd_soc_codec *codec) }
snd_soc_write(codec, WM8903_MIC_BIAS_CONTROL_0, - pdata->micdet_cfg); - - /* Microphone detection needs the WSEQ clock */ - if (pdata->micdet_cfg) - snd_soc_update_bits(codec, WM8903_WRITE_SEQUENCER_0, - WM8903_WSEQ_ENA, WM8903_WSEQ_ENA); + pdata->micdet_cfg & + (WM8903_MICDET_THR_MASK | + WM8903_MICSHORT_THR_MASK));
wm8903->mic_delay = pdata->micdet_delay; }
On Thu, Feb 10, 2011 at 03:37:15PM -0700, Stephen Warren wrote:
Specifically, if WM8903_MICDET_ENA and WM8903_MICBIAS_ENA are both part of micdet_cfg, and a mic is plugged in when the driver initializes, the interrupt will fire almost immediately. However, at that point, the micdet interrupt is probably still masked, and hence wm8903_irq does not handle it. However, the act of reading the interrupt status register in the ISR clears the micdet interrupt status, so it is lost.
To avoid this, refuse to write any fields from micdet_cfg to the register except those simply configuring detection thresholds; actual enabling of mic detecion should always be implemented by calling wm8903_mic_detect().
As well as being detected via the main interrupt of the WM8903 the detection signals can also be brought out to the WM8903 GPIOs directly, allowing detection to operate without any I2C I/O. That mode can be configured via platform data only and the WM8903 microphone detection functions will never be called so the user needs to be able to enable detection via platform data.
I'll have a think and send something out later today; the easiest thing is to add more documentation and possibly some sanity checking code in the platform data parsing.
Mark Brown wrote at Friday, February 11, 2011 4:12 AM:
On Thu, Feb 10, 2011 at 03:37:15PM -0700, Stephen Warren wrote:
Specifically, if WM8903_MICDET_ENA and WM8903_MICBIAS_ENA are both part of micdet_cfg, and a mic is plugged in when the driver initializes, the interrupt will fire almost immediately. However, at that point, the micdet interrupt is probably still masked, and hence wm8903_irq does not handle it. However, the act of reading the interrupt status register in the ISR clears the micdet interrupt status, so it is lost.
To avoid this, refuse to write any fields from micdet_cfg to the register except those simply configuring detection thresholds; actual enabling of mic detecion should always be implemented by calling wm8903_mic_detect().
As well as being detected via the main interrupt of the WM8903 the detection signals can also be brought out to the WM8903 GPIOs directly, allowing detection to operate without any I2C I/O. That mode can be configured via platform data only and the WM8903 microphone detection functions will never be called so the user needs to be able to enable detection via platform data.
Oh yes, of course.
Probably the best thing is to disable, then immediately re-enable the relevant fields (micdet_ena) in the register during wm8903_mic_detect. That will turn off the HW mic detection module, then start detection afresh, hence causing the "initial" interrupt to fire when desired.
I'll take a crack at getting that working.
Stephen Warren wrote at Friday, February 11, 2011 9:06 AM:
Mark Brown wrote at Friday, February 11, 2011 4:12 AM:
... As well as being detected via the main interrupt of the WM8903 the detection signals can also be brought out to the WM8903 GPIOs directly, allowing detection to operate without any I2C I/O. That mode can be configured via platform data only and the WM8903 microphone detection functions will never be called so the user needs to be able to enable detection via platform data.
Probably the best thing is to disable, then immediately re-enable the relevant fields (micdet_ena) in the register during wm8903_mic_detect. That will turn off the HW mic detection module, then start detection afresh, hence causing the "initial" interrupt to fire when desired.
Strangely, modifying wm8903_mic_detect() to:
* Unconditionally clear MICDET_ENA * Update the IRQ mask like it does today * Re-enable MICDET_ENA if det||shrt
Doesn't actually solve the problem. I guess disabling MICDET_ENA doesn't actually reset the mic detection state machine.
Anyway, as you said, perhaps just documenting how to set up micdet_cfg in the platform_data header is indeed the best way, since everything works without any code changes if I simply don't set MICDET_ENA in micdet_cfg.
On Fri, Feb 11, 2011 at 09:29:06AM -0800, Stephen Warren wrote:
Strangely, modifying wm8903_mic_detect() to:
- Unconditionally clear MICDET_ENA
- Update the IRQ mask like it does today
- Re-enable MICDET_ENA if det||shrt
Doesn't actually solve the problem. I guess disabling MICDET_ENA doesn't actually reset the mic detection state machine.
That's kind of what I expect - remember that the microphone detection interrupst doesn't report the microphone status directly, they report deltas in the reported microphone status after debouncing.
Anyway, as you said, perhaps just documenting how to set up micdet_cfg in the platform_data header is indeed the best way, since everything works without any code changes if I simply don't set MICDET_ENA in micdet_cfg.
Yes, and it's not a particularly useful thing to do so I'm pretty comfortable just telling people not do do that.
* Add jack definition for mic jack * Request wm8903 to enable mic detection * Force mic bias on, since it's required for mic detection
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/harmony.c | 25 ++++++++++++++++++++++--- 1 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c index 2a473a8..2fbfa10 100644 --- a/sound/soc/tegra/harmony.c +++ b/sound/soc/tegra/harmony.c @@ -43,6 +43,8 @@ #include <sound/pcm_params.h> #include <sound/soc.h>
+#include "../codecs/wm8903.h" + #include "tegra_das.h" #include "tegra_i2s.h" #include "tegra_pcm.h" @@ -142,6 +144,15 @@ static struct snd_soc_jack_gpio harmony_hp_jack_gpios[] = { } };
+static struct snd_soc_jack harmony_mic_jack; + +static struct snd_soc_jack_pin harmony_mic_jack_pins[] = { + { + .pin = "Mic Jack", + .mask = SND_JACK_MICROPHONE, + }, +}; + static int harmony_event_int_spk(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event) { @@ -206,9 +217,6 @@ static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_add_routes(dapm, harmony_audio_map, ARRAY_SIZE(harmony_audio_map));
- snd_soc_dapm_enable_pin(dapm, "Mic Jack"); - snd_soc_dapm_sync(dapm); - harmony_hp_jack_gpios[0].gpio = pdata->gpio_hp_det; snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, &harmony_hp_jack); @@ -219,6 +227,17 @@ static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd) ARRAY_SIZE(harmony_hp_jack_gpios), harmony_hp_jack_gpios);
+ snd_soc_jack_new(codec, "Mic Jack", SND_JACK_MICROPHONE, + &harmony_mic_jack); + snd_soc_jack_add_pins(&harmony_mic_jack, + ARRAY_SIZE(harmony_mic_jack_pins), + harmony_mic_jack_pins); + wm8903_mic_detect(codec, &harmony_mic_jack, SND_JACK_MICROPHONE, 0); + + snd_soc_dapm_force_enable_pin(dapm, "Mic Bias"); + + snd_soc_dapm_sync(dapm); + return 0; }
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/harmony.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c index 2fbfa10..6cbb352 100644 --- a/sound/soc/tegra/harmony.c +++ b/sound/soc/tegra/harmony.c @@ -236,6 +236,11 @@ static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd)
snd_soc_dapm_force_enable_pin(dapm, "Mic Bias");
+ snd_soc_dapm_nc_pin(dapm, "IN3L"); + snd_soc_dapm_nc_pin(dapm, "IN3R"); + snd_soc_dapm_nc_pin(dapm, "LINEOUTL"); + snd_soc_dapm_nc_pin(dapm, "LINEOUTR"); + snd_soc_dapm_sync(dapm);
return 0;
Harmony has both an external mic (a regular mic jack) and an internal mic (a 0.1" two-pin header on the board).
The external mic is connected to the WM8903's IN1L pin, and is supported by the current driver.
The internal mic is connected to the WM8903's IN1R pin, and is not supported by the current driver.
It appears that no Harmony systems were shipped with any internal mic connected; users were expected to provide their own. This makes the internal mic connection less interesting.
The WM8903's Mic Bias signal is used for both of these mics. For each mic, a GPIO drives a transistor which gates whether the mic bias signal is actively connected to that mic, or isolated from it.
The dual use of the mic bias for both mics makes a general-purpose complete implementation of mic detection using the mic bias complex. So, for simplicity, the internal mic is currently ignored by the driver.
This patch configures the relevant GPIOs to enable the mic bias connection to the external mic, and disable the mic bias connection to the internal mic. Note that in practice, this is the default state if these GPIOs aren't configured.
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/harmony.c | 34 +++++++++++++++++++++++++++++++--- 1 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c index 6cbb352..c54d420 100644 --- a/sound/soc/tegra/harmony.c +++ b/sound/soc/tegra/harmony.c @@ -52,10 +52,14 @@
#define DRV_NAME "tegra-snd-harmony"
+#define GPIO_SPKR_EN BIT(0) +#define GPIO_INT_MIC_EN BIT(1) +#define GPIO_EXT_MIC_EN BIT(2) + struct tegra_harmony { struct tegra_asoc_utils_data util_data; struct harmony_audio_platform_data *pdata; - int gpio_spkr_en_requested; + int gpio_requested; };
static int harmony_asoc_hw_params(struct snd_pcm_substream *substream, @@ -202,10 +206,30 @@ static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd) dev_err(card->dev, "cannot get spkr_en gpio\n"); return ret; } - harmony->gpio_spkr_en_requested = 1; + harmony->gpio_requested |= GPIO_SPKR_EN;
gpio_direction_output(pdata->gpio_spkr_en, 0);
+ ret = gpio_request(pdata->gpio_int_mic_en, "int_mic_en"); + if (ret) { + dev_err(card->dev, "cannot get int_mic_en gpio\n"); + return ret; + } + harmony->gpio_requested |= GPIO_INT_MIC_EN; + + /* Disable int mic; enable signal is active-high */ + gpio_direction_output(pdata->gpio_int_mic_en, 0); + + ret = gpio_request(pdata->gpio_ext_mic_en, "ext_mic_en"); + if (ret) { + dev_err(card->dev, "cannot get ext_mic_en gpio\n"); + return ret; + } + harmony->gpio_requested |= GPIO_EXT_MIC_EN; + + /* Enable ext mic; enable signal is active-low */ + gpio_direction_output(pdata->gpio_ext_mic_en, 0); + ret = snd_soc_add_controls(codec, harmony_controls, ARRAY_SIZE(harmony_controls)); if (ret < 0) @@ -330,7 +354,11 @@ static int __devexit tegra_snd_harmony_remove(struct platform_device *pdev)
tegra_asoc_utils_fini(&harmony->util_data);
- if (harmony->gpio_spkr_en_requested) + if (harmony->gpio_requested & GPIO_EXT_MIC_EN) + gpio_free(pdata->gpio_ext_mic_en); + if (harmony->gpio_requested & GPIO_INT_MIC_EN) + gpio_free(pdata->gpio_int_mic_en); + if (harmony->gpio_requested & GPIO_SPKR_EN) gpio_free(pdata->gpio_spkr_en);
kfree(harmony);
With the appropriate MODULE_ALIAS in place, the audio modules will be automatically loaded; there is no longer a need for manual modprobes.
Signed-off-by: Stephen Warren swarren@nvidia.com --- sound/soc/tegra/harmony.c | 1 + sound/soc/tegra/tegra_das.c | 1 + sound/soc/tegra/tegra_i2s.c | 1 + sound/soc/tegra/tegra_pcm.c | 5 ++++- 4 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c index c54d420..e6216b4 100644 --- a/sound/soc/tegra/harmony.c +++ b/sound/soc/tegra/harmony.c @@ -390,3 +390,4 @@ module_exit(snd_tegra_harmony_exit); MODULE_AUTHOR("Stephen Warren swarren@nvidia.com"); MODULE_DESCRIPTION("Harmony machine ASoC driver"); MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:" DRV_NAME); diff --git a/sound/soc/tegra/tegra_das.c b/sound/soc/tegra/tegra_das.c index 01eb9c9..9f24ef7 100644 --- a/sound/soc/tegra/tegra_das.c +++ b/sound/soc/tegra/tegra_das.c @@ -262,3 +262,4 @@ module_exit(tegra_das_modexit); MODULE_AUTHOR("Stephen Warren swarren@nvidia.com"); MODULE_DESCRIPTION("Tegra DAS driver"); MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:" DRV_NAME); diff --git a/sound/soc/tegra/tegra_i2s.c b/sound/soc/tegra/tegra_i2s.c index 870ee36..4f5e2c9 100644 --- a/sound/soc/tegra/tegra_i2s.c +++ b/sound/soc/tegra/tegra_i2s.c @@ -500,3 +500,4 @@ module_exit(snd_tegra_i2s_exit); MODULE_AUTHOR("Stephen Warren swarren@nvidia.com"); MODULE_DESCRIPTION("Tegra I2S ASoC driver"); MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:" DRV_NAME); diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c index 663ea9f..40540b1 100644 --- a/sound/soc/tegra/tegra_pcm.c +++ b/sound/soc/tegra/tegra_pcm.c @@ -39,6 +39,8 @@
#include "tegra_pcm.h"
+#define DRV_NAME "tegra-pcm-audio" + static const struct snd_pcm_hardware tegra_pcm_hardware = { .info = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID | @@ -377,7 +379,7 @@ static int __devexit tegra_pcm_platform_remove(struct platform_device *pdev)
static struct platform_driver tegra_pcm_driver = { .driver = { - .name = "tegra-pcm-audio", + .name = DRV_NAME, .owner = THIS_MODULE, }, .probe = tegra_pcm_platform_probe, @@ -399,3 +401,4 @@ module_exit(snd_tegra_pcm_exit); MODULE_AUTHOR("Stephen Warren swarren@nvidia.com"); MODULE_DESCRIPTION("Tegra PCM ASoC driver"); MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:" DRV_NAME);
On Thu, 2011-02-10 at 15:37 -0700, Stephen Warren wrote:
- There is no hysteresis enable field in the current datasheet.
- Mic detection threshold field is only 2 bits wide.
Signed-off-by: Stephen Warren swarren@nvidia.com
All
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
On Sun, Feb 13, 2011 at 04:21:47PM +0000, Liam Girdwood wrote:
All
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
Applied, thanks.
participants (3)
-
Liam Girdwood -
Mark Brown -
Stephen Warren