[alsa-devel] [PATCH 1/3] ASoC: sgtl5000: prevent playback to be muted when terminating concurrent capture
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;
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",
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
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.
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);
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.
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.
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);
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.
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
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.
participants (3)
-
Fabio Estevam -
Lothar Waßmann -
Mark Brown