diff mbox

[alsa-devel,07/10] ASoC: ux500: Store DMA data in the DAI differently in the pdata and DT case

Message ID 20131202101256.GA29373@lee--X1
State New
Headers show

Commit Message

Lee Jones Dec. 2, 2013, 10:12 a.m. UTC 
On Sun, 01 Dec 2013, Olof Johansson wrote:

> Hi,
> 
> On Tue, Nov 19, 2013 at 3:07 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > In this patch we do two things. Firstly, instead of open coding the
> > store of DMA data in to the DAI for later use, we use the API provided.
> > Secondly we create and store similar DMA data for the DT case, only
> > this time we use 'struct snd_dmaengine_dai_dma_data' which is provided
> > by the core for this very reason.
> >
> > Cc: alsa-devel@alsa-project.org
> > Cc: Mark Brown <broonie@kernel.org>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> 
> Seems like this hit -next for the first time today, and it panics
> snowball on boot of u8500_defconfig. I bisected down to this patch.
> 
> The panic is below. Last output is the dev_dbg() in
> ux500_pcm_request_chan. dma_cfg seems to be 0x0000004(!) at that
> point. It's indeed crashing on first deref of dma_cfg (confirmed via
> addr2line).

Okay, I just debugged this.

It's actually my fault. I had two patches round the wrong way in the
series. The imediately subsequent patch in the set fixes this issue:

Author: Lee Jones <lee.jones@linaro.org>
Date:   Tue Nov 5 22:57:31 2013 +0000

    ASoC: ux500_pcm: Differentiate between pdata and DT initialisation
    
    If booting with full DT support (i.e. DMA too, the last piece of the
    puzzle), then we don't need to use the compatible request channel call
    back and, due to the work we laid down earlier in this patch-set, we
    can use core function calls to populate the DMA slave_config. We also
    require slightly different flags to inform the core that we 'are'
    booting with DT.
    
    Cc: alsa-devel@alsa-project.org
    Cc: Mark Brown <broonie@kernel.org>
    Acked-by: Linus Walleij <linus.walleij@linaro.org>
    Signed-off-by: Lee Jones <lee.jones@linaro.org>

Comments

Mark Brown Dec. 2, 2013, 1:54 p.m. UTC | #1 
On Mon, Dec 02, 2013 at 10:12:56AM +0000, Lee Jones wrote:

> It's actually my fault. I had two patches round the wrong way in the
> series. The imediately subsequent patch in the set fixes this issue:

OK, so what do we do about this?  Is there going to be a revision of
that patch adding the note about not using the autoconfiguration stuff
and what (if any) are the dependencies on the arch/arm changes?
Lee Jones Dec. 2, 2013, 2:03 p.m. UTC | #2 
On Mon, 02 Dec 2013, Mark Brown wrote:

> On Mon, Dec 02, 2013 at 10:12:56AM +0000, Lee Jones wrote:
> 
> > It's actually my fault. I had two patches round the wrong way in the
> > series. The imediately subsequent patch in the set fixes this issue:
> 
> OK, so what do we do about this?  Is there going to be a revision of
> that patch adding the note about not using the autoconfiguration stuff
> and what (if any) are the dependencies on the arch/arm changes?

I can shoot out a revision no problem.

I'm not aware of any dependencies on the ARM changes. I haven't tested
the arch/arm and sound/soc adaptions orthogonally, but I _think_ the
right decisions should be made depending on the information passed
from platform/dt code.
Mark Brown Dec. 2, 2013, 2:04 p.m. UTC | #3 
On Mon, Dec 02, 2013 at 02:03:24PM +0000, Lee Jones wrote:

> I'm not aware of any dependencies on the ARM changes. I haven't tested
> the arch/arm and sound/soc adaptions orthogonally, but I _think_ the
> right decisions should be made depending on the information passed
> from platform/dt code.

OK, that sounds great thanks.
diff mbox

Patch

diff --git a/sound/soc/ux500/ux500_pcm.c b/sound/soc/ux500/ux500_pcm.c
index ce554de..acfec98 100644
--- a/sound/soc/ux500/ux500_pcm.c
+++ b/sound/soc/ux500/ux500_pcm.c
@@ -139,15 +139,33 @@  static const struct snd_dmaengine_pcm_config ux500_dmaengine_pcm_config = {
 	.prepare_slave_config = ux500_pcm_prepare_slave_config,
 };
 
+static const struct snd_dmaengine_pcm_config ux500_dmaengine_of_pcm_config = {
+	.pcm_hardware = &ux500_pcm_hw,
+	.prealloc_buffer_size = 128 * 1024,
+	.prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
+};
+
 int ux500_pcm_register_platform(struct platform_device *pdev)
 {
+	const struct snd_dmaengine_pcm_config *pcm_config;
+	struct device_node *np = pdev->dev.of_node;
+	unsigned int pcm_flags;
 	int ret;
 
-	ret = snd_dmaengine_pcm_register(&pdev->dev,
-			&ux500_dmaengine_pcm_config,
-			SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
-			SND_DMAENGINE_PCM_FLAG_COMPAT |
-			SND_DMAENGINE_PCM_FLAG_NO_DT);
+	if (np) {
+		pcm_config = &ux500_dmaengine_of_pcm_config;
+
+		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
+			SND_DMAENGINE_PCM_FLAG_COMPAT;
+	} else {
+		pcm_config = &ux500_dmaengine_pcm_config;
+
+		pcm_flags = SND_DMAENGINE_PCM_FLAG_NO_RESIDUE |
+			SND_DMAENGINE_PCM_FLAG_NO_DT |
+			SND_DMAENGINE_PCM_FLAG_COMPAT;
+	}
+
+	ret = snd_dmaengine_pcm_register(&pdev->dev, pcm_config, pcm_flags);
 	if (ret < 0) {
 		dev_err(&pdev->dev,
 			"%s: ERROR: Failed to register platform '%s' (%d)!\n",