[PATCH 0/4] DMA: PL330 fixes

Hi,

The first patch fixes lockup caused by pl330 driver.  It doesn't add
any dependencies to later patches and can be applied independently.

The second patch fixes PL330 regression on ARM Exynos 4210 revision 0
SOC (used by Universal C210 board).

The third patch adds PL330 MDMA1 controller node to device tree on
ARM Exynos4 SOCs.

The fourth patch adds device tree property for PL330 driver to be used
by controllers that have DMA_MEMCPY capability and updates device tree
files for ARM platforms using PL330.  Since this patch depends on
patches #2-3 it would be probably best to also push it through Kukjin's
tree (with the relevant ACKs added of course).


Bartlomiej Zolnierkiewicz (4):
  DMA: PL330: fix locking in pl330_free_chan_resources()
  ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC
  ARM: dts: exynos4: add node for PL330 MDMA1 controller
  DMA: PL330: add device tree property for DMA_MEMCPY capability

 Documentation/devicetree/bindings/dma/arm-pl330.txt |  1 +
 arch/arm/boot/dts/exynos4.dtsi                      |  7 +++++++
 arch/arm/boot/dts/exynos5250.dtsi                   |  2 ++
 arch/arm/boot/dts/highbank.dts                      |  1 +
 arch/arm/boot/dts/socfpga.dtsi                      |  1 +
 arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts         |  1 +
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts          |  1 +
 arch/arm/mach-exynos/dma.c                          |  5 ++++-
 arch/arm/mach-exynos/include/mach/map.h             |  3 ++-
 arch/arm/mach-exynos/mach-exynos4-dt.c              |  1 +
 drivers/dma/pl330.c                                 | 15 ++++++++++-----
 11 files changed, 31 insertions(+), 7 deletions(-)

-- 
1.8.0

tasklet_kill() may sleep so call it before taking pch->lock.

Fixes following lockup:

[  345.470000] BUG: scheduling while atomic: cat/2383/0x00000002
[  345.470000] Modules linked in:
[  345.470000] [<c0015858>] (unwind_backtrace+0x0/0xfc) from [<c004d980>] (__schedule_bug+0x4c/0x58)
[  345.470000] [<c004d980>] (__schedule_bug+0x4c/0x58) from [<c0360b6c>] (__schedule+0x690/0x6e0)
[  345.470000] [<c0360b6c>] (__schedule+0x690/0x6e0) from [<c004f2b4>] (sys_sched_yield+0x70/0x78)
[  345.470000] [<c004f2b4>] (sys_sched_yield+0x70/0x78) from [<c002acec>] (tasklet_kill+0x34/0x8c)
[  345.470000] [<c002acec>] (tasklet_kill+0x34/0x8c) from [<c01da4cc>] (pl330_free_chan_resources+0x24/0x88)
[  345.470000] [<c01da4cc>] (pl330_free_chan_resources+0x24/0x88) from [<c01d81f4>] (dma_chan_put+0x4c/0x50)
[  345.470000] [<c01d81f4>] (dma_chan_put+0x4c/0x50) from [<c01d82c0>] (dma_release_channel+0x28/0x98)
[...]
[  368.335000] BUG: spinlock lockup suspected on CPU#0, swapper/0/0
[  368.340000]  lock: 0xe52aa04c, .magic: dead4ead, .owner: cat/2383, .owner_cpu: 1
[  368.350000] [<c0015858>] (unwind_backtrace+0x0/0xfc) from [<c01b3d78>] (do_raw_spin_lock+0x194/0x204)
[  368.360000] [<c01b3d78>] (do_raw_spin_lock+0x194/0x204) from [<c0361adc>] (_raw_spin_lock_irqsave+0x20/0x28)
[  368.365000] [<c0361adc>] (_raw_spin_lock_irqsave+0x20/0x28) from [<c01da80c>] (pl330_tasklet+0x2c/0x5a8)
[  368.375000] [<c01da80c>] (pl330_tasklet+0x2c/0x5a8) from [<c002ac04>] (tasklet_action+0xfc/0x114)
[  368.385000] [<c002ac04>] (tasklet_action+0xfc/0x114) from [<c002b204>] (__do_softirq+0xe4/0x19c)
[  368.395000] [<c002b204>] (__do_softirq+0xe4/0x19c) from [<c002b398>] (irq_exit+0x98/0x9c)
[  368.405000] [<c002b398>] (irq_exit+0x98/0x9c) from [<c0013ebc>] (handle_IPI+0x124/0x16c)
[  368.410000] [<c0013ebc>] (handle_IPI+0x124/0x16c) from [<c000857c>] (gic_handle_irq+0x64/0x68)
[  368.420000] [<c000857c>] (gic_handle_irq+0x64/0x68) from [<c000e740>] (__irq_svc+0x40/0x70)
[  368.430000] Exception stack(0xc04a3f00 to 0xc04a3f48)
[  368.435000] 3f00: c04a3f48 00000000 6f9e23e8 00000050 c07492c8 c04a3f48 00000000 c04ccc88
[  368.440000] 3f20: 6f9dbac3 00000050 6f9e23e8 00000050 3b9aca00 c04a3f48 c005cfa4 c02946d4
[  368.450000] 3f40: 60000013 ffffffff
[  368.455000] [<c000e740>] (__irq_svc+0x40/0x70) from [<c02946d4>] (cpuidle_wrap_enter+0x4c/0xa0)
[  368.460000] [<c02946d4>] (cpuidle_wrap_enter+0x4c/0xa0) from [<c02940dc>] (cpuidle_enter_state+0x18/0x68)
[  368.470000] [<c02940dc>] (cpuidle_enter_state+0x18/0x68) from [<c02948e0>] (cpuidle_idle_call+0xac/0xe0)
[  368.480000] [<c02948e0>] (cpuidle_idle_call+0xac/0xe0) from [<c00102f8>] (cpu_idle+0xac/0xf0)
[  368.490000] [<c00102f8>] (cpu_idle+0xac/0xf0) from [<c04796a0>] (start_kernel+0x28c/0x294)

Cc: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Vinod Koul <redacted>
Cc: Tomasz Figa <redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/dma/pl330.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 665668b..db7574b 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c

@@ -2459,10 +2459,10 @@ static void pl330_free_chan_resources(struct dma_chan *chan)
 	struct dma_pl330_chan *pch = to_pchan(chan);
 	unsigned long flags;
 
-	spin_lock_irqsave(&pch->lock, flags);
-
 	tasklet_kill(&pch->task);
 
+	spin_lock_irqsave(&pch->lock, flags);
+
 	pl330_release_channel(pch->pl330_chid);
 	pch->pl330_chid = NULL;
 

-- 
1.8.0

Add missing PL330 MDMA1 controller node to the device tree (DT).

[ Currently there is no problem with using 'non-secure' mdma1 address
  instead of 'secure' one on revision 0 of Exynos4210 SOC (as used by
  Universal C210 board) as this SOC revision is unsupported by DT. ]

Cc: Tomasz Figa <redacted>
Cc: Kukjin Kim <redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi         | 6 ++++++
 arch/arm/mach-exynos/mach-exynos4-dt.c | 1 +
 2 files changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index a26c3dd..96d4462 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi

@@ -244,5 +244,11 @@
 			reg = <0x12690000 0x1000>;
 			interrupts = <0 36 0>;
 		};
+
+		mdma1: mdma at 12850000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0x12850000 0x1000>;
+			interrupts = <0 34 0>;
+		};
 	};
 };

diff --git a/arch/arm/mach-exynos/mach-exynos4-dt.c b/arch/arm/mach-exynos/mach-exynos4-dt.c
index eadf4b5..d737968 100644
--- a/arch/arm/mach-exynos/mach-exynos4-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos4-dt.c

@@ -77,6 +77,7 @@ static const struct of_dev_auxdata exynos4_auxdata_lookup[] __initconst = {
 				"exynos4210-spi.2", NULL),
 	OF_DEV_AUXDATA("arm,pl330", EXYNOS4_PA_PDMA0, "dma-pl330.0", NULL),
 	OF_DEV_AUXDATA("arm,pl330", EXYNOS4_PA_PDMA1, "dma-pl330.1", NULL),
