[alsa-devel] [PATCH 0/3] ASoC: fsl_sai: Fix some issues in fsl_sai_trigger()
The series of patches focus on issue fix inside fsl_sai_trigger().
Nicolin Chen (3): ASoC: fsl_sai: Reduce race condition during TE/RE enabling ASoC: fsl_sai: Don't reset FIFO until TE/RE bit is unset ASoC: fsl_sai: Improve enable flow in fsl_sai_trigger()
sound/soc/fsl/fsl_sai.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-)
From: Nicolin Chen Guangyu.Chen@freescale.com
For trigger start, we don't need to check if it's the first time to enable TE/RE or second time. It doesn't hurt to enable them any way, which in the meantime can reduce race condition for TE/RE enabling.
For trigger stop, we will definitely clear FRDE of current direction. Thus the driver only needs to read the opposite one's.
Signed-off-by: Nicolin Chen nicoleotsuka@gmail.com --- sound/soc/fsl/fsl_sai.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 1b6ee2c..a437899 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -327,7 +327,7 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, { struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; - u32 tcsr, rcsr; + u32 xcsr;
/* * The transmitter bit clock and frame sync are to be @@ -338,9 +338,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, regmap_update_bits(sai->regmap, FSL_SAI_RCR2, FSL_SAI_CR2_SYNC, FSL_SAI_CR2_SYNC);
- regmap_read(sai->regmap, FSL_SAI_TCSR, &tcsr); - regmap_read(sai->regmap, FSL_SAI_RCSR, &rcsr); - /* * It is recommended that the transmitter is the last enabled * and the first disabled. @@ -349,12 +346,10 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - if (!(tcsr & FSL_SAI_CSR_FRDE || rcsr & FSL_SAI_CSR_FRDE)) { - regmap_update_bits(sai->regmap, FSL_SAI_RCSR, - FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE); - regmap_update_bits(sai->regmap, FSL_SAI_TCSR, - FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE); - } + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE); + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, + FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS); @@ -370,7 +365,8 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, FSL_SAI_CSR_xIE_MASK, 0);
/* Check if the opposite FRDE is also disabled */ - if (!(tx ? rcsr & FSL_SAI_CSR_FRDE : tcsr & FSL_SAI_CSR_FRDE)) { + regmap_read(sai->regmap, FSL_SAI_xCSR(!tx), &xcsr); + if (!(xcsr & FSL_SAI_CSR_FRDE)) { /* Disable both directions and reset their FIFOs */ regmap_update_bits(sai->regmap, FSL_SAI_TCSR, FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
From: Nicolin Chen Guangyu.Chen@freescale.com
TE/RE bit of T/RCSR will remain set untill the current frame is physically finished. The FIFO reset operation should wait this bit's totally cleared rather than ignoring its status which might cause TE/RE disabling failed.
This patch adds delay and timeout to wait for its completion before FIFO reset.
Signed-off-by: Nicolin Chen nicoleotsuka@gmail.com --- sound/soc/fsl/fsl_sai.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index a437899..a79a9b0 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -327,7 +327,7 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, { struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; - u32 xcsr; + u32 xcsr, count = 100;
/* * The transmitter bit clock and frame sync are to be @@ -369,11 +369,20 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, if (!(xcsr & FSL_SAI_CSR_FRDE)) { /* Disable both directions and reset their FIFOs */ regmap_update_bits(sai->regmap, FSL_SAI_TCSR, - FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR, - FSL_SAI_CSR_FR); + FSL_SAI_CSR_TERE, 0); regmap_update_bits(sai->regmap, FSL_SAI_RCSR, - FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR, - FSL_SAI_CSR_FR); + FSL_SAI_CSR_TERE, 0); + + /* TERE will remain set till the end of current frame */ + do { + udelay(10); + regmap_read(sai->regmap, FSL_SAI_xCSR(tx), &xcsr); + } while (--count && xcsr & FSL_SAI_CSR_TERE); + + regmap_update_bits(sai->regmap, FSL_SAI_TCSR, + FSL_SAI_CSR_FR, FSL_SAI_CSR_FR); + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, + FSL_SAI_CSR_FR, FSL_SAI_CSR_FR); } break; default:
From: Nicolin Chen Guangyu.Chen@freescale.com
The previous enable flow: 1, Enable TE&RE (SAI starts to consume tx FIFO and feed rx FIFO) 2, Mask IRQ of Tx/Rx to enable its interrupt. 3, Enable DMA request of Tx/Rx.
As this flow would enable DMA request later than TERE, the Tx FIFO would be easily emptied into underrun while Rx FIFO would be easily stuffed into overrun due to the delayed DMA transfering.
This issue happened merely occational before the patch 'ASoC: fsl_sai: Reset FIFOs after disabling TE/RE' because there were useless data remaining in the FIFO for the gap. However, it manifested after FIFO reset's implemented.
After this patch, the new flow: 1, Enable DMA request of Tx/Rx. 2, Enable TE&RE (SAI starts to consume tx FIFO and feed rx FIFO) 3, Mask IRQ of Tx/Rx to enable its interrupt.
Signed-off-by: Nicolin Chen nicoleotsuka@gmail.com --- sound/soc/fsl/fsl_sai.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index a79a9b0..364410b 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -346,6 +346,9 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), + FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE); + regmap_update_bits(sai->regmap, FSL_SAI_RCSR, FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE); regmap_update_bits(sai->regmap, FSL_SAI_TCSR, @@ -353,8 +356,6 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS); - regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx), - FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE); break; case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND:
participants (2)
-
Mark Brown -
Nicolin Chen