Re: [PATCH 3/3] ASoC: Add support for AIF channel muxing on WM8903

Re: [PATCH 3/3] ASoC: Add support for AIF channel muxing on WM8903

[Stephen Warren]

Stephen Warren wrote at Wednesday, February 09, 2011 9:33 AM:
> 
> Mark Brown wrote at Wednesday, February 09, 2011 6:47 AM:
> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > ---
> >  sound/soc/codecs/wm8903.c |   66 ++++++++++++++++++++++++++++++++++++++++++---
> >  1 files changed, 62 insertions(+), 4 deletions(-)
> 
> ...I'll make sure to test it within the next few days, once it's applied.

Mark,

I did find one issue with this.

With the default Left/Right Capture Mux settings, and the patch series I just
posted, mic detection now works fine in all the scenarios I tested:

1) Mic plugged in before loading driver, or not.
2) Mic plugged in when starting to capture, or not.
3) Mic removed and plugged back in while capture active.

However, if I flip the Right Capture Mux to setting Left (while not capturing),
then this breaks case (3) above; the initial portion of the recording is fine,
but when I remove and plug the mic back in, something isn't turned on, and so
nothing is recorded after that.

I haven't had a chance to look into this yet, but can later; probably on
Saturday evening.
 
-- 
nvpublic

Re: [PATCH 3/3] ASoC: Add support for AIF channel muxing on WM8903

[Stephen Warren]

Stephen Warren wrote at Thursday, February 10, 2011 3:59 PM:
> Stephen Warren wrote at Wednesday, February 09, 2011 9:33 AM:
> >
> > ...I'll make sure to test it within the next few days, once it's
> > applied.
> 
> Mark,
> 
> I did find one issue with this.
> ...
> However, if I flip the Right Capture Mux to setting Left (while not
> capturing),then this breaks case (3) above; the initial portion of the
> recording is fine, but when I remove and plug the mic back in, something
> isn't turned on, and so nothing is recorded after that.

So, what's happening is that once the mic is removed, there's no active path
in the whole codec, so everything gets shut down, including CLK_SYS.
In turn, this means that mic detection doesn't work, so when the mic gets
plugged back in, that event is not noticed, and nothing gets re-enabled.

If I am playing a stream at the same time as recording, that solves (hides)
the problem, since the codec is active and CLK_SYS stays enabled.

I found that if I revert:

2c8be5a26e42cfc4906c4daa8a5a5c82610ddb3d
ASoC: Dynamically manage CLK_SYS in WM8903

That also fixes the issue. I'm not entirely clear why though; perhaps there
are simply missing entries in the route map for CLK_SYS to mic-related
widgets?
 
-- 
nvpublic

Re: [PATCH 3/3] ASoC: Add support for AIF channel muxing on WM8903

[Mark Brown]

On Thu, Feb 10, 2011 at 09:40:37PM -0800, Stephen Warren wrote:

> So, what's happening is that once the mic is removed, there's no active path
> in the whole codec, so everything gets shut down, including CLK_SYS.
> In turn, this means that mic detection doesn't work, so when the mic gets
> plugged back in, that event is not noticed, and nothing gets re-enabled.

> If I am playing a stream at the same time as recording, that solves (hides)
> the problem, since the codec is active and CLK_SYS stays enabled.

> I found that if I revert:

> 2c8be5a26e42cfc4906c4daa8a5a5c82610ddb3d
> ASoC: Dynamically manage CLK_SYS in WM8903

> That also fixes the issue. I'm not entirely clear why though; perhaps there
> are simply missing entries in the route map for CLK_SYS to mic-related
> widgets?

There's a link from CLK_SYS to Mic Bias so if Mic Bias is enabled then
CLK_SYS should be kept up too.  However, it looks like the core DAPM
logic is getting this wrong and the force enable is only applying to the
immediate widget.  In the case of a supply widget this is clearly the
wrong thing to do.  I'll send out a patch for this shortly.

Re: [PATCH 3/3] ASoC: Add support for AIF channel muxing on WM8903

[Mark Brown]

On Thu, Feb 10, 2011 at 02:59:17PM -0800, Stephen Warren wrote:

> However, if I flip the Right Capture Mux to setting Left (while not capturing),
> then this breaks case (3) above; the initial portion of the recording is fine,
> but when I remove and plug the mic back in, something isn't turned on, and so
> nothing is recorded after that.

That's just bizzare; I can't off the top of my head think of any reason
for it and I'm out of the office today so can't test.

Does the left channel data stay OK (assuming you're mapping the data
through on both channels)?

