[PATCH 1/3] ASoC: sgtl5000: prevent playback to be muted when terminating concurrent capture

[PATCH 1/3] ASoC: sgtl5000: prevent playback to be muted when terminating concurrent capture

[Lothar Waßmann]

When a sound capture/playback is terminated while a playback/capture
is running, power_vag_event() will clear SGTL5000_CHIP_ANA_POWER in
the SND_SOC_DAPM_PRE_PMD event, thus muting the respective other
channel.

Don't clear SGTL5000_CHIP_ANA_POWER when both DAC and ADC are active
to prevent this.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 sound/soc/codecs/sgtl5000.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index c8b9090..a4b2973 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -153,6 +153,8 @@ static int mic_bias_event(struct snd_soc_dapm_widget *w,
 static int power_vag_event(struct snd_soc_dapm_widget *w,
 	struct snd_kcontrol *kcontrol, int event)
 {
+	const u32 mask = SGTL5000_DAC_POWERUP | SGTL5000_ADC_POWERUP;
+
 	switch (event) {
 	case SND_SOC_DAPM_POST_PMU:
 		snd_soc_update_bits(w->codec, SGTL5000_CHIP_ANA_POWER,
@@ -160,9 +162,17 @@ static int power_vag_event(struct snd_soc_dapm_widget *w,
 		break;
 
 	case SND_SOC_DAPM_PRE_PMD:
-		snd_soc_update_bits(w->codec, SGTL5000_CHIP_ANA_POWER,
-			SGTL5000_VAG_POWERUP, 0);
-		msleep(400);
+		/*
+		 * Don't clear VAG_POWERUP, when both DAC and ADC are
+		 * operational to prevent inadvertently starving the
+		 * other one of them.
+		 */
+		if ((snd_soc_read(w->codec, SGTL5000_CHIP_ANA_POWER) &
+				mask) != mask) {
+			snd_soc_update_bits(w->codec, SGTL5000_CHIP_ANA_POWER,
+				SGTL5000_VAG_POWERUP, 0);
+			msleep(400);
+		}
 		break;
 	default:
 		break;
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 2/3] ASoC: sgtl5000: fix buggy 'Capture Attenuate Switch' control

[Lothar Waßmann]

The SGTL5000 Capture Attenuate Switch (or "ADC Volume Range Reduction"
as it is called in the manual) is single bit only.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 sound/soc/codecs/sgtl5000.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index a4b2973..ba5ca67 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -398,7 +398,7 @@ static const struct snd_kcontrol_new sgtl5000_snd_controls[] = {
 	SOC_DOUBLE("Capture Volume", SGTL5000_CHIP_ANA_ADC_CTRL, 0, 4, 0xf, 0),
 	SOC_SINGLE_TLV("Capture Attenuate Switch (-6dB)",
 			SGTL5000_CHIP_ANA_ADC_CTRL,
-			8, 2, 0, capture_6db_attenuate),
+			8, 1, 0, capture_6db_attenuate),
 	SOC_SINGLE("Capture ZC Switch", SGTL5000_CHIP_ANA_CTRL, 1, 1, 0),
 
 	SOC_DOUBLE_TLV("Headphone Playback Volume",
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[PATCH 3/3] ASoC: mxs: add some error messages to help identifying problems

[Lothar Waßmann]

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 sound/soc/mxs/mxs-sgtl5000.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
index b2e372d..cdd39b5 100644
--- a/sound/soc/mxs/mxs-sgtl5000.c
+++ b/sound/soc/mxs/mxs-sgtl5000.c
@@ -50,18 +50,26 @@ static int mxs_sgtl5000_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	/* Sgtl5000 sysclk should be >= 8MHz and <= 27M */
-	if (mclk < 8000000 || mclk > 27000000)
+	if (mclk < 8000000 || mclk > 27000000) {
+		dev_err(codec_dai->dev, "invalid mclk: %u\n", mclk);
 		return -EINVAL;
+	}
 
 	/* Set SGTL5000's SYSCLK (provided by SAIF MCLK) */
 	ret = snd_soc_dai_set_sysclk(codec_dai, SGTL5000_SYSCLK, mclk, 0);
-	if (ret)
+	if (ret) {
+		dev_err(codec_dai->dev, "Failed to set sysclk to %u kHz\n",
+			mclk / 1000);
 		return ret;
+	}
 
 	/* The SAIF MCLK should be the same as SGTL5000_SYSCLK */
 	ret = snd_soc_dai_set_sysclk(cpu_dai, MXS_SAIF_MCLK, mclk, 0);
-	if (ret)
+	if (ret) {
+		dev_err(cpu_dai->dev, "Failed to set sysclk to %u kHz\n",
+			mclk / 1000);
 		return ret;
+	}
 
 	/* set codec to slave mode */
 	dai_format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
@@ -69,13 +77,19 @@ static int mxs_sgtl5000_hw_params(struct snd_pcm_substream *substream,
 
 	/* set codec DAI configuration */
 	ret = snd_soc_dai_set_fmt(codec_dai, dai_format);
-	if (ret)
+	if (ret) {
+		dev_err(codec_dai->dev, "Failed to set dai format to %08x\n",
+			dai_format);
 		return ret;
+	}
 
 	/* set cpu DAI configuration */
 	ret = snd_soc_dai_set_fmt(cpu_dai, dai_format);
-	if (ret)
+	if (ret) {
+		dev_err(cpu_dai->dev, "Failed to set dai format to %08x\n",
+			dai_format);
 		return ret;
+	}
 
 	return 0;
 }
@@ -153,8 +167,10 @@ static int mxs_sgtl5000_probe(struct platform_device *pdev)
 	 * should be >= 8MHz and <= 27M.
 	 */
 	ret = mxs_saif_get_mclk(0, 44100 * 256, 44100);
-	if (ret)
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get mclk\n");
 		return ret;
+	}
 
 	card->dev = &pdev->dev;
 	platform_set_drvdata(pdev, card);
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 3/3] ASoC: mxs: add some error messages to help identifying problems

[Fabio Estevam]

On Wed, Jul 31, 2013 at 11:44 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:

>         /* Set SGTL5000's SYSCLK (provided by SAIF MCLK) */
>         ret = snd_soc_dai_set_sysclk(codec_dai, SGTL5000_SYSCLK, mclk, 0);
> -       if (ret)
> +       if (ret) {
> +               dev_err(codec_dai->dev, "Failed to set sysclk to %u kHz\n",
> +                       mclk / 1000);

Wouldn't it make more sense to express it in MHz instead of kHz?

>                 return ret;
> +       }
>
>         /* The SAIF MCLK should be the same as SGTL5000_SYSCLK */
>         ret = snd_soc_dai_set_sysclk(cpu_dai, MXS_SAIF_MCLK, mclk, 0);
> -       if (ret)
> +       if (ret) {
> +               dev_err(cpu_dai->dev, "Failed to set sysclk to %u kHz\n",
> +                       mclk / 1000);

Same here.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 3/3] ASoC: mxs: add some error messages to help identifying problems

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 846 bytes --]

On Wed, Jul 31, 2013 at 07:11:48PM -0300, Fabio Estevam wrote:
> On Wed, Jul 31, 2013 at 11:44 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:

> >         /* Set SGTL5000's SYSCLK (provided by SAIF MCLK) */
> >         ret = snd_soc_dai_set_sysclk(codec_dai, SGTL5000_SYSCLK, mclk, 0);
> > -       if (ret)
> > +       if (ret) {
> > +               dev_err(codec_dai->dev, "Failed to set sysclk to %u kHz\n",
> > +                       mclk / 1000);

> Wouldn't it make more sense to express it in MHz instead of kHz?

Well, ideally it'd be fractional MHz (ie, 24.576MHz).  I'd either leave
it as Hz or do the proper pretty print.  Also if we're nitpicking the
format then there's usually no space between the number and units.

Personally I think the bike shed would be most attractive if it were
pink with yellow spots.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/3] ASoC: sgtl5000: prevent playback to be muted when terminating concurrent capture

[Fabio Estevam]

On Wed, Jul 31, 2013 at 11:44 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> When a sound capture/playback is terminated while a playback/capture
> is running, power_vag_event() will clear SGTL5000_CHIP_ANA_POWER in
> the SND_SOC_DAPM_PRE_PMD event, thus muting the respective other
> channel.
>
> Don't clear SGTL5000_CHIP_ANA_POWER when both DAC and ADC are active
> to prevent this.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>

Reviewed-by: Fabio Estevam <fabio.estevam@freescale.com>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/3] ASoC: sgtl5000: fix buggy 'Capture Attenuate Switch' control

[Fabio Estevam]

Hi Lothar,

On Wed, Jul 31, 2013 at 11:44 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> The SGTL5000 Capture Attenuate Switch (or "ADC Volume Range Reduction"
> as it is called in the manual) is single bit only.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>

Nice catch.

Reviewed-by: Fabio Estevam <fabio.estevam@freescale.com>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ASoC: sgtl5000: prevent playback to be muted when terminating concurrent capture

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 308 bytes --]