+	OF_DEV_AUXDATA("arm,pl330", EXYNOS4_PA_NS_MDMA1, "dma-pl330.2", NULL),
 	{},
 };
 

-- 
1.8.0

* Add device tree (DT) property ("pl330,dma-memcpy") for DMA_MEMCPY
  capability and instead of setting this capability unconditionally
  in pl330_probe() do it only when property is present.

* Set the new "pl330,dma-memcpy" device tree property for all
  current pl330 driver users except pdma controllers on ARM EXYNOS
  platforms (so the DT case matches non-DT one).

It fixes the issue on ARM EXYNOS platforms using DT where pdma
controller erroneously was used for DMA_MEMCPY operations instead
of mdma one (it surprisingly seems to work but at the cost of
worse performance).

Cc: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Vinod Koul <redacted>
Cc: Kukjin Kim <redacted>
Cc: Rob Herring <redacted>
Cc: Dinh Nguyen <redacted>
Cc: Pawel Moll <redacted>
Cc: Tomasz Figa <redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/devicetree/bindings/dma/arm-pl330.txt |  1 +
 arch/arm/boot/dts/exynos4.dtsi                      |  1 +
 arch/arm/boot/dts/exynos5250.dtsi                   |  2 ++
 arch/arm/boot/dts/highbank.dts                      |  1 +
 arch/arm/boot/dts/socfpga.dtsi                      |  1 +
 arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts         |  1 +
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts          |  1 +
 drivers/dma/pl330.c                                 | 11 ++++++++---
 8 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/arm-pl330.txt b/Documentation/devicetree/bindings/dma/arm-pl330.txt
index 36e27d5..2661c7b 100644
--- a/Documentation/devicetree/bindings/dma/arm-pl330.txt
+++ b/Documentation/devicetree/bindings/dma/arm-pl330.txt

@@ -11,6 +11,7 @@ Required properties:
 
 Optional properties:
 - dma-coherent      : Present if dma operations are coherent
+- pl330,dma-memcpy  : Present if controller has DMA_MEMCPY capability
 
 Example:
 

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index 96d4462..ce5b03f 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi

@@ -249,6 +249,7 @@
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x12850000 0x1000>;
 			interrupts = <0 34 0>;
+			pl330,dma-memcpy;
 		};
 	};
 };

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 49546bc..d659e7b 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi

@@ -247,12 +247,14 @@
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x10800000 0x1000>;
 			interrupts = <0 33 0>;
+			pl330,dma-memcpy;
 		};
 
 		mdma1: mdma at 11C10000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x11C10000 0x1000>;
 			interrupts = <0 124 0>;
+			pl330,dma-memcpy;
 		};
 	};
 

diff --git a/arch/arm/boot/dts/highbank.dts b/arch/arm/boot/dts/highbank.dts
index 0c6fc34..87f1d25 100644
--- a/arch/arm/boot/dts/highbank.dts
+++ b/arch/arm/boot/dts/highbank.dts

@@ -297,6 +297,7 @@
 			interrupts = <0 92 4>;
 			clocks = <&pclk>;
 			clock-names = "apb_pclk";
+			pl330,dma-memcpy;
 		};
 
 		ethernet at fff50000 {

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 0772f57..2fe1697 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi

@@ -71,6 +71,7 @@
 				compatible = "arm,pl330", "arm,primecell";
 				reg = <0xffe01000 0x1000>;
 				interrupts = <0 180 4>;
+				pl330,dma-memcpy;
 			};
 		};
 

diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts b/arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts
index d12b34c..d82953c 100644
--- a/arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts
+++ b/arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts

@@ -94,6 +94,7 @@
 			     <0 89 4>,
 			     <0 90 4>,
 			     <0 91 4>;
+		pl330,dma-memcpy;
 	};
 
 	timer {

diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
index 4890a81..b9e6ba2 100644
--- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
+++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts

@@ -114,6 +114,7 @@
 			     <0 89 4>,
 			     <0 90 4>,
 			     <0 91 4>;
+		pl330,dma-memcpy;
 	};
 
 	timer {

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index db7574b..e10290b 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c

@@ -2860,6 +2860,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 	struct pl330_info *pi;
 	struct dma_device *pd;
 	struct resource *res;
+	struct device_node *node;
 	int i, ret, irq;
 	int num_chan;
 

@@ -2921,12 +2922,14 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 		goto probe_err4;
 	}
 
