| Message ID | 20090404183319.GA22203@oksana.dev.rtsoft.ru (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 5c15a6869a75000fecea61e9985f4753311ec534 |
| Delegated to: | Kumar Gala |
| Headers | show |
Becky & Kumar, Considering these fixups, would it be advisable for the dma functions to climb up the dev->parent linkage when dma_ops are NULL? g. On Sat, Apr 4, 2009 at 12:33 PM, Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > The driver should pass a device that specifies internal DMA ops, but > substream->pcm is just a logical device, and thus doesn't have arch- > specific dma callbacks, therefore following bug appears: > > Freescale Synchronous Serial Interface (SSI) ASoC Driver > ------------[ cut here ]------------ > kernel BUG at arch/powerpc/include/asm/dma-mapping.h:237! > Oops: Exception in kernel mode, sig: 5 [#1] > ... > NIP [c02259c4] snd_malloc_dev_pages+0x58/0xac > LR [c0225c74] snd_dma_alloc_pages+0xf8/0x108 > Call Trace: > [df02bde0] [df02be2c] 0xdf02be2c (unreliable) > [df02bdf0] [c0225c74] snd_dma_alloc_pages+0xf8/0x108 > [df02be10] [c023a100] fsl_dma_new+0x68/0x124 > [df02be20] [c02342ac] soc_new_pcm+0x1bc/0x234 > [df02bea0] [c02343dc] snd_soc_new_pcms+0xb8/0x148 > [df02bed0] [c023824c] cs4270_probe+0x34/0x124 > [df02bef0] [c0232fe8] snd_soc_instantiate_card+0x1a4/0x2f4 > [df02bf20] [c0233164] snd_soc_instantiate_cards+0x2c/0x68 > [df02bf30] [c0234704] snd_soc_register_platform+0x60/0x80 > [df02bf50] [c03d5664] fsl_soc_platform_init+0x18/0x28 > ... > > This patch fixes the issue by using card's device instead. > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > --- > sound/soc/fsl/fsl_dma.c | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c > index b3eb857..835e43c 100644 > --- a/sound/soc/fsl/fsl_dma.c > +++ b/sound/soc/fsl/fsl_dma.c > @@ -300,7 +300,7 @@ static int fsl_dma_new(struct snd_card *card, struct snd_soc_dai *dai, > if (!card->dev->coherent_dma_mask) > card->dev->coherent_dma_mask = fsl_dma_dmamask; > > - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, > + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, card->dev, > fsl_dma_hardware.buffer_bytes_max, > &pcm->streams[0].substream->dma_buffer); > if (ret) { > @@ -310,7 +310,7 @@ static int fsl_dma_new(struct snd_card *card, struct snd_soc_dai *dai, > return -ENOMEM; > } > > - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, > + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, card->dev, > fsl_dma_hardware.buffer_bytes_max, > &pcm->streams[1].substream->dma_buffer); > if (ret) { > @@ -418,7 +418,7 @@ static int fsl_dma_open(struct snd_pcm_substream *substream) > return -EBUSY; > } > > - dma_private = dma_alloc_coherent(substream->pcm->dev, > + dma_private = dma_alloc_coherent(substream->pcm->card->dev, > sizeof(struct fsl_dma_private), &ld_buf_phys, GFP_KERNEL); > if (!dma_private) { > dev_err(substream->pcm->card->dev, > @@ -445,7 +445,7 @@ static int fsl_dma_open(struct snd_pcm_substream *substream) > dev_err(substream->pcm->card->dev, > "can't register ISR for IRQ %u (ret=%i)\n", > dma_private->irq, ret); > - dma_free_coherent(substream->pcm->dev, > + dma_free_coherent(substream->pcm->card->dev, > sizeof(struct fsl_dma_private), > dma_private, dma_private->ld_buf_phys); > return ret; > @@ -761,13 +761,13 @@ static int fsl_dma_close(struct snd_pcm_substream *substream) > free_irq(dma_private->irq, dma_private); > > if (dma_private->ld_buf_phys) { > - dma_unmap_single(substream->pcm->dev, > + dma_unmap_single(substream->pcm->card->dev, > dma_private->ld_buf_phys, > sizeof(dma_private->link), DMA_TO_DEVICE); > } > > /* Deallocate the fsl_dma_private structure */ > - dma_free_coherent(substream->pcm->dev, > + dma_free_coherent(substream->pcm->card->dev, > sizeof(struct fsl_dma_private), > dma_private, dma_private->ld_buf_phys); > substream->runtime->private_data = NULL; > -- > 1.5.6.5 > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-dev >
On Sat, Apr 04, 2009 at 11:59:39PM -0600, Grant Likely wrote: > Becky & Kumar, > > Considering these fixups, would it be advisable for the dma functions > to climb up the dev->parent linkage when dma_ops are NULL? pcm->dev->parent doesn't point to the proper device, so this won't work. > On Sat, Apr 4, 2009 at 12:33 PM, Anton Vorontsov > <avorontsov@ru.mvista.com> wrote: > > The driver should pass a device that specifies internal DMA ops, but > > substream->pcm is just a logical device, and thus doesn't have arch- > > specific dma callbacks, therefore following bug appears: > > > > Freescale Synchronous Serial Interface (SSI) ASoC Driver > > ------------[ cut here ]------------ > > kernel BUG at arch/powerpc/include/asm/dma-mapping.h:237! > > Oops: Exception in kernel mode, sig: 5 [#1] > > ... > > NIP [c02259c4] snd_malloc_dev_pages+0x58/0xac > > LR [c0225c74] snd_dma_alloc_pages+0xf8/0x108 > > Call Trace: > > [df02bde0] [df02be2c] 0xdf02be2c (unreliable) > > [df02bdf0] [c0225c74] snd_dma_alloc_pages+0xf8/0x108 > > [df02be10] [c023a100] fsl_dma_new+0x68/0x124 > > [df02be20] [c02342ac] soc_new_pcm+0x1bc/0x234 > > [df02bea0] [c02343dc] snd_soc_new_pcms+0xb8/0x148 > > [df02bed0] [c023824c] cs4270_probe+0x34/0x124 > > [df02bef0] [c0232fe8] snd_soc_instantiate_card+0x1a4/0x2f4 > > [df02bf20] [c0233164] snd_soc_instantiate_cards+0x2c/0x68 > > [df02bf30] [c0234704] snd_soc_register_platform+0x60/0x80 > > [df02bf50] [c03d5664] fsl_soc_platform_init+0x18/0x28 > > ... > > > > This patch fixes the issue by using card's device instead. > > > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > > --- > > sound/soc/fsl/fsl_dma.c | 12 ++++++------ > > 1 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c > > index b3eb857..835e43c 100644 > > --- a/sound/soc/fsl/fsl_dma.c > > +++ b/sound/soc/fsl/fsl_dma.c > > @@ -300,7 +300,7 @@ static int fsl_dma_new(struct snd_card *card, struct snd_soc_dai *dai, > > if (!card->dev->coherent_dma_mask) > > card->dev->coherent_dma_mask = fsl_dma_dmamask; > > > > - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, > > + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, card->dev, > > fsl_dma_hardware.buffer_bytes_max, > > &pcm->streams[0].substream->dma_buffer); > > if (ret) { > > @@ -310,7 +310,7 @@ static int fsl_dma_new(struct snd_card *card, struct snd_soc_dai *dai, > > return -ENOMEM; > > } > > > > - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, > > + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, card->dev, > > fsl_dma_hardware.buffer_bytes_max, > > &pcm->streams[1].substream->dma_buffer); > > if (ret) { > > @@ -418,7 +418,7 @@ static int fsl_dma_open(struct snd_pcm_substream *substream) > > return -EBUSY; > > } > > > > - dma_private = dma_alloc_coherent(substream->pcm->dev, > > + dma_private = dma_alloc_coherent(substream->pcm->card->dev, > > sizeof(struct fsl_dma_private), &ld_buf_phys, GFP_KERNEL); > > if (!dma_private) { > > dev_err(substream->pcm->card->dev, > > @@ -445,7 +445,7 @@ static int fsl_dma_open(struct snd_pcm_substream *substream) > > dev_err(substream->pcm->card->dev, > > "can't register ISR for IRQ %u (ret=%i)\n", > > dma_private->irq, ret); > > - dma_free_coherent(substream->pcm->dev, > > + dma_free_coherent(substream->pcm->card->dev, > > sizeof(struct fsl_dma_private), > > dma_private, dma_private->ld_buf_phys); > > return ret; > > @@ -761,13 +761,13 @@ static int fsl_dma_close(struct snd_pcm_substream *substream) > > free_irq(dma_private->irq, dma_private); > > > > if (dma_private->ld_buf_phys) { > > - dma_unmap_single(substream->pcm->dev, > > + dma_unmap_single(substream->pcm->card->dev, > > dma_private->ld_buf_phys, > > sizeof(dma_private->link), DMA_TO_DEVICE); > > } > > > > /* Deallocate the fsl_dma_private structure */ > > - dma_free_coherent(substream->pcm->dev, > > + dma_free_coherent(substream->pcm->card->dev, > > sizeof(struct fsl_dma_private), > > dma_private, dma_private->ld_buf_phys); > > substream->runtime->private_data = NULL; > > -- > > 1.5.6.5 > > _______________________________________________ > > Linuxppc-dev mailing list > > Linuxppc-dev@ozlabs.org > > https://ozlabs.org/mailman/listinfo/linuxppc-dev > > > > > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd.
On Sun, Apr 5, 2009 at 2:52 AM, Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > On Sat, Apr 04, 2009 at 11:59:39PM -0600, Grant Likely wrote: >> Becky & Kumar, >> >> Considering these fixups, would it be advisable for the dma functions >> to climb up the dev->parent linkage when dma_ops are NULL? > > pcm->dev->parent doesn't point to the proper device, so > this won't work. What does it point to then? IIUC, following the ->parent links should eventually get to a struct device that represents the platform or of_platform device for purposes of power management. BTW, I'm not NAKing your patch. The patch looks fine to me. g. > >> On Sat, Apr 4, 2009 at 12:33 PM, Anton Vorontsov >> <avorontsov@ru.mvista.com> wrote: >> > The driver should pass a device that specifies internal DMA ops, but >> > substream->pcm is just a logical device, and thus doesn't have arch- >> > specific dma callbacks, therefore following bug appears: >> > >> > Freescale Synchronous Serial Interface (SSI) ASoC Driver >> > ------------[ cut here ]------------ >> > kernel BUG at arch/powerpc/include/asm/dma-mapping.h:237! >> > Oops: Exception in kernel mode, sig: 5 [#1] >> > ... >> > NIP [c02259c4] snd_malloc_dev_pages+0x58/0xac >> > LR [c0225c74] snd_dma_alloc_pages+0xf8/0x108 >> > Call Trace: >> > [df02bde0] [df02be2c] 0xdf02be2c (unreliable) >> > [df02bdf0] [c0225c74] snd_dma_alloc_pages+0xf8/0x108 >> > [df02be10] [c023a100] fsl_dma_new+0x68/0x124 >> > [df02be20] [c02342ac] soc_new_pcm+0x1bc/0x234 >> > [df02bea0] [c02343dc] snd_soc_new_pcms+0xb8/0x148 >> > [df02bed0] [c023824c] cs4270_probe+0x34/0x124 >> > [df02bef0] [c0232fe8] snd_soc_instantiate_card+0x1a4/0x2f4 >> > [df02bf20] [c0233164] snd_soc_instantiate_cards+0x2c/0x68 >> > [df02bf30] [c0234704] snd_soc_register_platform+0x60/0x80 >> > [df02bf50] [c03d5664] fsl_soc_platform_init+0x18/0x28 >> > ... >> > >> > This patch fixes the issue by using card's device instead. >> > >> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> >> > --- >> > sound/soc/fsl/fsl_dma.c | 12 ++++++------ >> > 1 files changed, 6 insertions(+), 6 deletions(-) >> > >> > diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c >> > index b3eb857..835e43c 100644 >> > --- a/sound/soc/fsl/fsl_dma.c >> > +++ b/sound/soc/fsl/fsl_dma.c >> > @@ -300,7 +300,7 @@ static int fsl_dma_new(struct snd_card *card, struct snd_soc_dai *dai, >> > if (!card->dev->coherent_dma_mask) >> > card->dev->coherent_dma_mask = fsl_dma_dmamask; >> > >> > - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, >> > + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, card->dev, >> > fsl_dma_hardware.buffer_bytes_max, >> > &pcm->streams[0].substream->dma_buffer); >> > if (ret) { >> > @@ -310,7 +310,7 @@ static int fsl_dma_new(struct snd_card *card, struct snd_soc_dai *dai, >> > return -ENOMEM; >> > } >> > >> > - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, >> > + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, card->dev, >> > fsl_dma_hardware.buffer_bytes_max, >> > &pcm->streams[1].substream->dma_buffer); >> > if (ret) { >> > @@ -418,7 +418,7 @@ static int fsl_dma_open(struct snd_pcm_substream *substream) >> > return -EBUSY; >> > } >> > >> > - dma_private = dma_alloc_coherent(substream->pcm->dev, >> > + dma_private = dma_alloc_coherent(substream->pcm->card->dev, >> > sizeof(struct fsl_dma_private), &ld_buf_phys, GFP_KERNEL); >> > if (!dma_private) { >> > dev_err(substream->pcm->card->dev, >> > @@ -445,7 +445,7 @@ static int fsl_dma_open(struct snd_pcm_substream *substream) >> > dev_err(substream->pcm->card->dev, >> > "can't register ISR for IRQ %u (ret=%i)\n", >> > dma_private->irq, ret); >> > - dma_free_coherent(substream->pcm->dev, >> > + dma_free_coherent(substream->pcm->card->dev, >> > sizeof(struct fsl_dma_private), >> > dma_private, dma_private->ld_buf_phys); >> > return ret; >> > @@ -761,13 +761,13 @@ static int fsl_dma_close(struct snd_pcm_substream *substream) >> > free_irq(dma_private->irq, dma_private); >> > >> > if (dma_private->ld_buf_phys) { >> > - dma_unmap_single(substream->pcm->dev, >> > + dma_unmap_single(substream->pcm->card->dev, >> > dma_private->ld_buf_phys, >> > sizeof(dma_private->link), DMA_TO_DEVICE); >> > } >> > >> > /* Deallocate the fsl_dma_private structure */ >> > - dma_free_coherent(substream->pcm->dev, >> > + dma_free_coherent(substream->pcm->card->dev, >> > sizeof(struct fsl_dma_private), >> > dma_private, dma_private->ld_buf_phys); >> > substream->runtime->private_data = NULL; >> > -- >> > 1.5.6.5 >> > _______________________________________________ >> > Linuxppc-dev mailing list >> > Linuxppc-dev@ozlabs.org >> > https://ozlabs.org/mailman/listinfo/linuxppc-dev >> > >> >> >> >> -- >> Grant Likely, B.Sc., P.Eng. >> Secret Lab Technologies Ltd. > > -- > Anton Vorontsov > email: cbouatmailru@gmail.com > irc://irc.freenode.net/bd2 >
On Apr 5, 2009, at 9:39 AM, Grant Likely wrote: > On Sun, Apr 5, 2009 at 2:52 AM, Anton Vorontsov > <avorontsov@ru.mvista.com> wrote: >> On Sat, Apr 04, 2009 at 11:59:39PM -0600, Grant Likely wrote: >>> Becky & Kumar, >>> >>> Considering these fixups, would it be advisable for the dma >>> functions >>> to climb up the dev->parent linkage when dma_ops are NULL? >> >> pcm->dev->parent doesn't point to the proper device, so >> this won't work. > > What does it point to then? IIUC, following the ->parent links should > eventually get to a struct device that represents the platform or > of_platform device for purposes of power management. > > BTW, I'm not NAKing your patch. The patch looks fine to me. > > g. No, I think we should just fix drivers. There aren't that many SoC drivers and once they are fixed this will resolve itself. - k
On Sun, Apr 5, 2009 at 9:59 AM, Kumar Gala <galak@kernel.crashing.org> wrote: > No, I think we should just fix drivers. There aren't that many SoC drivers > and once they are fixed this will resolve itself. I agree. The only reason I passed NULL when I wrote the drivers was because I was too lazy to figure out what the right dev pointer was. ASoC is kinda screwy in that many fields are not initialized when you think they should be. More than once I passed a pointer to a wrong object without realizing it, even though the code worked. I'll check out these two patches on Monday.
Anton Vorontsov wrote: > The driver should pass a device that specifies internal DMA ops, but > substream->pcm is just a logical device, and thus doesn't have arch- > specific dma callbacks, therefore following bug appears: > > Freescale Synchronous Serial Interface (SSI) ASoC Driver > ------------[ cut here ]------------ > kernel BUG at arch/powerpc/include/asm/dma-mapping.h:237! > Oops: Exception in kernel mode, sig: 5 [#1] > ... > NIP [c02259c4] snd_malloc_dev_pages+0x58/0xac > LR [c0225c74] snd_dma_alloc_pages+0xf8/0x108 > Call Trace: > [df02bde0] [df02be2c] 0xdf02be2c (unreliable) > [df02bdf0] [c0225c74] snd_dma_alloc_pages+0xf8/0x108 > [df02be10] [c023a100] fsl_dma_new+0x68/0x124 > [df02be20] [c02342ac] soc_new_pcm+0x1bc/0x234 > [df02bea0] [c02343dc] snd_soc_new_pcms+0xb8/0x148 > [df02bed0] [c023824c] cs4270_probe+0x34/0x124 > [df02bef0] [c0232fe8] snd_soc_instantiate_card+0x1a4/0x2f4 > [df02bf20] [c0233164] snd_soc_instantiate_cards+0x2c/0x68 > [df02bf30] [c0234704] snd_soc_register_platform+0x60/0x80 > [df02bf50] [c03d5664] fsl_soc_platform_init+0x18/0x28 > ... > > This patch fixes the issue by using card's device instead. > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> > --- Acked-by: Timur Tabi <timur@freescale.com> Mark and Takashi: this patch is a must-fix for 2.6.30
On Mon, Apr 06, 2009 at 04:06:22PM -0500, Timur Tabi wrote: > Acked-by: Timur Tabi <timur@freescale.com> > Mark and Takashi: this patch is a must-fix for 2.6.30 Applied, thanks.
diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c index b3eb857..835e43c 100644 --- a/sound/soc/fsl/fsl_dma.c +++ b/sound/soc/fsl/fsl_dma.c @@ -300,7 +300,7 @@ static int fsl_dma_new(struct snd_card *card, struct snd_soc_dai *dai, if (!card->dev->coherent_dma_mask) card->dev->coherent_dma_mask = fsl_dma_dmamask; - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, card->dev, fsl_dma_hardware.buffer_bytes_max, &pcm->streams[0].substream->dma_buffer); if (ret) { @@ -310,7 +310,7 @@ static int fsl_dma_new(struct snd_card *card, struct snd_soc_dai *dai, return -ENOMEM; } - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, pcm->dev, + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, card->dev, fsl_dma_hardware.buffer_bytes_max, &pcm->streams[1].substream->dma_buffer); if (ret) { @@ -418,7 +418,7 @@ static int fsl_dma_open(struct snd_pcm_substream *substream) return -EBUSY; } - dma_private = dma_alloc_coherent(substream->pcm->dev, + dma_private = dma_alloc_coherent(substream->pcm->card->dev, sizeof(struct fsl_dma_private), &ld_buf_phys, GFP_KERNEL); if (!dma_private) { dev_err(substream->pcm->card->dev, @@ -445,7 +445,7 @@ static int fsl_dma_open(struct snd_pcm_substream *substream) dev_err(substream->pcm->card->dev, "can't register ISR for IRQ %u (ret=%i)\n", dma_private->irq, ret); - dma_free_coherent(substream->pcm->dev, + dma_free_coherent(substream->pcm->card->dev, sizeof(struct fsl_dma_private), dma_private, dma_private->ld_buf_phys); return ret; @@ -761,13 +761,13 @@ static int fsl_dma_close(struct snd_pcm_substream *substream) free_irq(dma_private->irq, dma_private); if (dma_private->ld_buf_phys) { - dma_unmap_single(substream->pcm->dev, + dma_unmap_single(substream->pcm->card->dev, dma_private->ld_buf_phys, sizeof(dma_private->link), DMA_TO_DEVICE); } /* Deallocate the fsl_dma_private structure */ - dma_free_coherent(substream->pcm->dev, + dma_free_coherent(substream->pcm->card->dev, sizeof(struct fsl_dma_private), dma_private, dma_private->ld_buf_phys); substream->runtime->private_data = NULL;
The driver should pass a device that specifies internal DMA ops, but substream->pcm is just a logical device, and thus doesn't have arch- specific dma callbacks, therefore following bug appears: Freescale Synchronous Serial Interface (SSI) ASoC Driver ------------[ cut here ]------------ kernel BUG at arch/powerpc/include/asm/dma-mapping.h:237! Oops: Exception in kernel mode, sig: 5 [#1] ... NIP [c02259c4] snd_malloc_dev_pages+0x58/0xac LR [c0225c74] snd_dma_alloc_pages+0xf8/0x108 Call Trace: [df02bde0] [df02be2c] 0xdf02be2c (unreliable) [df02bdf0] [c0225c74] snd_dma_alloc_pages+0xf8/0x108 [df02be10] [c023a100] fsl_dma_new+0x68/0x124 [df02be20] [c02342ac] soc_new_pcm+0x1bc/0x234 [df02bea0] [c02343dc] snd_soc_new_pcms+0xb8/0x148 [df02bed0] [c023824c] cs4270_probe+0x34/0x124 [df02bef0] [c0232fe8] snd_soc_instantiate_card+0x1a4/0x2f4 [df02bf20] [c0233164] snd_soc_instantiate_cards+0x2c/0x68 [df02bf30] [c0234704] snd_soc_register_platform+0x60/0x80 [df02bf50] [c03d5664] fsl_soc_platform_init+0x18/0x28 ... This patch fixes the issue by using card's device instead. Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- sound/soc/fsl/fsl_dma.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)