[PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[Stephen Warren]

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 sound/soc/tegra/harmony.c |   30 +++++++++++++++++++++++++++++-
 1 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c
index 1b80c4f..26ab4fd 100644
--- a/sound/soc/tegra/harmony.c
+++ b/sound/soc/tegra/harmony.c
@@ -38,6 +38,7 @@
 #include <mach/harmony_audio.h>
 
 #include <sound/core.h>
+#include <sound/jack.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -123,6 +124,24 @@ static struct snd_soc_ops harmony_asoc_ops = {
 	.hw_params = harmony_asoc_hw_params,
 };
 
+static struct snd_soc_jack harmony_hp_jack;
+
+static struct snd_soc_jack_pin harmony_hp_jack_pins[] = {
+	{
+		.pin = "Headphone Jack",
+		.mask = SND_JACK_HEADPHONE,
+	},
+};
+
+static struct snd_soc_jack_gpio harmony_hp_jack_gpios[] = {
+	{
+		.name = "headphone detect",
+		.report = SND_JACK_HEADPHONE,
+		.debounce_time = 150,
+		.invert = 1,
+	}
+};
+
 static int harmony_event_int_spk(struct snd_soc_dapm_widget *w,
 					struct snd_kcontrol *k, int event)
 {
@@ -178,11 +197,20 @@ 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, "Headphone Jack");
 	snd_soc_dapm_enable_pin(dapm, "Int Spk");
 	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);
+	snd_soc_jack_add_pins(&harmony_hp_jack,
+			      ARRAY_SIZE(harmony_hp_jack_pins),
+			      harmony_hp_jack_pins);
+	snd_soc_jack_add_gpios(&harmony_hp_jack,
+			       ARRAY_SIZE(harmony_hp_jack_gpios),
+			       harmony_hp_jack_gpios);
+
 	return 0;
 }
 
-- 
1.7.1

[PATCH 2/6] ASoC: Tegra: Harmony: Add switch control for speaker

[Stephen Warren]

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 sound/soc/tegra/harmony.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c
index 26ab4fd..e87395c 100644
--- a/sound/soc/tegra/harmony.c
+++ b/sound/soc/tegra/harmony.c
@@ -173,6 +173,10 @@ static const struct snd_soc_dapm_route harmony_audio_map[] = {
 	{"IN1L", NULL, "Mic Bias"},
 };
 
+static const struct snd_kcontrol_new harmony_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Int Spk"),
+};
+
 static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_codec *codec = rtd->codec;
@@ -191,13 +195,17 @@ static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd)
 
 	gpio_direction_output(pdata->gpio_spkr_en, 0);
 
+	ret = snd_soc_add_controls(codec, harmony_controls,
+				   ARRAY_SIZE(harmony_controls));
+	if (ret < 0)
+		return ret;
+
 	snd_soc_dapm_new_controls(dapm, harmony_dapm_widgets,
 					ARRAY_SIZE(harmony_dapm_widgets));
 
 	snd_soc_dapm_add_routes(dapm, harmony_audio_map,
 				ARRAY_SIZE(harmony_audio_map));
 
-	snd_soc_dapm_enable_pin(dapm, "Int Spk");
 	snd_soc_dapm_enable_pin(dapm, "Mic Jack");
 	snd_soc_dapm_sync(dapm);
 
-- 
1.7.1

[PATCH 3/6] ASoC: WM8903: Fix mic detection issues

[Stephen Warren]

wm8903.h:
* There is no hysteresis enable field in the current datasheet.
* Mic detection threshold field is only 2 bits wide.

wm8903.c:
* Mic detection HW should be enabled if SW wants either mic detection or
  short detection, rather than only when it wants both.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 include/sound/wm8903.h    |   10 +++-------
 sound/soc/codecs/wm8903.c |    2 +-
 2 files changed, 4 insertions(+), 8 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] */
diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c
index 3d4c55f..dc96573 100644
--- a/sound/soc/codecs/wm8903.c
+++ b/sound/soc/codecs/wm8903.c
@@ -1489,7 +1489,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,
-- 
1.7.1

[PATCH 4/6] ASoC: Tegra: Harmony: Implement mic detection

[Stephen Warren]

* Add jack definition for mic jack
* Request wm8903 to enable mic detection
* Remove Mic Bias from DAPM route map, so that Mic Bias isn't disable
  when capture is stopped, thus preventing further mic detection from
  operating.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 sound/soc/tegra/harmony.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c
