2013-06-20 21:08:06

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 00/11] DMA Engine support for AM33XX

From: Joel A Fernandes <[email protected]>

This series is remaining of Matt Porter's EDMA patches for AM33XX EDMA support
with changes for few pending review comments on v11 series.

Also included are EDMA config and build options for OMAP2PLUS and Davinci by
selecting DMADEVICES, and options to enable MMCSD on Davinci (da8xx_omapl).

Currently this is required for AM33XX (Beaglebone or EVM) to access MMC
and be able mount to rootfs and boot till command prompt over MMC.
Unless there are other pending review comments, I hope this series can
make it into 3.11 merge window, the dependent series has been posted at [1]
and completed review.

Tested EDMA on AM1808 EVM and AM33XX Beaglebone with MMC.

Sekhar Nori has posted a GIT PULL [1] which has 2 patches this series depends on:
[1] http://www.spinics.net/lists/arm-kernel/msg251503.html

Changes since v11:
- Fixed several style issues and warnings
- Broke up the build option patch into smaller ones
- Various miscellaneous fixups addressed from v10 feedback

Changes since v10:
- Reworked documentation based on Arnd's feedback
- Moved EDMA bindings documentation earlier in series
- Dropped mention on am33xx on 2/8 and 3/8 in the series

Changes since v9:
- Droped reserved and queue DT entries from Documentation
for now from the original patch series.
- Drop DT entries that are non-hardware-description
- Split EDMA xbar support out of original EDMA DT parsing patch
to keep it easier for review.
- Rewrite shift and offset calculation xbar event mapping.
- Setup default one-to-one mapping for queue_priority and queue_tc
mapping as discussed in.
- Split out xbar stuff to separate patch.

Reference discussion:
https://patchwork.kernel.org/patch/2226761/

Changes since v8:
- Removed edma node interrupt-parent property, it is inherited

Changes since v7:
- Dropped dmaengine compat() patch. It is upstream.
- Submitted edma_alloc_slot() error checking bug fix separately,
now a dependency
- Fixed bisect issues due to 3/10 hunks that went into 1/10
- Fixed incorrect IS_ERRVALUE() use in 3/10

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.10-rc4.

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 the upstream generic DT DMA helpers are
leveraged to register EDMA DMAC with the of_dma framework. SPI (and
MMC in a separate series) are supported using the upstream
dma_request_slave_channel_compat() dmaengine call that allows
compatibility with !DT platforms.

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. 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.


Joel A Fernandes (3):
edma: config: Enable config options for EDMA
da8xx: config: Enable MMC and FS options
ARM: davinci: Fix compiler warnings in devices-da8xx

Matt Porter (8):
dmaengine: edma: Add TI EDMA device tree binding
ARM: edma: Add DT and runtime PM support to the private EDMA API
ARM: edma: Add EDMA crossbar event mux support
dmaengine: edma: enable build for AM33XX
spi: omap2-mcspi: add generic DMA request support to the DT binding
spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
ARM: dts: add AM33XX EDMA support
ARM: dts: add AM33XX SPI DMA support

Documentation/devicetree/bindings/dma/ti-edma.txt | 34 +++
Documentation/devicetree/bindings/spi/omap-spi.txt | 27 ++-
arch/arm/Kconfig | 1 +
arch/arm/boot/dts/am33xx.dtsi | 22 ++
arch/arm/common/edma.c | 242 ++++++++++++++++++--
arch/arm/configs/da8xx_omapl_defconfig | 3 +
arch/arm/mach-davinci/devices-da8xx.c | 8 +-
arch/arm/mach-omap2/Kconfig | 2 +
drivers/dma/Kconfig | 4 +-
drivers/spi/spi-omap2-mcspi.c | 64 ++++--
include/linux/platform_data/edma.h | 5 +-
11 files changed, 369 insertions(+), 43 deletions(-)
create mode 100644 Documentation/devicetree/bindings/dma/ti-edma.txt

--
1.7.9.5


2013-06-20 21:08:13

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 04/11] dmaengine: edma: enable build for AM33XX

From: Matt Porter <[email protected]>

Enable TI EDMA option on OMAP and TI_PRIV_EDMA

Signed-off-by: Matt Porter <[email protected]>
Signed-off-by: Joel A Fernandes <[email protected]>
---
arch/arm/mach-omap2/Kconfig | 1 +
drivers/dma/Kconfig | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index f49cd51..f91b07f 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -17,6 +17,7 @@ config ARCH_OMAP2PLUS
select PROC_DEVICETREE if PROC_FS
select SOC_BUS
select SPARSE_IRQ
+ select TI_PRIV_EDMA
select USE_OF
help
Systems based on OMAP2, OMAP3, OMAP4 or OMAP5
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index e992489..3215a3c 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -213,7 +213,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

2013-06-20 21:08:09

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 02/11] ARM: edma: Add DT and runtime PM support to the private EDMA API

From: Matt Porter <[email protected]>

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. Enables build on OMAP.

Changes by Joel:
* Setup default one-to-one mapping for queue_priority and queue_tc
mapping as discussed in [1].
* Split out xbar stuff to separate patch. [1]
* Dropped unused DT helper to convert to array

[1] https://patchwork.kernel.org/patch/2226761/

Signed-off-by: Matt Porter <[email protected]>
Acked-by: Sekhar Nori <[email protected]>
Signed-off-by: Joel A Fernandes <[email protected]>
---
arch/arm/common/edma.c | 180 +++++++++++++++++++++++++++++++++---
include/linux/platform_data/edma.h | 4 +-
2 files changed, 169 insertions(+), 15 deletions(-)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 7658874..407e01e 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -25,6 +25,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>

@@ -1369,13 +1376,102 @@ void edma_clear_event(unsigned channel)
}
EXPORT_SYMBOL(edma_clear_event);

-/*-----------------------------------------------------------------------*/
+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_of_parse_dt(struct device *dev,
+ struct device_node *node,
+ struct edma_soc_info *pdata)
+{
+ int ret = 0, i;
+ u32 value;
+ struct property *prop;
+ size_t sz;
+ struct edma_rsv_info *rsv_info;
+ 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;
+
+ queue_tc_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
+ if (!queue_tc_map)
+ return -ENOMEM;
+
+ for (i = 0; i < 3; i++) {
+ queue_tc_map[i][0] = i;
+ queue_tc_map[i][1] = i;
+ }
+ queue_tc_map[i][0] = -1;
+ queue_tc_map[i][1] = -1;
+
+ pdata->queue_tc_mapping = queue_tc_map;
+
+ queue_priority_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
+ if (!queue_priority_map)
+ return -ENOMEM;
+
+ for (i = 0; i < 3; i++) {
+ queue_priority_map[i][0] = i;
+ queue_priority_map[i][1] = i;
+ }
+ queue_priority_map[i][0] = -1;
+ queue_priority_map[i][1] = -1;
+
+ pdata->queue_priority_mapping = queue_priority_map;
+
+ pdata->default_queue = 0;
+
+ 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];
- const s8 (*queue_tc_mapping)[2];
+ struct edma_soc_info *ninfo[EDMA_MAX_CC] = {NULL};
+ struct edma_soc_info tmpinfo;
+ s8 (*queue_priority_mapping)[2];
+ s8 (*queue_tc_mapping)[2];
int i, j, off, ln, found = 0;
int status = -1;
const s16 (*rsv_chans)[2];
@@ -1383,17 +1479,56 @@ static int __init edma_probe(struct platform_device *pdev)
int irq[EDMA_MAX_CC] = {0, 0};
int err_irq[EDMA_MAX_CC] = {0, 0};
struct resource *r[EDMA_MAX_CC] = {NULL};
+ struct resource res[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 (ret < 0) {
+ 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 (!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
@@ -1440,7 +1575,7 @@ static int __init edma_probe(struct platform_device *pdev)
off = rsv_chans[i][0];
ln = rsv_chans[i][1];
clear_bits(off, ln,
- edma_cc[j]->edma_unused);
+ edma_cc[j]->edma_unused);
}
}

@@ -1456,8 +1591,14 @@ static int __init edma_probe(struct platform_device *pdev)
}
}

- sprintf(irq_name, "edma%d", j);
- irq[j] = platform_get_irq_byname(pdev, irq_name);
+
+ 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 = devm_request_irq(&pdev->dev, irq[j],
dma_irq_handler, 0, "edma",
@@ -1469,8 +1610,13 @@ static int __init edma_probe(struct platform_device *pdev)
return status;
}

- 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 = devm_request_irq(&pdev->dev, err_irq[j],
dma_ccerr_handler, 0,
@@ -1516,9 +1662,17 @@ static int __init edma_probe(struct platform_device *pdev)
return 0;
}

+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/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index 2344ea2..317f2be 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -175,8 +175,8 @@ struct edma_soc_info {
/* Resource reservation for other cores */
struct edma_rsv_info *rsv;

- const s8 (*queue_tc_mapping)[2];
- const s8 (*queue_priority_mapping)[2];
+ s8 (*queue_tc_mapping)[2];
+ s8 (*queue_priority_mapping)[2];
};

#endif
--
1.7.9.5

2013-06-20 21:08:02

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 10/11] ARM: dts: add AM33XX EDMA support

From: Matt Porter <[email protected]>

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

Joel: Drop DT entries that are non-hardware-description for now as discussed in [1]

[1] https://patchwork.kernel.org/patch/2226761/

Signed-off-by: Matt Porter <[email protected]>
Signed-off-by: Joel A Fernandes <[email protected]>
---
arch/arm/boot/dts/am33xx.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index d9cad72..3d59bb3 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -89,6 +89,18 @@
reg = <0x48200000 0x1000>;
};

+ edma: edma@49000000 {
+ compatible = "ti,edma3";
+ ti,hwmods = "tpcc", "tptc0", "tptc1", "tptc2";
+ reg = <0x49000000 0x10000>,
+ <0x44e10f90 0x10>;
+ interrupts = <12 13 14>;
+ #dma-cells = <1>;
+ dma-channels = <64>;
+ ti,edma-regions = <4>;
+ ti,edma-slots = <256>;
+ };
+
gpio0: gpio@44e07000 {
compatible = "ti,omap4-gpio";
ti,hwmods = "gpio1";
--
1.7.9.5

2013-06-20 21:09:09

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 08/11] spi: omap2-mcspi: add generic DMA request support to the DT binding

From: Matt Porter <[email protected]>

The binding definition is based on the generic DMA request binding

Signed-off-by: Matt Porter <[email protected]>
Signed-off-by: Joel A Fernandes <[email protected]>
---
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

2013-06-20 21:08:00

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 11/11] ARM: dts: add AM33XX SPI DMA support

From: Matt Porter <[email protected]>

Adds DMA resources to the AM33XX SPI nodes.

Signed-off-by: Matt Porter <[email protected]>
Signed-off-by: Joel A Fernandes <[email protected]>
---
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 3d59bb3..fb17103 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -361,6 +361,11 @@
interrupts = <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";
};

@@ -372,6 +377,11 @@
interrupts = <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

2013-06-20 21:09:49

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 05/11] edma: config: Enable config options for EDMA

From: Joel A Fernandes <[email protected]>

Build TI_EDMA by default for ARCH_DAVINCI and ARCH_OMAP2PLUS

Signed-off-by: Joel A Fernandes <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/mach-omap2/Kconfig | 1 +
drivers/dma/Kconfig | 2 +-
3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b1c66a4..7d58cd9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -841,6 +841,7 @@ config ARCH_DAVINCI
select HAVE_IDE
select NEED_MACH_GPIO_H
select TI_PRIV_EDMA
+ select DMADEVICES
select USE_OF
select ZONE_DMA
help
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index f91b07f..c02f083 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -17,6 +17,7 @@ config ARCH_OMAP2PLUS
select PROC_DEVICETREE if PROC_FS
select SOC_BUS
select SPARSE_IRQ
+ select DMADEVICES
select TI_PRIV_EDMA
select USE_OF
help
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 3215a3c..b2d6f15 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -216,7 +216,7 @@ config TI_EDMA
depends on ARCH_DAVINCI || ARCH_OMAP
select DMA_ENGINE
select DMA_VIRTUAL_CHANNELS
- default n
+ default y
help
Enable support for the TI EDMA controller. This DMA
engine is found on TI DaVinci and AM33xx parts.
--
1.7.9.5

2013-06-20 21:10:14

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 09/11] spi: omap2-mcspi: convert to dma_request_slave_channel_compat()

From: Matt Porter <[email protected]>

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 <[email protected]>
Acked-by: Mark Brown <[email protected]>
Signed-off-by: Joel A Fernandes <[email protected]>
---
drivers/spi/spi-omap2-mcspi.c | 64 ++++++++++++++++++++++++++++-------------
1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index 86d2158..ca4ab78 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
@@ -830,12 +833,20 @@ 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)
goto no_dma;

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) {
dma_release_channel(mcspi_dma->dma_rx);
mcspi_dma->dma_rx = NULL;
@@ -1256,29 +1267,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

2013-06-20 21:07:57

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 07/11] ARM: davinci: Fix compiler warnings in devices-da8xx

From: Joel A Fernandes <[email protected]>

queue_priority_mapping and queue_tc_mapping are no longer
const anymore generating a bunch of warnings in devices-da8xx.
Fix them by not doing constant assignments.

Signed-off-by: Joel A Fernandes <[email protected]>
---
arch/arm/mach-davinci/devices-da8xx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index bf57252..eb254fe 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -105,27 +105,27 @@ struct platform_device da8xx_serial_device = {
},
};

-static const s8 da8xx_queue_tc_mapping[][2] = {
+static s8 da8xx_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
{1, 1},
{-1, -1}
};

-static const s8 da8xx_queue_priority_mapping[][2] = {
+static s8 da8xx_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
{1, 7},
{-1, -1}
};

-static const s8 da850_queue_tc_mapping[][2] = {
+static s8 da850_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
{-1, -1}
};

