2013-04-08 15:23:32

by Lee Jones

[permalink] [raw]
Subject: [PATCH 1/5] ARM: ux500: Move DMA40 platform data includes file out to include/

The pin names for DB8500 based platforms need to be moved out of
ux500 platform data and into the new proper location in include/
linux/platform_data/. This way we an reference them from other
external locations, such as the main DMA50 driver(s).

Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/board-mop500-audio.c | 2 +-
arch/arm/mach-ux500/board-mop500-sdi.c | 2 +-
arch/arm/mach-ux500/board-mop500.c | 2 +-
arch/arm/mach-ux500/cpu-db8500.c | 2 +-
arch/arm/mach-ux500/devices-db8500.c | 2 +-
drivers/dma/ste_dma40.c | 1 +
.../linux/platform_data/dma-ste-dma40-db8500.h | 3 +--
7 files changed, 7 insertions(+), 7 deletions(-)
rename arch/arm/mach-ux500/ste-dma40-db8500.h => include/linux/platform_data/dma-ste-dma40-db8500.h (98%)

diff --git a/arch/arm/mach-ux500/board-mop500-audio.c b/arch/arm/mach-ux500/board-mop500-audio.c
index 7209db7..1f8c6e4 100644
--- a/arch/arm/mach-ux500/board-mop500-audio.c
+++ b/arch/arm/mach-ux500/board-mop500-audio.c
@@ -9,13 +9,13 @@
#include <linux/gpio.h>
#include <linux/platform_data/pinctrl-nomadik.h>
#include <linux/platform_data/dma-ste-dma40.h>
+#include <linux/platform_data/dma-ste-dma40-db8500.h>

#include <mach/devices.h>
#include <mach/hardware.h>
#include <mach/irqs.h>
#include <mach/msp.h>

-#include "ste-dma40-db8500.h"
#include "board-mop500.h"
#include "devices-db8500.h"
#include "pins-db8500.h"
diff --git a/arch/arm/mach-ux500/board-mop500-sdi.c b/arch/arm/mach-ux500/board-mop500-sdi.c
index 7f2cb6c..e4c0a74 100644
--- a/arch/arm/mach-ux500/board-mop500-sdi.c
+++ b/arch/arm/mach-ux500/board-mop500-sdi.c
@@ -12,6 +12,7 @@
#include <linux/mmc/host.h>
#include <linux/platform_device.h>
#include <linux/platform_data/dma-ste-dma40.h>
+#include <linux/platform_data/dma-ste-dma40-db8500.h>

#include <asm/mach-types.h>
#include <mach/devices.h>
@@ -19,7 +20,6 @@

#include "devices-db8500.h"
#include "board-mop500.h"
-#include "ste-dma40-db8500.h"

