[alsa-devel] [PATCH] ASoC: soc-pcm: Use valid condition for snd_soc_dai_digital_mute() in hw_free()
The snd_soc_dai_digital_mute() here will be never executed because we only decrease codec->active in snd_soc_close(). Thus correct it.
Signed-off-by: Nicolin Chen b42378@freescale.com --- sound/soc/soc-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 22af038..7d19117 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -689,7 +689,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) }
/* apply codec digital mute */ - if (!codec->active) + if (codec->active == 1) snd_soc_dai_digital_mute(codec_dai, 1, substream->stream);
/* free any machine hw params */
At Mon, 2 Dec 2013 17:34:34 +0800, Nicolin Chen wrote:
The snd_soc_dai_digital_mute() here will be never executed because we only decrease codec->active in snd_soc_close(). Thus correct it.
A couple of minor problems by this approach:
- snd_soc_dai_digital_mute() will be called twice in the normal PCM close path.
- In full duplex mode where both playback/capture streams share the same codec, one of the directions won't be handled correctly because codec->active is incremented/decremented twice (once for each direction).
Takashi
Signed-off-by: Nicolin Chen b42378@freescale.com
sound/soc/soc-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 22af038..7d19117 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -689,7 +689,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) }
/* apply codec digital mute */
- if (!codec->active)
if (codec->active == 1) snd_soc_dai_digital_mute(codec_dai, 1, substream->stream);
/* free any machine hw params */
-- 1.8.4
On Mon, Dec 02, 2013 at 11:05:31AM +0100, Takashi Iwai wrote:
At Mon, 2 Dec 2013 17:34:34 +0800, Nicolin Chen wrote:
The snd_soc_dai_digital_mute() here will be never executed because we only decrease codec->active in snd_soc_close(). Thus correct it.
A couple of minor problems by this approach:
snd_soc_dai_digital_mute() will be called twice in the normal PCM close path.
In full duplex mode where both playback/capture streams share the same codec, one of the directions won't be handled correctly because codec->active is incremented/decremented twice (once for each direction).
Thank you for the comments. I didn't look at the whole board. Please Drop this patch.
Thank you. Nicolin Chen
Takashi
Signed-off-by: Nicolin Chen b42378@freescale.com
sound/soc/soc-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 22af038..7d19117 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -689,7 +689,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) }
/* apply codec digital mute */
- if (!codec->active)
if (codec->active == 1) snd_soc_dai_digital_mute(codec_dai, 1, substream->stream);
/* free any machine hw params */
-- 1.8.4
On Mon, Dec 02, 2013 at 11:05:31AM +0100, Takashi Iwai wrote:
Nicolin Chen wrote:
The snd_soc_dai_digital_mute() here will be never executed because we only decrease codec->active in snd_soc_close(). Thus correct it.
A couple of minor problems by this approach:
- snd_soc_dai_digital_mute() will be called twice in the normal PCM close path.
Right, it's called in close() so it's redundant to call it here most of the time. On the other hand it will at least get called slightly sooner which is good and in the case of reconfiguration we'll mute while doing that - doing the mute earlier is better.
- In full duplex mode where both playback/capture streams share the same codec, one of the directions won't be handled correctly because codec->active is incremented/decremented twice (once for each direction).
Indeed, the check needs to be on playback_active or capture_active rather than on the overall DAI.
On Mon, Dec 02, 2013 at 12:19:16PM +0000, Mark Brown wrote:
On Mon, Dec 02, 2013 at 11:05:31AM +0100, Takashi Iwai wrote:
Nicolin Chen wrote:
The snd_soc_dai_digital_mute() here will be never executed because we only decrease codec->active in snd_soc_close(). Thus correct it.
A couple of minor problems by this approach:
- snd_soc_dai_digital_mute() will be called twice in the normal PCM close path.
Right, it's called in close() so it's redundant to call it here most of the time. On the other hand it will at least get called slightly sooner which is good and in the case of reconfiguration we'll mute while doing that - doing the mute earlier is better.
- In full duplex mode where both playback/capture streams share the same codec, one of the directions won't be handled correctly because codec->active is incremented/decremented twice (once for each direction).
Indeed, the check needs to be on playback_active or capture_active rather than on the overall DAI.
Sir, thank you for the hint. I will reconsider about the modification.
And also thank Iwai-san for pointing out the flaws in my patch.
Best regards, Nicolin Chen
participants (3)
-
Mark Brown
-
Nicolin Chen
-
Takashi Iwai