-static const s8 da850_queue_priority_mapping[][2] = {
+static s8 da850_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
{-1, -1}
--
1.7.9.5

2013-06-20 21:10:52

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 03/11] ARM: edma: Add EDMA crossbar event mux support

From: Matt Porter <[email protected]>

Changes by Joel:
* Split EDMA xbar support out of original EDMA DT parsing patch
to keep it easier for review.
* Rewrite shift and offset calculation.

Suggested-by: Sekhar Nori <[email protected]>
Suggested by: Andy Shevchenko <[email protected]>
Signed-off-by: Joel A Fernandes <[email protected]>

Reference:
[1] https://patchwork.kernel.org/patch/2226991/
---
arch/arm/common/edma.c | 62 +++++++++++++++++++++++++++++++++++-
include/linux/platform_data/edma.h | 1 +
2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 407e01e..a03d4f0 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -568,7 +568,7 @@ static int prepare_unused_channel_list(struct device *dev, void *data)
(int)pdev->resource[i].start >= 0) {
ctlr = EDMA_CTLR(pdev->resource[i].start);
clear_bit(EDMA_CHAN_SLOT(pdev->resource[i].start),
- edma_cc[ctlr]->edma_unused);
+ edma_cc[ctlr]->edma_unused);
}
}

@@ -1393,6 +1393,52 @@ static int edma_of_read_u32_to_s16_array(const struct device_node *np,
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 __iomem *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 (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 (ret)
+ return -EIO;
+
+ for (i = 0; xbar_chans[i][0] != -1; i++) {
+ shift = (xbar_chans[i][1] & 0x03) << 3;
+ offset = xbar_chans[i][1] & 0xfffffffc;
+ mux = readl(xbar + offset);
+ mux &= ~(0xff << shift);
+ mux |= xbar_chans[i][0] << shift;
+ writel(mux, (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)
@@ -1458,6 +1504,10 @@ static int edma_of_parse_dt(struct device *dev,

pdata->default_queue = 0;

+ prop = of_find_property(node, "ti,edma-xbar-event-map", &sz);
+ if (prop)
+ ret = edma_xbar_event_map(dev, node, pdata, sz);
+
return ret;
}

@@ -1476,6 +1526,7 @@ static int edma_probe(struct platform_device *pdev)
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};
@@ -1591,6 +1642,15 @@ static int edma_probe(struct platform_device *pdev)
}
}

+ /* 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);
diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index 317f2be..57300fd 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -177,6 +177,7 @@ struct edma_soc_info {

s8 (*queue_tc_mapping)[2];
s8 (*queue_priority_mapping)[2];
+ const s16 (*xbar_chans)[2];
};

#endif
--
1.7.9.5

2013-06-20 21:11:16

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 01/11] dmaengine: edma: Add TI EDMA device tree binding

From: Matt Porter <[email protected]>

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

Joel:
* Droped reserved and queue DT entries from Documentation
for now from the original patch series (v10)
* Included properties in Documentation and clarified DMA properties (V11)
* Made ti,hwmod option
* Clarified DMA entries

Acked-by: Arnd Bergmann <[email protected]>
Signed-off-by: Matt Porter <[email protected]>
Signed-off-by: Joel A Fernandes <[email protected]>
---
Documentation/devicetree/bindings/dma/ti-edma.txt | 34 +++++++++++++++++++++
1 file changed, 34 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..9fbbdb7
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/ti-edma.txt
@@ -0,0 +1,34 @@
+TI EDMA
+
+Required properties:
+- compatible : "ti,edma3"
+- ti,edma-regions: Number of regions
+- ti,edma-slots: Number of slots
+- #dma-cells: Should be set to <1>
+ Clients should use a single channel number per DMA request.
+- dma-channels: Specify total DMA channels per CC
+- reg: Memory map for accessing module
+- interrupt-parent: Interrupt controller the interrupt is routed through
+- interrupts: Exactly 3 interrupts need to be specified in the order:
+ 1. Transfer completion interrupt.
+ 2. Memory protection interrupt.
+ 3. Error interrupt.
+Optional properties:
+- ti,hwmods: Name of the hwmods associated to the EDMA
+- 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-xbar-event-map = <1 12
+ 2 13>;
+};
--
1.7.9.5

2013-06-20 21:07:54

by Joel A Fernandes

[permalink] [raw]
Subject: [PATCH v12 06/11] da8xx: config: Enable MMC and FS options

From: Joel A Fernandes <[email protected]>

Required to get OMAP-L1 EVM access MMC and mount rootfs

Signed-off-by: Joel A Fernandes <[email protected]>
---
arch/arm/configs/da8xx_omapl_defconfig | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/da8xx_omapl_defconfig b/arch/arm/configs/da8xx_omapl_defconfig
index 7c86813..35919ca 100644
--- a/arch/arm/configs/da8xx_omapl_defconfig
+++ b/arch/arm/configs/da8xx_omapl_defconfig
@@ -104,6 +104,7 @@ CONFIG_SND_DAVINCI_SOC=m
# CONFIG_USB_SUPPORT is not set
CONFIG_EXT2_FS=y
CONFIG_EXT3_FS=y
+CONFIG_EXT4_FS=y
CONFIG_XFS_FS=m
CONFIG_INOTIFY=y
CONFIG_AUTOFS4_FS=m
@@ -135,3 +136,5 @@ CONFIG_DEBUG_ERRORS=y
# CONFIG_CRYPTO_HW is not set
CONFIG_CRC_CCITT=m
CONFIG_CRC_T10DIF=m
+CONFIG_MMC=y
+CONFIG_MMC_DAVINCI=y
--
1.7.9.5

2013-06-21 09:38:32

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 02/11] ARM: edma: Add DT and runtime PM support to the private EDMA API

Joel,

Looks like you have not addressed all comments from last time.
For now, in the interest of time, I have fixed them myself. But next
time on, if you are not going to fix any comment, then please reply
upfront before sending out *any* new versions.

Also, *please* run checkpatch and fix all build warnings before you send
the patch out. You seem to have introduced new checkpatch errors and
compile warnings with this version.

On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
> From: Matt Porter <[email protected]>
>
> 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. Enables build on OMAP.
>
> Changes by Joel:
> * Setup default one-to-one mapping for queue_priority and queue_tc
> mapping as discussed in [1].
> * Split out xbar stuff to separate patch. [1]
> * Dropped unused DT helper to convert to array
>
> [1] https://patchwork.kernel.org/patch/2226761/
>
> Signed-off-by: Matt Porter <[email protected]>
> Acked-by: Sekhar Nori <[email protected]>
> Signed-off-by: Joel A Fernandes <[email protected]>
> ---
> arch/arm/common/edma.c | 180 +++++++++++++++++++++++++++++++++---
> include/linux/platform_data/edma.h | 4 +-
> 2 files changed, 169 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
> index 7658874..407e01e 100644
> --- a/arch/arm/common/edma.c
> +++ b/arch/arm/common/edma.c
> @@ -25,6 +25,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>
>
> @@ -1369,13 +1376,102 @@ void edma_clear_event(unsigned channel)
> }
> EXPORT_SYMBOL(edma_clear_event);
>
> -/*-----------------------------------------------------------------------*/
> +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;
> +}

I asked you to get rid of unused functions last time around. This
function is unused here.

> +
> +static int edma_of_parse_dt(struct device *dev,
> + struct device_node *node,
> + struct edma_soc_info *pdata)
> +{
> + int ret = 0, i;
> + u32 value;
> + struct property *prop;
> + size_t sz;

sz and prop are not used in this function and result in unused variable
warnings.

> + struct edma_rsv_info *rsv_info;
> + 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;

n_tc setting is not used in driver as I mentioned last time. I dropped
this line.

> +
> + rsv_info =
> + devm_kzalloc(dev, sizeof(struct edma_rsv_info), GFP_KERNEL);
> + if (!rsv_info)
> + return -ENOMEM;
> + pdata->rsv = rsv_info;
> +
> + queue_tc_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
> + if (!queue_tc_map)
> + return -ENOMEM;
> +
> + for (i = 0; i < 3; i++) {
> + queue_tc_map[i][0] = i;
> + queue_tc_map[i][1] = i;
> + }
> + queue_tc_map[i][0] = -1;
> + queue_tc_map[i][1] = -1;
> +
> + pdata->queue_tc_mapping = queue_tc_map;
> +
> + queue_priority_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
> + if (!queue_priority_map)
> + return -ENOMEM;
> +
> + for (i = 0; i < 3; i++) {
> + queue_priority_map[i][0] = i;
> + queue_priority_map[i][1] = i;
> + }
> + queue_priority_map[i][0] = -1;
> + queue_priority_map[i][1] = -1;
> +
> + pdata->queue_priority_mapping = queue_priority_map;
> +
> + pdata->default_queue = 0;
> +
> + 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];
> - const s8 (*queue_tc_mapping)[2];
> + struct edma_soc_info *ninfo[EDMA_MAX_CC] = {NULL};
> + struct edma_soc_info tmpinfo;
> + s8 (*queue_priority_mapping)[2];
> + s8 (*queue_tc_mapping)[2];
> int i, j, off, ln, found = 0;
> int status = -1;
> const s16 (*rsv_chans)[2];
> @@ -1383,17 +1479,56 @@ static int __init edma_probe(struct platform_device *pdev)
> int irq[EDMA_MAX_CC] = {0, 0};
> int err_irq[EDMA_MAX_CC] = {0, 0};
> struct resource *r[EDMA_MAX_CC] = {NULL};
> + struct resource res[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);

As I mentioned last time, this potentially leaks memory since you need
to return error for the memory allocated through devres APIs to get freed.

> + 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);

It turns out this patch fails to build if CONFIG_DMADEVICES is not
enabled. I fixed that in my version. And no, forcing DMADEVICES to be
on all the time is not right. More on that later.

> + }
>
> if (!info)
> return -ENODEV;
>
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + 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 (!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
> @@ -1440,7 +1575,7 @@ static int __init edma_probe(struct platform_device *pdev)
> off = rsv_chans[i][0];
> ln = rsv_chans[i][1];
> clear_bits(off, ln,
> - edma_cc[j]->edma_unused);
> + edma_cc[j]->edma_unused);
> }
> }
>
> @@ -1456,8 +1591,14 @@ static int __init edma_probe(struct platform_device *pdev)
> }
> }
>
> - sprintf(irq_name, "edma%d", j);
> - irq[j] = platform_get_irq_byname(pdev, irq_name);
> +
> + 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);
> + }

You ended up introducing checkpatch error here..

> edma_cc[j]->irq_res_start = irq[j];
> status = devm_request_irq(&pdev->dev, irq[j],
> dma_irq_handler, 0, "edma",
> @@ -1469,8 +1610,13 @@ static int __init edma_probe(struct platform_device *pdev)
> return status;
> }
>
> - 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);
> + }

.. and here.

> edma_cc[j]->irq_res_end = err_irq[j];
> status = devm_request_irq(&pdev->dev, err_irq[j],
> dma_ccerr_handler, 0,
> @@ -1516,9 +1662,17 @@ static int __init edma_probe(struct platform_device *pdev)
> return 0;
> }
>
> +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/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
> index 2344ea2..317f2be 100644
> --- a/include/linux/platform_data/edma.h
> +++ b/include/linux/platform_data/edma.h
> @@ -175,8 +175,8 @@ struct edma_soc_info {
> /* Resource reservation for other cores */
> struct edma_rsv_info *rsv;
>
> - const s8 (*queue_tc_mapping)[2];
> - const s8 (*queue_priority_mapping)[2];
> + s8 (*queue_tc_mapping)[2];
> + s8 (*queue_priority_mapping)[2];

So the warnings/errors introduced in this patch must be fixed in this
patch only. I merged your 07/11 into this. But that does not solve all
the problems since you ignored non-DA8XX platforms. I fixed those.

Also, Arnd acked the whole series so I added his ack to this.

I have tested the updated on non-DT DM644x board. That means a large
part of the patch is actually untested. Can you please test it and let
me know it works? I will sent the patch as a reply to this mail.

Thanks,
Sekhar

2013-06-21 09:54:30

by Sekhar Nori

[permalink] [raw]
Subject: [PATCH v13] ARM: edma: Add DT and runtime PM support to the private EDMA API

From: Matt Porter <[email protected]>

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. Enables build on OMAP.

Changes by Joel:
* Setup default one-to-one mapping for queue_priority and queue_tc
mapping as discussed in [1].
* Split out xbar stuff to separate patch. [1]
* Dropped unused DT helper to convert to array

[1] https://patchwork.kernel.org/patch/2226761/

Signed-off-by: Matt Porter <[email protected]>
[[email protected]: fix checkpatch errors, build breakages. Introduce
edma_setup_info_from_dt() as part of that effort]
Signed-off-by: Joel A Fernandes <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
Signed-off-by: Sekhar Nori <[email protected]>
---
arch/arm/common/edma.c | 186 +++++++++++++++++++++++++++--
arch/arm/mach-davinci/devices-da8xx.c | 8 +-
arch/arm/mach-davinci/devices-tnetv107x.c | 4 +-
arch/arm/mach-davinci/dm355.c | 4 +-
arch/arm/mach-davinci/dm365.c | 4 +-
arch/arm/mach-davinci/dm644x.c | 4 +-
arch/arm/mach-davinci/dm646x.c | 4 +-
include/linux/platform_data/edma.h | 4 +-
8 files changed, 189 insertions(+), 29 deletions(-)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 7658874..b3770ab 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -25,6 +25,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>

@@ -1369,13 +1376,118 @@ void edma_clear_event(unsigned channel)
}
EXPORT_SYMBOL(edma_clear_event);

-/*-----------------------------------------------------------------------*/
+#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DMADEVICES)

