ASoC: fsl_sai: Two bug fixes for fsl_sai driver

PATCH In review Nicolin Chen 4 participants 8 messages Jul 17, 2014 – Jul 23, 2014

Subsystems (3)

Sound ASoC Freescale SoC sound drivers

lore

Share & tools

lore

Nicolin Chen

author reviewer

2014-07-17 13:19 UTC #0 lore raw

cover ASoC: fsl_sai: Two bug fixes for fsl_sai driver

Nicolin Chen (2):
ASoC: fsl_sai: Reset FIFOs after disabling TE/RE
ASoC: fsl_sai: Fix incorrect register writing in fsl_sai_isr()

 sound/soc/fsl/fsl_sai.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Signature · 1 line

1.8.4

Nicolin Chen

author reviewer

2014-07-17 13:19 UTC #1 lore raw

PATCH 1/2 ASoC: fsl_sai: Reset FIFOs after disabling TE/RE

SAI will not clear their FIFOs after disabling TE/RE. Therfore, the driver should take care the task so as not to let useless data remain in the FIFO.

Signed-off-by Nicolin Chen <[email protected]>

---
 sound/soc/fsl/fsl_sai.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

sound/soc/fsl/fsl_sai.c +5 -2

index c5a0e8a..b10dbd8 100644
@@ -371,10 +371,13 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 		/* Check if the opposite FRDE is also disabled */
 		if (!(tx ? rcsr & FSL_SAI_CSR_FRDE : tcsr & FSL_SAI_CSR_FRDE)) {
+			/* Disable both directions and reset their FIFOs */
 			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
-					   FSL_SAI_CSR_TERE, 0);
+					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
+					   FSL_SAI_CSR_FR);
 			regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
-					   FSL_SAI_CSR_TERE, 0);
+					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
+					   FSL_SAI_CSR_FR);
 		}
 		break;
 	default:

Signature · 1 line

1.8.4

Nicolin Chen

2014-07-18 10:18 UTC #2 lore raw

Re: [PATCH 1/2] ASoC: fsl_sai: Reset FIFOs after disabling TE/RE

Mark,

Please disregard this single patch.

Quoted 15 lines · Nicolin Chen

SAI will not clear their FIFOs after disabling TE/RE. Therfore, the driver should take care the task so as not to let useless data remain in the FIFO.
Signed-off-by Nicolin Chen <[email protected]>
---
 sound/soc/fsl/fsl_sai.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
sound/soc/fsl/fsl_sai.c +0 -0
index c5a0e8a..b10dbd8 100644
@@ -371,10 +371,13 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 		/* Check if the opposite FRDE is also disabled */

sound/soc/fsl/fsl_sai.c static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, · line 380

Quoted 10 lines

sound/soc/fsl/fsl_sai.c +5 -2

 		if (!(tx ? rcsr & FSL_SAI_CSR_FRDE : tcsr & FSL_SAI_CSR_FRDE)) {
+			/* Disable both directions and reset their FIFOs */
 			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
-					   FSL_SAI_CSR_TERE, 0);
+					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
+					   FSL_SAI_CSR_FR);
 			regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
-					   FSL_SAI_CSR_TERE, 0);
+					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
+					   FSL_SAI_CSR_FR);

The FR should be set _after_ clear TERE, not at the same time because it still may have tiny possibility to remain data.

I'll send another version later for this patch.

The other patch for isr() should be still fine.

Thank you,
Nicolin

Quoted 6 lines

sound/soc/fsl/fsl_sai.c +0 -0
 		}
 		break;
 	default:
Signature · 1 line
1.8.4

Nicolin Chen

2014-07-23 10:03 UTC #3 lore raw

Re: [PATCH 1/2] ASoC: fsl_sai: Reset FIFOs after disabling TE/RE

Sir,

I found this two patches are merged into for-next branch, although I haven't got the 'applied' email.

Is that possible for you to drop this one? If not, I'll send another patch to fix this.

Thank you,
Nicolin

Quoted 48 lines · Nicolin Chen

Mark,
Please disregard this single patch.
Quoted 15 lines · Nicolin Chen
SAI will not clear their FIFOs after disabling TE/RE. Therfore, the driver should take care the task so as not to let useless data remain in the FIFO.
Signed-off-by Nicolin Chen <[email protected]>
---
 sound/soc/fsl/fsl_sai.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
