[PATCH v7 00/10] DMA Engine support for AM33XX

[PATCH v7 00/10] DMA Engine support for AM33XX

[Matt Porter]

Changes since v6:
	- Converted edma_of_read_*() to wrappers around of_property_read_*()
	- Fixed wording on the omap-spi generic DMA properties
	- Added comment/check to clarify that the driver only supports
	  a single EDMA instance when booting from DT

Changes since v5:
	- Dropped mmc portion and moved it to a separate series
	- Incorporate corrected version of dma_request_slave_channel_compat()
	- Fix #defines and enablement of TI_PRIV_EDMA option

Changes since v4:
	- Fixed debug section mismatch in private edma api [01/14]
	- Respun format-patch to catch the platform_data/edma.h rename [01/14]
	- Removed address/size-cells from the EDMA binding [05/14]

Changes since v3:
	- Rebased on 3.8-rc3
	- No longer an RFC
	- Fixed bugs in DT/pdata parsing reported by Vaibhav Bedia
	- Restored all the Davinci pdata to const
	- Removed max_segs hack in favor of using dma_get_channel_caps()
	- Fixed extra parens, __raw_* accessors and, ioremap error checks
	  in xbar handling
	- Removed excess license info in platform_data/edma.h
	- Removed unneeded reserved channels data for AM33xx
	- Removed test-specific pinmuxing from dts files
	- Adjusted mmc1 node to be disabled by default in the dtsi

Changes since v2:
	- Rebased on 3.7-rc1
	- Fixed bug in DT/pdata parsing first found by Gururaja
	  that turned out to be masked by some toolchains
	- Dropped unused mach-omap2/devices.c hsmmc patch
	- Added AM33XX crossbar DMA event mux support
	- Added am335x-evm support

Changes since v1:
	- Rebased on top of mainline from 12250d8
	- Dropped the feature removal schedule patch
	- Implemented dma_request_slave_channel_compat() and
	  converted the mmc and spi drivers to use it
	- Dropped unneeded #address-cells and #size-cells from
	  EDMA DT support
	- Moved private EDMA header to linux/platform_data/ and
	  removed some unneeded definitions
	- Fixed parsing of optional properties

This series adds DMA Engine support for AM33xx, which uses
an EDMA DMAC. The EDMA DMAC has been previously supported by only
a private API implementation (much like the situation with OMAP
DMA) found on the DaVinci family of SoCs.

The series applies on top of 3.8-rc5 and the following patches:

	- dmaengine DT support and edma dmaengine driver fix from 
	  the git://git.infradead.org/users/vkoul/slave-dma.git next
	  branch

The approach taken is similar to how OMAP DMA is being converted to
DMA Engine support. With the functional EDMA private API already
existing in mach-davinci/dma.c, we first move that to an ARM common
area so it can be shared. Adding DT and runtime PM support to the
private EDMA API implementation allows it to run on AM33xx. AM33xx
*only* boots using DT so we leverage Jon's generic DT DMA helpers to
register EDMA DMAC with the of_dma framework and then add support
for calling the dma_request_slave_channel() API to both the mmc
and spi drivers.

With this series both BeagleBone and the AM335x EVM have working
SPI DMA support (and MMC support with the separate MMC series).

This is tested on BeagleBone with a SPI framebuffer driver and MMC
rootfs. A trivial gpio DMA event misc driver was used to test the
crossbar DMA event support. It is also tested on the AM335x EVM
with the onboard SPI flash and MMC rootfs. The branch at
https://github.com/ohporter/linux/tree/edma-dmaengine-am33xx-v7
has the complete series, dependencies, and some test
drivers/defconfigs. Note that MMC can only be tested with a
separate MMC dmaengine/DT series applied.

Regression testing was done on AM180x-EVM (which also makes use
of the EDMA dmaengine driver and the EDMA private API) using SD,
SPI flash, and the onboard audio supported by the ASoC Davinci
driver. Regression testing was also done on a BeagleBoard xM
booting from the legacy board file using MMC rootfs.


Matt Porter (10):
  ARM: davinci: move private EDMA API to arm/common
  ARM: edma: remove unused transfer controller handlers
  ARM: edma: add AM33XX support to the private EDMA API
  dmaengine: edma: enable build for AM33XX
  dmaengine: edma: Add TI EDMA device tree binding
  ARM: dts: add AM33XX EDMA support
  dmaengine: add dma_request_slave_channel_compat()
  spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
  spi: omap2-mcspi: add generic DMA request support to the DT binding
  ARM: dts: add AM33XX SPI DMA support

 Documentation/devicetree/bindings/dma/ti-edma.txt  |   49 +++
 Documentation/devicetree/bindings/spi/omap-spi.txt |   27 +-
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/am33xx.dtsi                      |   30 ++
 arch/arm/common/Kconfig                            |    3 +
 arch/arm/common/Makefile                           |    1 +
 arch/arm/{mach-davinci/dma.c => common/edma.c}     |  342 +++++++++++++++++---
 arch/arm/mach-davinci/Makefile                     |    2 +-
 arch/arm/mach-davinci/board-tnetv107x-evm.c        |    2 +-
 arch/arm/mach-davinci/davinci.h                    |    2 +-
 arch/arm/mach-davinci/devices-tnetv107x.c          |    2 +-
 arch/arm/mach-davinci/devices.c                    |    6 +-
 arch/arm/mach-davinci/dm355.c                      |    2 +-
 arch/arm/mach-davinci/dm365.c                      |    2 +-
 arch/arm/mach-davinci/dm644x.c                     |    2 +-
 arch/arm/mach-davinci/dm646x.c                     |    2 +-
 arch/arm/mach-davinci/include/mach/da8xx.h         |    2 +-
 arch/arm/plat-omap/Kconfig                         |    1 +
 drivers/dma/Kconfig                                |    2 +-
 drivers/dma/edma.c                                 |    2 +-
 drivers/mmc/host/davinci_mmc.c                     |    1 +
 drivers/spi/spi-omap2-mcspi.c                      |   65 ++--
 include/linux/dmaengine.h                          |   16 +
 include/linux/mfd/davinci_voicecodec.h             |    3 +-
 .../mach => include/linux/platform_data}/edma.h    |   90 +-----
 include/linux/platform_data/spi-davinci.h          |    2 +-
 sound/soc/davinci/davinci-evm.c                    |    1 +
 sound/soc/davinci/davinci-pcm.c                    |    1 +
 sound/soc/davinci/davinci-pcm.h                    |    2 +-
 sound/soc/davinci/davinci-sffsdr.c                 |    7 +-
 30 files changed, 496 insertions(+), 174 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dma/ti-edma.txt
 rename arch/arm/{mach-davinci/dma.c => common/edma.c} (86%)
 rename {arch/arm/mach-davinci/include/mach => include/linux/platform_data}/edma.h (59%)

-- 
1.7.9.5


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

[PATCH v7 03/10] ARM: edma: add AM33XX support to the private EDMA API

[Matt Porter]

Adds support for parsing the TI EDMA DT data into the
required EDMA private API platform data. Enables runtime
PM support to initialize the EDMA hwmod. Adds AM33XX EDMA
crossbar event mux support. Enables build on OMAP.

Signed-off-by: Matt Porter <mporter@ti.com>
Acked-by: Sekhar Nori <nsekhar@ti.com>
---
 arch/arm/common/edma.c             |   96 ++++++++++++++++++++++++++++++++----
 arch/arm/plat-omap/Kconfig         |    1 +
 include/linux/platform_data/edma.h |    1 +
 3 files changed, 89 insertions(+), 9 deletions(-)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 3440d16..bd2416a 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -24,6 +24,13 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/edma.h>
+#include <linux/err.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_dma.h>
+#include <linux/of_irq.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/platform_data/edma.h>
 
@@ -723,6 +730,9 @@ EXPORT_SYMBOL(edma_free_channel);
  */
 int edma_alloc_slot(unsigned ctlr, int slot)
 {
+	if (!edma_cc[ctlr])
+		return -EINVAL;
+
 	if (slot >= 0)
 		slot = EDMA_CHAN_SLOT(slot);
 
@@ -1575,27 +1585,69 @@ static struct of_dma_filter_info edma_filter_info = {
 static int edma_probe(struct platform_device *pdev)
 {
 	struct edma_soc_info	**info = pdev->dev.platform_data;
+	struct edma_soc_info	*ninfo[EDMA_MAX_CC] = {NULL, NULL};
+	struct edma_soc_info	tmpinfo;
 	const s8		(*queue_priority_mapping)[2];
 	const s8		(*queue_tc_mapping)[2];
 	int			i, j, off, ln, found = 0;
 	int			status = -1;
 	const s16		(*rsv_chans)[2];
 	const s16		(*rsv_slots)[2];
+	const s16		(*xbar_chans)[2];
 	int			irq[EDMA_MAX_CC] = {0, 0};
 	int			err_irq[EDMA_MAX_CC] = {0, 0};
-	struct resource		*r[EDMA_MAX_CC] = {NULL};
+	struct resource		*r[EDMA_MAX_CC] = {NULL, NULL};
+	struct resource		res[EDMA_MAX_CC];
 	resource_size_t		len[EDMA_MAX_CC];
 	char			res_name[10];
 	char			irq_name[10];
+	struct device_node	*node = pdev->dev.of_node;
+	struct device		*dev = &pdev->dev;
+	int			ret;
+
+	if (node) {
+		/* Check if this is a second instance registered */
+		if (arch_num_cc) {
+			dev_err(dev, "only one EDMA instance is supported via DT\n");
+			return -ENODEV;
+		}
+		info = ninfo;
+		edma_of_parse_dt(dev, node, &tmpinfo);
+		info[0] = &tmpinfo;
+
+		dma_cap_set(DMA_SLAVE, edma_filter_info.dma_cap);
+		of_dma_controller_register(dev->of_node,
+					   of_dma_simple_xlate,
+					   &edma_filter_info);
+	}
 
 	if (!info)
 		return -ENODEV;
 
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (IS_ERR_VALUE(ret)) {
+		dev_err(dev, "pm_runtime_get_sync() failed\n");
+		return ret;
+	}
+
 	for (j = 0; j < EDMA_MAX_CC; j++) {
-		sprintf(res_name, "edma_cc%d", j);
-		r[j] = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+		if (!info[j]) {
+			if (!found)
+				return -ENODEV;
+			break;
+		}
+		if (node) {
+			ret = of_address_to_resource(node, j, &res[j]);
+			if (!IS_ERR_VALUE(ret))
+				r[j] = &res[j];
+		} else {
+			sprintf(res_name, "edma_cc%d", j);
+			r[j] = platform_get_resource_byname(pdev,
+						IORESOURCE_MEM,
 						res_name);
-		if (!r[j] || !info[j]) {
+		}
+		if (!r[j]) {
 			if (found)
 				break;
 			else
@@ -1670,8 +1722,22 @@ static int edma_probe(struct platform_device *pdev)
 			}
 		}
 
-		sprintf(irq_name, "edma%d", j);
-		irq[j] = platform_get_irq_byname(pdev, irq_name);
+		/* Clear the xbar mapped channels in unused list */
+		xbar_chans = info[j]->xbar_chans;
+		if (xbar_chans) {
+			for (i = 0; xbar_chans[i][1] != -1; i++) {
+				off = xbar_chans[i][1];
+				clear_bits(off, 1,
+					edma_cc[j]->edma_unused);
+			}
+		}
+
+		if (node)
+			irq[j] = irq_of_parse_and_map(node, 0);
+		else {
+			sprintf(irq_name, "edma%d", j);
+			irq[j] = platform_get_irq_byname(pdev, irq_name);
+		}
 		edma_cc[j]->irq_res_start = irq[j];
 		status = request_irq(irq[j], dma_irq_handler, 0, "edma",
 					&pdev->dev);
@@ -1681,8 +1747,12 @@ static int edma_probe(struct platform_device *pdev)
 			goto fail;
 		}
 
-		sprintf(irq_name, "edma%d_err", j);
-		err_irq[j] = platform_get_irq_byname(pdev, irq_name);
+		if (node)
+			err_irq[j] = irq_of_parse_and_map(node, 2);
+		else {
+			sprintf(irq_name, "edma%d_err", j);
+			err_irq[j] = platform_get_irq_byname(pdev, irq_name);
+		}
 		edma_cc[j]->irq_res_end = err_irq[j];
 		status = request_irq(err_irq[j], dma_ccerr_handler, 0,
 					"edma_error", &pdev->dev);
@@ -1743,9 +1813,17 @@ fail1:
 	return status;
 }
 
+static const struct of_device_id edma_of_ids[] = {
+	{ .compatible = "ti,edma3", },
+	{}
+};
 
 static struct platform_driver edma_driver = {
-	.driver.name	= "edma",
+	.driver = {
+		.name	= "edma",
+		.of_match_table = edma_of_ids,
+	},
+	.probe = edma_probe,
 };
 
 static int __init edma_init(void)
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 665870d..0b81d6c 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -29,6 +29,7 @@ config ARCH_OMAP2PLUS
 	select PINCTRL
 	select PROC_DEVICETREE if PROC_FS
 	select SPARSE_IRQ
+	select TI_PRIV_EDMA
 	select USE_OF
 	help
 	  "Systems based on OMAP2, OMAP3, OMAP4 or OMAP5"
diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index 2344ea2..ffc1fb2 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -177,6 +177,7 @@ struct edma_soc_info {
 
 	const s8	(*queue_tc_mapping)[2];
 	const s8	(*queue_priority_mapping)[2];
+	const s16	(*xbar_chans)[2];
 };
 
 #endif
-- 
1.7.9.5

[PATCH v7 04/10] dmaengine: edma: enable build for AM33XX

[Matt Porter]

Enable TI EDMA option on OMAP.

Signed-off-by: Matt Porter <mporter@ti.com>
---
 drivers/dma/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 0b408bb..239020b 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -220,7 +220,7 @@ config SIRF_DMA
 
 config TI_EDMA
 	tristate "TI EDMA support"
-	depends on ARCH_DAVINCI
+	depends on ARCH_DAVINCI || ARCH_OMAP
 	select DMA_ENGINE
 	select DMA_VIRTUAL_CHANNELS
 	default n
-- 
1.7.9.5

[PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Matt Porter]

Move mach-davinci/dma.c to common/edma.c so it can be used
by OMAP (specifically AM33xx) as well.

Signed-off-by: Matt Porter <mporter-l0cyMroinI0@public.gmane.org>
Acked-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
 arch/arm/Kconfig                                   |    1 +
 arch/arm/common/Kconfig                            |    3 +
 arch/arm/common/Makefile                           |    1 +
 arch/arm/{mach-davinci/dma.c => common/edma.c}     |  209 +++++++++++++++++++-
 arch/arm/mach-davinci/Makefile                     |    2 +-
 arch/arm/mach-davinci/board-tnetv107x-evm.c        |    2 +-
 arch/arm/mach-davinci/davinci.h                    |    2 +-
 arch/arm/mach-davinci/devices-tnetv107x.c          |    2 +-
 arch/arm/mach-davinci/devices.c                    |    6 +-
 arch/arm/mach-davinci/dm355.c                      |    2 +-
 arch/arm/mach-davinci/dm365.c                      |    2 +-
 arch/arm/mach-davinci/dm644x.c                     |    2 +-
 arch/arm/mach-davinci/dm646x.c                     |    2 +-
 arch/arm/mach-davinci/include/mach/da8xx.h         |    2 +-
 drivers/dma/edma.c                                 |    2 +-
 drivers/mmc/host/davinci_mmc.c                     |    1 +
 include/linux/mfd/davinci_voicecodec.h             |    3 +-
 .../mach => include/linux/platform_data}/edma.h    |   89 +--------
 include/linux/platform_data/spi-davinci.h          |    2 +-
 sound/soc/davinci/davinci-evm.c                    |    1 +
 sound/soc/davinci/davinci-pcm.c                    |    1 +
 sound/soc/davinci/davinci-pcm.h                    |    2 +-
 sound/soc/davinci/davinci-sffsdr.c                 |    7 +-
 23 files changed, 240 insertions(+), 106 deletions(-)
 rename arch/arm/{mach-davinci/dma.c => common/edma.c} (90%)
 rename {arch/arm/mach-davinci/include/mach => include/linux/platform_data}/edma.h (59%)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 67874b8..7637d31 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -932,6 +932,7 @@ config ARCH_DAVINCI
 	select GENERIC_IRQ_CHIP
 	select HAVE_IDE
 	select NEED_MACH_GPIO_H
+	select TI_PRIV_EDMA
 	select USE_OF
 	select ZONE_DMA
 	help
diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 45ceeb0..9e32d0d 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -40,3 +40,6 @@ config SHARP_PARAM
 
 config SHARP_SCOOP
 	bool
+
+config TI_PRIV_EDMA
+	bool
diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
index e8a4e58..d09a39b 100644
--- a/arch/arm/common/Makefile
+++ b/arch/arm/common/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_SHARP_PARAM)	+= sharpsl_param.o
 obj-$(CONFIG_SHARP_SCOOP)	+= scoop.o
 obj-$(CONFIG_PCI_HOST_ITE8152)  += it8152.o
 obj-$(CONFIG_ARM_TIMER_SP804)	+= timer-sp.o
+obj-$(CONFIG_TI_PRIV_EDMA)	+= edma.o
diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/common/edma.c
similarity index 90%
rename from arch/arm/mach-davinci/dma.c
rename to arch/arm/common/edma.c
index a685e97..aa4a49a 100644
--- a/arch/arm/mach-davinci/dma.c
+++ b/arch/arm/common/edma.c
@@ -25,7 +25,7 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 
-#include <mach/edma.h>
+#include <linux/platform_data/edma.h>
 
 /* Offsets matching "struct edmacc_param" */
 #define PARM_OPT		0x00
@@ -1386,8 +1386,213 @@ void edma_clear_event(unsigned channel)
 EXPORT_SYMBOL(edma_clear_event);
 
 /*-----------------------------------------------------------------------*/
+static int edma_of_read_u32_to_s8_array(const struct device_node *np,
+					 const char *propname, s8 *out_values,
+					 size_t sz)
+{
+	int ret;
+
+	ret = of_property_read_u8_array(np, propname, out_values, sz);
+	if (ret)
+		return ret;
+
+	/* Terminate it */
+	*out_values++ = -1;
+	*out_values++ = -1;
+
+	return 0;
+}
+
+static int edma_of_read_u32_to_s16_array(const struct device_node *np,
+					 const char *propname, s16 *out_values,
+					 size_t sz)
+{
+	int ret;
+
+	ret = of_property_read_u16_array(np, propname, out_values, sz);
+	if (ret)
+		return ret;
+
+	/* Terminate it */
+	*out_values++ = -1;
+	*out_values++ = -1;
+
+	return 0;
+}
+
+static int edma_xbar_event_map(struct device *dev,
+			       struct device_node *node,
+			       struct edma_soc_info *pdata, int len)
+{
+	int ret = 0;
+	int i;
+	struct resource res;
+	void *xbar;
+	const s16 (*xbar_chans)[2];
+	u32 shift, offset, mux;
+
+	xbar_chans = devm_kzalloc(dev,
+				  len/sizeof(s16) + 2*sizeof(s16),
+				  GFP_KERNEL);
+	if (!xbar_chans)
+		return -ENOMEM;
+
+	ret = of_address_to_resource(node, 1, &res);
+	if (IS_ERR_VALUE(ret))
+		return -EIO;
+
+	xbar = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!xbar)
+		return -ENOMEM;
+
+	ret = edma_of_read_u32_to_s16_array(node,
+					    "ti,edma-xbar-event-map",
+					    (s16 *)xbar_chans,
+					    len/sizeof(u32));
+	if (IS_ERR_VALUE(ret))
+		return -EIO;
+
+	for (i = 0; xbar_chans[i][0] != -1; i++) {
+		shift = (xbar_chans[i][1] % 4) * 8;
+		offset = xbar_chans[i][1] >> 2;
+		offset <<= 2;
+		mux = readl((void *)((u32)xbar + offset));
+		mux &= ~(0xff << shift);
+		mux |= xbar_chans[i][0] << shift;
+		writel(mux, (void *)((u32)xbar + offset));
+	}
+
+	pdata->xbar_chans = xbar_chans;
+
+	return 0;
+}
+
+static int edma_of_parse_dt(struct device *dev,
+			    struct device_node *node,
+			    struct edma_soc_info *pdata)
+{
+	int ret = 0;
+	u32 value;
+	struct property *prop;
+	size_t sz;
+	struct edma_rsv_info *rsv_info;
+	const s16 (*rsv_chans)[2], (*rsv_slots)[2];
+	const s8 (*queue_tc_map)[2], (*queue_priority_map)[2];
+
+	memset(pdata, 0, sizeof(struct edma_soc_info));
+
+	ret = of_property_read_u32(node, "dma-channels", &value);
+	if (ret < 0)
+		return ret;
+	pdata->n_channel = value;
+
+	ret = of_property_read_u32(node, "ti,edma-regions", &value);
+	if (ret < 0)
+		return ret;
+	pdata->n_region = value;
+
+	ret = of_property_read_u32(node, "ti,edma-slots", &value);
+	if (ret < 0)
+		return ret;
+	pdata->n_slot = value;
+
+	pdata->n_cc = 1;
+	pdata->n_tc = 3;
+
+	rsv_info =
+		devm_kzalloc(dev, sizeof(struct edma_rsv_info), GFP_KERNEL);
+	if (!rsv_info)
+		return -ENOMEM;
+	pdata->rsv = rsv_info;
+
+	/* Build the reserved channel/slots arrays */
+	prop = of_find_property(node, "ti,edma-reserved-channels", &sz);
+	if (prop) {
+		rsv_chans = devm_kzalloc(dev,
+					 sz/sizeof(s16) + 2*sizeof(s16),
+					 GFP_KERNEL);
+		if (!rsv_chans)
+			return -ENOMEM;
+		pdata->rsv->rsv_chans = rsv_chans;
+
+		ret = edma_of_read_u32_to_s16_array(node,
+						    "ti,edma-reserved-channels",
+						    (s16 *)rsv_chans,
+						    sz/sizeof(u32));
+		if (ret < 0)
+			return ret;
+	}
+
+	prop = of_find_property(node, "ti,edma-reserved-slots", &sz);
+	if (prop) {
+		rsv_slots = devm_kzalloc(dev,
+					 sz/sizeof(s16) + 2*sizeof(s16),
+					 GFP_KERNEL);
+		if (!rsv_slots)
+			return -ENOMEM;
+		pdata->rsv->rsv_slots = rsv_slots;
+
+		ret = edma_of_read_u32_to_s16_array(node,
+						    "ti,edma-reserved-slots",
+						    (s16 *)rsv_slots,
+						    sz/sizeof(u32));
+		if (ret < 0)
+			return ret;
+	}
+
+	prop = of_find_property(node, "ti,edma-queue-tc-map", &sz);
+	if (!prop)
+		return -EINVAL;
+
+	queue_tc_map = devm_kzalloc(dev,
+				    sz/sizeof(s8) + 2*sizeof(s8),
+				    GFP_KERNEL);
+	if (!queue_tc_map)
+		return -ENOMEM;
+	pdata->queue_tc_mapping = queue_tc_map;
+
+	ret = edma_of_read_u32_to_s8_array(node,
+					   "ti,edma-queue-tc-map",
+					   (s8 *)queue_tc_map,
+					   sz/sizeof(u32));
+	if (ret < 0)
+		return ret;
+
+	prop = of_find_property(node, "ti,edma-queue-priority-map", &sz);
+	if (!prop)
+		return -EINVAL;
+
+	queue_priority_map = devm_kzalloc(dev,
+					  sz/sizeof(s8) + 2*sizeof(s8),
+					  GFP_KERNEL);
+	if (!queue_priority_map)
+		return -ENOMEM;
+	pdata->queue_priority_mapping = queue_priority_map;
+
+	ret = edma_of_read_u32_to_s8_array(node,
+					   "ti,edma-queue-tc-map",
+					   (s8 *)queue_priority_map,
+					   sz/sizeof(u32));
+	if (ret < 0)
+		return ret;
+
+	ret = of_property_read_u32(node, "ti,edma-default-queue", &value);
+	if (ret < 0)
+		return ret;
+	pdata->default_queue = value;
+
+	prop = of_find_property(node, "ti,edma-xbar-event-map", &sz);
+	if (prop)
+		ret = edma_xbar_event_map(dev, node, pdata, sz);
+
+	return ret;
+}
+
+static struct of_dma_filter_info edma_filter_info = {
+	.filter_fn = edma_filter_fn,
+};
 