-static int __init edma_probe(struct platform_device *pdev)
+static int edma_of_parse_dt(struct device *dev,
+ struct device_node *node,
+ struct edma_soc_info *pdata)
+{
+ int ret = 0, i;
+ u32 value;
+ struct edma_rsv_info *rsv_info;
+ 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;
+
+ rsv_info = devm_kzalloc(dev, sizeof(struct edma_rsv_info), GFP_KERNEL);
+ if (!rsv_info)
+ return -ENOMEM;
+ pdata->rsv = rsv_info;
+
+ queue_tc_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
+ if (!queue_tc_map)
+ return -ENOMEM;
+
+ for (i = 0; i < 3; i++) {
+ queue_tc_map[i][0] = i;
+ queue_tc_map[i][1] = i;
+ }
+ queue_tc_map[i][0] = -1;
+ queue_tc_map[i][1] = -1;
+
+ pdata->queue_tc_mapping = queue_tc_map;
+
+ queue_priority_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
+ if (!queue_priority_map)
+ return -ENOMEM;
+
+ for (i = 0; i < 3; i++) {
+ queue_priority_map[i][0] = i;
+ queue_priority_map[i][1] = i;
+ }
+ queue_priority_map[i][0] = -1;
+ queue_priority_map[i][1] = -1;
+
+ pdata->queue_priority_mapping = queue_priority_map;
+
+ pdata->default_queue = 0;
+
+ return ret;
+}
+
+static struct of_dma_filter_info edma_filter_info = {
+ .filter_fn = edma_filter_fn,
+};
+
+static struct edma_soc_info **edma_setup_info_from_dt(struct device *dev,
+ struct device_node *node)
+{
+ static struct edma_soc_info **info;
+ struct edma_soc_info *ninfo;
+ int ret;
+
+ /* 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 ERR_PTR(-ENODEV);
+ }
+
+ ninfo = devm_kzalloc(dev, sizeof(struct edma_soc_info), GFP_KERNEL);
+ if (!ninfo)
+ return ERR_PTR(-ENOMEM);
+
+ ret = edma_of_parse_dt(dev, node, ninfo);
+ if (ret)
+ return ERR_PTR(ret);
+
+ dma_cap_set(DMA_SLAVE, edma_filter_info.dma_cap);
+ of_dma_controller_register(dev->of_node, of_dma_simple_xlate,
+ &edma_filter_info);
+
+ info = &ninfo;
+
+ return info;
+}
+#else
+static struct edma_soc_info **edma_setup_info_from_dt(struct device *dev,
+ struct device_node *node)
+{
+ return ERR_PTR(-ENOSYS);
+}
+#endif
+
+static int edma_probe(struct platform_device *pdev)
{
struct edma_soc_info **info = pdev->dev.platform_data;
- const s8 (*queue_priority_mapping)[2];
- const s8 (*queue_tc_mapping)[2];
+ s8 (*queue_priority_mapping)[2];
+ s8 (*queue_tc_mapping)[2];
int i, j, off, ln, found = 0;
int status = -1;
const s16 (*rsv_chans)[2];
@@ -1383,17 +1495,48 @@ static int __init edma_probe(struct platform_device *pdev)
int irq[EDMA_MAX_CC] = {0, 0};
int err_irq[EDMA_MAX_CC] = {0, 0};
struct resource *r[EDMA_MAX_CC] = {NULL};
+ struct resource res[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) {
+ info = edma_setup_info_from_dt(dev, node);
+ if (IS_ERR(info)) {
+ dev_err(dev, "failed to get DT data\n");
+ return PTR_ERR(info);
+ }
+ }

if (!info)
return -ENODEV;

+ pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ 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 (!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
@@ -1440,7 +1583,7 @@ static int __init edma_probe(struct platform_device *pdev)
off = rsv_chans[i][0];
ln = rsv_chans[i][1];
clear_bits(off, ln,
- edma_cc[j]->edma_unused);
+ edma_cc[j]->edma_unused);
}
}

@@ -1456,8 +1599,13 @@ static int __init edma_probe(struct platform_device *pdev)
}
}

- sprintf(irq_name, "edma%d", j);
- irq[j] = platform_get_irq_byname(pdev, irq_name);
+
+ 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 = devm_request_irq(&pdev->dev, irq[j],
dma_irq_handler, 0, "edma",
@@ -1469,8 +1617,12 @@ static int __init edma_probe(struct platform_device *pdev)
return status;
}

- 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 = devm_request_irq(&pdev->dev, err_irq[j],
dma_ccerr_handler, 0,
@@ -1516,9 +1668,17 @@ static int __init edma_probe(struct platform_device *pdev)
return 0;
}

+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/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index e09647c..407d93c 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -133,27 +133,27 @@ struct platform_device da8xx_serial_device[] = {
}
};

-static const s8 da8xx_queue_tc_mapping[][2] = {
+static s8 da8xx_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
{1, 1},
{-1, -1}
};

-static const s8 da8xx_queue_priority_mapping[][2] = {
+static s8 da8xx_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
{1, 7},
{-1, -1}
};

-static const s8 da850_queue_tc_mapping[][2] = {
+static s8 da850_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
{-1, -1}
};

-static const s8 da850_queue_priority_mapping[][2] = {
+static s8 da850_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
{-1, -1}
diff --git a/arch/arm/mach-davinci/devices-tnetv107x.c b/arch/arm/mach-davinci/devices-tnetv107x.c
index 04b1ae5..fc4a0fe 100644
--- a/arch/arm/mach-davinci/devices-tnetv107x.c
+++ b/arch/arm/mach-davinci/devices-tnetv107x.c
@@ -58,14 +58,14 @@
#define TNETV107X_DMACH_SDIO1_RX 28
#define TNETV107X_DMACH_SDIO1_TX 29

-static const s8 edma_tc_mapping[][2] = {
+static s8 edma_tc_mapping[][2] = {
/* event queue no TC no */
{ 0, 0 },
{ 1, 1 },
{ -1, -1 }
};

-static const s8 edma_priority_mapping[][2] = {
+static s8 edma_priority_mapping[][2] = {
/* event queue no Prio */
{ 0, 3 },
{ 1, 7 },
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index ad520d7..29fdbae 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -569,7 +569,7 @@ static u8 dm355_default_priorities[DAVINCI_N_AINTC_IRQ] = {

/*----------------------------------------------------------------------*/

-static const s8
+static s8
queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
@@ -577,7 +577,7 @@ queue_tc_mapping[][2] = {
{-1, -1},
};

-static const s8
+static s8
queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index b4230b1..b601189 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -826,7 +826,7 @@ static u8 dm365_default_priorities[DAVINCI_N_AINTC_IRQ] = {
};

/* Four Transfer Controllers on DM365 */
-static const s8
+static s8
dm365_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
@@ -836,7 +836,7 @@ dm365_queue_tc_mapping[][2] = {
{-1, -1},
};

-static const s8
+static s8
dm365_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 7},
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index 17d5459..490eb8c 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -497,7 +497,7 @@ static u8 dm644x_default_priorities[DAVINCI_N_AINTC_IRQ] = {

/*----------------------------------------------------------------------*/

-static const s8
+static s8
queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
@@ -505,7 +505,7 @@ queue_tc_mapping[][2] = {
{-1, -1},
};

-static const s8
+static s8
queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index 48ece86..23609b1 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -531,7 +531,7 @@ static u8 dm646x_default_priorities[DAVINCI_N_AINTC_IRQ] = {
/*----------------------------------------------------------------------*/

/* Four Transfer Controllers on DM646x */
-static const s8
+static s8
dm646x_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
@@ -541,7 +541,7 @@ dm646x_queue_tc_mapping[][2] = {
{-1, -1},
};

-static const s8
+static s8
dm646x_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 4},
diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index 2344ea2..317f2be 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -175,8 +175,8 @@ struct edma_soc_info {
/* Resource reservation for other cores */
struct edma_rsv_info *rsv;

- const s8 (*queue_tc_mapping)[2];
- const s8 (*queue_priority_mapping)[2];
+ s8 (*queue_tc_mapping)[2];
+ s8 (*queue_priority_mapping)[2];
};

#endif
--
1.7.10.1

2013-06-21 10:13:25

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 03/11] ARM: edma: Add EDMA crossbar event mux support

On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
> From: Matt Porter <[email protected]>
>
>

You should add a description apart from just documenting what you
changed in this patch.

Changes by Joel:
> * Split EDMA xbar support out of original EDMA DT parsing patch
> to keep it easier for review.
> * Rewrite shift and offset calculation.
>
> Suggested-by: Sekhar Nori <[email protected]>
> Suggested by: Andy Shevchenko <[email protected]>
> Signed-off-by: Joel A Fernandes <[email protected]>
>
> Reference:
> [1] https://patchwork.kernel.org/patch/2226991/
> ---
> arch/arm/common/edma.c | 62 +++++++++++++++++++++++++++++++++++-
> include/linux/platform_data/edma.h | 1 +
> 2 files changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
> index 407e01e..a03d4f0 100644
> --- a/arch/arm/common/edma.c
> +++ b/arch/arm/common/edma.c
> @@ -568,7 +568,7 @@ static int prepare_unused_channel_list(struct device *dev, void *data)
> (int)pdev->resource[i].start >= 0) {
> ctlr = EDMA_CTLR(pdev->resource[i].start);
> clear_bit(EDMA_CHAN_SLOT(pdev->resource[i].start),
> - edma_cc[ctlr]->edma_unused);
> + edma_cc[ctlr]->edma_unused);

There are two problems with this hunk:

1) It does not belong to this patch. I said that earlier.
2) It actually introduces a new checkpatch issue.

I dropped this hunk.