index e87395c..799b1ef 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)
 {
@@ -169,8 +180,7 @@ static const struct snd_soc_dapm_route harmony_audio_map[] = {
 	{"Int Spk", NULL, "RON"},
 	{"Int Spk", NULL, "LOP"},
 	{"Int Spk", NULL, "LON"},
-	{"Mic Bias", NULL, "Mic Jack"},
-	{"IN1L", NULL, "Mic Bias"},
+	{"IN1L", NULL, "Mic Jack"},
 };
 
 static const struct snd_kcontrol_new harmony_controls[] = {
@@ -206,9 +216,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 +226,13 @@ 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);
+
 	return 0;
 }
 
-- 
1.7.1

[PATCH 5/6] ASoC: Harmony: Call snd_soc_dapm_nc_pin

[Stephen Warren]

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 sound/soc/tegra/harmony.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c
index 799b1ef..8ed88e3 100644
--- a/sound/soc/tegra/harmony.c
+++ b/sound/soc/tegra/harmony.c
@@ -205,6 +205,12 @@ static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd)
 
 	gpio_direction_output(pdata->gpio_spkr_en, 0);
 
+	/* Not connected */
+	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");
+
 	ret = snd_soc_add_controls(codec, harmony_controls,
 				   ARRAY_SIZE(harmony_controls));
 	if (ret < 0)
-- 
1.7.1

[PATCH 6/6] ASoC: Tegra: Harmony: Support both int and ext mics

[Stephen Warren]

Note that the external mic is only connected to the codec's left channel,
and the internal mic is only connected to the codec's right channel.

Hence, when recording a stereo stream, which is the only supported channel
configuration, the recorded audio will only appear within one channel, and
which channel that is will differ depending on which mic is used.

At present, I'm not sure how best to resolve this. Downstream drivers
directly tweaked the wm8903's registers so that both ADCs processed the
same input channel.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 sound/soc/tegra/harmony.c |   88 +++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c
index 8ed88e3..b4526c4 100644
--- a/sound/soc/tegra/harmony.c
+++ b/sound/soc/tegra/harmony.c
@@ -52,10 +52,19 @@
 
 #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)
+
+#define MIC_SELECTION_OFF     0
+#define MIC_SELECTION_EXT_MIC 1
+#define MIC_SELECTION_INT_MIC 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;
+	int mic_selection;
 };
 
 static int harmony_asoc_hw_params(struct snd_pcm_substream *substream,
@@ -181,10 +190,61 @@ static const struct snd_soc_dapm_route harmony_audio_map[] = {
 	{"Int Spk", NULL, "LOP"},
 	{"Int Spk", NULL, "LON"},
 	{"IN1L", NULL, "Mic Jack"},
+	{"IN1R", NULL, "Mic Jack"},
+};
+
+static void harmony_mic_control(struct snd_soc_codec *codec)
+{
+	struct snd_soc_card *card = codec->card;
+	struct tegra_harmony *harmony = snd_soc_card_get_drvdata(card);
+	struct harmony_audio_platform_data *pdata = harmony->pdata;
+	int int_mic_en = harmony->mic_selection == MIC_SELECTION_INT_MIC;
+	int ext_mic_en = harmony->mic_selection == MIC_SELECTION_EXT_MIC;
+
+	gpio_set_value_cansleep(pdata->gpio_int_mic_en, int_mic_en);
+	/* ext_mic_en is active low  */
+	gpio_set_value_cansleep(pdata->gpio_ext_mic_en, !ext_mic_en);
+}
+
+static int harmony_get_mic_selection(struct snd_kcontrol *kcontrol,
+				     struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_card *card = codec->card;
+	struct tegra_harmony *harmony = snd_soc_card_get_drvdata(card);
+
+	ucontrol->value.integer.value[0] = harmony->mic_selection;
+
+	return 0;
+}
+
+static int harmony_set_mic_selection(struct snd_kcontrol *kcontrol,
+				     struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct snd_soc_card *card = codec->card;
+	struct tegra_harmony *harmony = snd_soc_card_get_drvdata(card);
+
+	if (harmony->mic_selection == ucontrol->value.integer.value[0])
+		return 0;
+
+	harmony->mic_selection = ucontrol->value.integer.value[0];
+	harmony_mic_control(codec);
+
+	return 1;
+}
+
+static const char * const mic_selection_names[] = {"Off", "External",
+						   "Internal"};
+static const struct soc_enum harmony_enum[] = {
+	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(mic_selection_names),
+			    mic_selection_names),
 };
 
 static const struct snd_kcontrol_new harmony_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Int Spk"),
