Patchwork ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

login
register
mail settings
Submitter Nicolin Chen
Date July 10, 2013, 10:43 a.m.
Message ID <1373453034-21618-1-git-send-email-b42378@freescale.com>
Download mbox | patch
Permalink /patch/258009/
State Not Applicable
Headers show

Comments

Nicolin Chen - July 10, 2013, 10:43 a.m.
The code enabled SSIEN when triggered by SNDRV_PCM_TRIGGER_START,
so move the disable code to SNDRV_PCM_TRIGGER_STOP for symmetric.

This also allows us to use the SSI driver more flexible so that
it can support some use cases like "aplay S16_LE.wav S24_LE.wav"
which would call the driver in sequence like:
 startup()->hw_params(S16_LE)->trigger(START)->tirgger(STOP)->
 hw_params(S24_LE)->trigger(START)->tirgger(STOP)->shutdown()

If we disable SSIEN in shutdown(), the second hw_params() would
bypass the sample bits setting while using symmetric_rate.

Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
 sound/soc/fsl/fsl_ssi.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)
Shawn Guo - July 11, 2013, 2:07 p.m.
On Wed, Jul 10, 2013 at 06:43:54PM +0800, Nicolin Chen wrote:
> The code enabled SSIEN when triggered by SNDRV_PCM_TRIGGER_START,
> so move the disable code to SNDRV_PCM_TRIGGER_STOP for symmetric.
> 
> This also allows us to use the SSI driver more flexible so that
> it can support some use cases like "aplay S16_LE.wav S24_LE.wav"
> which would call the driver in sequence like:
>  startup()->hw_params(S16_LE)->trigger(START)->tirgger(STOP)->
>  hw_params(S24_LE)->trigger(START)->tirgger(STOP)->shutdown()
> 
> If we disable SSIEN in shutdown(), the second hw_params() would
> bypass the sample bits setting while using symmetric_rate.
> 
> Signed-off-by: Nicolin Chen <b42378@freescale.com>

Acked-by: Shawn Guo <shawn.guo@linaro.org>

> ---
>  sound/soc/fsl/fsl_ssi.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
> index 2f2d837..b6ab341 100644
> --- a/sound/soc/fsl/fsl_ssi.c
> +++ b/sound/soc/fsl/fsl_ssi.c
> @@ -510,6 +510,9 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
>  			write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_TE, 0);
>  		else
>  			write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_RE, 0);
> +
> +		if ((read_ssi(&ssi->scr) & (CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE)) == 0)
> +			write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_SSIEN, 0);
>  		break;
>  
>  	default:
> @@ -534,15 +537,6 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
>  		ssi_private->first_stream = ssi_private->second_stream;
>  
>  	ssi_private->second_stream = NULL;
> -
> -	/*
> -	 * If this is the last active substream, disable the SSI.
> -	 */
> -	if (!ssi_private->first_stream) {
> -		struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
> -
> -		write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_SSIEN, 0);
> -	}
>  }
>  
>  static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)
> -- 
> 1.7.1
> 
>
Mark Brown - July 11, 2013, 4:54 p.m.
On Wed, Jul 10, 2013 at 06:43:54PM +0800, Nicolin Chen wrote:
> The code enabled SSIEN when triggered by SNDRV_PCM_TRIGGER_START,
> so move the disable code to SNDRV_PCM_TRIGGER_STOP for symmetric.

Applied, thanks.
Timur Tabi - July 11, 2013, 5:14 p.m.
On 07/11/2013 11:54 AM, Mark Brown wrote:
>> > The code enabled SSIEN when triggered by SNDRV_PCM_TRIGGER_START,
>> > so move the disable code to SNDRV_PCM_TRIGGER_STOP for symmetric.
> Applied, thanks.

Don't you need an ACK from the maintainer of the driver before applying
it?  I haven't had a chance to test it yet.
Nicolin Chen - July 12, 2013, 3:57 a.m.
Hi Timur,

On Thu, Jul 11, 2013 at 12:14:56PM -0500, timur@tabi.org wrote:
> Don't you need an ACK from the maintainer of the driver before applying
> it?  I haven't had a chance to test it yet.
If I'm not missing some part of branch updating, it looks like Mark hasn't
pushed it to the branch yet.
Please test it for me. Thank you.

Best regards,
Nicolin Chen
Timur Tabi - July 12, 2013, 4 a.m.
Nicolin Chen wrote:
> Hi Timur,
>
> On Thu, Jul 11, 2013 at 12:14:56PM -0500, timur@tabi.org wrote:
>> Don't you need an ACK from the maintainer of the driver before applying
>> it?  I haven't had a chance to test it yet.
> If I'm not missing some part of branch updating, it looks like Mark hasn't
> pushed it to the branch yet.
> Please test it for me. Thank you.