-static int __init edma_probe(struct platform_device *pdev)
+static int edma_probe(struct platform_device *pdev)
 {
 	struct edma_soc_info	**info = pdev->dev.platform_data;
 	const s8		(*queue_priority_mapping)[2];
diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile
index fb5c1aa..493a36b 100644
--- a/arch/arm/mach-davinci/Makefile
+++ b/arch/arm/mach-davinci/Makefile
@@ -5,7 +5,7 @@
 
 # Common objects
 obj-y 			:= time.o clock.o serial.o psc.o \
-			   dma.o usb.o common.o sram.o aemif.o
+			   usb.o common.o sram.o aemif.o
 
 obj-$(CONFIG_DAVINCI_MUX)		+= mux.o
 
diff --git a/arch/arm/mach-davinci/board-tnetv107x-evm.c b/arch/arm/mach-davinci/board-tnetv107x-evm.c
index be30997..86f55ba 100644
--- a/arch/arm/mach-davinci/board-tnetv107x-evm.c
+++ b/arch/arm/mach-davinci/board-tnetv107x-evm.c
@@ -26,12 +26,12 @@
 #include <linux/input.h>
 #include <linux/input/matrix_keypad.h>
 #include <linux/spi/spi.h>
+#include <linux/platform_data/edma.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach-types.h>
 
 #include <mach/irqs.h>
-#include <mach/edma.h>
 #include <mach/mux.h>
 #include <mach/cp_intc.h>
 #include <mach/tnetv107x.h>
diff --git a/arch/arm/mach-davinci/davinci.h b/arch/arm/mach-davinci/davinci.h
index 12d544b..d26a6bc 100644
--- a/arch/arm/mach-davinci/davinci.h
+++ b/arch/arm/mach-davinci/davinci.h
@@ -23,9 +23,9 @@
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/platform_data/davinci_asp.h>
+#include <linux/platform_data/edma.h>
 #include <linux/platform_data/keyscan-davinci.h>
 #include <mach/hardware.h>
-#include <mach/edma.h>
 
 #include <media/davinci/vpfe_capture.h>
 #include <media/davinci/vpif_types.h>
diff --git a/arch/arm/mach-davinci/devices-tnetv107x.c b/arch/arm/mach-davinci/devices-tnetv107x.c
index 773ab07..ba37760 100644
--- a/arch/arm/mach-davinci/devices-tnetv107x.c
+++ b/arch/arm/mach-davinci/devices-tnetv107x.c
@@ -18,10 +18,10 @@
 #include <linux/dma-mapping.h>
 #include <linux/clk.h>
 #include <linux/slab.h>
+#include <linux/platform_data/edma.h>
 
 #include <mach/common.h>
 #include <mach/irqs.h>
-#include <mach/edma.h>
 #include <mach/tnetv107x.h>
 
 #include "clock.h"
diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c
index 4c48a36..ca0c7b3 100644
--- a/arch/arm/mach-davinci/devices.c
+++ b/arch/arm/mach-davinci/devices.c
@@ -19,9 +19,10 @@
 #include <mach/irqs.h>
 #include <mach/cputype.h>
 #include <mach/mux.h>
-#include <mach/edma.h>
 #include <linux/platform_data/mmc-davinci.h>
 #include <mach/time.h>
+#include <linux/platform_data/edma.h>
+
 
 #include "davinci.h"
 #include "clock.h"
@@ -34,6 +35,9 @@
 #define DM365_MMCSD0_BASE	     0x01D11000
 #define DM365_MMCSD1_BASE	     0x01D00000
 
+#define DAVINCI_DMA_MMCRXEVT	26
+#define DAVINCI_DMA_MMCTXEVT	27
+
 void __iomem  *davinci_sysmod_base;
 
 void davinci_map_sysmod(void)
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index b49c3b7..53998d8 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -19,7 +19,6 @@
 #include <asm/mach/map.h>
 
 #include <mach/cputype.h>
-#include <mach/edma.h>
 #include <mach/psc.h>
 #include <mach/mux.h>
 #include <mach/irqs.h>
@@ -28,6 +27,7 @@
 #include <mach/common.h>
 #include <linux/platform_data/spi-davinci.h>
 #include <mach/gpio-davinci.h>
+#include <linux/platform_data/edma.h>
 
 #include "davinci.h"
 #include "clock.h"
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index 6c39805..9b41d33 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -18,11 +18,11 @@
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 #include <linux/spi/spi.h>
+#include <linux/platform_data/edma.h>
 
 #include <asm/mach/map.h>
 
 #include <mach/cputype.h>
-#include <mach/edma.h>
 #include <mach/psc.h>
 #include <mach/mux.h>
 #include <mach/irqs.h>
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index 11c79a3..a08910e 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -12,11 +12,11 @@
 #include <linux/clk.h>
 #include <linux/serial_8250.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/edma.h>
 
 #include <asm/mach/map.h>
 
 #include <mach/cputype.h>
-#include <mach/edma.h>
 #include <mach/irqs.h>
 #include <mach/psc.h>
 #include <mach/mux.h>
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index ac7b431..6d52a32 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -13,11 +13,11 @@
 #include <linux/clk.h>
 #include <linux/serial_8250.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/edma.h>
 
 #include <asm/mach/map.h>
 
 #include <mach/cputype.h>
-#include <mach/edma.h>
 #include <mach/irqs.h>
 #include <mach/psc.h>
 #include <mach/mux.h>
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 700d311..9d77f9b 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -20,8 +20,8 @@
 #include <linux/videodev2.h>
 
 #include <mach/serial.h>
-#include <mach/edma.h>
 #include <mach/pm.h>
+#include <linux/platform_data/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 06ea4b8..9c7d16b 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -24,7 +24,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 
-#include <mach/edma.h>
+#include <linux/platform_data/edma.h>
 
 #include "dmaengine.h"
 #include "virt-dma.h"
diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index 2063677..f5d46ea 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -35,6 +35,7 @@
 #include <linux/edma.h>
 #include <linux/mmc/mmc.h>
 
+#include <linux/platform_data/edma.h>
 #include <linux/platform_data/mmc-davinci.h>
 
 /*
diff --git a/include/linux/mfd/davinci_voicecodec.h b/include/linux/mfd/davinci_voicecodec.h
index 0ab6132..7dd6524 100644
--- a/include/linux/mfd/davinci_voicecodec.h
+++ b/include/linux/mfd/davinci_voicecodec.h
@@ -26,8 +26,7 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/core.h>
-
-#include <mach/edma.h>
+#include <linux/platform_data/edma.h>
 
 /*
  * Register values.
diff --git a/arch/arm/mach-davinci/include/mach/edma.h b/include/linux/platform_data/edma.h
similarity index 59%
rename from arch/arm/mach-davinci/include/mach/edma.h
rename to include/linux/platform_data/edma.h
index 7e84c90..2344ea2 100644
--- a/arch/arm/mach-davinci/include/mach/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -1,28 +1,12 @@
 /*
- *  TI DAVINCI dma definitions
+ *  TI EDMA definitions
  *
- *  Copyright (C) 2006-2009 Texas Instruments.
+ *  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;  either version 2 of the  License, or (at your
  *  option) any later version.
- *
- *  THIS  SOFTWARE  IS PROVIDED   ``AS  IS'' AND   ANY  EXPRESS OR IMPLIED
- *  WARRANTIES,   INCLUDING, BUT NOT  LIMITED  TO, THE IMPLIED WARRANTIES OF
- *  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN
- *  NO  EVENT  SHALL   THE AUTHOR  BE    LIABLE FOR ANY   DIRECT, INDIRECT,
- *  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- *  NOT LIMITED   TO, PROCUREMENT OF  SUBSTITUTE GOODS  OR SERVICES; LOSS OF
- *  USE, DATA,  OR PROFITS; OR  BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
- *  ANY THEORY OF LIABILITY, WHETHER IN  CONTRACT, STRICT LIABILITY, OR TORT
- *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- *  THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- *  You should have received a copy of the  GNU General Public License along
- *  with this program; if not, write  to the Free Software Foundation, Inc.,
- *  675 Mass Ave, Cambridge, MA 02139, USA.
- *
  */
 
 /*
@@ -69,11 +53,6 @@ struct edmacc_param {
 	unsigned int ccnt;
 };
 
-#define CCINT0_INTERRUPT     16
-#define CCERRINT_INTERRUPT   17
-#define TCERRINT0_INTERRUPT   18
-#define TCERRINT1_INTERRUPT   19
-
 /* fields in edmacc_param.opt */
 #define SAM		BIT(0)
 #define DAM		BIT(1)
@@ -87,70 +66,6 @@ struct edmacc_param {
 #define TCCHEN		BIT(22)
 #define ITCCHEN		BIT(23)
 
-#define TRWORD (0x7<<2)
-#define PAENTRY (0x1ff<<5)
-
-/* Drivers should avoid using these symbolic names for dm644x
- * channels, and use platform_device IORESOURCE_DMA resources
- * instead.  (Other DaVinci chips have different peripherals
- * and thus have different DMA channel mappings.)
- */
-#define DAVINCI_DMA_MCBSP_TX              2
-#define DAVINCI_DMA_MCBSP_RX              3
-#define DAVINCI_DMA_VPSS_HIST             4
-#define DAVINCI_DMA_VPSS_H3A              5
-#define DAVINCI_DMA_VPSS_PRVU             6
-#define DAVINCI_DMA_VPSS_RSZ              7
-#define DAVINCI_DMA_IMCOP_IMXINT          8
-#define DAVINCI_DMA_IMCOP_VLCDINT         9
-#define DAVINCI_DMA_IMCO_PASQINT         10
-#define DAVINCI_DMA_IMCOP_DSQINT         11
-#define DAVINCI_DMA_SPI_SPIX             16
-#define DAVINCI_DMA_SPI_SPIR             17
-#define DAVINCI_DMA_UART0_URXEVT0        18
-#define DAVINCI_DMA_UART0_UTXEVT0        19
-#define DAVINCI_DMA_UART1_URXEVT1        20
-#define DAVINCI_DMA_UART1_UTXEVT1        21
-#define DAVINCI_DMA_UART2_URXEVT2        22
-#define DAVINCI_DMA_UART2_UTXEVT2        23
-#define DAVINCI_DMA_MEMSTK_MSEVT         24
-#define DAVINCI_DMA_MMCRXEVT             26
-#define DAVINCI_DMA_MMCTXEVT             27
-#define DAVINCI_DMA_I2C_ICREVT           28
-#define DAVINCI_DMA_I2C_ICXEVT           29
-#define DAVINCI_DMA_GPIO_GPINT0          32
-#define DAVINCI_DMA_GPIO_GPINT1          33
-#define DAVINCI_DMA_GPIO_GPINT2          34
-#define DAVINCI_DMA_GPIO_GPINT3          35
-#define DAVINCI_DMA_GPIO_GPINT4          36
-#define DAVINCI_DMA_GPIO_GPINT5          37
-#define DAVINCI_DMA_GPIO_GPINT6          38
-#define DAVINCI_DMA_GPIO_GPINT7          39
-#define DAVINCI_DMA_GPIO_GPBNKINT0       40
-#define DAVINCI_DMA_GPIO_GPBNKINT1       41
-#define DAVINCI_DMA_GPIO_GPBNKINT2       42
-#define DAVINCI_DMA_GPIO_GPBNKINT3       43
-#define DAVINCI_DMA_GPIO_GPBNKINT4       44
-#define DAVINCI_DMA_TIMER0_TINT0         48
-#define DAVINCI_DMA_TIMER1_TINT1         49
-#define DAVINCI_DMA_TIMER2_TINT2         50
-#define DAVINCI_DMA_TIMER3_TINT3         51
-#define DAVINCI_DMA_PWM0                 52
-#define DAVINCI_DMA_PWM1                 53
-#define DAVINCI_DMA_PWM2                 54
-
-/* DA830 specific EDMA3 information */
-#define EDMA_DA830_NUM_DMACH		32
-#define EDMA_DA830_NUM_TCC		32
-#define EDMA_DA830_NUM_PARAMENTRY	128
-#define EDMA_DA830_NUM_EVQUE		2
-#define EDMA_DA830_NUM_TC		2
-#define EDMA_DA830_CHMAP_EXIST		0
-#define EDMA_DA830_NUM_REGIONS		4
-#define DA830_DMACH2EVENT_MAP0		0x000FC03Fu
-#define DA830_DMACH2EVENT_MAP1		0x00000000u
-#define DA830_EDMA_ARM_OWN		0x30FFCCFFu
-
 /*ch_status paramater of callback function possible values*/
 #define DMA_COMPLETE 1
 #define DMA_CC_ERROR 2
diff --git a/include/linux/platform_data/spi-davinci.h b/include/linux/platform_data/spi-davinci.h
index 7af305b..8dc2fa47 100644
--- a/include/linux/platform_data/spi-davinci.h
+++ b/include/linux/platform_data/spi-davinci.h
@@ -19,7 +19,7 @@
 #ifndef __ARCH_ARM_DAVINCI_SPI_H
 #define __ARCH_ARM_DAVINCI_SPI_H
 
-#include <mach/edma.h>
+#include <linux/platform_data/edma.h>
 
 #define SPI_INTERN_CS	0xFF
 
diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c
index d55e647..591f547 100644
--- a/sound/soc/davinci/davinci-evm.c
+++ b/sound/soc/davinci/davinci-evm.c
@@ -14,6 +14,7 @@
 #include <linux/timer.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/edma.h>
 #include <linux/i2c.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c
index afab81f..9bdd71b 100644
--- a/sound/soc/davinci/davinci-pcm.c
+++ b/sound/soc/davinci/davinci-pcm.c
@@ -17,6 +17,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/kernel.h>
 #include <linux/genalloc.h>
+#include <linux/platform_data/edma.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
diff --git a/sound/soc/davinci/davinci-pcm.h b/sound/soc/davinci/davinci-pcm.h
index b6ef703..fbb710c 100644
--- a/sound/soc/davinci/davinci-pcm.h
+++ b/sound/soc/davinci/davinci-pcm.h
@@ -14,7 +14,7 @@
 
 #include <linux/genalloc.h>
 #include <linux/platform_data/davinci_asp.h>
-#include <mach/edma.h>
+#include <linux/platform_data/edma.h>
 
 struct davinci_pcm_dma_params {
 	int channel;			/* sync dma channel ID */
diff --git a/sound/soc/davinci/davinci-sffsdr.c b/sound/soc/davinci/davinci-sffsdr.c
index 5be65aa..a45af64 100644
--- a/sound/soc/davinci/davinci-sffsdr.c
+++ b/sound/soc/davinci/davinci-sffsdr.c
@@ -17,6 +17,7 @@
 #include <linux/timer.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/platform_data/edma.h>
 #include <linux/gpio.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -28,12 +29,14 @@
 #include <asm/plat-sffsdr/sffsdr-fpga.h>
 #endif
 
-#include <mach/edma.h>
 
 #include "../codecs/pcm3008.h"
 #include "davinci-pcm.h"
 #include "davinci-i2s.h"
 
+#define DAVINCI_DMA_MCBSP_TX	2
+#define DAVINCI_DMA_MCBSP_RX	3
+
 /*
  * CLKX and CLKR are the inputs for the Sample Rate Generator.
  * FSX and FSR are outputs, driven by the sample Rate Generator.
@@ -124,7 +127,7 @@ static struct resource sffsdr_snd_resources[] = {
 
 static struct evm_snd_platform_data sffsdr_snd_data = {
 	.tx_dma_ch	= DAVINCI_DMA_MCBSP_TX,
-	.rx_dma_ch	= DAVINCI_DMA_MCBSP_RX,
+	.rx_dma_ch	= DAVINCI_DMA_MCBAP_RX,
 };
 
 static struct platform_device *sffsdr_snd_device;
-- 
1.7.9.5

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Tony Lindgren]

* Matt Porter <mporter@ti.com> [130201 10:25]:
> Move mach-davinci/dma.c to common/edma.c so it can be used
> by OMAP (specifically AM33xx) as well.

I think this should rather go to drivers/dma/?

Tony

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Fri, Feb 01, 2013 at 10:41:08AM -0800, Tony Lindgren wrote:
> * Matt Porter <mporter@ti.com> [130201 10:25]:
> > Move mach-davinci/dma.c to common/edma.c so it can be used
> > by OMAP (specifically AM33xx) as well.
> 
> I think this should rather go to drivers/dma/?

Yes, it should, but just like OMAP, there's a conversion effort that needs
to be gone through.  It has one point - and only one point - which allows
its continued existence under arch/arm, and that is it already exists
there.

If it was new code, the answer would be a definite NACK, but it isn't.
It's pre-existing code which is already in mainline.  It's merely being
moved.

Another plus point for it is that there does seem to be a DMA engine
driver for it, so hopefully we'll see it killed off in arch/arm soon.

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Matt Porter]

On Sat, Feb 02, 2013 at 12:49:06PM +0000, Russell King wrote:
> On Fri, Feb 01, 2013 at 10:41:08AM -0800, Tony Lindgren wrote:
> > * Matt Porter <mporter@ti.com> [130201 10:25]:
> > > Move mach-davinci/dma.c to common/edma.c so it can be used
> > > by OMAP (specifically AM33xx) as well.
> > 
> > I think this should rather go to drivers/dma/?
> 
> Yes, it should, but just like OMAP, there's a conversion effort that needs
> to be gone through.  It has one point - and only one point - which allows
> its continued existence under arch/arm, and that is it already exists
> there.
> 
> If it was new code, the answer would be a definite NACK, but it isn't.
> It's pre-existing code which is already in mainline.  It's merely being
> moved.
> 
> Another plus point for it is that there does seem to be a DMA engine
> driver for it, so hopefully we'll see it killed off in arch/arm soon.

That's definitely the plan. I was able to start this effort
independently by converting the Davinci mmc and spi drivers to dmaengine
before I took this step. I've got the next micro-step of addressing
omap_hsmmc in process (pending on agreement on a dmaengine api change
with Vinod), cleaning up the mcasp driver is also pending this series so
that it can also be converted to dmaengine. Once mcasp (or "davinci
audio") is converted, we're rid of all the in-kernel users of the
private API and can get rid of this...which also is helping clean up
mach-davinci, of course.

If I can get your ack on this patch that should move things along to
these next steps.

Thanks,
Matt

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Matt Porter]

On Fri, Feb 01, 2013 at 06:41:08PM +0000, Tony Lindgren wrote:
> * Matt Porter <mporter@ti.com> [130201 10:25]:
> > Move mach-davinci/dma.c to common/edma.c so it can be used
> > by OMAP (specifically AM33xx) as well.
> 
> I think this should rather go to drivers/dma/?

No, this is the private EDMA API. It's the analogous thing to
the private OMAP dma API that is in plat-omap/dma.c. The actual
dmaengine driver is in drivers/dma/edma.c as a wrapper around
this...same way OMAP DMA engine conversion is being done.

-Matt

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Matt Porter]

On Fri, Feb 01, 2013 at 07:52:46PM +0000, Sergei Shtylyov wrote:
> Hello.
> 
> On 02/01/2013 09:49 PM, Matt Porter wrote:
> 
> >>> Move mach-davinci/dma.c to common/edma.c so it can be used
> >>> by OMAP (specifically AM33xx) as well.
> 
> >> I think this should rather go to drivers/dma/?
> 
> > No, this is the private EDMA API. It's the analogous thing to
> > the private OMAP dma API that is in plat-omap/dma.c. The actual
> > dmaengine driver is in drivers/dma/edma.c as a wrapper around
> > this...same way OMAP DMA engine conversion is being done.
> 
>   Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
> that, instead of waiting indefinitely for TI to convert it to drivers/dma/
> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.

That is a shame. Yeah, I've pointed out that I was doing this exactly
the same way as was acceptable for the OMAP DMA conversion since it was
in RFC. The reasons are sound since in both cases, we have many drivers
to convert that need to continue using the private DMA APIs.

-Matt
> 

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 01-02-2013 22:59, Matt Porter wrote:

>>>>> Move mach-davinci/dma.c to common/edma.c so it can be used
>>>>> by OMAP (specifically AM33xx) as well.

>>>> I think this should rather go to drivers/dma/?

>>> No, this is the private EDMA API. It's the analogous thing to
>>> the private OMAP dma API that is in plat-omap/dma.c. The actual
>>> dmaengine driver is in drivers/dma/edma.c as a wrapper around
>>> this...same way OMAP DMA engine conversion is being done.

>>    Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
>> that, instead of waiting indefinitely for TI to convert it to drivers/dma/
>> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.

> That is a shame. Yeah, I've pointed out that I was doing this exactly
> the same way as was acceptable for the OMAP DMA conversion since it was
> in RFC. The reasons are sound since in both cases, we have many drivers
> to convert that need to continue using the private DMA APIs.

    In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other 
in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is 
sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't 
know them well).

> -Matt

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Fri, Feb 01, 2013 at 01:59:59PM -0500, Matt Porter wrote:
> On Fri, Feb 01, 2013 at 07:52:46PM +0000, Sergei Shtylyov wrote:
> > Hello.
> > 
> > On 02/01/2013 09:49 PM, Matt Porter wrote:
> > 
> > >>> Move mach-davinci/dma.c to common/edma.c so it can be used
> > >>> by OMAP (specifically AM33xx) as well.
> > 
> > >> I think this should rather go to drivers/dma/?
> > 
> > > No, this is the private EDMA API. It's the analogous thing to
> > > the private OMAP dma API that is in plat-omap/dma.c. The actual
> > > dmaengine driver is in drivers/dma/edma.c as a wrapper around
> > > this...same way OMAP DMA engine conversion is being done.
> > 
> >   Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
> > that, instead of waiting indefinitely for TI to convert it to drivers/dma/
> > directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.
> 
> That is a shame. Yeah, I've pointed out that I was doing this exactly
> the same way as was acceptable for the OMAP DMA conversion since it was
> in RFC. The reasons are sound since in both cases, we have many drivers
> to convert that need to continue using the private DMA APIs.

It's very simple.

The OMAP DMA stuff in plat-omap/dma.c has existed for a long time, well
before things like the DMA engine API came into being.  Same with a lot
of the DMA code in arch/arm.  It is therefore in the privileged position
of having "grandfather" rights when it comes to existing in mainline.

However, today what we're finding is that these private per-platform APIs
are sub-optimal; they prevent the peripheral drivers in the drivers/
subtree from being re-used on other SoCs.  So, even through they pre-exist
the DMA engine support, they're being gradually converted to the API.

Moreover, we have _far_ too much code in arch/arm.  It's something that
has been attracted complaints from Linus.  It's something that's got us
close to having updates to arch/arm refused during merge windows.  It's
got us close to having parts of arch/arm deleted from the mainline kernel.

The long term plan is _still_ to remove arch/arm/*omap*/dma.c and move
the whole thing to drivers/dma.  That can only happen when everything is
converted (or the drivers which aren't converted have been retired and
deleted) - and provided that TI decide to continue funding that work.
That's still uncertain since the whole TI OMAP thing imploded two months
ago.

Now, CPPI is brand new code to arch/arm - always has been.  It post-dates
the DMA engine API.  And it's been said many times about moving it to
drivers/dma.  The problem is Sergei doesn't want to do it - he's anti the
DMA engine API for whatever reasons he can dream up.  He professes no
knowledge of my dislike for having it in arch/arm, yet there's emails
from years back showing that he knew about it.  TI knows about it; Ajay
about it.  Yet... well... history speaks volumes about this.

Now, there may be a new problem with CPPI.  That being we're now requiring
DT support.  _Had_ this been done prior to the push for DT, then it would
not have been a concern, because then the problem becomes one for DT.
But now that OMAP is being converted to DT, and DT is The Way To Go now,
that's an additional problem that needs to be grappled with - or it may
make things easier.

However, as I've said now many times: CPPI is _not_ going in arch/arm.
Period.  That makes it not my problem.  Period.

I really have nothing further to say on CPPI; everything that can be said
has already been said.  If there's still pressure to have it in arch/arm,
I will _NOT_ respond to it, because there's nothing to be said about it
which hasn't been said before.  The only reason it's still being pressed
for is a total reluctance to do anything about it being in arch/arm.

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 16:45, Russell King - ARM Linux wrote:

> Now, CPPI is brand new code to arch/arm - always has been.  It post-dates
> the DMA engine API.  And it's been said many times about moving it to
> drivers/dma.  The problem is Sergei doesn't want to do it - he's anti the

    I *can't* do it, and I have denied all further responibility for it.

> DMA engine API for whatever reasons he can dream up.  He professes no

    I'm not dreaming anything up. Please understand that CPPI 4.1 is not just 
a normal DMA controller -- it's a complex of several devices with DMA 
controller being only one of them, and one that can't work autonomously but 
only thru the proxy of the queue manager. That queue manager stuff I found 
hard to fit into drivers/dma/ infrastructure. Myabe it was a honest mistake, 
of course.

> knowledge of my dislike for having it in arch/arm, yet there's emails
> from years back showing that he knew about it.  TI knows about it; Ajay
> about it.  Yet... well... history speaks volumes about this.

    Some details have slipped rom my memory after that many years. Im sorry 
for that.

> Now, there may be a new problem with CPPI.  That being we're now requiring
> DT support.  _Had_ this been done prior to the push for DT, then it would
> not have been a concern, because then the problem becomes one for DT.
> But now that OMAP is being converted to DT, and DT is The Way To Go now,
> that's an additional problem that needs to be grappled with - or it may
> make things easier.

    DaVinci is also being converted to device tree. However, that support 
remains optional for now. Not sure it will make things easier, as we still 
have two distinct devices to declare in the device tree: DMA controller and 
queue manager, and we'll have to describe the interconnect between them too 
(as a props of DMA controller I guess)...

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Matt Porter]

On Sat, Feb 02, 2013 at 12:01:37AM +0000, Sergei Shtylyov wrote:
> Hello.
> 
> On 01-02-2013 22:59, Matt Porter wrote:
> 
> >>>>> Move mach-davinci/dma.c to common/edma.c so it can be used
> >>>>> by OMAP (specifically AM33xx) as well.
> 
> >>>> I think this should rather go to drivers/dma/?
> 
> >>> No, this is the private EDMA API. It's the analogous thing to
> >>> the private OMAP dma API that is in plat-omap/dma.c. The actual
> >>> dmaengine driver is in drivers/dma/edma.c as a wrapper around
> >>> this...same way OMAP DMA engine conversion is being done.
> 
> >>    Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
> >> that, instead of waiting indefinitely for TI to convert it to drivers/dma/
> >> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.
> 
> > That is a shame. Yeah, I've pointed out that I was doing this exactly
> > the same way as was acceptable for the OMAP DMA conversion since it was
> > in RFC. The reasons are sound since in both cases, we have many drivers
> > to convert that need to continue using the private DMA APIs.
> 
>     In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other 
> in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is 
> sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't 
> know them well).

Well, it's pretty clear to me now that there's good reason for it not
landing in arch/arm/ so the obvious path is to do the dmaengine
conversion and put it in drivers/dma/ if it's really a generic dma engine.
I'm not sure why you express concern over the dma engine api not fitting
with CPPI 4.1? If it doesn't work, work with Vinod to fix the api. It's
expected, I'm working on dmaengine API changes right now to deal with a
limitation of EDMA that needs to be abstracted.

As pointed out, edma.c is already here in arch/arm/, so moving it doesn't
add something new. It does let us regression test all platforms that use it
(both Davinci and AM33xx) as I go through the conversion process.

-Matt

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Tony Lindgren]