On Wed, Jul 31, 2013 at 04:44:29PM +0200, Lothar Waßmann wrote:
> When a sound capture/playback is terminated while a playback/capture
> is running, power_vag_event() will clear SGTL5000_CHIP_ANA_POWER in
> the SND_SOC_DAPM_PRE_PMD event, thus muting the respective other
> channel.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/3] ASoC: sgtl5000: fix buggy 'Capture Attenuate Switch' control

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 213 bytes --]

On Wed, Jul 31, 2013 at 04:44:30PM +0200, Lothar Waßmann wrote:
> The SGTL5000 Capture Attenuate Switch (or "ADC Volume Range Reduction"
> as it is called in the manual) is single bit only.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v2] ASoC: mxs: add some error messages to help identifying problems

[Lothar Waßmann]

On Thu, 1 Aug 2013 11:22:19 +0100, Mark Brown wrote:
> On Wed, Jul 31, 2013 at 07:11:48PM -0300, Fabio Estevam wrote:
>> On Wed, Jul 31, 2013 at 11:44 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:

>> >         /* Set SGTL5000's SYSCLK (provided by SAIF MCLK) */
>> >         ret = snd_soc_dai_set_sysclk(codec_dai, SGTL5000_SYSCLK, mclk, 0);
>> > -       if (ret)
>> > +       if (ret) {
>> > +               dev_err(codec_dai->dev, "Failed to set sysclk to %u kHz\n",
>> > +                       mclk / 1000);
>
>> Wouldn't it make more sense to express it in MHz instead of kHz?
>
> Well, ideally it'd be fractional MHz (ie, 24.576MHz).  I'd either leave
> it as Hz or do the proper pretty print.  Also if we're nitpicking the
> format then there's usually no space between the number and units.
>
> Personally I think the bike shed would be most attractive if it were
> pink with yellow spots.
>
Now pink with yellow spots. ;)

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 sound/soc/mxs/mxs-sgtl5000.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/sound/soc/mxs/mxs-sgtl5000.c b/sound/soc/mxs/mxs-sgtl5000.c
index b2e372d..ce084eb 100644
--- a/sound/soc/mxs/mxs-sgtl5000.c
+++ b/sound/soc/mxs/mxs-sgtl5000.c
@@ -50,18 +50,27 @@ static int mxs_sgtl5000_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	/* Sgtl5000 sysclk should be >= 8MHz and <= 27M */
-	if (mclk < 8000000 || mclk > 27000000)
+	if (mclk < 8000000 || mclk > 27000000) {
+		dev_err(codec_dai->dev, "Invalid mclk frequency: %u.%03uMHz\n",
+			mclk / 1000000, mclk / 1000 % 1000);
 		return -EINVAL;
+	}
 
 	/* Set SGTL5000's SYSCLK (provided by SAIF MCLK) */
 	ret = snd_soc_dai_set_sysclk(codec_dai, SGTL5000_SYSCLK, mclk, 0);