I definitely want to.  Unfortunately, I'm literally in the middle of 
moving into a new house, and so everything is packed up.  I won't be able 
to look at it for another week.
Nicolin Chen - July 12, 2013, 4:09 a.m.
On Thu, Jul 11, 2013 at 11:00:15PM -0500, Timur Tabi wrote:
> I definitely want to.  Unfortunately, I'm literally in the middle of
> moving into a new house, and so everything is packed up.  I won't be
> able to look at it for another week.

Oh, I see. Actually we've been using this patch for quite a while,
it should be okay.
And I still have some patches pending for the fsl_ssi.c, but looks
like it'll be better for me to wait after you settle down.

Hope everything will run smoothly during your house-moving.
Timur Tabi - July 12, 2013, 4:13 a.m.
Nicolin Chen wrote:
> Oh, I see. Actually we've been using this patch for quite a while,
> it should be okay.

Have you tested it on PowerPC?
Nicolin Chen - July 12, 2013, 4:19 a.m.
On Thu, Jul 11, 2013 at 11:13:32PM -0500, Timur Tabi wrote:
> Have you tested it on PowerPC?

Not actually. But I think it won't break the work flow since
the SSI modules on two platform should be literally identical.

But if you still have concern with it, you can ask Mark to wait
and test it later. I'm okay with your decision.

Thanks,
Nicolin Chen
Mark Brown - July 12, 2013, 8:43 a.m.
On Thu, Jul 11, 2013 at 11:00:15PM -0500, Timur Tabi wrote:
> Nicolin Chen wrote:

> >If I'm not missing some part of branch updating, it looks like Mark hasn't
> >pushed it to the branch yet.
> >Please test it for me. Thank you.

> I definitely want to.  Unfortunately, I'm literally in the middle of
> moving into a new house, and so everything is packed up.  I won't be
> able to look at it for another week.

Yes, that's why I just went ahead - you'd said recently that you'd be
out of action for a while and not able to test anything.
Timur Tabi - July 12, 2013, 12:11 p.m.
Mark Brown wrote:
> Yes, that's why I just went ahead - you'd said recently that you'd be
> out of action for a while and not able to test anything.

Yeah, I'd rather you waited until I at least made sure it compiled.  Is 
there any way you can at least install the Freescale PowerPC SDK and do 
compile tests?  At least that way you won't have to wait for me for every 
little thing.
Shawn Guo - July 12, 2013, 1:32 p.m.
On Fri, Jul 12, 2013 at 07:11:35AM -0500, Timur Tabi wrote:
> Mark Brown wrote:
> >Yes, that's why I just went ahead - you'd said recently that you'd be
> >out of action for a while and not able to test anything.
> 
> Yeah, I'd rather you waited until I at least made sure it compiled.
> Is there any way you can at least install the Freescale PowerPC SDK
> and do compile tests?  At least that way you won't have to wait for
> me for every little thing.

I just compile-tested it with mpc85xx_defconfig.  It compiles good.

Shawn
Mark Brown - July 12, 2013, 2:23 p.m.
On Fri, Jul 12, 2013 at 07:11:35AM -0500, Timur Tabi wrote:
> Mark Brown wrote:

> >Yes, that's why I just went ahead - you'd said recently that you'd be
> >out of action for a while and not able to test anything.

> Yeah, I'd rather you waited until I at least made sure it compiled.
> Is there any way you can at least install the Freescale PowerPC SDK
> and do compile tests?  At least that way you won't have to wait for
> me for every little thing.

Several of the automated systems that cover -next do PowerPC test builds
so I think it's getting covered already (Stephen Rothwell plus I think
that Fengguang's system has some too).  Now that I'm keeping everything
in topic branches it's much easier for me to discard anything found in
such testing.

But yes, it's easy enough for me to install a PowerPC toolchain - in
fact I was going to do that anyway as there's some SPI drivers that only
build for PowerPC which I want to clean up.

Patch

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 2f2d837..b6ab341 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -510,6 +510,9 @@  static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
 			write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_TE, 0);
 		else
 			write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_RE, 0);
+
+		if ((read_ssi(&ssi->scr) & (CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE)) == 0)
+			write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_SSIEN, 0);
 		break;
 
 	default:
@@ -534,15 +537,6 @@  static void fsl_ssi_shutdown(struct snd_pcm_substream *substream,
 		ssi_private->first_stream = ssi_private->second_stream;
 
 	ssi_private->second_stream = NULL;
-
-	/*
-	 * If this is the last active substream, disable the SSI.
-	 */
-	if (!ssi_private->first_stream) {
-		struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
-
-		write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_SSIEN, 0);
-	}
 }
 
 static int fsl_ssi_dai_probe(struct snd_soc_dai *dai)