* Matt Porter <mporter@ti.com> [130202 10:10]:
> If it doesn't work, work with Vinod to fix the api. It's expected,
> I'm working on dmaengine API changes right now to deal with a
> limitation of EDMA that needs to be abstracted.

Regarding the DMA API limitations, I'm only aware of lack of capability
to configure autoincrement at the device end. And that keeps us from
converting all GPMC related devices using omap SDMA to use the DMA API.

Are there other limitations currently known with the DMA API?

Regards,

Tony

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Matt Porter]

On Sat, Feb 02, 2013 at 10:16:43AM -0800, Tony Lindgren wrote:
> * Matt Porter <mporter@ti.com> [130202 10:10]:
> > If it doesn't work, work with Vinod to fix the api. It's expected,
> > I'm working on dmaengine API changes right now to deal with a
> > limitation of EDMA that needs to be abstracted.
> 
> Regarding the DMA API limitations, I'm only aware of lack of capability
> to configure autoincrement at the device end. And that keeps us from
> converting all GPMC related devices using omap SDMA to use the DMA API.
> 
> Are there other limitations currently known with the DMA API?

Yes, I called one out when this was first posted as an RFC and was
working it in parallel with this support. This one blocks AM33XX support
in mmc and is the reason I split all of the mmc support out of the base
edma dmaengine for am33xx series. Independent of the blockage, whatever
we finally settle on to address this API need will also cleanup the use
of magic values in the davinci mmc driver (that's part of the second
thread below).

RFC posting:
https://patchwork.kernel.org/patch/1583531/
Posting with initial attempt at a caps api:
http://www.spinics.net/lists/linux-mmc/msg18351.html

Also, I haven't fully vetted the situation with cyclic transfers and
EDMA, however, I'm pretty sure that we'll need some API changes there.
The reason is that some Davinci platforms have no FIFO on their McASP
implementation (that was a new feature added on DA8xx and also AM33xx).
As such they have audio support implemented using ping-pong buffering
via an SRAM buffer. There's been a number of patches ahead of all this
that myself and others have worked upstream to get the SRAM stuff to be
Davinci-independent (genalloc). But, at the end of all of this, there's
no notion in a cyclic transfer of doing synchronized ping-pong buffering
using two chain channels. This is how it is implemented (and documented
in EDMA docs going back to the DSPs this IP first showed up on) using
the private API. I'll be looking at this soon, but, I'm more interested
in 1) getting the base support in 2) then the mmc stuff blocking DT
populated platforms using omap_hsmmc (split out and posted) 3) v3 of the
caps api w/ vinod's concerns address (working it not)

Normally, I'm not going to bring up the cyclic transfer issue until I
have some code to show or reference for discussion...but it's worth
being aware. But, in any case, I'm confident that will gate having the
mcasp driver that am33xx also uses converted to dmaengine.  Except for
the gpmc limitation you menationed, that's the last in kernel user of
the privat edma api needed to be converted such that we can kill off
arch/arm/common/edma.c once it's taken in.

-Matt

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Tony Lindgren]

* Matt Porter <mporter@ti.com> [130202 11:51]:
> On Sat, Feb 02, 2013 at 10:16:43AM -0800, Tony Lindgren wrote:
> > * Matt Porter <mporter@ti.com> [130202 10:10]:
> > > If it doesn't work, work with Vinod to fix the api. It's expected,
> > > I'm working on dmaengine API changes right now to deal with a
> > > limitation of EDMA that needs to be abstracted.
> > 
> > Regarding the DMA API limitations, I'm only aware of lack of capability
> > to configure autoincrement at the device end. And that keeps us from
> > converting all GPMC related devices using omap SDMA to use the DMA API.
> > 
> > Are there other limitations currently known with the DMA API?
> 
> Yes, I called one out when this was first posted as an RFC and was
> working it in parallel with this support. This one blocks AM33XX support
> in mmc and is the reason I split all of the mmc support out of the base
> edma dmaengine for am33xx series. Independent of the blockage, whatever
> we finally settle on to address this API need will also cleanup the use
> of magic values in the davinci mmc driver (that's part of the second
> thread below).
> 
> RFC posting:
> https://patchwork.kernel.org/patch/1583531/
> Posting with initial attempt at a caps api:
> http://www.spinics.net/lists/linux-mmc/msg18351.html

OK thanks for the links, good to hear at least some work is happening
on it.

> Also, I haven't fully vetted the situation with cyclic transfers and
> EDMA, however, I'm pretty sure that we'll need some API changes there.
> The reason is that some Davinci platforms have no FIFO on their McASP
> implementation (that was a new feature added on DA8xx and also AM33xx).
> As such they have audio support implemented using ping-pong buffering
> via an SRAM buffer. There's been a number of patches ahead of all this
> that myself and others have worked upstream to get the SRAM stuff to be
> Davinci-independent (genalloc). But, at the end of all of this, there's
> no notion in a cyclic transfer of doing synchronized ping-pong buffering
> using two chain channels. This is how it is implemented (and documented
> in EDMA docs going back to the DSPs this IP first showed up on) using
> the private API. I'll be looking at this soon, but, I'm more interested
> in 1) getting the base support in 2) then the mmc stuff blocking DT
> populated platforms using omap_hsmmc (split out and posted) 3) v3 of the
> caps api w/ vinod's concerns address (working it not)
> 
> Normally, I'm not going to bring up the cyclic transfer issue until I
> have some code to show or reference for discussion...but it's worth
> being aware. But, in any case, I'm confident that will gate having the
> mcasp driver that am33xx also uses converted to dmaengine.  Except for
> the gpmc limitation you menationed, that's the last in kernel user of
> the privat edma api needed to be converted such that we can kill off
> arch/arm/common/edma.c once it's taken in.

And then there's the case of device-to-device DMA that we're not
currently doing luckily. And I don't think I've even seen that being
used so we probably don't need to worry about that one right now.

Regards,

Tony

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 22:07, Matt Porter wrote:

>>>>>>> Move mach-davinci/dma.c to common/edma.c so it can be used
>>>>>>> by OMAP (specifically AM33xx) as well.

>>>>>> I think this should rather go to drivers/dma/?

>>>>> No, this is the private EDMA API. It's the analogous thing to
>>>>> the private OMAP dma API that is in plat-omap/dma.c. The actual
>>>>> dmaengine driver is in drivers/dma/edma.c as a wrapper around
>>>>> this...same way OMAP DMA engine conversion is being done.

>>>>     Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
>>>> that, instead of waiting indefinitely for TI to convert it to drivers/dma/
>>>> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.

>>> That is a shame. Yeah, I've pointed out that I was doing this exactly
>>> the same way as was acceptable for the OMAP DMA conversion since it was
>>> in RFC. The reasons are sound since in both cases, we have many drivers
>>> to convert that need to continue using the private DMA APIs.

>>      In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other
>> in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is
>> sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't
>> know them well).

> Well, it's pretty clear to me now that there's good reason for it not
> landing in arch/arm/ so the obvious path is to do the dmaengine
> conversion and put it in drivers/dma/ if it's really a generic dma engine.
> I'm not sure why you express concern over the dma engine api not fitting
> with CPPI 4.1?

    It's not a DMA controller only, it's 3 distinct devices, with the DMA 
controller being one among them and using another one, the queue manager, as 
some sort of proxy. The third device doesn't exist on OMAP-L1x SoCs -- it's 
the buffer manager.

> If it doesn't work, work with Vinod to fix the api. It's
> expected, I'm working on dmaengine API changes right now to deal with a
> limitation of EDMA that needs to be abstracted.

    Sorry, now it's TI's task. I no longer have time to work on this (my 
internal project to push OMAP-L1x support upstream has expired at Sep 2010) 
and my future in MV is very uncertain at this moment. Most probably I'll leave 
it (or be forced to leave).

> As pointed out, edma.c is already here in arch/arm/, so moving it doesn't
> add something new. It does let us regression test all platforms that use it
> (both Davinci and AM33xx) as I go through the conversion process.

    Could have been the same with CPPI 4.1 in theory if it was added to 
mach-davinci/ back in 2009... we'd then only have to move it. EDMA code is 
much older of course, so it's probably more justified.

> -Matt

WBR, Sergei


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Matt Porter]

On Sat, Feb 02, 2013 at 07:06:06PM +0000, Sergei Shtylyov wrote:
> Hello.
> 
> On 02-02-2013 22:07, Matt Porter wrote:
> 
> >>>>>>> Move mach-davinci/dma.c to common/edma.c so it can be used
> >>>>>>> by OMAP (specifically AM33xx) as well.
> 
> >>>>>> I think this should rather go to drivers/dma/?
> 
> >>>>> No, this is the private EDMA API. It's the analogous thing to
> >>>>> the private OMAP dma API that is in plat-omap/dma.c. The actual
> >>>>> dmaengine driver is in drivers/dma/edma.c as a wrapper around
> >>>>> this...same way OMAP DMA engine conversion is being done.
> 
> >>>>     Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
> >>>> that, instead of waiting indefinitely for TI to convert it to drivers/dma/
> >>>> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.
> 
> >>> That is a shame. Yeah, I've pointed out that I was doing this exactly
> >>> the same way as was acceptable for the OMAP DMA conversion since it was
> >>> in RFC. The reasons are sound since in both cases, we have many drivers
> >>> to convert that need to continue using the private DMA APIs.
> 
> >>      In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other
> >> in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is
> >> sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't
> >> know them well).
> 
> > Well, it's pretty clear to me now that there's good reason for it not
> > landing in arch/arm/ so the obvious path is to do the dmaengine
> > conversion and put it in drivers/dma/ if it's really a generic dma engine.
> > I'm not sure why you express concern over the dma engine api not fitting
> > with CPPI 4.1?
> 
>     It's not a DMA controller only, it's 3 distinct devices, with the DMA 
> controller being one among them and using another one, the queue manager, as 
> some sort of proxy. The third device doesn't exist on OMAP-L1x SoCs -- it's 
> the buffer manager.

Interesting, and you don't see a way to have this split between
dmaengine and the musb driver? This all assumes there's even a match as
RMK did raise concerns elsewhere in this thread.

> > If it doesn't work, work with Vinod to fix the api. It's
> > expected, I'm working on dmaengine API changes right now to deal with a
> > limitation of EDMA that needs to be abstracted.
> 
>     Sorry, now it's TI's task. I no longer have time to work on this (my 
> internal project to push OMAP-L1x support upstream has expired at Sep 2010) 
> and my future in MV is very uncertain at this moment. Most probably I'll leave 
> it (or be forced to leave).

Too bad, it's certainly "somebody's task". The people employed by TI
have names too. ;) I suspect it falls to Felipe or Kishon these days,
but I try to avoid USB as it's generally a source of pain.

> > As pointed out, edma.c is already here in arch/arm/, so moving it doesn't
> > add something new. It does let us regression test all platforms that use it
> > (both Davinci and AM33xx) as I go through the conversion process.
> 
>     Could have been the same with CPPI 4.1 in theory if it was added to 
> mach-davinci/ back in 2009... we'd then only have to move it. EDMA code is 
> much older of course, so it's probably more justified.

Absolutely, timing is everything. I can assure you I've flamed enough
people "internally" about leaving this EDMA dmaengine conversion for so
long. As you might have guessed, AM33xx is a bit of a driver for it, but
all of this would have been quite a bit simpler had somebody taken a
little effort and moved edma to dmaengine years ago when slave dma
support was added to dmaengine. ;)

-Matt

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 23:55, Matt Porter wrote:

>>>>>>>>> Move mach-davinci/dma.c to common/edma.c so it can be used
>>>>>>>>> by OMAP (specifically AM33xx) as well.

>>>>>>>> I think this should rather go to drivers/dma/?

>>>>>>> No, this is the private EDMA API. It's the analogous thing to
>>>>>>> the private OMAP dma API that is in plat-omap/dma.c. The actual
>>>>>>> dmaengine driver is in drivers/dma/edma.c as a wrapper around
>>>>>>> this...same way OMAP DMA engine conversion is being done.

>>>>>>      Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
>>>>>> that, instead of waiting indefinitely for TI to convert it to drivers/dma/
>>>>>> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.

>>>>> That is a shame. Yeah, I've pointed out that I was doing this exactly
>>>>> the same way as was acceptable for the OMAP DMA conversion since it was
>>>>> in RFC. The reasons are sound since in both cases, we have many drivers
>>>>> to convert that need to continue using the private DMA APIs.

>>>>       In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other
>>>> in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is
>>>> sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't
>>>> know them well).

>>> Well, it's pretty clear to me now that there's good reason for it not
>>> landing in arch/arm/ so the obvious path is to do the dmaengine
>>> conversion and put it in drivers/dma/ if it's really a generic dma engine.
>>> I'm not sure why you express concern over the dma engine api not fitting
>>> with CPPI 4.1?

>>      It's not a DMA controller only, it's 3 distinct devices, with the DMA
>> controller being one among them and using another one, the queue manager, as
>> some sort of proxy. The third device doesn't exist on OMAP-L1x SoCs -- it's
>> the buffer manager.

> Interesting, and you don't see a way to have this split between
> dmaengine and the musb driver?

    This can't be split into the MUSB DMA driver, as the neither of CPPI 4.1 
devices are really MUSB specific. The support should be generic.

> This all assumes there's even a match as
> RMK did raise concerns elsewhere in this thread.

    Didn't quite get this sentence.

>>> If it doesn't work, work with Vinod to fix the api. It's
>>> expected, I'm working on dmaengine API changes right now to deal with a
>>> limitation of EDMA that needs to be abstracted.

>>      Sorry, now it's TI's task. I no longer have time to work on this (my
>> internal project to push OMAP-L1x support upstream has expired at Sep 2010)

    If not somewhat earlier... anyway, I wasn't able to spent much time on 
this project in 2010.

>> and my future in MV is very uncertain at this moment. Most probably I'll leave
>> it (or be forced to leave).

> Too bad, it's certainly "somebody's task". The people employed by TI
> have names too. ;) I suspect it falls to Felipe or Kishon these days,
> but I try to avoid USB as it's generally a source of pain.

    I'm probably masochistic then since I'm still sending MUSB patches, eben 
though I wasn't working on it at MV until I switched to Kontron boards 2 weeks 
ago. Now my task is fixing USB bugs on real Sitara with dual MUSB. :-)

>>> As pointed out, edma.c is already here in arch/arm/, so moving it doesn't
>>> add something new. It does let us regression test all platforms that use it
>>> (both Davinci and AM33xx) as I go through the conversion process.

>>      Could have been the same with CPPI 4.1 in theory if it was added to
>> mach-davinci/ back in 2009... we'd then only have to move it. EDMA code is

    I don't know why Kevin didn't merge it then. I remembere he didn't like 
that it was not a proper platform driver and was tied with the platform code 
thru some variables, and I refused to change that...

>> much older of course, so it's probably more justified.

> Absolutely, timing is everything. I can assure you I've flamed enough
> people "internally" about leaving this EDMA dmaengine conversion for so
> long. As you might have guessed, AM33xx is a bit of a driver for it, but
> all of this would have been quite a bit simpler had somebody taken a
> little effort and moved edma to dmaengine years ago when slave dma
> support was added to dmaengine. ;)

    Hm, it seems to have happened back in 2008 when I was working on CPPI 4.1 
code. Too bad I only knew that drivers/dma/ are for accelerating RAIDs back 
then (if actually noot later than that).
    TI seems to put more efforts into its Arago project than in supoporting 
the cutting edge (or not) CPUs in mainline -- at lest things seem go there 
first. Many Arago patches never reached mainline (not that they can be applied 
without cleanup though).

> -Matt

WBR, Sergei


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02/01/2013 09:49 PM, Matt Porter wrote:

>>> Move mach-davinci/dma.c to common/edma.c so it can be used
>>> by OMAP (specifically AM33xx) as well.

>> I think this should rather go to drivers/dma/?

> No, this is the private EDMA API. It's the analogous thing to
> the private OMAP dma API that is in plat-omap/dma.c. The actual
> dmaengine driver is in drivers/dma/edma.c as a wrapper around
> this...same way OMAP DMA engine conversion is being done.

  Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
that, instead of waiting indefinitely for TI to convert it to drivers/dma/
directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.

> -Matt

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 1158 bytes --]

Hi,

On Fri, Feb 01, 2013 at 10:52:46PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 02/01/2013 09:49 PM, Matt Porter wrote:
> 
> >>> Move mach-davinci/dma.c to common/edma.c so it can be used
> >>> by OMAP (specifically AM33xx) as well.
> 
> >> I think this should rather go to drivers/dma/?
> 
> > No, this is the private EDMA API. It's the analogous thing to
> > the private OMAP dma API that is in plat-omap/dma.c. The actual
> > dmaengine driver is in drivers/dma/edma.c as a wrapper around
> > this...same way OMAP DMA engine conversion is being done.
> 
>   Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
> that, instead of waiting indefinitely for TI to convert it to drivers/dma/
> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.

good point, do you wanna send some patches ?

I guess to make the MUSB side simpler we would need musb-dma-engine glue
to map dmaengine to the private MUSB API. Then we would have some
starting point to also move inventra (and anybody else) to dmaengine
API.

Once that's done, we drop MUSB's private API.

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02/01/2013 09:58 PM, Felipe Balbi wrote:

>>>>> Move mach-davinci/dma.c to common/edma.c so it can be used
>>>>> by OMAP (specifically AM33xx) as well.

>>>> I think this should rather go to drivers/dma/?

>>> No, this is the private EDMA API. It's the analogous thing to
>>> the private OMAP dma API that is in plat-omap/dma.c. The actual
>>> dmaengine driver is in drivers/dma/edma.c as a wrapper around
>>> this...same way OMAP DMA engine conversion is being done.

>>   Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed
>> that, instead of waiting indefinitely for TI to convert it to drivers/dma/
>> directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh.