sound/soc/fsl/fsl_sai.c +0 -0
index c5a0e8a..b10dbd8 100644
@@ -371,10 +371,13 @@ static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 		/* Check if the opposite FRDE is also disabled */
sound/soc/fsl/fsl_sai.c static int fsl_sai_trigger(struct snd_pcm_substream *substream, int cmd, · line 380
Quoted 10 lines
sound/soc/fsl/fsl_sai.c +5 -2
 		if (!(tx ? rcsr & FSL_SAI_CSR_FRDE : tcsr & FSL_SAI_CSR_FRDE)) {
+			/* Disable both directions and reset their FIFOs */
 			regmap_update_bits(sai->regmap, FSL_SAI_TCSR,
-					   FSL_SAI_CSR_TERE, 0);
+					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
+					   FSL_SAI_CSR_FR);
 			regmap_update_bits(sai->regmap, FSL_SAI_RCSR,
-					   FSL_SAI_CSR_TERE, 0);
+					   FSL_SAI_CSR_TERE | FSL_SAI_CSR_FR,
+					   FSL_SAI_CSR_FR);
The FR should be set _after_ clear TERE, not at the same time because it still may have tiny possibility to remain data.
I'll send another version later for this patch.
The other patch for isr() should be still fine.
Thank you,
Nicolin
Quoted 6 lines
sound/soc/fsl/fsl_sai.c +0 -0
 		}
 		break;
 	default:
Signature · 1 line
1.8.4

Mark Brown

maintainer

2014-07-23 10:07 UTC #4 lore raw

Re: [PATCH 1/2] ASoC: fsl_sai: Reset FIFOs after disabling TE/RE

Quoted 5 lines · Nicolin Chen

I found this two patches are merged into for-next branch, although I haven't got the 'applied' email.
Is that possible for you to drop this one? If not, I'll send another patch to fix this.

Please send a patch, I'd already applied it by the time you asked me to drop it.

Nicolin Chen

2014-07-23 09:59 UTC #5 lore raw

Re: [PATCH 1/2] ASoC: fsl_sai: Reset FIFOs after disabling TE/RE

Quoted 10 lines · Mark Brown

Quoted 5 lines · Nicolin Chen
I found this two patches are merged into for-next branch, although I haven't got the 'applied' email.
Is that possible for you to drop this one? If not, I'll send another patch to fix this.
Please send a patch, I'd already applied it by the time you asked me to drop it.

Got it. Thank you.
Nicolin

Nicolin Chen

author reviewer

2014-07-17 13:19 UTC #6 lore raw

PATCH 2/2 ASoC: fsl_sai: Fix incorrect register writing in fsl_sai_isr()

In reply to Nicolin Chen

In the rx irq handling part, we should clear the flags in RCSR not TCSR.

Signed-off-by Nicolin Chen <[email protected]>

---
 sound/soc/fsl/fsl_sai.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

sound/soc/fsl/fsl_sai.c +1 -1

index b10dbd8..1b6ee2c 100644
@@ -106,7 +106,7 @@ irq_rx:
 	xcsr &= ~FSL_SAI_CSR_xF_MASK;
 
 	if (flags)
-		regmap_write(sai->regmap, FSL_SAI_TCSR, flags | xcsr);
+		regmap_write(sai->regmap, FSL_SAI_RCSR, flags | xcsr);
 
 out:
 	if (irq_none)

Signature · 1 line

1.8.4

[email protected]

2014-07-18 02:38 UTC #7 lore raw

In reply to Nicolin Chen

Quoted 9 lines

-----Original Message-----
From: Nicolin Chen [mailto:[email protected]]
Sent: Thursday, July 17, 2014 9:22 PM
To: [email protected]
Cc: [email protected]; [email protected]; alsa- [email protected]; [email protected]; Xiubo Li-B47053; Wang Shengjiu-B02247; Chen Guangyu-B42378
Subject: [PATCH 0/2] ASoC: fsl_sai: Two bug fixes for fsl_sai driver

Quoted 4 lines

Nicolin Chen (2):
ASoC: fsl_sai: Reset FIFOs after disabling TE/RE
ASoC: fsl_sai: Fix incorrect register writing in fsl_sai_isr()

Yes, they are all looks good to me.

Signed-off-by Xiubo Li <[email protected]>

Thanks,

BRs
Xiubo

Quoted 5 lines

 sound/soc/fsl/fsl_sai.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--
1.8.4