/*
* v2 has a new version of this block that need to be forced, the number found
diff --git a/arch/arm/mach-ux500/board-mop500.c b/arch/arm/mach-ux500/board-mop500.c
index 87d2d7b..dd95399 100644
--- a/arch/arm/mach-ux500/board-mop500.c
+++ b/arch/arm/mach-ux500/board-mop500.c
@@ -38,6 +38,7 @@
#include <linux/pinctrl/consumer.h>
#include <linux/platform_data/pinctrl-nomadik.h>
#include <linux/platform_data/dma-ste-dma40.h>
+#include <linux/platform_data/dma-ste-dma40-db8500.h>

#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -48,7 +49,6 @@
#include <mach/irqs.h>
#include <linux/platform_data/crypto-ux500.h>

-#include "ste-dma40-db8500.h"
#include "devices-db8500.h"
#include "board-mop500.h"
#include "board-mop500-regulators.h"
diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index e027865..8553e14 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -22,6 +22,7 @@
#include <linux/of_platform.h>
#include <linux/regulator/machine.h>
#include <linux/platform_data/pinctrl-nomadik.h>
+#include <linux/platform_data/dma-ste-dma40-db8500.h>
#include <linux/random.h>

#include <asm/pmu.h>
@@ -35,7 +36,6 @@
#include <mach/irqs.h>

#include "devices-db8500.h"
-#include "ste-dma40-db8500.h"

#include "board-mop500.h"
#include "id.h"
diff --git a/arch/arm/mach-ux500/devices-db8500.c b/arch/arm/mach-ux500/devices-db8500.c
index afa5b04..a7c0e521 100644
--- a/arch/arm/mach-ux500/devices-db8500.c
+++ b/arch/arm/mach-ux500/devices-db8500.c
@@ -13,6 +13,7 @@
#include <linux/amba/bus.h>
#include <linux/amba/pl022.h>
#include <linux/platform_data/dma-ste-dma40.h>
+#include <linux/platform_data/dma-ste-dma40-db8500.h>
#include <linux/mfd/dbx500-prcmu.h>

#include <mach/hardware.h>
@@ -20,7 +21,6 @@
#include <mach/irqs.h>

#include "devices-db8500.h"
-#include "ste-dma40-db8500.h"

static struct resource dma40_resources[] = {
[0] = {
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 1734fee..35f97d2 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -20,6 +20,7 @@
#include <linux/amba/bus.h>
#include <linux/regulator/consumer.h>
#include <linux/platform_data/dma-ste-dma40.h>
+#include <linux/platform_data/dma-ste-dma40-db8500.h>

#include "dmaengine.h"
#include "ste_dma40_ll.h"
diff --git a/arch/arm/mach-ux500/ste-dma40-db8500.h b/include/linux/platform_data/dma-ste-dma40-db8500.h
similarity index 98%
rename from arch/arm/mach-ux500/ste-dma40-db8500.h
rename to include/linux/platform_data/dma-ste-dma40-db8500.h
index a616419..f538917 100644
--- a/arch/arm/mach-ux500/ste-dma40-db8500.h
+++ b/include/linux/platform_data/dma-ste-dma40-db8500.h
@@ -1,8 +1,7 @@
/*
- * arch/arm/mach-ux500/ste_dma40_db8500.h
* DB8500-SoC-specific configuration for DMA40
*
- * Copyright (C) ST-Ericsson 2007-2010
+ * Copyright (C) ST-Ericsson 2007-2013
* License terms: GNU General Public License (GPL) version 2
* Author: Per Friden <[email protected]>
* Author: Jonas Aaberg <[email protected]>
--
1.7.10.4


2013-04-08 15:23:37

by Lee Jones

[permalink] [raw]
Subject: [PATCH 3/5] dmaengine: ste_dma40: Move default memcpy configs into the driver

There are only two default memcpy configurations used for the DMA40
driver; one for physical memcpy and one for logical memcpy. Instead
of invariably passing the same configurations though platform data,
we're moving them into the driver instead.

Cc: Vinod Koul <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Per Forlin <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/devices-db8500.c | 28 -----------------------
drivers/dma/ste_dma40.c | 32 +++++++++++++++++++++++++--
include/linux/platform_data/dma-ste-dma40.h | 4 ----
3 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/arch/arm/mach-ux500/devices-db8500.c b/arch/arm/mach-ux500/devices-db8500.c
index 2a63c19..c7da6e1 100644
--- a/arch/arm/mach-ux500/devices-db8500.c
+++ b/arch/arm/mach-ux500/devices-db8500.c
@@ -42,32 +42,6 @@ static struct resource dma40_resources[] = {
}
};

-/* Default configuration for physcial memcpy */
-struct stedma40_chan_cfg dma40_memcpy_conf_phy = {
- .mode = STEDMA40_MODE_PHYSICAL,
- .dir = STEDMA40_MEM_TO_MEM,
-
- .src_info.data_width = STEDMA40_BYTE_WIDTH,
- .src_info.psize = STEDMA40_PSIZE_PHY_1,
- .src_info.flow_ctrl = STEDMA40_NO_FLOW_CTRL,
-
- .dst_info.data_width = STEDMA40_BYTE_WIDTH,
- .dst_info.psize = STEDMA40_PSIZE_PHY_1,
- .dst_info.flow_ctrl = STEDMA40_NO_FLOW_CTRL,
-};
-/* Default configuration for logical memcpy */
-struct stedma40_chan_cfg dma40_memcpy_conf_log = {
- .dir = STEDMA40_MEM_TO_MEM,
-
- .src_info.data_width = STEDMA40_BYTE_WIDTH,
- .src_info.psize = STEDMA40_PSIZE_LOG_1,
- .src_info.flow_ctrl = STEDMA40_NO_FLOW_CTRL,
-
- .dst_info.data_width = STEDMA40_BYTE_WIDTH,
- .dst_info.psize = STEDMA40_PSIZE_LOG_1,
- .dst_info.flow_ctrl = STEDMA40_NO_FLOW_CTRL,
-};
-
/*
* Mapping between destination event lines and physical device address.
* The event line is tied to a device and therefore the address is constant.
@@ -150,8 +124,6 @@ static struct stedma40_platform_data dma40_plat_data = {
.dev_len = DB8500_DMA_NR_DEV,
.dev_rx = dma40_rx_map,
.dev_tx = dma40_tx_map,
- .memcpy_conf_phy = &dma40_memcpy_conf_phy,
- .memcpy_conf_log = &dma40_memcpy_conf_log,
.disabled_channels = {-1},
};

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 3ae5f99..ecca492 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -66,6 +66,34 @@ static int dma40_memcpy_channels[] = {
DB8500_DMA_MEMCPY_TX_5,
};

+/* Default configuration for physcial memcpy */
+struct stedma40_chan_cfg dma40_memcpy_conf_phy = {
+ .mode = STEDMA40_MODE_PHYSICAL,
+ .dir = STEDMA40_MEM_TO_MEM,
+
+ .src_info.data_width = STEDMA40_BYTE_WIDTH,
+ .src_info.psize = STEDMA40_PSIZE_PHY_1,
+ .src_info.flow_ctrl = STEDMA40_NO_FLOW_CTRL,
+
+ .dst_info.data_width = STEDMA40_BYTE_WIDTH,
+ .dst_info.psize = STEDMA40_PSIZE_PHY_1,
+ .dst_info.flow_ctrl = STEDMA40_NO_FLOW_CTRL,
+};
+
+/* Default configuration for logical memcpy */
+struct stedma40_chan_cfg dma40_memcpy_conf_log = {
+ .mode = STEDMA40_MODE_LOGICAL,
+ .dir = STEDMA40_MEM_TO_MEM,
+
+ .src_info.data_width = STEDMA40_BYTE_WIDTH,
+ .src_info.psize = STEDMA40_PSIZE_LOG_1,
+ .src_info.flow_ctrl = STEDMA40_NO_FLOW_CTRL,
+
+ .dst_info.data_width = STEDMA40_BYTE_WIDTH,
+ .dst_info.psize = STEDMA40_PSIZE_LOG_1,
+ .dst_info.flow_ctrl = STEDMA40_NO_FLOW_CTRL,
+};
+
/**
* enum 40_command - The different commands and/or statuses.
*
@@ -2023,13 +2051,13 @@ static int d40_config_memcpy(struct d40_chan *d40c)
dma_cap_mask_t cap = d40c->chan.device->cap_mask;

if (dma_has_cap(DMA_MEMCPY, cap) && !dma_has_cap(DMA_SLAVE, cap)) {
- d40c->dma_cfg = *d40c->base->plat_data->memcpy_conf_log;
+ d40c->dma_cfg = dma40_memcpy_conf_log;
d40c->dma_cfg.src_dev_type = STEDMA40_DEV_SRC_MEMORY;
d40c->dma_cfg.dst_dev_type = dma40_memcpy_channels[d40c->chan.chan_id];

} else if (dma_has_cap(DMA_MEMCPY, cap) &&
dma_has_cap(DMA_SLAVE, cap)) {
- d40c->dma_cfg = *d40c->base->plat_data->memcpy_conf_phy;
+ d40c->dma_cfg = dma40_memcpy_conf_phy;
} else {
chan_err(d40c, "No memcpy\n");
return -EINVAL;
diff --git a/include/linux/platform_data/dma-ste-dma40.h b/include/linux/platform_data/dma-ste-dma40.h
index a808784..869c571 100644
--- a/include/linux/platform_data/dma-ste-dma40.h
+++ b/include/linux/platform_data/dma-ste-dma40.h
@@ -141,8 +141,6 @@ struct stedma40_chan_cfg {
* @dev_len: length of dev_tx and dev_rx
* @dev_tx: mapping between destination event line and io address
* @dev_rx: mapping between source event line and io address
- * @memcpy_conf_phy: default configuration of physical channel memcpy
- * @memcpy_conf_log: default configuration of logical channel memcpy
* @disabled_channels: A vector, ending with -1, that marks physical channels
* that are for different reasons not available for the driver.
* @soft_lli_chans: A vector, that marks physical channels will use LLI by SW
@@ -160,8 +158,6 @@ struct stedma40_platform_data {
u32 dev_len;
const dma_addr_t *dev_tx;
const dma_addr_t *dev_rx;
- struct stedma40_chan_cfg *memcpy_conf_phy;
- struct stedma40_chan_cfg *memcpy_conf_log;
int disabled_channels[STEDMA40_MAX_PHYS];
int *soft_lli_chans;
int num_of_soft_lli_chans;
--
1.7.10.4

2013-04-08 15:23:35

by Lee Jones

[permalink] [raw]
Subject: [PATCH 2/5] dmaengine: ste_dma40: Assign memcpy channels in the driver

The channels reserved for memcpy are the same for all currently
supported platforms. With this in mind, we can ease the platform
data passing requirement by moving these assignments out from
platform code and place them directly into the driver.

Cc: Vinod Koul <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Per Forlin <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/mach-ux500/devices-db8500.c | 12 ------------
drivers/dma/ste_dma40.c | 19 ++++++++++++++-----
include/linux/platform_data/dma-ste-dma40.h | 4 ----
3 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-ux500/devices-db8500.c b/arch/arm/mach-ux500/devices-db8500.c
index a7c0e521..2a63c19 100644
--- a/arch/arm/mach-ux500/devices-db8500.c
+++ b/arch/arm/mach-ux500/devices-db8500.c
@@ -146,22 +146,10 @@ static const dma_addr_t dma40_rx_map[DB8500_DMA_NR_DEV] = {
[DB8500_DMA_DEV48_CAC1_RX] = U8500_CRYP1_BASE + CRYP1_RX_REG_OFFSET,
};

-/* Reserved event lines for memcpy only */
-static int dma40_memcpy_event[] = {
- DB8500_DMA_MEMCPY_TX_0,
- DB8500_DMA_MEMCPY_TX_1,
- DB8500_DMA_MEMCPY_TX_2,
- DB8500_DMA_MEMCPY_TX_3,
- DB8500_DMA_MEMCPY_TX_4,
- DB8500_DMA_MEMCPY_TX_5,
-};
-
static struct stedma40_platform_data dma40_plat_data = {
.dev_len = DB8500_DMA_NR_DEV,
.dev_rx = dma40_rx_map,
.dev_tx = dma40_tx_map,
- .memcpy = dma40_memcpy_event,
- .memcpy_len = ARRAY_SIZE(dma40_memcpy_event),
.memcpy_conf_phy = &dma40_memcpy_conf_phy,
.memcpy_conf_log = &dma40_memcpy_conf_log,
.disabled_channels = {-1},
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 35f97d2..3ae5f99 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -56,6 +56,16 @@

#define MAX(a, b) (((a) < (b)) ? (b) : (a))

+/* Reserved event lines for memcpy only. */
+static int dma40_memcpy_channels[] = {
+ DB8500_DMA_MEMCPY_TX_0,
+ DB8500_DMA_MEMCPY_TX_1,
+ DB8500_DMA_MEMCPY_TX_2,
+ DB8500_DMA_MEMCPY_TX_3,
+ DB8500_DMA_MEMCPY_TX_4,
+ DB8500_DMA_MEMCPY_TX_5,
+};
+
/**
* enum 40_command - The different commands and/or statuses.
*
@@ -2015,8 +2025,7 @@ static int d40_config_memcpy(struct d40_chan *d40c)
if (dma_has_cap(DMA_MEMCPY, cap) && !dma_has_cap(DMA_SLAVE, cap)) {
d40c->dma_cfg = *d40c->base->plat_data->memcpy_conf_log;
d40c->dma_cfg.src_dev_type = STEDMA40_DEV_SRC_MEMORY;
- d40c->dma_cfg.dst_dev_type = d40c->base->plat_data->
- memcpy[d40c->chan.chan_id];
+ d40c->dma_cfg.dst_dev_type = dma40_memcpy_channels[d40c->chan.chan_id];

} else if (dma_has_cap(DMA_MEMCPY, cap) &&
dma_has_cap(DMA_SLAVE, cap)) {
@@ -2928,7 +2937,7 @@ static int __init d40_dmaengine_init(struct d40_base *base,
}

d40_chan_init(base, &base->dma_memcpy, base->log_chans,
- base->num_log_chans, base->plat_data->memcpy_len);
+ base->num_log_chans, ARRAY_SIZE(dma40_memcpy_channels));

dma_cap_zero(base->dma_memcpy.cap_mask);
dma_cap_set(DMA_MEMCPY, base->dma_memcpy.cap_mask);
@@ -3216,7 +3225,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
num_log_chans++;

base = kzalloc(ALIGN(sizeof(struct d40_base), 4) +
- (num_phy_chans + num_log_chans + plat_data->memcpy_len) *
+ (num_phy_chans + num_log_chans + ARRAY_SIZE(dma40_memcpy_channels)) *
sizeof(struct d40_chan), GFP_KERNEL);

if (base == NULL) {
@@ -3277,7 +3286,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
if (!base->lookup_phy_chans)
goto failure;

- if (num_log_chans + plat_data->memcpy_len) {
+ if (num_log_chans + ARRAY_SIZE(dma40_memcpy_channels)) {
/*
* The max number of logical channels are event lines for all
* src devices and dst devices
diff --git a/include/linux/platform_data/dma-ste-dma40.h b/include/linux/platform_data/dma-ste-dma40.h
index 4b78101..a808784 100644
--- a/include/linux/platform_data/dma-ste-dma40.h
+++ b/include/linux/platform_data/dma-ste-dma40.h
@@ -141,8 +141,6 @@ struct stedma40_chan_cfg {
* @dev_len: length of dev_tx and dev_rx
* @dev_tx: mapping between destination event line and io address
* @dev_rx: mapping between source event line and io address
- * @memcpy: list of memcpy event lines
- * @memcpy_len: length of memcpy
* @memcpy_conf_phy: default configuration of physical channel memcpy
* @memcpy_conf_log: default configuration of logical channel memcpy
* @disabled_channels: A vector, ending with -1, that marks physical channels
@@ -162,8 +160,6 @@ struct stedma40_platform_data {
u32 dev_len;
const dma_addr_t *dev_tx;
const dma_addr_t *dev_rx;
- int *memcpy;
- u32 memcpy_len;
struct stedma40_chan_cfg *memcpy_conf_phy;
struct stedma40_chan_cfg *memcpy_conf_log;
int disabled_channels[STEDMA40_MAX_PHYS];
--
1.7.10.4

2013-04-08 15:23:42

by Lee Jones

[permalink] [raw]
Subject: [PATCH 4/5] dmaengine: ste_dma40: Only configure a channel during a configure request

According to the DMA documentation allocating a channel and configuring
it are two separate actions. By moving the configuration code into the
correct code path we lighten the burden on the information required to
successfully allocate a channel.

Cc: Vinod Koul <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Per Forlin <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/dma/ste_dma40.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index ecca492..3543ea4 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2471,16 +2471,10 @@ static int d40_alloc_chan_resources(struct dma_chan *chan)
}

pm_runtime_get_sync(d40c->base->dev);
- /* Fill in basic CFG register values */
- d40_phy_cfg(&d40c->dma_cfg, &d40c->src_def_cfg,
- &d40c->dst_def_cfg, chan_is_logical(d40c));

d40_set_prio_realtime(d40c);

if (chan_is_logical(d40c)) {
- d40_log_cfg(&d40c->dma_cfg,
- &d40c->log_def.lcsp1, &d40c->log_def.lcsp3);
-
if (d40c->dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
d40c->lcpa = d40c->base->lcpa_base +
d40c->dma_cfg.src_dev_type * D40_LCPA_CHAN_SIZE;
--
1.7.10.4

2013-04-08 15:31:32

by Lee Jones

[permalink] [raw]
Subject: [PATCH 5/5] dmaengine: ste_dma40: Move LCPA allocation and real-time config

There are various pieces of configuration which do not need to happen
until after a channel is allocated. Here we move some of these so they're
dealt with when it's time to configure the channel, rather than allocate
it.

Cc: Vinod Koul <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Per Forlin <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/dma/ste_dma40.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 3543ea4..8a71bf6 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2472,18 +2472,6 @@ static int d40_alloc_chan_resources(struct dma_chan *chan)

pm_runtime_get_sync(d40c->base->dev);

- d40_set_prio_realtime(d40c);
-
- if (chan_is_logical(d40c)) {
- if (d40c->dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
- d40c->lcpa = d40c->base->lcpa_base +
- d40c->dma_cfg.src_dev_type * D40_LCPA_CHAN_SIZE;
- else
- d40c->lcpa = d40c->base->lcpa_base +
- d40c->dma_cfg.dst_dev_type *
- D40_LCPA_CHAN_SIZE + D40_LCPA_CHAN_DST_DELTA;
- }
-
dev_dbg(chan2dev(d40c), "allocated %s channel (phy %d%s)\n",
chan_is_logical(d40c) ? "logical" : "physical",
d40c->phy_chan->num,
@@ -2830,6 +2818,18 @@ static int d40_set_runtime_config(struct dma_chan *chan,
d40_phy_cfg(cfg, &d40c->src_def_cfg,
&d40c->dst_def_cfg, false);

+ d40_set_prio_realtime(d40c);
+
+ if (chan_is_logical(d40c)) {
+ if (cfg->dir == STEDMA40_PERIPH_TO_MEM)
+ d40c->lcpa = d40c->base->lcpa_base +
+ cfg->src_dev_type * D40_LCPA_CHAN_SIZE;
+ else
+ d40c->lcpa = d40c->base->lcpa_base +
+ cfg->dst_dev_type *
+ D40_LCPA_CHAN_SIZE + D40_LCPA_CHAN_DST_DELTA;
+ }
+
/* These settings will take precedence later */
d40c->runtime_addr = config_addr;
d40c->runtime_direction = config->direction;
--
1.7.10.4

2013-04-08 16:13:54

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/5] dmaengine: ste_dma40: Assign memcpy channels in the driver

On Monday 08 April 2013, Lee Jones wrote:
> The channels reserved for memcpy are the same for all currently
> supported platforms. With this in mind, we can ease the platform
> data passing requirement by moving these assignments out from
> platform code and place them directly into the driver.
>
> Cc: Vinod Koul <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Per Forlin <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

2013-04-08 16:14:05

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 3/5] dmaengine: ste_dma40: Move default memcpy configs into the driver

On Monday 08 April 2013, Lee Jones wrote:
> There are only two default memcpy configurations used for the DMA40
> driver; one for physical memcpy and one for logical memcpy. Instead
> of invariably passing the same configurations though platform data,
> we're moving them into the driver instead.
>
> Cc: Vinod Koul <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Per Forlin <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

2013-04-08 16:17:57

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] ARM: ux500: Move DMA40 platform data includes file out to include/

On Monday 08 April 2013, Lee Jones wrote:
> The pin names for DB8500 based platforms need to be moved out of
> ux500 platform data and into the new proper location in include/
> linux/platform_data/. This way we an reference them from other
> external locations, such as the main DMA50 driver(s).
>
> Signed-off-by: Lee Jones <[email protected]>

Hmm, not sure about this one. The slave numbers are not really platform_data
and I think they should not be exposed to drivers. It makes sense to
keep the numbers for the memcpy channels in the driver itself as they
are hardwired in the driver, but the slave channels should stay in the
platform I think.

Arnd

2013-04-08 16:20:31

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 4/5] dmaengine: ste_dma40: Only configure a channel during a configure request

On Monday 08 April 2013, Lee Jones wrote:
> According to the DMA documentation allocating a channel and configuring
> it are two separate actions. By moving the configuration code into the
> correct code path we lighten the burden on the information required to
> successfully allocate a channel.
>
> Cc: Vinod Koul <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Per Forlin <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>

I think the patch doesn't match the description. You only remove
the calls to configure the channel but don't add it in a proper
place.

Arnd

2013-04-09 07:06:06

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/5] ARM: ux500: Move DMA40 platform data includes file out to include/

On Mon, 08 Apr 2013, Arnd Bergmann wrote:

> On Monday 08 April 2013, Lee Jones wrote:
> > The pin names for DB8500 based platforms need to be moved out of
> > ux500 platform data and into the new proper location in include/
> > linux/platform_data/. This way we an reference them from other
> > external locations, such as the main DMA50 driver(s).
> >
> > Signed-off-by: Lee Jones <[email protected]>
>
> Hmm, not sure about this one. The slave numbers are not really platform_data
> and I think they should not be exposed to drivers. It makes sense to
> keep the numbers for the memcpy channels in the driver itself as they
> are hardwired in the driver, but the slave channels should stay in the
> platform I think.

Just looking at this again now. So you're right in that the main
reason for moving these out of platform code and into the system-wide
platform data area is for the memcpy channels. However, I'd prefer to
keep the channel allocation enum fully contiguous rather than
splitting them out.

The channels are only exposed to the one external driver which
includes them, and that's the D40 driver.

How against this move are you?

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-04-09 07:12:00

by Lee Jones

[permalink] [raw]
Subject: [PATCH 4/5] dmaengine: ste_dma40: Do not configure channels during an channel allocation

According to the DMA documentation allocating a channel and configuring
it are two separate actions. By removing the configuration code from the
channel allocation path we lighten the burden on the information required to
successfully allocate a channel.

Cc: Vinod Koul <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Per Forlin <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/dma/ste_dma40.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index ecca492..3543ea4 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2471,16 +2471,10 @@ static int d40_alloc_chan_resources(struct dma_chan *chan)
}