> good point, do you wanna send some patches ?

   I have already sent them countless times and even stuck CPPI 4.1 support (in
arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
patch. :-(

> I guess to make the MUSB side simpler we would need musb-dma-engine glue
> to map dmaengine to the private MUSB API. Then we would have some
> starting point to also move inventra (and anybody else) to dmaengine
> API.

   Why? Inventra is a dedicated device's private DMA controller, why make
universal DMA driver for it?

> Once that's done, we drop MUSB's private API.

   Don't think it's a good idea.

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 1117 bytes --]

hi,

On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote:
> > good point, do you wanna send some patches ?
> 
>    I have already sent them countless times and even stuck CPPI 4.1 support (in
> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
> patch. :-(

sticking into arch/arm/common/ wasn't a nice move. But then again, so
wasn't asking for the patch to be removed :-s

> > I guess to make the MUSB side simpler we would need musb-dma-engine glue
> > to map dmaengine to the private MUSB API. Then we would have some
> > starting point to also move inventra (and anybody else) to dmaengine
> > API.
> 
>    Why? Inventra is a dedicated device's private DMA controller, why make
> universal DMA driver for it?

because it doesn't make sense to support multiple DMA APIs. We can check
from MUSB's registers if it was configured with Inventra DMA support and
based on that we can register MUSB's own DMA Engine to dmaengine API.

> > Once that's done, we drop MUSB's private API.
> 
>    Don't think it's a good idea.

see above.

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Fri, Feb 01, 2013 at 10:56:00PM +0200, Felipe Balbi wrote:
> hi,
> 
> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote:
> > > good point, do you wanna send some patches ?
> > 
> >    I have already sent them countless times and even stuck CPPI 4.1 support (in
> > arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
> > patch. :-(
> 
> sticking into arch/arm/common/ wasn't a nice move. But then again, so
> wasn't asking for the patch to be removed :-s

Err, patches don't get removed, they get moved to 'discarded'.

> > > I guess to make the MUSB side simpler we would need musb-dma-engine glue
> > > to map dmaengine to the private MUSB API. Then we would have some
> > > starting point to also move inventra (and anybody else) to dmaengine
> > > API.
> > 
> >    Why? Inventra is a dedicated device's private DMA controller, why make
> > universal DMA driver for it?
> 
> because it doesn't make sense to support multiple DMA APIs. We can check
> from MUSB's registers if it was configured with Inventra DMA support and
> based on that we can register MUSB's own DMA Engine to dmaengine API.

Hang on.  This is one of the DMA implementations which is closely
coupled with the USB and only the USB?  If it is...

I thought this had been discussed _extensively_ before.  I thought the
resolution on it was:
1. It would not use the DMA engine API.
2. It would not live in arch/arm.
3. It would be placed nearby the USB driver it's associated with.

(1) because we don't use APIs just for the hell of it - think.  Do we
use the DMA engine API for PCI bus mastering ethernet controllers?  No.
Do we use it for PCI bus mastering SCSI controllers?  No.  Because the
DMA is integral to the rest of the device.

The DMA engine API only makes sense if the DMA engine is a shared
system resource.

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 1:30, Russell King - ARM Linux wrote:

>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote:
>>>> good point, do you wanna send some patches ?

>>>     I have already sent them countless times and even stuck CPPI 4.1 support (in
>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
>>> patch. :-(

>> sticking into arch/arm/common/ wasn't a nice move. But then again, so
>> wasn't asking for the patch to be removed :-s

> Err, patches don't get removed, they get moved to 'discarded'.

    Any chance to bring it back to life? :-)
    Although... drivers/usb/musb/cppi41.c would need to be somewhat reworked 
for at least AM35x and I don't have time. But that may change, of course.

>>>> I guess to make the MUSB side simpler we would need musb-dma-engine glue
>>>> to map dmaengine to the private MUSB API. Then we would have some
>>>> starting point to also move inventra (and anybody else) to dmaengine
>>>> API.

>>>     Why? Inventra is a dedicated device's private DMA controller, why make
>>> universal DMA driver for it?

>> because it doesn't make sense to support multiple DMA APIs. We can check
>> from MUSB's registers if it was configured with Inventra DMA support and
>> based on that we can register MUSB's own DMA Engine to dmaengine API.

> Hang on.  This is one of the DMA implementations which is closely
> coupled with the USB and only the USB?  If it is...

> I thought this had been discussed _extensively_ before.  I thought the
> resolution on it was:
> 1. It would not use the DMA engine API.
> 2. It would not live in arch/arm.
> 3. It would be placed nearby the USB driver it's associated with.

> (1) because we don't use APIs just for the hell of it - think.  Do we
> use the DMA engine API for PCI bus mastering ethernet controllers?  No.
> Do we use it for PCI bus mastering SCSI controllers?  No.  Because the
> DMA is integral to the rest of the device.

> The DMA engine API only makes sense if the DMA engine is a shared
> system resource.

    Thanks for such extensive wording in the support of my point. :-)

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Sat, Feb 02, 2013 at 04:07:59AM +0400, Sergei Shtylyov wrote:
> Hello.
>
> On 02-02-2013 1:30, Russell King - ARM Linux wrote:
>
>>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote:
>>>>> good point, do you wanna send some patches ?
>
>>>>     I have already sent them countless times and even stuck CPPI 4.1 support (in
>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
>>>> patch. :-(
>
>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so
>>> wasn't asking for the patch to be removed :-s
>
>> Err, patches don't get removed, they get moved to 'discarded'.
>
>    Any chance to bring it back to life? :-)
>    Although... drivers/usb/musb/cppi41.c would need to be somewhat 
> reworked for at least AM35x and I don't have time. But that may change, 
> of course.

Right, I've just looked back at the various meeting minutes from December
2010 when the CPPI stuff was discussed.  Yes, I archive these things and
all email discussions for referencing in cases like this.

Unfortunately, they do not contain any useful information other than the
topic having been brought up.  At that point, the CPPI stuff was in
mach-davinci, and I had suggested moving it into drivers/dma.

The result of that was to say that it doesn't fit the DMA engine APIs.
So someone came up with the idea of putting it in arch/arm/common - which
I frankly ignored by email (how long have we been saying "no drivers in
arch/arm" ?)

Now, it would've been discussed in that meeting, but unfortunately no
record exists of that.  What does follow that meeting is a discussion
trail.  From what I can see there, but it looks to me like the decision
was taken to move it to the DMA engine API, and work on sorting out MUSB
was going to commence.

The last email in that says "I'll get to that soon"... and that is also
the final email I have on this topic.  I guess if nothing has happened...
Shrug, that's someone elses problem.

Anyway, the answer for putting it in arch/arm/common hasn't changed,
and really, where we are now, post Linus having a moan about the size
of arch/arm, that answer is even more concrete in the negative.  It's
54K of code which should not be under arch/arm at all.

Anyway, if you need to look at the patch, it's 6305/1.  Typing into the
summary search box 'cppi' found it in one go.

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 4:44, Russell King - ARM Linux wrote:

>>>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote:
>>>>>> good point, do you wanna send some patches ?

>>>>>      I have already sent them countless times and even stuck CPPI 4.1 support (in
>>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
>>>>> patch. :-(

>>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so
>>>> wasn't asking for the patch to be removed :-s

>>> Err, patches don't get removed, they get moved to 'discarded'.

>>     Any chance to bring it back to life? :-)
>>     Although... drivers/usb/musb/cppi41.c would need to be somewhat
>> reworked for at least AM35x and I don't have time. But that may change,
>> of course.

> Right, I've just looked back at the various meeting minutes from December
> 2010 when the CPPI stuff was discussed.  Yes, I archive these things and
> all email discussions for referencing in cases like this.

    Thanks.

> Unfortunately, they do not contain any useful information other than the
> topic having been brought up.  At that point, the CPPI stuff was in
> mach-davinci, and I had suggested moving it into drivers/dma.

    I don't remember that, probably was out of the loop again.

> The result of that was to say that it doesn't fit the DMA engine APIs.

    I remember this as a discussion happening post me sending the patch to the 
patch system and it being discarded...

> So someone came up with the idea of putting it in arch/arm/common - which

    Probably was me. There was also idea of putting it into drivers/usb/musb/ 
-- which TI indeed followed in its Arago prject. I firmly denied that suggestion.

> I frankly ignored by email (how long have we been saying "no drivers in
> arch/arm" ?)

    But there *are* drivers there! And look at edma.c which is about to be 
moved there... Anyway, I haven't seen such warnings, probably was too late in 
the game.

> Now, it would've been discussed in that meeting, but unfortunately no
> record exists of that.  What does follow that meeting is a discussion
> trail.  From what I can see there, but it looks to me like the decision
> was taken to move it to the DMA engine API, and work on sorting out MUSB
> was going to commence.

> The last email in that says "I'll get to that soon"... and that is also
> the final email I have on this topic.  I guess if nothing has happened...
> Shrug, that's someone elses problem.

    Well, as usual... :-(

> Anyway, the answer for putting it in arch/arm/common hasn't changed,
> and really, where we are now, post Linus having a moan about the size
> of arch/arm, that answer is even more concrete in the negative.  It's
> 54K of code which should not be under arch/arm at all.

> Anyway, if you need to look at the patch, it's 6305/1.  Typing into the
> summary search box 'cppi' found it in one go.

    Thanks, I remember this variant was under arch/arm/common/.
    Now however, I see what happened to that variant in somewhat different 
light. Looks like it was entirely your decision to discard the patch, without 
TI's request...

WBR, Sergei


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Sat, Feb 02, 2013 at 06:09:24AM +0400, Sergei Shtylyov wrote:
> Hello.
>
> On 02-02-2013 4:44, Russell King - ARM Linux wrote:
>
>>>>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote:
>>>>>>> good point, do you wanna send some patches ?
>
>>>>>>      I have already sent them countless times and even stuck CPPI 4.1 support (in
>>>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
>>>>>> patch. :-(
>
>>>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so
>>>>> wasn't asking for the patch to be removed :-s
>
>>>> Err, patches don't get removed, they get moved to 'discarded'.
>
>>>     Any chance to bring it back to life? :-)
>>>     Although... drivers/usb/musb/cppi41.c would need to be somewhat
>>> reworked for at least AM35x and I don't have time. But that may change,
>>> of course.
>
>> Right, I've just looked back at the various meeting minutes from December
>> 2010 when the CPPI stuff was discussed.  Yes, I archive these things and
>> all email discussions for referencing in cases like this.
>
>    Thanks.
>
>> Unfortunately, they do not contain any useful information other than the
>> topic having been brought up.  At that point, the CPPI stuff was in
>> mach-davinci, and I had suggested moving it into drivers/dma.
>
>    I don't remember that, probably was out of the loop again.
>
>> The result of that was to say that it doesn't fit the DMA engine APIs.
>
>    I remember this as a discussion happening post me sending the patch to 
> the patch system and it being discarded...
>
>> So someone came up with the idea of putting it in arch/arm/common - which
>
>    Probably was me. There was also idea of putting it into 
> drivers/usb/musb/ -- which TI indeed followed in its Arago prject. I 
> firmly denied that suggestion.
>
>> I frankly ignored by email (how long have we been saying "no drivers in
>> arch/arm" ?)
>
>    But there *are* drivers there! And look at edma.c which is about to be 
> moved there... Anyway, I haven't seen such warnings, probably was too 
> late in the game.

I've already objected about the header moving to some random place in
arch/arm/include.  Really, edma.c needs to find another home too - but
there's a difference here.  edma.c is already present under arch/arm.
CPPI is _not_.  CPPI is new code appearing under arch/arm (you can see
that for yourself by looking at the diffstat of 6305/1... it doesn't
move files, it adds new code.)

>> Now, it would've been discussed in that meeting, but unfortunately no
>> record exists of that.  What does follow that meeting is a discussion
>> trail.  From what I can see there, but it looks to me like the decision
>> was taken to move it to the DMA engine API, and work on sorting out MUSB
>> was going to commence.
>
>> The last email in that says "I'll get to that soon"... and that is also
>> the final email I have on this topic.  I guess if nothing has happened...
>> Shrug, that's someone elses problem.
>
>    Well, as usual... :-(
>
>> Anyway, the answer for putting it in arch/arm/common hasn't changed,
>> and really, where we are now, post Linus having a moan about the size
>> of arch/arm, that answer is even more concrete in the negative.  It's
>> 54K of code which should not be under arch/arm at all.
>
>> Anyway, if you need to look at the patch, it's 6305/1.  Typing into the
>> summary search box 'cppi' found it in one go.
>
>    Thanks, I remember this variant was under arch/arm/common/.
>    Now however, I see what happened to that variant in somewhat different 
> light. Looks like it was entirely your decision to discard the patch, 
> without TI's request...

Firstly, it is *my* perogative to say no to anything in arch/arm, and I
really don't have to give reasons for it if I choose to.

Secondly, it *was* discussed with TI, and the following thread of
discussion (threaded to the minutes email) shows that *something* was
going to happen _as a result of that meeting_ to address the problem of
it being under arch/arm.  And *therefore* it was discarded from the patch
system - because there was expectation that it was going to get fixed.

For christ sake, someone even agreed to do it.  Even a target was mentioned,
of 2.6.39.  That was mentioned on 7th December 2010.  And 6305/1 was
discarded on 8th December 2010.  Cause and effect.

And yes, *you* were not part of that discussion.  You work for Montavista
which contracts with TI to provide this support.  It is up to TI to pass
stuff like this on to their contractors.

There are two people on this thread CC list who were also involved or
CC'd on the mails from the thread in 2010...  Tony and Felipe.
Unfortunately, the person who agreed to do the work is no longer in the
land of the living.  Yes I know it's inconvenient for people to die
when they've still got lots of important work to do but that's what can
happen...

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Sat, Feb 02, 2013 at 10:18:51AM +0000, Russell King - ARM Linux wrote:
> On Sat, Feb 02, 2013 at 06:09:24AM +0400, Sergei Shtylyov wrote:
> > Hello.
> >
> > On 02-02-2013 4:44, Russell King - ARM Linux wrote:
> >
> >>>>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote:
> >>>>>>> good point, do you wanna send some patches ?
> >
> >>>>>>      I have already sent them countless times and even stuck CPPI 4.1 support (in
> >>>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
> >>>>>> patch. :-(
> >
> >>>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so
> >>>>> wasn't asking for the patch to be removed :-s
> >
> >>>> Err, patches don't get removed, they get moved to 'discarded'.
> >
> >>>     Any chance to bring it back to life? :-)
> >>>     Although... drivers/usb/musb/cppi41.c would need to be somewhat
> >>> reworked for at least AM35x and I don't have time. But that may change,
> >>> of course.
> >
> >> Right, I've just looked back at the various meeting minutes from December
> >> 2010 when the CPPI stuff was discussed.  Yes, I archive these things and
> >> all email discussions for referencing in cases like this.
> >
> >    Thanks.
> >
> >> Unfortunately, they do not contain any useful information other than the
> >> topic having been brought up.  At that point, the CPPI stuff was in
> >> mach-davinci, and I had suggested moving it into drivers/dma.
> >
> >    I don't remember that, probably was out of the loop again.

Here you go - this goes back even _further_ - November 2009 - on the
mailing list.  The entire thread:

http://lists.arm.linux.org.uk/lurker/thread/20091102.105759.a54cf3f5.en.html

And selected emails from it:

http://lists.arm.linux.org.uk/lurker/message/20091102.103706.38c029b5.en.html
On Mon, Nov 02, 2009 at 10:37:06AM +0000, I wrote:
| On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote:
| > Another option is to create arch/arm/ti-common to place all TI platform's
| > common software, such as CPPI4.1 used both in DA8xx and AM3517.
| 
| No thanks.  I really don't see why we should allow TI to have yet more
| directories scattered throughout the tree that are out of place with
| existing conventions.
| 
| And what is this CPPI thing anyway?
| 
| http://acronyms.thefreedictionary.com/CPPI
| 
| "Communications Port Programming Interface" seems to be about the best
| applicable one from that list!
| 
| If it's a USB DMA device (from the patches I can find, that seems to be
| the case) then why can't it live in drivers/usb or drivers/dma ?

And again:

http://lists.arm.linux.org.uk/lurker/message/20091102.115458.61cde450.en.html
On Mon, Nov 02, 2009 at 11:54:58AM +0000, I wrote:
| On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote:
| > CPPI4.1 DMA engine can be used either by USB or by Ethernet interface though
| > currently only USB is using it but in future even Ethernet devices may use it.
| 
| drivers/dma does seem to be the right place for this.

http://lists.arm.linux.org.uk/lurker/message/20091102.110217.adec3ca7.en.html
Even Felipe Balbi said so:
| you might want to provide support for it via drivers/dma and for the
| musb stuff, you just add the wrappers musb uses. See how tusb6010_omap.c
| uses OMAP's system dma which is also used by any other driver which
| requests a dma channel.

And it seems that _YOU_ did get the message - see your quoted text in:
http://lists.arm.linux.org.uk/lurker/message/20091230.132240.ecd56b3d.en.html
> We're currently having it there but the matter is it should be shred
> between different platforms, so arch/arm/common/ seems like the right
> place (which Russell didn't like, suggesting ill suited for that
> drivers/dma/ instead).

See - you acknowledge here that I don't like it.  So you _KNOW_ my views
on it in December 2009, contary to what you're saying in this thread.

Yet, you persisted with putting it in arch/arm/common:

http://lists.arm.linux.org.uk/lurker/message/20100515.181453.472c7c10.en.html
| Changes since the previous version:
| - moved everything from arch/arm/mach-davinci/ to arch/arm/common/;
| - s/CONFIG_CPPI41/CONFIG_TI_CPPI41/, made that option invisible;
| - added #include <linux/slab.h> for kzalloc();
| - switched alloc_queue() and cppi41_queue_free() to using bit operations;
| - replaced 'static' linking_ram[] by local variable in cppi41_queue_mgr_init();
| - fixed pr_debug() in cppi41_dma_ctrlr_init() to print the real queue manager #.

So, see, I had already objected to it being in arch/arm well before
you stuck your patch into the patch system.  And somehow you think
that ignoring my previous comments and doing it anyway will result in
progress?

So, let's recap.  The timeline behind this is:

+ 2 Nov 2009: Question asked about putting it in arch/arm/ti-common
  + I responded saying a clear no to that, suggesting other locations
    all of which were outside arch/arm.
  + I responded again saying an hour or two later saying the same thing.
  + Felipe Balbi agreed with drivers/dma.
+ 15 May 2010: v5 posted with it in arch/arm/common
+ 06 Aug 2010: put into patch system sa 6305/1
+ 06 Dec 2010: TI meeting.
  + Pre-meeting notes show that my views on this are known:
    + I'll quote this from it: "Russell suggested to move the driver at
      drivers/dma/".
    + Raises concern that DMA engine API may not fit.
  + I respond to that concern as work has been done on the DMA engine API
    to improve the slave mode support recently as a result of Linus Walleij's
    work on AMBA PL08x DMA support.
    (I would _not_ have done so if I had changed my view about it being
     under drivers/dma/).
+ 07 Dec 2010: emails talking about moving MUSB over to DMA engine API
  so that MUSB should not care about its DMA backend (that being CPPI
  or some other one.)
  + Email with "Let's see if I can get it all done by 2.6.39."
+ 08 Dec 2010: patch 6305/1 discarded from the patch system as there now
  seems to be concensus on the issue.
+ 03 Jan 2011: you ask me why it was discarded
  http://lists.arm.linux.org.uk/lurker/thread/20110103.160610.8cbe8e7d.en.html
  + I respond "It may have been that it's inventing its own API rather
    than using something like the DMA engine API."
  + Ajay said: "This issue was discussed recently at TI and proposal was
    to place it to drivers/dma folder. Moreover, even Felipe also seems
    to move other musb DMAs (Inventra, CPPI3.0, TUSB) to drivers/dma."

Oh, and then we come to this interesting email from Felipe to you, in
response to your reluctance to put it in drivers/dma.
| Do I really have to spell it out ? Really ?
| 
| You don't need to physically move the part of the code to drivers/dma,
| but it has to use the API. The mentor DMA is internal to MUSB.
| tusb6010_omap.c isn't.
| 
| Where it makes sense to move the code under drivers/dma, it will be
| done, where it doesn't, it won't be done, but it will use the same API.
| That's all.
| 
| The end goal is just to drop all these ad-hoc "APIs" for accessing DMA
| on musb code.

See the common theme here?  I don't like it under arch/arm.  I've been
pretty _consistent_ in suggesting drivers/dma/ all through that...
Others have said it.  People even acknowledge that's what I've been
saying, people who were not in the original discussion.

What I think is this: it is _YOU_ who don't want to hear that message,
so _YOU_ are intentionally ignoring it, and _YOU_ are looking for
someone to blame for it.  You've decided I'm to blame because _YOU_
aren't listening.

The reason there hasn't been any progress on this is _NOT_ down to me.
I've provided my feedback, and promptly been ignored.  Others have told
you the same thing, and promptly been ignored.  Sorry, this is not my
problem.  This is entirely YOUR problem, one of your own making.

But whatever.  We are NOT going to put CPPI under arch/arm.  Not now that
during the last merge window Linus complained to Arnd about the amount of
code coming through for arch/arm _AGAIN_.  Not after last time Linus
complained about TI OMAP which prompted the creation of arm-soc.  AND THAT
IS *FINAL*.  CPPI DMA SUPPORT IS *NOT* GOING UNDER ARCH/ARM.  PERIOD.  NOT
GOING TO HAPPEN.

Is this clear enough yet, or how many more years of emails do you need
yet more emails to get this message through?

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 16:17, Russell King - ARM Linux wrote:

>>>>>>>>> good point, do you wanna send some patches ?

>>>>>>>>       I have already sent them countless times and even stuck CPPI 4.1 support (in
>>>>>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
>>>>>>>> patch. :-(

>>>>>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so
>>>>>>> wasn't asking for the patch to be removed :-s

>>>>>> Err, patches don't get removed, they get moved to 'discarded'.

>>>>>      Any chance to bring it back to life? :-)
>>>>>      Although... drivers/usb/musb/cppi41.c would need to be somewhat
>>>>> reworked for at least AM35x and I don't have time. But that may change,
>>>>> of course.

>>>> Right, I've just looked back at the various meeting minutes from December
>>>> 2010 when the CPPI stuff was discussed.  Yes, I archive these things and
>>>> all email discussions for referencing in cases like this.

>>>     Thanks.

    Thanks again for taking your time to rummage thru the mail archives. I 
also did it, however not thru all threads (it turned out that the placement of 
CPPI 4.1 code was discussed also in the MUSB DMA driver related threads). 
Anyway, I was not a position to do extensive searching as it was already dead 
of the night in Moscow.

>>>> Unfortunately, they do not contain any useful information other than the
>>>> topic having been brought up.  At that point, the CPPI stuff was in
>>>> mach-davinci, and I had suggested moving it into drivers/dma.

>>>     I don't remember that, probably was out of the loop again.

> Here you go - this goes back even _further_ - November 2009 - on the
> mailing list.  The entire thread:

> http://lists.arm.linux.org.uk/lurker/thread/20091102.105759.a54cf3f5.en.html

> And selected emails from it:

> http://lists.arm.linux.org.uk/lurker/message/20091102.103706.38c029b5.en.html
> On Mon, Nov 02, 2009 at 10:37:06AM +0000, I wrote:
> | On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote:
> | > Another option is to create arch/arm/ti-common to place all TI platform's
> | > common software, such as CPPI4.1 used both in DA8xx and AM3517.
> |
> | No thanks.  I really don't see why we should allow TI to have yet more
> | directories scattered throughout the tree that are out of place with
> | existing conventions.
> |
> | And what is this CPPI thing anyway?
> |
> | http://acronyms.thefreedictionary.com/CPPI
> |
> | "Communications Port Programming Interface" seems to be about the best
> | applicable one from that list!
> |
> | If it's a USB DMA device (from the patches I can find, that seems to be
> | the case) then why can't it live in drivers/usb or drivers/dma ?

> And again:

> http://lists.arm.linux.org.uk/lurker/message/20091102.115458.61cde450.en.html
> On Mon, Nov 02, 2009 at 11:54:58AM +0000, I wrote:
> | On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote:
> | > CPPI4.1 DMA engine can be used either by USB or by Ethernet interface though
> | > currently only USB is using it but in future even Ethernet devices may use it.
> |
> | drivers/dma does seem to be the right place for this.

> http://lists.arm.linux.org.uk/lurker/message/20091102.110217.adec3ca7.en.html
> Even Felipe Balbi said so:
> | you might want to provide support for it via drivers/dma and for the
> | musb stuff, you just add the wrappers musb uses. See how tusb6010_omap.c
> | uses OMAP's system dma which is also used by any other driver which
> | requests a dma channel.

> And it seems that _YOU_ did get the message - see your quoted text in:
> http://lists.arm.linux.org.uk/lurker/message/20091230.132240.ecd56b3d.en.html
>> We're currently having it there but the matter is it should be shred
>> between different platforms, so arch/arm/common/ seems like the right
>> place (which Russell didn't like, suggesting ill suited for that
>> drivers/dma/ instead).

> See - you acknowledge here that I don't like it.  So you _KNOW_ my views
> on it in December 2009, contary to what you're saying in this thread.

    OK, now it seems I misremembered.

> Yet, you persisted with putting it in arch/arm/common:

    Being unable to fit it into drivers/dma/, and loating to place it into 
drivers/usb/, I had no other option. :-)

> http://lists.arm.linux.org.uk/lurker/message/20100515.181453.472c7c10.en.html
> | Changes since the previous version:
> | - moved everything from arch/arm/mach-davinci/ to arch/arm/common/;
> | - s/CONFIG_CPPI41/CONFIG_TI_CPPI41/, made that option invisible;
> | - added #include <linux/slab.h> for kzalloc();
> | - switched alloc_queue() and cppi41_queue_free() to using bit operations;
> | - replaced 'static' linking_ram[] by local variable in cppi41_queue_mgr_init();
> | - fixed pr_debug() in cppi41_dma_ctrlr_init() to print the real queue manager #.

> So, see, I had already objected to it being in arch/arm well before
> you stuck your patch into the patch system.  And somehow you think
> that ignoring my previous comments and doing it anyway will result in
> progress?

    I probably did that out of hopelessness partly.

> So, let's recap.  The timeline behind this is:

> + 2 Nov 2009: Question asked about putting it in arch/arm/ti-common
>    + I responded saying a clear no to that, suggesting other locations
>      all of which were outside arch/arm.
>    + I responded again saying an hour or two later saying the same thing.
>    + Felipe Balbi agreed with drivers/dma.
> + 15 May 2010: v5 posted with it in arch/arm/common
> + 06 Aug 2010: put into patch system sa 6305/1
> + 06 Dec 2010: TI meeting.
>    + Pre-meeting notes show that my views on this are known:
>      + I'll quote this from it: "Russell suggested to move the driver at
>        drivers/dma/".
>      + Raises concern that DMA engine API may not fit.
>    + I respond to that concern as work has been done on the DMA engine API
>      to improve the slave mode support recently as a result of Linus Walleij's
>      work on AMBA PL08x DMA support.
>      (I would _not_ have done so if I had changed my view about it being
>       under drivers/dma/).

    Unfortunately, as I see it, that work is far from enough for CPPI 4.1.
I maybe don't know drivers/dma/ slave mode well, but adding CPPI 4.1 support 
would have required a complete redesign of all related interfaces.

> + 07 Dec 2010: emails talking about moving MUSB over to DMA engine API
>    so that MUSB should not care about its DMA backend (that being CPPI
>    or some other one.)

    As we know, it was a bad idea generally.

>    + Email with "Let's see if I can get it all done by 2.6.39."

    Didn't ever see that one.

> + 08 Dec 2010: patch 6305/1 discarded from the patch system as there now
>    seems to be concensus on the issue.
> + 03 Jan 2011: you ask me why it was discarded
>    http://lists.arm.linux.org.uk/lurker/thread/20110103.160610.8cbe8e7d.en.html
>    + I respond "It may have been that it's inventing its own API rather
>      than using something like the DMA engine API."
>    + Ajay said: "This issue was discussed recently at TI and proposal was
>      to place it to drivers/dma folder. Moreover, even Felipe also seems
>      to move other musb DMAs (Inventra, CPPI3.0, TUSB) to drivers/dma."

> Oh, and then we come to this interesting email from Felipe to you, in
> response to your reluctance to put it in drivers/dma.

    It was in reposnse to Ajay's email about Felipe's assumed intent to switch 
MUSB to drivers/dma/ API insted of its custom DMA API, AFAIR. To which I, of 
course, started to object. It wasn't about CPPI 4.1 driver at all.

> | Do I really have to spell it out ? Really ?
> |
> | You don't need to physically move the part of the code to drivers/dma,
> | but it has to use the API. The mentor DMA is internal to MUSB.
> | tusb6010_omap.c isn't.
> |
> | Where it makes sense to move the code under drivers/dma, it will be
> | done, where it doesn't, it won't be done, but it will use the same API.
> | That's all.
> |
> | The end goal is just to drop all these ad-hoc "APIs" for accessing DMA
> | on musb code.

    Now I don't even quite understand what (multiple?) ad-hoc APIs Felipe had 
in mind. There\ is one and only one DMA API in MUSB. Perhaps he meant the 
tusb6010_omap.c driver which is using OMAP DMA API?

> See the common theme here?  I don't like it under arch/arm.  I've been
> pretty _consistent_ in suggesting drivers/dma/ all through that...
> Others have said it.  People even acknowledge that's what I've been
> saying, people who were not in the original discussion.

> What I think is this: it is _YOU_ who don't want to hear that message,
> so _YOU_ are intentionally ignoring it, and _YOU_ are looking for
> someone to blame for it.  You've decided I'm to blame because _YOU_
> aren't listening.

    My intent wasn't to lay blame on anybody, just to restore the sequence of 
events which passed completely around me.

> The reason there hasn't been any progress on this is _NOT_ down to me.
> I've provided my feedback, and promptly been ignored.  Others have told
> you the same thing, and promptly been ignored.  Sorry, this is not my
> problem.  This is entirely YOUR problem, one of your own making.

   I didn't really ignore requests to move to drivers/dma/, I just honestly 
didn't see how it can be done, and frankly didnt have much time to massively 
rewrite my stuff written back in 2008 based on earlier TI work. Then my 
project to push OMAP-L137 support simply ended (I can't restore the date now, 
perhaps this was somewhere in 2010), so I simply couldn't continue to invest 
big efforts in this work.

> But whatever.  We are NOT going to put CPPI under arch/arm.  Not now that
> during the last merge window Linus complained to Arnd about the amount of
> code coming through for arch/arm _AGAIN_.  Not after last time Linus
> complained about TI OMAP which prompted the creation of arm-soc.  AND THAT
> IS *FINAL*.  CPPI DMA SUPPORT IS *NOT* GOING UNDER ARCH/ARM.  PERIOD.  NOT
> GOING TO HAPPEN.

> Is this clear enough yet, or how many more years of emails do you need
> yet more emails to get this message through?

    It's crystal clear, thank you. I just thought something might have changed 
seeing how EDMA support moves to arch/arm/common/. Now I see it's not.

WBR, Sergei


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 14:18, Russell King - ARM Linux wrote:

>>>>>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote:
>>>>>>>> good point, do you wanna send some patches ?

>>>>>>>       I have already sent them countless times and even stuck CPPI 4.1 support (in
>>>>>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
>>>>>>> patch. :-(

>>>>>> sticking into arch/arm/common/ wasn't a nice move. But then again, so
>>>>>> wasn't asking for the patch to be removed :-s

>>>>> Err, patches don't get removed, they get moved to 'discarded'.

>>>>      Any chance to bring it back to life? :-)
>>>>      Although... drivers/usb/musb/cppi41.c would need to be somewhat
>>>> reworked for at least AM35x and I don't have time. But that may change,
>>>> of course.

>>> Right, I've just looked back at the various meeting minutes from December
>>> 2010 when the CPPI stuff was discussed.  Yes, I archive these things and
>>> all email discussions for referencing in cases like this.

>>     Thanks.

>>> Unfortunately, they do not contain any useful information other than the
>>> topic having been brought up.  At that point, the CPPI stuff was in
>>> mach-davinci, and I had suggested moving it into drivers/dma.

>>     I don't remember that, probably was out of the loop again.

    I looked back at the history of CPPI 4.1 driver related threads, and found 
that Kevin Hilman gas suggested it too while the driver was in mach-davinci/ 
still...

>>> The result of that was to say that it doesn't fit the DMA engine APIs.

    Right, I tried to fit it (in my thought only though) in and it didn't work 
out.

>>     I remember this as a discussion happening post me sending the patch to
>> the patch system and it being discarded...

    Well, actually before doing this too...

>>> So someone came up with the idea of putting it in arch/arm/common - which

>>     Probably was me.

    No, it was someone from TI.

>> There was also idea of putting it into
>> drivers/usb/musb/ -- which TI indeed followed in its Arago prject. I
>> firmly denied that suggestion.

    Moving it to drivers/usb/ is probably the reason TI has been quite content 
with the situation -- their clients kept receiving MUSB DMA support on both 
OMAP-L1x and then Sitara, so all looked well for them.

>>> I frankly ignored by email (how long have we been saying "no drivers in
>>> arch/arm" ?)

    Well, maybe you should have said it one more time for those who were late 
in the game like me.

>>     But there *are* drivers there! And look at edma.c which is about to be
>> moved there... Anyway, I haven't seen such warnings, probably was too
>> late in the game.

> I've already objected about the header moving to some random place in
> arch/arm/include.  Really, edma.c needs to find another home too - but
> there's a difference here.  edma.c is already present under arch/arm.
> CPPI is _not_.  CPPI is new code appearing under arch/arm (you can see
> that for yourself by looking at the diffstat of 6305/1... it doesn't
> move files, it adds new code.)

    Yes, of course, that's clear.

>>> Now, it would've been discussed in that meeting, but unfortunately no
>>> record exists of that.  What does follow that meeting is a discussion
>>> trail.  From what I can see there, but it looks to me like the decision
>>> was taken to move it to the DMA engine API, and work on sorting out MUSB
>>> was going to commence.

>>> The last email in that says "I'll get to that soon"... and that is also
>>> the final email I have on this topic.  I guess if nothing has happened...
>>> Shrug, that's someone elses problem.

>>     Well, as usual... :-(

>>> Anyway, the answer for putting it in arch/arm/common hasn't changed,
>>> and really, where we are now, post Linus having a moan about the size
>>> of arch/arm, that answer is even more concrete in the negative.  It's
>>> 54K of code which should not be under arch/arm at all.

>>> Anyway, if you need to look at the patch, it's 6305/1.  Typing into the
>>> summary search box 'cppi' found it in one go.

>>     Thanks, I remember this variant was under arch/arm/common/.
>>     Now however, I see what happened to that variant in somewhat different
>> light. Looks like it was entirely your decision to discard the patch,
>> without TI's request...

> Firstly, it is *my* perogative to say no to anything in arch/arm, and I
> really don't have to give reasons for it if I choose to.

    That's clear. You're the ARM King. :-)

> Secondly, it *was* discussed with TI, and the following thread of
> discussion (threaded to the minutes email) shows that *something* was
> going to happen _as a result of that meeting_ to address the problem of
> it being under arch/arm.  And *therefore* it was discarded from the patch
> system - because there was expectation that it was going to get fixed.

> For christ sake, someone even agreed to do it.  Even a target was mentioned,
> of 2.6.39.  That was mentioned on 7th December 2010.  And 6305/1 was
> discarded on 8th December 2010.  Cause and effect.

> And yes, *you* were not part of that discussion.  You work for Montavista
> which contracts with TI to provide this support.

    Here you're not quite correct. TI did not prolongate contgract with MV 
after our releasing the support for OMAP-L137, which is early 2009, AFAIR.

> It is up to TI to pass > stuff like this on to their contractors.

    As you can see, TI didn't feel obliged to do so already.

> There are two people on this thread CC list who were also involved or
> CC'd on the mails from the thread in 2010...  Tony and Felipe.
> Unfortunately, the person who agreed to do the work is no longer in the
> land of the living.  Yes I know it's inconvenient for people to die
> when they've still got lots of important work to do but that's what can
> happen...

    Hm... wasn't it David Brownell? He's the only person who I know has died 
recently who has dealt with DaVinci, MUSB and the releated stuff.

WBR, Sergei


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Sat, Feb 02, 2013 at 08:27:42PM +0400, Sergei Shtylyov wrote:
>> There are two people on this thread CC list who were also involved or
>> CC'd on the mails from the thread in 2010...  Tony and Felipe.
>> Unfortunately, the person who agreed to do the work is no longer in the
>> land of the living.  Yes I know it's inconvenient for people to die
>> when they've still got lots of important work to do but that's what can
>> happen...
>
>    Hm... wasn't it David Brownell? He's the only person who I know has 
> died recently who has dealt with DaVinci, MUSB and the releated stuff.

Actually, it wasn't David who was going to do it - that's where the email
thread gets messy because the mailer David was using makes no distinction
in text format between what bits of text make up the original email being
replied to, and which bits of text are written by David.

It might have been Felipe; there appears to be an email from Felipe saying
that as the immediate parent to David's email.  But that's not really the
point here.  The point is that _someone_ agreed to put the work in, and
_that_ agreement is what caused the patch to be discarded.

And, as I've already explained, you brought up the subject of it being
discarded shortly after, and it got discussed there _again_, and the
same things were said _again_ by at least two people about it being in
drivers/dma.

But anyway, that's all past history.  What was said back then about it
being elsewhere in the tree is just as relevant today as it was back
then.  The only difference is that because that message wasn't received,
it's now two years later with no progress on that.  And that's got
*nothing* what so ever to do with me.

I know people like to blame me just because I'm apparantly the focus of
the blame culture, but really this is getting beyond a joke.

So, I want an apology from you for your insistance that I'm to blame
for this.  Moreover, _I_ want to know what is going to happen in the
future with this so that I don't end up being blamed anymore for the
lack of progress on this issue.

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 20:45, Russell King - ARM Linux wrote:

>>> There are two people on this thread CC list who were also involved or
>>> CC'd on the mails from the thread in 2010...  Tony and Felipe.
>>> Unfortunately, the person who agreed to do the work is no longer in the
>>> land of the living.  Yes I know it's inconvenient for people to die
>>> when they've still got lots of important work to do but that's what can
>>> happen...

>>     Hm... wasn't it David Brownell? He's the only person who I know has
>> died recently who has dealt with DaVinci, MUSB and the releated stuff.

> Actually, it wasn't David who was going to do it - that's where the email
> thread gets messy because the mailer David was using makes no distinction
> in text format between what bits of text make up the original email being
> replied to, and which bits of text are written by David.

    Hm, strange...

> It might have been Felipe; there appears to be an email from Felipe saying
> that as the immediate parent to David's email.  But that's not really the
> point here.  The point is that _someone_ agreed to put the work in, and
> _that_ agreement is what caused the patch to be discarded.

> And, as I've already explained, you brought up the subject of it being
> discarded shortly after, and it got discussed there _again_, and the
> same things were said _again_ by at least two people about it being in
> drivers/dma.

    It wasn't said that somebody concrete was going to work on it. I had to 
explcitly write an email laying all further responsibility on CPPI 4.1 support 
on TI back then.

> But anyway, that's all past history.  What was said back then about it
> being elsewhere in the tree is just as relevant today as it was back
> then.  The only difference is that because that message wasn't received,
> it's now two years later with no progress on that.  And that's got
> *nothing* what so ever to do with me.

    Yes, of course. In my original mail that started the discussion I said 
that we have to wait indefinitely for TI to write the new DMA driver. I just 
wondered wouldn't it be better to use the same approach as for EDMA with 
transitioning to drivers/dma/ step by step.

> I know people like to blame me just because I'm apparantly the focus of
> the blame culture, but really this is getting beyond a joke.

> So, I want an apology from you for your insistance that I'm to blame
> for this.

    OK, I apologise if you consider yourself the target of my blame. My aim 
was rather to establish the truth about that decision taken back in Dec 2010 
-- which we seem to have achieved.

> Moreover, _I_ want to know what is going to happen in the
> future with this so that I don't end up being blamed anymore for the
> lack of progress on this issue.

    Nothing. My blame for the lack of progress has long been laid on TI, after 
I explictly passed the responsibility for the driver to them. My intent with 
the mail that started the discussion was to probe whether we still have 
another opportunity of having MUSB DMA support for OMAP-L1x and Sitara. I just 
thought that you might have changed your mind somehow on the matter.

WBR, Sergei


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Arnd Bergmann]

On Saturday 02 February 2013 04:07:59 Sergei Shtylyov wrote:
> On 02-02-2013 1:30, Russell King - ARM Linux wrote:

> >> because it doesn't make sense to support multiple DMA APIs. We can check
> >> from MUSB's registers if it was configured with Inventra DMA support and
> >> based on that we can register MUSB's own DMA Engine to dmaengine API.
> 
> > Hang on.  This is one of the DMA implementations which is closely
> > coupled with the USB and only the USB?  If it is...
> 
> > I thought this had been discussed _extensively_ before.  I thought the
> > resolution on it was:
> > 1. It would not use the DMA engine API.
> > 2. It would not live in arch/arm.
> > 3. It would be placed nearby the USB driver it's associated with.
> 
> > (1) because we don't use APIs just for the hell of it - think.  Do we
> > use the DMA engine API for PCI bus mastering ethernet controllers?  No.
> > Do we use it for PCI bus mastering SCSI controllers?  No.  Because the
> > DMA is integral to the rest of the device.
> 
> > The DMA engine API only makes sense if the DMA engine is a shared
> > system resource.
> 
>     Thanks for such extensive wording in the support of my point. 

Actually there is another problem with the musb driver, which is that
you can only have one DMA controller configured at build time, and
that currently prevents us from building a kernel that supports both
the Inventra and TUSB variants together. With multiplatform coming
up for both ux500 and omap2, we will also have to make a decision
about ux500 vs omap being supported in DMA mode.

I can see two ways out of this: either we extend the musb driver to
have run-time registration of the DMA controller, which duplicates
infrastructure that already exists in the dmaengine API, or we
make the dmaengine back-end for musb the default and require all
platforms to use that if they want to coexist with other platforms
and also use DMA support in musb.

Note that the ux500_dma code in musb already uses dmaengine. It could
probably be extended to support any platform that has a dmaengine
driver for the DMA controller used in MUSB.

	Arnd

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 1:30, Russell King - ARM Linux wrote:

>> On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote:
>>>> good point, do you wanna send some patches ?

>>>     I have already sent them countless times and even stuck CPPI 4.1 support (in
>>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
>>> patch. :-(

>> sticking into arch/arm/common/ wasn't a nice move. But then again, so
>> wasn't asking for the patch to be removed :-s

> Err, patches don't get removed, they get moved to 'discarded'.

>>>> I guess to make the MUSB side simpler we would need musb-dma-engine glue
>>>> to map dmaengine to the private MUSB API. Then we would have some
>>>> starting point to also move inventra (and anybody else) to dmaengine
>>>> API.

>>>     Why? Inventra is a dedicated device's private DMA controller, why make
>>> universal DMA driver for it?

>> because it doesn't make sense to support multiple DMA APIs. We can check
>> from MUSB's registers if it was configured with Inventra DMA support and
>> based on that we can register MUSB's own DMA Engine to dmaengine API.

> Hang on.  This is one of the DMA implementations which is closely
> coupled with the USB and only the USB?  If it is...

> I thought this had been discussed _extensively_ before.  I thought the
> resolution on it was:
> 1. It would not use the DMA engine API.
> 2. It would not live in arch/arm.
> 3. It would be placed nearby the USB driver it's associated with.

    Note that all this doesn't apply to CPPI 4.1 controller (as contrasted to 
CPPI 3.0 support in MUSB aand EMAC drivers) -- it's shared by design. Just the 
implementations that are in tree have it as MUSB's sub-block, serving only MUSB.

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 2381 bytes --]

Hi,

On Fri, Feb 01, 2013 at 09:30:03PM +0000, Russell King - ARM Linux wrote:
> > > > I guess to make the MUSB side simpler we would need musb-dma-engine glue
> > > > to map dmaengine to the private MUSB API. Then we would have some
> > > > starting point to also move inventra (and anybody else) to dmaengine
> > > > API.
> > > 
> > >    Why? Inventra is a dedicated device's private DMA controller, why make
> > > universal DMA driver for it?
> > 
> > because it doesn't make sense to support multiple DMA APIs. We can check
> > from MUSB's registers if it was configured with Inventra DMA support and
> > based on that we can register MUSB's own DMA Engine to dmaengine API.
> 
> Hang on.  This is one of the DMA implementations which is closely
> coupled with the USB and only the USB?  If it is...
> 
> I thought this had been discussed _extensively_ before.  I thought the
> resolution on it was:
> 1. It would not use the DMA engine API.
> 2. It would not live in arch/arm.
> 3. It would be placed nearby the USB driver it's associated with.
> 
> (1) because we don't use APIs just for the hell of it - think.  Do we
> use the DMA engine API for PCI bus mastering ethernet controllers?  No.
> Do we use it for PCI bus mastering SCSI controllers?  No.  Because the
> DMA is integral to the rest of the device.

that's not really a fair comparison, however. MUSB is used with several
DMA engines.

The only DMA engine which is really coupled with MUSB is the Inventra
DMA engine which comes as an optional feature to the IP. Many users have
opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1,
Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver.

Granted, CPPI 4.1 makes some assumptions about the fact that it's
handling USB tranfers, but nevertheless, the IP can be, and in fact is,
used with many different DMA engines and driver needs to cope with it.

Current DMA abstraction is quite poor, for example there's no way to
compile support for multiple DMA engines. Code also makes certain, IMO
unnecessary, assumptions about the underlying DMA engine (abstraction is
poor, as said above but it we could follow MUSB's programming guide when
it comes to programming DMA transfers).

Considering all of the above, it's far better to use DMA engine and get
rid of all the mess.

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Mon, Feb 04, 2013 at 05:41:53PM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Fri, Feb 01, 2013 at 09:30:03PM +0000, Russell King - ARM Linux wrote:
> > > > > I guess to make the MUSB side simpler we would need musb-dma-engine glue
> > > > > to map dmaengine to the private MUSB API. Then we would have some
> > > > > starting point to also move inventra (and anybody else) to dmaengine
> > > > > API.
> > > > 
> > > >    Why? Inventra is a dedicated device's private DMA controller, why make
> > > > universal DMA driver for it?
> > > 
> > > because it doesn't make sense to support multiple DMA APIs. We can check
> > > from MUSB's registers if it was configured with Inventra DMA support and
> > > based on that we can register MUSB's own DMA Engine to dmaengine API.
> > 
> > Hang on.  This is one of the DMA implementations which is closely
> > coupled with the USB and only the USB?  If it is...
> > 
> > I thought this had been discussed _extensively_ before.  I thought the
> > resolution on it was:
> > 1. It would not use the DMA engine API.
> > 2. It would not live in arch/arm.
> > 3. It would be placed nearby the USB driver it's associated with.
> > 
> > (1) because we don't use APIs just for the hell of it - think.  Do we
> > use the DMA engine API for PCI bus mastering ethernet controllers?  No.
> > Do we use it for PCI bus mastering SCSI controllers?  No.  Because the
> > DMA is integral to the rest of the device.
> 
> that's not really a fair comparison, however. MUSB is used with several
> DMA engines.

I only mentioned it because it _was_ brought up as an argument against
using the DMA engine API in the previous discussions.  I'm just reminding
people what was discussed.

> Considering all of the above, it's far better to use DMA engine and get
> rid of all the mess.

Which is what both you and I have been saying for the last 3 or so years
on this subject...

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02/04/2013 06:41 PM, Felipe Balbi wrote:

>>>>> I guess to make the MUSB side simpler we would need musb-dma-engine glue
>>>>> to map dmaengine to the private MUSB API. Then we would have some
>>>>> starting point to also move inventra (and anybody else) to dmaengine
>>>>> API.

>>>>    Why? Inventra is a dedicated device's private DMA controller, why make
>>>> universal DMA driver for it?

>>> because it doesn't make sense to support multiple DMA APIs. We can check
>>> from MUSB's registers if it was configured with Inventra DMA support and
>>> based on that we can register MUSB's own DMA Engine to dmaengine API.

>> Hang on.  This is one of the DMA implementations which is closely
>> coupled with the USB and only the USB?  If it is...

>> I thought this had been discussed _extensively_ before.  I thought the
>> resolution on it was:
>> 1. It would not use the DMA engine API.
>> 2. It would not live in arch/arm.
>> 3. It would be placed nearby the USB driver it's associated with.

>> (1) because we don't use APIs just for the hell of it - think.  Do we
>> use the DMA engine API for PCI bus mastering ethernet controllers?  No.
>> Do we use it for PCI bus mastering SCSI controllers?  No.  Because the
>> DMA is integral to the rest of the device.

> that's not really a fair comparison, however. MUSB is used with several
> DMA engines.

> The only DMA engine which is really coupled with MUSB is the Inventra
> DMA engine which comes as an optional feature to the IP. Many users have

   That's not quite true. At least CPPI 3.0 is coupled with MUSB as well. The
programming interface is MUSB specific, and differs from that of the EMAC driver
which also uses CPPI 3.0 (however, the DMA descriptor format is the same between
both, IIRC).

> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1,
> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver.

> Granted, CPPI 4.1 makes some assumptions about the fact that it's
> handling USB tranfers,

   What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just
natural. Generic CPPI 4.1 support code (as was posted for both mach-dacinci/ or
common/ placement) made no such assumptions.

> but nevertheless, the IP can be, and in fact is,
> used with many different DMA engines and driver needs to cope with it.

   What IP, CPPI 4.1 or MUSB?

> Current DMA abstraction is quite poor, for example there's no way to
> compile support for multiple DMA engines. Code also makes certain, IMO
> unnecessary, assumptions about the underlying DMA engine (abstraction is
> poor, as said above but it we could follow MUSB's programming guide when
> it comes to programming DMA transfers).

   Don't know, I was quite content with the abstraction when writing CPPI 4.1
driver for MUSB...

> Considering all of the above, it's far better to use DMA engine and get
> rid of all the mess.

   In my eyes, getting rid of the mess doesn't justify breaking the rules that
Russell formulated above.

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]

Hi,

On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote:
> > opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1,
> > Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver.
> 
> > Granted, CPPI 4.1 makes some assumptions about the fact that it's
> > handling USB tranfers,
> 
>    What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just

HW makes the asumptions

> > but nevertheless, the IP can be, and in fact is,
> > used with many different DMA engines and driver needs to cope with it.
> 
>    What IP, CPPI 4.1 or MUSB?

MUSB

> > Current DMA abstraction is quite poor, for example there's no way to
> > compile support for multiple DMA engines. Code also makes certain, IMO
> > unnecessary, assumptions about the underlying DMA engine (abstraction is
> > poor, as said above but it we could follow MUSB's programming guide when
> > it comes to programming DMA transfers).
> 
>    Don't know, I was quite content with the abstraction when writing CPPI 4.1
> driver for MUSB...

look closer. The whole:

if (is_cppi())
	foo();
else if (is_inventra())
	bar();
else if (is_omap_sdma())
	baz();

is bogus.

> > Considering all of the above, it's far better to use DMA engine and get
> > rid of all the mess.
> 
>    In my eyes, getting rid of the mess doesn't justify breaking the rules that
> Russell formulated above.

MUSB is no PCI, there is no single, standard interface to the DMA
engine (unlike Synopsys' dwc-usb3 and dwc-usb2, where we don't use DMA
engine), every DMA engine comes with its own set of registers, its own
programming model and so forth.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Mon, Feb 04, 2013 at 06:47:12PM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote:
> >    In my eyes, getting rid of the mess doesn't justify breaking the rules that
> > Russell formulated above.
> 
> MUSB is no PCI, there is no single, standard interface to the DMA
> engine (unlike Synopsys' dwc-usb3 and dwc-usb2, where we don't use DMA
> engine), every DMA engine comes with its own set of registers, its own
> programming model and so forth.

Err, before you get too wrapped up in that argument... if you think there's
anything like a common hardware interface for DMA on PCI, you'd be wrong.
There isn't.

What there is is a bus protocol for it.  How the device decides to perform
DMA is up to the device; there's no standard register definitions across
all devices.  Yes, some classes have a standard register set (like USB,
and ATA) but others are totally device specific (eg, network chips).

However, on PCI, the DMA support is always integrated with the rest of
the device itself.  That is not the case here.

So, bringing PCI up into this discussion is totally irrelevant.  As I've
already explained, this was brought up in the _previous_ discussion as
a reason why _not_ to use the DMA engine API for this.

So, can we please leave PCI totally out of this?  It didn't belong here
2-3 years ago, and it sure enough doesn't belong here now.  It's nothing
but pure distraction.  And the more this goes on, the more I can see,
yet again, the CPPI 4.1 going nowhere at all.

As I can see it, there's nothing more to talk about.  The issue has been
extensively talked to death.  What it needs now is not _more_ talk, it
needs someone to actually go and _do_ something useful with it.

I realise that may be difficult given the mess that TI must still be in
internally since the upheaval of OMAP, but it sounds like there's a
different group needing this stuff to work...

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02/04/2013 07:47 PM, Felipe Balbi wrote:

> On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote:
>>> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1,
>>> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver.

>>> Granted, CPPI 4.1 makes some assumptions about the fact that it's
>>> handling USB tranfers,

>>    What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just

> HW makes the asumptions

   Not true at all. There is a separate set of registers (at offset 0) which
copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything.
It's just the way the driver was written that it used both sets of registers but
this needs to be changed into more abstacted accesses to the USB-specific part,
to cope with it being different on different platfroms, like AM35x. The driver
as it was last posted, just needs rework now.

>>> but nevertheless, the IP can be, and in fact is,
>>> used with many different DMA engines and driver needs to cope with it.

>>    What IP, CPPI 4.1 or MUSB?

> MUSB

>>> Current DMA abstraction is quite poor, for example there's no way to
>>> compile support for multiple DMA engines. Code also makes certain, IMO
>>> unnecessary, assumptions about the underlying DMA engine (abstraction is
>>> poor, as said above but it we could follow MUSB's programming guide when
>>> it comes to programming DMA transfers).

>>    Don't know, I was quite content with the abstraction when writing CPPI 4.1
>> driver for MUSB...

> look closer. The whole:

> if (is_cppi())
> 	foo();
> else if (is_inventra())
> 	bar();
> else if (is_omap_sdma())
> 	baz();

> is bogus.

   That part -- yes. There were attempt to get rid of this, but without changing
the DMA API. It was left halfway done after my only critical comment, IIRC. Will
we ever see the continuation of this effort?

>>> Considering all of the above, it's far better to use DMA engine and get
>>> rid of all the mess.

>>    In my eyes, getting rid of the mess doesn't justify breaking the rules that
>> Russell formulated above.

> MUSB is no PCI, there is no single, standard interface to the DMA
> engine (unlike Synopsys' dwc-usb3 and dwc-usb2, where we don't use DMA
> engine), every DMA engine comes with its own set of registers, its own
> programming model and so forth.

   Same can be said about PCI where each bus master has its own programming i/f
-- so I didn't really dig this example.

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 1614 bytes --]

Hi,

On Mon, Feb 04, 2013 at 08:54:17PM +0300, Sergei Shtylyov wrote:
> > On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote:
> >>> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1,
> >>> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver.
> 
> >>> Granted, CPPI 4.1 makes some assumptions about the fact that it's
> >>> handling USB tranfers,
> 
> >>    What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just
> 
> > HW makes the asumptions
> 
>    Not true at all. There is a separate set of registers (at offset 0) which
> copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything.

CPPI 4.1 was made for USB transfers.

> It's just the way the driver was written that it used both sets of registers but
> this needs to be changed into more abstacted accesses to the USB-specific part,
> to cope with it being different on different platfroms, like AM35x. The driver
> as it was last posted, just needs rework now.

are you saying you will do the work ?

> >>    Don't know, I was quite content with the abstraction when writing CPPI 4.1
> >> driver for MUSB...
> 
> > look closer. The whole:
> 
> > if (is_cppi())
> > 	foo();
> > else if (is_inventra())
> > 	bar();
> > else if (is_omap_sdma())
> > 	baz();
> 
> > is bogus.
> 
>    That part -- yes. There were attempt to get rid of this, but without changing
> the DMA API. It was left halfway done after my only critical comment, IIRC. Will
> we ever see the continuation of this effort?

patches are welcome

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02/04/2013 08:02 PM, Felipe Balbi wrote:

>>> On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote:
>>>>> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1,
>>>>> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver.

>>>>> Granted, CPPI 4.1 makes some assumptions about the fact that it's
>>>>> handling USB tranfers,

>>>>    What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just

>>> HW makes the asumptions

>>    Not true at all. There is a separate set of registers (at offset 0) which
>> copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything.

> CPPI 4.1 was made for USB transfers.

   Utter nonsense, CPPI 4.1 is hardware abstract DMA engine. It's used for
Ethernet transfers on out-of-tree platforms like mach-avalanche/.

>> It's just the way the driver was written that it used both sets of registers but
>> this needs to be changed into more abstacted accesses to the USB-specific part,
>> to cope with it being different on different platfroms, like AM35x. The driver
>> as it was last posted, just needs rework now.

> are you saying you will do the work ?

   My efforts stopped to be funded by MV back in 2010. Hell, I'm not even
working in MV as I used to, hanging on the verge of my current contract being
terminated.

>>>>    Don't know, I was quite content with the abstraction when writing CPPI 4.1
>>>> driver for MUSB...

>>> look closer. The whole:

>>> if (is_cppi())
>>> 	foo();
>>> else if (is_inventra())
>>> 	bar();
>>> else if (is_omap_sdma())
>>> 	baz();

>>> is bogus.

>>    That part -- yes. There were attempt to get rid of this, but without changing
>> the DMA API. It was left halfway done after my only critical comment, IIRC. Will
>> we ever see the continuation of this effort?

> patches are welcome

   There was a patch, it only needed comments addressed. I think the author was
Heikki Krogerus. As for me, my time is very limited, so be thankful even for
those scarce patches I'm sending out now -- I'm doing them in my copious free time.

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Cyril Chemparathy]

On 02/04/2013 12:02 PM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Feb 04, 2013 at 08:54:17PM +0300, Sergei Shtylyov wrote:
>>> On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote:
>>>>> opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1,
>>>>> Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver.
>>
>>>>> Granted, CPPI 4.1 makes some assumptions about the fact that it's
>>>>> handling USB tranfers,
>>
>>>>     What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just
>>
>>> HW makes the asumptions
>>
>>     Not true at all. There is a separate set of registers (at offset 0) which
>> copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything.
>
> CPPI 4.1 was made for USB transfers.
>

I have been dealing with CPPI hardware on KeyStone platforms (CPPI 4.2). 
  Our experiences with this DMA hardware may help with CPPI 4.1 on 
earlier generations.

CPPI 4.2 serves as a truly common interface to a number of hardware 
blocks on KeyStone SoCs - including Ethernet, RapidIO, Crypto 
accelerators, and a bunch of other accelerator thingies.  Given the 
commonality across subsystems, we've built a shared CPPI 4.2 DMA-Engine 
implementation.  You can take a sneak peek at this implementation at [1].

Based on our experience with fitting multiple subsystems on top of this 
DMA-Engine driver, I must say that the DMA-Engine interface has proven 
to be a less than ideal fit for the network driver use case.

The first problem is that the DMA-Engine interface expects to "push" 
completed traffic up into the upper layer as a part of its callback. 
This doesn't fit cleanly with NAPI, which expects to "pull" completed 
traffic from below in the NAPI poll.  We've somehow kludged together a 
solution around this, but it isn't very elegant.

The second problem is one of binding fixed DMA resources to fixed users. 
  AFAICT, the stock DMA-Engine mechanism works best when one DMA 
resource is as good as any other.  To get over this problem, we've added 
support for named channels, and drivers specifically request for a DMA 
resource by name.  Again, this is less than ideal.

We found that virtio devices offer a more elegant solution to this 
problem.  First, the virtqueue interface is a much better fit into NAPI 
(callback --> napi schedule, napi poll --> get_buf), and this eliminates 
the need for aforementioned kludges in the code.  Second, the virtio 
device infrastructure nicely uses the device model to solve the problem 
of binding DMA users to specific DMA resources.

These patches haven't (yet) been posted on the MLs, but you can peek at 
[2].  While we are on the topic, I'd certainly appreciate feedback on 
the concept of using virtqueues as an interface to peripheral 
independent packet oriented DMA hardware. :-)

Cheers
-- Cyril

[1] - 
http://arago-project.org/git/projects/?p=linux-keystone.git;a=shortlog;h=refs/heads/rebuild/23-drivers-dmaengine
[2] - 
http://arago-project.org/git/projects/?p=linux-keystone.git;a=shortlog;h=refs/heads/rebuild/21-drivers-virtio

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Linus Walleij]

On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril@ti.com> wrote:

> Based on our experience with fitting multiple subsystems on top of this
> DMA-Engine driver, I must say that the DMA-Engine interface has proven
> to be a less than ideal fit for the network driver use case.
>
> The first problem is that the DMA-Engine interface expects to "push"
> completed traffic up into the upper layer as a part of its callback.
> This doesn't fit cleanly with NAPI, which expects to "pull" completed
> traffic from below in the NAPI poll.  We've somehow kludged together a
> solution around this, but it isn't very elegant.

I cannot understand the actual technical problem from the above
paragraphs though. dmaengine doesn't have a concept of pushing
nor polling, it basically copies streams of words from A to B, where
A/B can be a device or a buffer, nothing else.

The thing you're looking for sounds more like an adapter on top
of dmaengine, which can surely be constructed, some
drivers/dma/dmaengine-napi.c or whatever.

> The second problem is one of binding fixed DMA resources to fixed users.
>   AFAICT, the stock DMA-Engine mechanism works best when one DMA
> resource is as good as any other.

The filter function picks a channel for whatever reason. That reason
can be, well whatever. Some engines have a clever mechanism to
select resources on the other end.

Then for tying devices to channels we have the dmaengine
DT branch:
http://git.infradead.org/users/vkoul/slave-dma.git/shortlog/refs/heads/topic/dmaengine_dt

This stuff didn't go into v3.8 but you can *sure* expect it
to be in v3.9.

Or are you referring to a multi-engine scenario? Say if there is engine
A and B and depending on circumstances A or B may be preferred
in some order (and permutations of this problem). That is currently
identified as a shortcoming that we need help to address.

> To get over this problem, we've added
> support for named channels, and drivers specifically request for a DMA
> resource by name.  Again, this is less than ideal.

Jon Hunter has been working on a mechanism to look up DMA channels
from struct device *, dev_name() or a device tree node for example.
Just like we do with clocks or regulators.

Look at this patch from the dmaengine_dt branch:
http://git.infradead.org/users/vkoul/slave-dma.git/commitdiff/528499a7037ebec0636d928f88cd783c618df3c5

Looks up an optionally named channel for a certain
device.

It currently only supports device tree, but you are free to
patch in whatever mechanism you need there. Static tables
in platform data works too. Just nobody did it.

So go ahead and hack on dma_request_slave_channel().
(I would just branch of the DT branch.)

> We found that virtio devices offer a more elegant solution to this
> problem.  First, the virtqueue interface is a much better fit into NAPI
> (callback --> napi schedule, napi poll --> get_buf), and this eliminates
> the need for aforementioned kludges in the code.  Second, the virtio
> device infrastructure nicely uses the device model to solve the problem
> of binding DMA users to specific DMA resources.

Not that I understand the polling issue, but it sounds to me like
what Jon is doing is similar.

Surely the way to look up resources cannot be paramount in this
discussion, I think the real problem must be your specific networking
usecase, so we need to drill into that.

Yours,
Linus Walleij

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]

On Mon, Feb 04, 2013 at 09:29:46PM +0100, Linus Walleij wrote:
> On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril@ti.com> wrote:

> > Based on our experience with fitting multiple subsystems on top of this
> > DMA-Engine driver, I must say that the DMA-Engine interface has proven
> > to be a less than ideal fit for the network driver use case.

> > The first problem is that the DMA-Engine interface expects to "push"
> > completed traffic up into the upper layer as a part of its callback.
> > This doesn't fit cleanly with NAPI, which expects to "pull" completed
> > traffic from below in the NAPI poll.  We've somehow kludged together a
> > solution around this, but it isn't very elegant.

> I cannot understand the actual technical problem from the above
> paragraphs though. dmaengine doesn't have a concept of pushing
> nor polling, it basically copies streams of words from A to B, where
> A/B can be a device or a buffer, nothing else.

> The thing you're looking for sounds more like an adapter on top
> of dmaengine, which can surely be constructed, some
> drivers/dma/dmaengine-napi.c or whatever.

Broadly speaking what NAPI wants is to never get any callbacks from the
hardware (or DMAs).  It wants to wake up periodically, take a look at
what packets have been read by the hardware and process them.  The goal
is to have the DMAs sitting and running without disturbing the processor
at all after the first packet has been handled.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Linus Walleij]

On Mon, Feb 4, 2013 at 9:33 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Feb 04, 2013 at 09:29:46PM +0100, Linus Walleij wrote:
>> On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril@ti.com> wrote:
>
>> > Based on our experience with fitting multiple subsystems on top of this
>> > DMA-Engine driver, I must say that the DMA-Engine interface has proven
>> > to be a less than ideal fit for the network driver use case.
>
>> > The first problem is that the DMA-Engine interface expects to "push"
>> > completed traffic up into the upper layer as a part of its callback.
>> > This doesn't fit cleanly with NAPI, which expects to "pull" completed
>> > traffic from below in the NAPI poll.  We've somehow kludged together a
>> > solution around this, but it isn't very elegant.
>
>> I cannot understand the actual technical problem from the above
>> paragraphs though. dmaengine doesn't have a concept of pushing
>> nor polling, it basically copies streams of words from A to B, where
>> A/B can be a device or a buffer, nothing else.
>
>> The thing you're looking for sounds more like an adapter on top
>> of dmaengine, which can surely be constructed, some
>> drivers/dma/dmaengine-napi.c or whatever.
>
> Broadly speaking what NAPI wants is to never get any callbacks from the
> hardware (or DMAs).  It wants to wake up periodically, take a look at
> what packets have been read by the hardware and process them.  The goal
> is to have the DMAs sitting and running without disturbing the processor
> at all after the first packet has been handled.

OK we should definately be able to encompass that in dmaengine
quite easily.

So I think the above concerns are moot. The callback we can
set on cookies is entirely optional, and it's even implemented by
each DMA engine, and some may not even support it but *require*
polling, and then it won't even be implemented by the driver.

Which probably stems from the original design of the dmaengine
API, which was for TCP networking acceleration, mainly.

Cyril, just stack up the cookies and take a sweep over them to see
which ones are baked when the NAPI poll comes in -> problem
solved.

Yours,
Linus Walleij

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Arnd Bergmann]

On Monday 04 February 2013, Linus Walleij wrote:
> So I think the above concerns are moot. The callback we can
> set on cookies is entirely optional, and it's even implemented by
> each DMA engine, and some may not even support it but require
> polling, and then it won't even be implemented by the driver.

Just to ensure that everybody is talking about the same thing here:
Is it just the callback that is optional, or also the interrupt
coming from the hardware? With NAPI, you want to avoid both, since
getting an interrupt for every packet adds noticeable overhead,
but you still want to be able to tell the hardware that you
are fed up with polling and would like to get an interrupt again
when the next data arrives (ideally, after either a little time
has passed after the next packet, or a specific number of packets
has arrived).

	Arnd

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Mon, Feb 04, 2013 at 09:47:38PM +0000, Arnd Bergmann wrote:
> On Monday 04 February 2013, Linus Walleij wrote:
> > So I think the above concerns are moot. The callback we can
> > set on cookies is entirely optional, and it's even implemented by
> > each DMA engine, and some may not even support it but require
> > polling, and then it won't even be implemented by the driver.
> 
> Just to ensure that everybody is talking about the same thing here:
> Is it just the callback that is optional, or also the interrupt
> coming from the hardware?

If everyone implements stuff correctly, both.  The callback most certainly
is optional as things stand.  The interrupt - that depends on the DMA
engine.

Some DMA engines you can't avoid it because you need to reprogram the
hardware with the next+1 transfer upon completion of an existing transfer.
Others may allow you to chain transfers in hardware.  That's all up to
how the DMA engine driver is implemented and how the hardware behaves.

Now, there's another problem here: that is, people abuse the API.  People
don't pass DMA_CTRL_ACK | DMA_PREP_INTERRUPT into their operations by
default.  People like typing '0'.

The intention of the "DMA_PREP_INTERRUPT" is significant here: it means
"ask the hardware to send an interrupt upon completion of this transfer".

Because soo many people like to type '0' instead in their DMA engine
clients, it means that this flag is utterly useless today - you have to
ignore it.  So there's _no_ way for client drivers to actually tell the
a DMA engine driver which _doesn't_ need to signal interrupts at the end
of every transfer not to do so.

So yes, the DMA engine API supports it.  Whether the _implementations_
themselves do is very much hit and miss (and in reality is much more
miss than hit.)

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Cyril Chemparathy]

On 02/05/2013 07:38 AM, Russell King - ARM Linux wrote:
> On Mon, Feb 04, 2013 at 09:47:38PM +0000, Arnd Bergmann wrote:
>> On Monday 04 February 2013, Linus Walleij wrote:
>>> So I think the above concerns are moot. The callback we can
>>> set on cookies is entirely optional, and it's even implemented by
>>> each DMA engine, and some may not even support it but require
>>> polling, and then it won't even be implemented by the driver.
>>
>> Just to ensure that everybody is talking about the same thing here:
>> Is it just the callback that is optional, or also the interrupt
>> coming from the hardware?
>
> If everyone implements stuff correctly, both.  The callback most certainly
> is optional as things stand.  The interrupt - that depends on the DMA
> engine.
>
> Some DMA engines you can't avoid it because you need to reprogram the
> hardware with the next+1 transfer upon completion of an existing transfer.
> Others may allow you to chain transfers in hardware.  That's all up to
> how the DMA engine driver is implemented and how the hardware behaves.
>
> Now, there's another problem here: that is, people abuse the API.  People
> don't pass DMA_CTRL_ACK | DMA_PREP_INTERRUPT into their operations by
> default.  People like typing '0'.
>
> The intention of the "DMA_PREP_INTERRUPT" is significant here: it means
> "ask the hardware to send an interrupt upon completion of this transfer".
>
> Because soo many people like to type '0' instead in their DMA engine
> clients, it means that this flag is utterly useless today - you have to
> ignore it.  So there's _no_ way for client drivers to actually tell the
> a DMA engine driver which _doesn't_ need to signal interrupts at the end
> of every transfer not to do so.
>
> So yes, the DMA engine API supports it.  Whether the _implementations_
> themselves do is very much hit and miss (and in reality is much more
> miss than hit.)
>

Don't these assume that the driver can determine the need for an 
interrupt upfront at prep/submit time?  AFAICT, this assumption doesn't 
hold true with NAPI.

Thanks
-- Cyril.

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Cyril Chemparathy]

On 02/04/2013 04:11 PM, Linus Walleij wrote:
> On Mon, Feb 4, 2013 at 9:33 PM, Mark Brown
> <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> wrote:
>> On Mon, Feb 04, 2013 at 09:29:46PM +0100, Linus Walleij wrote:
>>> On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril-l0cyMroinI0@public.gmane.org> wrote:
>>
>>>> Based on our experience with fitting multiple subsystems on top of this
>>>> DMA-Engine driver, I must say that the DMA-Engine interface has proven
>>>> to be a less than ideal fit for the network driver use case.
>>
>>>> The first problem is that the DMA-Engine interface expects to "push"
>>>> completed traffic up into the upper layer as a part of its callback.
>>>> This doesn't fit cleanly with NAPI, which expects to "pull" completed
>>>> traffic from below in the NAPI poll.  We've somehow kludged together a
>>>> solution around this, but it isn't very elegant.
>>
>>> I cannot understand the actual technical problem from the above
>>> paragraphs though. dmaengine doesn't have a concept of pushing
>>> nor polling, it basically copies streams of words from A to B, where
>>> A/B can be a device or a buffer, nothing else.
>>
>>> The thing you're looking for sounds more like an adapter on top
>>> of dmaengine, which can surely be constructed, some
>>> drivers/dma/dmaengine-napi.c or whatever.
>>
>> Broadly speaking what NAPI wants is to never get any callbacks from the
>> hardware (or DMAs).  It wants to wake up periodically, take a look at
>> what packets have been read by the hardware and process them.  The goal
>> is to have the DMAs sitting and running without disturbing the processor
>> at all after the first packet has been handled.
>
> OK we should definately be able to encompass that in dmaengine
> quite easily.
>
> So I think the above concerns are moot. The callback we can
> set on cookies is entirely optional, and it's even implemented by
> each DMA engine, and some may not even support it but *require*
> polling, and then it won't even be implemented by the driver.
>
> Which probably stems from the original design of the dmaengine
> API, which was for TCP networking acceleration, mainly.
>
> Cyril, just stack up the cookies and take a sweep over them to see
> which ones are baked when the NAPI poll comes in -> problem
> solved.
>

You're assuming that cookies complete in order.  That is not necessarily 
true.

Thanks
-- Cyril.

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Mon, Feb 04, 2013 at 04:54:45PM -0500, Cyril Chemparathy wrote:
> You're assuming that cookies complete in order.  That is not necessarily  
> true.

Under what circumstances is that not true?

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Cyril Chemparathy]

On 02/05/2013 07:41 AM, Russell King - ARM Linux wrote:
> On Mon, Feb 04, 2013 at 04:54:45PM -0500, Cyril Chemparathy wrote:
>> You're assuming that cookies complete in order.  That is not necessarily
>> true.
>
> Under what circumstances is that not true?
>

Notably when hardware can prioritize certain classes of traffic over 
others, for instance with data center bridging mechanisms.

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Linus Walleij]

On Mon, Feb 4, 2013 at 10:54 PM, Cyril Chemparathy <cyril@ti.com> wrote:
> On 02/04/2013 04:11 PM, Linus Walleij wrote:

>> Cyril, just stack up the cookies and take a sweep over them to see
>> which ones are baked when the NAPI poll comes in -> problem
>> solved.
>
> You're assuming that cookies complete in order.  That is not necessarily
> true.

So put them on a wait list? Surely you will have a list of pending
cookies and pick from the front of the queue if there isn't a hole on
queue position 0.

Yours,
Linus Walleij

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Tue, Feb 05, 2013 at 04:30:45PM +0100, Linus Walleij wrote:
> On Mon, Feb 4, 2013 at 10:54 PM, Cyril Chemparathy <cyril@ti.com> wrote:
> > On 02/04/2013 04:11 PM, Linus Walleij wrote:
> 
> >> Cyril, just stack up the cookies and take a sweep over them to see
> >> which ones are baked when the NAPI poll comes in -> problem
> >> solved.
> >
> > You're assuming that cookies complete in order.  That is not necessarily
> > true.
> 
> So put them on a wait list? Surely you will have a list of pending
> cookies and pick from the front of the queue if there isn't a hole on
> queue position 0.

Not quite.  The key is the cookie system DMA engine employs to indicate
when a cookie is complete.

Cookies between the "issued sequence" and "completed sequence" are defined
to be in progress, everything else is defined to be completed.

This means that if "completed sequence" is 1, and "issued sequence" is 5,
then cookies with values 2, 3, 4, 5 are in progress.  You can't mark
sequence 4 as being complete until 2 and 3 have completed.

If we need out-of-order completion, then that's a problem for the DMA
engine API, because you'd need to radically change the way "completion"
is marked.

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Linus Walleij]

