[alsa-devel] [PATCH 0/3] ARM: edma: Correct header file usage
Hi,
The linux/platform_data/edma.h file was used for API definition as well, which is not correct since the header should only contain platform data related structures, defines, etc.
Mark: I think this series can go via arm since it is just changing include stuff under ASoC.
Regards, Peter --- Peter Ujfalusi (3): ASoC: davinci-evm: Do not include edma headers ARM: edma: Rename header file for dmaengine filter function definition ARM: edma: Split up header file to platform_data and API file
arch/arm/common/edma.c | 3 +- arch/arm/mach-davinci/devices.c | 1 + arch/arm/mach-davinci/include/mach/da8xx.h | 1 + drivers/dma/edma.c | 2 +- drivers/mmc/host/davinci_mmc.c | 3 +- drivers/spi/spi-davinci.c | 2 +- include/linux/edma-dmaengine.h | 29 ++++++ include/linux/edma.h | 152 ++++++++++++++++++++++++++--- include/linux/platform_data/edma.h | 148 ++-------------------------- sound/soc/davinci/davinci-evm.c | 3 - sound/soc/davinci/davinci-pcm.h | 1 + sound/soc/davinci/edma-pcm.c | 2 +- 12 files changed, 182 insertions(+), 165 deletions(-) create mode 100644 include/linux/edma-dmaengine.h
The machine driver has no business with the underlying dma.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/davinci/davinci-evm.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c index 158cb3d1db70..d1a8b6e3ac3b 100644 --- a/sound/soc/davinci/davinci-evm.c +++ b/sound/soc/davinci/davinci-evm.c @@ -14,7 +14,6 @@ #include <linux/timer.h> #include <linux/interrupt.h> #include <linux/platform_device.h> -#include <linux/platform_data/edma.h> #include <linux/i2c.h> #include <linux/of_platform.h> #include <linux/clk.h> @@ -25,8 +24,6 @@ #include <asm/dma.h> #include <asm/mach-types.h>
-#include <linux/edma.h> - #include "davinci-pcm.h" #include "davinci-i2s.h"
On Thu, Nov 27, 2014 at 12:41:29PM +0200, Peter Ujfalusi wrote:
The machine driver has no business with the underlying dma.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
Acked-by: Mark Brown broonie@kernel.org
Rename the include/linux/edma.h to include/linux/edma-dmaengine.h
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/common/edma.c | 2 +- drivers/mmc/host/davinci_mmc.c | 3 +-- drivers/spi/spi-davinci.c | 2 +- include/linux/edma-dmaengine.h | 29 +++++++++++++++++++++++++++++ include/linux/edma.h | 29 ----------------------------- sound/soc/davinci/edma-pcm.c | 2 +- 6 files changed, 33 insertions(+), 34 deletions(-) create mode 100644 include/linux/edma-dmaengine.h delete mode 100644 include/linux/edma.h
diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c index 5662a872689b..bac677e65c76 100644 --- a/arch/arm/common/edma.c +++ b/arch/arm/common/edma.c @@ -25,7 +25,7 @@ #include <linux/platform_device.h> #include <linux/io.h> #include <linux/slab.h> -#include <linux/edma.h> +#include <linux/edma-dmaengine.h> #include <linux/dma-mapping.h> #include <linux/of_address.h> #include <linux/of_device.h> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c index 1625f908dc70..47323662c818 100644 --- a/drivers/mmc/host/davinci_mmc.c +++ b/drivers/mmc/host/davinci_mmc.c @@ -32,12 +32,11 @@ #include <linux/delay.h> #include <linux/dmaengine.h> #include <linux/dma-mapping.h> -#include <linux/edma.h> +#include <linux/edma-dmaengine.h> #include <linux/mmc/mmc.h> #include <linux/of.h> #include <linux/of_device.h>
-#include <linux/platform_data/edma.h> #include <linux/platform_data/mmc-davinci.h>
/* diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c index b3707badb1e5..d61b7cdb2deb 100644 --- a/drivers/spi/spi-davinci.c +++ b/drivers/spi/spi-davinci.c @@ -27,7 +27,7 @@ #include <linux/clk.h> #include <linux/dmaengine.h> #include <linux/dma-mapping.h> -#include <linux/edma.h> +#include <linux/edma-dmaengine.h> #include <linux/of.h> #include <linux/of_device.h> #include <linux/of_gpio.h> diff --git a/include/linux/edma-dmaengine.h b/include/linux/edma-dmaengine.h new file mode 100644 index 000000000000..8a2602809a77 --- /dev/null +++ b/include/linux/edma-dmaengine.h @@ -0,0 +1,29 @@ +/* + * TI EDMA DMA engine driver + * + * Copyright 2012 Texas Instruments + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation version 2. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ +#ifndef __LINUX_EDMA_DMAENGINE_H +#define __LINUX_EDMA_DMAENGINE_H + +struct dma_chan; + +#if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE) +bool edma_filter_fn(struct dma_chan *, void *); +#else +static inline bool edma_filter_fn(struct dma_chan *chan, void *param) +{ + return false; +} +#endif + +#endif diff --git a/include/linux/edma.h b/include/linux/edma.h deleted file mode 100644 index a1307e7827e8..000000000000 --- a/include/linux/edma.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * TI EDMA DMA engine driver - * - * Copyright 2012 Texas Instruments - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation version 2. - * - * This program is distributed "as is" WITHOUT ANY WARRANTY of any - * kind, whether express or implied; without even the implied warranty - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ -#ifndef __LINUX_EDMA_H -#define __LINUX_EDMA_H - -struct dma_chan; - -#if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE) -bool edma_filter_fn(struct dma_chan *, void *); -#else -static inline bool edma_filter_fn(struct dma_chan *chan, void *param) -{ - return false; -} -#endif - -#endif diff --git a/sound/soc/davinci/edma-pcm.c b/sound/soc/davinci/edma-pcm.c index 59e588abe54b..873c8a090427 100644 --- a/sound/soc/davinci/edma-pcm.c +++ b/sound/soc/davinci/edma-pcm.c @@ -23,7 +23,7 @@ #include <sound/pcm_params.h> #include <sound/soc.h> #include <sound/dmaengine_pcm.h> -#include <linux/edma.h> +#include <linux/edma-dmaengine.h>
#include "edma-pcm.h"
On Thursday 27 November 2014 12:41:30 Peter Ujfalusi wrote:
diff --git a/include/linux/edma-dmaengine.h b/include/linux/edma-dmaengine.h new file mode 100644 index 000000000000..8a2602809a77 --- /dev/null +++ b/include/linux/edma-dmaengine.h @@ -0,0 +1,29 @@ +struct dma_chan;
+#if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE) +bool edma_filter_fn(struct dma_chan *, void *); +#else +static inline bool edma_filter_fn(struct dma_chan *chan, void *param) +{
- return false;
+} +#endif
+#endif
It's better not to hardcode the name of the filter function in a driver, better move the filter function into platform code and pass it down to the driver in the platform_data pointer in davinci_mmc_config and davinci_spi_platform_data to get rid of this dependency.
Something roughly like the patch below.
Arnd
--- [DRAFT] ARM: davinci: pass dma config through platform data
DMA slave drivers are supposed to be written independent of the dma engine driver, which means that the mmc and spi drivers on davinci should not reference the edma_filter_fn or know about the specific format of the data passed into it.
This changes the channel data from a resource into a void pointer and passes it as platform data along with the filter function.
TODO: do the same thing for SPI, and adapt the filter function for the new format.
Signed-off-by: Arnd Bergmann arnd@arndb.de
arch/arm/mach-davinci/devices-da8xx.c | 26 ++++++-------------------- arch/arm/mach-davinci/devices.c | 22 ++++++---------------- drivers/mmc/host/davinci_mmc.c | 27 +++++++++++---------------- include/linux/platform_data/mmc-davinci.h | 5 +++++ 4 files changed, 28 insertions(+), 52 deletions(-)
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c index b85b781b05fd..4aa872dc6dc3 100644 --- a/arch/arm/mach-davinci/devices-da8xx.c +++ b/arch/arm/mach-davinci/devices-da8xx.c @@ -671,16 +671,6 @@ static struct resource da8xx_mmcsd0_resources[] = { .end = IRQ_DA8XX_MMCSDINT0, .flags = IORESOURCE_IRQ, }, - { /* DMA RX */ - .start = DA8XX_DMA_MMCSD0_RX, - .end = DA8XX_DMA_MMCSD0_RX, - .flags = IORESOURCE_DMA, - }, - { /* DMA TX */ - .start = DA8XX_DMA_MMCSD0_TX, - .end = DA8XX_DMA_MMCSD0_TX, - .flags = IORESOURCE_DMA, - }, };
static struct platform_device da8xx_mmcsd0_device = { @@ -693,6 +683,9 @@ static struct platform_device da8xx_mmcsd0_device = { int __init da8xx_register_mmcsd0(struct davinci_mmc_config *config) { da8xx_mmcsd0_device.dev.platform_data = config; + config->filter = edma_filter_fn; + config->rxdma = (void *)DA8XX_DMA_MMCSD0_RX; + config->txdma = (void *)DA8XX_DMA_MMCSD0_TX; return platform_device_register(&da8xx_mmcsd0_device); }
@@ -708,16 +701,6 @@ static struct resource da850_mmcsd1_resources[] = { .end = IRQ_DA850_MMCSDINT0_1, .flags = IORESOURCE_IRQ, }, - { /* DMA RX */ - .start = DA850_DMA_MMCSD1_RX, - .end = DA850_DMA_MMCSD1_RX, - .flags = IORESOURCE_DMA, - }, - { /* DMA TX */ - .start = DA850_DMA_MMCSD1_TX, - .end = DA850_DMA_MMCSD1_TX, - .flags = IORESOURCE_DMA, - }, };
static struct platform_device da850_mmcsd1_device = { @@ -730,6 +713,9 @@ static struct platform_device da850_mmcsd1_device = { int __init da850_register_mmcsd1(struct davinci_mmc_config *config) { da850_mmcsd1_device.dev.platform_data = config; + config->filter = edma_filter_fn; + config->rxdma = (void *)DA8XX_DMA_MMCSD1_RX; + config->txdma = (void *)DA8XX_DMA_MMCSD1_TX; return platform_device_register(&da850_mmcsd1_device); } #endif diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c index 6257aa452568..7b7039d9b079 100644 --- a/arch/arm/mach-davinci/devices.c +++ b/arch/arm/mach-davinci/devices.c @@ -144,14 +144,6 @@ static struct resource mmcsd0_resources[] = { .start = IRQ_SDIOINT, .flags = IORESOURCE_IRQ, }, - /* DMA channels: RX, then TX */ - { - .start = EDMA_CTLR_CHAN(0, DAVINCI_DMA_MMCRXEVT), - .flags = IORESOURCE_DMA, - }, { - .start = EDMA_CTLR_CHAN(0, DAVINCI_DMA_MMCTXEVT), - .flags = IORESOURCE_DMA, - }, };
static struct platform_device davinci_mmcsd0_device = { @@ -181,14 +173,6 @@ static struct resource mmcsd1_resources[] = { .start = IRQ_DM355_SDIOINT1, .flags = IORESOURCE_IRQ, }, - /* DMA channels: RX, then TX */ - { - .start = EDMA_CTLR_CHAN(0, 30), /* rx */ - .flags = IORESOURCE_DMA, - }, { - .start = EDMA_CTLR_CHAN(0, 31), /* tx */ - .flags = IORESOURCE_DMA, - }, };
static struct platform_device davinci_mmcsd1_device = { @@ -218,6 +202,9 @@ void __init davinci_setup_mmc(int module, struct davinci_mmc_config *config) */ switch (module) { case 1: + config->filter = edma_filter_fn; + config->rxdma = (void *)EDMA_CTLR_CHAN(0, DAVINCI_DMA_MMCRXEVT); + config->txdma = (void *)EDMA_CTLR_CHAN(0, DAVINCI_DMA_MMCTXEVT); if (cpu_is_davinci_dm355()) { /* REVISIT we may not need all these pins if e.g. this * is a hard-wired SDIO device... @@ -247,6 +234,9 @@ void __init davinci_setup_mmc(int module, struct davinci_mmc_config *config) pdev = &davinci_mmcsd1_device; break; case 0: + config->filter = edma_filter_fn; + config->rxdma = (void *)EDMA_CTLR_CHAN(0, 30); + config->txdma = (void *)EDMA_CTLR_CHAN(0, 31); if (cpu_is_davinci_dm355()) { mmcsd0_resources[0].start = DM355_MMCSD0_BASE; mmcsd0_resources[0].end = DM355_MMCSD0_BASE + SZ_4K - 1; diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c index 1625f908dc70..3f25be9942c8 100644 --- a/drivers/mmc/host/davinci_mmc.c +++ b/drivers/mmc/host/davinci_mmc.c @@ -202,7 +202,8 @@ struct mmc_davinci_host { u32 buffer_bytes_left; u32 bytes_left;
- u32 rxdma, txdma; + dma_filter_fn filter; + void *rxdma, *txdma; struct dma_chan *dma_tx; struct dma_chan *dma_rx; bool use_dma; @@ -524,16 +525,16 @@ static int __init davinci_acquire_dma_channels(struct mmc_davinci_host *host) dma_cap_set(DMA_SLAVE, mask);
host->dma_tx = - dma_request_slave_channel_compat(mask, edma_filter_fn, - &host->txdma, mmc_dev(host->mmc), "tx"); + dma_request_slave_channel_compat(mask, host->filter, + host->txdma, mmc_dev(host->mmc), "tx"); if (!host->dma_tx) { dev_err(mmc_dev(host->mmc), "Can't get dma_tx channel\n"); return -ENODEV; }
host->dma_rx = - dma_request_slave_channel_compat(mask, edma_filter_fn, - &host->rxdma, mmc_dev(host->mmc), "rx"); + dma_request_slave_channel_compat(mask, host->filter, + host->rxdma, mmc_dev(host->mmc), "rx"); if (!host->dma_rx) { dev_err(mmc_dev(host->mmc), "Can't get dma_rx channel\n"); r = -ENODEV; @@ -1262,17 +1263,11 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev) host = mmc_priv(mmc); host->mmc = mmc; /* Important */
- r = platform_get_resource(pdev, IORESOURCE_DMA, 0); - if (!r) - dev_warn(&pdev->dev, "RX DMA resource not specified\n"); - else - host->rxdma = r->start; - - r = platform_get_resource(pdev, IORESOURCE_DMA, 1); - if (!r) - dev_warn(&pdev->dev, "TX DMA resource not specified\n"); - else - host->txdma = r->start; + if (pdata) { + host->filter = pdata->filter; + host->rxdma = pdata->rxdma; + host->txdma = pdata->txdma; + }
host->mem_res = mem; host->base = ioremap(mem->start, mem_size); diff --git a/include/linux/platform_data/mmc-davinci.h b/include/linux/platform_data/mmc-davinci.h index 9cea4ee377b5..0e5809bcaff0 100644 --- a/include/linux/platform_data/mmc-davinci.h +++ b/include/linux/platform_data/mmc-davinci.h @@ -25,6 +25,11 @@ struct davinci_mmc_config {
/* Number of sg segments */ u8 nr_sg; + + /* DMA setup */ + dma_filter_fn filter; + void *rxdma; + void *txdma; }; void davinci_setup_mmc(int module, struct davinci_mmc_config *config);
On 11/27/2014 01:14 PM, Arnd Bergmann wrote:
On Thursday 27 November 2014 12:41:30 Peter Ujfalusi wrote:
diff --git a/include/linux/edma-dmaengine.h b/include/linux/edma-dmaengine.h new file mode 100644 index 000000000000..8a2602809a77 --- /dev/null +++ b/include/linux/edma-dmaengine.h @@ -0,0 +1,29 @@ +struct dma_chan;
+#if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE) +bool edma_filter_fn(struct dma_chan *, void *); +#else +static inline bool edma_filter_fn(struct dma_chan *chan, void *param) +{
- return false;
+} +#endif
+#endif
It's better not to hardcode the name of the filter function in a driver, better move the filter function into platform code and pass it down to the driver in the platform_data pointer in davinci_mmc_config and davinci_spi_platform_data to get rid of this dependency.
Something roughly like the patch below.
This will only work in case of legacy boot. When booting with DT we do not have pdata and after this patch in dt boot we are not going to be able to get the DMA resources either.
I think if we want to do something like this, it has to be done within the dmaengine framework. The dma controller's of_dma_filter_info already have .filter_fn which could be used by the framework.
On the other hand the davinci_mmc (and spi-davinci) is only going to work on davinci devices, so I don't really see issue with 'hard coding' davinci related callback in the code.
On Thursday 27 November 2014 16:23:31 Peter Ujfalusi wrote:
This will only work in case of legacy boot. When booting with DT we do not have pdata and after this patch in dt boot we are not going to be able to get the DMA resources either.
No, when booting with DT, the filter_fn and data are not used at all, we get the dma channel by parsing the DT instead.
I think if we want to do something like this, it has to be done within the dmaengine framework. The dma controller's of_dma_filter_info already have .filter_fn which could be used by the framework.
No, of_dma_filter_info/of_dma_simple_xlate was a mistake, we should never have even introduced that. All drivers that rely on this can simply provide their own xlate function that calls of_dma_get_slave_channel() or one of the related functions.
edma is particularly trivial, it can just use of_dma_xlate_by_chan_id() instead of of_dma_simple_xlate, as it looks up the channel by its number.
Arnd
On 11/27/2014 04:50 PM, Arnd Bergmann wrote:
On Thursday 27 November 2014 16:23:31 Peter Ujfalusi wrote:
This will only work in case of legacy boot. When booting with DT we do not have pdata and after this patch in dt boot we are not going to be able to get the DMA resources either.
No, when booting with DT, the filter_fn and data are not used at all, we get the dma channel by parsing the DT instead.
Correct.
I think if we want to do something like this, it has to be done within the dmaengine framework. The dma controller's of_dma_filter_info already have .filter_fn which could be used by the framework.
No, of_dma_filter_info/of_dma_simple_xlate was a mistake, we should never have even introduced that. All drivers that rely on this can simply provide their own xlate function that calls of_dma_get_slave_channel() or one of the related functions.
edma is particularly trivial, it can just use of_dma_xlate_by_chan_id() instead of of_dma_simple_xlate, as it looks up the channel by its number.
I see. With this series I did not planed to fix all edma related issues, just as a start clean up the related header files. I would rather not add fixes to mmc, spi, etc drivers since while you have valid point it is not in the scope of this series. Can we do the changes you are suggesting in an incremental manner?
On Thursday 27 November 2014 20:46:12 Peter Ujfalusi wrote:
I see. With this series I did not planed to fix all edma related issues, just as a start clean up the related header files. I would rather not add fixes to mmc, spi, etc drivers since while you have valid point it is not in the scope of this series. Can we do the changes you are suggesting in an incremental manner?
Sure, but I'd leave the existing filter function declaration alone then and not move it, since we wouldn't want to keep it in the long run.
Arnd
On 11/27/2014 11:52 PM, Arnd Bergmann wrote:
On Thursday 27 November 2014 20:46:12 Peter Ujfalusi wrote:
I see. With this series I did not planed to fix all edma related issues, just as a start clean up the related header files. I would rather not add fixes to mmc, spi, etc drivers since while you have valid point it is not in the scope of this series. Can we do the changes you are suggesting in an incremental manner?
Sure, but I'd leave the existing filter function declaration alone then and not move it, since we wouldn't want to keep it in the long run.
but if you want to reference the filter function (which is in drivers/dma/edma.c) in arch/arm/mach-davinci/ directory, we will need it. Don't we?
If I leave the header as it is, then how would we clean up the edma headers? I would not put the API definitions for the arch code into the same file as we have the filter definition.
Arnd
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 28 November 2014 09:16:24 Peter Ujfalusi wrote:
On 11/27/2014 11:52 PM, Arnd Bergmann wrote:
On Thursday 27 November 2014 20:46:12 Peter Ujfalusi wrote:
I see. With this series I did not planed to fix all edma related issues, just as a start clean up the related header files. I would rather not add fixes to mmc, spi, etc drivers since while you have valid point it is not in the scope of this series. Can we do the changes you are suggesting in an incremental manner?
Sure, but I'd leave the existing filter function declaration alone then and not move it, since we wouldn't want to keep it in the long run.
but if you want to reference the filter function (which is in drivers/dma/edma.c) in arch/arm/mach-davinci/ directory, we will need it. Don't we?
Yes, unless you move the definition of the filter function into arch/arm/common/edma.c or arch/arm/mach-davinci/devices.c, but that would require other changes.
If I leave the header as it is, then how would we clean up the edma headers? I would not put the API definitions for the arch code into the same file as we have the filter definition.
Ok, just go ahead with your current patch then, we can always follow up. The most important cleanup for edma is elsewhere anyway, so once the asoc drivers can use the dmaengine interface, this should be easier.
Arnd
On 11/28/2014 12:51 PM, Arnd Bergmann wrote:
On Friday 28 November 2014 09:16:24 Peter Ujfalusi wrote:
On 11/27/2014 11:52 PM, Arnd Bergmann wrote:
On Thursday 27 November 2014 20:46:12 Peter Ujfalusi wrote:
I see. With this series I did not planed to fix all edma related issues, just as a start clean up the related header files. I would rather not add fixes to mmc, spi, etc drivers since while you have valid point it is not in the scope of this series. Can we do the changes you are suggesting in an incremental manner?
Sure, but I'd leave the existing filter function declaration alone then and not move it, since we wouldn't want to keep it in the long run.
but if you want to reference the filter function (which is in drivers/dma/edma.c) in arch/arm/mach-davinci/ directory, we will need it. Don't we?
Yes, unless you move the definition of the filter function into arch/arm/common/edma.c or arch/arm/mach-davinci/devices.c, but that would require other changes.
At the end the aim is to get rid of the edma code form arch/arm and have only dmaengine API towards eDMA. The ASoC davinci-pcm is the only user of the legacy API AFAIK. It has a mode called ping-pong which is not possible with the dmaeingine at all. This is to overcome underflow situations on parts where the audio IP does not have FIFO. My edma-pcm (which is using dmaengine) should be able to handle this situation, but I need to verify it before I can remove the davinci-pcm and then we can get rid of the direct eDMA API and code.
If I leave the header as it is, then how would we clean up the edma headers? I would not put the API definitions for the arch code into the same file as we have the filter definition.
Ok, just go ahead with your current patch then, we can always follow up. The most important cleanup for edma is elsewhere anyway, so once the asoc drivers can use the dmaengine interface, this should be easier.
Arnd
On 27 November 2014 at 11:41, Peter Ujfalusi peter.ujfalusi@ti.com wrote:
Rename the include/linux/edma.h to include/linux/edma-dmaengine.h
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
For the mmc parts:
Acked-by: Ulf Hansson ulf.hansson@linaro.org
arch/arm/common/edma.c | 2 +- drivers/mmc/host/davinci_mmc.c | 3 +-- drivers/spi/spi-davinci.c | 2 +- include/linux/edma-dmaengine.h | 29 +++++++++++++++++++++++++++++ include/linux/edma.h | 29 ----------------------------- sound/soc/davinci/edma-pcm.c | 2 +- 6 files changed, 33 insertions(+), 34 deletions(-) create mode 100644 include/linux/edma-dmaengine.h delete mode 100644 include/linux/edma.h
diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c index 5662a872689b..bac677e65c76 100644 --- a/arch/arm/common/edma.c +++ b/arch/arm/common/edma.c @@ -25,7 +25,7 @@ #include <linux/platform_device.h> #include <linux/io.h> #include <linux/slab.h> -#include <linux/edma.h> +#include <linux/edma-dmaengine.h> #include <linux/dma-mapping.h> #include <linux/of_address.h> #include <linux/of_device.h> diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c index 1625f908dc70..47323662c818 100644 --- a/drivers/mmc/host/davinci_mmc.c +++ b/drivers/mmc/host/davinci_mmc.c @@ -32,12 +32,11 @@ #include <linux/delay.h> #include <linux/dmaengine.h> #include <linux/dma-mapping.h> -#include <linux/edma.h> +#include <linux/edma-dmaengine.h> #include <linux/mmc/mmc.h> #include <linux/of.h> #include <linux/of_device.h>
-#include <linux/platform_data/edma.h> #include <linux/platform_data/mmc-davinci.h>
/* diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c index b3707badb1e5..d61b7cdb2deb 100644 --- a/drivers/spi/spi-davinci.c +++ b/drivers/spi/spi-davinci.c @@ -27,7 +27,7 @@ #include <linux/clk.h> #include <linux/dmaengine.h> #include <linux/dma-mapping.h> -#include <linux/edma.h> +#include <linux/edma-dmaengine.h> #include <linux/of.h> #include <linux/of_device.h> #include <linux/of_gpio.h> diff --git a/include/linux/edma-dmaengine.h b/include/linux/edma-dmaengine.h new file mode 100644 index 000000000000..8a2602809a77 --- /dev/null +++ b/include/linux/edma-dmaengine.h @@ -0,0 +1,29 @@ +/*
- TI EDMA DMA engine driver
- Copyright 2012 Texas Instruments
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation version 2.
- This program is distributed "as is" WITHOUT ANY WARRANTY of any
- kind, whether express or implied; without even the implied warranty
- of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- */
+#ifndef __LINUX_EDMA_DMAENGINE_H +#define __LINUX_EDMA_DMAENGINE_H
+struct dma_chan;
+#if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE) +bool edma_filter_fn(struct dma_chan *, void *); +#else +static inline bool edma_filter_fn(struct dma_chan *chan, void *param) +{
return false;+} +#endif
+#endif diff --git a/include/linux/edma.h b/include/linux/edma.h deleted file mode 100644 index a1307e7827e8..000000000000 --- a/include/linux/edma.h +++ /dev/null @@ -1,29 +0,0 @@ -/*
- TI EDMA DMA engine driver
- Copyright 2012 Texas Instruments
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation version 2.
- This program is distributed "as is" WITHOUT ANY WARRANTY of any
- kind, whether express or implied; without even the implied warranty
- of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- */
-#ifndef __LINUX_EDMA_H -#define __LINUX_EDMA_H
-struct dma_chan;
-#if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE) -bool edma_filter_fn(struct dma_chan *, void *); -#else -static inline bool edma_filter_fn(struct dma_chan *chan, void *param) -{
return false;-} -#endif
-#endif diff --git a/sound/soc/davinci/edma-pcm.c b/sound/soc/davinci/edma-pcm.c index 59e588abe54b..873c8a090427 100644 --- a/sound/soc/davinci/edma-pcm.c +++ b/sound/soc/davinci/edma-pcm.c @@ -23,7 +23,7 @@ #include <sound/pcm_params.h> #include <sound/soc.h> #include <sound/dmaengine_pcm.h> -#include <linux/edma.h> +#include <linux/edma-dmaengine.h>
#include "edma-pcm.h"
-- 2.1.3
include/linux/platform_data/ is not a correct place to keep the API definitions for edma, it is meant to be only for the pdata for the device. Clean up this by moving the API to include/linux/edma.h
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/common/edma.c | 3 +- arch/arm/mach-davinci/devices.c | 1 + arch/arm/mach-davinci/include/mach/da8xx.h | 1 + drivers/dma/edma.c | 2 +- include/linux/edma.h | 153 +++++++++++++++++++++++++++++ include/linux/platform_data/edma.h | 148 ++-------------------------- sound/soc/davinci/davinci-pcm.h | 1 + 7 files changed, 165 insertions(+), 144 deletions(-) create mode 100644 include/linux/edma.h
diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c index bac677e65c76..6c49887d326e 100644 --- a/arch/arm/common/edma.c +++ b/arch/arm/common/edma.c @@ -25,6 +25,7 @@ #include <linux/platform_device.h> #include <linux/io.h> #include <linux/slab.h> +#include <linux/edma.h> #include <linux/edma-dmaengine.h> #include <linux/dma-mapping.h> #include <linux/of_address.h> @@ -33,8 +34,6 @@ #include <linux/of_irq.h> #include <linux/pm_runtime.h>
-#include <linux/platform_data/edma.h> - /* Offsets matching "struct edmacc_param" */ #define PARM_OPT 0x00 #define PARM_SRC 0x04 diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c index 6257aa452568..28572ef24d9c 100644 --- a/arch/arm/mach-davinci/devices.c +++ b/arch/arm/mach-davinci/devices.c @@ -23,6 +23,7 @@ #include <linux/platform_data/mmc-davinci.h> #include <mach/time.h> #include <linux/platform_data/edma.h> +#include <linux/edma.h>
#include "davinci.h" diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h index 39e58b48e826..313129cb6f4a 100644 --- a/arch/arm/mach-davinci/include/mach/da8xx.h +++ b/arch/arm/mach-davinci/include/mach/da8xx.h @@ -23,6 +23,7 @@ #include <mach/serial.h> #include <mach/pm.h> #include <linux/platform_data/edma.h> +#include <linux/edma.h> #include <linux/platform_data/i2c-davinci.h> #include <linux/platform_data/mmc-davinci.h> #include <linux/platform_data/usb-davinci.h> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 8880d6977e5d..126048e79dfc 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -25,7 +25,7 @@ #include <linux/spinlock.h> #include <linux/of.h>
-#include <linux/platform_data/edma.h> +#include <linux/edma.h>
#include "dmaengine.h" #include "virt-dma.h" diff --git a/include/linux/edma.h b/include/linux/edma.h new file mode 100644 index 000000000000..9df92198c117 --- /dev/null +++ b/include/linux/edma.h @@ -0,0 +1,153 @@ +/* + * TI EDMA definitions + * + * Copyright (C) 2006-2013 Texas Instruments. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation version 2. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +/* + * This EDMA3 programming framework exposes two basic kinds of resource: + * + * Channel Triggers transfers, usually from a hardware event but + * also manually or by "chaining" from DMA completions. + * Each channel is coupled to a Parameter RAM (PaRAM) slot. + * + * Slot Each PaRAM slot holds a DMA transfer descriptor (PaRAM + * "set"), source and destination addresses, a link to a + * next PaRAM slot (if any), options for the transfer, and + * instructions for updating those addresses. There are + * more than twice as many slots as event channels. + * + * Each PaRAM set describes a sequence of transfers, either for one large + * buffer or for several discontiguous smaller buffers. An EDMA transfer + * is driven only from a channel, which performs the transfers specified + * in its PaRAM slot until there are no more transfers. When that last + * transfer completes, the "link" field may be used to reload the channel's + * PaRAM slot with a new transfer descriptor. + * + * The EDMA Channel Controller (CC) maps requests from channels into physical + * Transfer Controller (TC) requests when the channel triggers (by hardware + * or software events, or by chaining). The two physical DMA channels provided + * by the TCs are thus shared by many logical channels. + * + * DaVinci hardware also has a "QDMA" mechanism which is not currently + * supported through this interface. (DSP firmware uses it though.) + */ + +#ifndef __LINUX_EDMA_H_ +#define __LINUX_EDMA_H_ + +#include <linux/platform_data/edma.h> + +/* PaRAM slots are laid out like this */ +struct edmacc_param { + u32 opt; + u32 src; + u32 a_b_cnt; + u32 dst; + u32 src_dst_bidx; + u32 link_bcntrld; + u32 src_dst_cidx; + u32 ccnt; +} __packed; + +/* fields in edmacc_param.opt */ +#define SAM BIT(0) +#define DAM BIT(1) +#define SYNCDIM BIT(2) +#define STATIC BIT(3) +#define EDMA_FWID (0x07 << 8) +#define TCCMODE BIT(11) +#define EDMA_TCC(t) ((t) << 12) +#define TCINTEN BIT(20) +#define ITCINTEN BIT(21) +#define TCCHEN BIT(22) +#define ITCCHEN BIT(23) + +/* ch_status parameter of callback function possible values*/ +#define EDMA_DMA_COMPLETE 1 +#define EDMA_DMA_CC_ERROR 2 +#define EDMA_DMA_TC1_ERROR 3 +#define EDMA_DMA_TC2_ERROR 4 + +enum address_mode { + INCR = 0, + FIFO = 1 +}; + +enum fifo_width { + W8BIT = 0, + W16BIT = 1, + W32BIT = 2, + W64BIT = 3, + W128BIT = 4, + W256BIT = 5 +}; + +enum sync_dimension { + ASYNC = 0, + ABSYNC = 1 +}; + +#define EDMA_CTLR_CHAN(ctlr, chan) (((ctlr) << 16) | (chan)) +#define EDMA_CTLR(i) ((i) >> 16) +#define EDMA_CHAN_SLOT(i) ((i) & 0xffff) + +#define EDMA_CHANNEL_ANY -1 /* for edma_alloc_channel() */ +#define EDMA_SLOT_ANY -1 /* for edma_alloc_slot() */ +#define EDMA_CONT_PARAMS_ANY 1001 +#define EDMA_CONT_PARAMS_FIXED_EXACT 1002 +#define EDMA_CONT_PARAMS_FIXED_NOT_EXACT 1003 + +/* alloc/free DMA channels and their dedicated parameter RAM slots */ +int edma_alloc_channel(int channel, + void (*callback)(unsigned channel, u16 ch_status, void *data), + void *data, enum dma_event_q); +void edma_free_channel(unsigned channel); + +/* alloc/free parameter RAM slots */ +int edma_alloc_slot(unsigned ctlr, int slot); +void edma_free_slot(unsigned slot); + +/* alloc/free a set of contiguous parameter RAM slots */ +int edma_alloc_cont_slots(unsigned ctlr, unsigned int id, int slot, int count); +int edma_free_cont_slots(unsigned slot, int count); + +/* calls that operate on part of a parameter RAM slot */ +void edma_set_src(unsigned slot, dma_addr_t src_port, enum address_mode mode, + enum fifo_width); +void edma_set_dest(unsigned slot, dma_addr_t dest_port, enum address_mode mode, + enum fifo_width); +dma_addr_t edma_get_position(unsigned slot, bool dst); +void edma_set_src_index(unsigned slot, s16 src_bidx, s16 src_cidx); +void edma_set_dest_index(unsigned slot, s16 dest_bidx, s16 dest_cidx); +void edma_set_transfer_params(unsigned slot, u16 acnt, u16 bcnt, u16 ccnt, + u16 bcnt_rld, enum sync_dimension sync_mode); +void edma_link(unsigned from, unsigned to); +void edma_unlink(unsigned from); + +/* calls that operate on an entire parameter RAM slot */ +void edma_write_slot(unsigned slot, const struct edmacc_param *params); +void edma_read_slot(unsigned slot, struct edmacc_param *params); + +/* channel control operations */ +int edma_start(unsigned channel); +void edma_stop(unsigned channel); +void edma_clean_channel(unsigned channel); +void edma_clear_event(unsigned channel); +void edma_pause(unsigned channel); +void edma_resume(unsigned channel); + +void edma_assign_channel_eventq(unsigned channel, enum dma_event_q eventq_no); + +int edma_trigger_channel(unsigned); + +#endif /* __LINUX_EDMA_H_ */ diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h index bdb2710e2aab..aaf6fc5769eb 100644 --- a/include/linux/platform_data/edma.h +++ b/include/linux/platform_data/edma.h @@ -8,83 +8,10 @@ * Free Software Foundation; either version 2 of the License, or (at your * option) any later version. */ - -/* - * This EDMA3 programming framework exposes two basic kinds of resource: - * - * Channel Triggers transfers, usually from a hardware event but - * also manually or by "chaining" from DMA completions. - * Each channel is coupled to a Parameter RAM (PaRAM) slot. - * - * Slot Each PaRAM slot holds a DMA transfer descriptor (PaRAM - * "set"), source and destination addresses, a link to a - * next PaRAM slot (if any), options for the transfer, and - * instructions for updating those addresses. There are - * more than twice as many slots as event channels. - * - * Each PaRAM set describes a sequence of transfers, either for one large - * buffer or for several discontiguous smaller buffers. An EDMA transfer - * is driven only from a channel, which performs the transfers specified - * in its PaRAM slot until there are no more transfers. When that last - * transfer completes, the "link" field may be used to reload the channel's - * PaRAM slot with a new transfer descriptor. - * - * The EDMA Channel Controller (CC) maps requests from channels into physical - * Transfer Controller (TC) requests when the channel triggers (by hardware - * or software events, or by chaining). The two physical DMA channels provided - * by the TCs are thus shared by many logical channels. - * - * DaVinci hardware also has a "QDMA" mechanism which is not currently - * supported through this interface. (DSP firmware uses it though.) - */ - #ifndef EDMA_H_ #define EDMA_H_
-/* PaRAM slots are laid out like this */ -struct edmacc_param { - u32 opt; - u32 src; - u32 a_b_cnt; - u32 dst; - u32 src_dst_bidx; - u32 link_bcntrld; - u32 src_dst_cidx; - u32 ccnt; -} __packed; - -/* fields in edmacc_param.opt */ -#define SAM BIT(0) -#define DAM BIT(1) -#define SYNCDIM BIT(2) -#define STATIC BIT(3) -#define EDMA_FWID (0x07 << 8) -#define TCCMODE BIT(11) -#define EDMA_TCC(t) ((t) << 12) -#define TCINTEN BIT(20) -#define ITCINTEN BIT(21) -#define TCCHEN BIT(22) -#define ITCCHEN BIT(23) - -/*ch_status paramater of callback function possible values*/ -#define EDMA_DMA_COMPLETE 1 -#define EDMA_DMA_CC_ERROR 2 -#define EDMA_DMA_TC1_ERROR 3 -#define EDMA_DMA_TC2_ERROR 4 - -enum address_mode { - INCR = 0, - FIFO = 1 -}; - -enum fifo_width { - W8BIT = 0, - W16BIT = 1, - W32BIT = 2, - W64BIT = 3, - W128BIT = 4, - W256BIT = 5 -}; +#define EDMA_MAX_CC 2
enum dma_event_q { EVENTQ_0 = 0, @@ -94,68 +21,9 @@ enum dma_event_q { EVENTQ_DEFAULT = -1 };
-enum sync_dimension { - ASYNC = 0, - ABSYNC = 1 -}; - -#define EDMA_CTLR_CHAN(ctlr, chan) (((ctlr) << 16) | (chan)) -#define EDMA_CTLR(i) ((i) >> 16) -#define EDMA_CHAN_SLOT(i) ((i) & 0xffff) - -#define EDMA_CHANNEL_ANY -1 /* for edma_alloc_channel() */ -#define EDMA_SLOT_ANY -1 /* for edma_alloc_slot() */ -#define EDMA_CONT_PARAMS_ANY 1001 -#define EDMA_CONT_PARAMS_FIXED_EXACT 1002 -#define EDMA_CONT_PARAMS_FIXED_NOT_EXACT 1003 - -#define EDMA_MAX_CC 2 - -/* alloc/free DMA channels and their dedicated parameter RAM slots */ -int edma_alloc_channel(int channel, - void (*callback)(unsigned channel, u16 ch_status, void *data), - void *data, enum dma_event_q); -void edma_free_channel(unsigned channel); - -/* alloc/free parameter RAM slots */ -int edma_alloc_slot(unsigned ctlr, int slot); -void edma_free_slot(unsigned slot); - -/* alloc/free a set of contiguous parameter RAM slots */ -int edma_alloc_cont_slots(unsigned ctlr, unsigned int id, int slot, int count); -int edma_free_cont_slots(unsigned slot, int count); - -/* calls that operate on part of a parameter RAM slot */ -void edma_set_src(unsigned slot, dma_addr_t src_port, - enum address_mode mode, enum fifo_width); -void edma_set_dest(unsigned slot, dma_addr_t dest_port, - enum address_mode mode, enum fifo_width); -dma_addr_t edma_get_position(unsigned slot, bool dst); -void edma_set_src_index(unsigned slot, s16 src_bidx, s16 src_cidx); -void edma_set_dest_index(unsigned slot, s16 dest_bidx, s16 dest_cidx); -void edma_set_transfer_params(unsigned slot, u16 acnt, u16 bcnt, u16 ccnt, - u16 bcnt_rld, enum sync_dimension sync_mode); -void edma_link(unsigned from, unsigned to); -void edma_unlink(unsigned from); - -/* calls that operate on an entire parameter RAM slot */ -void edma_write_slot(unsigned slot, const struct edmacc_param *params); -void edma_read_slot(unsigned slot, struct edmacc_param *params); - -/* channel control operations */ -int edma_start(unsigned channel); -void edma_stop(unsigned channel); -void edma_clean_channel(unsigned channel); -void edma_clear_event(unsigned channel); -void edma_pause(unsigned channel); -void edma_resume(unsigned channel); - -void edma_assign_channel_eventq(unsigned channel, enum dma_event_q eventq_no); - struct edma_rsv_info { - - const s16 (*rsv_chans)[2]; - const s16 (*rsv_slots)[2]; + const s16 (*rsv_chans)[2]; + const s16 (*rsv_slots)[2]; };
/* platform_data for EDMA driver */ @@ -165,15 +33,13 @@ struct edma_soc_info { * This way, long transfers on the default queue started * by the codec engine will not cause audio defects. */ - enum dma_event_q default_queue; + enum dma_event_q default_queue;
/* Resource reservation for other cores */ - struct edma_rsv_info *rsv; + struct edma_rsv_info *rsv;
- s8 (*queue_priority_mapping)[2]; - const s16 (*xbar_chans)[2]; + s8 (*queue_priority_mapping)[2]; + const s16 (*xbar_chans)[2]; };
-int edma_trigger_channel(unsigned); - #endif diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h index 0fe2346a9aa2..ddc07de04067 100644 --- a/sound/soc/davinci/davinci-pcm.h +++ b/sound/soc/davinci/davinci-pcm.h @@ -15,6 +15,7 @@ #include <linux/genalloc.h> #include <linux/platform_data/davinci_asp.h> #include <linux/platform_data/edma.h> +#include <linux/edma.h>
struct davinci_pcm_dma_params { int channel; /* sync dma channel ID */
On Thu, Nov 27, 2014 at 12:41:31PM +0200, Peter Ujfalusi wrote:
include/linux/platform_data/ is not a correct place to keep the API definitions for edma, it is meant to be only for the pdata for the device. Clean up this by moving the API to include/linux/edma.h
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
arch/arm/common/edma.c | 3 +- arch/arm/mach-davinci/devices.c | 1 + arch/arm/mach-davinci/include/mach/da8xx.h | 1 + drivers/dma/edma.c | 2 +- include/linux/edma.h | 153 +++++++++++++++++++++++++++++ include/linux/platform_data/edma.h | 148 ++-------------------------- sound/soc/davinci/davinci-pcm.h | 1 + 7 files changed, 165 insertions(+), 144 deletions(-) create mode 100644 include/linux/edma.h
I was hoping that this will have delete for platform_data/edma.h, do we still need that and why shouldn't we get rid of this :)
On Monday 08 December 2014 18:19:17 Vinod Koul wrote:
On Thu, Nov 27, 2014 at 12:41:31PM +0200, Peter Ujfalusi wrote:
include/linux/platform_data/ is not a correct place to keep the API definitions for edma, it is meant to be only for the pdata for the device. Clean up this by moving the API to include/linux/edma.h
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
arch/arm/common/edma.c | 3 +- arch/arm/mach-davinci/devices.c | 1 + arch/arm/mach-davinci/include/mach/da8xx.h | 1 + drivers/dma/edma.c | 2 +- include/linux/edma.h | 153 +++++++++++++++++++++++++++++ include/linux/platform_data/edma.h | 148 ++-------------------------- sound/soc/davinci/davinci-pcm.h | 1 + 7 files changed, 165 insertions(+), 144 deletions(-) create mode 100644 include/linux/edma.h
I was hoping that this will have delete for platform_data/edma.h, do we still need that and why shouldn't we get rid of this
I think the platform_data/edma.h file is needed for as long as we have mach-davinci systems that are not converted to boot using DT. At the current pace of development in that area, I would expect that to take a few more years at least: da850 support is slowly proceeding (since 2012), da830 should be fairly straightforward to add once da850 is done, but I haven't seen anybody start working on the dm* socs for DT.
Arnd
On 12/08/2014 02:49 PM, Vinod Koul wrote:
On Thu, Nov 27, 2014 at 12:41:31PM +0200, Peter Ujfalusi wrote:
include/linux/platform_data/ is not a correct place to keep the API definitions for edma, it is meant to be only for the pdata for the device. Clean up this by moving the API to include/linux/edma.h
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
arch/arm/common/edma.c | 3 +- arch/arm/mach-davinci/devices.c | 1 + arch/arm/mach-davinci/include/mach/da8xx.h | 1 + drivers/dma/edma.c | 2 +- include/linux/edma.h | 153 +++++++++++++++++++++++++++++ include/linux/platform_data/edma.h | 148 ++-------------------------- sound/soc/davinci/davinci-pcm.h | 1 + 7 files changed, 165 insertions(+), 144 deletions(-) create mode 100644 include/linux/edma.h
I was hoping that this will have delete for platform_data/edma.h, do we still need that and why shouldn't we get rid of this :)
We still need it for the legacy boot of davinci devices. We can boot some davinci SoC/boards with DT, but not all of them.
Hi,
On Thu, Nov 27, 2014 at 2:41 AM, Peter Ujfalusi peter.ujfalusi@ti.com wrote:
include/linux/platform_data/ is not a correct place to keep the API definitions for edma, it is meant to be only for the pdata for the device. Clean up this by moving the API to include/linux/edma.h
It's a nice net improvement, but it moves some things that should be in _neither_ location to a new place where it doesn't belong either -- and the new location is even more global. See below.
...
diff --git a/include/linux/edma.h b/include/linux/edma.h new file mode 100644 index 000000000000..9df92198c117 --- /dev/null +++ b/include/linux/edma.h @@ -0,0 +1,153 @@ +/*
- TI EDMA definitions
- Copyright (C) 2006-2013 Texas Instruments.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation version 2.
- This program is distributed "as is" WITHOUT ANY WARRANTY of any
- kind, whether express or implied; without even the implied warranty
- of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- */
+/*
- This EDMA3 programming framework exposes two basic kinds of resource:
- Channel Triggers transfers, usually from a hardware event but
also manually or by "chaining" from DMA completions.
Each channel is coupled to a Parameter RAM (PaRAM) slot.
- Slot Each PaRAM slot holds a DMA transfer descriptor (PaRAM
"set"), source and destination addresses, a link to a
next PaRAM slot (if any), options for the transfer, and
instructions for updating those addresses. There are
more than twice as many slots as event channels.
- Each PaRAM set describes a sequence of transfers, either for one large
- buffer or for several discontiguous smaller buffers. An EDMA transfer
- is driven only from a channel, which performs the transfers specified
- in its PaRAM slot until there are no more transfers. When that last
- transfer completes, the "link" field may be used to reload the channel's
- PaRAM slot with a new transfer descriptor.
- The EDMA Channel Controller (CC) maps requests from channels into physical
- Transfer Controller (TC) requests when the channel triggers (by hardware
- or software events, or by chaining). The two physical DMA channels provided
- by the TCs are thus shared by many logical channels.
- DaVinci hardware also has a "QDMA" mechanism which is not currently
- supported through this interface. (DSP firmware uses it though.)
- */
+#ifndef __LINUX_EDMA_H_ +#define __LINUX_EDMA_H_
+#include <linux/platform_data/edma.h>
+/* PaRAM slots are laid out like this */ +struct edmacc_param {
u32 opt;u32 src;u32 a_b_cnt;u32 dst;u32 src_dst_bidx;u32 link_bcntrld;u32 src_dst_cidx;u32 ccnt;+} __packed;
+/* fields in edmacc_param.opt */ +#define SAM BIT(0) +#define DAM BIT(1) +#define SYNCDIM BIT(2) +#define STATIC BIT(3) +#define EDMA_FWID (0x07 << 8) +#define TCCMODE BIT(11) +#define EDMA_TCC(t) ((t) << 12) +#define TCINTEN BIT(20) +#define ITCINTEN BIT(21) +#define TCCHEN BIT(22) +#define ITCCHEN BIT(23)
This seems like the kind of thing that should go with the edma driver instead of being globally exported to the kernel through a include/linux header file.
-Olof
Hi,
On 01/22/2015 03:40 AM, Olof Johansson wrote:
Hi,
On Thu, Nov 27, 2014 at 2:41 AM, Peter Ujfalusi peter.ujfalusi@ti.com wrote:
include/linux/platform_data/ is not a correct place to keep the API definitions for edma, it is meant to be only for the pdata for the device. Clean up this by moving the API to include/linux/edma.h
It's a nice net improvement, but it moves some things that should be in _neither_ location to a new place where it doesn't belong either -- and the new location is even more global. See below.
...
diff --git a/include/linux/edma.h b/include/linux/edma.h new file mode 100644 index 000000000000..9df92198c117 --- /dev/null +++ b/include/linux/edma.h @@ -0,0 +1,153 @@ +/*
- TI EDMA definitions
- Copyright (C) 2006-2013 Texas Instruments.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation version 2.
- This program is distributed "as is" WITHOUT ANY WARRANTY of any
- kind, whether express or implied; without even the implied warranty
- of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- */
+/*
- This EDMA3 programming framework exposes two basic kinds of resource:
- Channel Triggers transfers, usually from a hardware event but
also manually or by "chaining" from DMA completions.
Each channel is coupled to a Parameter RAM (PaRAM) slot.
- Slot Each PaRAM slot holds a DMA transfer descriptor (PaRAM
"set"), source and destination addresses, a link to a
next PaRAM slot (if any), options for the transfer, and
instructions for updating those addresses. There are
more than twice as many slots as event channels.
- Each PaRAM set describes a sequence of transfers, either for one large
- buffer or for several discontiguous smaller buffers. An EDMA transfer
- is driven only from a channel, which performs the transfers specified
- in its PaRAM slot until there are no more transfers. When that last
- transfer completes, the "link" field may be used to reload the channel's
- PaRAM slot with a new transfer descriptor.
- The EDMA Channel Controller (CC) maps requests from channels into physical
- Transfer Controller (TC) requests when the channel triggers (by hardware
- or software events, or by chaining). The two physical DMA channels provided
- by the TCs are thus shared by many logical channels.
- DaVinci hardware also has a "QDMA" mechanism which is not currently
- supported through this interface. (DSP firmware uses it though.)
- */
+#ifndef __LINUX_EDMA_H_ +#define __LINUX_EDMA_H_
+#include <linux/platform_data/edma.h>
+/* PaRAM slots are laid out like this */ +struct edmacc_param {
u32 opt;u32 src;u32 a_b_cnt;u32 dst;u32 src_dst_bidx;u32 link_bcntrld;u32 src_dst_cidx;u32 ccnt;+} __packed;
+/* fields in edmacc_param.opt */ +#define SAM BIT(0) +#define DAM BIT(1) +#define SYNCDIM BIT(2) +#define STATIC BIT(3) +#define EDMA_FWID (0x07 << 8) +#define TCCMODE BIT(11) +#define EDMA_TCC(t) ((t) << 12) +#define TCINTEN BIT(20) +#define ITCINTEN BIT(21) +#define TCCHEN BIT(22) +#define ITCCHEN BIT(23)
This seems like the kind of thing that should go with the edma driver instead of being globally exported to the kernel through a include/linux header file.
Currently the edmacc_param struct is used by arch/arm/common/edma.c, drivers/dma/edma.c and the sound/soc/davinci/davinci-pcm.c
For now this has to be in a global header file. Even with my local VIP branch where I had removed the davinci-pcm already the two drivers for edma needs this in global header file. I can assure you, it will be gone (along with the arch/arm/common/edma.c) but not right away.
On Thursday 27 November 2014 04:11 PM, Peter Ujfalusi wrote:
Hi,
The linux/platform_data/edma.h file was used for API definition as well, which is not correct since the header should only contain platform data related structures, defines, etc.
I would like to queue this series through ARM-SoC for v3.20 with Mark's and Ulf's acks added. I know its not the complete clean-up everyone wanted to see but I also know Peter working on doing the complete clean-up as well. So we are firmly on the path to removing arch/arm/common/edma.c.
Thanks, Sekhar
participants (7)
-
Arnd Bergmann -
Mark Brown -
Olof Johansson -
Peter Ujfalusi -
Sekhar Nori -
Ulf Hansson -
Vinod Koul