* Unconditional registering EMDA platform devices @ 2014-10-24 14:29 Andrew Lunn 2014-10-24 16:14 ` Arnd Bergmann 0 siblings, 1 reply; 5+ messages in thread From: Andrew Lunn @ 2014-10-24 14:29 UTC (permalink / raw) To: linux-arm-kernel Hi Matt I've had a report of a Debian kernel running on a Marvell XP system giving warnings: [ 0.114771] edma-dma-engine edma-dma-engine.0: Can't allocate PaRAM dummy slot [ 0.114794] edma-dma-engine: probe of edma-dma-engine.0 failed with error -5 These seem to be coming from drivers/dma/emda.c That driver has a subsys_initcall(edma_init); and the edma_init function is unconditionally registering a driver and a platform device. For a multiarch kernel, this is not a good idea. Please could you make this conditionally. Maybe look into the DT and see if the DMA is needed on the platform? Thanks Andrew ^ permalink raw reply [flat|nested] 5+ messages in thread
* Unconditional registering EMDA platform devices 2014-10-24 14:29 Unconditional registering EMDA platform devices Andrew Lunn @ 2014-10-24 16:14 ` Arnd Bergmann 2014-10-25 18:48 ` Uwe Kleine-König 0 siblings, 1 reply; 5+ messages in thread From: Arnd Bergmann @ 2014-10-24 16:14 UTC (permalink / raw) To: linux-arm-kernel On Friday 24 October 2014 16:29:04 Andrew Lunn wrote: > Hi Matt > > I've had a report of a Debian kernel running on a Marvell XP system > giving warnings: > > [ 0.114771] edma-dma-engine edma-dma-engine.0: Can't allocate PaRAM dummy slot > [ 0.114794] edma-dma-engine: probe of edma-dma-engine.0 failed with error -5 > > These seem to be coming from drivers/dma/emda.c > > That driver has a subsys_initcall(edma_init); > > and the edma_init function is unconditionally registering a driver and > a platform device. For a multiarch kernel, this is not a good idea. > > Please could you make this conditionally. Maybe look into the DT and > see if the DMA is needed on the platform? I just looked at that code an I'm completely confused about how that even works today. I do see that the driver is used on ATAGS based davinci machines, which means we can't just look into the DT. The main problem seems to stem from arch/arm/common/edma.c being half the driver that provides interfaces to both drivers/dma/edma.c and to sound/soc/davinci/davinci-pcm.c, while drivers/dma/edma.c is not really a driver by itself. My preferred solution to this would be to move arch/arm/common/edma.c into drivers/dma/edma.c and still have it export its private API, but I assume that the dmaengine maintainers have already NAKed that approach. Would the approach below work? Arnd 8<------- Subject: dma: edma: move device registration to platform code The horrible split between the low-level part of the edma support and the dmaengine front-end driver causes problems on multiplatform kernels. This is an attempt to improve the situation slightly by only registering the dmaengine devices that are actually present. Signed-off-by: Arnd Bergmann <arnd@arndb.de> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c index d86771abbf57..f6cffee3c6ee 100644 --- a/arch/arm/common/edma.c +++ b/arch/arm/common/edma.c @@ -1623,6 +1623,11 @@ static int edma_probe(struct platform_device *pdev) struct device_node *node = pdev->dev.of_node; struct device *dev = &pdev->dev; int ret; + struct platform_device_info edma_dev_info = { + .name = "edma-dma-engine", + .dma_mask = DMA_BIT_MASK(32), + .parent = &pdev->dev, + }; if (node) { /* Check if this is a second instance registered */ @@ -1793,6 +1798,9 @@ static int edma_probe(struct platform_device *pdev) edma_write_array(j, EDMA_QRAE, i, 0x0); } arch_num_cc++; + + edma_dev_info.id = j; + platform_device_register_full(&edma_dev_info); } return 0; diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 123f578d6dd3..4cfaaa5a49be 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -1107,52 +1107,14 @@ bool edma_filter_fn(struct dma_chan *chan, void *param) } EXPORT_SYMBOL(edma_filter_fn); -static struct platform_device *pdev0, *pdev1; - -static const struct platform_device_info edma_dev_info0 = { - .name = "edma-dma-engine", - .id = 0, - .dma_mask = DMA_BIT_MASK(32), -}; - -static const struct platform_device_info edma_dev_info1 = { - .name = "edma-dma-engine", - .id = 1, - .dma_mask = DMA_BIT_MASK(32), -}; - static int edma_init(void) { - int ret = platform_driver_register(&edma_driver); - - if (ret == 0) { - pdev0 = platform_device_register_full(&edma_dev_info0); - if (IS_ERR(pdev0)) { - platform_driver_unregister(&edma_driver); - ret = PTR_ERR(pdev0); - goto out; - } - } - - if (!of_have_populated_dt() && EDMA_CTLRS == 2) { - pdev1 = platform_device_register_full(&edma_dev_info1); - if (IS_ERR(pdev1)) { - platform_driver_unregister(&edma_driver); - platform_device_unregister(pdev0); - ret = PTR_ERR(pdev1); - } - } - -out: - return ret; + return platform_driver_register(&edma_driver); } subsys_initcall(edma_init); static void __exit edma_exit(void) { - platform_device_unregister(pdev0); - if (pdev1) - platform_device_unregister(pdev1); platform_driver_unregister(&edma_driver); } module_exit(edma_exit); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Unconditional registering EMDA platform devices 2014-10-24 16:14 ` Arnd Bergmann @ 2014-10-25 18:48 ` Uwe Kleine-König 2014-10-25 18:57 ` Arnd Bergmann 0 siblings, 1 reply; 5+ messages in thread From: Uwe Kleine-König @ 2014-10-25 18:48 UTC (permalink / raw) To: linux-arm-kernel Hello Arnd, On Fri, Oct 24, 2014 at 06:14:01PM +0200, Arnd Bergmann wrote: > On Friday 24 October 2014 16:29:04 Andrew Lunn wrote: > > Hi Matt > > > > I've had a report of a Debian kernel running on a Marvell XP system Not really relevant, but it's a Armada 370, not XP system. > > giving warnings: > > > > [ 0.114771] edma-dma-engine edma-dma-engine.0: Can't allocate PaRAM dummy slot > > [ 0.114794] edma-dma-engine: probe of edma-dma-engine.0 failed with error -5 > > > > These seem to be coming from drivers/dma/emda.c > > > > That driver has a subsys_initcall(edma_init); > > > > and the edma_init function is unconditionally registering a driver and > > a platform device. For a multiarch kernel, this is not a good idea. > > > > Please could you make this conditionally. Maybe look into the DT and > > see if the DMA is needed on the platform? > > I just looked at that code an I'm completely confused about how that > even works today. I do see that the driver is used on ATAGS based > davinci machines, which means we can't just look into the DT. > > The main problem seems to stem from arch/arm/common/edma.c being > half the driver that provides interfaces to both drivers/dma/edma.c > and to sound/soc/davinci/davinci-pcm.c, while drivers/dma/edma.c > is not really a driver by itself. My preferred solution to this would > be to move arch/arm/common/edma.c into drivers/dma/edma.c and still > have it export its private API, but I assume that the dmaengine > maintainers have already NAKed that approach. Isn't the preferred solution that sound/soc/davinci/davinci-pcm.c only uses dmaengine stuff and the private API goes away? > Would the approach below work? I didn't test it yet, but I assume it makes the warning disappear on machines without an "edma" platform device. So it would solve my problem. > 8<------- > Subject: dma: edma: move device registration to platform code > > The horrible split between the low-level part of the edma support > and the dmaengine front-end driver causes problems on multiplatform > kernels. This is an attempt to improve the situation slightly > by only registering the dmaengine devices that are actually > present. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > [...] > diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c > index 123f578d6dd3..4cfaaa5a49be 100644 > --- a/drivers/dma/edma.c > +++ b/drivers/dma/edma.c There is a comment in drivers/dma/edma.c reading: /* * This will go away when the private EDMA API is folded * into this driver and the platform device(s) are * instantiated in the arch code. We can only get away * with this simplification because DA8XX may not be built * in the same kernel image with other DaVinci parts. This * avoids having to sprinkle dmaengine driver platform devices * and data throughout all the existing board files. */ Just looking into arch/arm/mach-davinci/Kconfig it seems wrong that DA8XX may not be enabled with other DaVinci parts. So probably there is really more broken here ... Best regards Uwe ^ permalink raw reply [flat|nested] 5+ messages in thread
* Unconditional registering EMDA platform devices 2014-10-25 18:48 ` Uwe Kleine-König @ 2014-10-25 18:57 ` Arnd Bergmann 2014-11-06 10:22 ` Sekhar Nori 0 siblings, 1 reply; 5+ messages in thread From: Arnd Bergmann @ 2014-10-25 18:57 UTC (permalink / raw) To: linux-arm-kernel On Saturday 25 October 2014 20:48:54 Uwe Kleine-K?nig wrote: > On Fri, Oct 24, 2014 at 06:14:01PM +0200, Arnd Bergmann wrote: > > On Friday 24 October 2014 16:29:04 Andrew Lunn wrote: > > > giving warnings: > > > > > > [ 0.114771] edma-dma-engine edma-dma-engine.0: Can't allocate PaRAM dummy slot > > > [ 0.114794] edma-dma-engine: probe of edma-dma-engine.0 failed with error -5 > > > > > > These seem to be coming from drivers/dma/emda.c > > > > > > That driver has a subsys_initcall(edma_init); > > > > > > and the edma_init function is unconditionally registering a driver and > > > a platform device. For a multiarch kernel, this is not a good idea. > > > > > > Please could you make this conditionally. Maybe look into the DT and > > > see if the DMA is needed on the platform? > > > > I just looked at that code an I'm completely confused about how that > > even works today. I do see that the driver is used on ATAGS based > > davinci machines, which means we can't just look into the DT. > > > > The main problem seems to stem from arch/arm/common/edma.c being > > half the driver that provides interfaces to both drivers/dma/edma.c > > and to sound/soc/davinci/davinci-pcm.c, while drivers/dma/edma.c > > is not really a driver by itself. My preferred solution to this would > > be to move arch/arm/common/edma.c into drivers/dma/edma.c and still > > have it export its private API, but I assume that the dmaengine > > maintainers have already NAKed that approach. > Isn't the preferred solution that sound/soc/davinci/davinci-pcm.c only > uses dmaengine stuff and the private API goes away? Absolutely, yes. I believe all other drivers have been converted already, and it's on somebody's TODO list. > > 8<------- > > Subject: dma: edma: move device registration to platform code > > > > The horrible split between the low-level part of the edma support > > and the dmaengine front-end driver causes problems on multiplatform > > kernels. This is an attempt to improve the situation slightly > > by only registering the dmaengine devices that are actually > > present. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > [...] > > diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c > > index 123f578d6dd3..4cfaaa5a49be 100644 > > --- a/drivers/dma/edma.c > > +++ b/drivers/dma/edma.c > There is a comment in drivers/dma/edma.c reading: > > /* > * This will go away when the private EDMA API is folded > * into this driver and the platform device(s) are > * instantiated in the arch code. We can only get away > * with this simplification because DA8XX may not be built > * in the same kernel image with other DaVinci parts. This > * avoids having to sprinkle dmaengine driver platform devices > * and data throughout all the existing board files. > */ > > Just looking into arch/arm/mach-davinci/Kconfig it seems wrong that > DA8XX may not be enabled with other DaVinci parts. So probably there is > really more broken here ... Right, and it also seems that this would be fairly simple to fix as a follow-up patch. I already eliminated the only use of EDMA_CTLRS, and the da8xx type could be derived from the device identifier. Arnd ^ permalink raw reply [flat|nested] 5+ messages in thread
* Unconditional registering EMDA platform devices 2014-10-25 18:57 ` Arnd Bergmann @ 2014-11-06 10:22 ` Sekhar Nori 0 siblings, 0 replies; 5+ messages in thread From: Sekhar Nori @ 2014-11-06 10:22 UTC (permalink / raw) To: linux-arm-kernel + Peter, Vinod On Sunday 26 October 2014 12:27 AM, Arnd Bergmann wrote: > On Saturday 25 October 2014 20:48:54 Uwe Kleine-K?nig wrote: >> On Fri, Oct 24, 2014 at 06:14:01PM +0200, Arnd Bergmann wrote: >>> On Friday 24 October 2014 16:29:04 Andrew Lunn wrote: >>>> giving warnings: >>>> >>>> [ 0.114771] edma-dma-engine edma-dma-engine.0: Can't allocate PaRAM dummy slot >>>> [ 0.114794] edma-dma-engine: probe of edma-dma-engine.0 failed with error -5 >>>> >>>> These seem to be coming from drivers/dma/emda.c >>>> >>>> That driver has a subsys_initcall(edma_init); >>>> >>>> and the edma_init function is unconditionally registering a driver and >>>> a platform device. For a multiarch kernel, this is not a good idea. >>>> >>>> Please could you make this conditionally. Maybe look into the DT and >>>> see if the DMA is needed on the platform? >>> >>> I just looked at that code an I'm completely confused about how that >>> even works today. I do see that the driver is used on ATAGS based >>> davinci machines, which means we can't just look into the DT. >>> >>> The main problem seems to stem from arch/arm/common/edma.c being >>> half the driver that provides interfaces to both drivers/dma/edma.c >>> and to sound/soc/davinci/davinci-pcm.c, while drivers/dma/edma.c >>> is not really a driver by itself. My preferred solution to this would >>> be to move arch/arm/common/edma.c into drivers/dma/edma.c and still >>> have it export its private API, but I assume that the dmaengine >>> maintainers have already NAKed that approach. >> Isn't the preferred solution that sound/soc/davinci/davinci-pcm.c only >> uses dmaengine stuff and the private API goes away? > > Absolutely, yes. I believe all other drivers have been converted > already, and it's on somebody's TODO list. Yes, Peter was looking at removing the usage of private DMA API from davinci-pcm. https://lkml.org/lkml/2014/8/11/85 It is still on his TODO. > >>> 8<------- >>> Subject: dma: edma: move device registration to platform code >>> >>> The horrible split between the low-level part of the edma support >>> and the dmaengine front-end driver causes problems on multiplatform >>> kernels. This is an attempt to improve the situation slightly >>> by only registering the dmaengine devices that are actually >>> present. >>> >>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> I tested this patch on DA850 using MMC/SD as EDMA client and it worked fine. I think it will serve well as intermediate solution while Peter works on folding in arch/arm/common/edma.c into drivers/dma/edma.c I had to add the following patch on top to get DMA_BIT_MASK defined in edma.c ---8<--- diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c index f6cffee3c6ee..66725eb143ce 100644 --- a/arch/arm/common/edma.c +++ b/arch/arm/common/edma.c @@ -31,6 +31,7 @@ #include <linux/of_dma.h> #include <linux/of_irq.h> #include <linux/pm_runtime.h> +#include <linux/dma-mapping.h> #include <linux/platform_data/edma.h> ---8<--- In the same patch you can probably get rid of EDMA_CTLRS definition too as you noted below. If you decide to post it formally for Vinod to pick, you can add: Acked-by: Sekhar Nori <nsekhar@ti.com> >>> [...] >>> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c >>> index 123f578d6dd3..4cfaaa5a49be 100644 >>> --- a/drivers/dma/edma.c >>> +++ b/drivers/dma/edma.c >> There is a comment in drivers/dma/edma.c reading: >> >> /* >> * This will go away when the private EDMA API is folded >> * into this driver and the platform device(s) are >> * instantiated in the arch code. We can only get away >> * with this simplification because DA8XX may not be built >> * in the same kernel image with other DaVinci parts. This >> * avoids having to sprinkle dmaengine driver platform devices >> * and data throughout all the existing board files. >> */ >> >> Just looking into arch/arm/mach-davinci/Kconfig it seems wrong that >> DA8XX may not be enabled with other DaVinci parts. So probably there is >> really more broken here ... This was true when the comment was written. We have since moved to AUTO_ZRELADDR and a single config builds all DaVinci boards now. Thanks, Sekhar ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-11-06 10:22 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-24 14:29 Unconditional registering EMDA platform devices Andrew Lunn 2014-10-24 16:14 ` Arnd Bergmann 2014-10-25 18:48 ` Uwe Kleine-König 2014-10-25 18:57 ` Arnd Bergmann 2014-11-06 10:22 ` Sekhar Nori
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).