[alsa-devel] [PATCH] ASoC: fsl: Disable SSI in trigger() if RE/TE are both cleared
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)
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
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.
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.
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
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.
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.
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
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.
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.
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
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.
participants (5)
-
Mark Brown -
Nicolin Chen -
Shawn Guo -
Timur Tabi -
timur＠tabi.org