+	SOC_ENUM_EXT("Mic Selection", harmony_enum[0],
+		     harmony_get_mic_selection, harmony_set_mic_selection),
 };
 
 static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd)
@@ -201,10 +261,28 @@ 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;
+
+	gpio_direction_output(pdata->gpio_int_mic_en, 1);
+
+	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;
+
+	gpio_direction_output(pdata->gpio_ext_mic_en, 0);
+
 	/* Not connected */
 	snd_soc_dapm_nc_pin(dapm, "IN3L");
 	snd_soc_dapm_nc_pin(dapm, "IN3R");
@@ -326,7 +404,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);
-- 
1.7.1

Re: [PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[Mark Brown]

On Thu, Feb 03, 2011 at 01:56:13PM -0700, Stephen Warren wrote:

> +static struct snd_soc_jack harmony_hp_jack;
> +

Since you've changed to using a platform device you should really be
dynamically allocating this I guess.  But this isn't actually a
practical problem so not worth caring about.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] ASoC: WM8903: Fix mic detection issues

[Mark Brown]

On Thu, Feb 03, 2011 at 01:56:15PM -0700, Stephen Warren wrote:
> wm8903.h:
> * There is no hysteresis enable field in the current datasheet.
> * Mic detection threshold field is only 2 bits wide.
> 
> wm8903.c:
> * Mic detection HW should be enabled if SW wants either mic detection or
>   short detection, rather than only when it wants both.

Please don't combine unrelated changes into a single patch, especially
in cases like this where one is a bug fix and the other is essentially a
cosmetic fixup.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/6] ASoC: Tegra: Harmony: Implement mic detection

[Mark Brown]

On Thu, Feb 03, 2011 at 01:56:16PM -0700, Stephen Warren wrote:

> * Remove Mic Bias from DAPM route map, so that Mic Bias isn't disable
>   when capture is stopped, thus preventing further mic detection from
>   operating.

You should be using snd_soc_dapm_force_enable_pin() to do this.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] ASoC: Tegra: Harmony: Support both int and ext mics

[Mark Brown]

On Thu, Feb 03, 2011 at 01:56:18PM -0700, Stephen Warren wrote:

> At present, I'm not sure how best to resolve this. Downstream drivers
> directly tweaked the wm8903's registers so that both ADCs processed the
> same input channel.

The CODEC driver needs to have support for routing the left and right
channels like things like the WM8993 do.

>  	{"IN1L", NULL, "Mic Jack"},
> +	{"IN1R", NULL, "Mic Jack"},

This looks odd - I'd expect to see separate widgets for the internal
microphone rather than using the same widget.  This would greatly
simplify the driver code without really impacting the level of
configuration applications need to do (and allowing them to use both
simultaneously if they want to).

> +static int harmony_set_mic_selection(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
> +	struct snd_soc_card *card = codec->card;
> +	struct tegra_harmony *harmony = snd_soc_card_get_drvdata(card);
> +
> +	if (harmony->mic_selection == ucontrol->value.integer.value[0])
> +		return 0;
> +
> +	harmony->mic_selection = ucontrol->value.integer.value[0];
> +	harmony_mic_control(codec);

This should be locking the CODEC mutex while doing the update.

> +static const struct soc_enum harmony_enum[] = {
> +	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(mic_selection_names),
> +			    mic_selection_names),
>  };

Don't create arrays of enums, declare individual varaibles and reference
them...

> +	SOC_ENUM_EXT("Mic Selection", harmony_enum[0],
> +		     harmony_get_mic_selection, harmony_set_mic_selection),

...as it makes the references to them much easier to follow and less
error prone.

RE: [PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[Stephen Warren]

Mark Brown wrote at Thursday, February 03, 2011 3:36 PM:
> On Thu, Feb 03, 2011 at 01:56:13PM -0700, Stephen Warren wrote:
> 
> > +static struct snd_soc_jack harmony_hp_jack;
> > +
> 
> Since you've changed to using a platform device you should really be
> dynamically allocating this I guess.  But this isn't actually a
> practical problem so not worth caring about.

Uggh. The code may as well be consistent. I'll fix this up and resubmit
if you haven't already applied it.

-- 
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] ASoC: Tegra: Harmony: Add switch control for speaker