+	node = adev->dev.of_node;
+
 	for (i = 0; i < num_chan; i++) {
 		pch = &pdmac->peripherals[i];
-		if (!adev->dev.of_node)
+		if (!node)
 			pch->chan.private = pdat ? &pdat->peri_id[i] : NULL;
 		else
-			pch->chan.private = adev->dev.of_node;
+			pch->chan.private = node;
 
 		INIT_LIST_HEAD(&pch->work_list);
 		spin_lock_init(&pch->lock);

@@ -2942,7 +2945,9 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 	if (pdat) {
 		pd->cap_mask = pdat->cap_mask;
 	} else {
-		dma_cap_set(DMA_MEMCPY, pd->cap_mask);
+		if (adev->dev.of_node &&
+		    of_find_property(node, "pl330,dma-memcpy", NULL))
+			dma_cap_set(DMA_MEMCPY, pd->cap_mask);
 		if (pi->pcfg.num_peri) {
 			dma_cap_set(DMA_SLAVE, pd->cap_mask);
 			dma_cap_set(DMA_CYCLIC, pd->cap_mask);

-- 
1.8.0

Commit 8214513 ("ARM: EXYNOS: fix address for EXYNOS4 MDMA1")
changed EXYNOS specific setup of PL330 DMA engine to use 'non-secure'
mdma1 address instead of 'secure' one (from 0x12840000 to 0x12850000)
to fix issue with some Exynos4212 SOCs.  Unfortunately it brakes
PL330 setup for revision 0 of Exynos4210 SOC (mdma1 device cannot
be found at 'non-secure' address):

[    0.566245] dma-pl330 dma-pl330.2: PERIPH_ID 0x0, PCELL_ID 0x0 !
[    0.566278] dma-pl330: probe of dma-pl330.2 failed with error -22

Fix it by using 'secure' mdma1 address on Exynos4210 revision 0 SOC.

Cc: Tomasz Figa <redacted>
Cc: Kukjin Kim <redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos/dma.c              | 5 ++++-
 arch/arm/mach-exynos/include/mach/map.h | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-exynos/dma.c b/arch/arm/mach-exynos/dma.c
index 21d568b..dcc9f74 100644
--- a/arch/arm/mach-exynos/dma.c
+++ b/arch/arm/mach-exynos/dma.c

@@ -261,7 +261,7 @@ static struct dma_pl330_platdata exynos_mdma1_pdata = {
 };
 
 static AMBA_AHB_DEVICE(exynos_mdma1,  "dma-pl330.2", 0x00041330,
-	EXYNOS4_PA_MDMA1, {EXYNOS4_IRQ_MDMA1}, &exynos_mdma1_pdata);
+	EXYNOS4_PA_NS_MDMA1, {EXYNOS4_IRQ_MDMA1}, &exynos_mdma1_pdata);
 
 static int __init exynos_dma_init(void)
 {

@@ -275,6 +275,9 @@ static int __init exynos_dma_init(void)
 		exynos_pdma1_pdata.nr_valid_peri =
 			ARRAY_SIZE(exynos4210_pdma1_peri);
 		exynos_pdma1_pdata.peri_id = exynos4210_pdma1_peri;
+
+		if (samsung_rev() == EXYNOS4210_REV_0)
+			exynos_mdma1_device.res.start = EXYNOS4_PA_S_MDMA1;
 	} else if (soc_is_exynos4212() || soc_is_exynos4412()) {
 		exynos_pdma0_pdata.nr_valid_peri =
 			ARRAY_SIZE(exynos4212_pdma0_peri);

diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 8480849..0abfe78 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h

@@ -89,7 +89,8 @@
 #define EXYNOS4_PA_L2CC			0x10502000
 
 #define EXYNOS4_PA_MDMA0		0x10810000
-#define EXYNOS4_PA_MDMA1		0x12850000
+#define EXYNOS4_PA_NS_MDMA1		0x12850000
+#define EXYNOS4_PA_S_MDMA1		0x12840000
 #define EXYNOS4_PA_PDMA0		0x12680000
 #define EXYNOS4_PA_PDMA1		0x12690000
 #define EXYNOS5_PA_MDMA0		0x10800000

-- 
1.8.0

Hi Bartlomiej,

On Monday 29 of October 2012 10:59:52 Bartlomiej Zolnierkiewicz wrote:

Hi,

The first patch fixes lockup caused by pl330 driver.  It doesn't add
any dependencies to later patches and can be applied independently.

The second patch fixes PL330 regression on ARM Exynos 4210 revision 0
SOC (used by Universal C210 board).

The third patch adds PL330 MDMA1 controller node to device tree on
ARM Exynos4 SOCs.

The fourth patch adds device tree property for PL330 driver to be used
by controllers that have DMA_MEMCPY capability and updates device tree
files for ARM platforms using PL330.  Since this patch depends on
patches #2-3 it would be probably best to also push it through Kukjin's
tree (with the relevant ACKs added of course).


Bartlomiej Zolnierkiewicz (4):
  DMA: PL330: fix locking in pl330_free_chan_resources()
  ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC
  ARM: dts: exynos4: add node for PL330 MDMA1 controller
  DMA: PL330: add device tree property for DMA_MEMCPY capability

 Documentation/devicetree/bindings/dma/arm-pl330.txt |  1 +
 arch/arm/boot/dts/exynos4.dtsi                      |  7 +++++++
 arch/arm/boot/dts/exynos5250.dtsi                   |  2 ++
 arch/arm/boot/dts/highbank.dts                      |  1 +
 arch/arm/boot/dts/socfpga.dtsi                      |  1 +
 arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts         |  1 +
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts          |  1 +
 arch/arm/mach-exynos/dma.c                          |  5 ++++-
 arch/arm/mach-exynos/include/mach/map.h             |  3 ++-
 arch/arm/mach-exynos/mach-exynos4-dt.c              |  1 +
 drivers/dma/pl330.c                                 | 15
++++++++++----- 11 files changed, 31 insertions(+), 7 deletions(-)

For all four patches:

Reviewed-by: Tomasz Figa <redacted>

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

On 10/29/12 10:59, Bartlomiej Zolnierkiewicz wrote:

Commit 8214513 ("ARM: EXYNOS: fix address for EXYNOS4 MDMA1")
changed EXYNOS specific setup of PL330 DMA engine to use 'non-secure'
mdma1 address instead of 'secure' one (from 0x12840000 to 0x12850000)
to fix issue with some Exynos4212 SOCs.  Unfortunately it brakes
PL330 setup for revision 0 of Exynos4210 SOC (mdma1 device cannot
be found at 'non-secure' address):

[    0.566245] dma-pl330 dma-pl330.2: PERIPH_ID 0x0, PCELL_ID 0x0 !
[    0.566278] dma-pl330: probe of dma-pl330.2 failed with error -22

Fix it by using 'secure' mdma1 address on Exynos4210 revision 0 SOC.

Cc: Tomasz Figa<redacted>
Cc: Kukjin Kim<redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz<redacted>
Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
---
  arch/arm/mach-exynos/dma.c              | 5 ++++-
  arch/arm/mach-exynos/include/mach/map.h | 3 ++-
  2 files changed, 6 insertions(+), 2 deletions(-)

[...]

+		if (samsung_rev() == EXYNOS4210_REV_0)
+			exynos_mdma1_device.res.start = EXYNOS4_PA_S_MDMA1;

Hi Bart,

Hmm...above change and adding definition of EXYNOS_PA_S_MDMA1 address 
can fix the problem you commented on EXYNOS4210 Rev0 without others?...

Thanks.

Best regards,
Kgene.
--
Kukjin Kim [off-list ref], Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

* Add device tree (DT) property ("pl330,dma-memcpy") for DMA_MEMCPY
  capability and instead of setting this capability unconditionally
  in pl330_probe() do it only when property is present.

Perhaps we should pass the array of peripheral interfaces via DT, the
lack of which could imply MEMCPY capability ? (while it works, I doubt
if pl330 is supposed to have SLAVE and MEMCPY capabilities in any
instance)
That would also be a step towards discarding "struct dma_pl330_platdata".

On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

Thanks.

Acked-by: Jassi Brar <jassisinghbrar@gmail.com>

Hi,

On Monday 29 October 2012 18:24:06 Kukjin Kim wrote:

On 10/29/12 10:59, Bartlomiej Zolnierkiewicz wrote:

quoted

Commit 8214513 ("ARM: EXYNOS: fix address for EXYNOS4 MDMA1")
changed EXYNOS specific setup of PL330 DMA engine to use 'non-secure'
mdma1 address instead of 'secure' one (from 0x12840000 to 0x12850000)
to fix issue with some Exynos4212 SOCs.  Unfortunately it brakes
PL330 setup for revision 0 of Exynos4210 SOC (mdma1 device cannot
be found at 'non-secure' address):

[    0.566245] dma-pl330 dma-pl330.2: PERIPH_ID 0x0, PCELL_ID 0x0 !
[    0.566278] dma-pl330: probe of dma-pl330.2 failed with error -22

Fix it by using 'secure' mdma1 address on Exynos4210 revision 0 SOC.

Cc: Tomasz Figa<redacted>
Cc: Kukjin Kim<redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz<redacted>
Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
---
  arch/arm/mach-exynos/dma.c              | 5 ++++-
  arch/arm/mach-exynos/include/mach/map.h | 3 ++-
  2 files changed, 6 insertions(+), 2 deletions(-)

[...]

quoted

+		if (samsung_rev() == EXYNOS4210_REV_0)
+			exynos_mdma1_device.res.start = EXYNOS4_PA_S_MDMA1;

Hi Bart,

Hmm...above change and adding definition of EXYNOS_PA_S_MDMA1 address 
can fix the problem you commented on EXYNOS4210 Rev0 without others?...

The problem is affecting only EXYNOS4210 Rev0 and the fix is applied only
for case when soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_0,
or did you mean something else?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung Poland R&D Center

Thanks.

Best regards,
Kgene.
--
Kukjin Kim [off-list ref], Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Hi,

On Monday 29 October 2012 22:45:48 Jassi Brar wrote:

On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

quoted

* Add device tree (DT) property ("pl330,dma-memcpy") for DMA_MEMCPY
  capability and instead of setting this capability unconditionally
  in pl330_probe() do it only when property is present.

Perhaps we should pass the array of peripheral interfaces via DT, the
lack of which could imply MEMCPY capability ? (while it works, I doubt
if pl330 is supposed to have SLAVE and MEMCPY capabilities in any
instance)

In case of PL330 on EXYNOS4 we have two interfaces with SLAVE capability
and one interface with MEMCPY capability.  Could you please explain more
the idea of passing the array of peripherals through DT so we can detect
which interface has MEMCPY capability?

That would also be a step towards discarding "struct dma_pl330_platdata".

I don't know if getting rid of "struct dma_pl330_platdata" is possible
but we still need to come up with some way to pass the needed information
through DT.  Do you have an idea how it could be done?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung Poland R&D Center

Bartlomiej Zolnierkiewicz wrote:

quoted

Hmm...above change and adding definition of EXYNOS_PA_S_MDMA1 address
can fix the problem you commented on EXYNOS4210 Rev0 without others?...

The problem is affecting only EXYNOS4210 Rev0 and the fix is applied only
for case when soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_0,
or did you mean something else?

Yeah, I know. I mean just adding secure mdma1 address is enough for
exynos4210 rev0.

8<-----

@@ -275,6 +275,9 @@ static int __init exynos_dma_init(void)
 		exynos_pdma1_pdata.nr_valid_peri =
 			ARRAY_SIZE(exynos4210_pdma1_peri);
 		exynos_pdma1_pdata.peri_id = exynos4210_pdma1_peri;
+
+		if (samsung_rev() == EXYNOS4210_REV_0)
+			exynos_mdma1_device.res.start = EXYNOS4_PA_S_MDMA1;
 	} else if (soc_is_exynos4212() || soc_is_exynos4412()) {
 		exynos_pdma0_pdata.nr_valid_peri =
 			ARRAY_SIZE(exynos4212_pdma0_peri);

diff --git a/arch/arm/mach-exynos/include/mach/map.h

b/arch/arm/mach-exynos/include/mach/map.h
index 8480849..0abfe78 100644

--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h

@@ -90,6 +90,7 @@
 
 #define EXYNOS4_PA_MDMA0		0x10810000
 #define EXYNOS4_PA_MDMA1		0x12850000
+#define EXYNOS4_PA_S_MDMA1		0x12840000
 #define EXYNOS4_PA_PDMA0		0x12680000
 #define EXYNOS4_PA_PDMA1		0x12690000
 #define EXYNOS5_PA_MDMA0		0x10800000

8<----

Thanks.

Best regards,
Kgene.
--
Kukjin Kim [off-list ref], Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

On Thursday 08 November 2012 05:49:47 Kukjin Kim wrote:

Ah, okay.  Here is full simplified patch.

From: Bartlomiej Zolnierkiewicz <redacted>
Subject: [PATCH v2] ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC

Commit 8214513 ("ARM: EXYNOS: fix address for EXYNOS4 MDMA1")
changed EXYNOS specific setup of PL330 DMA engine to use 'non-secure'
mdma1 address instead of 'secure' one (from 0x12840000 to 0x12850000)
to fix issue with some Exynos4212 SOCs.  Unfortunately it brakes
PL330 setup for revision 0 of Exynos4210 SOC (mdma1 device cannot
be found at 'non-secure' address):

[    0.566245] dma-pl330 dma-pl330.2: PERIPH_ID 0x0, PCELL_ID 0x0 !
[    0.566278] dma-pl330: probe of dma-pl330.2 failed with error -22

Fix it by using 'secure' mdma1 address on Exynos4210 revision 0 SOC.

Reviewed-by: Tomasz Figa <redacted>
Cc: Kukjin Kim <redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/mach-exynos/dma.c              |    3 +++
 arch/arm/mach-exynos/include/mach/map.h |    1 +
 2 files changed, 4 insertions(+)

Index: b/arch/arm/mach-exynos/dma.c
===================================================================

--- a/arch/arm/mach-exynos/dma.c	2012-11-07 18:20:36.561743865 +0100
+++ b/arch/arm/mach-exynos/dma.c	2012-11-08 10:48:23.445067606 +0100

@@ -275,6 +275,9 @@ static int __init exynos_dma_init(void)
 		exynos_pdma1_pdata.nr_valid_peri =
 			ARRAY_SIZE(exynos4210_pdma1_peri);
 		exynos_pdma1_pdata.peri_id = exynos4210_pdma1_peri;
+
+		if (samsung_rev() == EXYNOS4210_REV_0)
+			exynos_mdma1_device.res.start = EXYNOS4_PA_S_MDMA1;
 	} else if (soc_is_exynos4212() || soc_is_exynos4412()) {
 		exynos_pdma0_pdata.nr_valid_peri =
 			ARRAY_SIZE(exynos4212_pdma0_peri);

Index: b/arch/arm/mach-exynos/include/mach/map.h
===================================================================

--- a/arch/arm/mach-exynos/include/mach/map.h	2012-11-07 18:20:44.801743862 +0100
+++ b/arch/arm/mach-exynos/include/mach/map.h	2012-11-08 10:48:40.597067605 +0100

@@ -92,6 +92,7 @@
 
 #define EXYNOS4_PA_MDMA0		0x10810000
 #define EXYNOS4_PA_MDMA1		0x12850000
+#define EXYNOS4_PA_S_MDMA1		0x12840000
 #define EXYNOS4_PA_PDMA0		0x12680000
 #define EXYNOS4_PA_PDMA1		0x12690000
 #define EXYNOS5_PA_MDMA0		0x10800000

On 30 October 2012 14:51, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

Hi,

On Monday 29 October 2012 22:45:48 Jassi Brar wrote:

quoted

On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

quoted

* Add device tree (DT) property ("pl330,dma-memcpy") for DMA_MEMCPY
  capability and instead of setting this capability unconditionally
  in pl330_probe() do it only when property is present.

Perhaps we should pass the array of peripheral interfaces via DT, the
lack of which could imply MEMCPY capability ? (while it works, I doubt
if pl330 is supposed to have SLAVE and MEMCPY capabilities in any
instance)

In case of PL330 on EXYNOS4 we have two interfaces with SLAVE capability
and one interface with MEMCPY capability.  Could you please explain more
the idea of passing the array of peripherals through DT so we can detect
which interface has MEMCPY capability?

The DT node of a 'pdma' should have the array of indices of
peripherals it caters to (what is currently peri_id of 'struct
dma_pl330_platdata'). The array would be missing in the DT node of
'mdma' since all channels are equal.
During probe if the array, say as property 'peri_map', is missing from
DT node of the dmac, that would imply the dmac is 'mdma' and hence the
pl330.c sets DMA_MEMCPY in its cap_mask. Otherwise the peri_map
implies a 'pdma' and hence SLAVE|CYCLIC is set.

quoted

That would also be a step towards discarding "struct dma_pl330_platdata".

I don't know if getting rid of "struct dma_pl330_platdata" is possible
but we still need to come up with some way to pass the needed information
through DT.  Do you have an idea how it could be done?

struct dma_pl330_platdata {
  u8 nr_valid_peri;
  u8 *peri_id;
      As explain above, these two should move to DT node of the dma controller.

  dma_cap_mask_t cap_mask;
      Should be set in pl330.c : MEMCPY for mdma,  SLAVE|CYCLIC for pdma

  unsigned mcbuf_sz;
      Currently unused and already safe enough default value set in driver.
}

Bartlomiej Zolnierkiewicz wrote:

Looks good to me, and I think, this can be handled separate from this
series.

Vinod, if you're ok, let me pick this up into Samsung tree.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim [off-list ref], Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Bartlomiej Zolnierkiewicz wrote:

Applied, thanks.

Best regards,
Kgene.
--
Kukjin Kim [off-list ref], Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Hi Kukjin,

Could you also apply this patch?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung Poland R&D Center


From: Bartlomiej Zolnierkiewicz <redacted>
Subject: [PATCH v2] ARM: dts: exynos4: add node for PL330 MDMA1 controller

Add missing PL330 MDMA1 controller node to the device tree (DT).

[ Currently there is no problem with using 'non-secure' mdma1 address
  instead of 'secure' one on revision 0 of Exynos4210 SOC (as used by
  Universal C210 board) as this SOC revision is unsupported by DT. ]

Reviewed-by: Tomasz Figa <redacted>
Cc: Kukjin Kim <redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
v2: refreshed againt next-20121115 and added Reviewed-by tag

 arch/arm/boot/dts/exynos4.dtsi         |    6 ++++++
 arch/arm/mach-exynos/mach-exynos4-dt.c |    1 +
 2 files changed, 7 insertions(+)

Index: b/arch/arm/boot/dts/exynos4.dtsi
===================================================================

--- a/arch/arm/boot/dts/exynos4.dtsi	2012-11-22 11:40:51.000000000 +0100
+++ b/arch/arm/boot/dts/exynos4.dtsi	2012-11-22 11:53:29.520815274 +0100

@@ -244,5 +244,11 @@
 			reg = <0x12690000 0x1000>;
 			interrupts = <0 36 0>;
 		};
+
+		mdma1: mdma at 12850000 {
+			compatible = "arm,pl330", "arm,primecell";
+			reg = <0x12850000 0x1000>;
+			interrupts = <0 34 0>;
+		};
 	};
 };

Index: b/arch/arm/mach-exynos/mach-exynos4-dt.c
===================================================================

--- a/arch/arm/mach-exynos/mach-exynos4-dt.c	2012-11-22 11:40:51.000000000 +0100
+++ b/arch/arm/mach-exynos/mach-exynos4-dt.c	2012-11-22 11:53:55.248815271 +0100

@@ -77,6 +77,7 @@ static const struct of_dev_auxdata exyno
 				"exynos4210-spi.2", NULL),
 	OF_DEV_AUXDATA("arm,pl330", EXYNOS4_PA_PDMA0, "dma-pl330.0", NULL),
 	OF_DEV_AUXDATA("arm,pl330", EXYNOS4_PA_PDMA1, "dma-pl330.1", NULL),
