diff mbox

[v3,4/9] ASoC: fsl-ssi: Fix interrupt mapping and release

Message ID 1387367865-26391-5-git-send-email-mpa@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Markus Pargmann Dec. 18, 2013, 11:57 a.m. UTC
irqs should only be requested/released with enabled DMA and not on MX21.
MX21 has some special register write configurations, so we do not add
support for statistics on mx21 for the moment.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 sound/soc/fsl/fsl_ssi.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

Comments

Nicolin Chen Dec. 20, 2013, 9:05 a.m. UTC | #1
Hi Markus,

On Wed, Dec 18, 2013 at 12:57:40PM +0100, Markus Pargmann wrote:
> irqs should only be requested/released with enabled DMA and not on MX21.
> MX21 has some special register write configurations, so we do not add
> support for statistics on mx21 for the moment.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  sound/soc/fsl/fsl_ssi.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> @@ -227,13 +228,15 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
>  
>  	if (sisr & CCSR_SSI_SISR_RFRC) {
>  		ssi_private->stats.rfrc++;
> -		sisr2 |= CCSR_SSI_SISR_RFRC;
> +		if (ssi_private->hw_type != FSL_SSI_MX51)
> +			sisr2 |= CCSR_SSI_SISR_RFRC;
>  		ret = IRQ_HANDLED;
>  	}
>  
>  	if (sisr & CCSR_SSI_SISR_TFRC) {
>  		ssi_private->stats.tfrc++;
> -		sisr2 |= CCSR_SSI_SISR_TFRC;
> +		if (ssi_private->hw_type != FSL_SSI_MX51)
> +			sisr2 |= CCSR_SSI_SISR_TFRC;

I had a hard time on this part of modification because I just saw the commit
comments 'not on MX21' until I checked the RM of imx35, which should be same
as imx21 one: RFRC and RFRC became non-writeable since imx51. The should be
the reason here right?

And just for curiosity, is there any wrong with this part, like causing SSI
mis-behavior?

Thank you,
Nicolin Chen
Markus Pargmann Dec. 20, 2013, 10:31 a.m. UTC | #2
Hi Nicolin,

On Fri, Dec 20, 2013 at 05:05:28PM +0800, Nicolin Chen wrote:
> Hi Markus,
> 
> On Wed, Dec 18, 2013 at 12:57:40PM +0100, Markus Pargmann wrote:
> > irqs should only be requested/released with enabled DMA and not on MX21.
> > MX21 has some special register write configurations, so we do not add
> > support for statistics on mx21 for the moment.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  sound/soc/fsl/fsl_ssi.c | 24 ++++++++++++++++++------
> >  1 file changed, 18 insertions(+), 6 deletions(-)
> > 
> > @@ -227,13 +228,15 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
> >  
> >  	if (sisr & CCSR_SSI_SISR_RFRC) {
> >  		ssi_private->stats.rfrc++;
> > -		sisr2 |= CCSR_SSI_SISR_RFRC;
> > +		if (ssi_private->hw_type != FSL_SSI_MX51)
> > +			sisr2 |= CCSR_SSI_SISR_RFRC;
> >  		ret = IRQ_HANDLED;
> >  	}
> >  
> >  	if (sisr & CCSR_SSI_SISR_TFRC) {
> >  		ssi_private->stats.tfrc++;
> > -		sisr2 |= CCSR_SSI_SISR_TFRC;
> > +		if (ssi_private->hw_type != FSL_SSI_MX51)
> > +			sisr2 |= CCSR_SSI_SISR_TFRC;
> 
> I had a hard time on this part of modification because I just saw the commit
> comments 'not on MX21' until I checked the RM of imx35, which should be same
> as imx21 one: RFRC and RFRC became non-writeable since imx51. The should be
> the reason here right?

mx21 has actually no writable bits in SISR at all, it's the same for
imx27 and imx31. imx35 would actually work with the current
fsl_ssi_isr().

However I just realised that only imx35 has all 6 bits writeable right?
So I added another hw_type MX35 to get irq stats working for all imx's.

> 
> And just for curiosity, is there any wrong with this part, like causing SSI
> mis-behavior?

Yes it doesn't work for all SoCs:
f0377086 ASoC: fsl: disable ssi irq for imx

Thanks,

Markus
diff mbox

Patch

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index a889df5..c361730 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -150,6 +150,7 @@  struct fsl_ssi_private {
 	struct snd_soc_dai_driver cpu_dai_drv;
 	struct platform_device *pdev;
 
+	enum fsl_ssi_type hw_type;
 	bool new_binding;
 	bool ssi_on_imx;
 	bool imx_ac97;
@@ -227,13 +228,15 @@  static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
 
 	if (sisr & CCSR_SSI_SISR_RFRC) {
 		ssi_private->stats.rfrc++;
-		sisr2 |= CCSR_SSI_SISR_RFRC;
+		if (ssi_private->hw_type != FSL_SSI_MX51)
+			sisr2 |= CCSR_SSI_SISR_RFRC;
 		ret = IRQ_HANDLED;
 	}
 
 	if (sisr & CCSR_SSI_SISR_TFRC) {
 		ssi_private->stats.tfrc++;
-		sisr2 |= CCSR_SSI_SISR_TFRC;
+		if (ssi_private->hw_type != FSL_SSI_MX51)
+			sisr2 |= CCSR_SSI_SISR_TFRC;
 		ret = IRQ_HANDLED;
 	}
 
@@ -925,6 +928,7 @@  static int fsl_ssi_probe(struct platform_device *pdev)
 
 	ssi_private->use_dma = !of_property_read_bool(np,
 			"fsl,fiq-stream-filter");
+	ssi_private->hw_type = hw_type;
 
 	if (ac97) {
 		memcpy(&ssi_private->cpu_dai_drv, &fsl_ssi_ac97_dai,
@@ -1030,7 +1034,14 @@  static int fsl_ssi_probe(struct platform_device *pdev)
 			dma_events[0], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI);
 		imx_pcm_dma_params_init_data(&ssi_private->filter_data_rx,
 			dma_events[1], shared ? IMX_DMATYPE_SSI_SP : IMX_DMATYPE_SSI);
-	} else if (ssi_private->use_dma) {
+	}
+
+	/*
+	 * Enable interrupts only for MCP8610 and MX51. The other MXs have
+	 * different writeable interrupt status registers.
+	 */
+	if (ssi_private->use_dma &&
+			(hw_type == FSL_SSI_MCP8610 || hw_type == FSL_SSI_MX51)) {
 		/* The 'name' should not have any slashes in it. */
 		ret = devm_request_irq(&pdev->dev, ssi_private->irq,
 					fsl_ssi_isr, 0, ssi_private->name,
@@ -1138,8 +1149,8 @@  error_clk:
 		clk_disable_unprepare(ssi_private->clk);
 
 error_irqmap:
-	if (ssi_private->use_dma && !of_device_is_compatible(pdev->dev.of_node,
-				"fsl,imx21-ssi"))
+	if (ssi_private->use_dma &&
+			(hw_type == FSL_SSI_MCP8610 || hw_type == FSL_SSI_MX51))
 		irq_dispose_mapping(ssi_private->irq);
 
 	return ret;
@@ -1156,7 +1167,8 @@  static int fsl_ssi_remove(struct platform_device *pdev)
 	snd_soc_unregister_component(&pdev->dev);
 	if (ssi_private->ssi_on_imx)
 		clk_disable_unprepare(ssi_private->clk);
-	irq_dispose_mapping(ssi_private->irq);
+	if (ssi_private->use_dma)
+		irq_dispose_mapping(ssi_private->irq);
 
 	return 0;
 }