On Tue, Feb 5, 2013 at 6:14 PM, Russell King - ARM Linux
<linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> On Tue, Feb 05, 2013 at 04:30:45PM +0100, Linus Walleij wrote:

>> So put them on a wait list? Surely you will have a list of pending
>> cookies and pick from the front of the queue if there isn't a hole on
>> queue position 0.
>
> Not quite.  The key is the cookie system DMA engine employs to indicate
> when a cookie is complete.
>
> Cookies between the "issued sequence" and "completed sequence" are defined
> to be in progress, everything else is defined to be completed.
>
> This means that if "completed sequence" is 1, and "issued sequence" is 5,
> then cookies with values 2, 3, 4, 5 are in progress.  You can't mark
> sequence 4 as being complete until 2 and 3 have completed.

Yes that is true. DMA transfers on a certain channel are defined
as progressing linearly per-cookie. I wonder if that is a problem
in this case though (actually it seems the reverse, this helps
in Cyril's case.)

> If we need out-of-order completion, then that's a problem for the DMA
> engine API, because you'd need to radically change the way "completion"
> is marked.

True. I wonder if this usecase is ever going to be applicable
however. It could maybe be useful in some instances of
memcpy() I could dream up, whereas for device transfers it
seems unlikely to me.

Yours,
Linus Walleij

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Cyril Chemparathy]