+	OF_DEV_AUXDATA("arm,pl330", EXYNOS4_PA_MDMA1, "dma-pl330.2", NULL),
 	OF_DEV_AUXDATA("samsung,exynos4210-tmu", EXYNOS4_PA_TMU,
 				"exynos-tmu", NULL),
 	{},

Bartlomiej Zolnierkiewicz wrote:

Hi Kukjin,

Hi Bart,

Could you also apply this patch?

Yeah, looks good to me :-)

Will apply, thanks.

Best regards,
Kgene.
--
Kukjin Kim [off-list ref], Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

On Monday 29 October 2012 22:48:05 Jassi Brar wrote:

On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

quoted

tasklet_kill() may sleep so call it before taking pch->lock.

Fixes following lockup:

[  345.470000] BUG: scheduling while atomic: cat/2383/0x00000002
[  345.470000] Modules linked in:
[  345.470000] [<c0015858>] (unwind_backtrace+0x0/0xfc) from [<c004d980>] (__schedule_bug+0x4c/0x58)
[  345.470000] [<c004d980>] (__schedule_bug+0x4c/0x58) from [<c0360b6c>] (__schedule+0x690/0x6e0)
[  345.470000] [<c0360b6c>] (__schedule+0x690/0x6e0) from [<c004f2b4>] (sys_sched_yield+0x70/0x78)
[  345.470000] [<c004f2b4>] (sys_sched_yield+0x70/0x78) from [<c002acec>] (tasklet_kill+0x34/0x8c)
[  345.470000] [<c002acec>] (tasklet_kill+0x34/0x8c) from [<c01da4cc>] (pl330_free_chan_resources+0x24/0x88)
[  345.470000] [<c01da4cc>] (pl330_free_chan_resources+0x24/0x88) from [<c01d81f4>] (dma_chan_put+0x4c/0x50)
[  345.470000] [<c01d81f4>] (dma_chan_put+0x4c/0x50) from [<c01d82c0>] (dma_release_channel+0x28/0x98)
[...]
[  368.335000] BUG: spinlock lockup suspected on CPU#0, swapper/0/0
[  368.340000]  lock: 0xe52aa04c, .magic: dead4ead, .owner: cat/2383, .owner_cpu: 1
[  368.350000] [<c0015858>] (unwind_backtrace+0x0/0xfc) from [<c01b3d78>] (do_raw_spin_lock+0x194/0x204)
[  368.360000] [<c01b3d78>] (do_raw_spin_lock+0x194/0x204) from [<c0361adc>] (_raw_spin_lock_irqsave+0x20/0x28)
[  368.365000] [<c0361adc>] (_raw_spin_lock_irqsave+0x20/0x28) from [<c01da80c>] (pl330_tasklet+0x2c/0x5a8)
[  368.375000] [<c01da80c>] (pl330_tasklet+0x2c/0x5a8) from [<c002ac04>] (tasklet_action+0xfc/0x114)
[  368.385000] [<c002ac04>] (tasklet_action+0xfc/0x114) from [<c002b204>] (__do_softirq+0xe4/0x19c)
[  368.395000] [<c002b204>] (__do_softirq+0xe4/0x19c) from [<c002b398>] (irq_exit+0x98/0x9c)
[  368.405000] [<c002b398>] (irq_exit+0x98/0x9c) from [<c0013ebc>] (handle_IPI+0x124/0x16c)
[  368.410000] [<c0013ebc>] (handle_IPI+0x124/0x16c) from [<c000857c>] (gic_handle_irq+0x64/0x68)
[  368.420000] [<c000857c>] (gic_handle_irq+0x64/0x68) from [<c000e740>] (__irq_svc+0x40/0x70)
[  368.430000] Exception stack(0xc04a3f00 to 0xc04a3f48)
[  368.435000] 3f00: c04a3f48 00000000 6f9e23e8 00000050 c07492c8 c04a3f48 00000000 c04ccc88
[  368.440000] 3f20: 6f9dbac3 00000050 6f9e23e8 00000050 3b9aca00 c04a3f48 c005cfa4 c02946d4
[  368.450000] 3f40: 60000013 ffffffff
[  368.455000] [<c000e740>] (__irq_svc+0x40/0x70) from [<c02946d4>] (cpuidle_wrap_enter+0x4c/0xa0)
[  368.460000] [<c02946d4>] (cpuidle_wrap_enter+0x4c/0xa0) from [<c02940dc>] (cpuidle_enter_state+0x18/0x68)
[  368.470000] [<c02940dc>] (cpuidle_enter_state+0x18/0x68) from [<c02948e0>] (cpuidle_idle_call+0xac/0xe0)
[  368.480000] [<c02948e0>] (cpuidle_idle_call+0xac/0xe0) from [<c00102f8>] (cpu_idle+0xac/0xf0)
[  368.490000] [<c00102f8>] (cpu_idle+0xac/0xf0) from [<c04796a0>] (start_kernel+0x28c/0x294)

