[alsa-devel] [PATCH] ASoC: soc-pcm: Only do snd_soc_dai_digital_mute() in hw_free()
This patch removed the redundant snd_soc_dai_digital_mute() in close() since it is better to mute in hw_free() which is slightly earlier and symmetrical for the case of reconfiguration, 'aplay 44100.wav 48000.wav' for example: open()->hw_params()->prepare(unmute)->playing->hw_free(mute)->hw_params()-> parepare(unmute)->playing->hw_free(mute)->close()
This patch also corrects the condition of the snd_soc_dai_digital_mute() in hw_free() because it would not be executed as we only decrease codec->active in snd_soc_close().
Signed-off-by: Nicolin Chen b42378@freescale.com --- sound/soc/soc-pcm.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index eb340a8..eac82858 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -474,11 +474,6 @@ static int soc_pcm_close(struct snd_pcm_substream *substream) codec_dai->active--; codec->active--;
- /* Muting the DAC suppresses artifacts caused during digital - * shutdown, for example from stopping clocks. - */ - snd_soc_dai_digital_mute(codec_dai, 1, substream->stream); - if (cpu_dai->driver->ops->shutdown) cpu_dai->driver->ops->shutdown(substream, cpu_dai);
@@ -689,7 +684,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) struct snd_soc_platform *platform = rtd->platform; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai = rtd->codec_dai; - struct snd_soc_codec *codec = rtd->codec; + bool playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ? 1 : 0;
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
@@ -707,7 +702,8 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) }
/* apply codec digital mute */ - if (!codec->active) + if ((playback && codec_dai->playback_active == 1) || + (!playback && codec_dai->capture_active == 1)) snd_soc_dai_digital_mute(codec_dai, 1, substream->stream);
/* free any machine hw params */
On Tue, Dec 03, 2013 at 05:16:06PM +0800, Nicolin Chen wrote:
@@ -689,7 +684,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) struct snd_soc_platform *platform = rtd->platform; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai = rtd->codec_dai;
- struct snd_soc_codec *codec = rtd->codec;
- bool playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ? 1 : 0;
Please don't use the ternery operator, you could've just used the result of the comparison directly here.
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
@@ -707,7 +702,8 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) }
/* apply codec digital mute */
- if (!codec->active)
- if ((playback && codec_dai->playback_active == 1) ||
snd_soc_dai_digital_mute(codec_dai, 1, substream->stream);(!playback && codec_dai->capture_active == 1))
This is a bit confusing to read due to the strange indentation (the second line is massively indented with respect to the first line for some reason). This hunk also ought to be split out as a separate patch since it's a bug fix.
On Tue, Dec 03, 2013 at 03:04:23PM +0000, Mark Brown wrote:
On Tue, Dec 03, 2013 at 05:16:06PM +0800, Nicolin Chen wrote:
@@ -689,7 +684,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) struct snd_soc_platform *platform = rtd->platform; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct snd_soc_dai *codec_dai = rtd->codec_dai;
- struct snd_soc_codec *codec = rtd->codec;
- bool playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ? 1 : 0;
Please don't use the ternery operator, you could've just used the result of the comparison directly here.
Oh, a bad habit. Thank you for teaching me this.
mutex_lock_nested(&rtd->pcm_mutex, rtd->pcm_subclass);
@@ -707,7 +702,8 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) }
/* apply codec digital mute */
- if (!codec->active)
- if ((playback && codec_dai->playback_active == 1) ||
snd_soc_dai_digital_mute(codec_dai, 1, substream->stream);(!playback && codec_dai->capture_active == 1))
This is a bit confusing to read due to the strange indentation (the second line is massively indented with respect to the first line for some reason). This hunk also ought to be split out as a separate patch since it's a bug fix.
I'll refine the indentation and split the patch into two.
Thank you indeed. Nicolin Chen
participants (2)
-
Mark Brown
-
Nicolin Chen