> }
> }
>
> @@ -1393,6 +1393,52 @@ static int edma_of_read_u32_to_s16_array(const struct device_node *np,
> 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;

No need to initialize ret here since you use it only after init.
I fixed that.

> + int i;
> + struct resource res;
> + void __iomem *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 (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 (ret)
> + return -EIO;
> +
> + for (i = 0; xbar_chans[i][0] != -1; i++) {
> + shift = (xbar_chans[i][1] & 0x03) << 3;
> + offset = xbar_chans[i][1] & 0xfffffffc;
> + mux = readl(xbar + offset);
> + mux &= ~(0xff << shift);
> + mux |= xbar_chans[i][0] << shift;
> + writel(mux, (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)
> @@ -1458,6 +1504,10 @@ static int edma_of_parse_dt(struct device *dev,
>
> pdata->default_queue = 0;
>
> + prop = of_find_property(node, "ti,edma-xbar-event-map", &sz);
> + if (prop)
> + ret = edma_xbar_event_map(dev, node, pdata, sz);
> +
> return ret;
> }
>
> @@ -1476,6 +1526,7 @@ static int edma_probe(struct platform_device *pdev)
> 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};
> @@ -1591,6 +1642,15 @@ static int edma_probe(struct platform_device *pdev)
> }
> }
>
> + /* 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);

You ignored my request to fix the alignment issue here. I will send the
updated patch separately.

Thanks,
Sekhar

2013-06-21 10:16:15

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 04/11] dmaengine: edma: enable build for AM33XX

On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
> From: Matt Porter <[email protected]>
>
> Enable TI EDMA option on OMAP and TI_PRIV_EDMA
>
> Signed-off-by: Matt Porter <[email protected]>
> Signed-off-by: Joel A Fernandes <[email protected]>

This will have to be taken by Tony.

Thanks,
Sekhar

> ---
> arch/arm/mach-omap2/Kconfig | 1 +
> drivers/dma/Kconfig | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index f49cd51..f91b07f 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -17,6 +17,7 @@ config ARCH_OMAP2PLUS
> select PROC_DEVICETREE if PROC_FS
> select SOC_BUS
> select SPARSE_IRQ
> + select TI_PRIV_EDMA
> select USE_OF
> help
> Systems based on OMAP2, OMAP3, OMAP4 or OMAP5
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index e992489..3215a3c 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -213,7 +213,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
>

2013-06-21 10:16:13

by Sekhar Nori

[permalink] [raw]
Subject: [PATCH] ARM: edma: Add EDMA crossbar event mux support

From: Matt Porter <[email protected]>

EDMA supports a cross bar which provides ability
to mux additional events into physical channels
present in the channel controller.

This is required when the number of events present
in the system are more than number of available
physical channels.

Changes by Joel:
* Split EDMA xbar support out of original EDMA DT parsing patch
to keep it easier for review.
* Rewrite shift and offset calculation.

Suggested-by: Sekhar Nori <[email protected]>
Suggested by: Andy Shevchenko <[email protected]>
Signed-off-by: Joel A Fernandes <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
[[email protected]: fix checkpatch errors and a minor coding improvement]
Signed-off-by: Sekhar Nori <[email protected]>
---
arch/arm/common/edma.c | 78 ++++++++++++++++++++++++++++++++++++
include/linux/platform_data/edma.h | 1 +
2 files changed, 79 insertions(+)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index b3770ab..2b591b1 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1378,12 +1378,76 @@ EXPORT_SYMBOL(edma_clear_event);

#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DMADEVICES)

+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, i;
+ struct resource res;
+ void __iomem *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 (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 (ret)
+ return -EIO;
+
+ for (i = 0; xbar_chans[i][0] != -1; i++) {
+ shift = (xbar_chans[i][1] & 0x03) << 3;
+ offset = xbar_chans[i][1] & 0xfffffffc;
+ mux = readl(xbar + offset);
+ mux &= ~(0xff << shift);
+ mux |= xbar_chans[i][0] << shift;
+ writel(mux, (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, i;
u32 value;
+ struct property *prop;
+ size_t sz;
struct edma_rsv_info *rsv_info;
s8 (*queue_tc_map)[2], (*queue_priority_map)[2];

@@ -1439,6 +1503,10 @@ static int edma_of_parse_dt(struct device *dev,

pdata->default_queue = 0;

+ prop = of_find_property(node, "ti,edma-xbar-event-map", &sz);
+ if (prop)
+ ret = edma_xbar_event_map(dev, node, pdata, sz);
+
return ret;
}

@@ -1492,6 +1560,7 @@ static int edma_probe(struct platform_device *pdev)
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};
@@ -1599,6 +1668,15 @@ static int edma_probe(struct platform_device *pdev)
}
}

+ /* 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);
diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index 317f2be..57300fd 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -177,6 +177,7 @@ struct edma_soc_info {

s8 (*queue_tc_mapping)[2];
s8 (*queue_priority_mapping)[2];
+ const s16 (*xbar_chans)[2];
};

#endif
--
1.7.10.1

2013-06-21 10:16:55

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
> From: Joel A Fernandes <[email protected]>
>
> Build TI_EDMA by default for ARCH_DAVINCI and ARCH_OMAP2PLUS
>
> Signed-off-by: Joel A Fernandes <[email protected]>

You should sign-off with author e-mail address.

> ---
> arch/arm/Kconfig | 1 +
> arch/arm/mach-omap2/Kconfig | 1 +
> drivers/dma/Kconfig | 2 +-
> 3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index b1c66a4..7d58cd9 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -841,6 +841,7 @@ config ARCH_DAVINCI
> select HAVE_IDE
> select NEED_MACH_GPIO_H
> select TI_PRIV_EDMA
> + select DMADEVICES

It is generally a bad idea to force select on something that can be
enabled using menuconfig. Unless used carefully, select causes "unmet
direct dependency" warnings which folks are already fighting hard to
fix. This leads to what Russell referred in the past as "select madness" [1]

In this particular case, it is perfectly okay to have a DaVinci platform
without DMA engine support. Drivers figure out they don't have DMA
support and switch to PIO mode.

Add this in defconfig if its useful for most folks using the platform,
but don't force it for everyone through select.

> select USE_OF
> select ZONE_DMA
> help
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index f91b07f..c02f083 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -17,6 +17,7 @@ config ARCH_OMAP2PLUS
> select PROC_DEVICETREE if PROC_FS
> select SOC_BUS
> select SPARSE_IRQ
> + select DMADEVICES
> select TI_PRIV_EDMA
> select USE_OF
> help
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 3215a3c..b2d6f15 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -216,7 +216,7 @@ config TI_EDMA
> depends on ARCH_DAVINCI || ARCH_OMAP
> select DMA_ENGINE
> select DMA_VIRTUAL_CHANNELS
> - default n
> + default y

Can't see why DMA support should default to y.

Thanks,
Sekhar

[1] http://lkml.org/lkml/2013/3/4/114

2013-06-21 10:27:53

by Sourav Poddar

[permalink] [raw]
Subject: Re: [PATCH v12 09/11] spi: omap2-mcspi: convert to dma_request_slave_channel_compat()

Hi Mark,
On Friday 21 June 2013 02:36 AM, Joel A Fernandes wrote:
> From: Matt Porter<[email protected]>
>
> 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<[email protected]>
> Acked-by: Mark Brown<[email protected]>
> Signed-off-by: Joel A Fernandes<[email protected]>
> ---
> drivers/spi/spi-omap2-mcspi.c | 64 ++++++++++++++++++++++++++++-------------
> 1 file changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
> index 86d2158..ca4ab78 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
> @@ -830,12 +833,20 @@ 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)
> goto no_dma;
>
> 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) {
> dma_release_channel(mcspi_dma->dma_rx);
> mcspi_dma->dma_rx = NULL;
> @@ -1256,29 +1267,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)
Acked-by: Sourav Poddar <[email protected]>
Tested-by: Sourav Poddar <[email protected]>

This patch can go independently and does not depend on the rest of the
series.
Can these patch be pulled?


2013-06-21 10:28:08

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

Joel,

On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
> From: Joel A Fernandes <[email protected]>
>
> This series is remaining of Matt Porter's EDMA patches for AM33XX EDMA support
> with changes for few pending review comments on v11 series.

I have posted a branch with fixes for patches accepted by me in this
series (also posted inline as replies to specific patches).

https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=v3.11/soc-2

Can you please give it a good test - especially on AM335x? I will send a
pull request after you confirm. If you find bugs please send incremental
patches so I can merge into the original patch.

Thanks,
Sekhar

2013-06-21 10:36:39

by Sourav Poddar

[permalink] [raw]
Subject: Re: [PATCH v12 08/11] spi: omap2-mcspi: add generic DMA request support to the DT binding

Hi Benoit,
On Friday 21 June 2013 02:36 AM, Joel A Fernandes wrote:
> From: Matt Porter<[email protected]>
>
> The binding definition is based on the generic DMA request binding
>
> Signed-off-by: Matt Porter<[email protected]>
> Signed-off-by: Joel A Fernandes<[email protected]>
> ---
> 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";
> +};
If the patch looks good to you, these can go independently in your
tree.

Reviewed-by: Sourav Poddar <[email protected]>

~Sourav

2013-06-21 10:38:48

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 09/11] spi: omap2-mcspi: convert to dma_request_slave_channel_compat()

Fixing Mark's e-mail.

Mark,

On 6/21/2013 3:56 PM, Sourav Poddar wrote:
> Hi Mark,

> On Friday 21 June 2013 02:36 AM, Joel A Fernandes wrote:
>> From: Matt Porter<[email protected]>
>>
>> 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<[email protected]>
>> Acked-by: Mark Brown<[email protected]>
>> Signed-off-by: Joel A Fernandes<[email protected]>

[...]

> Acked-by: Sourav Poddar <[email protected]>
> Tested-by: Sourav Poddar <[email protected]>
>
> This patch can go independently and does not depend on the rest of the
> series.
> Can these patch be pulled?

We can resend the patch if you don't have it from the mailing list.

Thanks,
Sekhar

2013-06-21 11:29:03

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v12 09/11] spi: omap2-mcspi: convert to dma_request_slave_channel_compat()

On Fri, Jun 21, 2013 at 04:07:51PM +0530, Sekhar Nori wrote:

> We can resend the patch if you don't have it from the mailing list.

I'll probably have it assuming it's been sent to some mailing list I
read (the CC list here looks absurldy large...) but if you don't send me
the patch and/or ignore bounces then it's going to take longer.


Attachments:
(No filename) (339.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-06-21 12:23:41

by Sourav Poddar

[permalink] [raw]
Subject: Re: [PATCH v12 09/11] spi: omap2-mcspi: convert to dma_request_slave_channel_compat()

Hi Mark,
On Friday 21 June 2013 04:58 PM, Mark Brown wrote:
> On Fri, Jun 21, 2013 at 04:07:51PM +0530, Sekhar Nori wrote:
>
>> We can resend the patch if you don't have it from the mailing list.
> I'll probably have it assuming it's been sent to some mailing list I
> read (the CC list here looks absurldy large...) but if you don't send me
> the patch and/or ignore bounces then it's going to take longer.
I have send you this patch seperately.

Thanks ,
Sourav

2013-06-21 13:38:01

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 02/11] ARM: edma: Add DT and runtime PM support to the private EDMA API

On Fri, Jun 21, 2013 at 4:37 AM, Sekhar Nori <[email protected]> wrote:
> Joel,
>
> Looks like you have not addressed all comments from last time.
> For now, in the interest of time, I have fixed them myself. But next
> time on, if you are not going to fix any comment, then please reply
> upfront before sending out *any* new versions.

Will do. I thought there was just 1 other disagreement. Neverthless
I will make it a point to reply upfront next time.

> Also, *please* run checkpatch and fix all build warnings before you send
> the patch out. You seem to have introduced new checkpatch errors and
> compile warnings with this version.

Ok, this really sucks. Somehow I missed the new checkpatch error.
Thanks for fixing it & Sorry.

Joel

> On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
>> From: Matt Porter <[email protected]>
>>
>> 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. Enables build on OMAP.
>>
>> Changes by Joel:
>> * Setup default one-to-one mapping for queue_priority and queue_tc
>> mapping as discussed in [1].
>> * Split out xbar stuff to separate patch. [1]
>> * Dropped unused DT helper to convert to array
>>
>> [1] https://patchwork.kernel.org/patch/2226761/
>>
>> Signed-off-by: Matt Porter <[email protected]>
>> Acked-by: Sekhar Nori <[email protected]>
>> Signed-off-by: Joel A Fernandes <[email protected]>
>> ---
>> arch/arm/common/edma.c | 180 +++++++++++++++++++++++++++++++++---
>> include/linux/platform_data/edma.h | 4 +-
>> 2 files changed, 169 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
>> index 7658874..407e01e 100644
>> --- a/arch/arm/common/edma.c
>> +++ b/arch/arm/common/edma.c
>> @@ -25,6 +25,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>
>>
>> @@ -1369,13 +1376,102 @@ void edma_clear_event(unsigned channel)
>> }
>> EXPORT_SYMBOL(edma_clear_event);
>>
>> -/*-----------------------------------------------------------------------*/
>> +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;
>> +}
>
> I asked you to get rid of unused functions last time around. This
> function is unused here.
>
>> +
>> +static int edma_of_parse_dt(struct device *dev,
>> + struct device_node *node,
>> + struct edma_soc_info *pdata)
>> +{
>> + int ret = 0, i;
>> + u32 value;
>> + struct property *prop;
>> + size_t sz;
>
> sz and prop are not used in this function and result in unused variable
> warnings.
>
>> + struct edma_rsv_info *rsv_info;
>> + 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;
>
> n_tc setting is not used in driver as I mentioned last time. I dropped
> this line.
>
>> +
>> + rsv_info =
>> + devm_kzalloc(dev, sizeof(struct edma_rsv_info), GFP_KERNEL);
>> + if (!rsv_info)
>> + return -ENOMEM;
>> + pdata->rsv = rsv_info;
>> +
>> + queue_tc_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
>> + if (!queue_tc_map)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < 3; i++) {
>> + queue_tc_map[i][0] = i;
>> + queue_tc_map[i][1] = i;
>> + }
>> + queue_tc_map[i][0] = -1;
>> + queue_tc_map[i][1] = -1;
>> +
>> + pdata->queue_tc_mapping = queue_tc_map;
>> +
>> + queue_priority_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
>> + if (!queue_priority_map)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < 3; i++) {
>> + queue_priority_map[i][0] = i;
>> + queue_priority_map[i][1] = i;
>> + }
>> + queue_priority_map[i][0] = -1;
>> + queue_priority_map[i][1] = -1;
>> +
>> + pdata->queue_priority_mapping = queue_priority_map;
>> +
>> + pdata->default_queue = 0;
>> +
>> + 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];
>> - const s8 (*queue_tc_mapping)[2];
>> + struct edma_soc_info *ninfo[EDMA_MAX_CC] = {NULL};
>> + struct edma_soc_info tmpinfo;
>> + s8 (*queue_priority_mapping)[2];
>> + s8 (*queue_tc_mapping)[2];
>> int i, j, off, ln, found = 0;
>> int status = -1;
>> const s16 (*rsv_chans)[2];
>> @@ -1383,17 +1479,56 @@ static int __init edma_probe(struct platform_device *pdev)
>> int irq[EDMA_MAX_CC] = {0, 0};
>> int err_irq[EDMA_MAX_CC] = {0, 0};
>> struct resource *r[EDMA_MAX_CC] = {NULL};
>> + struct resource res[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);
>
> As I mentioned last time, this potentially leaks memory since you need
> to return error for the memory allocated through devres APIs to get freed.
>
>> + 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);
>
> It turns out this patch fails to build if CONFIG_DMADEVICES is not
> enabled. I fixed that in my version. And no, forcing DMADEVICES to be
> on all the time is not right. More on that later.
>
>> + }
>>
>> if (!info)
>> return -ENODEV;
>>
>> + pm_runtime_enable(dev);
>> + ret = pm_runtime_get_sync(dev);
>> + if (ret < 0) {
>> + 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 (!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
>> @@ -1440,7 +1575,7 @@ static int __init edma_probe(struct platform_device *pdev)
>> off = rsv_chans[i][0];
>> ln = rsv_chans[i][1];
>> clear_bits(off, ln,
>> - edma_cc[j]->edma_unused);
>> + edma_cc[j]->edma_unused);
>> }
>> }
>>
>> @@ -1456,8 +1591,14 @@ static int __init edma_probe(struct platform_device *pdev)
>> }
>> }
>>
>> - sprintf(irq_name, "edma%d", j);
>> - irq[j] = platform_get_irq_byname(pdev, irq_name);
>> +
>> + 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);
>> + }
>
> You ended up introducing checkpatch error here..
>
>> edma_cc[j]->irq_res_start = irq[j];
>> status = devm_request_irq(&pdev->dev, irq[j],
>> dma_irq_handler, 0, "edma",
>> @@ -1469,8 +1610,13 @@ static int __init edma_probe(struct platform_device *pdev)
>> return status;
>> }
>>
>> - 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);
>> + }
>
> .. and here.
>
>> edma_cc[j]->irq_res_end = err_irq[j];
>> status = devm_request_irq(&pdev->dev, err_irq[j],
>> dma_ccerr_handler, 0,
>> @@ -1516,9 +1662,17 @@ static int __init edma_probe(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +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/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
>> index 2344ea2..317f2be 100644
>> --- a/include/linux/platform_data/edma.h
>> +++ b/include/linux/platform_data/edma.h
>> @@ -175,8 +175,8 @@ struct edma_soc_info {
>> /* Resource reservation for other cores */
>> struct edma_rsv_info *rsv;
>>
>> - const s8 (*queue_tc_mapping)[2];
>> - const s8 (*queue_priority_mapping)[2];
>> + s8 (*queue_tc_mapping)[2];
>> + s8 (*queue_priority_mapping)[2];
>
> So the warnings/errors introduced in this patch must be fixed in this
> patch only. I merged your 07/11 into this. But that does not solve all
> the problems since you ignored non-DA8XX platforms. I fixed those.
>
> Also, Arnd acked the whole series so I added his ack to this.
>
> I have tested the updated on non-DT DM644x board. That means a large
> part of the patch is actually untested. Can you please test it and let
> me know it works? I will sent the patch as a reply to this mail.
>
> Thanks,
> Sekhar
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-06-21 13:52:16

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Fri, Jun 21, 2013 at 5:16 AM, Sekhar Nori <[email protected]> wrote:
> On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
>> From: Joel A Fernandes <[email protected]>
>>
>> Build TI_EDMA by default for ARCH_DAVINCI and ARCH_OMAP2PLUS
>>
>> Signed-off-by: Joel A Fernandes <[email protected]>
>
> You should sign-off with author e-mail address.
>
>> ---
>> arch/arm/Kconfig | 1 +
>> arch/arm/mach-omap2/Kconfig | 1 +
>> drivers/dma/Kconfig | 2 +-
>> 3 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index b1c66a4..7d58cd9 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -841,6 +841,7 @@ config ARCH_DAVINCI
>> select HAVE_IDE
>> select NEED_MACH_GPIO_H
>> select TI_PRIV_EDMA
>> + select DMADEVICES
>
> It is generally a bad idea to force select on something that can be
> enabled using menuconfig. Unless used carefully, select causes "unmet
> direct dependency" warnings which folks are already fighting hard to
> fix. This leads to what Russell referred in the past as "select madness" [1]

Are you concerned with bloat issues? I know your point of view but the idea
was to build these options by default for these platforms even though
in some cases
it might not be used. I have seen folks including myself select the wrong
option. Having the build system automatically select the correct option for the
most common cases can be very useful I feel and not require manual
configuration.

> In this particular case, it is perfectly okay to have a DaVinci platform
> without DMA engine support. Drivers figure out they don't have DMA
> support and switch to PIO mode.

Drivers can still use PIO mode if they wish even though DMA is built into the
kernel right.

>> select USE_OF
>> select ZONE_DMA
>> help
>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
>> index f91b07f..c02f083 100644
>> --- a/arch/arm/mach-omap2/Kconfig
>> +++ b/arch/arm/mach-omap2/Kconfig
>> @@ -17,6 +17,7 @@ config ARCH_OMAP2PLUS
>> select PROC_DEVICETREE if PROC_FS
>> select SOC_BUS
>> select SPARSE_IRQ
>> + select DMADEVICES
>> select TI_PRIV_EDMA
>> select USE_OF
>> help
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index 3215a3c..b2d6f15 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -216,7 +216,7 @@ config TI_EDMA
>> depends on ARCH_DAVINCI || ARCH_OMAP
>> select DMA_ENGINE
>> select DMA_VIRTUAL_CHANNELS
>> - default n
>> + default y
>
> Can't see why DMA support should default to y.
>

For same reason as above.

Thanks,
Joel

2013-06-21 14:00:49

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Friday 21 June 2013, Joel A Fernandes wrote:
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index b1c66a4..7d58cd9 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -841,6 +841,7 @@ config ARCH_DAVINCI
> >> select HAVE_IDE
> >> select NEED_MACH_GPIO_H
> >> select TI_PRIV_EDMA
> >> + select DMADEVICES
> >
> > It is generally a bad idea to force select on something that can be
> > enabled using menuconfig. Unless used carefully, select causes "unmet
> > direct dependency" warnings which folks are already fighting hard to
> > fix. This leads to what Russell referred in the past as "select madness" [1]
>
> Are you concerned with bloat issues? I know your point of view but the idea
> was to build these options by default for these platforms even though
> in some cases
> it might not be used. I have seen folks including myself select the wrong
> option. Having the build system automatically select the correct option for the
> most common cases can be very useful I feel and not require manual
> configuration.

For defaults, you should use the defconfig, not 'select' in Kconfig.

A lot of the 'select' statements are actually wrong because they
do not take dependencies into account where A selects B but not C,
and B depends on C, which leads to broken builds when C is disabled
by a user (or randconfig).

You should only ever use 'select' from platforms on silent options
that are not themselves user selectable like the above 'HAVE_IDE'
and 'NEED_MACH_GPIO_H'.

Arnd

2013-06-21 14:20:05

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Fri, Jun 21, 2013 at 9:00 AM, Arnd Bergmann <[email protected]> wrote:
> On Friday 21 June 2013, Joel A Fernandes wrote:
>> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> >> index b1c66a4..7d58cd9 100644
>> >> --- a/arch/arm/Kconfig
>> >> +++ b/arch/arm/Kconfig
>> >> @@ -841,6 +841,7 @@ config ARCH_DAVINCI
>> >> select HAVE_IDE
>> >> select NEED_MACH_GPIO_H
>> >> select TI_PRIV_EDMA
>> >> + select DMADEVICES
>> >
>> > It is generally a bad idea to force select on something that can be
>> > enabled using menuconfig. Unless used carefully, select causes "unmet
>> > direct dependency" warnings which folks are already fighting hard to
>> > fix. This leads to what Russell referred in the past as "select madness" [1]
>>
>> Are you concerned with bloat issues? I know your point of view but the idea
>> was to build these options by default for these platforms even though
>> in some cases
>> it might not be used. I have seen folks including myself select the wrong
>> option. Having the build system automatically select the correct option for the
>> most common cases can be very useful I feel and not require manual
>> configuration.
>
> For defaults, you should use the defconfig, not 'select' in Kconfig.
>
> A lot of the 'select' statements are actually wrong because they
> do not take dependencies into account where A selects B but not C,
> and B depends on C, which leads to broken builds when C is disabled
> by a user (or randconfig).

I haven't come across this problem but- are you saying there is a
shortcoming in Kbuild/Kconfig that selects an option even if its
dependency is not met?

The problem with defconfig is also too many options I feel for a common case.
CONFIG_DMADEVICES=y
CONFIG_TI_EDMA=y

Most if not all future OMAPs from will use EDMA. Why not we can be
explicit about it and just built it in anyway. If ARCH_OMAP and
DMADEVICES are selected, then we can just build EDMA in by default.

I agree maybe the option can be dropped from Davinci but I suggest
let's keep it for OMAP. Is that ok?

Thanks,
Joel

2013-06-21 14:33:19

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Friday 21 June 2013, Joel A Fernandes wrote:
> I haven't come across this problem but- are you saying there is a
> shortcoming in Kbuild/Kconfig that selects an option even if its
> dependency is not met?

Well, the shortcoming is that it lets you specify impossible
contraints. You get a warning from Kconfig when building
such a configuration, but then it continues.

> The problem with defconfig is also too many options I feel for a common case.
> CONFIG_DMADEVICES=y
> CONFIG_TI_EDMA=y
>
> Most if not all future OMAPs from will use EDMA. Why not we can be
> explicit about it and just built it in anyway. If ARCH_OMAP and
> DMADEVICES are selected, then we can just build EDMA in by default.

It's just not how we do things. Kconfig is a mess because we are
not consistent in the way this is done.

> I agree maybe the option can be dropped from Davinci but I suggest
> let's keep it for OMAP. Is that ok?

No, I would still like you to not add it to either one. I'm spending
a lot of my time tracking down incorrect 'select' statements and I'd
rather spend it in a different way. I've had to a number of 'select'
statements from OMAP in the past, please don't add any new ones
unless there is a strict build dependency (which normally should not
exist).

Arnd

2013-06-21 18:40:51

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Fri, Jun 21, 2013 at 9:32 AM, Arnd Bergmann <[email protected]> wrote:
> On Friday 21 June 2013, Joel A Fernandes wrote:
>> I haven't come across this problem but- are you saying there is a
>> shortcoming in Kbuild/Kconfig that selects an option even if its
>> dependency is not met?
>
> Well, the shortcoming is that it lets you specify impossible
> contraints. You get a warning from Kconfig when building
> such a configuration, but then it continues.
>
>> The problem with defconfig is also too many options I feel for a common case.
>> CONFIG_DMADEVICES=y
>> CONFIG_TI_EDMA=y
>>
>> Most if not all future OMAPs from will use EDMA. Why not we can be
>> explicit about it and just built it in anyway. If ARCH_OMAP and
>> DMADEVICES are selected, then we can just build EDMA in by default.
>
> It's just not how we do things. Kconfig is a mess because we are
> not consistent in the way this is done.
>
>> I agree maybe the option can be dropped from Davinci but I suggest
>> let's keep it for OMAP. Is that ok?
>
> No, I would still like you to not add it to either one. I'm spending
> a lot of my time tracking down incorrect 'select' statements and I'd
> rather spend it in a different way. I've had to a number of 'select'
> statements from OMAP in the past, please don't add any new ones
> unless there is a strict build dependency (which normally should not
> exist).

I think we are talking about different things, I agree the 'select
DMADEVICES' can be dropped but lets please keep the default y option
(not adding new select statements, just saying that if someone select
DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
EDMA). This will simply allow people to have a default. Thanks.

Thanks
Joel

2013-06-21 18:44:40

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Friday 21 June 2013, Joel A Fernandes wrote:
> I think we are talking about different things, I agree the 'select
> DMADEVICES' can be dropped but lets please keep the default y option
> (not adding new select statements, just saying that if someone select
> DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
> EDMA). This will simply allow people to have a default. Thanks.

Yes, that's ok.

Also, if the driver doesn't strictly depend on these platforms but
can build on anything, I would write it as

config TI_EDMA
tristate "TI EDMA support"
default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
select DMA_ENGINE
select DMA_VIRTUAL_CHANNELS

Arnd

2013-06-21 21:54:00

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

Hi Arnd,

On Fri, Jun 21, 2013 at 1:44 PM, Arnd Bergmann <[email protected]> wrote:
> On Friday 21 June 2013, Joel A Fernandes wrote:
>> I think we are talking about different things, I agree the 'select
>> DMADEVICES' can be dropped but lets please keep the default y option
>> (not adding new select statements, just saying that if someone select
>> DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
>> EDMA). This will simply allow people to have a default. Thanks.
>
> Yes, that's ok.

Ok, thanks. I will follow up with a patch in my next submissions that builds it.

Perhaps a:
default y if 'ARCH_OMAP2PLUS'

and leave the existing as it is...
default n if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2'

would make most sense to me. Basically EDMA is seen on current and all
new OMAP2PLUS.

>
> config TI_EDMA
> tristate "TI EDMA support"
> default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
> select DMA_ENGINE
> select DMA_VIRTUAL_CHANNELS



MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
'm' option will require some initramfs to load the module when needing
to MMC boot, I suggest lets leave it as y.

Thanks,
Joel

2013-06-21 22:14:28

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Friday 21 June 2013, Joel A Fernandes wrote:
> Hi Arnd,
>
> On Fri, Jun 21, 2013 at 1:44 PM, Arnd Bergmann <[email protected]> wrote:
> > On Friday 21 June 2013, Joel A Fernandes wrote:
> >> I think we are talking about different things, I agree the 'select
> >> DMADEVICES' can be dropped but lets please keep the default y option
> >> (not adding new select statements, just saying that if someone select
> >> DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
> >> EDMA). This will simply allow people to have a default. Thanks.
> >
> > Yes, that's ok.
>
> Ok, thanks. I will follow up with a patch in my next submissions that builds it.
>
> Perhaps a:
> default y if 'ARCH_OMAP2PLUS'
>
> and leave the existing as it is...
> default n if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2'
>
> would make most sense to me. Basically EDMA is seen on current and all
> new OMAP2PLUS.

Ok

> > config TI_EDMA
> > tristate "TI EDMA support"
> > default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
> > select DMA_ENGINE
> > select DMA_VIRTUAL_CHANNELS
>
>
> MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
> 'm' option will require some initramfs to load the module when needing
> to MMC boot, I suggest lets leave it as y.
>

Ah, right: you still export a filter function from the edma driver
and use it in slave drivers:

drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
drivers/spi/spi-davinci.c: dspi->dma_rx = dma_request_channel(mask, edma_filter_fn,
drivers/spi/spi-davinci.c: dspi->dma_tx = dma_request_channel(mask, edma_filter_fn,

As long as this is the case, you have to be careful with the dependencies
to make sure that davinci_mmc and spi-davinci either depend on TI_EDMA, or
edma_filter_fn gets defined to NULL when you are building for a DT-only platform.

Arnd

2013-06-22 00:07:10

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

Hi Sekhar,

On Fri, Jun 21, 2013 at 5:27 AM, Sekhar Nori <[email protected]> wrote:
> Joel,
>
> On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
>> From: Joel A Fernandes <[email protected]>
>>
>> This series is remaining of Matt Porter's EDMA patches for AM33XX EDMA support
>> with changes for few pending review comments on v11 series.
>
> I have posted a branch with fixes for patches accepted by me in this
> series (also posted inline as replies to specific patches).
>
> https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=v3.11/soc-2
>
> Can you please give it a good test - especially on AM335x? I will send a
> pull request after you confirm. If you find bugs please send incremental
> patches so I can merge into the original patch.

This breaks on AM33XX due to dangling pointer, I sent a follow up
patch addressing it.

However, with this branch I still get..
mmc: unable to obtain RX DMA engine channel 25

Is there anything obvious you might have changed that is causing this?

Thanks,
Joel

2013-06-22 02:53:21

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

>> > config TI_EDMA
>> > tristate "TI EDMA support"
>> > default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
>> > select DMA_ENGINE
>> > select DMA_VIRTUAL_CHANNELS
>>
>>
>> MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
>> 'm' option will require some initramfs to load the module when needing
>> to MMC boot, I suggest lets leave it as y.
>>
>
> Ah, right: you still export a filter function from the edma driver
> and use it in slave drivers:
>
> drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
> drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
> drivers/spi/spi-davinci.c: dspi->dma_rx = dma_request_channel(mask, edma_filter_fn,
> drivers/spi/spi-davinci.c: dspi->dma_tx = dma_request_channel(mask, edma_filter_fn,
>
> As long as this is the case, you have to be careful with the dependencies
> to make sure that davinci_mmc and spi-davinci either depend on TI_EDMA, or
> edma_filter_fn gets defined to NULL when you are building for a DT-only platform.

Yes sure, right now they are defined as follows in include/linux/edma.h:

#if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
bool edma_filter_fn(struct dma_chan *, void *);
#else
static inline bool edma_filter_fn(struct dma_chan *chan, void *param)
{
return false;
}
#endif

This also has the side effect of causing DMA requests to fail if
TI_EDMA is not built, causing frustration for a lot of people some of
whom don't want to deal with DMA so I think it is OK to build the
driver in by default as it is (and will be) used by a lot of
OMAP2PLUS.

Thanks,
Joel

2013-06-22 03:36:59

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

On Fri, Jun 21, 2013 at 7:07 PM, Joel A Fernandes <[email protected]> wrote:
> Hi Sekhar,
>
> On Fri, Jun 21, 2013 at 5:27 AM, Sekhar Nori <[email protected]> wrote:
>> Joel,
>>
>> On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
>>> From: Joel A Fernandes <[email protected]>
>>>
>>> This series is remaining of Matt Porter's EDMA patches for AM33XX EDMA support
>>> with changes for few pending review comments on v11 series.
>>
>> I have posted a branch with fixes for patches accepted by me in this
>> series (also posted inline as replies to specific patches).
>>
>> https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=v3.11/soc-2
>>
>> Can you please give it a good test - especially on AM335x? I will send a
>> pull request after you confirm. If you find bugs please send incremental
>> patches so I can merge into the original patch.
>
> [Joel]
> This breaks on AM33XX due to dangling pointer, I sent a follow up
> patch addressing it.
>
> However, with this branch I still get..
> mmc: unable to obtain RX DMA engine channel 25

[Joel]
Fixed. I posted a v2 patch. With this, on v3.11/soc-2 branch - AM33xx
MMC is working (root fs mount).

Tested-by: Joel A Fernandes <[email protected]>

Thanks,
Joel

2013-06-24 10:07:44

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v13] ARM: edma: Add DT and runtime PM support to the private EDMA API

On 6/21/2013 3:23 PM, Sekhar Nori wrote:
> From: Matt Porter <[email protected]>
>
> 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. Enables build on OMAP.
>
> Changes by Joel:
> * Setup default one-to-one mapping for queue_priority and queue_tc
> mapping as discussed in [1].
> * Split out xbar stuff to separate patch. [1]
> * Dropped unused DT helper to convert to array
>
> [1] https://patchwork.kernel.org/patch/2226761/
>
> Signed-off-by: Matt Porter <[email protected]>
> [[email protected]: fix checkpatch errors, build breakages. Introduce
> edma_setup_info_from_dt() as part of that effort]
> Signed-off-by: Joel A Fernandes <[email protected]>
> Acked-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Sekhar Nori <[email protected]>

So here is an updated version of this patch after merging your
fix sent over the weekend . I tested the on AM335x, DA850 and
DM644x boards using MMC/SD as the DMA client. With that I think
this has gotten enough testing now and I plan to send a pull
request for this later today and hope we make it in time.

Thanks,
Sekhar

---8<---
>From 6cba4355066bda19f14d4da66b8abbca0ffdfd59 Mon Sep 17 00:00:00 2001
From: Matt Porter <[email protected]>
Date: Thu, 20 Jun 2013 16:06:38 -0500
Subject: [PATCH 3/4] ARM: edma: Add DT and runtime PM support to the private
EDMA API

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. Enables build on OMAP.

Changes by Joel:
* Setup default one-to-one mapping for queue_priority and queue_tc
mapping as discussed in [1].
* Split out xbar stuff to separate patch. [1]
* Dropped unused DT helper to convert to array
* Fixed dangling pointer issue with Sekhar's changes

[1] https://patchwork.kernel.org/patch/2226761/

Signed-off-by: Matt Porter <[email protected]>
[[email protected]: fix checkpatch errors, build breakages. Introduce
edma_setup_info_from_dt() as part of that effort]
Signed-off-by: Joel A Fernandes <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
Signed-off-by: Sekhar Nori <[email protected]>
---
arch/arm/common/edma.c | 186 +++++++++++++++++++++++++++--
arch/arm/mach-davinci/devices-da8xx.c | 8 +-
arch/arm/mach-davinci/devices-tnetv107x.c | 4 +-
arch/arm/mach-davinci/dm355.c | 4 +-
arch/arm/mach-davinci/dm365.c | 4 +-
arch/arm/mach-davinci/dm644x.c | 4 +-
arch/arm/mach-davinci/dm646x.c | 4 +-
include/linux/platform_data/edma.h | 4 +-
8 files changed, 189 insertions(+), 29 deletions(-)

diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 7658874..5183a31 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -25,6 +25,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>

@@ -1369,13 +1376,110 @@ void edma_clear_event(unsigned channel)
}
EXPORT_SYMBOL(edma_clear_event);

-/*-----------------------------------------------------------------------*/
+#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_DMADEVICES)
+
+static int edma_of_parse_dt(struct device *dev,
+ struct device_node *node,
+ struct edma_soc_info *pdata)
+{
+ int ret = 0, i;
+ u32 value;
+ struct edma_rsv_info *rsv_info;
+ 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;
+
+ rsv_info = devm_kzalloc(dev, sizeof(struct edma_rsv_info), GFP_KERNEL);
+ if (!rsv_info)
+ return -ENOMEM;
+ pdata->rsv = rsv_info;
+
+ queue_tc_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
+ if (!queue_tc_map)
+ return -ENOMEM;
+
+ for (i = 0; i < 3; i++) {
+ queue_tc_map[i][0] = i;
+ queue_tc_map[i][1] = i;
+ }
+ queue_tc_map[i][0] = -1;
+ queue_tc_map[i][1] = -1;
+
+ pdata->queue_tc_mapping = queue_tc_map;
+
+ queue_priority_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
+ if (!queue_priority_map)
+ return -ENOMEM;
+
+ for (i = 0; i < 3; i++) {
+ queue_priority_map[i][0] = i;
+ queue_priority_map[i][1] = i;
+ }
+ queue_priority_map[i][0] = -1;
+ queue_priority_map[i][1] = -1;
+
+ pdata->queue_priority_mapping = queue_priority_map;
+
+ pdata->default_queue = 0;

-static int __init edma_probe(struct platform_device *pdev)
+ return ret;
+}
+
+static struct of_dma_filter_info edma_filter_info = {
+ .filter_fn = edma_filter_fn,
+};
+
+static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
+ struct device_node *node)
+{
+ struct edma_soc_info *info;
+ int ret;
+
+ info = devm_kzalloc(dev, sizeof(struct edma_soc_info), GFP_KERNEL);
+ if (!info)
+ return ERR_PTR(-ENOMEM);
+
+ ret = edma_of_parse_dt(dev, node, info);
+ if (ret)
+ return ERR_PTR(ret);
+
+ dma_cap_set(DMA_SLAVE, edma_filter_info.dma_cap);
+ of_dma_controller_register(dev->of_node, of_dma_simple_xlate,
+ &edma_filter_info);
+
+ return info;
+}
+#else
+static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
+ struct device_node *node)
+{
+ return ERR_PTR(-ENOSYS);
+}
+#endif
+
+static int edma_probe(struct platform_device *pdev)
{
struct edma_soc_info **info = pdev->dev.platform_data;
- const s8 (*queue_priority_mapping)[2];
- const s8 (*queue_tc_mapping)[2];
+ struct edma_soc_info *ninfo[EDMA_MAX_CC] = {NULL};
+ s8 (*queue_priority_mapping)[2];
+ s8 (*queue_tc_mapping)[2];
int i, j, off, ln, found = 0;
int status = -1;
const s16 (*rsv_chans)[2];
@@ -1383,17 +1487,56 @@ static int __init edma_probe(struct platform_device *pdev)
int irq[EDMA_MAX_CC] = {0, 0};
int err_irq[EDMA_MAX_CC] = {0, 0};
struct resource *r[EDMA_MAX_CC] = {NULL};
+ struct resource res[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;
+ }
+
+ ninfo[0] = edma_setup_info_from_dt(dev, node);
+ if (IS_ERR(ninfo[0])) {
+ dev_err(dev, "failed to get DT data\n");
+ return PTR_ERR(ninfo[0]);
+ }
+
+ info = ninfo;
+ }

if (!info)
return -ENODEV;

+ pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
+ if (ret < 0) {
+ 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 (!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
@@ -1440,7 +1583,7 @@ static int __init edma_probe(struct platform_device *pdev)
off = rsv_chans[i][0];
ln = rsv_chans[i][1];
clear_bits(off, ln,
- edma_cc[j]->edma_unused);
+ edma_cc[j]->edma_unused);
}
}

@@ -1456,8 +1599,13 @@ static int __init edma_probe(struct platform_device *pdev)
}
}