Cc: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Vinod Koul <redacted>
Cc: Tomasz Figa <redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/dma/pl330.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 665668b..db7574b 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c

@@ -2459,10 +2459,10 @@ static void pl330_free_chan_resources(struct dma_chan *chan)
        struct dma_pl330_chan *pch = to_pchan(chan);
        unsigned long flags;

-       spin_lock_irqsave(&pch->lock, flags);
-
        tasklet_kill(&pch->task);

+       spin_lock_irqsave(&pch->lock, flags);
+
        pl330_release_channel(pch->pl330_chid);
        pch->pl330_chid = NULL;

Thanks.

Acked-by: Jassi Brar <jassisinghbrar@gmail.com>

Vinod/Dan could you please pick this patch for 3.8?  Thanks!

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung Poland R&D Center

On Friday 09 November 2012 07:11:30 Jassi Brar wrote:

On 30 October 2012 14:51, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

quoted

Hi,

On Monday 29 October 2012 22:45:48 Jassi Brar wrote:

quoted

On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

quoted

* Add device tree (DT) property ("pl330,dma-memcpy") for DMA_MEMCPY
  capability and instead of setting this capability unconditionally
  in pl330_probe() do it only when property is present.

Perhaps we should pass the array of peripheral interfaces via DT, the
lack of which could imply MEMCPY capability ? (while it works, I doubt
if pl330 is supposed to have SLAVE and MEMCPY capabilities in any
instance)

In case of PL330 on EXYNOS4 we have two interfaces with SLAVE capability
and one interface with MEMCPY capability.  Could you please explain more
the idea of passing the array of peripherals through DT so we can detect
which interface has MEMCPY capability?

