[PATCH] ASoC: soc-pcm: Use valid condition for snd_soc_dai_digital_mute() in hw_free()

[PATCH] ASoC: soc-pcm: Use valid condition for snd_soc_dai_digital_mute() in hw_free()

[Nicolin Chen]

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 */
-- 
1.8.4

Re: [PATCH] ASoC: soc-pcm: Use valid condition for snd_soc_dai_digital_mute() in hw_free()

[Takashi Iwai]

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
> 
> 

Re: [PATCH] ASoC: soc-pcm: Use valid condition for snd_soc_dai_digital_mute() in hw_free()

[Nicolin Chen]

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
> > 
> > 
> 

Re: [PATCH] ASoC: soc-pcm: Use valid condition for snd_soc_dai_digital_mute() in hw_free()

[Mark Brown]

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

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.

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

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

Re: [PATCH] ASoC: soc-pcm: Use valid condition for snd_soc_dai_digital_mute() in hw_free()

[Nicolin Chen]

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