-	if (ret)
+	if (ret) {
+		dev_err(codec_dai->dev, "Failed to set sysclk to %u.%03uMHz\n",
+			mclk / 1000000, mclk / 1000 % 1000);
 		return ret;
+	}
 
 	/* The SAIF MCLK should be the same as SGTL5000_SYSCLK */
 	ret = snd_soc_dai_set_sysclk(cpu_dai, MXS_SAIF_MCLK, mclk, 0);
-	if (ret)
+	if (ret) {
+		dev_err(cpu_dai->dev, "Failed to set sysclk to %u.%03uMHz\n",
+			mclk / 1000000, mclk / 1000 % 1000);
 		return ret;
+	}
 
 	/* set codec to slave mode */
 	dai_format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
@@ -69,13 +78,19 @@ static int mxs_sgtl5000_hw_params(struct snd_pcm_substream *substream,
 
 	/* set codec DAI configuration */
 	ret = snd_soc_dai_set_fmt(codec_dai, dai_format);
-	if (ret)
+	if (ret) {
+		dev_err(codec_dai->dev, "Failed to set dai format to %08x\n",
+			dai_format);
 		return ret;
+	}
 
 	/* set cpu DAI configuration */
 	ret = snd_soc_dai_set_fmt(cpu_dai, dai_format);
-	if (ret)
+	if (ret) {
+		dev_err(cpu_dai->dev, "Failed to set dai format to %08x\n",
+			dai_format);
 		return ret;
+	}
 
 	return 0;
 }
@@ -153,8 +168,10 @@ static int mxs_sgtl5000_probe(struct platform_device *pdev)
 	 * should be >= 8MHz and <= 27M.
 	 */
 	ret = mxs_saif_get_mclk(0, 44100 * 256, 44100);
-	if (ret)
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get mclk\n");
 		return ret;
+	}
 
 	card->dev = &pdev->dev;
 	platform_set_drvdata(pdev, card);
-- 
1.7.2.5

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2] ASoC: mxs: add some error messages to help identifying problems

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 417 bytes --]

On Fri, Aug 02, 2013 at 10:30:15AM +0200, Lothar Waßmann wrote:
> On Thu, 1 Aug 2013 11:22:19 +0100, Mark Brown wrote:
> > On Wed, Jul 31, 2013 at 07:11:48PM -0300, Fabio Estevam wrote:
> >> On Wed, Jul 31, 2013 at 11:44 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:

Applied, but please *always* submit patches as covered in
SubmittingPatches - if they won't apply properly with git am that's not
good.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]