Subject: [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


Subject: [PATCH 1/4] DMA: PL330: fix locking in pl330_free_chan_resources()

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 <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: Tomasz Figa <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
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

Subject: [PATCH 3/4] 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. ]

Cc: Tomasz Figa <[email protected]>
Cc: Kukjin Kim <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
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@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

Subject: [PATCH 4/4] DMA: PL330: add device tree property for DMA_MEMCPY capability

* 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 <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: Kukjin Kim <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Dinh Nguyen <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Tomasz Figa <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
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@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@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

Subject: [PATCH 2/4] 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.

Cc: Tomasz Figa <[email protected]>
Cc: Kukjin Kim <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
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

2012-10-29 10:06:24

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 0/4] DMA: PL330 fixes

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 <[email protected]>

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

2012-10-29 17:24:17

by Kukjin Kim

[permalink] [raw]
Subject: Re: [PATCH 2/4] ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC

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<[email protected]>
> Cc: Kukjin Kim<[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
> Signed-off-by: Kyungmin Park<[email protected]>
> ---
> 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 <[email protected]>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

2012-10-29 21:45:53

by Jassi Brar

[permalink] [raw]
Subject: Re: [PATCH 4/4] DMA: PL330: add device tree property for DMA_MEMCPY capability

On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
<[email protected]> 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".

2012-10-29 21:48:11

by Jassi Brar

[permalink] [raw]
Subject: Re: [PATCH 1/4] DMA: PL330: fix locking in pl330_free_chan_resources()

On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
<[email protected]> wrote:
> 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 <[email protected]>
> Cc: Vinod Koul <[email protected]>
> Cc: Tomasz Figa <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> 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 <[email protected]>

Subject: Re: [PATCH 2/4] ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC


Hi,

On Monday 29 October 2012 18:24:06 Kukjin Kim wrote:
> 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<[email protected]>
> > Cc: Kukjin Kim<[email protected]>
> > Signed-off-by: Bartlomiej Zolnierkiewicz<[email protected]>
> > Signed-off-by: Kyungmin Park<[email protected]>
> > ---
> > 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?...

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 <[email protected]>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.

Subject: Re: [PATCH 4/4] DMA: PL330: add device tree property for DMA_MEMCPY capability


Hi,

On Monday 29 October 2012 22:45:48 Jassi Brar wrote:
> On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
> <[email protected]> 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)

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

2012-11-08 04:49:56

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH 2/4] ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC

Bartlomiej Zolnierkiewicz wrote:
>
> > 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 <[email protected]>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Subject: Re: [PATCH 2/4] ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC

On Thursday 08 November 2012 05:49:47 Kukjin Kim wrote:
> Bartlomiej Zolnierkiewicz wrote:
> >
> > > 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<----

Ah, okay. Here is full simplified patch.

From: Bartlomiej Zolnierkiewicz <[email protected]>
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 <[email protected]>
Cc: Kukjin Kim <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
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

2012-11-09 06:11:35

by Jassi Brar

[permalink] [raw]
Subject: Re: [PATCH 4/4] DMA: PL330: add device tree property for DMA_MEMCPY capability

On 30 October 2012 14:51, Bartlomiej Zolnierkiewicz
<[email protected]> wrote:
>
> Hi,
>
> On Monday 29 October 2012 22:45:48 Jassi Brar wrote:
>> On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
>> <[email protected]> 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)
>
> 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.


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

2012-11-09 10:20:07

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH 2/4] ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC

Bartlomiej Zolnierkiewicz wrote:
>
> Ah, okay. Here is full simplified patch.
>
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> 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 <[email protected]>
> Cc: Kukjin Kim <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> 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

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 <[email protected]>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

2012-11-20 12:23:31

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH 2/4] ARM: EXYNOS: PL330 MDMA1 fix for revision 0 of Exynos4210 SOC

Bartlomiej Zolnierkiewicz wrote:
>
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> 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 <[email protected]>
> Cc: Kukjin Kim <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> 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

Applied, thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Subject: Re: [PATCH 3/4] ARM: dts: exynos4: add node for PL330 MDMA1 controller


Hi Kukjin,

Could you also apply this patch?

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


From: Bartlomiej Zolnierkiewicz <[email protected]>
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 <[email protected]>
Cc: Kukjin Kim <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
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@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),
{},

2012-11-23 01:56:06

by Kukjin Kim

[permalink] [raw]
Subject: RE: [PATCH 3/4] ARM: dts: exynos4: add node for PL330 MDMA1 controller

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 <[email protected]>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
>
>
> From: Bartlomiej Zolnierkiewicz <[email protected]>
> 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 <[email protected]>
> Cc: Kukjin Kim <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> ---
> 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@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),
> {},

Subject: Re: [PATCH 1/4] DMA: PL330: fix locking in pl330_free_chan_resources()

On Monday 29 October 2012 22:48:05 Jassi Brar wrote:
> On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
> <[email protected]> wrote:
> > 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 <[email protected]>
> > Cc: Vinod Koul <[email protected]>
> > Cc: Tomasz Figa <[email protected]>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
> > Signed-off-by: Kyungmin Park <[email protected]>
> > ---
> > 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 <[email protected]>

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

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

Subject: Re: [PATCH 4/4] DMA: PL330: add device tree property for DMA_MEMCPY capability

On Friday 09 November 2012 07:11:30 Jassi Brar wrote:
> On 30 October 2012 14:51, Bartlomiej Zolnierkiewicz
> <[email protected]> wrote:
> >
> > Hi,
> >
> > On Monday 29 October 2012 22:45:48 Jassi Brar wrote:
> >> On Mon, Oct 29, 2012 at 10:59 AM, Bartlomiej Zolnierkiewicz
> >> <[email protected]> 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)
> >
> > 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.
>
>
> >> 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 <[email protected]>
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 <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: Kukjin Kim <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Dinh Nguyen <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Tomasz Figa <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
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@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@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@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@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];

2012-11-30 18:27:00

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/4] DMA: PL330: fix locking in pl330_free_chan_resources()

On Fri, 2012-11-30 at 11:59 +0100, Bartlomiej Zolnierkiewicz wrote:
> > Acked-by: Jassi Brar <[email protected]>
>
> Vinod/Dan could you please pick this patch for 3.8? Thanks!
I will check and queue it up today

--
~Vinod