The DT node of a 'pdma' should have the array of indices of
peripherals it caters to (what is currently peri_id of 'struct
dma_pl330_platdata'). The array would be missing in the DT node of
'mdma' since all channels are equal.
During probe if the array, say as property 'peri_map', is missing from
DT node of the dmac, that would imply the dmac is 'mdma' and hence the
pl330.c sets DMA_MEMCPY in its cap_mask. Otherwise the peri_map
implies a 'pdma' and hence SLAVE|CYCLIC is set.

quoted

quoted

That would also be a step towards discarding "struct dma_pl330_platdata".

I don't know if getting rid of "struct dma_pl330_platdata" is possible
but we still need to come up with some way to pass the needed information
through DT.  Do you have an idea how it could be done?

struct dma_pl330_platdata {
  u8 nr_valid_peri;
  u8 *peri_id;
      As explain above, these two should move to DT node of the dma controller.

  dma_cap_mask_t cap_mask;
      Should be set in pl330.c : MEMCPY for mdma,  SLAVE|CYCLIC for pdma

  unsigned mcbuf_sz;
      Currently unused and already safe enough default value set in driver.
}

Thank you for explaining it.  Here is a patch implementing the idea:

From: Bartlomiej Zolnierkiewicz <redacted>
Subject: [PATCH] DMA: PL330: add peripherals map to the device tree

Add device tree (DT) property ("peri-map") for storing indices
of peripherals connected to DMAC and fix DT nodes of client
drivers to use 'dma peripheral id' instead of 'dma request id'.
Also instead of setting DMA_MEMCPY capability unconditionally in
pl330_probe() do it only when "peri-map" DT property is present
(idea from Jassi Brar).  It fixes the issue on ARM EXYNOS
platforms using DT where pdma controller erroneously was used
for DMA_MEMCPY operations instead of mdma one (it seems to work
correctly but at the cost of worse performance).

While at it:
- add missing kfree() to pl330_[probe,remove]()
- fix typo in samsung_dmadev_request()

Cc: Jassi Brar <jassisinghbrar@gmail.com>
Cc: Vinod Koul <redacted>
Cc: Kukjin Kim <redacted>
Cc: Rob Herring <redacted>
Cc: Dinh Nguyen <redacted>
Cc: Pawel Moll <redacted>
Cc: Tomasz Figa <redacted>
Signed-off-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
I wonder whether "peri-map" also needs to be added to following files:

	arch/arm/boot/dts/highbank.dts
	arch/arm/boot/dts/socfpga.dtsi
	arch/arm/boot/dts/vexpress-v2p-ca15-tc1.dts
	arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts

(since they're also using pl330)?

 Documentation/devicetree/bindings/dma/arm-pl330.txt |    5 
 arch/arm/boot/dts/exynos4.dtsi                      |   21 +-
 arch/arm/boot/dts/exynos5250.dtsi                   |   20 +-
 arch/arm/plat-samsung/dma-ops.c                     |    2 
 arch/arm/plat-samsung/include/plat/dma-pl330.h      |  155 +++++++++-----------
 drivers/dma/pl330.c                                 |   54 +++++-
 6 files changed, 152 insertions(+), 105 deletions(-)

Index: b/Documentation/devicetree/bindings/dma/arm-pl330.txt
===================================================================

--- a/Documentation/devicetree/bindings/dma/arm-pl330.txt	2012-11-28 17:41:36.997626033 +0100
+++ b/Documentation/devicetree/bindings/dma/arm-pl330.txt	2012-11-28 17:42:23.433626905 +0100

@@ -11,6 +11,7 @@ Required properties:
 
 Optional properties:
 - dma-coherent      : Present if dma operations are coherent
+- peri-map          : An array of indices of peripherals connected to DMAC
 
 Example:
 

@@ -24,9 +25,9 @@ Client drivers (device nodes requiring d
 mem-to-dev) should specify the DMA channel numbers using a two-value pair
 as shown below.
 
-  [property name]  = <[phandle of the dma controller] [dma request id]>;
+  [property name]  = <[phandle of the dma controller] [dma peripheral id]>;
 
-      where 'dma request id' is the dma request number which is connected
+      where 'dma peripheral id' is the id of peripheral which is connected
       to the client controller. The 'property name' is recommended to be
       of the form <name>-dma-channel.
 

Index: b/arch/arm/boot/dts/exynos4.dtsi
===================================================================

--- a/arch/arm/boot/dts/exynos4.dtsi	2012-11-28 17:41:37.033626034 +0100
+++ b/arch/arm/boot/dts/exynos4.dtsi	2012-11-28 17:42:23.433626905 +0100

@@ -256,8 +256,8 @@
 		compatible = "samsung,exynos4210-spi";
 		reg = <0x13920000 0x100>;
 		interrupts = <0 66 0>;
-		tx-dma-channel = <&pdma0 7>; /* preliminary */
-		rx-dma-channel = <&pdma0 6>; /* preliminary */
+		tx-dma-channel = <&pdma0 23>;
+		rx-dma-channel = <&pdma0 22>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 		status = "disabled";

@@ -267,8 +267,8 @@
 		compatible = "samsung,exynos4210-spi";
 		reg = <0x13930000 0x100>;
 		interrupts = <0 67 0>;
-		tx-dma-channel = <&pdma1 7>; /* preliminary */
-		rx-dma-channel = <&pdma1 6>; /* preliminary */
+		tx-dma-channel = <&pdma1 25>;
+		rx-dma-channel = <&pdma1 24>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 		status = "disabled";

@@ -278,8 +278,8 @@
 		compatible = "samsung,exynos4210-spi";
 		reg = <0x13940000 0x100>;
 		interrupts = <0 68 0>;
-		tx-dma-channel = <&pdma0 9>; /* preliminary */
-		rx-dma-channel = <&pdma0 8>; /* preliminary */
+		tx-dma-channel = <&pdma0 27>;
+		rx-dma-channel = <&pdma0 26>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 		status = "disabled";

@@ -303,12 +303,21 @@
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x12680000 0x1000>;
 			interrupts = <0 35 0>;
+			peri-map = < 37 36 41 40 60 61 22 23 26 27
+				     17 15 16 20 21  0  1  4  5  8
+				      9 46 47 52 53 56 57 28 29 30
+				     64 65 >;
+
 		};
 
 		pdma1: pdma at 12690000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x12690000 0x1000>;
 			interrupts = <0 36 0>;
+			peri-map = < 37 36 39 38 62 63 24 25 17 15
+				     16 18 19  0  1  2  3  6  7 50
+				     51 54 55 58 59 48 49 33 66 67 >;
+
 		};
 
 		mdma1: mdma at 12850000 {

Index: b/arch/arm/boot/dts/exynos5250.dtsi
===================================================================

--- a/arch/arm/boot/dts/exynos5250.dtsi	2012-11-28 17:41:37.021626034 +0100
+++ b/arch/arm/boot/dts/exynos5250.dtsi	2012-11-28 17:42:23.433626905 +0100

@@ -160,8 +160,8 @@
 		compatible = "samsung,exynos4210-spi";
 		reg = <0x12d20000 0x100>;
 		interrupts = <0 66 0>;
-		tx-dma-channel = <&pdma0 5>; /* preliminary */
-		rx-dma-channel = <&pdma0 4>; /* preliminary */
+		tx-dma-channel = <&pdma0 23>;
+		rx-dma-channel = <&pdma0 22>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 	};

@@ -170,8 +170,8 @@
 		compatible = "samsung,exynos4210-spi";
 		reg = <0x12d30000 0x100>;
 		interrupts = <0 67 0>;
-		tx-dma-channel = <&pdma1 5>; /* preliminary */
-		rx-dma-channel = <&pdma1 4>; /* preliminary */
+		tx-dma-channel = <&pdma1 25>;
+		rx-dma-channel = <&pdma1 24>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 	};

@@ -180,8 +180,8 @@
 		compatible = "samsung,exynos4210-spi";
 		reg = <0x12d40000 0x100>;
 		interrupts = <0 68 0>;
-		tx-dma-channel = <&pdma0 7>; /* preliminary */
-		rx-dma-channel = <&pdma0 6>; /* preliminary */
+		tx-dma-channel = <&pdma0 27>;
+		rx-dma-channel = <&pdma0 26>;
 		#address-cells = <1>;
 		#size-cells = <0>;
 	};

@@ -229,12 +229,20 @@
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x121A0000 0x1000>;
 			interrupts = <0 34 0>;
+			peri-map = < 37 36 41 40 22 23 26 27 17 15
+				     16 20 21  0  1  4  5  8  9 46
+				     47 52 53 56 57 28 29 30 60 62
+				     64 66 >;
 		};
 
 		pdma1: pdma at 121B0000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x121B0000 0x1000>;
 			interrupts = <0 35 0>;