On 02/04/2013 03:29 PM, Linus Walleij wrote:
> On Mon, Feb 4, 2013 at 8:22 PM, Cyril Chemparathy <cyril-l0cyMroinI0@public.gmane.org> wrote:
>
>> Based on our experience with fitting multiple subsystems on top of this
>> DMA-Engine driver, I must say that the DMA-Engine interface has proven
>> to be a less than ideal fit for the network driver use case.
>>
>> The first problem is that the DMA-Engine interface expects to "push"
>> completed traffic up into the upper layer as a part of its callback.
>> This doesn't fit cleanly with NAPI, which expects to "pull" completed
>> traffic from below in the NAPI poll.  We've somehow kludged together a
>> solution around this, but it isn't very elegant.
>
> I cannot understand the actual technical problem from the above
> paragraphs though. dmaengine doesn't have a concept of pushing
> nor polling, it basically copies streams of words from A to B, where
> A/B can be a device or a buffer, nothing else.
>

NAPI needs to switch between polled and interrupt driven modes of 
operation.  Further, in a given poll, it needs to be able to limit the 
amount of traffic processed to a specified budget.

> The thing you're looking for sounds more like an adapter on top
> of dmaengine, which can surely be constructed, some
> drivers/dma/dmaengine-napi.c or whatever.
>

I'm not debating the possibility of duct-taping a network driver on top 
of the dma-engine interface.  That is very much doable, and we have done 
this already.

Starting with a stock dma-engine driver, our first approach was to use 
dmaengine_pause() and dmaengine_resume() in the network driver to 
throttle completion callbacks per NAPI's needs.  This worked, but it was 
ugly because the completion callback was now being used in a 
multi-purpose fashion - (a) as an interrupt notifier [to do a 
napi_schedule()], and (b) as a hand over mechanism for completed packets 
[within a napi_poll()].  The network driver needed to maintain a nasty 
little state machine for this, and this ended up being quite non-trivial 
after we'd fixed up most of the issues.

Having learned our lessons from the first attempt, the second step was 
to add a separate notification callback from the dma-engine layer - a 
notification independent of any particular descriptor.  With this 
callback in place, we got rid of some of the state machine crap in the 
network driver.

The third step was to add a dmaengine_poll() call instead of constantly 
bouncing a channel between paused and running states.  This further 
cleaned up some of the network driver code, but now the dma-engine 
driver looks like crap if it needs to support both the traditional and 
new (i.e. notify + poll) modes.  This is where we are at today.

Even with the addition of these extensions, the interaction between the 
network driver and the dma-engine driver is clumsy and involves multiple 
back and forth calls per packet.  This is not elegant, and certainly not 
efficient.  In comparison, the virtqueue interface is a natural fit with 
the network driver, and is free of the aforementioned problems.

[...]
> Surely the way to look up resources cannot be paramount in this
> discussion, I think the real problem must be your specific networking
> usecase, so we need to drill into that.
>

Agreed.  The dma resource to driver binding is certainly a lesser 
problem than the first.  There are a variety of schemes that we could 
cook up here (filter functions, named lookups, etc.).  However, I think 
that these schemes offer advantages when the binding between the dma 
resource and the dma user is somewhat dynamic.

On the other hand, when the binding between a dma resource and user is 
fixed by hardware and/or configuration, the driver model approach works 
better, IMHO.

Thanks
-- Cyril.

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Linus Walleij]

On Mon, Feb 4, 2013 at 11:30 PM, Cyril Chemparathy <cyril-l0cyMroinI0@public.gmane.org> wrote:

> NAPI needs to switch between polled and interrupt driven modes of operation.
> Further, in a given poll, it needs to be able to limit the amount of traffic
> processed to a specified budget.

I don't think any of this is a problem.

Polling, just scan cookies. Disable or ignore the callbacks.

For IRQ mode, use the completion callback to push each cookie
to NAPI, and thus let the IRQ drive the traffic.

>> The thing you're looking for sounds more like an adapter on top
>> of dmaengine, which can surely be constructed, some
>> drivers/dma/dmaengine-napi.c or whatever.
>
> I'm not debating the possibility of duct-taping a network driver on top of
> the dma-engine interface.  That is very much doable, and we have done this
> already.

So it seems I have a different opinion on elegance.

I think it can be a nice little adapter, and you're talking about
duct-tape, like it's something ugly and crude.

So let's get to the bottom of that.

> Starting with a stock dma-engine driver, our first approach was to use
> dmaengine_pause() and dmaengine_resume() in the network driver to throttle
> completion callbacks per NAPI's needs.

Why? Do you really need to limit this in the middle of transfers?

I'm half-guessing that one transfer is typically something like one
packet. Pausing and starting would be something you would use
in case you had a circular buffer with an eternal ongoing transfer,
and you wanted to slow that down.

