diff mbox

[1/2] ASoC: fsl: imx-ssi: fix probe on imx31

Message ID 1380630971-11431-2-git-send-email-philippe.retornaz@epfl.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Philippe Rétornaz Oct. 1, 2013, 12:36 p.m. UTC
On imx31 with mc13783 codec the FIQ is not necessary and not enabled
as DMA transfer is available.
Change the probe() function to fail only if both FIQ and DMA are not
available.

Signed-off-by: Philippe Rétornaz <philippe.retornaz@epfl.ch>
---
 sound/soc/fsl/imx-ssi.c |   23 ++++++++++++-----------
 sound/soc/fsl/imx-ssi.h |    2 ++
 2 files changed, 14 insertions(+), 11 deletions(-)

Comments

Mark Brown Oct. 3, 2013, 3:59 p.m. UTC | #1
On Tue, Oct 01, 2013 at 02:36:10PM +0200, Philippe Rétornaz wrote:
> On imx31 with mc13783 codec the FIQ is not necessary and not enabled
> as DMA transfer is available.
> Change the probe() function to fail only if both FIQ and DMA are not
> available.

Is this something that used to work but has been broken or is this
enabling a new feture?  Just wondering if this needs to be sent to Linus
as a fix.
Philippe Rétornaz Oct. 3, 2013, 4:07 p.m. UTC | #2
Le 03/10/2013 17:59, Mark Brown a écrit :
> On Tue, Oct 01, 2013 at 02:36:10PM +0200, Philippe Rétornaz wrote:
>> On imx31 with mc13783 codec the FIQ is not necessary and not
>> enabled as DMA transfer is available. Change the probe() function
>> to fail only if both FIQ and DMA are not available.
>
> Is this something that used to work but has been broken or is this
> enabling a new feture?  Just wondering if this needs to be sent to
> Linus as a fix.
>

It was working but it is broken since a few release.
There is no need to rush for a fix, both patches can follow the standard
process.
Anyway, there is a pending fix on the imx-sdma driver in order to have the
sound fully working again on my board:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/201498.html

Thanks !

Philippe
Fabio Estevam Oct. 4, 2013, 6:10 p.m. UTC | #3
Hi Philippe,

On Thu, Oct 3, 2013 at 1:07 PM, Philippe Rétornaz
<philippe.retornaz@epfl.ch> wrote:

> It was working but it is broken since a few release.
> There is no need to rush for a fix, both patches can follow the standard
> process.

Thanks for the fix.

If you have a chance, it would be really nice if mx31-moboard could be
converted to device tree, as this would help us towards the goal of
getting rid of the sound/soc/fsl/imx-ssi.c driver, which only supports
the non-dt imx parts.

We have been using  sound/soc/fsl/fsl_ssi.c for the dt imx and powerpc
parts, and that's probably the reason we haven't noticed this bug.

Regards,

Fabio Estevam
Philippe Rétornaz Oct. 7, 2013, 9:07 a.m. UTC | #4
Hello

Le 04/10/2013 20:10, Fabio Estevam a écrit :
> On Thu, Oct 3, 2013 at 1:07 PM, Philippe Rétornaz wrote
>> It was working but it is broken since a few release. There is no
>> need to rush for a fix, both patches can follow the standard
>> process.
>
> We have been using  sound/soc/fsl/fsl_ssi.c for the dt imx and
> powerpc parts, and that's probably the reason we haven't noticed this
> bug.

Well, I don't think it's fair to break boards just because they are not
(yet) converted to DT ... this board was merged before the DT era on ARM.

Anyway even on DT it's broken: before the switch to the common asoc pcm
code, the dma API was misused to let it think that the audio part of the
SoC was doing the bus mastering, while in fact the sdma was doing it.

Regards,

Philippe
Mark Brown Oct. 7, 2013, 10:17 a.m. UTC | #5
On Tue, Oct 01, 2013 at 02:36:10PM +0200, Philippe Rétornaz wrote:
> On imx31 with mc13783 codec the FIQ is not necessary and not enabled
> as DMA transfer is available.
> Change the probe() function to fail only if both FIQ and DMA are not
> available.

Applied, thanks.
Fabio Estevam Oct. 7, 2013, 10:29 a.m. UTC | #6
On Mon, Oct 7, 2013 at 6:07 AM, Philippe Rétornaz
<philippe.retornaz@epfl.ch> wrote:

> Well, I don't think it's fair to break boards just because they are not
> (yet) converted to DT ... this board was merged before the DT era on ARM.

Sure, I agree. Just trying to avoid to keep two drivers for the same hardware.

I will try to convert mx31 to use sound/soc/fsl/fsl_ssi.c when time allows.

Regards,

Fabio Estevam
diff mbox

Patch

diff --git a/sound/soc/fsl/imx-ssi.c b/sound/soc/fsl/imx-ssi.c
index f58bcd8..57d6941 100644
--- a/sound/soc/fsl/imx-ssi.c
+++ b/sound/soc/fsl/imx-ssi.c
@@ -600,19 +600,17 @@  static int imx_ssi_probe(struct platform_device *pdev)
 	ssi->fiq_params.dma_params_rx = &ssi->dma_params_rx;
 	ssi->fiq_params.dma_params_tx = &ssi->dma_params_tx;
 
-	ret = imx_pcm_fiq_init(pdev, &ssi->fiq_params);
-	if (ret)
-		goto failed_pcm_fiq;
+	ssi->fiq_init = imx_pcm_fiq_init(pdev, &ssi->fiq_params);
+	ssi->dma_init = imx_pcm_dma_init(pdev);
 
-	ret = imx_pcm_dma_init(pdev);
-	if (ret)
-		goto failed_pcm_dma;
+	if (ssi->fiq_init && ssi->dma_init) {
+		ret = ssi->fiq_init;
+		goto failed_pcm;
+	}
 
 	return 0;
 
-failed_pcm_dma:
-	imx_pcm_fiq_exit(pdev);
-failed_pcm_fiq:
+failed_pcm:
 	snd_soc_unregister_component(&pdev->dev);
 failed_register:
 	release_mem_region(res->start, resource_size(res));
@@ -628,8 +626,11 @@  static int imx_ssi_remove(struct platform_device *pdev)
 	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	struct imx_ssi *ssi = platform_get_drvdata(pdev);
 
-	imx_pcm_dma_exit(pdev);
-	imx_pcm_fiq_exit(pdev);
+	if (!ssi->dma_init)
+		imx_pcm_dma_exit(pdev);
+
+	if (!ssi->fiq_init)
+		imx_pcm_fiq_exit(pdev);
 
 	snd_soc_unregister_component(&pdev->dev);
 
diff --git a/sound/soc/fsl/imx-ssi.h b/sound/soc/fsl/imx-ssi.h
index fb1616b..560c40f 100644
--- a/sound/soc/fsl/imx-ssi.h
+++ b/sound/soc/fsl/imx-ssi.h
@@ -211,6 +211,8 @@  struct imx_ssi {
 	struct imx_dma_data filter_data_rx;
 	struct imx_pcm_fiq_params fiq_params;
 
+	int fiq_init;
+	int dma_init;
 	int enabled;
 };