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

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

[Nicolin Chen]

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

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

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

[Nicolin Chen]

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

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

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Shawn Guo]

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

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Mark Brown]

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

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.

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

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

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 216 bytes --]

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.

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

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[timur]

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.

-- 
Timur Tabi

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Nicolin Chen]

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

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Nicolin Chen]

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

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Timur Tabi]

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.


-- 
Timur Tabi

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Timur Tabi]

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.


-- 
Timur Tabi

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Nicolin Chen]

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.

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Nicolin Chen]

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.

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Timur Tabi]

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?

-- 
Timur Tabi

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Timur Tabi]

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?

-- 
Timur Tabi

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Nicolin Chen]

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

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Nicolin Chen]

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

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Mark Brown]

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

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.

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

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

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 540 bytes --]

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.

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

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Timur Tabi]

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.

-- 
Timur Tabi

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Timur Tabi]

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.

-- 
Timur Tabi

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Shawn Guo]

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

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Shawn Guo]

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

Re: [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Mark Brown]

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

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.

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

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

Re: [alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

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.

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