> Having learned our lessons from the first attempt, the second step was to
> add a separate notification callback from the dma-engine layer - a
> notification independent of any particular descriptor.  With this callback
> in place, we got rid of some of the state machine crap in the network
> driver.
>
> The third step was to add a dmaengine_poll() call instead of constantly
> bouncing a channel between paused and running states.  This further cleaned
> up some of the network driver code, but now the dma-engine driver looks like
> crap if it needs to support both the traditional and new (i.e. notify +
> poll) modes.  This is where we are at today.

I don't see why you have to modify dmaengine to do poll.
It should be trivial as discussed to just keep track of the cookies,
scan over them and poll out the completed work.

Then to mitigate the load I guess you just do not issue
too many dma transfers? Can you not regulate the workload
by adjusting the number of transfer cookies you issue in
parallel, and if they're not issued in parallel but adjacent,
can you not just issue them so often?

Or are you polling out half transfers or something, so
that the granularity of one transfer/cookie would be
enough?

Maybe I'm not getting the picture here?

Can you describe how the network stream is chopped into
transfers in this hardware, I think that could ease our
understanding?

Especially I guess we need to know if the hardware is providing
useful chunks like one packet or if it's just one eternal stream of
bits that you then have to demux and put into skbufs or
something.

> Even with the addition of these extensions, the interaction between the
> network driver and the dma-engine driver is clumsy and involves multiple
> back and forth calls per packet.  This is not elegant, and certainly not
> efficient.  In comparison, the virtqueue interface is a natural fit with the
> network driver, and is free of the aforementioned problems.

Yes the described approach and hacking around in dmaengine to
do the polling seems ugly. But just a queue of cookies does not
seem so ugly, rather the opposite.

> [Russell writes]
>>  So yes, the DMA engine API supports it.  Whether the _implementations_
>>  themselves do is very much hit and miss (and in reality is much more
>> miss than hit.)
>
> Don't these assume that the driver can determine the need for an interrupt
> upfront at prep/submit time?  AFAICT, this assumption doesn't hold true
> with NAPI.

Yes it does. You can however stop an ongoing transfer
(by referring to the cookie), pick out the bytes transferred so far
and trigger a new transfer using/not using an IRQ if you want.

This is an abstracted way of doing the same brutal buffer
slaying that I hear NAPI is doing to some network drivers.

For example see the RX path of this driver:
drivers/tty/serial/amba-pl011.c

There is DMA for it, but we may stop the DMA transfer on
an IRQ, take the partial buffer out and feed it to the TTY.
Could just as well be a network packet. Sometimes it is, if
there is a modem on the other end.

RX DMA is triggered in pl011_dma_rx_trigger_dma(),
then either pl011_dma_rx_callback() gets called if the DMA
transfer completes, or we get an IRQ (like a timeout) and
endup in pl011_dma_rx_irq(), where the transfer is stopped,
buffer emtied and then we can decide what to do next.

This could just as well have been some API calling in and
saying "give me your buffer NOW".

I think we need to look at an NAPI driver that does this trick
so we understand what you want from the API.

Yours,
Linus Walleij

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote:

> For IRQ mode, use the completion callback to push each cookie
> to NAPI, and thus let the IRQ drive the traffic.

The whole purpose of NAPI is to avoid taking interrupts for completion
of transfers.  Anything that generates interrupts when NAPI is in
polling mode is defeating the point.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Tue, Feb 05, 2013 at 04:47:05PM +0000, Mark Brown wrote:
> On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote:
> 
> > For IRQ mode, use the completion callback to push each cookie
> > to NAPI, and thus let the IRQ drive the traffic.
> 
> The whole purpose of NAPI is to avoid taking interrupts for completion
> of transfers.  Anything that generates interrupts when NAPI is in
> polling mode is defeating the point.

Yes, but you're missing one very important point.  If your DMA engine
is decoupled from the network device, and the DMA engine relies upon
interrupts to queue further transfers to move data into RAM, then you
have two options:

1. Receive these interrupts so you can update the DMA engine for
   further data transfer.
2. Don't receive these interrupts, and cause the network device to
   error out with a FIFO overrun because its DMA requests have not
   been serviced. (which may raise an interrupt.)

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1238 bytes --]

On Tue, Feb 05, 2013 at 05:06:28PM +0000, Russell King - ARM Linux wrote:
> On Tue, Feb 05, 2013 at 04:47:05PM +0000, Mark Brown wrote:
> > On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote:

> > > For IRQ mode, use the completion callback to push each cookie
> > > to NAPI, and thus let the IRQ drive the traffic.

> > The whole purpose of NAPI is to avoid taking interrupts for completion
> > of transfers.  Anything that generates interrupts when NAPI is in
> > polling mode is defeating the point.

> Yes, but you're missing one very important point.  If your DMA engine
> is decoupled from the network device, and the DMA engine relies upon
> interrupts to queue further transfers to move data into RAM, then you
> have two options:

> 1. Receive these interrupts so you can update the DMA engine for
>    further data transfer.
> 2. Don't receive these interrupts, and cause the network device to
>    error out with a FIFO overrun because its DMA requests have not
>    been serviced. (which may raise an interrupt.)

There's still the third option of just implementing a non-NAPI driver,
though I guess we've now built so much infrastructure on top of NAPI
that it's still useful to pretend we can disable interrupts.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Linus Walleij]

On Tue, Feb 5, 2013 at 5:47 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote:
>
>> For IRQ mode, use the completion callback to push each cookie
>> to NAPI, and thus let the IRQ drive the traffic.
>
> The whole purpose of NAPI is to avoid taking interrupts for completion
> of transfers.  Anything that generates interrupts when NAPI is in
> polling mode is defeating the point.

So what I was trying to get across is that when you're in polling
mode you do not set DMA_PREP_INTERRUPT on your transfers,
just throw the obtained struct dma_async_tx_descriptor on some
list and then when polling use dma_async_is_tx_complete()
on the channel and the cookie inside the descriptor.

I was trying to describe that you can move from
IRQ mode to polling mode and back again by selectively
choosing to set/not set the DMA_PREP_INTERRUPT flag.

If polling is all you want you never set it.

Then there is the fact that the transfer needs to have
been flagged complete and it is indeed something that needs
to be set in some bytes somewhere. By something. But it
doesn't have to be an interrupt from the DMA controller.

In such cases we use dma_async_is_tx_complete() with
channel and cookies as parameter. This will call down into the
driver chan->device->device_tx_status() and there we can
actually poll the hardware to see if the transfer happens to
be complete, and if it is flag it complete.

Which is likely what we want.

No interrupts, only function calls as far as I can see.

(I bet Russell will poke a hole in my reasoning, but it's worth
a try.)

Yours,
Linus Walleij

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Cyril Chemparathy]

On 02/05/2013 01:29 PM, Linus Walleij wrote:
> On Tue, Feb 5, 2013 at 5:47 PM, Mark Brown
> <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> wrote:
>> On Tue, Feb 05, 2013 at 05:21:48PM +0100, Linus Walleij wrote:
>>
>>> For IRQ mode, use the completion callback to push each cookie
>>> to NAPI, and thus let the IRQ drive the traffic.
>>
>> The whole purpose of NAPI is to avoid taking interrupts for completion
>> of transfers.  Anything that generates interrupts when NAPI is in
>> polling mode is defeating the point.
>
> So what I was trying to get across is that when you're in polling
> mode you do not set DMA_PREP_INTERRUPT on your transfers,
> just throw the obtained struct dma_async_tx_descriptor on some
> list and then when polling use dma_async_is_tx_complete()
> on the channel and the cookie inside the descriptor.
>
> I was trying to describe that you can move from
> IRQ mode to polling mode and back again by selectively
> choosing to set/not set the DMA_PREP_INTERRUPT flag.
>

This does not work.  At prep/submit time, the network driver does not 
know if a particular packet buffer needs an interrupt or not.  It queues 
up a whole bunch of receive buffers upfront.  These buffers simply sit 
on the hardware queue/ring until the NIC receives traffic.  The driver 
throttles the receive processing rate by dynamically switching between 
interrupt and poll behaviors on the completion side, not on the 
submission side.

> If polling is all you want you never set it.
>

Another point here is that it is not simply a polling vs. interrupt 
problem.  The system needs to dynamically switch between the two 
behaviors depending on offered load conditions.  This dynamic switching 
is key to balancing latency (under low load) and throughput (at high 
rates).  It cannot be one or the other, it must be both.


Once again, I'm fairly sure that suitably reworking or extending the 
dma-engine interfaces will allow network DMAs to fit in nicely.

However, I'd also appreciate inputs on the alternative approach of using 
virtio devices as an interface to packet oriented DMA hardware.  From my 
perspective this offers the following advantages, some of which I've 
already mentioned in earlier postings:

1. The virtqueue interface is nice and clean, it fits very well with 
networking concepts such as NAPI.  In comparison, the dma-engine API 
will need extensions and/or rework to fit network DMAs.

2. Advantages from leveraging the virtio infrastructure.  For example, 
if a DMA pipe to a remote processor is exposed as a virtio device, 
something like rpmsg could very naturally fit on top of this without any 
other added glue.

3. Advantages from leveraging the driver model for binding dma clients 
to dma hardware, instead of resorting to name lookups and such.

Thanks
-- Cyril.

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [130204 07:46]:
> 
> Current DMA abstraction is quite poor, for example there's no way to
> compile support for multiple DMA engines. Code also makes certain, IMO
> unnecessary, assumptions about the underlying DMA engine (abstraction is
> poor, as said above but it we could follow MUSB's programming guide when
> it comes to programming DMA transfers).
> 
> Considering all of the above, it's far better to use DMA engine and get
> rid of all the mess.

How about just disable MUSB DMA support if ARCH_MULTIPLATFORM for now?
That way MUSB can be fixed up first to support ARCH_MULTIPLATFORM
using PIO while sorting out the DMA related issues.

Regards,

Tony

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Arnd Bergmann]

On Tuesday 05 February 2013, Tony Lindgren wrote:
> * Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> [130204 07:46]:
> > 
> > Current DMA abstraction is quite poor, for example there's no way to
> > compile support for multiple DMA engines. Code also makes certain, IMO
> > unnecessary, assumptions about the underlying DMA engine (abstraction is
> > poor, as said above but it we could follow MUSB's programming guide when
> > it comes to programming DMA transfers).
> > 
> > Considering all of the above, it's far better to use DMA engine and get
> > rid of all the mess.
> 
> How about just disable MUSB DMA support if ARCH_MULTIPLATFORM for now?
> That way MUSB can be fixed up first to support ARCH_MULTIPLATFORM
> using PIO while sorting out the DMA related issues.

Sounds ok to me. Someone still needs to do the work to make the non-DMA
variants of MUSB coexist in one kernel, but as we discussed erlier, that
should be much simpler.

	Arnd

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 1323 bytes --]

On Tue, Feb 05, 2013 at 10:26:30PM +0000, Arnd Bergmann wrote:
> On Tuesday 05 February 2013, Tony Lindgren wrote:
> > * Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> [130204 07:46]:
> > > 
> > > Current DMA abstraction is quite poor, for example there's no way to
> > > compile support for multiple DMA engines. Code also makes certain, IMO
> > > unnecessary, assumptions about the underlying DMA engine (abstraction is
> > > poor, as said above but it we could follow MUSB's programming guide when
> > > it comes to programming DMA transfers).
> > > 
> > > Considering all of the above, it's far better to use DMA engine and get
> > > rid of all the mess.
> > 
> > How about just disable MUSB DMA support if ARCH_MULTIPLATFORM for now?
> > That way MUSB can be fixed up first to support ARCH_MULTIPLATFORM
> > using PIO while sorting out the DMA related issues.
> 
> Sounds ok to me. Someone still needs to do the work to make the non-DMA
> variants of MUSB coexist in one kernel, but as we discussed erlier, that
> should be much simpler.

Yeah, I'm cooking some patches to at least make it buildable. Dropping
unnecessary dependencies and marking da8xx and davinci as depending on
BROKEN seems to be the easiest way; those two glues hasn't seen a real
patch since 2010 or so.

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

Hello.

On 02-02-2013 0:56, Felipe Balbi wrote:

>>> good point, do you wanna send some patches ?