pm_runtime_get_sync(d40c->base->dev);
- /* Fill in basic CFG register values */
- d40_phy_cfg(&d40c->dma_cfg, &d40c->src_def_cfg,
- &d40c->dst_def_cfg, chan_is_logical(d40c));

d40_set_prio_realtime(d40c);

if (chan_is_logical(d40c)) {
- d40_log_cfg(&d40c->dma_cfg,
- &d40c->log_def.lcsp1, &d40c->log_def.lcsp3);
-
if (d40c->dma_cfg.dir == STEDMA40_PERIPH_TO_MEM)
d40c->lcpa = d40c->base->lcpa_base +
d40c->dma_cfg.src_dev_type * D40_LCPA_CHAN_SIZE;
--
1.7.10.4

2013-04-09 09:35:27

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] ARM: ux500: Move DMA40 platform data includes file out to include/

On Tuesday 09 April 2013, Lee Jones wrote:
> On Mon, 08 Apr 2013, Arnd Bergmann wrote:
>
> > On Monday 08 April 2013, Lee Jones wrote:
> > > The pin names for DB8500 based platforms need to be moved out of
> > > ux500 platform data and into the new proper location in include/
> > > linux/platform_data/. This way we an reference them from other
> > > external locations, such as the main DMA50 driver(s).
> > >
> > > Signed-off-by: Lee Jones <[email protected]>
> >
> > Hmm, not sure about this one. The slave numbers are not really platform_data
> > and I think they should not be exposed to drivers. It makes sense to
> > keep the numbers for the memcpy channels in the driver itself as they
> > are hardwired in the driver, but the slave channels should stay in the
> > platform I think.
>
> Just looking at this again now. So you're right in that the main
> reason for moving these out of platform code and into the system-wide
> platform data area is for the memcpy channels. However, I'd prefer to
> keep the channel allocation enum fully contiguous rather than
> splitting them out.
>
> The channels are only exposed to the one external driver which
> includes them, and that's the D40 driver.
>
> How against this move are you?