+			peri-map = < 37 36 39 38 24 25 32 33 17 15
+				     16 18 19  0  1  2  3  6  7 50
+				     51 54 55 58 59 48 49 68 61 63
+				     65 67 >;
 		};
 
 		mdma0: mdma at 10800000 {

Index: b/arch/arm/plat-samsung/dma-ops.c
===================================================================

--- a/arch/arm/plat-samsung/dma-ops.c	2012-11-28 17:41:37.057626035 +0100
+++ b/arch/arm/plat-samsung/dma-ops.c	2012-11-28 17:42:23.433626905 +0100

@@ -29,7 +29,7 @@ static unsigned samsung_dmadev_request(e
 
 	/*
 	 * If a dma channel property of a device node from device tree is
-	 * specified, use that as the fliter parameter.
+	 * specified, use that as the filter parameter.
 	 */
 	filter_param = (dma_ch == DMACH_DT_PROP) ?
 		(void *)param->dt_dmach_prop : (void *)dma_ch;

Index: b/arch/arm/plat-samsung/include/plat/dma-pl330.h
===================================================================

--- a/arch/arm/plat-samsung/include/plat/dma-pl330.h	2012-11-28 17:41:37.045626034 +0100
+++ b/arch/arm/plat-samsung/include/plat/dma-pl330.h	2012-11-28 17:42:23.433626905 +0100

@@ -17,88 +17,87 @@
  * For the sake of consistency across client drivers,
  * We keep the channel names unchanged and only add
  * missing peripherals are added.
- * Order is not important since DMA PL330 API driver
- * use these just as IDs.
+ * Order is important since IDs are used by device tree.
  */
 enum dma_ch {
 	DMACH_DT_PROP = -1,
 	DMACH_UART0_RX = 0,
-	DMACH_UART0_TX,
-	DMACH_UART1_RX,
-	DMACH_UART1_TX,
-	DMACH_UART2_RX,
-	DMACH_UART2_TX,
-	DMACH_UART3_RX,
-	DMACH_UART3_TX,
-	DMACH_UART4_RX,
-	DMACH_UART4_TX,
-	DMACH_UART5_RX,
-	DMACH_UART5_TX,
-	DMACH_USI_RX,
-	DMACH_USI_TX,
-	DMACH_IRDA,
-	DMACH_I2S0_RX,
-	DMACH_I2S0_TX,
-	DMACH_I2S0S_TX,
-	DMACH_I2S1_RX,
-	DMACH_I2S1_TX,
-	DMACH_I2S2_RX,
-	DMACH_I2S2_TX,
-	DMACH_SPI0_RX,
-	DMACH_SPI0_TX,
-	DMACH_SPI1_RX,
-	DMACH_SPI1_TX,
-	DMACH_SPI2_RX,
-	DMACH_SPI2_TX,
-	DMACH_AC97_MICIN,
-	DMACH_AC97_PCMIN,
-	DMACH_AC97_PCMOUT,
-	DMACH_EXTERNAL,
-	DMACH_PWM,
-	DMACH_SPDIF,
-	DMACH_HSI_RX,
-	DMACH_HSI_TX,
-	DMACH_PCM0_TX,
-	DMACH_PCM0_RX,
-	DMACH_PCM1_TX,
-	DMACH_PCM1_RX,
-	DMACH_PCM2_TX,
-	DMACH_PCM2_RX,
-	DMACH_MSM_REQ3,
-	DMACH_MSM_REQ2,
-	DMACH_MSM_REQ1,
-	DMACH_MSM_REQ0,
-	DMACH_SLIMBUS0_RX,
-	DMACH_SLIMBUS0_TX,
-	DMACH_SLIMBUS0AUX_RX,
-	DMACH_SLIMBUS0AUX_TX,
-	DMACH_SLIMBUS1_RX,
-	DMACH_SLIMBUS1_TX,
-	DMACH_SLIMBUS2_RX,
-	DMACH_SLIMBUS2_TX,
-	DMACH_SLIMBUS3_RX,
-	DMACH_SLIMBUS3_TX,
-	DMACH_SLIMBUS4_RX,
-	DMACH_SLIMBUS4_TX,
-	DMACH_SLIMBUS5_RX,
-	DMACH_SLIMBUS5_TX,
-	DMACH_MIPI_HSI0,
-	DMACH_MIPI_HSI1,
-	DMACH_MIPI_HSI2,
-	DMACH_MIPI_HSI3,
-	DMACH_MIPI_HSI4,
-	DMACH_MIPI_HSI5,
-	DMACH_MIPI_HSI6,
-	DMACH_MIPI_HSI7,
-	DMACH_DISP1,
-	DMACH_MTOM_0,
-	DMACH_MTOM_1,
-	DMACH_MTOM_2,
-	DMACH_MTOM_3,
-	DMACH_MTOM_4,
-	DMACH_MTOM_5,
-	DMACH_MTOM_6,
-	DMACH_MTOM_7,
+	DMACH_UART0_TX = 1,
+	DMACH_UART1_RX = 2,
+	DMACH_UART1_TX = 3,
+	DMACH_UART2_RX = 4,
+	DMACH_UART2_TX = 5,
+	DMACH_UART3_RX = 6,
+	DMACH_UART3_TX = 7,
+	DMACH_UART4_RX = 8,
+	DMACH_UART4_TX = 9,
+	DMACH_UART5_RX = 10,
+	DMACH_UART5_TX = 11,
+	DMACH_USI_RX = 12,
+	DMACH_USI_TX = 13,
+	DMACH_IRDA = 14,
+	DMACH_I2S0_RX = 15,
+	DMACH_I2S0_TX = 16,
+	DMACH_I2S0S_TX = 17,
+	DMACH_I2S1_RX = 18,
+	DMACH_I2S1_TX = 19,
+	DMACH_I2S2_RX = 20,
+	DMACH_I2S2_TX = 21,
+	DMACH_SPI0_RX = 22,
+	DMACH_SPI0_TX = 23,
+	DMACH_SPI1_RX = 24,
+	DMACH_SPI1_TX = 25,
+	DMACH_SPI2_RX = 26,
+	DMACH_SPI2_TX = 27,
+	DMACH_AC97_MICIN = 28,
+	DMACH_AC97_PCMIN = 29,
+	DMACH_AC97_PCMOUT = 30,
+	DMACH_EXTERNAL = 31,
+	DMACH_PWM = 32,
+	DMACH_SPDIF = 33,
+	DMACH_HSI_RX = 34,
+	DMACH_HSI_TX = 35,
+	DMACH_PCM0_TX = 36,
+	DMACH_PCM0_RX = 37,
+	DMACH_PCM1_TX = 38,
+	DMACH_PCM1_RX = 39,
+	DMACH_PCM2_TX = 40,
+	DMACH_PCM2_RX = 41,
+	DMACH_MSM_REQ3 = 42,
+	DMACH_MSM_REQ2 = 43,
+	DMACH_MSM_REQ1 = 44,
+	DMACH_MSM_REQ0 = 45,
+	DMACH_SLIMBUS0_RX = 46,
+	DMACH_SLIMBUS0_TX = 47,
+	DMACH_SLIMBUS0AUX_RX = 48,
+	DMACH_SLIMBUS0AUX_TX = 49,
+	DMACH_SLIMBUS1_RX = 50,
+	DMACH_SLIMBUS1_TX = 51,
+	DMACH_SLIMBUS2_RX = 52,
+	DMACH_SLIMBUS2_TX = 53,
+	DMACH_SLIMBUS3_RX = 54,
+	DMACH_SLIMBUS3_TX = 55,
+	DMACH_SLIMBUS4_RX = 56,
+	DMACH_SLIMBUS4_TX = 57,
+	DMACH_SLIMBUS5_RX = 58,
+	DMACH_SLIMBUS5_TX = 59,
+	DMACH_MIPI_HSI0 = 60,
+	DMACH_MIPI_HSI1 = 61,
+	DMACH_MIPI_HSI2 = 62,
+	DMACH_MIPI_HSI3 = 63,
+	DMACH_MIPI_HSI4 = 64,
+	DMACH_MIPI_HSI5 = 65,
+	DMACH_MIPI_HSI6 = 66,
+	DMACH_MIPI_HSI7 = 67,
+	DMACH_DISP1 = 68,
+	DMACH_MTOM_0 = 69,
+	DMACH_MTOM_1 = 70,
+	DMACH_MTOM_2 = 71,
+	DMACH_MTOM_3 = 72,
+	DMACH_MTOM_4 = 73,
+	DMACH_MTOM_5 = 74,
+	DMACH_MTOM_6 = 75,
+	DMACH_MTOM_7 = 76,
 	/* END Marker, also used to denote a reserved channel */
 	DMACH_MAX,
 };

Index: b/drivers/dma/pl330.c
===================================================================

--- a/drivers/dma/pl330.c	2012-11-28 17:41:37.009626033 +0100
+++ b/drivers/dma/pl330.c	2012-11-28 17:50:27.301635989 +0100

@@ -306,6 +306,8 @@ struct pl330_config {
 struct pl330_info {
 	/* Owning device */
 	struct device *dev;
+	/* Array of valid peripherals */
+	u32 *peri_id;
 	/* Size of MicroCode buffers for each channel. */
 	unsigned mcbufsz;
 	/* ioremap'ed address of PL330 registers. */

@@ -2368,8 +2370,9 @@ bool pl330_filter(struct dma_chan *chan,
 		prop_value = ((struct property *)param)->value;
 		phandle = be32_to_cpup(prop_value++);
 		node = of_find_node_by_phandle(phandle);
-		return ((chan->private == node) &&
-				(chan->chan_id == be32_to_cpup(prop_value)));
+		peri_id = chan->private;
+		return chan->device->dev->of_node == node &&
+		       *peri_id == be32_to_cpup(prop_value);
 	}
 #endif
 

@@ -2911,8 +2914,28 @@ pl330_probe(struct amba_device *adev, co
 	/* Initialize channel parameters */
 	if (pdat)
 		num_chan = max_t(int, pdat->nr_valid_peri, pi->pcfg.num_chan);
-	else
-		num_chan = max_t(int, pi->pcfg.num_peri, pi->pcfg.num_chan);
+	else {
+		struct device_node *np = pi->dev->of_node;
+		int nr_valid_peri = 0;
+
+		of_find_property(np, "peri-map", &nr_valid_peri);
+		if (nr_valid_peri) {
+			nr_valid_peri /= 4;
+
+			pi->peri_id = kzalloc(nr_valid_peri * 4, GFP_KERNEL);
+			if (!pi->peri_id) {
+				ret = -ENOMEM;
+				dev_err(&adev->dev,
+					"unable to allocate pi->peri_id\n");
+				goto probe_err4;
+			}
+			of_property_read_u32_array(np, "peri-map", pi->peri_id,
+						   nr_valid_peri);
+		} else
+			nr_valid_peri = pi->pcfg.num_peri;
+
+		num_chan = max_t(int, nr_valid_peri, pi->pcfg.num_chan);
+	}
 
 	pdmac->peripherals = kzalloc(num_chan * sizeof(*pch), GFP_KERNEL);
 	if (!pdmac->peripherals) {

@@ -2923,10 +2946,11 @@ pl330_probe(struct amba_device *adev, co
 
 	for (i = 0; i < num_chan; i++) {
 		pch = &pdmac->peripherals[i];
-		if (!adev->dev.of_node)
-			pch->chan.private = pdat ? &pdat->peri_id[i] : NULL;
-		else
-			pch->chan.private = adev->dev.of_node;
+
+		if (pdat)
+			pch->chan.private = &pdat->peri_id[i];
+		else if (pi->peri_id)
+			pch->chan.private = &pi->peri_id[i];
 
 		INIT_LIST_HEAD(&pch->work_list);
 		spin_lock_init(&pch->lock);

@@ -2942,12 +2966,12 @@ pl330_probe(struct amba_device *adev, co
 	if (pdat) {
 		pd->cap_mask = pdat->cap_mask;
 	} else {
-		dma_cap_set(DMA_MEMCPY, pd->cap_mask);
-		if (pi->pcfg.num_peri) {
+		if (pi->peri_id) {
 			dma_cap_set(DMA_SLAVE, pd->cap_mask);
 			dma_cap_set(DMA_CYCLIC, pd->cap_mask);
 			dma_cap_set(DMA_PRIVATE, pd->cap_mask);
-		}
+		} else
+			dma_cap_set(DMA_MEMCPY, pd->cap_mask);
 	}
 
 	pd->device_alloc_chan_resources = pl330_alloc_chan_resources;

@@ -2962,7 +2986,7 @@ pl330_probe(struct amba_device *adev, co
 	ret = dma_async_device_register(pd);
 	if (ret) {
 		dev_err(&adev->dev, "unable to register DMAC\n");
-		goto probe_err4;
+		goto probe_err5;
 	}
 
 	dev_info(&adev->dev,

@@ -2975,7 +2999,10 @@ pl330_probe(struct amba_device *adev, co
 
 	return 0;
 
+probe_err5:
+	kfree(pdmac->peripherals);
 probe_err4:
+	kfree(pi->peri_id);
 	pl330_del(pi);
 probe_err3:
 	free_irq(irq, pi);

@@ -3013,8 +3040,11 @@ static int __devexit pl330_remove(struct
 		pl330_free_chan_resources(&pch->chan);
 	}
 
+	kfree(pdmac->peripherals);
+
 	pi = &pdmac->pif;
 
+	kfree(pi->peri_id);
 	pl330_del(pi);
 
 	irq = adev->irq[0];

On Fri, 2012-11-30 at 11:59 +0100, Bartlomiej Zolnierkiewicz wrote:

quoted

Acked-by: Jassi Brar <jassisinghbrar@gmail.com>

Vinod/Dan could you please pick this patch for 3.8?  Thanks!

I will check and queue it up today

-- 
~Vinod

On Fri, Nov 30, 2012 at 4:26 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

Thank you for explaining it.  Here is a patch implementing the idea:

From: Bartlomiej Zolnierkiewicz <redacted>
Subject: [PATCH] DMA: PL330: add peripherals map to the device tree

Add device tree (DT) property ("peri-map") for storing indices
of peripherals connected to DMAC and fix DT nodes of client
drivers to use 'dma peripheral id' instead of 'dma request id'.
Also instead of setting DMA_MEMCPY capability unconditionally in
pl330_probe() do it only when "peri-map" DT property is present
(idea from Jassi Brar).  It fixes the issue on ARM EXYNOS
platforms using DT where pdma controller erroneously was used
for DMA_MEMCPY operations instead of mdma one (it seems to work
correctly but at the cost of worse performance).

Sorry, we need to change it as per new generic dma DT bindings
  http://git.infradead.org/users/vkoul/slave-dma.git/blob/refs/heads/next:/Documentation/devicetree/bindings/dma/dma.txt

Thanks.

Hi,

On Wednesday 12 December 2012 08:36:14 Jassi Brar wrote:

On Fri, Nov 30, 2012 at 4:26 PM, Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

quoted

Thank you for explaining it.  Here is a patch implementing the idea:

From: Bartlomiej Zolnierkiewicz <redacted>
Subject: [PATCH] DMA: PL330: add peripherals map to the device tree

Add device tree (DT) property ("peri-map") for storing indices
of peripherals connected to DMAC and fix DT nodes of client
drivers to use 'dma peripheral id' instead of 'dma request id'.
Also instead of setting DMA_MEMCPY capability unconditionally in
pl330_probe() do it only when "peri-map" DT property is present
(idea from Jassi Brar).  It fixes the issue on ARM EXYNOS
platforms using DT where pdma controller erroneously was used
for DMA_MEMCPY operations instead of mdma one (it seems to work
correctly but at the cost of worse performance).

Sorry, we need to change it as per new generic dma DT bindings
  http://git.infradead.org/users/vkoul/slave-dma.git/blob/refs/heads/next:/Documentation/devicetree/bindings/dma/dma.txt

From looking at the new generic DMA DT bindings code it seems that:

- There is no way to specify different mappings of DMA request signals
  for different DMA controllers using the same driver (cause different
  of_dma_xlate translation functions would be needed in place of
  "peri-map" DT properties).

- There is no way to distinguish DMA capabilities for different DMA
  controllers using the same driver (i.e. differentiate between
  DMA_MEMCPY and DMA_SLAVE controllers, which is what I really wanted
  to fix in the first place; now it seems that in order to add this
  relatively trivial change I would have to do a major rewrite of
  the whole DT support for PL330, hmmm...).

Could you please describe how I should proceed with implementing 
needed functionality in the PL330 driver or/and generic DMA DT layer?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung Poland R&D Center
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121212/01eb2dcb/attachment-0001.html>

On Friday 30 November 2012 19:07:53 Vinod Koul wrote:

On Fri, 2012-11-30 at 11:59 +0100, Bartlomiej Zolnierkiewicz wrote:

quoted

quoted

Acked-by: Jassi Brar <jassisinghbrar@gmail.com>

Vinod/Dan could you please pick this patch for 3.8?  Thanks!

I will check and queue it up today

ping?  [ I can't find the patch in slave-dma.git tree. ]

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung Poland R&D Center