- sprintf(irq_name, "edma%d", j);
- irq[j] = platform_get_irq_byname(pdev, irq_name);
+
+ 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 = devm_request_irq(&pdev->dev, irq[j],
dma_irq_handler, 0, "edma",
@@ -1469,8 +1617,12 @@ static int __init edma_probe(struct platform_device *pdev)
return status;
}

- 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 = devm_request_irq(&pdev->dev, err_irq[j],
dma_ccerr_handler, 0,
@@ -1516,9 +1668,17 @@ static int __init edma_probe(struct platform_device *pdev)
return 0;
}

+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/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index bf57252..eb254fe 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -105,27 +105,27 @@ struct platform_device da8xx_serial_device = {
},
};

-static const s8 da8xx_queue_tc_mapping[][2] = {
+static s8 da8xx_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
{1, 1},
{-1, -1}
};

-static const s8 da8xx_queue_priority_mapping[][2] = {
+static s8 da8xx_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
{1, 7},
{-1, -1}
};

-static const s8 da850_queue_tc_mapping[][2] = {
+static s8 da850_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
{-1, -1}
};

-static const s8 da850_queue_priority_mapping[][2] = {
+static s8 da850_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
{-1, -1}
diff --git a/arch/arm/mach-davinci/devices-tnetv107x.c b/arch/arm/mach-davinci/devices-tnetv107x.c
index 612a085..128cb9a 100644
--- a/arch/arm/mach-davinci/devices-tnetv107x.c
+++ b/arch/arm/mach-davinci/devices-tnetv107x.c
@@ -58,14 +58,14 @@
#define TNETV107X_DMACH_SDIO1_RX 28
#define TNETV107X_DMACH_SDIO1_TX 29

-static const s8 edma_tc_mapping[][2] = {
+static s8 edma_tc_mapping[][2] = {
/* event queue no TC no */
{ 0, 0 },
{ 1, 1 },
{ -1, -1 }
};

-static const s8 edma_priority_mapping[][2] = {
+static s8 edma_priority_mapping[][2] = {
/* event queue no Prio */
{ 0, 3 },
{ 1, 7 },
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index 526cf7d..42ef53f 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -569,7 +569,7 @@ static u8 dm355_default_priorities[DAVINCI_N_AINTC_IRQ] = {

/*----------------------------------------------------------------------*/

-static const s8
+static s8
queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
@@ -577,7 +577,7 @@ queue_tc_mapping[][2] = {
{-1, -1},
};

-static const s8
+static s8
queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index c4b7411..fa7af5e 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -826,7 +826,7 @@ static u8 dm365_default_priorities[DAVINCI_N_AINTC_IRQ] = {
};

/* Four Transfer Controllers on DM365 */
-static const s8
+static s8
dm365_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
@@ -836,7 +836,7 @@ dm365_queue_tc_mapping[][2] = {
{-1, -1},
};

-static const s8
+static s8
dm365_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 7},
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index dd156d5..a49d182 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -497,7 +497,7 @@ static u8 dm644x_default_priorities[DAVINCI_N_AINTC_IRQ] = {

/*----------------------------------------------------------------------*/

-static const s8
+static s8
queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
@@ -505,7 +505,7 @@ queue_tc_mapping[][2] = {
{-1, -1},
};

-static const s8
+static s8
queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 3},
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index 6d52a32..d1259e8 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -531,7 +531,7 @@ static u8 dm646x_default_priorities[DAVINCI_N_AINTC_IRQ] = {
/*----------------------------------------------------------------------*/

/* Four Transfer Controllers on DM646x */
-static const s8
+static s8
dm646x_queue_tc_mapping[][2] = {
/* {event queue no, TC no} */
{0, 0},
@@ -541,7 +541,7 @@ dm646x_queue_tc_mapping[][2] = {
{-1, -1},
};

-static const s8
+static s8
dm646x_queue_priority_mapping[][2] = {
/* {event queue no, Priority} */
{0, 4},
diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index 2344ea2..317f2be 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -175,8 +175,8 @@ struct edma_soc_info {
/* Resource reservation for other cores */
struct edma_rsv_info *rsv;

- const s8 (*queue_tc_mapping)[2];
- const s8 (*queue_priority_mapping)[2];
+ s8 (*queue_tc_mapping)[2];
+ s8 (*queue_priority_mapping)[2];
};

#endif
--
1.7.10.1

2013-06-24 10:18:14

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v12 04/11] dmaengine: edma: enable build for AM33XX

* Sekhar Nori <[email protected]> [130621 03:21]:
> On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
> > From: Matt Porter <[email protected]>
> >
> > Enable TI EDMA option on OMAP and TI_PRIV_EDMA
> >
> > Signed-off-by: Matt Porter <[email protected]>
> > Signed-off-by: Joel A Fernandes <[email protected]>
>
> This will have to be taken by Tony.

Sekhar, please go ahead and take this one. I'll reply
to the header email of this series how it should be
queued.

For the mach-omap2/Kconfig change:

Acked-by: Tony Lindgren <[email protected]>

> > ---
> > arch/arm/mach-omap2/Kconfig | 1 +
> > drivers/dma/Kconfig | 2 +-
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index f49cd51..f91b07f 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -17,6 +17,7 @@ config ARCH_OMAP2PLUS
> > select PROC_DEVICETREE if PROC_FS
> > select SOC_BUS
> > select SPARSE_IRQ
> > + select TI_PRIV_EDMA
> > select USE_OF
> > help
> > Systems based on OMAP2, OMAP3, OMAP4 or OMAP5
> > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > index e992489..3215a3c 100644
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
> > @@ -213,7 +213,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
> >

2013-06-24 10:19:14

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

Hi,

For merging this series, I suggest the following sets:

* Joel A Fernandes <[email protected]> [130620 14:13]:
>
> Joel A Fernandes (3):
> edma: config: Enable config options for EDMA
> da8xx: config: Enable MMC and FS options
> ARM: davinci: Fix compiler warnings in devices-da8xx
>
> Matt Porter (8):
> dmaengine: edma: Add TI EDMA device tree binding
> ARM: edma: Add DT and runtime PM support to the private EDMA API
> ARM: edma: Add EDMA crossbar event mux support
> dmaengine: edma: enable build for AM33XX

Sekhar should queue the above patches, I've acked the
mach-omap2/Kconfig change.

> spi: omap2-mcspi: add generic DMA request support to the DT binding
> spi: omap2-mcspi: convert to dma_request_slave_channel_compat()


The spi changes should get merged via the driver list.

> ARM: dts: add AM33XX EDMA support
> ARM: dts: add AM33XX SPI DMA support

Benoit should queue the .dts changes.

> Documentation/devicetree/bindings/dma/ti-edma.txt | 34 +++
> Documentation/devicetree/bindings/spi/omap-spi.txt | 27 ++-
> arch/arm/Kconfig | 1 +
> arch/arm/boot/dts/am33xx.dtsi | 22 ++
> arch/arm/common/edma.c | 242 ++++++++++++++++++--
> arch/arm/configs/da8xx_omapl_defconfig | 3 +
> arch/arm/mach-davinci/devices-da8xx.c | 8 +-
> arch/arm/mach-omap2/Kconfig | 2 +
> drivers/dma/Kconfig | 4 +-
> drivers/spi/spi-omap2-mcspi.c | 64 ++++--
> include/linux/platform_data/edma.h | 5 +-
> 11 files changed, 369 insertions(+), 43 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/dma/ti-edma.txt

Regards,

Tony

2013-06-24 10:32:39

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 04/11] dmaengine: edma: enable build for AM33XX


On 6/24/2013 3:47 PM, Tony Lindgren wrote:
> * Sekhar Nori <[email protected]> [130621 03:21]:
>> On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
>>> From: Matt Porter <[email protected]>
>>>
>>> Enable TI EDMA option on OMAP and TI_PRIV_EDMA
>>>
>>> Signed-off-by: Matt Porter <[email protected]>
>>> Signed-off-by: Joel A Fernandes <[email protected]>
>>
>> This will have to be taken by Tony.
>
> Sekhar, please go ahead and take this one. I'll reply
> to the header email of this series how it should be
> queued.
>
> For the mach-omap2/Kconfig change:
>
> Acked-by: Tony Lindgren <[email protected]>

Thanks Tony. Added to v3.11/soc-2 branch with your and Arnd's acks included.

Vinod, it will be nice to get your ack too for the drivers/dma/Kconfig
change.

Regards,
Sekhar

>
>>> ---
>>> arch/arm/mach-omap2/Kconfig | 1 +
>>> drivers/dma/Kconfig | 2 +-
>>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
>>> index f49cd51..f91b07f 100644
>>> --- a/arch/arm/mach-omap2/Kconfig
>>> +++ b/arch/arm/mach-omap2/Kconfig
>>> @@ -17,6 +17,7 @@ config ARCH_OMAP2PLUS
>>> select PROC_DEVICETREE if PROC_FS
>>> select SOC_BUS
>>> select SPARSE_IRQ
>>> + select TI_PRIV_EDMA
>>> select USE_OF
>>> help
>>> Systems based on OMAP2, OMAP3, OMAP4 or OMAP5
>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>> index e992489..3215a3c 100644
>>> --- a/drivers/dma/Kconfig
>>> +++ b/drivers/dma/Kconfig
>>> @@ -213,7 +213,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
>>>
>
>

2013-06-24 11:24:49

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On 6/22/2013 3:23 AM, Joel A Fernandes wrote:
> Hi Arnd,
>
> On Fri, Jun 21, 2013 at 1:44 PM, Arnd Bergmann <[email protected]> wrote:
>> On Friday 21 June 2013, Joel A Fernandes wrote:
>>> I think we are talking about different things, I agree the 'select
>>> DMADEVICES' can be dropped but lets please keep the default y option
>>> (not adding new select statements, just saying that if someone select
>>> DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
>>> EDMA). This will simply allow people to have a default. Thanks.
>>
>> Yes, that's ok.
>
> Ok, thanks. I will follow up with a patch in my next submissions that builds it.
>
> Perhaps a:
> default y if 'ARCH_OMAP2PLUS'
>
> and leave the existing as it is...
> default n if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2'
>
> would make most sense to me. Basically EDMA is seen on current and all
> new OMAP2PLUS.

OMAP2PLUS devices like OMAP3/4 do not have EDMA so this is not really true.

>
>>
>> config TI_EDMA
>> tristate "TI EDMA support"
>> default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
>> select DMA_ENGINE
>> select DMA_VIRTUAL_CHANNELS
>
>
>
> MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
> 'm' option will require some initramfs to load the module when needing
> to MMC boot, I suggest lets leave it as y.

But there is no reason why it cannot work with PIO, right? Sounds like
the right fix is in driver.

Thanks,
Sekhar

2013-06-24 11:35:29

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On 6/24/2013 4:53 PM, Sekhar Nori wrote:
> On 6/22/2013 3:23 AM, Joel A Fernandes wrote:
>> Hi Arnd,
>>
>> On Fri, Jun 21, 2013 at 1:44 PM, Arnd Bergmann <[email protected]> wrote:
>>> On Friday 21 June 2013, Joel A Fernandes wrote:
>>>> I think we are talking about different things, I agree the 'select
>>>> DMADEVICES' can be dropped but lets please keep the default y option
>>>> (not adding new select statements, just saying that if someone select
>>>> DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
>>>> EDMA). This will simply allow people to have a default. Thanks.
>>>
>>> Yes, that's ok.
>>
>> Ok, thanks. I will follow up with a patch in my next submissions that builds it.
>>
>> Perhaps a:
>> default y if 'ARCH_OMAP2PLUS'
>>
>> and leave the existing as it is...
>> default n if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2'
>>
>> would make most sense to me. Basically EDMA is seen on current and all
>> new OMAP2PLUS.
>
> OMAP2PLUS devices like OMAP3/4 do not have EDMA so this is not really true.

More accurately, there is EDMA hardware on these devices, but its usage
is limited to DSP. ARM uses SDMA instead.

Thanks,
Sekhar

2013-06-24 11:40:36

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

Sourav,

On 6/24/2013 3:49 PM, Tony Lindgren wrote:
> Hi,
>
> For merging this series, I suggest the following sets:
>
> * Joel A Fernandes <[email protected]> [130620 14:13]:

>> spi: omap2-mcspi: add generic DMA request support to the DT binding
>> spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
>
>
> The spi changes should get merged via the driver list.

Can you please send just the DT binding patch above to Mark's correct
address with the relevant lists copied.

Thanks,
Sekhar

2013-06-24 11:49:42

by Sourav Poddar

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

On Monday 24 June 2013 05:09 PM, Sekhar Nori wrote:
> Sourav,
>
> On 6/24/2013 3:49 PM, Tony Lindgren wrote:
>> Hi,
>>
>> For merging this series, I suggest the following sets:
>>
>> * Joel A Fernandes<[email protected]> [130620 14:13]:
>>> spi: omap2-mcspi: add generic DMA request support to the DT binding
>>> spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
>>
>> The spi changes should get merged via the driver list.
> Can you please send just the DT binding patch above to Mark's correct
> address with the relevant lists copied.
>
> Thanks,
> Sekhar
Sure. Will send that.

2013-06-24 11:54:40

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On 6/22/2013 8:23 AM, Joel A Fernandes wrote:
>>>> config TI_EDMA
>>>> tristate "TI EDMA support"
>>>> default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
>>>> select DMA_ENGINE
>>>> select DMA_VIRTUAL_CHANNELS
>>>
>>>
>>> MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
>>> 'm' option will require some initramfs to load the module when needing
>>> to MMC boot, I suggest lets leave it as y.
>>>
>>
>> Ah, right: you still export a filter function from the edma driver
>> and use it in slave drivers:
>>
>> drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
>> drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
>> drivers/spi/spi-davinci.c: dspi->dma_rx = dma_request_channel(mask, edma_filter_fn,
>> drivers/spi/spi-davinci.c: dspi->dma_tx = dma_request_channel(mask, edma_filter_fn,
>>
>> As long as this is the case, you have to be careful with the dependencies
>> to make sure that davinci_mmc and spi-davinci either depend on TI_EDMA, or
>> edma_filter_fn gets defined to NULL when you are building for a DT-only platform.
>
> Yes sure, right now they are defined as follows in include/linux/edma.h:
>
> #if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
> bool edma_filter_fn(struct dma_chan *, void *);
> #else
> static inline bool edma_filter_fn(struct dma_chan *chan, void *param)
> {
> return false;
> }
> #endif
>
> This also has the side effect of causing DMA requests to fail if
> TI_EDMA is not built, causing frustration for a lot of people some of
> whom don't want to deal with DMA so I think it is OK to build the
> driver in by default as it is (and will be) used by a lot of
> OMAP2PLUS.

Solution for this is to enable TI_EDMA in relevant defconfigs. Most
folks who would get frustrated by such issues would be using defconfigs
and for those who are building their configuration from scratch this
will be pretty low in their list of worries.

Thanks,
Sekhar

2013-06-24 14:38:33

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

Hi Tony,

On 06/24/2013 12:19 PM, Tony Lindgren wrote:
> Hi,
>
> For merging this series, I suggest the following sets:
>
> * Joel A Fernandes <[email protected]> [130620 14:13]:
>>
>> Joel A Fernandes (3):
>> edma: config: Enable config options for EDMA
>> da8xx: config: Enable MMC and FS options
>> ARM: davinci: Fix compiler warnings in devices-da8xx
>>
>> Matt Porter (8):
>> dmaengine: edma: Add TI EDMA device tree binding
>> ARM: edma: Add DT and runtime PM support to the private EDMA API
>> ARM: edma: Add EDMA crossbar event mux support
>> dmaengine: edma: enable build for AM33XX
>
> Sekhar should queue the above patches, I've acked the
> mach-omap2/Kconfig change.
>
>> spi: omap2-mcspi: add generic DMA request support to the DT binding
>> spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
>
>
> The spi changes should get merged via the driver list.
>
>> ARM: dts: add AM33XX EDMA support
>> ARM: dts: add AM33XX SPI DMA support
>
> Benoit should queue the .dts changes.

I'll do that, but do you consider them for 3.11?

Thanks,
Benoit

2013-06-24 14:55:28

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Mon, Jun 24, 2013 at 6:53 AM, Sekhar Nori <[email protected]> wrote:
> On 6/22/2013 8:23 AM, Joel A Fernandes wrote:
>>>>> config TI_EDMA
>>>>> tristate "TI EDMA support"
>>>>> default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
>>>>> select DMA_ENGINE
>>>>> select DMA_VIRTUAL_CHANNELS
>>>>
>>>>
>>>> MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
>>>> 'm' option will require some initramfs to load the module when needing
>>>> to MMC boot, I suggest lets leave it as y.
>>>>
>>>
>>> Ah, right: you still export a filter function from the edma driver
>>> and use it in slave drivers:
>>>
>>> drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
>>> drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
>>> drivers/spi/spi-davinci.c: dspi->dma_rx = dma_request_channel(mask, edma_filter_fn,
>>> drivers/spi/spi-davinci.c: dspi->dma_tx = dma_request_channel(mask, edma_filter_fn,
>>>
>>> As long as this is the case, you have to be careful with the dependencies
>>> to make sure that davinci_mmc and spi-davinci either depend on TI_EDMA, or
>>> edma_filter_fn gets defined to NULL when you are building for a DT-only platform.
>>
>> Yes sure, right now they are defined as follows in include/linux/edma.h:
>>
>> #if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
>> bool edma_filter_fn(struct dma_chan *, void *);
>> #else
>> static inline bool edma_filter_fn(struct dma_chan *chan, void *param)
>> {
>> return false;
>> }
>> #endif
>>
>> This also has the side effect of causing DMA requests to fail if
>> TI_EDMA is not built, causing frustration for a lot of people some of
>> whom don't want to deal with DMA so I think it is OK to build the
>> driver in by default as it is (and will be) used by a lot of
>> OMAP2PLUS.
>
> Solution for this is to enable TI_EDMA in relevant defconfigs. Most
> folks who would get frustrated by such issues would be using defconfigs
> and for those who are building their configuration from scratch this
> will be pretty low in their list of worries.

Yes, it is not in omap2plus_defconfig either. I will post a patch to
add it to the same, and we can ignore the Kconfig defaults and select
patches.

Thanks,
Joel

2013-06-24 20:10:19

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Mon, Jun 24, 2013 at 6:23 AM, Sekhar Nori <[email protected]> wrote:
> On 6/22/2013 3:23 AM, Joel A Fernandes wrote:
>> Hi Arnd,
>>
>> On Fri, Jun 21, 2013 at 1:44 PM, Arnd Bergmann <[email protected]> wrote:
>>> On Friday 21 June 2013, Joel A Fernandes wrote:
>>>> I think we are talking about different things, I agree the 'select
>>>> DMADEVICES' can be dropped but lets please keep the default y option
>>>> (not adding new select statements, just saying that if someone select
>>>> DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
>>>> EDMA). This will simply allow people to have a default. Thanks.
>>>
>>> Yes, that's ok.
>>
>> Ok, thanks. I will follow up with a patch in my next submissions that builds it.
>>
>> Perhaps a:
>> default y if 'ARCH_OMAP2PLUS'
>>
>> and leave the existing as it is...
>> default n if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2'
>>
>> would make most sense to me. Basically EDMA is seen on current and all
>> new OMAP2PLUS.
>
> OMAP2PLUS devices like OMAP3/4 do not have EDMA so this is not really true.

Sure. Right now though, and from what I've seen, all future SoCs are
supporting EDMA.

>>
>>>
>>> config TI_EDMA
>>> tristate "TI EDMA support"
>>> default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
>>> select DMA_ENGINE
>>> select DMA_VIRTUAL_CHANNELS
>>
>>
>>
>> MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
>> 'm' option will require some initramfs to load the module when needing
>> to MMC boot, I suggest lets leave it as y.
>
> But there is no reason why it cannot work with PIO, right? Sounds like
> the right fix is in driver.

I am not sure about this. I agree no reason it cannot do PIO. I will check.
But even if it did, loading EDMA as a module will require omap_hsmmc to switch
to DMA mode which I am sure the driver doesn't support today.

Thanks,
Joel

2013-06-24 20:29:14

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Saturday 22 June 2013, Joel A Fernandes wrote:
>
> >> > config TI_EDMA
> >> > tristate "TI EDMA support"
> >> > default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
> >> > select DMA_ENGINE
> >> > select DMA_VIRTUAL_CHANNELS
> >>
> >>
> >> MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
> >> 'm' option will require some initramfs to load the module when needing
> >> to MMC boot, I suggest lets leave it as y.
> >>
> >
> > Ah, right: you still export a filter function from the edma driver
> > and use it in slave drivers:
> >
> > drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
> > drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
> > drivers/spi/spi-davinci.c: dspi->dma_rx = dma_request_channel(mask, edma_filter_fn,
> > drivers/spi/spi-davinci.c: dspi->dma_tx = dma_request_channel(mask, edma_filter_fn,
> >
> > As long as this is the case, you have to be careful with the dependencies
> > to make sure that davinci_mmc and spi-davinci either depend on TI_EDMA, or
> > edma_filter_fn gets defined to NULL when you are building for a DT-only platform.
>
> Yes sure, right now they are defined as follows in include/linux/edma.h:
>
> #if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
> bool edma_filter_fn(struct dma_chan *, void *);
> #else
> static inline bool edma_filter_fn(struct dma_chan *chan, void *param)
> {
> return false;
> }
> #endif

It's best to just define the filter function in the platform
code and pass a pointer to it through platform data. The way you do
it above makes the slave drivers inherently nonportable.

Arnd

2013-06-24 20:32:14

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Mon, Jun 24, 2013 at 3:28 PM, Arnd Bergmann <[email protected]> wrote:
> On Saturday 22 June 2013, Joel A Fernandes wrote:
>>
>> >> > config TI_EDMA
>> >> > tristate "TI EDMA support"
>> >> > default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
>> >> > select DMA_ENGINE
>> >> > select DMA_VIRTUAL_CHANNELS
>> >>
>> >>
>> >> MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
>> >> 'm' option will require some initramfs to load the module when needing
>> >> to MMC boot, I suggest lets leave it as y.
>> >>
>> >
>> > Ah, right: you still export a filter function from the edma driver
>> > and use it in slave drivers:
>> >
>> > drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
>> > drivers/mmc/host/davinci_mmc.c: dma_request_slave_channel_compat(mask, edma_filter_fn,
>> > drivers/spi/spi-davinci.c: dspi->dma_rx = dma_request_channel(mask, edma_filter_fn,
>> > drivers/spi/spi-davinci.c: dspi->dma_tx = dma_request_channel(mask, edma_filter_fn,
>> >
>> > As long as this is the case, you have to be careful with the dependencies
>> > to make sure that davinci_mmc and spi-davinci either depend on TI_EDMA, or
>> > edma_filter_fn gets defined to NULL when you are building for a DT-only platform.
>>
>> Yes sure, right now they are defined as follows in include/linux/edma.h:
>>
>> #if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
>> bool edma_filter_fn(struct dma_chan *, void *);
>> #else
>> static inline bool edma_filter_fn(struct dma_chan *chan, void *param)
>> {
>> return false;
>> }
>> #endif
>
> It's best to just define the filter function in the platform
> code and pass a pointer to it through platform data. The way you do
> it above makes the slave drivers inherently nonportable.

But with DT-only platforms, can you really do that?

Thanks,
Joel

2013-06-24 21:07:52

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

On Monday 24 June 2013, Joel A Fernandes wrote:
> >> Yes sure, right now they are defined as follows in include/linux/edma.h:
> >>
> >> #if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
> >> bool edma_filter_fn(struct dma_chan *, void *);
> >> #else
> >> static inline bool edma_filter_fn(struct dma_chan *chan, void *param)
> >> {
> >> return false;
> >> }
> >> #endif
> >
> > It's best to just define the filter function in the platform
> > code and pass a pointer to it through platform data. The way you do
> > it above makes the slave drivers inherently nonportable.
>
> But with DT-only platforms, can you really do that?

The nice thing about this is that with a DT-only platform, the
filter function will automatically go away: you have no
platform_data, which means that if you are using
dma_request_slave_channel_compat, you just pass NULL as the
filter and the filter-data, same as just calling
dma_request_slave_channel.

Arnd

2013-06-24 21:10:53

by Joel A Fernandes

[permalink] [raw]
Subject: RE: [PATCH v12 05/11] edma: config: Enable config options for EDMA

> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Monday, June 24, 2013 4:08 PM
> To: Joel A Fernandes
> Cc: Nori, Sekhar; Fernandes, Joel A; Tony Lindgren; Matt Porter; Grant Likely;
> Rob Herring; Vinod Koul; Mark Brown; Cousson, Benoit; Russell King; Rob
> Landley; Andrew Morton; Jason Kridner; Koen Kooi; Devicetree Discuss; Linux
> OMAP List; Linux ARM Kernel List; Linux DaVinci Kernel List; Linux Kernel
> Mailing List; Linux Documentation List; Linux MMC List; Linux SPI Devel List
> Subject: Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA
>
> On Monday 24 June 2013, Joel A Fernandes wrote:
> > >> Yes sure, right now they are defined as follows in include/linux/edma.h:
> > >>
> > >> #if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
> bool
> > >> edma_filter_fn(struct dma_chan *, void *); #else static inline bool
> > >> edma_filter_fn(struct dma_chan *chan, void *param) { return false;
> > >> } #endif
> > >
> > > It's best to just define the filter function in the platform code
> > > and pass a pointer to it through platform data. The way you do it
> > > above makes the slave drivers inherently nonportable.
> >
> > But with DT-only platforms, can you really do that?
>
> The nice thing about this is that with a DT-only platform, the filter function will
> automatically go away: you have no platform_data, which means that if you
> are using dma_request_slave_channel_compat, you just pass NULL as the filter
> and the filter-data, same as just calling dma_request_slave_channel.
>

[Joel] Ah yes! Thanks for that. Right, edma_filter_fn is not passed explicitly for DT case.

Thanks,
Joel

2013-06-25 06:55:07

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

* Benoit Cousson <[email protected]> [130624 07:42]:
> Hi Tony,
>
> On 06/24/2013 12:19 PM, Tony Lindgren wrote:
> > Hi,
> >
> > For merging this series, I suggest the following sets:
> >
> > * Joel A Fernandes <[email protected]> [130620 14:13]:
> >>
> >> Joel A Fernandes (3):
> >> edma: config: Enable config options for EDMA
> >> da8xx: config: Enable MMC and FS options
> >> ARM: davinci: Fix compiler warnings in devices-da8xx
> >>
> >> Matt Porter (8):
> >> dmaengine: edma: Add TI EDMA device tree binding
> >> ARM: edma: Add DT and runtime PM support to the private EDMA API
> >> ARM: edma: Add EDMA crossbar event mux support
> >> dmaengine: edma: enable build for AM33XX
> >
> > Sekhar should queue the above patches, I've acked the
> > mach-omap2/Kconfig change.
> >
> >> spi: omap2-mcspi: add generic DMA request support to the DT binding
> >> spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
> >
> >
> > The spi changes should get merged via the driver list.
> >
> >> ARM: dts: add AM33XX EDMA support
> >> ARM: dts: add AM33XX SPI DMA support
> >
> > Benoit should queue the .dts changes.
>
> I'll do that, but do you consider them for 3.11?

Yes right after -rc1 if it makes the DMA work. We almost
certainly need to send few branches of fixes after -rc1 once
we find out what might have broken by the removal of the
hwmod data..

But let's stick to minimal changes for the -rc series
naturally.

Regards,

Tony

2013-06-25 14:17:25

by Sekhar Nori

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

On 6/25/2013 12:24 PM, Tony Lindgren wrote:
> * Benoit Cousson <[email protected]> [130624 07:42]:
>> Hi Tony,
>>
>> On 06/24/2013 12:19 PM, Tony Lindgren wrote:
>>> Hi,
>>>
>>> For merging this series, I suggest the following sets:
>>>
>>> * Joel A Fernandes <[email protected]> [130620 14:13]:
>>>>
>>>> Joel A Fernandes (3):
>>>> edma: config: Enable config options for EDMA
>>>> da8xx: config: Enable MMC and FS options
>>>> ARM: davinci: Fix compiler warnings in devices-da8xx
>>>>
>>>> Matt Porter (8):
>>>> dmaengine: edma: Add TI EDMA device tree binding
>>>> ARM: edma: Add DT and runtime PM support to the private EDMA API
>>>> ARM: edma: Add EDMA crossbar event mux support
>>>> dmaengine: edma: enable build for AM33XX
>>>
>>> Sekhar should queue the above patches, I've acked the
>>> mach-omap2/Kconfig change.
>>>
>>>> spi: omap2-mcspi: add generic DMA request support to the DT binding
>>>> spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
>>>
>>>
>>> The spi changes should get merged via the driver list.
>>>
>>>> ARM: dts: add AM33XX EDMA support
>>>> ARM: dts: add AM33XX SPI DMA support
>>>
>>> Benoit should queue the .dts changes.
>>
>> I'll do that, but do you consider them for 3.11?
>
> Yes right after -rc1 if it makes the DMA work. We almost
> certainly need to send few branches of fixes after -rc1 once
> we find out what might have broken by the removal of the
> hwmod data..

The DMA related changes went into arm-soc yesterday. So it should be
safe to send the DT patches in before the merge window opens. If Tony is
done with sending his pull requests may be Benoit can send to arm-soc
directly?

Thanks,
Sekhar

2013-06-25 14:43:44

by Santosh Shilimkar

[permalink] [raw]
Subject: Re: [PATCH v12 00/11] DMA Engine support for AM33XX

On Tuesday 25 June 2013 10:16 AM, Sekhar Nori wrote:
> On 6/25/2013 12:24 PM, Tony Lindgren wrote:
>> * Benoit Cousson <[email protected]> [130624 07:42]:
>>> Hi Tony,
>>>
>>> On 06/24/2013 12:19 PM, Tony Lindgren wrote:
>>>> Hi,
>>>>
>>>> For merging this series, I suggest the following sets:
>>>>
>>>> * Joel A Fernandes <[email protected]> [130620 14:13]:
>>>>>
>>>>> Joel A Fernandes (3):
>>>>> edma: config: Enable config options for EDMA
>>>>> da8xx: config: Enable MMC and FS options
>>>>> ARM: davinci: Fix compiler warnings in devices-da8xx
>>>>>
>>>>> Matt Porter (8):
>>>>> dmaengine: edma: Add TI EDMA device tree binding
>>>>> ARM: edma: Add DT and runtime PM support to the private EDMA API
>>>>> ARM: edma: Add EDMA crossbar event mux support
>>>>> dmaengine: edma: enable build for AM33XX
>>>>
>>>> Sekhar should queue the above patches, I've acked the
>>>> mach-omap2/Kconfig change.
>>>>
>>>>> spi: omap2-mcspi: add generic DMA request support to the DT binding
>>>>> spi: omap2-mcspi: convert to dma_request_slave_channel_compat()
>>>>
>>>>
>>>> The spi changes should get merged via the driver list.
>>>>
>>>>> ARM: dts: add AM33XX EDMA support
>>>>> ARM: dts: add AM33XX SPI DMA support
>>>>
>>>> Benoit should queue the .dts changes.
>>>
>>> I'll do that, but do you consider them for 3.11?
>>
>> Yes right after -rc1 if it makes the DMA work. We almost
>> certainly need to send few branches of fixes after -rc1 once
>> we find out what might have broken by the removal of the
>> hwmod data..
>
> The DMA related changes went into arm-soc yesterday. So it should be
> safe to send the DT patches in before the merge window opens. If Tony is
> done with sending his pull requests may be Benoit can send to arm-soc
> directly?
>
I was just typing the same thing. Even if Tony send them during rc's,
thats fine.

Regards,
Santosh

Subject: Re: [PATCH v12 10/11] ARM: dts: add AM33XX EDMA support

On 06/20/2013 11:06 PM, Joel A Fernandes wrote:
> From: Matt Porter <[email protected]>
>
> Adds AM33XX EDMA support to the am33xx.dtsi as documented in
> Documentation/devicetree/bindings/dma/ti-edma.txt
>
> Joel: Drop DT entries that are non-hardware-description for now as discussed in [1]
>
> [1] https://patchwork.kernel.org/patch/2226761/
>
> Signed-off-by: Matt Porter <[email protected]>
> Signed-off-by: Joel A Fernandes <[email protected]>

I've been browsing my lkml folders and I haven't seen any later version
of this patch. It seems that other patches from this series have been
applied. I don't see this patch in linux-next as of Friday. Any reason
why this has not been applied yet?

Sebastian