I think it's a move in the wrong direction. If you think we should keep the
knowledge of which channels are used for memcpy outside of the driver,
I would prefer you to drop the the first two patches, which implies that
we will eventually pass that information using DT properties in order get
rid of the auxdata.

Arnd

2013-04-09 09:36:15

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 4/5] dmaengine: ste_dma40: Do not configure channels during an channel allocation

On Tuesday 09 April 2013, Lee Jones wrote:
> According to the DMA documentation allocating a channel and configuring
> it are two separate actions. By removing the configuration code from the
> channel allocation path we lighten the burden on the information required to
> successfully allocate a channel.

Did you send out the first version again? The text is the same as before.

Arnd

2013-04-09 10:04:42

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 4/5] dmaengine: ste_dma40: Do not configure channels during an channel allocation

On Tue, 09 Apr 2013, Arnd Bergmann wrote:

> On Tuesday 09 April 2013, Lee Jones wrote:
> > According to the DMA documentation allocating a channel and configuring
> > it are two separate actions. By removing the configuration code from the
> > channel allocation path we lighten the burden on the information required to
> > successfully allocate a channel.
>
> Did you send out the first version again? The text is the same as before.

No, the first commit message implied I'd moved the config code from
'allocate' to 'configure', whereas in reality I just removed it from
'allocate', as it's already in 'configure'.

