[alsa-devel] [PATCH] ASoC: fsl_ssi: separately enable and disable TIE/RIE in trigger()
This patch enables Tx-related SIER_FLAGS only when direction is PLAYBACK and does same thing for CAPTURE. Also, after TRIGGER_STOP/PAUSE, it will disable SIER_xFLAGS for symmetric.
[ Passed compile-test with mpc85xx_defconfig ]
Signed-off-by: Nicolin Chen b42378@freescale.com --- sound/soc/fsl/fsl_ssi.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 35e2773..3797bf0 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -107,11 +107,14 @@ static inline void write_ssi_mask(u32 __iomem *addr, u32 clear, u32 set) #endif
/* SIER bitflag of interrupts to enable */ -#define SIER_FLAGS (CCSR_SSI_SIER_TFRC_EN | CCSR_SSI_SIER_TDMAE | \ +#define SIER_TFLAGS (CCSR_SSI_SIER_TFRC_EN | CCSR_SSI_SIER_TDMAE | \ CCSR_SSI_SIER_TIE | CCSR_SSI_SIER_TUE0_EN | \ - CCSR_SSI_SIER_TUE1_EN | CCSR_SSI_SIER_RFRC_EN | \ - CCSR_SSI_SIER_RDMAE | CCSR_SSI_SIER_RIE | \ - CCSR_SSI_SIER_ROE0_EN | CCSR_SSI_SIER_ROE1_EN) + CCSR_SSI_SIER_TUE1_EN) +#define SIER_RFLAGS (CCSR_SSI_SIER_RFRC_EN | CCSR_SSI_SIER_RDMAE | \ + CCSR_SSI_SIER_RIE | CCSR_SSI_SIER_ROE0_EN | \ + CCSR_SSI_SIER_ROE1_EN) + +#define SIER_FLAGS (SIER_TFLAGS | SIER_RFLAGS)
/** * fsl_ssi_private: per-SSI private data @@ -560,12 +563,12 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { if (ssi_private->use_dma) - sier_bits = SIER_FLAGS; + sier_bits = SIER_TFLAGS; else sier_bits = CCSR_SSI_SIER_TIE | CCSR_SSI_SIER_TFE0_EN; } else { if (ssi_private->use_dma) - sier_bits = SIER_FLAGS; + sier_bits = SIER_RFLAGS; else sier_bits = CCSR_SSI_SIER_RIE | CCSR_SSI_SIER_RFF0_EN; } @@ -579,6 +582,8 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, else write_ssi_mask(&ssi->scr, 0, CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_RE); + + write_ssi_mask(&ssi->sier, 0, sier_bits); break;
case SNDRV_PCM_TRIGGER_STOP: @@ -591,14 +596,14 @@ static int fsl_ssi_trigger(struct snd_pcm_substream *substream, int cmd, if (!ssi_private->imx_ac97 && (read_ssi(&ssi->scr) & (CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE)) == 0) write_ssi_mask(&ssi->scr, CCSR_SSI_SCR_SSIEN, 0); + + write_ssi_mask(&ssi->sier, sier_bits, 0); break;
default: return -EINVAL; }
- write_ssi(sier_bits, &ssi->sier); - return 0; }
Nicolin Chen wrote:
This patch enables Tx-related SIER_FLAGS only when direction is PLAYBACK and does same thing for CAPTURE. Also, after TRIGGER_STOP/PAUSE, it will disable SIER_xFLAGS for symmetric.
I'm okay with this patch in principle, but why bother? The sysfs entry is going to display all interrupts anyway, and so the result will be the same.
On Tue, Oct 29, 2013 at 06:59:44AM -0500, Timur Tabi wrote:
Nicolin Chen wrote:
This patch enables Tx-related SIER_FLAGS only when direction is PLAYBACK and does same thing for CAPTURE. Also, after TRIGGER_STOP/PAUSE, it will disable SIER_xFLAGS for symmetric.
I'm okay with this patch in principle, but why bother? The sysfs entry is going to display all interrupts anyway, and so the result will be the same.
Well, actually I just wanted to clear T/RDMAE to disable DMA request, but it seems to be much easier to do it like this based on current code and disabling unused interrupts should be better right? :)
Thank you, Nicolin Chen
Nicolin Chen wrote:
Well, actually I just wanted to clear T/RDMAE to disable DMA request, but it seems to be much easier to do it like this based on current code and disabling unused interrupts should be better right?:)
It's not better if it complicates the code and has no real impact. The code has been running fine for years the way it is. Unless you can show me that it actually makes a difference, I will have to NACK this patch.
On Tue, Oct 29, 2013 at 07:18:21AM -0500, Timur Tabi wrote:
Nicolin Chen wrote:
Well, actually I just wanted to clear T/RDMAE to disable DMA request, but it seems to be much easier to do it like this based on current code and disabling unused interrupts should be better right?:)
It's not better if it complicates the code and has no real impact. The code has been running fine for years the way it is. Unless you can show me that it actually makes a difference, I will have to NACK this patch.
The DMA request might be remaining high if not disabling it. This would cause SDMA re-check this request, while it has no BD existing. For the other interrupts, if you don't like it, I can drop it. Just modification to the driver might not be less complicated.
Thank you, Nicolin Chen
Nicolin Chen wrote:
The DMA request might be remaining high if not disabling it.
Might? Are you just guessing?
This would cause SDMA re-check this request, while it has no BD existing. For the other interrupts, if you don't like it, I can drop it. Just modification to the driver might not be less complicated.
I'm only talking about this particular patch.
I mean there is a possibility.
I'm sorry if my patch is kinda annoying and it really bother you, sir. I also want to make things better.
If you really don't like it, we can drop it. It's all your call.
And thank you for reviewing.
Sent by Android device.
Timur Tabi timur@tabi.org wrote:
Nicolin Chen wrote:
The DMA request might be remaining high if not disabling it.
Might? Are you just guessing?
This would cause SDMA re-check this request, while it has no BD existing. For the other interrupts, if you don't like it, I can drop it. Just modification to the driver might not be less complicated.
I'm only talking about this particular patch.
Chen Guangyu-B42378 wrote:
I mean there is a possibility.
I'm sorry if my patch is kinda annoying and it really bother you, sir. I also want to make things better.
It does not bother me. I'm glad people are working on my driver. I just want to make sure that my driver does not get bloated.
If you really don't like it, we can drop it. It's all your call.
I think this patch should be dropped, because I don't see any real reason for it.
As you wish, sir.
Sent by Android device.
Timur Tabi timur@tabi.org wrote:
Chen Guangyu-B42378 wrote:
I mean there is a possibility.
I'm sorry if my patch is kinda annoying and it really bother you, sir. I also want to make things better.
It does not bother me. I'm glad people are working on my driver. I just want to make sure that my driver does not get bloated.
If you really don't like it, we can drop it. It's all your call.
I think this patch should be dropped, because I don't see any real reason for it.
participants (3)
-
Chen Guangyu-B42378 -
Nicolin Chen
-
Timur Tabi