[Mark Brown]

On Thu, Feb 03, 2011 at 01:56:14PM -0700, Stephen Warren wrote:
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

This looks good but won't apply without patch 1 so please resubmit.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] ASoC: Harmony: Call snd_soc_dapm_nc_pin

[Mark Brown]

On Thu, Feb 03, 2011 at 01:56:17PM -0700, Stephen Warren wrote:
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Similarly here.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[Mark Brown]

On Thu, Feb 03, 2011 at 03:25:04PM -0800, Stephen Warren wrote:

> Uggh. The code may as well be consistent. I'll fix this up and resubmit
> if you haven't already applied it.

Except when he's on holiday or otherwise unavailable I won't generally
apply anything unless it's an emergency bugfix until Liam has also
reviewed it.

RE: [PATCH 6/6] ASoC: Tegra: Harmony: Support both int and ext mics

[Stephen Warren]

Mark Brown wrote at Thursday, February 03, 2011 3:53 PM:
> On Thu, Feb 03, 2011 at 01:56:18PM -0700, Stephen Warren wrote:
> 
> > At present, I'm not sure how best to resolve this. Downstream drivers
> > directly tweaked the wm8903's registers so that both ADCs processed the
> > same input channel.
> 
> The CODEC driver needs to have support for routing the left and right
> channels like things like the WM8993 do.
> 
> >  	{"IN1L", NULL, "Mic Jack"},
> > +	{"IN1R", NULL, "Mic Jack"},
> 
> This looks odd - I'd expect to see separate widgets for the internal
> microphone rather than using the same widget.

Yes, separate widgets would make sense.

However, I wonder how this interacts with mic detection using micbias current.
There is only one mic bias signal, routed to a subset of the mics using the
two per-mic enable GPIOs. Should I just hook up the mic bias detection to
both jacks somehow, or is the only option to just punt on jack detection?

There are no physical plug detection mechanisms on this board.

> This would greatly simplify the driver code without really impacting the
> level of configuration applications need to do (and allowing them to use
> both simultaneously if they want to).

I guess you could, although even with L/R routing implemented for the
single-mic case, I think the only option for dual-mic would be to capture
each mic on its own channel, which seems like an unlikely use case to me.
But who knows, maybe it could be useful for e.g. noise-cancelling in SW?

-- 
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[Stephen Warren]

Stephen Warren wrote at Thursday, February 03, 2011 4:25 PM:
> 
> Mark Brown wrote at Thursday, February 03, 2011 3:36 PM:
> > On Thu, Feb 03, 2011 at 01:56:13PM -0700, Stephen Warren wrote:
> >
> > > +static struct snd_soc_jack harmony_hp_jack;
> > > +
> >
> > Since you've changed to using a platform device you should really be
> > dynamically allocating this I guess.  But this isn't actually a
> > practical problem so not worth caring about.
> 
> Uggh. The code may as well be consistent. I'll fix this up and resubmit
> if you haven't already applied it.

Hmm. Looking at this a bit more, solving it fully is kinda nasty; I'd have
to move not only that jack definition, but also all the pins and gpios into
struct tegra_harmony, since snd_soc_jack_add_{pins,gpios} modify all of
those structures. But, there would still have to be static const "template"
copies of those data structures to initialize the copies in struct
tegra_harmony.

The same thing then applies to some of the subsequent changes, for the
mic jacks etc.

That seems like a lot of overhead, both code-wise, and runtime space-wise,
to solve a problem that as you mention isn't a practical concern.

-- 
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 6/6] ASoC: Tegra: Harmony: Support both int and ext mics

[Mark Brown]

On Fri, Feb 04, 2011 at 09:20:26AM -0800, Stephen Warren wrote:

> However, I wonder how this interacts with mic detection using micbias current.
> There is only one mic bias signal, routed to a subset of the mics using the
> two per-mic enable GPIOs. Should I just hook up the mic bias detection to
> both jacks somehow, or is the only option to just punt on jack detection?

> There are no physical plug detection mechanisms on this board.

That's a fairly unusual hardware design, it really needs explaining in
the code - it wasn't at all obvious that you were controlling a FET
reading the patch (and I'm still not entirely clear how exactly things
are wired up).  The simplest way to handle this is probably with a
conditional DAPM route (look at how WM8994 handles SYSCLK for an example
of this), though you'll need to faff around a bit.