[PATCH 1/3] ASoC: Remove redundant -codec from WM8903 driver name

[Mark Brown]

It causes noisy -codecs to appear in things like .codec_name.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm8903.c |    2 +-
 sound/soc/tegra/harmony.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c
index 27cd8f5..04c3256 100644
--- a/sound/soc/codecs/wm8903.c
+++ b/sound/soc/codecs/wm8903.c
@@ -2134,7 +2134,7 @@ MODULE_DEVICE_TABLE(i2c, wm8903_i2c_id);
 
 static struct i2c_driver wm8903_i2c_driver = {
 	.driver = {
-		.name = "wm8903-codec",
+		.name = "wm8903",
 		.owner = THIS_MODULE,
 	},
 	.probe =    wm8903_i2c_probe,
diff --git a/sound/soc/tegra/harmony.c b/sound/soc/tegra/harmony.c
index 11e2cb8..d702abe 100644
--- a/sound/soc/tegra/harmony.c
+++ b/sound/soc/tegra/harmony.c
@@ -189,7 +189,7 @@ static int harmony_asoc_init(struct snd_soc_pcm_runtime *rtd)
 static struct snd_soc_dai_link harmony_wm8903_dai = {
 	.name = "WM8903",
 	.stream_name = "WM8903 PCM",
-	.codec_name = "wm8903-codec.0-001a",
+	.codec_name = "wm8903.0-001a",
 	.platform_name = "tegra-pcm-audio",
 	.cpu_dai_name = "tegra-i2s.0",
 	.codec_dai_name = "wm8903-hifi",
-- 
1.7.2.3

[PATCH 3/3] ASoC: Add support for AIF channel muxing on WM8903

[Mark Brown]

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/codecs/wm8903.c |   66 ++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c
index caf41ad..b20611b 100644
--- a/sound/soc/codecs/wm8903.c
+++ b/sound/soc/codecs/wm8903.c
@@ -679,6 +679,22 @@ static const struct soc_enum lsidetone_enum =
 static const struct soc_enum rsidetone_enum =
 	SOC_ENUM_SINGLE(WM8903_DAC_DIGITAL_0, 0, 3, sidetone_text);
 
+static const char *aif_text[] = {
+	"Left", "Right"
+};
+
+static const struct soc_enum lcapture_enum =
+	SOC_ENUM_SINGLE(WM8903_AUDIO_INTERFACE_0, 7, 2, aif_text);
+
+static const struct soc_enum rcapture_enum =
+	SOC_ENUM_SINGLE(WM8903_AUDIO_INTERFACE_0, 6, 2, aif_text);
+
+static const struct soc_enum lplay_enum =
+	SOC_ENUM_SINGLE(WM8903_AUDIO_INTERFACE_0, 5, 2, aif_text);
+
+static const struct soc_enum rplay_enum =
+	SOC_ENUM_SINGLE(WM8903_AUDIO_INTERFACE_0, 4, 2, aif_text);
+
 static const struct snd_kcontrol_new wm8903_snd_controls[] = {
 
 /* Input PGAs - No TLV since the scale depends on PGA mode */
@@ -796,6 +812,18 @@ static const struct snd_kcontrol_new lsidetone_mux =
 static const struct snd_kcontrol_new rsidetone_mux =
 	SOC_DAPM_ENUM("DACR Sidetone Mux", rsidetone_enum);
 
+static const struct snd_kcontrol_new lcapture_mux =
+	SOC_DAPM_ENUM("Left Capture Mux", lcapture_enum);
+
+static const struct snd_kcontrol_new rcapture_mux =
+	SOC_DAPM_ENUM("Right Capture Mux", rcapture_enum);
+
+static const struct snd_kcontrol_new lplay_mux =
+	SOC_DAPM_ENUM("Left Playback Mux", lplay_enum);
+
+static const struct snd_kcontrol_new rplay_mux =
+	SOC_DAPM_ENUM("Right Playback Mux", rplay_enum);
+
 static const struct snd_kcontrol_new left_output_mixer[] = {
 SOC_DAPM_SINGLE("DACL Switch", WM8903_ANALOGUE_LEFT_MIX_0, 3, 1, 0),
 SOC_DAPM_SINGLE("DACR Switch", WM8903_ANALOGUE_LEFT_MIX_0, 2, 1, 0),
@@ -859,14 +887,26 @@ SND_SOC_DAPM_MUX("Right Input Mode Mux", SND_SOC_NOPM, 0, 0, &rinput_mode_mux),
 SND_SOC_DAPM_PGA("Left Input PGA", WM8903_POWER_MANAGEMENT_0, 1, 0, NULL, 0),
 SND_SOC_DAPM_PGA("Right Input PGA", WM8903_POWER_MANAGEMENT_0, 0, 0, NULL, 0),
 
-SND_SOC_DAPM_ADC("ADCL", "Left HiFi Capture", WM8903_POWER_MANAGEMENT_6, 1, 0),
-SND_SOC_DAPM_ADC("ADCR", "Right HiFi Capture", WM8903_POWER_MANAGEMENT_6, 0, 0),
+SND_SOC_DAPM_ADC("ADCL", NULL, WM8903_POWER_MANAGEMENT_6, 1, 0),
+SND_SOC_DAPM_ADC("ADCR", NULL, WM8903_POWER_MANAGEMENT_6, 0, 0),
+
+SND_SOC_DAPM_MUX("Left Capture Mux", SND_SOC_NOPM, 0, 0, &lcapture_mux),
+SND_SOC_DAPM_MUX("Right Capture Mux", SND_SOC_NOPM, 0, 0, &rcapture_mux),
+
+SND_SOC_DAPM_AIF_OUT("AIFTXL", "Left HiFi Capture", 0, SND_SOC_NOPM, 0, 0),
+SND_SOC_DAPM_AIF_OUT("AIFTXR", "Right HiFi Capture", 0, SND_SOC_NOPM, 0, 0),
 
 SND_SOC_DAPM_MUX("DACL Sidetone", SND_SOC_NOPM, 0, 0, &lsidetone_mux),
 SND_SOC_DAPM_MUX("DACR Sidetone", SND_SOC_NOPM, 0, 0, &rsidetone_mux),
 
-SND_SOC_DAPM_DAC("DACL", "Left Playback", WM8903_POWER_MANAGEMENT_6, 3, 0),
-SND_SOC_DAPM_DAC("DACR", "Right Playback", WM8903_POWER_MANAGEMENT_6, 2, 0),
+SND_SOC_DAPM_AIF_IN("AIFRXL", "Left Playback", 0, SND_SOC_NOPM, 0, 0),
+SND_SOC_DAPM_AIF_IN("AIFRXR", "Right Playback", 0, SND_SOC_NOPM, 0, 0),
+
+SND_SOC_DAPM_MUX("Left Playback Mux", SND_SOC_NOPM, 0, 0, &lplay_mux),
+SND_SOC_DAPM_MUX("Right Playback Mux", SND_SOC_NOPM, 0, 0, &rplay_mux),
+
+SND_SOC_DAPM_DAC("DACL", NULL, WM8903_POWER_MANAGEMENT_6, 3, 0),
+SND_SOC_DAPM_DAC("DACR", NULL, WM8903_POWER_MANAGEMENT_6, 2, 0),
 
 SND_SOC_DAPM_MIXER("Left Output Mixer", WM8903_POWER_MANAGEMENT_1, 1, 0,
 		   left_output_mixer, ARRAY_SIZE(left_output_mixer)),
@@ -948,18 +988,36 @@ static const struct snd_soc_dapm_route intercon[] = {
 	{ "Left Input PGA", NULL, "Left Input Mode Mux" },
 	{ "Right Input PGA", NULL, "Right Input Mode Mux" },
 
+	{ "Left Capture Mux", "Left", "ADCL" },
+	{ "Left Capture Mux", "Right", "ADCR" },
+
+	{ "Right Capture Mux", "Left", "ADCL" },
+	{ "Right Capture Mux", "Right", "ADCR" },
+
+	{ "AIFTXL", NULL, "Left Capture Mux" },
+	{ "AIFTXR", NULL, "Right Capture Mux" },
+
 	{ "ADCL", NULL, "Left Input PGA" },
 	{ "ADCL", NULL, "CLK_DSP" },
 	{ "ADCR", NULL, "Right Input PGA" },
 	{ "ADCR", NULL, "CLK_DSP" },
 
+	{ "Left Playback Mux", "Left", "AIFRXL" },
+	{ "Left Playback Mux", "Right", "AIFRXR" },
+
+	{ "Right Playback Mux", "Left", "AIFRXL" },
+	{ "Right Playback Mux", "Right", "AIFRXR" },
+
 	{ "DACL Sidetone", "Left", "ADCL" },
 	{ "DACL Sidetone", "Right", "ADCR" },
 	{ "DACR Sidetone", "Left", "ADCL" },
 	{ "DACR Sidetone", "Right", "ADCR" },
 
+	{ "DACL", NULL, "Left Playback Mux" },
 	{ "DACL", NULL, "DACL Sidetone" },
 	{ "DACL", NULL, "CLK_DSP" },
+
+	{ "DACR", NULL, "Right Playback Mux" },
 	{ "DACR", NULL, "DACR Sidetone" },
 	{ "DACR", NULL, "CLK_DSP" },
 
-- 
1.7.2.3

Re: [PATCH 3/3] ASoC: Add support for AIF channel muxing on WM8903

[Stephen Warren]

Mark Brown wrote at Wednesday, February 09, 2011 6:47 AM:
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  sound/soc/codecs/wm8903.c |   66 ++++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 62 insertions(+), 4 deletions(-)

Oh, thanks very much!

I was going to write this myself, and took a look at the wm8993(?) that you
mentioned over the weekend, but got side-tracked on other things. What you
wrote looks basically the same as what I was going to port over. I'll make
sure to test it within the next few days, once it's applied.

Thanks again.

> diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c
> index caf41ad..b20611b 100644
> --- a/sound/soc/codecs/wm8903.c
> +++ b/sound/soc/codecs/wm8903.c
> @@ -679,6 +679,22 @@ static const struct soc_enum lsidetone_enum =
>  static const struct soc_enum rsidetone_enum =
>  	SOC_ENUM_SINGLE(WM8903_DAC_DIGITAL_0, 0, 3, sidetone_text);
> 
> +static const char *aif_text[] = {
> +	"Left", "Right"
> +};
> +
> +static const struct soc_enum lcapture_enum =
> +	SOC_ENUM_SINGLE(WM8903_AUDIO_INTERFACE_0, 7, 2, aif_text);
> +
> +static const struct soc_enum rcapture_enum =
> +	SOC_ENUM_SINGLE(WM8903_AUDIO_INTERFACE_0, 6, 2, aif_text);
> +
> +static const struct soc_enum lplay_enum =
> +	SOC_ENUM_SINGLE(WM8903_AUDIO_INTERFACE_0, 5, 2, aif_text);
> +
> +static const struct soc_enum rplay_enum =
> +	SOC_ENUM_SINGLE(WM8903_AUDIO_INTERFACE_0, 4, 2, aif_text);
> +
>  static const struct snd_kcontrol_new wm8903_snd_controls[] = {
> 
>  /* Input PGAs - No TLV since the scale depends on PGA mode */
> @@ -796,6 +812,18 @@ static const struct snd_kcontrol_new lsidetone_mux =
>  static const struct snd_kcontrol_new rsidetone_mux =
>  	SOC_DAPM_ENUM("DACR Sidetone Mux", rsidetone_enum);
> 
> +static const struct snd_kcontrol_new lcapture_mux =
> +	SOC_DAPM_ENUM("Left Capture Mux", lcapture_enum);
> +
> +static const struct snd_kcontrol_new rcapture_mux =
> +	SOC_DAPM_ENUM("Right Capture Mux", rcapture_enum);
> +
> +static const struct snd_kcontrol_new lplay_mux =
> +	SOC_DAPM_ENUM("Left Playback Mux", lplay_enum);
> +
> +static const struct snd_kcontrol_new rplay_mux =
> +	SOC_DAPM_ENUM("Right Playback Mux", rplay_enum);
> +
>  static const struct snd_kcontrol_new left_output_mixer[] = {
>  SOC_DAPM_SINGLE("DACL Switch", WM8903_ANALOGUE_LEFT_MIX_0, 3, 1, 0),
>  SOC_DAPM_SINGLE("DACR Switch", WM8903_ANALOGUE_LEFT_MIX_0, 2, 1, 0),
> @@ -859,14 +887,26 @@ SND_SOC_DAPM_MUX("Right Input Mode Mux",
> SND_SOC_NOPM, 0, 0, &rinput_mode_mux),
>  SND_SOC_DAPM_PGA("Left Input PGA", WM8903_POWER_MANAGEMENT_0, 1, 0, NULL,
> 0),
>  SND_SOC_DAPM_PGA("Right Input PGA", WM8903_POWER_MANAGEMENT_0, 0, 0,
> NULL, 0),
> 
> -SND_SOC_DAPM_ADC("ADCL", "Left HiFi Capture", WM8903_POWER_MANAGEMENT_6,
> 1, 0),
> -SND_SOC_DAPM_ADC("ADCR", "Right HiFi Capture", WM8903_POWER_MANAGEMENT_6,
> 0, 0),
> +SND_SOC_DAPM_ADC("ADCL", NULL, WM8903_POWER_MANAGEMENT_6, 1, 0),
> +SND_SOC_DAPM_ADC("ADCR", NULL, WM8903_POWER_MANAGEMENT_6, 0, 0),
> +
> +SND_SOC_DAPM_MUX("Left Capture Mux", SND_SOC_NOPM, 0, 0, &lcapture_mux),
> +SND_SOC_DAPM_MUX("Right Capture Mux", SND_SOC_NOPM, 0, 0, &rcapture_mux),
> +
> +SND_SOC_DAPM_AIF_OUT("AIFTXL", "Left HiFi Capture", 0, SND_SOC_NOPM, 0,
> 0),
> +SND_SOC_DAPM_AIF_OUT("AIFTXR", "Right HiFi Capture", 0, SND_SOC_NOPM, 0,
> 0),
> 
>  SND_SOC_DAPM_MUX("DACL Sidetone", SND_SOC_NOPM, 0, 0, &lsidetone_mux),
>  SND_SOC_DAPM_MUX("DACR Sidetone", SND_SOC_NOPM, 0, 0, &rsidetone_mux),
> 
> -SND_SOC_DAPM_DAC("DACL", "Left Playback", WM8903_POWER_MANAGEMENT_6, 3,
> 0),
> -SND_SOC_DAPM_DAC("DACR", "Right Playback", WM8903_POWER_MANAGEMENT_6, 2,
> 0),
> +SND_SOC_DAPM_AIF_IN("AIFRXL", "Left Playback", 0, SND_SOC_NOPM, 0, 0),
> +SND_SOC_DAPM_AIF_IN("AIFRXR", "Right Playback", 0, SND_SOC_NOPM, 0, 0),
> +
> +SND_SOC_DAPM_MUX("Left Playback Mux", SND_SOC_NOPM, 0, 0, &lplay_mux),
> +SND_SOC_DAPM_MUX("Right Playback Mux", SND_SOC_NOPM, 0, 0, &rplay_mux),
> +
> +SND_SOC_DAPM_DAC("DACL", NULL, WM8903_POWER_MANAGEMENT_6, 3, 0),
> +SND_SOC_DAPM_DAC("DACR", NULL, WM8903_POWER_MANAGEMENT_6, 2, 0),
> 
>  SND_SOC_DAPM_MIXER("Left Output Mixer", WM8903_POWER_MANAGEMENT_1, 1, 0,
>  		   left_output_mixer, ARRAY_SIZE(left_output_mixer)),
> @@ -948,18 +988,36 @@ static const struct snd_soc_dapm_route intercon[] =
> {
>  	{ "Left Input PGA", NULL, "Left Input Mode Mux" },
>  	{ "Right Input PGA", NULL, "Right Input Mode Mux" },
> 
> +	{ "Left Capture Mux", "Left", "ADCL" },
> +	{ "Left Capture Mux", "Right", "ADCR" },
> +
> +	{ "Right Capture Mux", "Left", "ADCL" },
> +	{ "Right Capture Mux", "Right", "ADCR" },
> +
> +	{ "AIFTXL", NULL, "Left Capture Mux" },
> +	{ "AIFTXR", NULL, "Right Capture Mux" },
> +
>  	{ "ADCL", NULL, "Left Input PGA" },
>  	{ "ADCL", NULL, "CLK_DSP" },
>  	{ "ADCR", NULL, "Right Input PGA" },
>  	{ "ADCR", NULL, "CLK_DSP" },
> 
> +	{ "Left Playback Mux", "Left", "AIFRXL" },
> +	{ "Left Playback Mux", "Right", "AIFRXR" },
> +
> +	{ "Right Playback Mux", "Left", "AIFRXL" },
> +	{ "Right Playback Mux", "Right", "AIFRXR" },
> +
>  	{ "DACL Sidetone", "Left", "ADCL" },
>  	{ "DACL Sidetone", "Right", "ADCR" },
>  	{ "DACR Sidetone", "Left", "ADCL" },
>  	{ "DACR Sidetone", "Right", "ADCR" },
> 
> +	{ "DACL", NULL, "Left Playback Mux" },
>  	{ "DACL", NULL, "DACL Sidetone" },
>  	{ "DACL", NULL, "CLK_DSP" },
> +
> +	{ "DACR", NULL, "Right Playback Mux" },
>  	{ "DACR", NULL, "DACR Sidetone" },
>  	{ "DACR", NULL, "CLK_DSP" },
> 
> --
> 1.7.2.3

-- 
nvpublic