>>     I have already sent them countless times and even stuck CPPI 4.1 support (in
>> arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the
>> patch. :-(

> sticking into arch/arm/common/ wasn't a nice move.

    Like with EDMA we have nothing else to do with CPPI 4.1 being shared by 
DaVinci-like and OMAP-like SOCs. Thank TI for creatring this mess. And 
actually even that is not a good place since I think I know of a MIPS SoC 
having CPPI 4.1 as well, just out of tree.

 > But then again, so
 > wasn't asking for the patch to be removed :-s

    Unfortunately, Russell has done it -- all that was discuseed without me in 
the loop even. :-/

>>> I guess to make the MUSB side simpler we would need musb-dma-engine glue
>>> to map dmaengine to the private MUSB API. Then we would have some
>>> starting point to also move inventra (and anybody else) to dmaengine
>>> API.

>>     Why? Inventra is a dedicated device's private DMA controller, why make
>> universal DMA driver for it?

> because it doesn't make sense to support multiple DMA APIs. We can check
> from MUSB's registers if it was configured with Inventra DMA support and
> based on that we can register MUSB's own DMA Engine to dmaengine API.

     I still disagree. IMO drivers/dma/ is for standalone DMA engines. Else we 
could stick every bus mastering device's DMA engines there. CPPI 4.1 is in 
design standlone DMA engine, despite all in-tree implementations having it as 
subblock of MUSB and serving only MUSB.

WBR, Sergei

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sekhar Nori]

Hi Matt,

This version introduces build/bisect issues.

On 2/1/2013 11:52 PM, Matt Porter wrote:
> Move mach-davinci/dma.c to common/edma.c so it can be used
> by OMAP (specifically AM33xx) as well.
> 
> Signed-off-by: Matt Porter <mporter-l0cyMroinI0@public.gmane.org>
> Acked-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>

> diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/common/edma.c

> @@ -25,7 +25,7 @@
>  #include <linux/io.h>
>  #include <linux/slab.h>
>  
> -#include <mach/edma.h>
> +#include <linux/platform_data/edma.h>

>  /*-----------------------------------------------------------------------*/
> +static int edma_of_read_u32_to_s8_array(const struct device_node *np,
> +					 const char *propname, s8 *out_values,
> +					 size_t sz)
> +{
> +	int ret;
> +
> +	ret = of_property_read_u8_array(np, propname, out_values, sz);

This needs <linux/of.h> to be included in this file.

> +static int edma_xbar_event_map(struct device *dev,
> +			       struct device_node *node,
> +			       struct edma_soc_info *pdata, int len)
> +{
> +	int ret = 0;
> +	int i;
> +	struct resource res;
> +	void *xbar;
> +	const s16 (*xbar_chans)[2];
> +	u32 shift, offset, mux;
> +
> +	xbar_chans = devm_kzalloc(dev,
> +				  len/sizeof(s16) + 2*sizeof(s16),
> +				  GFP_KERNEL);
> +	if (!xbar_chans)
> +		return -ENOMEM;
> +
> +	ret = of_address_to_resource(node, 1, &res);

of_address_to_resource() needs <linux/of_address.h>

> +	if (IS_ERR_VALUE(ret))

This needs <linux/err.h>

> +		return -EIO;
> +
> +	xbar = devm_ioremap(dev, res.start, resource_size(&res));
> +	if (!xbar)
> +		return -ENOMEM;
> +
> +	ret = edma_of_read_u32_to_s16_array(node,
> +					    "ti,edma-xbar-event-map",
> +					    (s16 *)xbar_chans,
> +					    len/sizeof(u32));
> +	if (IS_ERR_VALUE(ret))
> +		return -EIO;
> +
> +	for (i = 0; xbar_chans[i][0] != -1; i++) {
> +		shift = (xbar_chans[i][1] % 4) * 8;
> +		offset = xbar_chans[i][1] >> 2;
> +		offset <<= 2;
> +		mux = readl((void *)((u32)xbar + offset));
> +		mux &= ~(0xff << shift);
> +		mux |= xbar_chans[i][0] << shift;
> +		writel(mux, (void *)((u32)xbar + offset));
> +	}
> +
> +	pdata->xbar_chans = xbar_chans;

There is no member xbar_chans ATM. It seems to be introduced in 03/10.

> +
> +static struct of_dma_filter_info edma_filter_info = {

of_dma_filter_info needs <linux/of_dma.h>.

> +	.filter_fn = edma_filter_fn,

edma_filter_fn needs <linux/edma.h>

BTW, I am not sure if you really intended to introduce EDMA DT support
in this patch. I thought 1/10 was supposed to just move EDMA private API
to a common place. If you really want to introduce DT support as well,
that should be noted in the description. I think it is better done in a
later patch.

> diff --git a/sound/soc/davinci/davinci-sffsdr.c b/sound/soc/davinci/davinci-sffsdr.c

> @@ -17,6 +17,7 @@
>  #include <linux/timer.h>
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
> +#include <linux/platform_data/edma.h>
>  #include <linux/gpio.h>
>  #include <sound/core.h>
>  #include <sound/pcm.h>
> @@ -28,12 +29,14 @@
>  #include <asm/plat-sffsdr/sffsdr-fpga.h>
>  #endif
>  
> -#include <mach/edma.h>
>  
>  #include "../codecs/pcm3008.h"
>  #include "davinci-pcm.h"
>  #include "davinci-i2s.h"
>  
> +#define DAVINCI_DMA_MCBSP_TX	2
> +#define DAVINCI_DMA_MCBSP_RX	3
> +
>  /*
>   * CLKX and CLKR are the inputs for the Sample Rate Generator.
>   * FSX and FSR are outputs, driven by the sample Rate Generator.
> @@ -124,7 +127,7 @@ static struct resource sffsdr_snd_resources[] = {
>  
>  static struct evm_snd_platform_data sffsdr_snd_data = {
>  	.tx_dma_ch	= DAVINCI_DMA_MCBSP_TX,
> -	.rx_dma_ch	= DAVINCI_DMA_MCBSP_RX,

> +	.rx_dma_ch	= DAVINCI_DMA_MCBAP_RX,

Typo here. Should be DAVINCI_DMA_MCBSP_RX. Unfortunately
davinci-sffsdr.c seems to be broken already.

Thanks,
Sekhar

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Russell King - ARM Linux]

On Sat, Feb 09, 2013 at 09:35:53PM +0530, Sekhar Nori wrote:
> On 2/1/2013 11:52 PM, Matt Porter wrote:
> > +	ret = of_address_to_resource(node, 1, &res);
> 
> of_address_to_resource() needs <linux/of_address.h>
> 
> > +	if (IS_ERR_VALUE(ret))
> 
> This needs <linux/err.h>

More importantly, is this the correct way to check for errors from
of_address_to_resource() ?  Grepping the source shows that either less
than 0 or non-zero are the commonly used conditions here.

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Matt Porter]

On Sat, Feb 09, 2013 at 09:35:53PM +0530, Sekhar Nori wrote:
> Hi Matt,
> 
> This version introduces build/bisect issues.

Ok, see later comment which addresses this...

> On 2/1/2013 11:52 PM, Matt Porter wrote:
> > Move mach-davinci/dma.c to common/edma.c so it can be used
> > by OMAP (specifically AM33xx) as well.
> > 
> > Signed-off-by: Matt Porter <mporter@ti.com>
> > Acked-by: Sekhar Nori <nsekhar@ti.com>
> 
> > diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/common/edma.c
> 
> > @@ -25,7 +25,7 @@
> >  #include <linux/io.h>
> >  #include <linux/slab.h>
> >  
> > -#include <mach/edma.h>
> > +#include <linux/platform_data/edma.h>
> 
> >  /*-----------------------------------------------------------------------*/
> > +static int edma_of_read_u32_to_s8_array(const struct device_node *np,
> > +					 const char *propname, s8 *out_values,
> > +					 size_t sz)
> > +{
> > +	int ret;
> > +
> > +	ret = of_property_read_u8_array(np, propname, out_values, sz);
> 
> This needs <linux/of.h> to be included in this file.
> 
> > +static int edma_xbar_event_map(struct device *dev,
> > +			       struct device_node *node,
> > +			       struct edma_soc_info *pdata, int len)
> > +{
> > +	int ret = 0;
> > +	int i;
> > +	struct resource res;
> > +	void *xbar;
> > +	const s16 (*xbar_chans)[2];
> > +	u32 shift, offset, mux;
> > +
> > +	xbar_chans = devm_kzalloc(dev,
> > +				  len/sizeof(s16) + 2*sizeof(s16),
> > +				  GFP_KERNEL);
> > +	if (!xbar_chans)
> > +		return -ENOMEM;
> > +
> > +	ret = of_address_to_resource(node, 1, &res);
> 
> of_address_to_resource() needs <linux/of_address.h>
> 
> > +	if (IS_ERR_VALUE(ret))
> 
> This needs <linux/err.h>
> 
> > +		return -EIO;
> > +
> > +	xbar = devm_ioremap(dev, res.start, resource_size(&res));
> > +	if (!xbar)
> > +		return -ENOMEM;
> > +
> > +	ret = edma_of_read_u32_to_s16_array(node,
> > +					    "ti,edma-xbar-event-map",
> > +					    (s16 *)xbar_chans,
> > +					    len/sizeof(u32));
> > +	if (IS_ERR_VALUE(ret))
> > +		return -EIO;
> > +
> > +	for (i = 0; xbar_chans[i][0] != -1; i++) {
> > +		shift = (xbar_chans[i][1] % 4) * 8;
> > +		offset = xbar_chans[i][1] >> 2;
> > +		offset <<= 2;
> > +		mux = readl((void *)((u32)xbar + offset));
> > +		mux &= ~(0xff << shift);
> > +		mux |= xbar_chans[i][0] << shift;
> > +		writel(mux, (void *)((u32)xbar + offset));
> > +	}
> > +
> > +	pdata->xbar_chans = xbar_chans;
> 
> There is no member xbar_chans ATM. It seems to be introduced in 03/10.
> 
> > +
> > +static struct of_dma_filter_info edma_filter_info = {
> 
> of_dma_filter_info needs <linux/of_dma.h>.
> 
> > +	.filter_fn = edma_filter_fn,
> 
> edma_filter_fn needs <linux/edma.h>
> 
> BTW, I am not sure if you really intended to introduce EDMA DT support
> in this patch. I thought 1/10 was supposed to just move EDMA private API
> to a common place. If you really want to introduce DT support as well,
> that should be noted in the description. I think it is better done in a
> later patch.

This was a complete fubared rebase for this patch. As you noticed, I
got hunks from 3/10 in there and the bisect test didn't run so it was
missed. I've got this fixed up for v8 which address all these comments
that stem from that issue.

> > diff --git a/sound/soc/davinci/davinci-sffsdr.c b/sound/soc/davinci/davinci-sffsdr.c
> 
> > @@ -17,6 +17,7 @@
> >  #include <linux/timer.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/platform_data/edma.h>
> >  #include <linux/gpio.h>
> >  #include <sound/core.h>
> >  #include <sound/pcm.h>
> > @@ -28,12 +29,14 @@
> >  #include <asm/plat-sffsdr/sffsdr-fpga.h>
> >  #endif
> >  
> > -#include <mach/edma.h>
> >  
> >  #include "../codecs/pcm3008.h"
> >  #include "davinci-pcm.h"
> >  #include "davinci-i2s.h"
> >  
> > +#define DAVINCI_DMA_MCBSP_TX	2
> > +#define DAVINCI_DMA_MCBSP_RX	3
> > +
> >  /*
> >   * CLKX and CLKR are the inputs for the Sample Rate Generator.
> >   * FSX and FSR are outputs, driven by the sample Rate Generator.
> > @@ -124,7 +127,7 @@ static struct resource sffsdr_snd_resources[] = {
> >  
> >  static struct evm_snd_platform_data sffsdr_snd_data = {
> >  	.tx_dma_ch	= DAVINCI_DMA_MCBSP_TX,
> > -	.rx_dma_ch	= DAVINCI_DMA_MCBSP_RX,
> 
> > +	.rx_dma_ch	= DAVINCI_DMA_MCBAP_RX,
> 
> Typo here. Should be DAVINCI_DMA_MCBSP_RX. Unfortunately
> davinci-sffsdr.c seems to be broken already.

Ok, fixed..definitely shouldn't break it more.

-Matt

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Matt Porter]

On Sat, Feb 09, 2013 at 08:08:50PM +0000, Russell King wrote:
> On Sat, Feb 09, 2013 at 09:35:53PM +0530, Sekhar Nori wrote:
> > On 2/1/2013 11:52 PM, Matt Porter wrote:
> > > +	ret = of_address_to_resource(node, 1, &res);
> > 
> > of_address_to_resource() needs <linux/of_address.h>
> > 
> > > +	if (IS_ERR_VALUE(ret))
> > 
> > This needs <linux/err.h>
> 
> More importantly, is this the correct way to check for errors from
> of_address_to_resource() ?  Grepping the source shows that either less
> than 0 or non-zero are the commonly used conditions here.

Yes, it's not correct. Fixed to check for non-zero/zero in the two cases
where of_address_to_resource() is used.

-Matt

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_feb

[PATCH v7 02/10] ARM: edma: remove unused transfer controller handlers

[Matt Porter]

Fix build on OMAP, the irqs are undefined on AM33xx.
These error interrupt handlers were hardcoded as disabled
so since they are unused code, simply remove them.

Signed-off-by: Matt Porter <mporter-l0cyMroinI0@public.gmane.org>
Acked-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
---
 arch/arm/common/edma.c |   37 -------------------------------------
 1 file changed, 37 deletions(-)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index aa4a49a..3440d16 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -494,26 +494,6 @@ static irqreturn_t dma_ccerr_handler(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-/******************************************************************************
- *
- * Transfer controller error interrupt handlers
- *
- *****************************************************************************/
-
-#define tc_errs_handled	false	/* disabled as long as they're NOPs */
-
-static irqreturn_t dma_tc0err_handler(int irq, void *data)
-{
-	dev_dbg(data, "dma_tc0err_handler\n");
-	return IRQ_HANDLED;
-}
-
-static irqreturn_t dma_tc1err_handler(int irq, void *data)
-{
-	dev_dbg(data, "dma_tc1err_handler\n");
-	return IRQ_HANDLED;
-}
-
 static int reserve_contiguous_slots(int ctlr, unsigned int id,
 				     unsigned int num_slots,
 				     unsigned int start_slot)
@@ -1743,23 +1723,6 @@ static int edma_probe(struct platform_device *pdev)
 		arch_num_cc++;
 	}
 
-	if (tc_errs_handled) {
-		status = request_irq(IRQ_TCERRINT0, dma_tc0err_handler, 0,
-					"edma_tc0", &pdev->dev);
-		if (status < 0) {
-			dev_dbg(&pdev->dev, "request_irq %d failed --> %d\n",
-				IRQ_TCERRINT0, status);
-			return status;
-		}
-		status = request_irq(IRQ_TCERRINT, dma_tc1err_handler, 0,
-					"edma_tc1", &pdev->dev);
-		if (status < 0) {
-			dev_dbg(&pdev->dev, "request_irq %d --> %d\n",
-				IRQ_TCERRINT, status);
-			return status;
-		}
-	}
-
 	return 0;
 
 fail:
-- 
1.7.9.5


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

[PATCH v7 05/10] dmaengine: edma: Add TI EDMA device tree binding

[Matt Porter]

The binding definition is based on the generic DMA controller
binding.

Signed-off-by: Matt Porter <mporter-l0cyMroinI0@public.gmane.org>
---
 Documentation/devicetree/bindings/dma/ti-edma.txt |   49 +++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/ti-edma.txt

diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt b/Documentation/devicetree/bindings/dma/ti-edma.txt
new file mode 100644
index 0000000..075a60e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/ti-edma.txt
@@ -0,0 +1,49 @@
+TI EDMA
+
+Required properties:
+- compatible : "ti,edma3"
+- ti,hwmods: Name of the hwmods associated to the EDMA
+- ti,edma-regions: Number of regions
+- ti,edma-slots: Number of slots
+- ti,edma-queue-tc-map: List of transfer control to queue mappings
+- ti,edma-queue-priority-map: List of queue priority mappings
+- ti,edma-default-queue: Default queue value
+
+Optional properties:
+- ti,edma-reserved-channels: List of reserved channel regions
+- ti,edma-reserved-slots: List of reserved slot regions
+- ti,edma-xbar-event-map: Crossbar event to channel map
+
+Example:
+
+edma: edma@49000000 {
+	reg = <0x49000000 0x10000>;
+	interrupt-parent = <&intc>;
+	interrupts = <12 13 14>;
+	compatible = "ti,edma3";
+	ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2";
+	#dma-cells = <1>;
+	dma-channels = <64>;
+	ti,edma-regions = <4>;
+	ti,edma-slots = <256>;
+	ti,edma-reserved-channels = <0  2
+				     14 2
+				     26 6
+				     48 4
+				     56 8>;
+	ti,edma-reserved-slots = <0  2
+				  14 2
+				  26 6
+				  48 4
+				  56 8
+				  64 127>;
+	ti,edma-queue-tc-map = <0 0
+				1 1
+				2 2>;
+	ti,edma-queue-priority-map = <0 0
+				      1 1
+				      2 2>;
+	ti,edma-default-queue = <0>;
+	ti,edma-xbar-event-map = <1 12
+				  2 13>;
+};
-- 
1.7.9.5


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 05/10] dmaengine: edma: Add TI EDMA device tree binding

[Matt Porter]

On Fri, Feb 01, 2013 at 01:22:50PM -0500, Matt Porter wrote:
> The binding definition is based on the generic DMA controller
> binding.
> 
> Signed-off-by: Matt Porter <mporter@ti.com>

Grant or Rob, can I get an ack on this binding and others in this
series?

> ---
>  Documentation/devicetree/bindings/dma/ti-edma.txt |   49 +++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/ti-edma.txt
> 
> diff --git a/Documentation/devicetree/bindings/dma/ti-edma.txt b/Documentation/devicetree/bindings/dma/ti-edma.txt
> new file mode 100644
> index 0000000..075a60e3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/ti-edma.txt
> @@ -0,0 +1,49 @@
> +TI EDMA
> +
> +Required properties:
> +- compatible : "ti,edma3"
> +- ti,hwmods: Name of the hwmods associated to the EDMA
> +- ti,edma-regions: Number of regions
> +- ti,edma-slots: Number of slots
> +- ti,edma-queue-tc-map: List of transfer control to queue mappings
> +- ti,edma-queue-priority-map: List of queue priority mappings
> +- ti,edma-default-queue: Default queue value
> +
> +Optional properties:
> +- ti,edma-reserved-channels: List of reserved channel regions
> +- ti,edma-reserved-slots: List of reserved slot regions
> +- ti,edma-xbar-event-map: Crossbar event to channel map
> +
> +Example:
> +
> +edma: edma@49000000 {
> +	reg = <0x49000000 0x10000>;
> +	interrupt-parent = <&intc>;
> +	interrupts = <12 13 14>;
> +	compatible = "ti,edma3";
> +	ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2";
> +	#dma-cells = <1>;
> +	dma-channels = <64>;
> +	ti,edma-regions = <4>;
> +	ti,edma-slots = <256>;
> +	ti,edma-reserved-channels = <0  2
> +				     14 2
> +				     26 6
> +				     48 4
> +				     56 8>;
> +	ti,edma-reserved-slots = <0  2
> +				  14 2
> +				  26 6
> +				  48 4
> +				  56 8
> +				  64 127>;
> +	ti,edma-queue-tc-map = <0 0
> +				1 1
> +				2 2>;
> +	ti,edma-queue-priority-map = <0 0
> +				      1 1
> +				      2 2>;
> +	ti,edma-default-queue = <0>;
> +	ti,edma-xbar-event-map = <1 12
> +				  2 13>;
> +};
> -- 
> 1.7.9.5
> 

[PATCH v7 06/10] ARM: dts: add AM33XX EDMA support

[Matt Porter]

Adds AM33XX EDMA support to the am33xx.dtsi as documented in
Documentation/devicetree/bindings/dma/ti-edma.txt

Signed-off-by: Matt Porter <mporter-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/am33xx.dtsi |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index c2f14e8..e711ffb 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -87,6 +87,26 @@
 			reg = <0x48200000 0x1000>;
 		};
 
+		edma: edma@49000000 {
+			compatible = "ti,edma3";
+			ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2";
+			reg =	<0x49000000 0x10000>,
+				<0x44e10f90 0x10>;
+			interrupt-parent = <&intc>;
+			interrupts = <12 13 14>;
+			#dma-cells = <1>;
+			dma-channels = <64>;
+			ti,edma-regions = <4>;
+			ti,edma-slots = <256>;
+			ti,edma-queue-tc-map = <0 0
+						1 1
+						2 2>;
+			ti,edma-queue-priority-map = <0 0
+						      1 1
+						      2 2>;
+			ti,edma-default-queue = <0>;
+		};
+
 		gpio1: gpio@44e07000 {
 			compatible = "ti,omap4-gpio";
 			ti,hwmods = "gpio1";
-- 
1.7.9.5


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

[PATCH v7 07/10] dmaengine: add dma_request_slave_channel_compat()

[Matt Porter]

Adds a dma_request_slave_channel_compat() wrapper which accepts
both the arguments from dma_request_channel() and
dma_request_slave_channel(). Based on whether the driver is
instantiated via DT, the appropriate channel request call will be
made.

This allows for a much cleaner migration of drivers to the
dmaengine DT API as platforms continue to be mixed between those
that boot using DT and those that do not.

Suggested-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Matt Porter <mporter-l0cyMroinI0@public.gmane.org>
Acked-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
 include/linux/dmaengine.h |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index bfcdecb..17d8ffd 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1001,6 +1001,22 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
 struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
 struct dma_chan *net_dma_find_channel(void);
 #define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
+#define dma_request_slave_channel_compat(mask, x, y, dev, name) \
+	__dma_request_slave_channel_compat(&(mask), x, y, dev, name)
+
+static inline struct dma_chan
+*__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
+				  void *fn_param, struct device *dev,
+				  char *name)
+{
+	struct dma_chan *chan;
+
+	chan = dma_request_slave_channel(dev, name);
+	if (chan)
+		return chan;
+
+	return __dma_request_channel(mask, fn, fn_param);
+}
 
 /* --- Helper iov-locking functions --- */
 
-- 
1.7.9.5


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 07/10] dmaengine: add dma_request_slave_channel_compat()

[Matt Porter]

On Fri, Feb 01, 2013 at 01:22:52PM -0500, Matt Porter wrote:
> Adds a dma_request_slave_channel_compat() wrapper which accepts
> both the arguments from dma_request_channel() and
> dma_request_slave_channel(). Based on whether the driver is
> instantiated via DT, the appropriate channel request call will be
> made.
> 
> This allows for a much cleaner migration of drivers to the
> dmaengine DT API as platforms continue to be mixed between those
> that boot using DT and those that do not.
> 
> Suggested-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Matt Porter <mporter@ti.com>
> Acked-by: Tony Lindgren <tony@atomide.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Vinod, can I get an ack on this one?

-Matt

> ---
>  include/linux/dmaengine.h |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index bfcdecb..17d8ffd 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1001,6 +1001,22 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
>  struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
>  struct dma_chan *net_dma_find_channel(void);
>  #define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
> +#define dma_request_slave_channel_compat(mask, x, y, dev, name) \
> +	__dma_request_slave_channel_compat(&(mask), x, y, dev, name)
> +
> +static inline struct dma_chan
> +*__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
> +				  void *fn_param, struct device *dev,
> +				  char *name)
> +{
> +	struct dma_chan *chan;
> +
> +	chan = dma_request_slave_channel(dev, name);
> +	if (chan)
> +		return chan;
> +
> +	return __dma_request_channel(mask, fn, fn_param);
> +}
>  
>  /* --- Helper iov-locking functions --- */
>  
> -- 
> 1.7.9.5
> 

Re: [PATCH v7 07/10] dmaengine: add dma_request_slave_channel_compat()

[Vinod Koul]

On Fri, Feb 01, 2013 at 01:22:52PM -0500, Matt Porter wrote:
> Adds a dma_request_slave_channel_compat() wrapper which accepts
> both the arguments from dma_request_channel() and
> dma_request_slave_channel(). Based on whether the driver is
> instantiated via DT, the appropriate channel request call will be
> made.
> 
> This allows for a much cleaner migration of drivers to the
> dmaengine DT API as platforms continue to be mixed between those
> that boot using DT and those that do not.
> 
> Suggested-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Matt Porter <mporter@ti.com>
> Acked-by: Tony Lindgren <tony@atomide.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Vinod Koul <vinod.koul@intel.com>

> ---
>  include/linux/dmaengine.h |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index bfcdecb..17d8ffd 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -1001,6 +1001,22 @@ void dma_run_dependencies(struct dma_async_tx_descriptor *tx);
>  struct dma_chan *dma_find_channel(enum dma_transaction_type tx_type);
>  struct dma_chan *net_dma_find_channel(void);
>  #define dma_request_channel(mask, x, y) __dma_request_channel(&(mask), x, y)
> +#define dma_request_slave_channel_compat(mask, x, y, dev, name) \
> +	__dma_request_slave_channel_compat(&(mask), x, y, dev, name)
> +
> +static inline struct dma_chan
> +*__dma_request_slave_channel_compat(dma_cap_mask_t *mask, dma_filter_fn fn,
> +				  void *fn_param, struct device *dev,
> +				  char *name)
> +{
> +	struct dma_chan *chan;
> +
> +	chan = dma_request_slave_channel(dev, name);
> +	if (chan)
> +		return chan;
> +
> +	return __dma_request_channel(mask, fn, fn_param);
> +}
>  
>  /* --- Helper iov-locking functions --- */
>  
> -- 
> 1.7.9.5
> 

[PATCH v7 08/10] spi: omap2-mcspi: convert to dma_request_slave_channel_compat()

[Matt Porter]

Convert dmaengine channel requests to use
dma_request_slave_channel_compat(). This supports the DT case of
platforms requiring channel selection from either the OMAP DMA or
the EDMA engine. AM33xx only boots from DT and is the only user
implementing EDMA so in the !DT case we can default to the OMAP DMA
filter.

Signed-off-by: Matt Porter <mporter-l0cyMroinI0@public.gmane.org>
Acked-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
---
 drivers/spi/spi-omap2-mcspi.c |   65 ++++++++++++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 20 deletions(-)

diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index b610f52..2c02c02 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -102,6 +102,9 @@ struct omap2_mcspi_dma {
 
 	struct completion dma_tx_completion;
 	struct completion dma_rx_completion;
+
+	char dma_rx_ch_name[14];
+	char dma_tx_ch_name[14];
 };
 
 /* use PIO for small transfers, avoiding DMA setup/teardown overhead and
@@ -822,14 +825,23 @@ static int omap2_mcspi_request_dma(struct spi_device *spi)
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
 	sig = mcspi_dma->dma_rx_sync_dev;
-	mcspi_dma->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+
+	mcspi_dma->dma_rx =
+		dma_request_slave_channel_compat(mask, omap_dma_filter_fn,
+						 &sig, &master->dev,
+						 mcspi_dma->dma_rx_ch_name);
+
 	if (!mcspi_dma->dma_rx) {
 		dev_err(&spi->dev, "no RX DMA engine channel for McSPI\n");
 		return -EAGAIN;
 	}
 
 	sig = mcspi_dma->dma_tx_sync_dev;
-	mcspi_dma->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+	mcspi_dma->dma_tx =
+		dma_request_slave_channel_compat(mask, omap_dma_filter_fn,
+						 &sig, &master->dev,
+						 mcspi_dma->dma_tx_ch_name);
+
 	if (!mcspi_dma->dma_tx) {
 		dev_err(&spi->dev, "no TX DMA engine channel for McSPI\n");
 		dma_release_channel(mcspi_dma->dma_rx);
@@ -1223,29 +1235,42 @@ static int omap2_mcspi_probe(struct platform_device *pdev)
 		goto free_master;
 
 	for (i = 0; i < master->num_chipselect; i++) {
-		char dma_ch_name[14];
+		char *dma_rx_ch_name = mcspi->dma_channels[i].dma_rx_ch_name;
+		char *dma_tx_ch_name = mcspi->dma_channels[i].dma_tx_ch_name;
 		struct resource *dma_res;
 
-		sprintf(dma_ch_name, "rx%d", i);
-		dma_res = platform_get_resource_byname(pdev, IORESOURCE_DMA,
-							dma_ch_name);
-		if (!dma_res) {
-			dev_dbg(&pdev->dev, "cannot get DMA RX channel\n");
-			status = -ENODEV;
-			break;
-		}
+		sprintf(dma_rx_ch_name, "rx%d", i);
+		if (!pdev->dev.of_node) {
+			dma_res =
+				platform_get_resource_byname(pdev,
+							     IORESOURCE_DMA,
+							     dma_rx_ch_name);
+			if (!dma_res) {
+				dev_dbg(&pdev->dev,
+					"cannot get DMA RX channel\n");
+				status = -ENODEV;
+				break;
+			}
 
-		mcspi->dma_channels[i].dma_rx_sync_dev = dma_res->start;
-		sprintf(dma_ch_name, "tx%d", i);
-		dma_res = platform_get_resource_byname(pdev, IORESOURCE_DMA,
-							dma_ch_name);
-		if (!dma_res) {
-			dev_dbg(&pdev->dev, "cannot get DMA TX channel\n");
-			status = -ENODEV;
-			break;
+			mcspi->dma_channels[i].dma_rx_sync_dev =
+				dma_res->start;
 		}
+		sprintf(dma_tx_ch_name, "tx%d", i);
+		if (!pdev->dev.of_node) {
+			dma_res =
+				platform_get_resource_byname(pdev,
+							     IORESOURCE_DMA,
+							     dma_tx_ch_name);
+			if (!dma_res) {
+				dev_dbg(&pdev->dev,
+					"cannot get DMA TX channel\n");
+				status = -ENODEV;
+				break;
+			}
 
-		mcspi->dma_channels[i].dma_tx_sync_dev = dma_res->start;
+			mcspi->dma_channels[i].dma_tx_sync_dev =
+				dma_res->start;
+		}
 	}
 
 	if (status < 0)
-- 
1.7.9.5


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

[PATCH v7 09/10] spi: omap2-mcspi: add generic DMA request support to the DT binding

[Matt Porter]

The binding definition is based on the generic DMA request binding

Signed-off-by: Matt Porter <mporter-l0cyMroinI0@public.gmane.org>
---
 Documentation/devicetree/bindings/spi/omap-spi.txt |   27 +++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/omap-spi.txt b/Documentation/devicetree/bindings/spi/omap-spi.txt
index 938809c..4c85c4c 100644
--- a/Documentation/devicetree/bindings/spi/omap-spi.txt
+++ b/Documentation/devicetree/bindings/spi/omap-spi.txt
@@ -10,7 +10,18 @@ Required properties:
 			  input. The default is D0 as input and
 			  D1 as output.
 
-Example:
+Optional properties:
+- dmas: List of DMA specifiers with the controller specific format
+	as described in the generic DMA client binding. A tx and rx
+	specifier is required for each chip select.
+- dma-names: List of DMA request names. These strings correspond
+	1:1 with the DMA specifiers listed in dmas. The string naming
+	is to be "rxN" and "txN" for RX and TX requests,
+	respectively, where N equals the chip select number.
+
+Examples:
+
+[hwmod populated DMA resources]
 
 mcspi1: mcspi@1 {
     #address-cells = <1>;
@@ -20,3 +31,17 @@ mcspi1: mcspi@1 {
     ti,spi-num-cs = <4>;
 };
 
+[generic DMA request binding]
+
+mcspi1: mcspi@1 {
+    #address-cells = <1>;
+    #size-cells = <0>;
+    compatible = "ti,omap4-mcspi";
+    ti,hwmods = "mcspi1";
+    ti,spi-num-cs = <2>;
+    dmas = <&edma 42
+	    &edma 43
+	    &edma 44
+	    &edma 45>;
+    dma-names = "tx0", "rx0", "tx1", "rx1";
+};
-- 
1.7.9.5


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

[PATCH v7 10/10] ARM: dts: add AM33XX SPI DMA support

[Matt Porter]

Adds DMA resources to the AM33XX SPI nodes.

Signed-off-by: Matt Porter <mporter-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/am33xx.dtsi |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index e711ffb..ddf702a 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -328,6 +328,11 @@
 			interrupt = <65>;
 			ti,spi-num-cs = <2>;
 			ti,hwmods = "spi0";
+			dmas = <&edma 16
+				&edma 17
+				&edma 18
+				&edma 19>;
+			dma-names = "tx0", "rx0", "tx1", "rx1";
 			status = "disabled";
 		};
 
@@ -339,6 +344,11 @@
 			interrupt = <125>;
 			ti,spi-num-cs = <2>;
 			ti,hwmods = "spi1";
+			dmas = <&edma 42
+				&edma 43
+				&edma 44
+				&edma 45>;
+			dma-names = "tx0", "rx0", "tx1", "rx1";
 			status = "disabled";
 		};
 
-- 
1.7.9.5


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_jan

Re: [PATCH v7 00/10] DMA Engine support for AM33XX

[Matt Porter]

On Fri, Feb 01, 2013 at 01:22:45PM -0500, Matt Porter wrote:

<snip>

> This series adds DMA Engine support for AM33xx, which uses
> an EDMA DMAC. The EDMA DMAC has been previously supported by only
> a private API implementation (much like the situation with OMAP
> DMA) found on the DaVinci family of SoCs.

Tony,

Will you be able to take this series in through your tree? It appears we
just need acks on the new bindings and the compat api change. I haven't
heard from you or Benoit on the dts patches so it seems those are ready
as well.

Thanks,
Matt

> 
> The series applies on top of 3.8-rc5 and the following patches:
> 
> 	- dmaengine DT support and edma dmaengine driver fix from 
> 	  the git://git.infradead.org/users/vkoul/slave-dma.git next
> 	  branch
> 
> The approach taken is similar to how OMAP DMA is being converted to
> DMA Engine support. With the functional EDMA private API already
> existing in mach-davinci/dma.c, we first move that to an ARM common
> area so it can be shared. Adding DT and runtime PM support to the
> private EDMA API implementation allows it to run on AM33xx. AM33xx
> *only* boots using DT so we leverage Jon's generic DT DMA helpers to
> register EDMA DMAC with the of_dma framework and then add support
> for calling the dma_request_slave_channel() API to both the mmc
> and spi drivers.
> 
> With this series both BeagleBone and the AM335x EVM have working
> SPI DMA support (and MMC support with the separate MMC series).
> 
> This is tested on BeagleBone with a SPI framebuffer driver and MMC
> rootfs. A trivial gpio DMA event misc driver was used to test the
> crossbar DMA event support. It is also tested on the AM335x EVM
> with the onboard SPI flash and MMC rootfs. The branch at
> https://github.com/ohporter/linux/tree/edma-dmaengine-am33xx-v7
> has the complete series, dependencies, and some test
> drivers/defconfigs. Note that MMC can only be tested with a
> separate MMC dmaengine/DT series applied.
> 
> Regression testing was done on AM180x-EVM (which also makes use
> of the EDMA dmaengine driver and the EDMA private API) using SD,
> SPI flash, and the onboard audio supported by the ASoC Davinci
> driver. Regression testing was also done on a BeagleBoard xM
> booting from the legacy board file using MMC rootfs.
> 
> 
> Matt Porter (10):
>   ARM: davinci: move private EDMA API to arm/common
>   ARM: edma: remove unused transfer controller handlers
>   ARM: edma: add AM33XX support to the private EDMA API
>   dmaengine: edma: enable build for AM33XX
>   dmaengine: edma: Add TI EDMA device tree binding
>   ARM: dts: add AM33XX EDMA support
>   dmaengine: add dma_request_slave_channel_compat()
>   spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
>   spi: omap2-mcspi: add generic DMA request support to the DT binding
>   ARM: dts: add AM33XX SPI DMA support
> 
>  Documentation/devicetree/bindings/dma/ti-edma.txt  |   49 +++
>  Documentation/devicetree/bindings/spi/omap-spi.txt |   27 +-
>  arch/arm/Kconfig                                   |    1 +
>  arch/arm/boot/dts/am33xx.dtsi                      |   30 ++
>  arch/arm/common/Kconfig                            |    3 +
>  arch/arm/common/Makefile                           |    1 +
>  arch/arm/{mach-davinci/dma.c => common/edma.c}     |  342 +++++++++++++++++---
>  arch/arm/mach-davinci/Makefile                     |    2 +-
>  arch/arm/mach-davinci/board-tnetv107x-evm.c        |    2 +-
>  arch/arm/mach-davinci/davinci.h                    |    2 +-
>  arch/arm/mach-davinci/devices-tnetv107x.c          |    2 +-
>  arch/arm/mach-davinci/devices.c                    |    6 +-
>  arch/arm/mach-davinci/dm355.c                      |    2 +-
>  arch/arm/mach-davinci/dm365.c                      |    2 +-
>  arch/arm/mach-davinci/dm644x.c                     |    2 +-
>  arch/arm/mach-davinci/dm646x.c                     |    2 +-
>  arch/arm/mach-davinci/include/mach/da8xx.h         |    2 +-
>  arch/arm/plat-omap/Kconfig                         |    1 +
>  drivers/dma/Kconfig                                |    2 +-
>  drivers/dma/edma.c                                 |    2 +-
>  drivers/mmc/host/davinci_mmc.c                     |    1 +
>  drivers/spi/spi-omap2-mcspi.c                      |   65 ++--
>  include/linux/dmaengine.h                          |   16 +
>  include/linux/mfd/davinci_voicecodec.h             |    3 +-
>  .../mach => include/linux/platform_data}/edma.h    |   90 +-----
>  include/linux/platform_data/spi-davinci.h          |    2 +-
>  sound/soc/davinci/davinci-evm.c                    |    1 +
>  sound/soc/davinci/davinci-pcm.c                    |    1 +
>  sound/soc/davinci/davinci-pcm.h                    |    2 +-
>  sound/soc/davinci/davinci-sffsdr.c                 |    7 +-
>  30 files changed, 496 insertions(+), 174 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/dma/ti-edma.txt
>  rename arch/arm/{mach-davinci/dma.c => common/edma.c} (86%)
>  rename {arch/arm/mach-davinci/include/mach => include/linux/platform_data}/edma.h (59%)
> 
> -- 
> 1.7.9.5
>