Skip to contentSkip to conversation

ASoC: fsl_sai: Fix some issues in fsl_sai_trigger()

PATCH Applied Nicolin Chen 2 participants 5 messages Jul 23, 2014 – Jul 25, 2014

Subsystems (3)
Sound ASoC Freescale SoC sound drivers
lore
Share & tools
lore

Nicolin Chen

author reviewer
#0 lore raw

cover 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(-)
Signature · 1 line
1.8.4
author reviewer
#1 lore raw

PATCH 1/3 ASoC: fsl_sai: Reduce race condition during TE/RE enabling

From: Nicolin Chen <[email protected]>

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 <[email protected]>
---
 sound/soc/fsl/fsl_sai.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)
sound/soc/fsl/fsl_sai.c +7 -11
index 1b6ee2c..a437899 100644
@@ -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,
Signature · 1 line
1.8.4
author reviewer
#2 lore raw

PATCH 2/3 ASoC: fsl_sai: Don't reset FIFO until TE/RE bit is unset

In reply to Nicolin Chen

From: Nicolin Chen <[email protected]>

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 <[email protected]>
---
 sound/soc/fsl/fsl_sai.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)
sound/soc/fsl/fsl_sai.c +14 -5
index a437899..a79a9b0 100644
@@ -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:
Signature · 1 line
1.8.4
author reviewer
#3 lore raw

PATCH 3/3 ASoC: fsl_sai: Improve enable flow in fsl_sai_trigger()

In reply to Nicolin Chen

From: Nicolin Chen <[email protected]>

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 <[email protected]>
---
 sound/soc/fsl/fsl_sai.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
sound/soc/fsl/fsl_sai.c +3 -2
index a79a9b0..364410b 100644
@@ -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:
Signature · 1 line
1.8.4