Ideally you want the micbias to be enabled all the time for detection of
jack insertion, only flipping the bias over to the internal mic when the
internal mic is being used for recording, and also disable the detection
when the internal microphone is in use.  I'm not sure that this is worth
the bother.

> > This would greatly simplify the driver code without really impacting the
> > level of configuration applications need to do (and allowing them to use
> > both simultaneously if they want to).

> I guess you could, although even with L/R routing implemented for the
> single-mic case, I think the only option for dual-mic would be to capture
> each mic on its own channel, which seems like an unlikely use case to me.
> But who knows, maybe it could be useful for e.g. noise-cancelling in SW?

Most likely would seem to be using the mic jack as a line input for some
purpose and combining it with the results of the internal microphone.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [alsa-devel] [PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[Stephen Warren]

Stephen Warren wrote at Friday, February 04, 2011 1:32 PM:
> Stephen Warren wrote at Thursday, February 03, 2011 4:25 PM:
> >
> > Mark Brown wrote at Thursday, February 03, 2011 3:36 PM:
> > > On Thu, Feb 03, 2011 at 01:56:13PM -0700, Stephen Warren wrote:
> > >
> > > > +static struct snd_soc_jack harmony_hp_jack;
> > > > +
> > >
> > > Since you've changed to using a platform device you should really be
> > > dynamically allocating this I guess.  But this isn't actually a
> > > practical problem so not worth caring about.
> >
> > Uggh. The code may as well be consistent. I'll fix this up and resubmit
> > if you haven't already applied it.
> 
> Hmm. Looking at this a bit more, solving it fully is kinda nasty; I'd have
> to move not only that jack definition, but also all the pins and gpios
> into struct tegra_harmony, since snd_soc_jack_add_{pins,gpios} modify all
> of those structures. But, there would still have to be static const
> "template" copies of those data structures to initialize the copies in
> struct tegra_harmony.
> 
> The same thing then applies to some of the subsequent changes, for the
> mic jacks etc.
> 
> That seems like a lot of overhead, both code-wise, and runtime space-wise,
> to solve a problem that as you mention isn't a practical concern.

Mark, I'm happy to go either way on this; the SW engineer in me doesn't like
non-const globals and inconsistency with the rest of the state data, but
equally fixing this seems like it would bloat the code and might make it
less readable.

Let me know which way you'd prefer it to go.

Thanks.

--
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [alsa-devel] [PATCH 6/6] ASoC: Tegra: Harmony: Support both int and ext mics

[Stephen Warren]

Mark Brown wrote at Monday, February 07, 2011 4:13 AM:
> On Fri, Feb 04, 2011 at 09:20:26AM -0800, Stephen Warren wrote:
> 
> > However, I wonder how this interacts with mic detection using micbias
> > current.  There is only one mic bias signal, routed to a subset of
> > the mics using the two per-mic enable GPIOs. Should I just hook up
> > the mic bias detection to both jacks somehow, or is the only option
> > to just punt on jack detection?
>  >
> > There are no physical plug detection mechanisms on this board.
> 
> That's a fairly unusual hardware design, it really needs explaining in
> the code - it wasn't at all obvious that you were controlling a FET
> reading the patch (and I'm still not entirely clear how exactly things
> are wired up).  The simplest way to handle this is probably with a
> conditional DAPM route (look at how WM8994 handles SYSCLK for an example
> of this), though you'll need to faff around a bit.
> 
> Ideally you want the micbias to be enabled all the time for detection of
> jack insertion, only flipping the bias over to the internal mic when the
> internal mic is being used for recording, and also disable the detection
> when the internal microphone is in use.  I'm not sure that this is worth
> the bother.

Maybe I should just not implement mic detection for Harmony due to these
issues? In which case, I'd just let mic bias be enabled whenever capture
was active, just like the speaker enable GPIO.

Either way, I suppose I should add two explicit mic widgets, each directly
driving the appropriate GPIO, so that the user can select which mic(s)
actually get used for capture.

Does that sound reasonable?

Thansk.

-- 
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 6/6] ASoC: Tegra: Harmony: Support both int and ext mics

[Mark Brown]

On Tue, Feb 08, 2011 at 12:34:09PM -0800, Stephen Warren wrote:

> Maybe I should just not implement mic detection for Harmony due to these
> issues? In which case, I'd just let mic bias be enabled whenever capture
> was active, just like the speaker enable GPIO.