This changelog is different (and more accurate) than the first.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-04-09 10:28:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 4/5] dmaengine: ste_dma40: Do not configure channels during an channel allocation

On Tuesday 09 April 2013, Lee Jones wrote:
> According to the DMA documentation allocating a channel and configuring
> it are two separate actions. By removing the configuration code from the
> channel allocation path we lighten the burden on the information required to
> successfully allocate a channel.
>
> Cc: Vinod Koul <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Per Forlin <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

2013-04-15 10:47:12

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/5] ARM: ux500: Move DMA40 platform data includes file out to include/

On Mon, Apr 8, 2013 at 5:23 PM, Lee Jones <[email protected]> wrote:

> The pin names for DB8500 based platforms need to be moved out of

"pin names" ???

> ux500 platform data and into the new proper location in include/
> linux/platform_data/. This way we an reference them from other
> external locations, such as the main DMA50 driver(s).
>
> Signed-off-by: Lee Jones <[email protected]>

(...)
> rename arch/arm/mach-ux500/ste-dma40-db8500.h => include/linux/platform_data/dma-ste-dma40-db8500.h (98%)

Those are the DB8500 DMA channel definitions...

I will not interfere with the fruitful discussion with Arnd on this
subject though, I suspect he's right.

Yours,
Linus Walleij

2013-04-15 10:58:07

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/5] ARM: ux500: Move DMA40 platform data includes file out to include/

On Mon, 15 Apr 2013, Linus Walleij wrote:

> On Mon, Apr 8, 2013 at 5:23 PM, Lee Jones <[email protected]> wrote:
>
> > The pin names for DB8500 based platforms need to be moved out of
>
> "pin names" ???
>
> > ux500 platform data and into the new proper location in include/
> > linux/platform_data/. This way we an reference them from other
> > external locations, such as the main DMA50 driver(s).
> >
> > Signed-off-by: Lee Jones <[email protected]>
>
> (...)
> > rename arch/arm/mach-ux500/ste-dma40-db8500.h => include/linux/platform_data/dma-ste-dma40-db8500.h (98%)
>
> Those are the DB8500 DMA channel definitions...
>
> I will not interfere with the fruitful discussion with Arnd on this
> subject though, I suspect he's right.

It's okay, I've since removed this patch.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog