2015-11-30 13:46:24

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

Hi,

Changes since RFC v01:
- dma_request_chan(); lost the mask parameter
- The new API does not rely on RESOURCE_DMA, instead the dma_filter_map table
will be used to provide the needed information to the filter function in
legacy mode
- Extended the example patches to convert most of daVinci to use the new API to
request the DMA channels.

TODO: Documentation update ;)

As it has been discussed in the following thread:
http://www.gossamer-threads.com/lists/linux/kernel/2181487#2181487

Andy: I did looked at the unified device properties, but I decided to not to use
it as I don't see it to fit well and most of the legacy board files are using
resources to specify at least their memory regions so adding the DMA resource
to them would be more inline with the rest of the code.

The ARM, mmc and spi patches are converting daVinci drivers board files to use
the new method of requesting DMA channel.

With this series I have taken a path which would result two new API, which can
be used to convert most of the current users already and with some work all
users might be able to move to this set.
With this set the filter_fn used for legacy (non DT/ACPI) channel request is no
longer needed to be exported to client drivers since the selection of the
correct filter_fn will be done in the core.

So, the first proposal is to have:

struct dma_chan *dma_request_chan(struct device *dev, const char *name);
struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);

The dma_request_chan_by_mask() is to request any channel matching with the
requested capabilities, can be used to request channel for memcpy, memset, xor,
etc where no hardware synchronization is needed.

The dma_request_chan() is to request a slave channel. The dma_request_chan() will try to find the
channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
it will use a filter lookup table and retrieves the needed information from
the dma_filter_map provided by the DMA drivers.
This legacy mode needs changes in platform code, in dmaengine drivers and
finally the dmaengine user drivers can be converted:

For each dmaengine driver an array of DMA device, slave and the parameter
for the filter function needs to be added:

static struct dma_filter_map da830_edma_map[] = {
DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
};

This information is going to be needed by the dmaengine driver, so
modification to the platform_data is needed, and the driver map should be
added to the pdata of the DMA driver:

da8xx_edma0_pdata.filter_map = da830_edma_map;
da8xx_edma0_pdata.filtercnt = ARRAY_SIZE(da830_edma_map);

The DMA driver then needs to convigure the needed device -> filter_fn
mapping before it registers with dma_async_device_register() :

if (info->filter_map) {
ecc->dma_slave.filter_map.map = info->filter_map;
ecc->dma_slave.filter_map.mapcnt = info->filtercnt;
ecc->dma_slave.filter_map.filter_fn = edma_filter_for_map;
}

When neither DT or ACPI lookup is available the dma_request_chan() will
try to match the requester's device name with the filter_map's list of
device names, when a match found it will use the information from the
dma_filter_map to get the channel with the dma_get_channel() internal
function.

Regards,
Peter
---
Peter Ujfalusi (15):
dmaengine: core: Allow NULL mask pointer in
__dma_device_satisfies_mask()
dmaengine: core: Move and merge the code paths using private_candidate
dmaengine: core: Introduce new, universal API to request a channel
dmaengine: edma: Add support for DMA filter mapping to slave devices
ARM: davinci: devices-da8xx: Add dma_filter_map to edma
ARM: davinci: dm355: Add dma_filter_map to edma
ARM: davinci: dm365: Add dma_filter_map to edma
ARM: davinci: dm644x: Add dma_filter_map to edma
ARM: davinci: dm646x: Add dma_filter_map to edma
mmc: davinci_mmc: Use dma_request_chan() to requesting DMA channel
spi: davinci: Use dma_request_chan() to requesting DMA channel
ARM: davinci: devices-da8xx: Remove DMA resources for MMC and SPI
ARM: davinci: devices: Remove DMA resources for MMC
ARM: davinci: dm355: Remove DMA resources for SPI
ARM: davinci: dm365: Remove DMA resources for SPI

arch/arm/mach-davinci/devices-da8xx.c | 95 ++++++++++----------
arch/arm/mach-davinci/devices.c | 19 ----
arch/arm/mach-davinci/dm355.c | 28 ++++--
arch/arm/mach-davinci/dm365.c | 30 +++++--
arch/arm/mach-davinci/dm644x.c | 12 +++
arch/arm/mach-davinci/dm646x.c | 11 +++
drivers/dma/dmaengine.c | 160 +++++++++++++++++++++++++---------
drivers/dma/edma.c | 24 +++++
drivers/mmc/host/davinci_mmc.c | 52 +++--------
drivers/spi/spi-davinci.c | 76 +++++-----------
include/linux/dmaengine.h | 31 +++++++
include/linux/platform_data/edma.h | 5 ++
12 files changed, 330 insertions(+), 213 deletions(-)

--
2.6.3


2015-11-30 13:46:26

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 01/15] dmaengine: core: Allow NULL mask pointer in __dma_device_satisfies_mask()

Treat as true condition the case when the mask is NULL.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/dmaengine.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index daf54a39bcc7..52c3eee48e2e 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -184,6 +184,9 @@ __dma_device_satisfies_mask(struct dma_device *device,
{
dma_cap_mask_t has;

+ if (!want)
+ return true;
+
bitmap_and(has.bits, want->bits, device->cap_mask.bits,
DMA_TX_TYPE_END);
return bitmap_equal(want->bits, has.bits, DMA_TX_TYPE_END);
--
2.6.3

2015-11-30 13:46:30

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 02/15] dmaengine: core: Move and merge the code paths using private_candidate

Channel matching with private_candidate() is used in two paths, the error
checking is slightly different in them and they are duplicating code also.
Move the code under dma_get_channel() to provide consistent execution and
going to allow us to reuse this mode of channel lookup later.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/dmaengine.c | 81 +++++++++++++++++++++++++------------------------
1 file changed, 42 insertions(+), 39 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 52c3eee48e2e..1249165fb4b2 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -549,6 +549,42 @@ static struct dma_chan *private_candidate(const dma_cap_mask_t *mask,
return NULL;
}

+static struct dma_chan *dma_get_channel(struct dma_device *device,
+ const dma_cap_mask_t *mask,
+ dma_filter_fn fn, void *fn_param)
+{
+ struct dma_chan *chan = private_candidate(mask, device, fn, fn_param);
+ int err;
+
+ if (chan) {
+ /* Found a suitable channel, try to grab, prep, and return it.
+ * We first set DMA_PRIVATE to disable balance_ref_count as this
+ * channel will not be published in the general-purpose
+ * allocator
+ */
+ dma_cap_set(DMA_PRIVATE, device->cap_mask);
+ device->privatecnt++;
+ err = dma_chan_get(chan);
+
+ if (err) {
+ if (err == -ENODEV) {
+ pr_debug("%s: %s module removed\n", __func__,
+ dma_chan_name(chan));
+ list_del_rcu(&device->global_node);
+ } else
+ pr_debug("%s: failed to get %s: (%d)\n",
+ __func__, dma_chan_name(chan), err);
+
+ if (--device->privatecnt == 0)
+ dma_cap_clear(DMA_PRIVATE, device->cap_mask);
+
+ chan = ERR_PTR(err);
+ }
+ }
+
+ return chan ? chan : ERR_PTR(-EPROBE_DEFER);
+}
+
/**
* dma_get_slave_channel - try to get specific channel exclusively
* @chan: target channel
@@ -587,7 +623,6 @@ struct dma_chan *dma_get_any_slave_channel(struct dma_device *device)
{
dma_cap_mask_t mask;
struct dma_chan *chan;
- int err;

dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
@@ -595,23 +630,11 @@ struct dma_chan *dma_get_any_slave_channel(struct dma_device *device)
/* lock against __dma_request_channel */
mutex_lock(&dma_list_mutex);

- chan = private_candidate(&mask, device, NULL, NULL);
- if (chan) {
- dma_cap_set(DMA_PRIVATE, device->cap_mask);
- device->privatecnt++;
- err = dma_chan_get(chan);
- if (err) {
- pr_debug("%s: failed to get %s: (%d)\n",
- __func__, dma_chan_name(chan), err);
- chan = NULL;
- if (--device->privatecnt == 0)
- dma_cap_clear(DMA_PRIVATE, device->cap_mask);
- }
- }
+ chan = dma_get_channel(device, &mask, NULL, NULL);

mutex_unlock(&dma_list_mutex);

- return chan;
+ return IS_ERR(chan) ? NULL : chan;
}
EXPORT_SYMBOL_GPL(dma_get_any_slave_channel);

@@ -628,35 +651,15 @@ struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask,
{
struct dma_device *device, *_d;
struct dma_chan *chan = NULL;
- int err;

/* Find a channel */
mutex_lock(&dma_list_mutex);
list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
- chan = private_candidate(mask, device, fn, fn_param);
- if (chan) {
- /* Found a suitable channel, try to grab, prep, and
- * return it. We first set DMA_PRIVATE to disable
- * balance_ref_count as this channel will not be
- * published in the general-purpose allocator
- */
- dma_cap_set(DMA_PRIVATE, device->cap_mask);
- device->privatecnt++;
- err = dma_chan_get(chan);
+ chan = dma_get_channel(device, mask, fn, fn_param);
+ if (!IS_ERR(chan))
+ break;

- if (err == -ENODEV) {
- pr_debug("%s: %s module removed\n",
- __func__, dma_chan_name(chan));
- list_del_rcu(&device->global_node);
- } else if (err)
- pr_debug("%s: failed to get %s: (%d)\n",
- __func__, dma_chan_name(chan), err);
- else
- break;
- if (--device->privatecnt == 0)
- dma_cap_clear(DMA_PRIVATE, device->cap_mask);
- chan = NULL;
- }
+ chan = NULL;
}
mutex_unlock(&dma_list_mutex);

--
2.6.3

2015-11-30 13:50:42

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

The two API function can cover most, if not all current APIs used to
request a channel. With minimal effort dmaengine drivers, platforms and
dmaengine user drivers can be converted to use the two function.

struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);

To request any channel matching with the requested capabilities, can be
used to request channel for memcpy, memset, xor, etc where no hardware
synchronization is needed.

struct dma_chan *dma_request_chan(struct device *dev, const char *name);
To request a slave channel. The dma_request_chan() will try to find the
channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
it will use a filter lookup table and retrieves the needed information from
the dma_filter_map provided by the DMA drivers.
This legacy mode needs changes in platform code, in dmaengine drivers and
finally the dmaengine user drivers can be converted:

For each dmaengine driver an array of DMA device, slave and the parameter
for the filter function needs to be added:

static struct dma_filter_map da830_edma_map[] = {
DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
};

This information is going to be needed by the dmaengine driver, so
modification to the platform_data is needed, and the driver map should be
added to the pdata of the DMA driver:

da8xx_edma0_pdata.filter_map = da830_edma_map;
da8xx_edma0_pdata.filtercnt = ARRAY_SIZE(da830_edma_map);

The DMA driver then needs to convigure the needed device -> filter_fn
mapping before it registers with dma_async_device_register() :

if (info->filter_map) {
ecc->dma_slave.filter_map.map = info->filter_map;
ecc->dma_slave.filter_map.mapcnt = info->filtercnt;
ecc->dma_slave.filter_map.filter_fn = edma_filter_for_map;
}

When neither DT or ACPI lookup is available the dma_request_chan() will
try to match the requester's device name with the filter_map's list of
device names, when a match found it will use the information from the
dma_filter_map to get the channel with the dma_get_channel() internal
function.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/dmaengine.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/dmaengine.h | 31 +++++++++++++++++++
2 files changed, 107 insertions(+)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 1249165fb4b2..bd774adff697 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -43,6 +43,7 @@

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <linux/platform_device.h>
#include <linux/dma-mapping.h>
#include <linux/init.h>
#include <linux/module.h>
@@ -715,6 +716,81 @@ struct dma_chan *dma_request_slave_channel(struct device *dev,
}
EXPORT_SYMBOL_GPL(dma_request_slave_channel);

+static struct dma_filter_map *dma_filter_match(struct dma_device *device,
+ const char *name,
+ struct device *dev)
+{
+ struct dma_filter_map *map_found = NULL;
+ int i;
+
+ if (!device->filter_map.mapcnt)
+ return NULL;
+
+ for (i = 0; i < device->filter_map.mapcnt; i++) {
+ struct dma_filter_map *map = &device->filter_map.map[i];
+
+ if (!strcmp(map->devname, dev_name(dev)) &&
+ !strcmp(map->slave, name)) {
+ map_found = map;
+ break;
+ }
+ }
+
+ return map_found;
+}
+
+struct dma_chan *dma_request_chan(struct device *dev, const char *name)
+{
+ struct dma_device *device, *_d;
+ struct dma_chan *chan = NULL;
+
+ /* If device-tree is present get slave info from here */
+ if (dev->of_node)
+ chan = of_dma_request_slave_channel(dev->of_node, name);
+
+ /* If device was enumerated by ACPI get slave info from here */
+ if (ACPI_HANDLE(dev) && !chan)
+ chan = acpi_dma_request_slave_chan_by_name(dev, name);
+
+ if (chan)
+ return chan;
+
+ /* Try to find the channel via the DMA filter map(s) */
+ mutex_lock(&dma_list_mutex);
+ list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
+ struct dma_filter_map *map = dma_filter_match(device, name,
+ dev);
+
+ if (!map)
+ continue;
+
+ chan = dma_get_channel(device, NULL,
+ device->filter_map.filter_fn,
+ map->param);
+ if (!IS_ERR(chan))
+ break;
+ }
+ mutex_unlock(&dma_list_mutex);
+
+ return chan ? chan : ERR_PTR(-EPROBE_DEFER);
+}
+EXPORT_SYMBOL_GPL(dma_request_chan);
+
+struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask)
+{
+ struct dma_chan *chan;
+
+ if (!mask)
+ return ERR_PTR(-ENODEV);
+
+ chan = __dma_request_channel(mask, NULL, NULL);
+ if (!chan)
+ chan = ERR_PTR(-ENODEV);
+
+ return chan;
+}
+EXPORT_SYMBOL_GPL(dma_request_chan_by_mask);
+
void dma_release_channel(struct dma_chan *chan)
{
mutex_lock(&dma_list_mutex);
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index a2b7c2071cf4..f7b06ff982c8 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -606,6 +606,24 @@ enum dmaengine_alignment {
DMAENGINE_ALIGN_64_BYTES = 6,
};

+struct dma_filter_map {
+ char *devname;
+ char *slave;
+ void *param;
+};
+#define DMA_FILTER_ENTRY(_devname, _slave, _param) \
+ { \
+ .devname = _devname, \
+ .slave = _slave, \
+ .param = (void*)_param, \
+ }
+
+struct dma_filter {
+ dma_filter_fn filter_fn;
+ int mapcnt;
+ struct dma_filter_map *map;
+};
+
/**
* struct dma_device - info on the entity supplying DMA services
* @chancnt: how many DMA channels are supported
@@ -669,6 +687,7 @@ struct dma_device {
unsigned int privatecnt;
struct list_head channels;
struct list_head global_node;
+ struct dma_filter filter_map;
dma_cap_mask_t cap_mask;
unsigned short max_xor;
unsigned short max_pq;
@@ -1235,6 +1254,10 @@ struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask,
struct dma_chan *dma_request_slave_channel_reason(struct device *dev,
const char *name);
struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
+
+struct dma_chan *dma_request_chan(struct device *dev, const char *name);
+struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
+
void dma_release_channel(struct dma_chan *chan);
int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);
#else
@@ -1268,6 +1291,14 @@ static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
{
return NULL;
}
+static inline struct dma_chan *dma_request_chan(struct device *dev)
+{
+ return ERR_PTR(-ENODEV);
+}
+static inline struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask)
+{
+ return ERR_PTR(-ENODEV);
+}
static inline void dma_release_channel(struct dma_chan *chan)
{
}
--
2.6.3

2015-11-30 13:46:36

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 04/15] dmaengine: edma: Add support for DMA filter mapping to slave devices

Add support for providing device to filter_fn mapping so client drivers
can switch to use the dma_request_chan() API.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/edma.c | 24 ++++++++++++++++++++++++
include/linux/platform_data/edma.h | 5 +++++
2 files changed, 29 insertions(+)

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 0675e268d577..386f8c9bd606 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -2098,6 +2098,8 @@ static struct dma_chan *of_edma_xlate(struct of_phandle_args *dma_spec,
}
#endif

+static bool edma_filter_for_map(struct dma_chan *chan, void *param);
+
static int edma_probe(struct platform_device *pdev)
{
struct edma_soc_info *info = pdev->dev.platform_data;
@@ -2297,6 +2299,12 @@ static int edma_probe(struct platform_device *pdev)
edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot);
}

+ if (info->filter_map) {
+ ecc->dma_slave.filter_map.map = info->filter_map;
+ ecc->dma_slave.filter_map.mapcnt = info->filtercnt;
+ ecc->dma_slave.filter_map.filter_fn = edma_filter_for_map;
+ }
+
ret = dma_async_device_register(&ecc->dma_slave);
if (ret) {
dev_err(dev, "slave ddev registration failed (%d)\n", ret);
@@ -2428,6 +2436,22 @@ bool edma_filter_fn(struct dma_chan *chan, void *param)
}
EXPORT_SYMBOL(edma_filter_fn);

+static bool edma_filter_for_map(struct dma_chan *chan, void *param)
+{
+ bool match = false;
+
+ if (chan->device->dev->driver == &edma_driver.driver) {
+ struct edma_chan *echan = to_edma_chan(chan);
+ unsigned ch_req = (unsigned)param;
+ if (ch_req == echan->ch_num) {
+ /* The channel is going to be used as HW synchronized */
+ echan->hw_triggered = true;
+ match = true;
+ }
+ }
+ return match;
+}
+
static int edma_init(void)
{
int ret;
diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index e2878baeb90e..117a36d63840 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -59,6 +59,8 @@ struct edma_rsv_info {
const s16 (*rsv_slots)[2];
};

+struct dma_filter_map;
+
/* platform_data for EDMA driver */
struct edma_soc_info {
/*
@@ -76,6 +78,9 @@ struct edma_soc_info {

s8 (*queue_priority_mapping)[2];
const s16 (*xbar_chans)[2];
+
+ struct dma_filter_map *filter_map;
+ int filtercnt;
};

#endif
--
2.6.3

2015-11-30 13:49:46

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 05/15] ARM: davinci: devices-da8xx: Add dma_filter_map to edma

Provide the dma_filter_map to edma which will allow us to move the drivers
to the new, simpler dmaengine API and we can remove the DMA resources also
for the devices.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/mach-davinci/devices-da8xx.c | 46 +++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 28c90bc372bd..3327cb22e0b5 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -17,6 +17,7 @@
#include <linux/ahci_platform.h>
#include <linux/clk.h>
#include <linux/reboot.h>
+#include <linux/dmaengine.h>

#include <mach/cputype.h>
#include <mach/common.h>
@@ -233,16 +234,54 @@ static const struct platform_device_info da850_edma1_device __initconst = {
.size_data = sizeof(da850_edma1_pdata),
};

+static struct dma_filter_map da830_edma_map[] = {
+ DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
+ DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
+ DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
+ DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
+ DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
+ DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
+ DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
+ DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
+ DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
+ DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
+ DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
+ DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
+};
+
int __init da830_register_edma(struct edma_rsv_info *rsv)
{
struct platform_device *edma_pdev;

da8xx_edma0_pdata.rsv = rsv;

+ da8xx_edma0_pdata.filter_map = da830_edma_map;
+ da8xx_edma0_pdata.filtercnt = ARRAY_SIZE(da830_edma_map);
+
edma_pdev = platform_device_register_full(&da8xx_edma0_device);
return IS_ERR(edma_pdev) ? PTR_ERR(edma_pdev) : 0;
}

+static struct dma_filter_map da850_edma0_map[] = {
+ DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
+ DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
+ DMA_FILTER_ENTRY("davinci-mcbsp.0", "rx", EDMA_CTLR_CHAN(0, 2)),
+ DMA_FILTER_ENTRY("davinci-mcbsp.0", "tx", EDMA_CTLR_CHAN(0, 3)),
+ DMA_FILTER_ENTRY("davinci-mcbsp.1", "rx", EDMA_CTLR_CHAN(0, 4)),
+ DMA_FILTER_ENTRY("davinci-mcbsp.1", "tx", EDMA_CTLR_CHAN(0, 5)),
+ DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
+ DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
+ DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
+ DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
+ DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
+ DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
+};
+
+static struct dma_filter_map da850_edma1_map[] = {
+ DMA_FILTER_ENTRY("da830-mmc.1", "tx", EDMA_CTLR_CHAN(0, 28)),
+ DMA_FILTER_ENTRY("da830-mmc.1", "rx", EDMA_CTLR_CHAN(0, 29)),
+};
+
int __init da850_register_edma(struct edma_rsv_info *rsv[2])
{
struct platform_device *edma_pdev;
@@ -252,11 +291,18 @@ int __init da850_register_edma(struct edma_rsv_info *rsv[2])
da850_edma1_pdata.rsv = rsv[1];
}

+ da8xx_edma0_pdata.filter_map = da850_edma0_map;
+ da8xx_edma0_pdata.filtercnt = ARRAY_SIZE(da850_edma0_map);
+
edma_pdev = platform_device_register_full(&da8xx_edma0_device);
if (IS_ERR(edma_pdev)) {
pr_warn("%s: Failed to register eDMA0\n", __func__);
return PTR_ERR(edma_pdev);
}
+
+ da850_edma1_pdata.filter_map = da850_edma1_map;
+ da850_edma1_pdata.filtercnt = ARRAY_SIZE(da850_edma1_map);
+
edma_pdev = platform_device_register_full(&da850_edma1_device);
return IS_ERR(edma_pdev) ? PTR_ERR(edma_pdev) : 0;
}
--
2.6.3

2015-11-30 13:46:41

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 06/15] ARM: davinci: dm355: Add dma_filter_map to edma

Provide the dma_filter_map to edma which will allow us to move the drivers
to the new, simpler dmaengine API and we can remove the DMA resources also
for the devices.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/mach-davinci/dm355.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index 609950b8c191..dc8d7ccf69f4 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -13,6 +13,7 @@
#include <linux/serial_8250.h>
#include <linux/platform_device.h>
#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
#include <linux/spi/spi.h>
#include <linux/platform_data/edma.h>
#include <linux/platform_data/gpio-davinci.h>
@@ -576,9 +577,28 @@ static s8 queue_priority_mapping[][2] = {
{-1, -1},
};

+static struct dma_filter_map da355_edma_map[] = {
+ DMA_FILTER_ENTRY("davinci-mcbsp.0", "tx", EDMA_CTLR_CHAN(0, 2)),
+ DMA_FILTER_ENTRY("davinci-mcbsp.0", "rx", EDMA_CTLR_CHAN(0, 3)),
+ DMA_FILTER_ENTRY("davinci-mcbsp.1", "tx", EDMA_CTLR_CHAN(0, 8)),
+ DMA_FILTER_ENTRY("davinci-mcbsp.1", "rx", EDMA_CTLR_CHAN(0, 9)),
+ DMA_FILTER_ENTRY("spi_davinci.2", "tx", EDMA_CTLR_CHAN(0, 10)),
+ DMA_FILTER_ENTRY("spi_davinci.2", "rx", EDMA_CTLR_CHAN(0, 11)),
+ DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 14)),
+ DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 15)),
+ DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 16)),
+ DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 17)),
+ DMA_FILTER_ENTRY("dm6441-mmc.0", "rx", EDMA_CTLR_CHAN(0, 26)),
+ DMA_FILTER_ENTRY("dm6441-mmc.0", "tx", EDMA_CTLR_CHAN(0, 27)),
+ DMA_FILTER_ENTRY("dm6441-mmc.1", "rx", EDMA_CTLR_CHAN(0, 30)),
+ DMA_FILTER_ENTRY("dm6441-mmc.1", "tx", EDMA_CTLR_CHAN(0, 31)),
+};
+
static struct edma_soc_info dm355_edma_pdata = {
.queue_priority_mapping = queue_priority_mapping,
.default_queue = EVENTQ_1,
+ .filter_map = da355_edma_map,
+ .filtercnt = ARRAY_SIZE(da355_edma_map),
};

static struct resource edma_resources[] = {
--
2.6.3

2015-11-30 13:46:42

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 07/15] ARM: davinci: dm365: Add dma_filter_map to edma

Provide the dma_filter_map to edma which will allow us to move the drivers
to the new, simpler dmaengine API and we can remove the DMA resources also
for the devices.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/mach-davinci/dm365.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index 2068cbeaeb03..3dfcf7c9aa20 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -17,6 +17,7 @@
#include <linux/serial_8250.h>
#include <linux/platform_device.h>
#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
#include <linux/spi/spi.h>
#include <linux/platform_data/edma.h>
#include <linux/platform_data/gpio-davinci.h>
@@ -862,9 +863,30 @@ static s8 dm365_queue_priority_mapping[][2] = {
{-1, -1},
};

+static struct dma_filter_map da365_edma_map[] = {
+ DMA_FILTER_ENTRY("davinci-mcbsp.0", "tx", EDMA_CTLR_CHAN(0, 2)),
+ DMA_FILTER_ENTRY("davinci-mcbsp.0", "rx", EDMA_CTLR_CHAN(0, 3)),
+ DMA_FILTER_ENTRY("davinci_voicecodec", "tx", EDMA_CTLR_CHAN(0, 2)),
+ DMA_FILTER_ENTRY("davinci_voicecodec", "rx", EDMA_CTLR_CHAN(0, 3)),
+ DMA_FILTER_ENTRY("spi_davinci.2", "tx", EDMA_CTLR_CHAN(0, 10)),
+ DMA_FILTER_ENTRY("spi_davinci.2", "rx", EDMA_CTLR_CHAN(0, 11)),
+ DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 14)),
+ DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 15)),
+ DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 16)),
+ DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 17)),
+ DMA_FILTER_ENTRY("spi_davinci.3", "tx", EDMA_CTLR_CHAN(0, 18)),
+ DMA_FILTER_ENTRY("spi_davinci.3", "rx", EDMA_CTLR_CHAN(0, 19)),
+ DMA_FILTER_ENTRY("dm6441-mmc.0", "rx", EDMA_CTLR_CHAN(0, 26)),
+ DMA_FILTER_ENTRY("dm6441-mmc.0", "tx", EDMA_CTLR_CHAN(0, 27)),
+ DMA_FILTER_ENTRY("dm6441-mmc.1", "rx", EDMA_CTLR_CHAN(0, 30)),
+ DMA_FILTER_ENTRY("dm6441-mmc.1", "tx", EDMA_CTLR_CHAN(0, 31)),
+};
+
static struct edma_soc_info dm365_edma_pdata = {
.queue_priority_mapping = dm365_queue_priority_mapping,
.default_queue = EVENTQ_3,
+ .filter_map = da365_edma_map,
+ .filtercnt = ARRAY_SIZE(da365_edma_map),
};

static struct resource edma_resources[] = {
--
2.6.3

2015-11-30 13:46:48

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 08/15] ARM: davinci: dm644x: Add dma_filter_map to edma

Provide the dma_filter_map to edma which will allow us to move the drivers
to the new, simpler dmaengine API and we can remove the DMA resources also
for the devices.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/mach-davinci/dm644x.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index d38f5049d56e..3550488b5bfd 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -11,6 +11,7 @@
#include <linux/init.h>
#include <linux/clk.h>
#include <linux/serial_8250.h>
+#include <linux/dmaengine.h>
#include <linux/platform_device.h>
#include <linux/platform_data/edma.h>
#include <linux/platform_data/gpio-davinci.h>
@@ -505,9 +506,20 @@ static s8 queue_priority_mapping[][2] = {
{-1, -1},
};

+static struct dma_filter_map da644x_edma_map[] = {
+ DMA_FILTER_ENTRY("davinci-mcbsp", "tx", EDMA_CTLR_CHAN(0, 2)),
+ DMA_FILTER_ENTRY("davinci-mcbsp", "rx", EDMA_CTLR_CHAN(0, 3)),
+ DMA_FILTER_ENTRY("spi_davinci", "tx", EDMA_CTLR_CHAN(0, 16)),
+ DMA_FILTER_ENTRY("spi_davinci", "rx", EDMA_CTLR_CHAN(0, 17)),
+ DMA_FILTER_ENTRY("dm6441-mmc.0", "rx", EDMA_CTLR_CHAN(0, 26)),
+ DMA_FILTER_ENTRY("dm6441-mmc.0", "tx", EDMA_CTLR_CHAN(0, 27)),
+};
+
static struct edma_soc_info dm644x_edma_pdata = {
.queue_priority_mapping = queue_priority_mapping,
.default_queue = EVENTQ_1,
+ .filter_map = da644x_edma_map,
+ .filtercnt = ARRAY_SIZE(da644x_edma_map),
};

static struct resource edma_resources[] = {
--
2.6.3

2015-11-30 13:48:51

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 09/15] ARM: davinci: dm646x: Add dma_filter_map to edma

Provide the dma_filter_map to edma which will allow us to move the drivers
to the new, simpler dmaengine API and we can remove the DMA resources also
for the devices.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/mach-davinci/dm646x.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index 70eb42725eec..560841a0c4fa 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -9,6 +9,7 @@
* or implied.
*/
#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
#include <linux/init.h>
#include <linux/clk.h>
#include <linux/serial_8250.h>
@@ -540,9 +541,19 @@ static s8 dm646x_queue_priority_mapping[][2] = {
{-1, -1},
};

+static struct dma_filter_map da646x_edma_map[] = {
+ DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 6)),
+ DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 9)),
+ DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 12)),
+ DMA_FILTER_ENTRY("spi_davinci", "tx", EDMA_CTLR_CHAN(0, 16)),
+ DMA_FILTER_ENTRY("spi_davinci", "rx", EDMA_CTLR_CHAN(0, 17)),
+};
+
static struct edma_soc_info dm646x_edma_pdata = {
.queue_priority_mapping = dm646x_queue_priority_mapping,
.default_queue = EVENTQ_1,
+ .filter_map = da646x_edma_map,
+ .filtercnt = ARRAY_SIZE(da646x_edma_map),
};

static struct resource edma_resources[] = {
--
2.6.3

2015-11-30 13:46:55

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 10/15] mmc: davinci_mmc: Use dma_request_chan() to requesting DMA channel

With the new dma_request_chan() the clinet driver does not need to look for
the DMA resource and it does not need to pass filter_fn anymore.
By switching to the new API the davinci_mmc driver can now support deferred
probing against DMA.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/mmc/host/davinci_mmc.c | 52 ++++++++++++------------------------------
1 file changed, 14 insertions(+), 38 deletions(-)

diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index ea2a2ebc6b91..8a0a2c291e0c 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -32,12 +32,10 @@
#include <linux/delay.h>
#include <linux/dmaengine.h>
#include <linux/dma-mapping.h>
-#include <linux/edma.h>
#include <linux/mmc/mmc.h>
#include <linux/of.h>
#include <linux/of_device.h>

-#include <linux/platform_data/edma.h>
#include <linux/platform_data/mmc-davinci.h>

/*
@@ -517,35 +515,20 @@ davinci_release_dma_channels(struct mmc_davinci_host *host)

static int __init davinci_acquire_dma_channels(struct mmc_davinci_host *host)
{
- int r;
- dma_cap_mask_t mask;
-
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
- host->dma_tx =
- dma_request_slave_channel_compat(mask, edma_filter_fn,
- &host->txdma, mmc_dev(host->mmc), "tx");
- if (!host->dma_tx) {
+ host->dma_tx = dma_request_chan(mmc_dev(host->mmc), "tx");
+ if (IS_ERR(host->dma_tx)) {
dev_err(mmc_dev(host->mmc), "Can't get dma_tx channel\n");
- return -ENODEV;
+ return PTR_ERR(host->dma_tx);
}

- host->dma_rx =
- dma_request_slave_channel_compat(mask, edma_filter_fn,
- &host->rxdma, mmc_dev(host->mmc), "rx");
- if (!host->dma_rx) {
+ host->dma_rx = dma_request_chan(mmc_dev(host->mmc), "rx");
+ if (IS_ERR(host->dma_rx)) {
dev_err(mmc_dev(host->mmc), "Can't get dma_rx channel\n");
- r = -ENODEV;
- goto free_master_write;
+ dma_release_channel(host->dma_tx);
+ return PTR_ERR(host->dma_rx);
}

return 0;
-
-free_master_write:
- dma_release_channel(host->dma_tx);
-
- return r;
}

/*----------------------------------------------------------------------*/
@@ -1262,18 +1245,6 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
host = mmc_priv(mmc);
host->mmc = mmc; /* Important */

- r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
- if (!r)
- dev_warn(&pdev->dev, "RX DMA resource not specified\n");
- else
- host->rxdma = r->start;
-
- r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
- if (!r)
- dev_warn(&pdev->dev, "TX DMA resource not specified\n");
- else
- host->txdma = r->start;
-
host->mem_res = mem;
host->base = ioremap(mem->start, mem_size);
if (!host->base)
@@ -1300,8 +1271,13 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)
host->mmc_irq = irq;
host->sdio_irq = platform_get_irq(pdev, 1);

- if (host->use_dma && davinci_acquire_dma_channels(host) != 0)
- host->use_dma = 0;
+ if (host->use_dma) {
+ ret = davinci_acquire_dma_channels(host);
+ if (ret == -EPROBE_DEFER)
+ goto out;
+ else if (ret)
+ host->use_dma = 0;
+ }

/* REVISIT: someday, support IRQ-driven card detection. */
mmc->caps |= MMC_CAP_NEEDS_POLL;
--
2.6.3

2015-11-30 13:47:03

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 11/15] spi: davinci: Use dma_request_chan() to requesting DMA channel

With the new dma_request_chan() the clinet driver does not need to look for
the DMA resource and it does not need to pass filter_fn anymore.
By switching to the new API the davinci_mmc driver can now support deferred
probing against DMA.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/spi/spi-davinci.c | 76 +++++++++++++++--------------------------------
1 file changed, 24 insertions(+), 52 deletions(-)

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 7d3af3eacf57..540fc8754085 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -23,7 +23,6 @@
#include <linux/clk.h>
#include <linux/dmaengine.h>
#include <linux/dma-mapping.h>
-#include <linux/edma.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_gpio.h>
@@ -33,8 +32,6 @@

#include <linux/platform_data/spi-davinci.h>

-#define SPI_NO_RESOURCE ((resource_size_t)-1)
-
#define CS_DEFAULT 0xFF

#define SPIFMT_PHASE_MASK BIT(16)
@@ -130,8 +127,6 @@ struct davinci_spi {

struct dma_chan *dma_rx;
struct dma_chan *dma_tx;
- int dma_rx_chnum;
- int dma_tx_chnum;

struct davinci_spi_platform_data pdata;

@@ -796,35 +791,19 @@ static irqreturn_t davinci_spi_irq(s32 irq, void *data)

static int davinci_spi_request_dma(struct davinci_spi *dspi)
{
- dma_cap_mask_t mask;
struct device *sdev = dspi->bitbang.master->dev.parent;
- int r;
-
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);

- dspi->dma_rx = dma_request_channel(mask, edma_filter_fn,
- &dspi->dma_rx_chnum);
- if (!dspi->dma_rx) {
- dev_err(sdev, "request RX DMA channel failed\n");
- r = -ENODEV;
- goto rx_dma_failed;
- }
+ dspi->dma_rx = dma_request_chan(sdev, "rx");
+ if (IS_ERR(dspi->dma_rx))
+ return PTR_ERR(dspi->dma_rx);

- dspi->dma_tx = dma_request_channel(mask, edma_filter_fn,
- &dspi->dma_tx_chnum);
- if (!dspi->dma_tx) {
- dev_err(sdev, "request TX DMA channel failed\n");
- r = -ENODEV;
- goto tx_dma_failed;
+ dspi->dma_tx = dma_request_chan(sdev, "tx");
+ if (IS_ERR(dspi->dma_tx)) {
+ dma_release_channel(dspi->dma_rx);
+ return PTR_ERR(dspi->dma_tx);
}

return 0;
-
-tx_dma_failed:
- dma_release_channel(dspi->dma_rx);
-rx_dma_failed:
- return r;
}

#if defined(CONFIG_OF)
@@ -935,8 +914,6 @@ static int davinci_spi_probe(struct platform_device *pdev)
struct davinci_spi *dspi;
struct davinci_spi_platform_data *pdata;
struct resource *r;
- resource_size_t dma_rx_chan = SPI_NO_RESOURCE;
- resource_size_t dma_tx_chan = SPI_NO_RESOURCE;
int ret = 0;
u32 spipc0;

@@ -1043,27 +1020,15 @@ static int davinci_spi_probe(struct platform_device *pdev)
}
}

- r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
- if (r)
- dma_rx_chan = r->start;
- r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
- if (r)
- dma_tx_chan = r->start;
-
dspi->bitbang.txrx_bufs = davinci_spi_bufs;
- if (dma_rx_chan != SPI_NO_RESOURCE &&
- dma_tx_chan != SPI_NO_RESOURCE) {
- dspi->dma_rx_chnum = dma_rx_chan;
- dspi->dma_tx_chnum = dma_tx_chan;
-
- ret = davinci_spi_request_dma(dspi);
- if (ret)
- goto free_clk;
-
- dev_info(&pdev->dev, "DMA: supported\n");
- dev_info(&pdev->dev, "DMA: RX channel: %pa, TX channel: %pa, event queue: %d\n",
- &dma_rx_chan, &dma_tx_chan,
- pdata->dma_event_q);
+
+ ret = davinci_spi_request_dma(dspi);
+ if (ret == -EPROBE_DEFER) {
+ goto free_clk;
+ } else if (ret) {
+ dev_info(&pdev->dev, "DMA is not supported (%d)\n", ret);
+ dspi->dma_rx = NULL;
+ dspi->dma_tx = NULL;
}

dspi->get_rx = davinci_spi_rx_buf_u8;
@@ -1101,8 +1066,10 @@ static int davinci_spi_probe(struct platform_device *pdev)
return ret;

free_dma:
- dma_release_channel(dspi->dma_rx);
- dma_release_channel(dspi->dma_tx);
+ if (dspi->dma_rx) {
+ dma_release_channel(dspi->dma_rx);
+ dma_release_channel(dspi->dma_tx);
+ }
free_clk:
clk_disable_unprepare(dspi->clk);
free_master:
@@ -1133,6 +1100,11 @@ static int davinci_spi_remove(struct platform_device *pdev)
clk_disable_unprepare(dspi->clk);
spi_master_put(master);

+ if (dspi->dma_rx) {
+ dma_release_channel(dspi->dma_rx);
+ dma_release_channel(dspi->dma_tx);
+ }
+
return 0;
}

--
2.6.3

2015-11-30 13:47:47

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 12/15] ARM: davinci: devices-da8xx: Remove DMA resources for MMC and SPI

The drivers are now converted to not use the DMA resource.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/mach-davinci/devices-da8xx.c | 49 -----------------------------------
1 file changed, 49 deletions(-)

diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index 3327cb22e0b5..5d96d8728c58 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -57,15 +57,6 @@
#define DA8XX_EMAC_RAM_OFFSET 0x0000
#define DA8XX_EMAC_CTRL_RAM_SIZE SZ_8K

-#define DA8XX_DMA_SPI0_RX EDMA_CTLR_CHAN(0, 14)
-#define DA8XX_DMA_SPI0_TX EDMA_CTLR_CHAN(0, 15)
-#define DA8XX_DMA_MMCSD0_RX EDMA_CTLR_CHAN(0, 16)
-#define DA8XX_DMA_MMCSD0_TX EDMA_CTLR_CHAN(0, 17)
-#define DA8XX_DMA_SPI1_RX EDMA_CTLR_CHAN(0, 18)
-#define DA8XX_DMA_SPI1_TX EDMA_CTLR_CHAN(0, 19)
-#define DA850_DMA_MMCSD1_RX EDMA_CTLR_CHAN(1, 28)
-#define DA850_DMA_MMCSD1_TX EDMA_CTLR_CHAN(1, 29)
-
void __iomem *da8xx_syscfg0_base;
void __iomem *da8xx_syscfg1_base;

@@ -751,16 +742,6 @@ static struct resource da8xx_mmcsd0_resources[] = {
.end = IRQ_DA8XX_MMCSDINT0,
.flags = IORESOURCE_IRQ,
},
- { /* DMA RX */
- .start = DA8XX_DMA_MMCSD0_RX,
- .end = DA8XX_DMA_MMCSD0_RX,
- .flags = IORESOURCE_DMA,
- },
- { /* DMA TX */
- .start = DA8XX_DMA_MMCSD0_TX,
- .end = DA8XX_DMA_MMCSD0_TX,
- .flags = IORESOURCE_DMA,
- },
};

static struct platform_device da8xx_mmcsd0_device = {
@@ -788,16 +769,6 @@ static struct resource da850_mmcsd1_resources[] = {
.end = IRQ_DA850_MMCSDINT0_1,
.flags = IORESOURCE_IRQ,
},
- { /* DMA RX */
- .start = DA850_DMA_MMCSD1_RX,
- .end = DA850_DMA_MMCSD1_RX,
- .flags = IORESOURCE_DMA,
- },
- { /* DMA TX */
- .start = DA850_DMA_MMCSD1_TX,
- .end = DA850_DMA_MMCSD1_TX,
- .flags = IORESOURCE_DMA,
- },
};

static struct platform_device da850_mmcsd1_device = {
@@ -984,16 +955,6 @@ static struct resource da8xx_spi0_resources[] = {
.end = IRQ_DA8XX_SPINT0,
.flags = IORESOURCE_IRQ,
},
- [2] = {
- .start = DA8XX_DMA_SPI0_RX,
- .end = DA8XX_DMA_SPI0_RX,
- .flags = IORESOURCE_DMA,
- },
- [3] = {
- .start = DA8XX_DMA_SPI0_TX,
- .end = DA8XX_DMA_SPI0_TX,
- .flags = IORESOURCE_DMA,
- },
};

static struct resource da8xx_spi1_resources[] = {
@@ -1007,16 +968,6 @@ static struct resource da8xx_spi1_resources[] = {
.end = IRQ_DA8XX_SPINT1,
.flags = IORESOURCE_IRQ,
},
- [2] = {
- .start = DA8XX_DMA_SPI1_RX,
- .end = DA8XX_DMA_SPI1_RX,
- .flags = IORESOURCE_DMA,
- },
- [3] = {
- .start = DA8XX_DMA_SPI1_TX,
- .end = DA8XX_DMA_SPI1_TX,
- .flags = IORESOURCE_DMA,
- },
};

static struct davinci_spi_platform_data da8xx_spi_pdata[] = {
--
2.6.3

2015-11-30 13:47:46

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 13/15] ARM: davinci: devices: Remove DMA resources for MMC

The driver is converted to not use the DMA resource.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/mach-davinci/devices.c | 19 -------------------
1 file changed, 19 deletions(-)

diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-davinci/devices.c
index 6257aa452568..3ae70f2909b0 100644
--- a/arch/arm/mach-davinci/devices.c
+++ b/arch/arm/mach-davinci/devices.c
@@ -36,9 +36,6 @@
#define DM365_MMCSD0_BASE 0x01D11000
#define DM365_MMCSD1_BASE 0x01D00000

-#define DAVINCI_DMA_MMCRXEVT 26
-#define DAVINCI_DMA_MMCTXEVT 27
-
void __iomem *davinci_sysmod_base;

void davinci_map_sysmod(void)
@@ -144,14 +141,6 @@ static struct resource mmcsd0_resources[] = {
.start = IRQ_SDIOINT,
.flags = IORESOURCE_IRQ,
},
- /* DMA channels: RX, then TX */
- {
- .start = EDMA_CTLR_CHAN(0, DAVINCI_DMA_MMCRXEVT),
- .flags = IORESOURCE_DMA,
- }, {
- .start = EDMA_CTLR_CHAN(0, DAVINCI_DMA_MMCTXEVT),
- .flags = IORESOURCE_DMA,
- },
};

static struct platform_device davinci_mmcsd0_device = {
@@ -181,14 +170,6 @@ static struct resource mmcsd1_resources[] = {
.start = IRQ_DM355_SDIOINT1,
.flags = IORESOURCE_IRQ,
},
- /* DMA channels: RX, then TX */
- {
- .start = EDMA_CTLR_CHAN(0, 30), /* rx */
- .flags = IORESOURCE_DMA,
- }, {
- .start = EDMA_CTLR_CHAN(0, 31), /* tx */
- .flags = IORESOURCE_DMA,
- },
};

static struct platform_device davinci_mmcsd1_device = {
--
2.6.3

2015-11-30 13:47:09

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 14/15] ARM: davinci: dm355: Remove DMA resources for SPI

The driver is converted to not use the DMA resource.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/mach-davinci/dm355.c | 8 --------
1 file changed, 8 deletions(-)

diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index dc8d7ccf69f4..7c6ab2d16e3e 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -397,14 +397,6 @@ static struct resource dm355_spi0_resources[] = {
.start = IRQ_DM355_SPINT0_0,
.flags = IORESOURCE_IRQ,
},
- {
- .start = 17,
- .flags = IORESOURCE_DMA,
- },
- {
- .start = 16,
- .flags = IORESOURCE_DMA,
- },
};

static struct davinci_spi_platform_data dm355_spi0_pdata = {
--
2.6.3

2015-11-30 13:47:13

by Peter Ujfalusi

[permalink] [raw]
Subject: [RFC v02 15/15] ARM: davinci: dm365: Remove DMA resources for SPI

The driver is converted to not use the DMA resource.

Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/mach-davinci/dm365.c | 8 --------
1 file changed, 8 deletions(-)

diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index 3dfcf7c9aa20..fe98e5f55634 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -660,14 +660,6 @@ static struct resource dm365_spi0_resources[] = {
.start = IRQ_DM365_SPIINT0_0,
.flags = IORESOURCE_IRQ,
},
- {
- .start = 17,
- .flags = IORESOURCE_DMA,
- },
- {
- .start = 16,
- .flags = IORESOURCE_DMA,
- },
};

static struct platform_device dm365_spi0_device = {
--
2.6.3

2015-11-30 14:10:12

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

On Monday 30 November 2015 15:45:33 Peter Ujfalusi wrote:
> const char *name);
> struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
> +
> +struct dma_chan *dma_request_chan(struct device *dev, const char *name);
> +struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
> +
> void dma_release_channel(struct dma_chan *chan);
> int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);
> #else
> @@ -1268,6 +1291,14 @@ static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
> {
> return NULL;
> }
> +static inline struct dma_chan *dma_request_chan(struct device *dev)
> +{
> + return ERR_PTR(-ENODEV);
> +}
>

The prototypes for dma_request_chan() don't match, otherwise looks good.

Arnd

2015-11-30 14:12:28

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 04/15] dmaengine: edma: Add support for DMA filter mapping to slave devices

On Monday 30 November 2015 15:45:34 Peter Ujfalusi wrote:
> @@ -2428,6 +2436,22 @@ bool edma_filter_fn(struct dma_chan *chan, void *param)
> }
> EXPORT_SYMBOL(edma_filter_fn);
>
> +static bool edma_filter_for_map(struct dma_chan *chan, void *param)
> +{
> + bool match = false;
> +
> + if (chan->device->dev->driver == &edma_driver.driver) {
> + struct edma_chan *echan = to_edma_chan(chan);
> + unsigned ch_req = (unsigned)param;
> + if (ch_req == echan->ch_num) {
> + /* The channel is going to be used as HW synchronized */
> + echan->hw_triggered = true;
> + match = true;
> + }
> + }
> + return match;
> +}
> +
> static int edma_init(void)
>

I don't see the difference between edma_filter_fn and edma_filter_for_map.
Why do you need both?

Arnd

2015-11-30 14:18:41

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On Monday 30 November 2015 15:45:30 Peter Ujfalusi wrote:
> Changes since RFC v01:
>- dma_request_chan(); lost the mask parameter
>- The new API does not rely on RESOURCE_DMA, instead the dma_filter_map table
> will be used to provide the needed information to the filter function in
> legacy mode
>- Extended the example patches to convert most of daVinci to use the new API to
> request the DMA channels.

Very nice overall. Just a few minor comments.

> static struct dma_filter_map da830_edma_map[] = {
> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> };

I wonder if we should mandate that the lookup table is 'const'.

We could also drop the DMA_FILTER_ENTRY() macro above, and open-code the
table like

static struct dma_filter_map da830_edma_map[] = {
{ "davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)},
{ "davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)},
{ "davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)},
...
};

which is a little more compact and more obvious, but loses the
named initializers.

Arnd

2015-11-30 14:32:40

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi <[email protected]> wrote:
> Hi,
>
> Changes since RFC v01:
> - dma_request_chan(); lost the mask parameter
> - The new API does not rely on RESOURCE_DMA, instead the dma_filter_map table
> will be used to provide the needed information to the filter function in
> legacy mode
> - Extended the example patches to convert most of daVinci to use the new API to
> request the DMA channels.
>
> TODO: Documentation update ;)
>
> As it has been discussed in the following thread:
> http://www.gossamer-threads.com/lists/linux/kernel/2181487#2181487
>
> Andy: I did looked at the unified device properties, but I decided to not to use
> it as I don't see it to fit well and most of the legacy board files are using
> resources to specify at least their memory regions so adding the DMA resource
> to them would be more inline with the rest of the code.

We could return back to this in the future, still we have to amend
built-in device properties (there is a patch series already in the
wild).

>
> The ARM, mmc and spi patches are converting daVinci drivers board files to use
> the new method of requesting DMA channel.
>
> With this series I have taken a path which would result two new API, which can
> be used to convert most of the current users already and with some work all
> users might be able to move to this set.
> With this set the filter_fn used for legacy (non DT/ACPI) channel request is no
> longer needed to be exported to client drivers since the selection of the
> correct filter_fn will be done in the core.
>
> So, the first proposal is to have:
>
> struct dma_chan *dma_request_chan(struct device *dev, const char *name);
> struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
>
> The dma_request_chan_by_mask() is to request any channel matching with the
> requested capabilities, can be used to request channel for memcpy, memset, xor,
> etc where no hardware synchronization is needed.
>
> The dma_request_chan() is to request a slave channel. The dma_request_chan() will try to find the
> channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
> it will use a filter lookup table and retrieves the needed information from
> the dma_filter_map provided by the DMA drivers.
> This legacy mode needs changes in platform code, in dmaengine drivers and
> finally the dmaengine user drivers can be converted:
>
> For each dmaengine driver an array of DMA device, slave and the parameter
> for the filter function needs to be added:
>
> static struct dma_filter_map da830_edma_map[] = {
> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),

Does this ".2" and so prevent driver to use auto ID for platform devices?

> };
>
> This information is going to be needed by the dmaengine driver, so
> modification to the platform_data is needed, and the driver map should be
> added to the pdata of the DMA driver:
>
> da8xx_edma0_pdata.filter_map = da830_edma_map;
> da8xx_edma0_pdata.filtercnt = ARRAY_SIZE(da830_edma_map);
>
> The DMA driver then needs to convigure the needed device -> filter_fn
> mapping before it registers with dma_async_device_register() :
>
> if (info->filter_map) {
> ecc->dma_slave.filter_map.map = info->filter_map;
> ecc->dma_slave.filter_map.mapcnt = info->filtercnt;
> ecc->dma_slave.filter_map.filter_fn = edma_filter_for_map;
> }
>
> When neither DT or ACPI lookup is available the dma_request_chan() will
> try to match the requester's device name with the filter_map's list of
> device names, when a match found it will use the information from the
> dma_filter_map to get the channel with the dma_get_channel() internal
> function.
>
> Regards,
> Peter
> ---
> Peter Ujfalusi (15):
> dmaengine: core: Allow NULL mask pointer in
> __dma_device_satisfies_mask()
> dmaengine: core: Move and merge the code paths using private_candidate
> dmaengine: core: Introduce new, universal API to request a channel
> dmaengine: edma: Add support for DMA filter mapping to slave devices
> ARM: davinci: devices-da8xx: Add dma_filter_map to edma
> ARM: davinci: dm355: Add dma_filter_map to edma
> ARM: davinci: dm365: Add dma_filter_map to edma
> ARM: davinci: dm644x: Add dma_filter_map to edma
> ARM: davinci: dm646x: Add dma_filter_map to edma
> mmc: davinci_mmc: Use dma_request_chan() to requesting DMA channel
> spi: davinci: Use dma_request_chan() to requesting DMA channel
> ARM: davinci: devices-da8xx: Remove DMA resources for MMC and SPI
> ARM: davinci: devices: Remove DMA resources for MMC
> ARM: davinci: dm355: Remove DMA resources for SPI
> ARM: davinci: dm365: Remove DMA resources for SPI
>
> arch/arm/mach-davinci/devices-da8xx.c | 95 ++++++++++----------
> arch/arm/mach-davinci/devices.c | 19 ----
> arch/arm/mach-davinci/dm355.c | 28 ++++--
> arch/arm/mach-davinci/dm365.c | 30 +++++--
> arch/arm/mach-davinci/dm644x.c | 12 +++
> arch/arm/mach-davinci/dm646x.c | 11 +++
> drivers/dma/dmaengine.c | 160 +++++++++++++++++++++++++---------
> drivers/dma/edma.c | 24 +++++
> drivers/mmc/host/davinci_mmc.c | 52 +++--------
> drivers/spi/spi-davinci.c | 76 +++++-----------
> include/linux/dmaengine.h | 31 +++++++
> include/linux/platform_data/edma.h | 5 ++
> 12 files changed, 330 insertions(+), 213 deletions(-)
>
> --
> 2.6.3
>



--
With Best Regards,
Andy Shevchenko

2015-11-30 14:35:35

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC v02 01/15] dmaengine: core: Allow NULL mask pointer in __dma_device_satisfies_mask()

On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi <[email protected]> wrote:
> Treat as true condition the case when the mask is NULL.

What do you think about setting some default (all "on") mask when mask
is not supplied?

I don't know for sure but there might be cases when you don't want
literally *any* channel to satisfy.

>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> drivers/dma/dmaengine.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index daf54a39bcc7..52c3eee48e2e 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -184,6 +184,9 @@ __dma_device_satisfies_mask(struct dma_device *device,
> {
> dma_cap_mask_t has;
>
> + if (!want)
> + return true;
> +
> bitmap_and(has.bits, want->bits, device->cap_mask.bits,
> DMA_TX_TYPE_END);
> return bitmap_equal(want->bits, has.bits, DMA_TX_TYPE_END);
> --
> 2.6.3
>



--
With Best Regards,
Andy Shevchenko

2015-11-30 14:42:37

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC v02 02/15] dmaengine: core: Move and merge the code paths using private_candidate

On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi <[email protected]> wrote:
> Channel matching with private_candidate() is used in two paths, the error
> checking is slightly different in them and they are duplicating code also.
> Move the code under dma_get_channel() to provide consistent execution and
> going to allow us to reuse this mode of channel lookup later.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> drivers/dma/dmaengine.c | 81 +++++++++++++++++++++++++------------------------
> 1 file changed, 42 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 52c3eee48e2e..1249165fb4b2 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -549,6 +549,42 @@ static struct dma_chan *private_candidate(const dma_cap_mask_t *mask,
> return NULL;
> }
>
> +static struct dma_chan *dma_get_channel(struct dma_device *device,

Naming scheme inside dmaengine.c looks like a mess.

Since it's static function that utilizes private_candidate() may I
propose the name like find_candidate() ?

> + const dma_cap_mask_t *mask,
> + dma_filter_fn fn, void *fn_param)
> +{
> + struct dma_chan *chan = private_candidate(mask, device, fn, fn_param);
> + int err;
> +
> + if (chan) {
> + /* Found a suitable channel, try to grab, prep, and return it.
> + * We first set DMA_PRIVATE to disable balance_ref_count as this
> + * channel will not be published in the general-purpose
> + * allocator
> + */
> + dma_cap_set(DMA_PRIVATE, device->cap_mask);
> + device->privatecnt++;
> + err = dma_chan_get(chan);
> +
> + if (err) {
> + if (err == -ENODEV) {
> + pr_debug("%s: %s module removed\n", __func__,
> + dma_chan_name(chan));
> + list_del_rcu(&device->global_node);
> + } else
> + pr_debug("%s: failed to get %s: (%d)\n",
> + __func__, dma_chan_name(chan), err);
> +
> + if (--device->privatecnt == 0)
> + dma_cap_clear(DMA_PRIVATE, device->cap_mask);
> +
> + chan = ERR_PTR(err);
> + }
> + }
> +
> + return chan ? chan : ERR_PTR(-EPROBE_DEFER);
> +}
> +
> /**
> * dma_get_slave_channel - try to get specific channel exclusively
> * @chan: target channel
> @@ -587,7 +623,6 @@ struct dma_chan *dma_get_any_slave_channel(struct dma_device *device)
> {
> dma_cap_mask_t mask;
> struct dma_chan *chan;
> - int err;
>
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
> @@ -595,23 +630,11 @@ struct dma_chan *dma_get_any_slave_channel(struct dma_device *device)
> /* lock against __dma_request_channel */
> mutex_lock(&dma_list_mutex);
>
> - chan = private_candidate(&mask, device, NULL, NULL);
> - if (chan) {
> - dma_cap_set(DMA_PRIVATE, device->cap_mask);
> - device->privatecnt++;
> - err = dma_chan_get(chan);
> - if (err) {
> - pr_debug("%s: failed to get %s: (%d)\n",
> - __func__, dma_chan_name(chan), err);
> - chan = NULL;
> - if (--device->privatecnt == 0)
> - dma_cap_clear(DMA_PRIVATE, device->cap_mask);
> - }
> - }
> + chan = dma_get_channel(device, &mask, NULL, NULL);
>
> mutex_unlock(&dma_list_mutex);
>
> - return chan;
> + return IS_ERR(chan) ? NULL : chan;
> }
> EXPORT_SYMBOL_GPL(dma_get_any_slave_channel);
>
> @@ -628,35 +651,15 @@ struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask,
> {
> struct dma_device *device, *_d;
> struct dma_chan *chan = NULL;
> - int err;
>
> /* Find a channel */
> mutex_lock(&dma_list_mutex);
> list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
> - chan = private_candidate(mask, device, fn, fn_param);
> - if (chan) {
> - /* Found a suitable channel, try to grab, prep, and
> - * return it. We first set DMA_PRIVATE to disable
> - * balance_ref_count as this channel will not be
> - * published in the general-purpose allocator
> - */
> - dma_cap_set(DMA_PRIVATE, device->cap_mask);
> - device->privatecnt++;
> - err = dma_chan_get(chan);
> + chan = dma_get_channel(device, mask, fn, fn_param);
> + if (!IS_ERR(chan))
> + break;
>
> - if (err == -ENODEV) {
> - pr_debug("%s: %s module removed\n",
> - __func__, dma_chan_name(chan));
> - list_del_rcu(&device->global_node);
> - } else if (err)
> - pr_debug("%s: failed to get %s: (%d)\n",
> - __func__, dma_chan_name(chan), err);
> - else
> - break;
> - if (--device->privatecnt == 0)
> - dma_cap_clear(DMA_PRIVATE, device->cap_mask);
> - chan = NULL;
> - }
> + chan = NULL;
> }
> mutex_unlock(&dma_list_mutex);
>
> --
> 2.6.3
>



--
With Best Regards,
Andy Shevchenko

2015-11-30 14:51:48

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi <[email protected]> wrote:
> The two API function can cover most, if not all current APIs used to
> request a channel. With minimal effort dmaengine drivers, platforms and
> dmaengine user drivers can be converted to use the two function.
>
> struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
>
> To request any channel matching with the requested capabilities, can be
> used to request channel for memcpy, memset, xor, etc where no hardware
> synchronization is needed.
>
> struct dma_chan *dma_request_chan(struct device *dev, const char *name);
> To request a slave channel. The dma_request_chan() will try to find the
> channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
> it will use a filter lookup table and retrieves the needed information from
> the dma_filter_map provided by the DMA drivers.
> This legacy mode needs changes in platform code, in dmaengine drivers and
> finally the dmaengine user drivers can be converted:
>
> For each dmaengine driver an array of DMA device, slave and the parameter
> for the filter function needs to be added:
>
> static struct dma_filter_map da830_edma_map[] = {
> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> };
>
> This information is going to be needed by the dmaengine driver, so
> modification to the platform_data is needed, and the driver map should be
> added to the pdata of the DMA driver:
>
> da8xx_edma0_pdata.filter_map = da830_edma_map;
> da8xx_edma0_pdata.filtercnt = ARRAY_SIZE(da830_edma_map);
>
> The DMA driver then needs to convigure the needed device -> filter_fn
> mapping before it registers with dma_async_device_register() :
>
> if (info->filter_map) {
> ecc->dma_slave.filter_map.map = info->filter_map;
> ecc->dma_slave.filter_map.mapcnt = info->filtercnt;
> ecc->dma_slave.filter_map.filter_fn = edma_filter_for_map;
> }
>
> When neither DT or ACPI lookup is available the dma_request_chan() will
> try to match the requester's device name with the filter_map's list of
> device names, when a match found it will use the information from the
> dma_filter_map to get the channel with the dma_get_channel() internal
> function.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> drivers/dma/dmaengine.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/dmaengine.h | 31 +++++++++++++++++++
> 2 files changed, 107 insertions(+)
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 1249165fb4b2..bd774adff697 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -43,6 +43,7 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/platform_device.h>
> #include <linux/dma-mapping.h>
> #include <linux/init.h>
> #include <linux/module.h>
> @@ -715,6 +716,81 @@ struct dma_chan *dma_request_slave_channel(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(dma_request_slave_channel);
>
> +static struct dma_filter_map *dma_filter_match(struct dma_device *device,
> + const char *name,
> + struct device *dev)
> +{
> + struct dma_filter_map *map_found = NULL;
> + int i;
> +
> + if (!device->filter_map.mapcnt)
> + return NULL;
> +
> + for (i = 0; i < device->filter_map.mapcnt; i++) {
> + struct dma_filter_map *map = &device->filter_map.map[i];
> +
> + if (!strcmp(map->devname, dev_name(dev)) &&
> + !strcmp(map->slave, name)) {
> + map_found = map;
> + break;
> + }
> + }
> +
> + return map_found;
> +}
> +
> +struct dma_chan *dma_request_chan(struct device *dev, const char *name)
> +{
> + struct dma_device *device, *_d;
> + struct dma_chan *chan = NULL;
> +
> + /* If device-tree is present get slave info from here */
> + if (dev->of_node)
> + chan = of_dma_request_slave_channel(dev->of_node, name);
> +
> + /* If device was enumerated by ACPI get slave info from here */
> + if (ACPI_HANDLE(dev) && !chan)

The preferable way is to use
has_acpi_companion() instead of ACPI_HANDLE().

> + chan = acpi_dma_request_slave_chan_by_name(dev, name);
> +
> + if (chan)
> + return chan;
> +
> + /* Try to find the channel via the DMA filter map(s) */
> + mutex_lock(&dma_list_mutex);
> + list_for_each_entry_safe(device, _d, &dma_device_list, global_node) {
> + struct dma_filter_map *map = dma_filter_match(device, name,
> + dev);
> +
> + if (!map)
> + continue;
> +
> + chan = dma_get_channel(device, NULL,
> + device->filter_map.filter_fn,
> + map->param);
> + if (!IS_ERR(chan))
> + break;
> + }
> + mutex_unlock(&dma_list_mutex);
> +
> + return chan ? chan : ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL_GPL(dma_request_chan);
> +
> +struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask)
> +{
> + struct dma_chan *chan;
> +
> + if (!mask)
> + return ERR_PTR(-ENODEV);
> +
> + chan = __dma_request_channel(mask, NULL, NULL);
> + if (!chan)
> + chan = ERR_PTR(-ENODEV);
> +
> + return chan;
> +}
> +EXPORT_SYMBOL_GPL(dma_request_chan_by_mask);
> +
> void dma_release_channel(struct dma_chan *chan)
> {
> mutex_lock(&dma_list_mutex);
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index a2b7c2071cf4..f7b06ff982c8 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -606,6 +606,24 @@ enum dmaengine_alignment {
> DMAENGINE_ALIGN_64_BYTES = 6,
> };
>
> +struct dma_filter_map {
> + char *devname;
> + char *slave;
> + void *param;
> +};
> +#define DMA_FILTER_ENTRY(_devname, _slave, _param) \
> + { \
> + .devname = _devname, \
> + .slave = _slave, \
> + .param = (void*)_param, \
> + }
> +
> +struct dma_filter {
> + dma_filter_fn filter_fn;
> + int mapcnt;
> + struct dma_filter_map *map;
> +};
> +
> /**
> * struct dma_device - info on the entity supplying DMA services
> * @chancnt: how many DMA channels are supported
> @@ -669,6 +687,7 @@ struct dma_device {
> unsigned int privatecnt;
> struct list_head channels;
> struct list_head global_node;
> + struct dma_filter filter_map;
> dma_cap_mask_t cap_mask;
> unsigned short max_xor;
> unsigned short max_pq;
> @@ -1235,6 +1254,10 @@ struct dma_chan *__dma_request_channel(const dma_cap_mask_t *mask,
> struct dma_chan *dma_request_slave_channel_reason(struct device *dev,
> const char *name);
> struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
> +
> +struct dma_chan *dma_request_chan(struct device *dev, const char *name);
> +struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
> +
> void dma_release_channel(struct dma_chan *chan);
> int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);
> #else
> @@ -1268,6 +1291,14 @@ static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
> {
> return NULL;
> }
> +static inline struct dma_chan *dma_request_chan(struct device *dev)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +static inline struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> static inline void dma_release_channel(struct dma_chan *chan)
> {
> }
> --
> 2.6.3
>



--
With Best Regards,
Andy Shevchenko

2015-11-30 15:51:51

by Tony Lindgren

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

Hi,

* Peter Ujfalusi <[email protected]> [151130 05:49]:
>
> For each dmaengine driver an array of DMA device, slave and the parameter
> for the filter function needs to be added:
>
> static struct dma_filter_map da830_edma_map[] = {
> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> };

FYI, if the EDMA_CTRL_CHAN above is just the evtmux registers, those
can be handled with the pinctrl framework. It seems that would allow
leaving out some of the built-in look up data, and have the mux parts
handled by a proper device driver. Below is a sample from the dm81xx
platform for reference.

SoC dtsi file:

evtmux: pinmux@f90 {
compatible = "pinctrl-single";
reg = <0xf90 0x40>;
#address-cells = <1>;
#size-cells = <0>;
pinctrl-single,register-width = <8>;
pinctrl-single,function-mask = <0x1f>;
};

Board specific dts file:

&evtmux {
sd2_edma_pins: pinmux_sd2_edma_pins {
pinctrl-single,pins = <
8 1 /* use SDTXEVT1 for EDMA instead of MCASP0TX */
9 2 /* use SDRXEVT1 for EDMA instead of MCASP0RX */
>;
};
};

Dynamic muxing of these channels can be done too using the pinctrl
framework named modes, but probably is not a good idea in the case of
SD card and MaASP in case something goes wrong :)

Regards,

Tony

2015-12-01 08:13:58

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

On 11/30/2015 05:51 PM, Tony Lindgren wrote:
> Hi,
>
> * Peter Ujfalusi <[email protected]> [151130 05:49]:
>>
>> For each dmaengine driver an array of DMA device, slave and the parameter
>> for the filter function needs to be added:
>>
>> static struct dma_filter_map da830_edma_map[] = {
>> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
>> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
>> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
>> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
>> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
>> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
>> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
>> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
>> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
>> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
>> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
>> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
>> };
>
> FYI, if the EDMA_CTRL_CHAN above is just the evtmux registers

No, they are not. They are the eDMA event numbers. I used the EDMA_CTRL_CHAN()
macro for all board files to have uniform look for the data. The first
parameter means the eDMA instance number while the second is the event number
on that eDMA. Since most devices have only one eDMA, we have 0 as eDMA id in
most cases.
The eventmux, or crossbar is different thing and we have several versions of
the event crossbar or mux used.

> those
> can be handled with the pinctrl framework. It seems that would allow
> leaving out some of the built-in look up data, and have the mux parts
> handled by a proper device driver. Below is a sample from the dm81xx
> platform for reference.
>
> SoC dtsi file:
>
> evtmux: pinmux@f90 {
> compatible = "pinctrl-single";
> reg = <0xf90 0x40>;
> #address-cells = <1>;
> #size-cells = <0>;
> pinctrl-single,register-width = <8>;
> pinctrl-single,function-mask = <0x1f>;
> };
>
> Board specific dts file:
>
> &evtmux {
> sd2_edma_pins: pinmux_sd2_edma_pins {
> pinctrl-single,pins = <
> 8 1 /* use SDTXEVT1 for EDMA instead of MCASP0TX */
> 9 2 /* use SDRXEVT1 for EDMA instead of MCASP0RX */
> >;
> };
> };

I see. The dm81xx basically am33xx/am43xx?

Actually I would prefer to use the dmaengine's event router framework and we
do have support for the am33xx/am43xx type of crossbar already implemented.
I'm going to resend the DTS series for am33xx/am43xx to convert them to use
the new DT bindings along with the dma event router support:
https://www.mail-archive.com/linux-omap%40vger.kernel.org/msg120828.html

> Dynamic muxing of these channels can be done too using the pinctrl
> framework named modes, but probably is not a good idea in the case of
> SD card and MaASP in case something goes wrong :)

In theory it can be done, but in practice it is not possible. It is up to the
board design decision to select which DMA event is not needed to be used in
default mode and that one can be used to route the crossbar hidden request to it.
Just imaging: playing audio from MMC (in the example you have), audio needs
constant DMA, so the MMC would never get DMA request, also the drivers tend to
request the DMA channel in their probe/init and hold to it as long as they are
loaded...

--
P?ter

2015-12-01 08:41:55

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 02/15] dmaengine: core: Move and merge the code paths using private_candidate

On 11/30/2015 04:42 PM, Andy Shevchenko wrote:
> On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi <[email protected]> wrote:
>> Channel matching with private_candidate() is used in two paths, the error
>> checking is slightly different in them and they are duplicating code also.
>> Move the code under dma_get_channel() to provide consistent execution and
>> going to allow us to reuse this mode of channel lookup later.
>>
>> Signed-off-by: Peter Ujfalusi <[email protected]>
>> ---
>> drivers/dma/dmaengine.c | 81 +++++++++++++++++++++++++------------------------
>> 1 file changed, 42 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
>> index 52c3eee48e2e..1249165fb4b2 100644
>> --- a/drivers/dma/dmaengine.c
>> +++ b/drivers/dma/dmaengine.c
>> @@ -549,6 +549,42 @@ static struct dma_chan *private_candidate(const dma_cap_mask_t *mask,
>> return NULL;
>> }
>>
>> +static struct dma_chan *dma_get_channel(struct dma_device *device,
>
> Naming scheme inside dmaengine.c looks like a mess.

Yes, I agree.

>
> Since it's static function that utilizes private_candidate() may I
> propose the name like find_candidate() ?

I had __* version as well, but the find_candidate() sounds better.

Thanks,
Péter

2015-12-01 09:48:09

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 01/15] dmaengine: core: Allow NULL mask pointer in __dma_device_satisfies_mask()

On 11/30/2015 04:35 PM, Andy Shevchenko wrote:
> On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi <[email protected]> wrote:
>> Treat as true condition the case when the mask is NULL.
>
> What do you think about setting some default (all "on") mask when mask
> is not supplied?

Probably rephrasing the commit message to say that when the mask is NULL it
means that the caller does not care about the capabilities of the dma device
thus return with true in such a case.

We could also drop this patch and in private_candidate() :

- if (!__dma_device_satisfies_mask(dev, mask)) {
+ if (mask && !__dma_device_satisfies_mask(dev, mask)) {
pr_debug("%s: wrong capabilities\n", __func__);
return NULL;
}


> I don't know for sure but there might be cases when you don't want
> literally *any* channel to satisfy.

Or set DMA_SLAVE only in dma_request_chan()? What happens if we have cases
when we are able to request channel for memcpy via dma_request_chan()
(dedicated memcpy channel/DMA engine?) in that case we will have the SLAVE
set, but not MEMCPY, or any other variation we do not know yet?

>>
>> Signed-off-by: Peter Ujfalusi <[email protected]>
>> ---
>> drivers/dma/dmaengine.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
>> index daf54a39bcc7..52c3eee48e2e 100644
>> --- a/drivers/dma/dmaengine.c
>> +++ b/drivers/dma/dmaengine.c
>> @@ -184,6 +184,9 @@ __dma_device_satisfies_mask(struct dma_device *device,
>> {
>> dma_cap_mask_t has;
>>
>> + if (!want)
>> + return true;
>> +
>> bitmap_and(has.bits, want->bits, device->cap_mask.bits,
>> DMA_TX_TYPE_END);
>> return bitmap_equal(want->bits, has.bits, DMA_TX_TYPE_END);
>> --
>> 2.6.3
>>
>
>
>


--
Péter

2015-12-01 09:49:11

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

On 11/30/2015 04:09 PM, Arnd Bergmann wrote:
> On Monday 30 November 2015 15:45:33 Peter Ujfalusi wrote:
>> const char *name);
>> struct dma_chan *dma_request_slave_channel(struct device *dev, const char *name);
>> +
>> +struct dma_chan *dma_request_chan(struct device *dev, const char *name);
>> +struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
>> +
>> void dma_release_channel(struct dma_chan *chan);
>> int dma_get_slave_caps(struct dma_chan *chan, struct dma_slave_caps *caps);
>> #else
>> @@ -1268,6 +1291,14 @@ static inline struct dma_chan *dma_request_slave_channel(struct device *dev,
>> {
>> return NULL;
>> }
>> +static inline struct dma_chan *dma_request_chan(struct device *dev)
>> +{
>> + return ERR_PTR(-ENODEV);
>> +}
>>
>
> The prototypes for dma_request_chan() don't match, otherwise looks good.

Aargh, the !CONFIG_DMA_ENGINE path...
Fixed for the next RFC

Thanks,
P?ter

2015-12-01 09:57:20

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

On 11/30/2015 04:51 PM, Andy Shevchenko wrote:
>> +struct dma_chan *dma_request_chan(struct device *dev, const char *name)
>> +{
>> + struct dma_device *device, *_d;
>> + struct dma_chan *chan = NULL;
>> +
>> + /* If device-tree is present get slave info from here */
>> + if (dev->of_node)
>> + chan = of_dma_request_slave_channel(dev->of_node, name);
>> +
>> + /* If device was enumerated by ACPI get slave info from here */
>> + if (ACPI_HANDLE(dev) && !chan)
>
> The preferable way is to use
> has_acpi_companion() instead of ACPI_HANDLE().

I have done this part based on the dma_request_slave_channel_reason().
Will switch to use the has_acpi_companion() for the next RFC.

>> + chan = acpi_dma_request_slave_chan_by_name(dev, name);
>> +
>> + if (chan)
>> + return chan;
>> +

--
Péter

2015-12-01 09:59:31

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 04/15] dmaengine: edma: Add support for DMA filter mapping to slave devices

On 11/30/2015 04:11 PM, Arnd Bergmann wrote:
> On Monday 30 November 2015 15:45:34 Peter Ujfalusi wrote:
>> @@ -2428,6 +2436,22 @@ bool edma_filter_fn(struct dma_chan *chan, void *param)
>> }
>> EXPORT_SYMBOL(edma_filter_fn);
>>
>> +static bool edma_filter_for_map(struct dma_chan *chan, void *param)
>> +{
>> + bool match = false;
>> +
>> + if (chan->device->dev->driver == &edma_driver.driver) {
>> + struct edma_chan *echan = to_edma_chan(chan);
>> + unsigned ch_req = (unsigned)param;
>> + if (ch_req == echan->ch_num) {
>> + /* The channel is going to be used as HW synchronized */
>> + echan->hw_triggered = true;
>> + match = true;
>> + }
>> + }
>> + return match;
>> +}
>> +
>> static int edma_init(void)
>>
>
> I don't see the difference between edma_filter_fn and edma_filter_for_map.
> Why do you need both?

edma_filter_fn:
unsigned ch_req = *(unsigned *)param;

edma_filter_for_map:
unsigned ch_req = (unsigned)param;


If I want to reuse the edma_filter_fn, I would need an unsigned array for the
eDMA event numbers in the board files to be able to provide the pointer to
each of them.

--
P?ter

2015-12-01 10:07:44

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

On Tue, Dec 1, 2015 at 11:56 AM, Peter Ujfalusi <[email protected]> wrote:
> On 11/30/2015 04:51 PM, Andy Shevchenko wrote:
>>> +struct dma_chan *dma_request_chan(struct device *dev, const char *name)
>>> +{
>>> + struct dma_device *device, *_d;
>>> + struct dma_chan *chan = NULL;
>>> +
>>> + /* If device-tree is present get slave info from here */
>>> + if (dev->of_node)
>>> + chan = of_dma_request_slave_channel(dev->of_node, name);
>>> +
>>> + /* If device was enumerated by ACPI get slave info from here */
>>> + if (ACPI_HANDLE(dev) && !chan)
>>
>> The preferable way is to use
>> has_acpi_companion() instead of ACPI_HANDLE().
>
> I have done this part based on the dma_request_slave_channel_reason().

Understood, though that function was implemented before
has_acpi_companion() has been introduced.

> Will switch to use the has_acpi_companion() for the next RFC.

Good.

--
With Best Regards,
Andy Shevchenko

2015-12-01 10:13:22

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On 11/30/2015 04:18 PM, Arnd Bergmann wrote:
> On Monday 30 November 2015 15:45:30 Peter Ujfalusi wrote:
>> Changes since RFC v01:
>> - dma_request_chan(); lost the mask parameter
>> - The new API does not rely on RESOURCE_DMA, instead the dma_filter_map table
>> will be used to provide the needed information to the filter function in
>> legacy mode
>> - Extended the example patches to convert most of daVinci to use the new API to
>> request the DMA channels.
>
> Very nice overall. Just a few minor comments.
>
>> static struct dma_filter_map da830_edma_map[] = {
>> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
>> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
>> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
>> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
>> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
>> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
>> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
>> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
>> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
>> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
>> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
>> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
>> };
>
> I wonder if we should mandate that the lookup table is 'const'.

I had been wondering the same, I'll make it const for the next round.

> We could also drop the DMA_FILTER_ENTRY() macro above, and open-code the
> table like
>
> static struct dma_filter_map da830_edma_map[] = {
> { "davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)},
> { "davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)},
> { "davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)},
> ...
> };
>
> which is a little more compact and more obvious, but loses the
> named initializers.

We would need:
{ "da830-mmc.0", "rx", (void*)EDMA_CTLR_CHAN(0, 16) },
{ "da830-mmc.0", "tx", (void*)EDMA_CTLR_CHAN(0, 17) },

as we need to cast the param.
It is still compact, but having to add the (void*) casting makes it a bit ugly.

--
P?ter

2015-12-01 10:15:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 04/15] dmaengine: edma: Add support for DMA filter mapping to slave devices

On Tuesday 01 December 2015 11:58:53 Peter Ujfalusi wrote:
> On 11/30/2015 04:11 PM, Arnd Bergmann wrote:
> > On Monday 30 November 2015 15:45:34 Peter Ujfalusi wrote:
> >> @@ -2428,6 +2436,22 @@ bool edma_filter_fn(struct dma_chan *chan, void *param)
> >> }
> >> EXPORT_SYMBOL(edma_filter_fn);
> >>
> >> +static bool edma_filter_for_map(struct dma_chan *chan, void *param)
> >> +{
> >> + bool match = false;
> >> +
> >> + if (chan->device->dev->driver == &edma_driver.driver) {
> >> + struct edma_chan *echan = to_edma_chan(chan);
> >> + unsigned ch_req = (unsigned)param;
> >> + if (ch_req == echan->ch_num) {
> >> + /* The channel is going to be used as HW synchronized */
> >> + echan->hw_triggered = true;
> >> + match = true;
> >> + }
> >> + }
> >> + return match;
> >> +}
> >> +
> >> static int edma_init(void)
> >>
> >
> > I don't see the difference between edma_filter_fn and edma_filter_for_map.
> > Why do you need both?
>
> edma_filter_fn:
> unsigned ch_req = *(unsigned *)param;
>
> edma_filter_for_map:
> unsigned ch_req = (unsigned)param;

I see.

> If I want to reuse the edma_filter_fn, I would need an unsigned array for the
> eDMA event numbers in the board files to be able to provide the pointer to
> each of them.

How about this:

#define EDMA_CTLR_CHANP(ctlr, chan) ((const int[]){ [0] = ((ctlr) << 16) | (chan)})

static struct dma_filter_map da830_edma_map[] = {
{ "davinci-mcasp.0", "rx", EDMA_CTLR_CHANP(0, 0)},
{ "davinci-mcasp.0", "tx", EDMA_CTLR_CHANP(0, 1)},
{ "davinci-mcasp.1", "rx", EDMA_CTLR_CHANP(0, 2)},
...
};

That way, you create an anonymous constant integer expression and the data
points to that.

Arnd

2015-12-01 10:16:17

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On Tuesday 01 December 2015 12:12:47 Peter Ujfalusi wrote:
>
> We would need:
> { "da830-mmc.0", "rx", (void*)EDMA_CTLR_CHAN(0, 16) },
> { "da830-mmc.0", "tx", (void*)EDMA_CTLR_CHAN(0, 17) },
>
> as we need to cast the param.
> It is still compact, but having to add the (void*) casting makes it a bit ugly.

Right, I forgot that, but the cast could also be part of EDMA_CTLR_CHAN(),
and with the version I just posted in my other reply that part is solved
as well.

Arnd

2015-12-01 12:58:39

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [RFC v02 01/15] dmaengine: core: Allow NULL mask pointer in __dma_device_satisfies_mask()

On Tue, Dec 1, 2015 at 11:47 AM, Peter Ujfalusi <[email protected]> wrote:
> On 11/30/2015 04:35 PM, Andy Shevchenko wrote:
>> On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi <[email protected]> wrote:
>>> Treat as true condition the case when the mask is NULL.
>>
>> What do you think about setting some default (all "on") mask when mask
>> is not supplied?
>
> Probably rephrasing the commit message to say that when the mask is NULL it
> means that the caller does not care about the capabilities of the dma device
> thus return with true in such a case.
>
> We could also drop this patch and in private_candidate() :
>
> - if (!__dma_device_satisfies_mask(dev, mask)) {
> + if (mask && !__dma_device_satisfies_mask(dev, mask)) {
> pr_debug("%s: wrong capabilities\n", __func__);
> return NULL;
> }

Between patch and above proposal I would choose the latter one.

>> I don't know for sure but there might be cases when you don't want
>> literally *any* channel to satisfy.
>
> Or set DMA_SLAVE only in dma_request_chan()? What happens if we have cases
> when we are able to request channel for memcpy via dma_request_chan()
> (dedicated memcpy channel/DMA engine?) in that case we will have the SLAVE
> set, but not MEMCPY, or any other variation we do not know yet?

Frankly, have no idea.

--
With Best Regards,
Andy Shevchenko

2015-12-01 13:46:08

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On 11/30/2015 04:32 PM, Andy Shevchenko wrote:
>> Andy: I did looked at the unified device properties, but I decided to not to use
>> it as I don't see it to fit well and most of the legacy board files are using
>> resources to specify at least their memory regions so adding the DMA resource
>> to them would be more inline with the rest of the code.
>
> We could return back to this in the future, still we have to amend
> built-in device properties (there is a patch series already in the
> wild).

I believe we could have similar dmaengine 'infra' for the built-in device
properties as we have now for DT and ACPI. I need to dig deeper to get full
understanding on the device properties, but from a quick view it looks to me
that it could replace the direct OF and ACPI property handing in a unified
API. I might be totally mistaken here ;)

>> static struct dma_filter_map da830_edma_map[] = {
>> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
>> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
>> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
>> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
>> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
>> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
>> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
>> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
>> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
>> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
>> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
>> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
>
> Does this ".2" and so prevent driver to use auto ID for platform devices?

Yes, as all the infra around the traditional board files with platform_device
creation does. Ideally we could have 'phandle' pointing from this table to the
device in question (or other way around), but I'm not aware of anything we can
use.
Auto ID did not really worked for us since the driver does need to know their
ID in some cases, or we need to be able to be sure that for example McASP1 is
handled as davinci-mcasp.1
We have clocks and other dependencies where the device name or device ID need
to be predictable.
It is different in DT cases, but we are talking about legacy things.

--
Péter

2015-12-01 14:25:18

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On Tuesday 01 December 2015 15:45:32 Peter Ujfalusi wrote:
> >> static struct dma_filter_map da830_edma_map[] = {
> >> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> >> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> >> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> >> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> >> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> >> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> >> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> >
> > Does this ".2" and so prevent driver to use auto ID for platform devices?
>
> Yes, as all the infra around the traditional board files with platform_device
> creation does. Ideally we could have 'phandle' pointing from this table to the
> device in question (or other way around), but I'm not aware of anything we can
> use.

I was thinking that we could use a pointer to the device structure, but
if you have that, you can also just read the name from it.

Arnd

2015-12-01 16:57:09

by Vinod Koul

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On Mon, Nov 30, 2015 at 03:45:30PM +0200, Peter Ujfalusi wrote:
> Hi,
>
> Changes since RFC v01:
> - dma_request_chan(); lost the mask parameter
> - The new API does not rely on RESOURCE_DMA, instead the dma_filter_map table
> will be used to provide the needed information to the filter function in
> legacy mode
> - Extended the example patches to convert most of daVinci to use the new API to
> request the DMA channels.

Hi Peter,

Thanks a bunch for fixing this up!

I think overall I am happy with the proposal and aligning with 2 APIs really
_helps_.

>
> TODO: Documentation update ;)
>
>
> As it has been discussed in the following thread:
> http://www.gossamer-threads.com/lists/linux/kernel/2181487#2181487
>
> Andy: I did looked at the unified device properties, but I decided to not to use
> it as I don't see it to fit well and most of the legacy board files are using
> resources to specify at least their memory regions so adding the DMA resource
> to them would be more inline with the rest of the code.

I think that is okay for now, we can align on that eventually. I think this
doesn't impact the API design though..

>
> The ARM, mmc and spi patches are converting daVinci drivers board files to use
> the new method of requesting DMA channel.
>
> With this series I have taken a path which would result two new API, which can
> be used to convert most of the current users already and with some work all
> users might be able to move to this set.
> With this set the filter_fn used for legacy (non DT/ACPI) channel request is no
> longer needed to be exported to client drivers since the selection of the
> correct filter_fn will be done in the core.
>
> So, the first proposal is to have:
>
> struct dma_chan *dma_request_chan(struct device *dev, const char *name);
> struct dma_chan *dma_request_chan_by_mask(const dma_cap_mask_t *mask);
>
> The dma_request_chan_by_mask() is to request any channel matching with the
> requested capabilities, can be used to request channel for memcpy, memset, xor,
> etc where no hardware synchronization is needed.
>
> The dma_request_chan() is to request a slave channel. The dma_request_chan() will try to find the
> channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
> it will use a filter lookup table and retrieves the needed information from
> the dma_filter_map provided by the DMA drivers.

That sounds right, for the third case would the arch, driver or someone else
configure this?

> This legacy mode needs changes in platform code, in dmaengine drivers and
> finally the dmaengine user drivers can be converted:

Are you marking the current APIs as dericated in the end of this series

>
> For each dmaengine driver an array of DMA device, slave and the parameter
> for the filter function needs to be added:
>
> static struct dma_filter_map da830_edma_map[] = {
> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> };
>
> This information is going to be needed by the dmaengine driver, so
> modification to the platform_data is needed, and the driver map should be
> added to the pdata of the DMA driver:
>
> da8xx_edma0_pdata.filter_map = da830_edma_map;
> da8xx_edma0_pdata.filtercnt = ARRAY_SIZE(da830_edma_map);
>
> The DMA driver then needs to convigure the needed device -> filter_fn
> mapping before it registers with dma_async_device_register() :
>
> if (info->filter_map) {
> ecc->dma_slave.filter_map.map = info->filter_map;
> ecc->dma_slave.filter_map.mapcnt = info->filtercnt;
> ecc->dma_slave.filter_map.filter_fn = edma_filter_for_map;
> }
>
> When neither DT or ACPI lookup is available the dma_request_chan() will
> try to match the requester's device name with the filter_map's list of
> device names, when a match found it will use the information from the
> dma_filter_map to get the channel with the dma_get_channel() internal
> function.
>
> Regards,
> Peter
> ---
> Peter Ujfalusi (15):
> dmaengine: core: Allow NULL mask pointer in
> __dma_device_satisfies_mask()
> dmaengine: core: Move and merge the code paths using private_candidate
> dmaengine: core: Introduce new, universal API to request a channel
> dmaengine: edma: Add support for DMA filter mapping to slave devices
> ARM: davinci: devices-da8xx: Add dma_filter_map to edma
> ARM: davinci: dm355: Add dma_filter_map to edma
> ARM: davinci: dm365: Add dma_filter_map to edma
> ARM: davinci: dm644x: Add dma_filter_map to edma
> ARM: davinci: dm646x: Add dma_filter_map to edma
> mmc: davinci_mmc: Use dma_request_chan() to requesting DMA channel
> spi: davinci: Use dma_request_chan() to requesting DMA channel
> ARM: davinci: devices-da8xx: Remove DMA resources for MMC and SPI
> ARM: davinci: devices: Remove DMA resources for MMC
> ARM: davinci: dm355: Remove DMA resources for SPI
> ARM: davinci: dm365: Remove DMA resources for SPI
>
> arch/arm/mach-davinci/devices-da8xx.c | 95 ++++++++++----------
> arch/arm/mach-davinci/devices.c | 19 ----
> arch/arm/mach-davinci/dm355.c | 28 ++++--
> arch/arm/mach-davinci/dm365.c | 30 +++++--
> arch/arm/mach-davinci/dm644x.c | 12 +++
> arch/arm/mach-davinci/dm646x.c | 11 +++
> drivers/dma/dmaengine.c | 160 +++++++++++++++++++++++++---------
> drivers/dma/edma.c | 24 +++++
> drivers/mmc/host/davinci_mmc.c | 52 +++--------
> drivers/spi/spi-davinci.c | 76 +++++-----------
> include/linux/dmaengine.h | 31 +++++++
> include/linux/platform_data/edma.h | 5 ++
> 12 files changed, 330 insertions(+), 213 deletions(-)
>
> --
> 2.6.3
>

--
~Vinod

2015-12-01 17:00:42

by Tony Lindgren

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

* Peter Ujfalusi <[email protected]> [151201 00:14]:
> On 11/30/2015 05:51 PM, Tony Lindgren wrote:
> > Hi,
> >
> > * Peter Ujfalusi <[email protected]> [151130 05:49]:
> >>
> >> For each dmaengine driver an array of DMA device, slave and the parameter
> >> for the filter function needs to be added:
> >>
> >> static struct dma_filter_map da830_edma_map[] = {
> >> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> >> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> >> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> >> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> >> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> >> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> >> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> >> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> >> };
> >
> > FYI, if the EDMA_CTRL_CHAN above is just the evtmux registers
>
> No, they are not. They are the eDMA event numbers. I used the EDMA_CTRL_CHAN()
> macro for all board files to have uniform look for the data. The first
> parameter means the eDMA instance number while the second is the event number
> on that eDMA. Since most devices have only one eDMA, we have 0 as eDMA id in
> most cases.
> The eventmux, or crossbar is different thing and we have several versions of
> the event crossbar or mux used.

OK

> > those
> > can be handled with the pinctrl framework. It seems that would allow
> > leaving out some of the built-in look up data, and have the mux parts
> > handled by a proper device driver. Below is a sample from the dm81xx
> > platform for reference.
> >
> > SoC dtsi file:
> >
> > evtmux: pinmux@f90 {
> > compatible = "pinctrl-single";
> > reg = <0xf90 0x40>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > pinctrl-single,register-width = <8>;
> > pinctrl-single,function-mask = <0x1f>;
> > };
> >
> > Board specific dts file:
> >
> > &evtmux {
> > sd2_edma_pins: pinmux_sd2_edma_pins {
> > pinctrl-single,pins = <
> > 8 1 /* use SDTXEVT1 for EDMA instead of MCASP0TX */
> > 9 2 /* use SDRXEVT1 for EDMA instead of MCASP0RX */
> > >;
> > };
> > };
>
> I see. The dm81xx basically am33xx/am43xx?

Yeah similar to am33xx with different clocks and with a bunch of accelerators.

> Actually I would prefer to use the dmaengine's event router framework and we
> do have support for the am33xx/am43xx type of crossbar already implemented.
> I'm going to resend the DTS series for am33xx/am43xx to convert them to use
> the new DT bindings along with the dma event router support:
> https://www.mail-archive.com/linux-omap%40vger.kernel.org/msg120828.html

OK yes a dmaengine event router works too when available. Good to see
them as separate driver instances now :) Are only the dts changes missing
now?

FYI, when we have separate interconnect driver instances, we don't want to
and cannot tweak registers outside the interconnect instance because of them
being in separate clock and/or power domains :p

In any case, it seems there's no harm using pinctrl for evtmux on dm81xx
until the event router is available. It's currently only needed on the
t410 emmc that I'm aware of :)

> > Dynamic muxing of these channels can be done too using the pinctrl
> > framework named modes, but probably is not a good idea in the case of
> > SD card and MaASP in case something goes wrong :)
>
> In theory it can be done, but in practice it is not possible. It is up to the
> board design decision to select which DMA event is not needed to be used in
> default mode and that one can be used to route the crossbar hidden request to it.
> Just imaging: playing audio from MMC (in the example you have), audio needs
> constant DMA, so the MMC would never get DMA request, also the drivers tend to
> request the DMA channel in their probe/init and hold to it as long as they are
> loaded...

Yes dynamic muxing of the dma channels sounds like file corruption waiting
to happen :) Let's stay away from that.

Regards,

Tony

2015-12-01 17:04:18

by Vinod Koul

[permalink] [raw]
Subject: Re: [RFC v02 01/15] dmaengine: core: Allow NULL mask pointer in __dma_device_satisfies_mask()

On Tue, Dec 01, 2015 at 02:58:35PM +0200, Andy Shevchenko wrote:
> On Tue, Dec 1, 2015 at 11:47 AM, Peter Ujfalusi <[email protected]> wrote:
> > On 11/30/2015 04:35 PM, Andy Shevchenko wrote:
> >> On Mon, Nov 30, 2015 at 3:45 PM, Peter Ujfalusi <[email protected]> wrote:
> >>> Treat as true condition the case when the mask is NULL.
> >>
> >> What do you think about setting some default (all "on") mask when mask
> >> is not supplied?
> >
> > Probably rephrasing the commit message to say that when the mask is NULL it
> > means that the caller does not care about the capabilities of the dma device
> > thus return with true in such a case.
> >
> > We could also drop this patch and in private_candidate() :
> >
> > - if (!__dma_device_satisfies_mask(dev, mask)) {
> > + if (mask && !__dma_device_satisfies_mask(dev, mask)) {
> > pr_debug("%s: wrong capabilities\n", __func__);
> > return NULL;
> > }
>
> Between patch and above proposal I would choose the latter one.

Sounds better to me as well

>
> >> I don't know for sure but there might be cases when you don't want
> >> literally *any* channel to satisfy.
> >
> > Or set DMA_SLAVE only in dma_request_chan()? What happens if we have cases
> > when we are able to request channel for memcpy via dma_request_chan()
> > (dedicated memcpy channel/DMA engine?) in that case we will have the SLAVE
> > set, but not MEMCPY, or any other variation we do not know yet?
>
> Frankly, have no idea.

In slave cases I know that some controllers support memcpy but they are not
generic memcpy as they cannot be used for system memcpy but for 'special'
memcpy. So this can be used for memcpy as well

--
~Vinod

2015-12-01 17:20:04

by Vinod Koul

[permalink] [raw]
Subject: Re: [RFC v02 04/15] dmaengine: edma: Add support for DMA filter mapping to slave devices

On Mon, Nov 30, 2015 at 03:45:34PM +0200, Peter Ujfalusi wrote:
> Add support for providing device to filter_fn mapping so client drivers
> can switch to use the dma_request_chan() API.

Any reason why we dont want to go with DT based only for edma here?

>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> drivers/dma/edma.c | 24 ++++++++++++++++++++++++
> include/linux/platform_data/edma.h | 5 +++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 0675e268d577..386f8c9bd606 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -2098,6 +2098,8 @@ static struct dma_chan *of_edma_xlate(struct of_phandle_args *dma_spec,
> }
> #endif
>
> +static bool edma_filter_for_map(struct dma_chan *chan, void *param);
> +
> static int edma_probe(struct platform_device *pdev)
> {
> struct edma_soc_info *info = pdev->dev.platform_data;
> @@ -2297,6 +2299,12 @@ static int edma_probe(struct platform_device *pdev)
> edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot);
> }
>
> + if (info->filter_map) {
> + ecc->dma_slave.filter_map.map = info->filter_map;
> + ecc->dma_slave.filter_map.mapcnt = info->filtercnt;
> + ecc->dma_slave.filter_map.filter_fn = edma_filter_for_map;
> + }
> +
> ret = dma_async_device_register(&ecc->dma_slave);
> if (ret) {
> dev_err(dev, "slave ddev registration failed (%d)\n", ret);
> @@ -2428,6 +2436,22 @@ bool edma_filter_fn(struct dma_chan *chan, void *param)
> }
> EXPORT_SYMBOL(edma_filter_fn);
>
> +static bool edma_filter_for_map(struct dma_chan *chan, void *param)
> +{
> + bool match = false;
> +
> + if (chan->device->dev->driver == &edma_driver.driver) {
> + struct edma_chan *echan = to_edma_chan(chan);
> + unsigned ch_req = (unsigned)param;
> + if (ch_req == echan->ch_num) {
> + /* The channel is going to be used as HW synchronized */
> + echan->hw_triggered = true;
> + match = true;
> + }
> + }
> + return match;
> +}
> +
> static int edma_init(void)
> {
> int ret;
> diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
> index e2878baeb90e..117a36d63840 100644
> --- a/include/linux/platform_data/edma.h
> +++ b/include/linux/platform_data/edma.h
> @@ -59,6 +59,8 @@ struct edma_rsv_info {
> const s16 (*rsv_slots)[2];
> };
>
> +struct dma_filter_map;
> +
> /* platform_data for EDMA driver */
> struct edma_soc_info {
> /*
> @@ -76,6 +78,9 @@ struct edma_soc_info {
>
> s8 (*queue_priority_mapping)[2];
> const s16 (*xbar_chans)[2];
> +
> + struct dma_filter_map *filter_map;
> + int filtercnt;
> };
>
> #endif
> --
> 2.6.3
>

--
~Vinod

2015-12-01 20:17:44

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On Tuesday 01 December 2015 22:29:54 Vinod Koul wrote:
> On Mon, Nov 30, 2015 at 03:45:30PM +0200, Peter Ujfalusi wrote:
> > channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
> > it will use a filter lookup table and retrieves the needed information from
> > the dma_filter_map provided by the DMA drivers.
>
> That sounds right, for the third case would the arch, driver or someone else
> configure this?

The typical case is for the configuration to be defined in arch or platform
code and passed down to the dmaengine driver.

I just noticed that the text above (and probably the code too) should
be changed so we always fall back to this. There are cases where the
platform is booted with DT in principle, but the DMA engine does not
(yet) use DT and still wants to be converted. I think we can easily
handle that case by always trying this if the other methods fail.

> > This legacy mode needs changes in platform code, in dmaengine drivers and
> > finally the dmaengine user drivers can be converted:
>
> Are you marking the current APIs as dericated in the end of this series

I think we practically stopped marking things as deprecated in general.
Per Linus decree, whenever we want to get rid of something, we should
instead change all users in tree and then remove the API, expecting
driver maintainers to do something just because you marked it as deprecated
often doesn't work.

I can help out converting a few platforms, maybe one interface at a time.
This is what I see:

arnd@wuerfel:~/arm-soc$ for i in dma_request_slave_channel_reason dma_request_slave_channel dma_request_slave_channel_compat dma_request_channel ; do echo `git grep -wl $i drivers/ | grep -v drivers/dma | wc -l`\ $i ; done
14 dma_request_slave_channel_reason
27 dma_request_slave_channel
25 dma_request_slave_channel_compat
34 dma_request_channel

I would probably leave the users of dma_request_channel() while converting
the others, as that is still used by all the platforms that don't use any DT
support.

Changing dma_request_slave_channel_reason and dma_request_slave_channel is
trivial, we can probably use coccinelle for that, as it does not require
any platform changes. That brings us to the users of
dma_request_slave_channel_compat, which currently includes these files:

$ git grep -wl dma_request_slave_channel_compat drivers/ata/pata_pxa.c
drivers/crypto/atmel-aes.c
drivers/crypto/atmel-sha.c
drivers/crypto/atmel-tdes.c
drivers/crypto/omap-aes.c
drivers/crypto/omap-des.c
drivers/crypto/omap-sham.c
drivers/media/platform/omap3isp/isphist.c
drivers/mmc/host/davinci_mmc.c
drivers/mmc/host/omap.c
drivers/mmc/host/omap_hsmmc.c
drivers/mmc/host/pxamci.c
drivers/mmc/host/s3cmci.c
drivers/mmc/host/tmio_mmc_dma.c
drivers/mtd/nand/pxa3xx_nand.c
drivers/net/ethernet/smsc/smc91x.c
drivers/net/irda/pxaficp_ir.c
drivers/spi/spi-omap2-mcspi.c
drivers/spi/spi-pxa2xx-dma.c
drivers/spi/spi-rspi.c
drivers/spi/spi-s3c64xx.c
drivers/spi/spi-sh-msiof.c
drivers/tty/serial/8250/8250_dma.c
drivers/tty/serial/samsung.c
drivers/tty/serial/sh-sci.c
include/linux/dmaengine.h

In other words: arch/avr32 and arch/sh along with omap1, omap2, davinci, pxa, and s3c
in terms of ARM platforms.

Arnd

2015-12-01 20:21:22

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 04/15] dmaengine: edma: Add support for DMA filter mapping to slave devices

On Tuesday 01 December 2015 22:52:12 Vinod Koul wrote:
> On Mon, Nov 30, 2015 at 03:45:34PM +0200, Peter Ujfalusi wrote:
> > Add support for providing device to filter_fn mapping so client drivers
> > can switch to use the dma_request_chan() API.
>
> Any reason why we dont want to go with DT based only for edma here?

I think the OMAP2 based platforms using edma are all DT-only, but mach-davinci
would need a lot of work for that.

Arnd

2015-12-02 04:34:36

by Vinod Koul

[permalink] [raw]
Subject: Re: [RFC v02 04/15] dmaengine: edma: Add support for DMA filter mapping to slave devices

On Tue, Dec 01, 2015 at 09:20:28PM +0100, Arnd Bergmann wrote:
> On Tuesday 01 December 2015 22:52:12 Vinod Koul wrote:
> > On Mon, Nov 30, 2015 at 03:45:34PM +0200, Peter Ujfalusi wrote:
> > > Add support for providing device to filter_fn mapping so client drivers
> > > can switch to use the dma_request_chan() API.
> >
> > Any reason why we dont want to go with DT based only for edma here?
>
> I think the OMAP2 based platforms using edma are all DT-only, but mach-davinci
> would need a lot of work for that.

Okay sound fine then

--
~Vinod

2015-12-02 04:49:15

by Vinod Koul

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On Tue, Dec 01, 2015 at 09:17:06PM +0100, Arnd Bergmann wrote:
> On Tuesday 01 December 2015 22:29:54 Vinod Koul wrote:
> > On Mon, Nov 30, 2015 at 03:45:30PM +0200, Peter Ujfalusi wrote:
> > > channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
> > > it will use a filter lookup table and retrieves the needed information from
> > > the dma_filter_map provided by the DMA drivers.
> >
> > That sounds right, for the third case would the arch, driver or someone else
> > configure this?
>
> The typical case is for the configuration to be defined in arch or platform
> code and passed down to the dmaengine driver.
>
> I just noticed that the text above (and probably the code too) should
> be changed so we always fall back to this. There are cases where the
> platform is booted with DT in principle, but the DMA engine does not
> (yet) use DT and still wants to be converted. I think we can easily
> handle that case by always trying this if the other methods fail.

I agree that this makes sense, not just for DT cases but ACPI as well

>
> > > This legacy mode needs changes in platform code, in dmaengine drivers and
> > > finally the dmaengine user drivers can be converted:
> >
> > Are you marking the current APIs as dericated in the end of this series
>
> I think we practically stopped marking things as deprecated in general.
> Per Linus decree, whenever we want to get rid of something, we should
> instead change all users in tree and then remove the API, expecting
> driver maintainers to do something just because you marked it as deprecated
> often doesn't work.

Yes but while we do conversion we don't know if new users get added which use
old API..

>
> I can help out converting a few platforms, maybe one interface at a time.

Great yes we all will have to chip in and start removing these, i will try
doing few after new year

Am sure Andy can chip in as well :)

> This is what I see:
>
> arnd@wuerfel:~/arm-soc$ for i in dma_request_slave_channel_reason dma_request_slave_channel dma_request_slave_channel_compat dma_request_channel ; do echo `git grep -wl $i drivers/ | grep -v drivers/dma | wc -l`\ $i ; done
> 14 dma_request_slave_channel_reason
> 27 dma_request_slave_channel
> 25 dma_request_slave_channel_compat
> 34 dma_request_channel
>
> I would probably leave the users of dma_request_channel() while converting
> the others, as that is still used by all the platforms that don't use any DT
> support.
>
> Changing dma_request_slave_channel_reason and dma_request_slave_channel is
> trivial, we can probably use coccinelle for that, as it does not require
> any platform changes. That brings us to the users of
> dma_request_slave_channel_compat, which currently includes these files:
>
> $ git grep -wl dma_request_slave_channel_compat drivers/ata/pata_pxa.c
> drivers/crypto/atmel-aes.c
> drivers/crypto/atmel-sha.c
> drivers/crypto/atmel-tdes.c
> drivers/crypto/omap-aes.c
> drivers/crypto/omap-des.c
> drivers/crypto/omap-sham.c
> drivers/media/platform/omap3isp/isphist.c
> drivers/mmc/host/davinci_mmc.c
> drivers/mmc/host/omap.c
> drivers/mmc/host/omap_hsmmc.c
> drivers/mmc/host/pxamci.c
> drivers/mmc/host/s3cmci.c
> drivers/mmc/host/tmio_mmc_dma.c
> drivers/mtd/nand/pxa3xx_nand.c
> drivers/net/ethernet/smsc/smc91x.c
> drivers/net/irda/pxaficp_ir.c
> drivers/spi/spi-omap2-mcspi.c
> drivers/spi/spi-pxa2xx-dma.c
> drivers/spi/spi-rspi.c
> drivers/spi/spi-s3c64xx.c
> drivers/spi/spi-sh-msiof.c
> drivers/tty/serial/8250/8250_dma.c
> drivers/tty/serial/samsung.c
> drivers/tty/serial/sh-sci.c
> include/linux/dmaengine.h
>
> In other words: arch/avr32 and arch/sh along with omap1, omap2, davinci, pxa, and s3c
> in terms of ARM platforms.
>
> Arnd

--
~Vinod

2015-12-02 08:24:35

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On Wednesday 02 December 2015 10:22:09 Vinod Koul wrote:
> >
> > > > This legacy mode needs changes in platform code, in dmaengine drivers and
> > > > finally the dmaengine user drivers can be converted:
> > >
> > > Are you marking the current APIs as dericated in the end of this series
> >
> > I think we practically stopped marking things as deprecated in general.
> > Per Linus decree, whenever we want to get rid of something, we should
> > instead change all users in tree and then remove the API, expecting
> > driver maintainers to do something just because you marked it as deprecated
> > often doesn't work.
>
> Yes but while we do conversion we don't know if new users get added which use
> old API..

We probably don't need to worry about new users of dma_request_channel(),
as we don't add new ARM platforms without DT, and other architectures
don't add a lot of new platforms. Similarly, I don't expect a whole lot
of dma_request_slave_channel_compat() users, because the conversion from
board files to DT based booting has slowed down a lot after most of the
actively maintained platforms are done with it.

If you do a one-line patch to add dma_request_chan() as an alias for
dma_request_slave_channel_reason() now, we can probably convert most
users of dma_request_slave_channel() and dma_request_slave_channel_reason()
to dma_request_chan() in the next merge window and do a patch to
replace the few remaining ones and remove the API one merge window later.

If wewant to stage out the conversion of the
dma_request_slave_channel_compat() and dma_request_channel() similarly,
it would be good to have all the interface changes for the dmaengine
core basically in place, so we can start to convert the platforms
independently for the 4.5 merge window without having a dependency on
dmaengine patches. It's probably best to not convert a slave driver
away from dma_request_channel() until the dma map has been created
for all platforms using that driver, again to avoid having too many
dependencies.

Arnd

2015-12-02 10:01:24

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

On 12/01/2015 07:00 PM, Tony Lindgren wrote:
>> I see. The dm81xx basically am33xx/am43xx?
>
> Yeah similar to am33xx with different clocks and with a bunch of accelerators.
>
>> Actually I would prefer to use the dmaengine's event router framework and we
>> do have support for the am33xx/am43xx type of crossbar already implemented.
>> I'm going to resend the DTS series for am33xx/am43xx to convert them to use
>> the new DT bindings along with the dma event router support:
>> https://www.mail-archive.com/linux-omap%40vger.kernel.org/msg120828.html
>
> OK yes a dmaengine event router works too when available. Good to see
> them as separate driver instances now :) Are only the dts changes missing
> now?
>
> FYI, when we have separate interconnect driver instances, we don't want to
> and cannot tweak registers outside the interconnect instance because of them
> being in separate clock and/or power domains :p

What does this mean in practice? We can not touch these registers? The DMA
crossbar is a separate driver from the eDMA driver.

> In any case, it seems there's no harm using pinctrl for evtmux on dm81xx
> until the event router is available. It's currently only needed on the
> t410 emmc that I'm aware of :)

The AM33xx/AM43xx DMA crossbar support is in 4.4 already:
42dbdcc6bf96 dmaengine: ti-dma-crossbar: Add support for crossbar on AM33xx/AM43xx

--
P?ter

2015-12-02 10:02:53

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 04/15] dmaengine: edma: Add support for DMA filter mapping to slave devices

On 12/02/2015 06:37 AM, Vinod Koul wrote:
> On Tue, Dec 01, 2015 at 09:20:28PM +0100, Arnd Bergmann wrote:
>> On Tuesday 01 December 2015 22:52:12 Vinod Koul wrote:
>>> On Mon, Nov 30, 2015 at 03:45:34PM +0200, Peter Ujfalusi wrote:
>>>> Add support for providing device to filter_fn mapping so client drivers
>>>> can switch to use the dma_request_chan() API.
>>>
>>> Any reason why we dont want to go with DT based only for edma here?
>>
>> I think the OMAP2 based platforms using edma are all DT-only, but mach-davinci
>> would need a lot of work for that.
>
> Okay sound fine then

Yes, daVinci is mostly board file based and the problem is that those devices
are hard to find to do the conversion to DT.

--
P?ter

2015-12-02 10:52:19

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On 12/01/2015 04:24 PM, Arnd Bergmann wrote:
> On Tuesday 01 December 2015 15:45:32 Peter Ujfalusi wrote:
>>>> static struct dma_filter_map da830_edma_map[] = {
>>>> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
>>>> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
>>>> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
>>>> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
>>>> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
>>>> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
>>>> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
>>>> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
>>>> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
>>>> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
>>>> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
>>>> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
>>>
>>> Does this ".2" and so prevent driver to use auto ID for platform devices?
>>
>> Yes, as all the infra around the traditional board files with platform_device
>> creation does. Ideally we could have 'phandle' pointing from this table to the
>> device in question (or other way around), but I'm not aware of anything we can
>> use.
>
> I was thinking that we could use a pointer to the device structure, but
> if you have that, you can also just read the name from it.

Hrm to have pdev pointer instead of the devname string? In the core we could
get the pdev from the caller's device with to_platform_device(dev) and simply
compare the pointers...

One of the issue I see (in mach-davinci and mach-omap1/2) is that the pdevs
are scattered around in the c files so gathering the pointers to a place where
we can see them to be able to use it in the dma_filter_map is not going to be
light weight task.
Furthermore we have the omap_hwmod for OMAP2+ which creates the actual pdevs
and resources from the omap_hwmod data structures so getting out the DMA event
numbers there is not going to be easy either...

I'll hold back the RFC v3 to see if we should switch to pdev pointer or stay
with the devname strings here.

--
P?ter

2015-12-02 12:30:09

by Peter Ujfalusi

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On 12/01/2015 10:17 PM, Arnd Bergmann wrote:
> On Tuesday 01 December 2015 22:29:54 Vinod Koul wrote:
>> On Mon, Nov 30, 2015 at 03:45:30PM +0200, Peter Ujfalusi wrote:
>>> channel via DT, ACPI or in case if the kernel booted in non DT/ACPI mode
>>> it will use a filter lookup table and retrieves the needed information from
>>> the dma_filter_map provided by the DMA drivers.
>>
>> That sounds right, for the third case would the arch, driver or someone else
>> configure this?
>
> The typical case is for the configuration to be defined in arch or platform
> code and passed down to the dmaengine driver.
>
> I just noticed that the text above (and probably the code too) should
> be changed so we always fall back to this. There are cases where the
> platform is booted with DT in principle, but the DMA engine does not
> (yet) use DT and still wants to be converted. I think we can easily
> handle that case by always trying this if the other methods fail.

Yes, it was intentional actually to not fall back to legacy lookup. With the
dma_request_slave_channel_compat() I have had cases when the DT binding was
not correct - or actually when trying to get deferred working that the code
would fall back to legacy mode and it got me a channel which is not usable.
I did not know that we have platforms booting in DT but the dma binding is not
working so it uses other method to get channel.
I think, if we allow the fall back within dma_request_chan() it would not
cause the same issue as the dma_request_slave_channel_compat() did as long as
we are not providing the lookup table on DT booted cases when the DMA binding
is working correctly.
Let me see the flow, but I think it is doable.

--
P?ter

2015-12-02 13:39:18

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC v02 00/15] dmaengine: New 'universal' API for requesting channel

On Wednesday 02 December 2015 12:51:43 Peter Ujfalusi wrote:
> On 12/01/2015 04:24 PM, Arnd Bergmann wrote:
> > On Tuesday 01 December 2015 15:45:32 Peter Ujfalusi wrote:
> >>>> static struct dma_filter_map da830_edma_map[] = {
> >>>> DMA_FILTER_ENTRY("davinci-mcasp.0", "rx", EDMA_CTLR_CHAN(0, 0)),
> >>>> DMA_FILTER_ENTRY("davinci-mcasp.0", "tx", EDMA_CTLR_CHAN(0, 1)),
> >>>> DMA_FILTER_ENTRY("davinci-mcasp.1", "rx", EDMA_CTLR_CHAN(0, 2)),
> >>>> DMA_FILTER_ENTRY("davinci-mcasp.1", "tx", EDMA_CTLR_CHAN(0, 3)),
> >>>> DMA_FILTER_ENTRY("davinci-mcasp.2", "rx", EDMA_CTLR_CHAN(0, 4)),
> >>>> DMA_FILTER_ENTRY("davinci-mcasp.2", "tx", EDMA_CTLR_CHAN(0, 5)),
> >>>> DMA_FILTER_ENTRY("spi_davinci.0", "rx", EDMA_CTLR_CHAN(0, 14)),
> >>>> DMA_FILTER_ENTRY("spi_davinci.0", "tx", EDMA_CTLR_CHAN(0, 15)),
> >>>> DMA_FILTER_ENTRY("da830-mmc.0", "rx", EDMA_CTLR_CHAN(0, 16)),
> >>>> DMA_FILTER_ENTRY("da830-mmc.0", "tx", EDMA_CTLR_CHAN(0, 17)),
> >>>> DMA_FILTER_ENTRY("spi_davinci.1", "rx", EDMA_CTLR_CHAN(0, 18)),
> >>>> DMA_FILTER_ENTRY("spi_davinci.1", "tx", EDMA_CTLR_CHAN(0, 19)),
> >>>
> >>> Does this ".2" and so prevent driver to use auto ID for platform devices?
> >>
> >> Yes, as all the infra around the traditional board files with platform_device
> >> creation does. Ideally we could have 'phandle' pointing from this table to the
> >> device in question (or other way around), but I'm not aware of anything we can
> >> use.
> >
> > I was thinking that we could use a pointer to the device structure, but
> > if you have that, you can also just read the name from it.
>
> Hrm to have pdev pointer instead of the devname string? In the core we could
> get the pdev from the caller's device with to_platform_device(dev) and simply
> compare the pointers...
>
> One of the issue I see (in mach-davinci and mach-omap1/2) is that the pdevs
> are scattered around in the c files so gathering the pointers to a place where
> we can see them to be able to use it in the dma_filter_map is not going to be
> light weight task.
> Furthermore we have the omap_hwmod for OMAP2+ which creates the actual pdevs
> and resources from the omap_hwmod data structures so getting out the DMA event
> numbers there is not going to be easy either...
>
> I'll hold back the RFC v3 to see if we should switch to pdev pointer or stay
> with the devname strings here.

Sorry, what I meant to say above is that I had already concluded that using
device pointers was a bad idea and we should stay with names.

Arnd

2015-12-02 15:00:15

by Tony Lindgren

[permalink] [raw]
Subject: Re: [RFC v02 03/15] dmaengine: core: Introduce new, universal API to request a channel

* Peter Ujfalusi <[email protected]> [151202 02:01]:
> On 12/01/2015 07:00 PM, Tony Lindgren wrote:
> >> I see. The dm81xx basically am33xx/am43xx?
> >
> > Yeah similar to am33xx with different clocks and with a bunch of accelerators.
> >
> >> Actually I would prefer to use the dmaengine's event router framework and we
> >> do have support for the am33xx/am43xx type of crossbar already implemented.
> >> I'm going to resend the DTS series for am33xx/am43xx to convert them to use
> >> the new DT bindings along with the dma event router support:
> >> https://www.mail-archive.com/linux-omap%40vger.kernel.org/msg120828.html
> >
> > OK yes a dmaengine event router works too when available. Good to see
> > them as separate driver instances now :) Are only the dts changes missing
> > now?
> >
> > FYI, when we have separate interconnect driver instances, we don't want to
> > and cannot tweak registers outside the interconnect instance because of them
> > being in separate clock and/or power domains :p
>
> What does this mean in practice? We can not touch these registers? The DMA
> crossbar is a separate driver from the eDMA driver.

Seem like it should not affect DMA crossbar as it's a separate driver.

What I meant is we need a separate driver for clocks, pinctrl, regulators,
PHY, crossbar or syscon to use the SCM registers from drivers sitting on a
L4 interonnect.

> > In any case, it seems there's no harm using pinctrl for evtmux on dm81xx
> > until the event router is available. It's currently only needed on the
> > t410 emmc that I'm aware of :)
>
> The AM33xx/AM43xx DMA crossbar support is in 4.4 already:
> 42dbdcc6bf96 dmaengine: ti-dma-crossbar: Add support for crossbar on AM33xx/AM43xx

OK great.

Tony