It certainly seems way more trouble than it's worth: as soon as the
on board mic is used the jack detection will get confused as it'll see
it.  Of course, the internal mic is actually plugged in to a header on
the main board so it may be absent too (I believe a lot of units were
shipped without a case...) which means detecting the internal mic might
actually be useful :)

> Either way, I suppose I should add two explicit mic widgets, each directly
> driving the appropriate GPIO, so that the user can select which mic(s)
> actually get used for capture.

> Does that sound reasonable?

I think that's the best approach.  If someone wants to do mic detection
they can always add it later.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[Mark Brown]

On Tue, Feb 08, 2011 at 12:28:48PM -0800, Stephen Warren wrote:

> Mark, I'm happy to go either way on this; the SW engineer in me doesn't like
> non-const globals and inconsistency with the rest of the state data, but
> equally fixing this seems like it would bloat the code and might make it
> less readable.

> Let me know which way you'd prefer it to go.

I think your current patch is fine.  Liam?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[Liam Girdwood]

On Tue, 2011-02-08 at 20:59 +0000, Mark Brown wrote:
> On Tue, Feb 08, 2011 at 12:28:48PM -0800, Stephen Warren wrote:
> 
> > Mark, I'm happy to go either way on this; the SW engineer in me doesn't like
> > non-const globals and inconsistency with the rest of the state data, but
> > equally fixing this seems like it would bloat the code and might make it
> > less readable.
> 
> > Let me know which way you'd prefer it to go.
> 
> I think your current patch is fine.  Liam?

Yeah, lets go with this.

Acked-by: Liam Girdwood <lrg-kDsPt+C1G03kYMGBc/C6ZA@public.gmane.org>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [alsa-devel] [PATCH 6/6] ASoC: Tegra: Harmony: Support both int and ext mics

[Stephen Warren]

Mark Brown wrote at Tuesday, February 08, 2011 1:59 PM:
> On Tue, Feb 08, 2011 at 12:34:09PM -0800, Stephen Warren wrote:
> 
> > Maybe I should just not implement mic detection for Harmony due to these
> > issues? In which case, I'd just let mic bias be enabled whenever capture
> > was active, just like the speaker enable GPIO.
> 
> It certainly seems way more trouble than it's worth: as soon as the
> on board mic is used the jack detection will get confused as it'll see
> it.  Of course, the internal mic is actually plugged in to a header on
> the main board so it may be absent too (I believe a lot of units were
> shipped without a case...) which means detecting the internal mic might
> actually be useful :)

Well, one other option is to completely ignore (and disable) the internal
mic, and just support the external mic (jack). Then, mic detection could
be implemented. The advantage here is similarity to other boards without
the mic bias sharing issue. Plus, the jack is probably far more useful
than the internal mic on this board:

I honestly don't know how many boards (if any) actually have anything
plugged into the internal mic header on the board. Mine certainly
didn't originally; I used a PC speaker I stole from somewhere else as
the internal mic;-)

-- 
nvpublic

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 1/6] ASoC: Tegra: Harmony: Add headphone jack detection

[Mark Brown]

On Thu, Feb 03, 2011 at 01:56:13PM -0700, Stephen Warren wrote:
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 2/6] ASoC: Tegra: Harmony: Add switch control for speaker

[Mark Brown]

On Thu, Feb 03, 2011 at 01:56:14PM -0700, Stephen Warren wrote:
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alsa-devel] [PATCH 6/6] ASoC: Tegra: Harmony: Support both int and ext mics

[Mark Brown]

On Tue, Feb 08, 2011 at 03:32:00PM -0800, Stephen Warren wrote:

> Well, one other option is to completely ignore (and disable) the internal
> mic, and just support the external mic (jack). Then, mic detection could
> be implemented. The advantage here is similarity to other boards without
> the mic bias sharing issue. Plus, the jack is probably far more useful
> than the internal mic on this board:

> I honestly don't know how many boards (if any) actually have anything
> plugged into the internal mic header on the board. Mine certainly
> didn't originally; I used a PC speaker I stole from somewhere else as
> the internal mic;-)

That's certainly a sensible option if we don't know that many systems
were shipped with the internal mic connected.  We could do something
like what the AT91SAM9G20 platform does with ifdefed code for the mic
(in that case there's no mic support in the hardware but the CODEC is
in a low density package so it's relatively common for users to lift
pins to flying wire in a mic) I guess.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html