* [PATCH 1/4] dmaengine: core: Skip mask matching when it is not provided to private_candidate
2015-12-03 14:33 [PATCH 0/4] dmaengine: New 'universal' API for requesting channel Peter Ujfalusi
@ 2015-12-03 14:33 ` Peter Ujfalusi
2015-12-03 14:33 ` [PATCH 2/4] dmaengine: core: Move and merge the code paths using private_candidate Peter Ujfalusi
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Peter Ujfalusi @ 2015-12-03 14:33 UTC (permalink / raw)
To: linux-arm-kernel
If mask is NULL skip the mask matching against the DMA device capabilities.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
drivers/dma/dmaengine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index daf54a39bcc7..6311e1fc80be 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -515,7 +515,7 @@ static struct dma_chan *private_candidate(const dma_cap_mask_t *mask,
{
struct dma_chan *chan;
- if (!__dma_device_satisfies_mask(dev, mask)) {
+ if (mask && !__dma_device_satisfies_mask(dev, mask)) {
pr_debug("%s: wrong capabilities\n", __func__);
return NULL;
}
--
2.6.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/4] dmaengine: core: Move and merge the code paths using private_candidate
2015-12-03 14:33 [PATCH 0/4] dmaengine: New 'universal' API for requesting channel Peter Ujfalusi
2015-12-03 14:33 ` [PATCH 1/4] dmaengine: core: Skip mask matching when it is not provided to private_candidate Peter Ujfalusi
@ 2015-12-03 14:33 ` Peter Ujfalusi
2015-12-03 14:33 ` [PATCH 3/4] dmaengine: core: Introduce new, universal API to request a channel Peter Ujfalusi
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Peter Ujfalusi @ 2015-12-03 14:33 UTC (permalink / raw)
To: linux-arm-kernel
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 find_candidate() to provide consistent execution and
going to allow us to reuse this mode of channel lookup later.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
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 6311e1fc80be..ea9d66982d40 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -546,6 +546,42 @@ static struct dma_chan *private_candidate(const dma_cap_mask_t *mask,
return NULL;
}
+static struct dma_chan *find_candidate(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
@@ -584,7 +620,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);
@@ -592,23 +627,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 = find_candidate(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);
@@ -625,35 +648,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 = find_candidate(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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/4] dmaengine: core: Introduce new, universal API to request a channel
2015-12-03 14:33 [PATCH 0/4] dmaengine: New 'universal' API for requesting channel Peter Ujfalusi
2015-12-03 14:33 ` [PATCH 1/4] dmaengine: core: Skip mask matching when it is not provided to private_candidate Peter Ujfalusi
2015-12-03 14:33 ` [PATCH 2/4] dmaengine: core: Move and merge the code paths using private_candidate Peter Ujfalusi
@ 2015-12-03 14:33 ` Peter Ujfalusi
2015-12-03 15:32 ` Arnd Bergmann
2015-12-03 16:30 ` Andy Shevchenko
2015-12-03 14:33 ` [PATCH 4/4] dmaengine: edma: Add support for DMA filter mapping to slave devices Peter Ujfalusi
2015-12-03 16:32 ` [PATCH 0/4] dmaengine: New 'universal' API for requesting channel Andy Shevchenko
4 siblings, 2 replies; 13+ messages in thread
From: Peter Ujfalusi @ 2015-12-03 14:33 UTC (permalink / raw)
To: linux-arm-kernel
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_slave_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 const struct dma_slave_map da830_edma_map[] = {
{ "davinci-mcasp.0", "rx", EDMA_FILTER_PARAM(0, 0) },
{ "davinci-mcasp.0", "tx", EDMA_FILTER_PARAM(0, 1) },
{ "davinci-mcasp.1", "rx", EDMA_FILTER_PARAM(0, 2) },
{ "davinci-mcasp.1", "tx", EDMA_FILTER_PARAM(0, 3) },
{ "davinci-mcasp.2", "rx", EDMA_FILTER_PARAM(0, 4) },
{ "davinci-mcasp.2", "tx", EDMA_FILTER_PARAM(0, 5) },
{ "spi_davinci.0", "rx", EDMA_FILTER_PARAM(0, 14) },
{ "spi_davinci.0", "tx", EDMA_FILTER_PARAM(0, 15) },
{ "da830-mmc.0", "rx", EDMA_FILTER_PARAM(0, 16) },
{ "da830-mmc.0", "tx", EDMA_FILTER_PARAM(0, 17) },
{ "spi_davinci.1", "rx", EDMA_FILTER_PARAM(0, 18) },
{ "spi_davinci.1", "tx", EDMA_FILTER_PARAM(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.slave_map = da830_edma_map;
da8xx_edma0_pdata.slavecnt = ARRAY_SIZE(da830_edma_map);
The DMA driver then needs to configure the needed device -> filter_fn
mapping before it registers with dma_async_device_register() :
if (info->slave_map) {
ecc->dma_slave.filter_map.map = info->slave_map;
ecc->dma_slave.filter_map.mapcnt = info->slavecnt;
ecc->dma_slave.filter_map.filter_fn = edma_filter_fn;
}
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_slave_map to get the channel with the dma_get_channel() internal
function.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
Documentation/dmaengine/client.txt | 23 +++-------
drivers/dma/dmaengine.c | 94 ++++++++++++++++++++++++++++++++++++++
include/linux/dmaengine.h | 42 +++++++++++++++++
3 files changed, 142 insertions(+), 17 deletions(-)
diff --git a/Documentation/dmaengine/client.txt b/Documentation/dmaengine/client.txt
index d9f9f461102a..9e33189745f0 100644
--- a/Documentation/dmaengine/client.txt
+++ b/Documentation/dmaengine/client.txt
@@ -22,25 +22,14 @@ The slave DMA usage consists of following steps:
Channel allocation is slightly different in the slave DMA context,
client drivers typically need a channel from a particular DMA
controller only and even in some cases a specific channel is desired.
- To request a channel dma_request_channel() API is used.
+ To request a channel dma_request_chan() API is used.
Interface:
- struct dma_chan *dma_request_channel(dma_cap_mask_t mask,
- dma_filter_fn filter_fn,
- void *filter_param);
- where dma_filter_fn is defined as:
- typedef bool (*dma_filter_fn)(struct dma_chan *chan, void *filter_param);
-
- The 'filter_fn' parameter is optional, but highly recommended for
- slave and cyclic channels as they typically need to obtain a specific
- DMA channel.
-
- When the optional 'filter_fn' parameter is NULL, dma_request_channel()
- simply returns the first channel that satisfies the capability mask.
-
- Otherwise, the 'filter_fn' routine will be called once for each free
- channel which has a capability in 'mask'. 'filter_fn' is expected to
- return 'true' when the desired DMA channel is found.
+ struct dma_chan *dma_request_chan(struct device *dev, const char *name);
+
+ Which will find and return the 'name' DMA channel associated with the 'dev'
+ device. The association is done via DT, ACPI or board file based
+ dma_slave_map matching table.
A channel allocated via this interface is exclusive to the caller,
until dma_release_channel() is called.
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index ea9d66982d40..fee4912d64d6 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>
@@ -712,6 +713,99 @@ struct dma_chan *dma_request_slave_channel(struct device *dev,
}
EXPORT_SYMBOL_GPL(dma_request_slave_channel);
+const static struct dma_slave_map *dma_filter_match(struct dma_device *device,
+ const char *name,
+ struct device *dev)
+{
+ int i;
+
+ if (!device->filter_map.mapcnt)
+ return NULL;
+
+ for (i = 0; i < device->filter_map.mapcnt; i++) {
+ const struct dma_slave_map *map = &device->filter_map.map[i];
+
+ if (!strcmp(map->devname, dev_name(dev)) &&
+ !strcmp(map->slave, name))
+ return map;
+ }
+
+ return NULL;
+}
+
+/**
+ * dma_request_chan - try to allocate an exclusive slave channel
+ * @dev: pointer to client device structure
+ * @name: slave channel name
+ *
+ * Returns pointer to appropriate DMA channel on success or an error pointer.
+ */
+struct dma_chan *dma_request_chan(struct device *dev, const char *name)
+{
+ struct dma_device *d, *_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 (has_acpi_companion(dev) && !chan)
+ chan = acpi_dma_request_slave_chan_by_name(dev, name);
+
+ if (chan) {
+ /* Valid channel found */
+ if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
+ return chan;
+
+ pr_warn("%s: %s DMA request failed, falling back to legacy\n",
+ __func__, dev->of_node ? "OF" : "ACPI");
+ }
+
+ /* Try to find the channel via the DMA filter map(s) */
+ mutex_lock(&dma_list_mutex);
+ list_for_each_entry_safe(d, _d, &dma_device_list, global_node) {
+ dma_cap_mask_t mask;
+ const struct dma_slave_map *map = dma_filter_match(d, name, dev);
+
+ if (!map)
+ continue;
+
+ dma_cap_zero(mask);
+ dma_cap_set(DMA_SLAVE, mask);
+
+ chan = find_candidate(d, &mask, d->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);
+
+/**
+ * dma_request_chan_by_mask - allocate a channel satisfying certain capabilities
+ * @mask: capabilities that the channel must satisfy
+ *
+ * Returns pointer to appropriate DMA channel on success or an error pointer.
+ */
+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..1c3997461ae7 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -607,11 +607,38 @@ enum dmaengine_alignment {
};
/**
+ * struct dma_slave_map - associates slave device and it's slave channel with
+ * parameter to be used by a filter function
+ * @devname: name of the device
+ * @slave: slave channel name
+ * @param: opaque parameter to pass to struct dma_filter.filter_fn
+ */
+struct dma_slave_map {
+ const char *devname;
+ const char *slave;
+ void *param;
+};
+
+/**
+ * struct dma_filter - information for slave device/channel to filter_fn/param
+ * mapping
+ * @filter_fn: filter function callback
+ * @mapcnt: number of slave device/channel in the map
+ * @map: array of channel to filter mapping data
+ */
+struct dma_filter {
+ dma_filter_fn filter_fn;
+ int mapcnt;
+ const struct dma_slave_map *map;
+};
+
+/**
* struct dma_device - info on the entity supplying DMA services
* @chancnt: how many DMA channels are supported
* @privatecnt: how many DMA channels are requested by dma_request_channel
* @channels: the list of struct dma_chan
* @global_node: list_head for global dma_device_list
+ * @filter_map: information for device/slave to filter function/param mapping
* @cap_mask: one or more dma_capability flags
* @max_xor: maximum number of xor sources, 0 if no capability
* @max_pq: maximum number of PQ sources and PQ-continue capability
@@ -669,6 +696,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 +1263,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 +1300,16 @@ 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,
+ const char *name)
+{
+ 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/4] dmaengine: core: Introduce new, universal API to request a channel
2015-12-03 14:33 ` [PATCH 3/4] dmaengine: core: Introduce new, universal API to request a channel Peter Ujfalusi
@ 2015-12-03 15:32 ` Arnd Bergmann
2015-12-03 15:42 ` Peter Ujfalusi
2015-12-03 16:30 ` Andy Shevchenko
1 sibling, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2015-12-03 15:32 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 03 December 2015 16:33:11 Peter Ujfalusi wrote:
> +
> +/**
> + * dma_request_chan - try to allocate an exclusive slave channel
> + * @dev: pointer to client device structure
> + * @name: slave channel name
> + *
> + * Returns pointer to appropriate DMA channel on success or an error pointer.
> + */
> +struct dma_chan *dma_request_chan(struct device *dev, const char *name)
> +{
> + struct dma_device *d, *_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 (has_acpi_companion(dev) && !chan)
> + chan = acpi_dma_request_slave_chan_by_name(dev, name);
I just noticed that you are creating this as a new interface, rather than
changing dma_request_slave_channel_reason() and making the old interface
a static inline wrapper. What is the reasoning behind that?
I think if we make both interfaces do the same thing, it's easier
to do the migration.
> + if (chan) {
> + /* Valid channel found */
> + if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
> + return chan;
> +
> + pr_warn("%s: %s DMA request failed, falling back to legacy\n",
> + __func__, dev->of_node ? "OF" : "ACPI");
> + }
Maybe print the error code as well?
Arnd
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 3/4] dmaengine: core: Introduce new, universal API to request a channel
2015-12-03 15:32 ` Arnd Bergmann
@ 2015-12-03 15:42 ` Peter Ujfalusi
2015-12-03 15:45 ` Arnd Bergmann
0 siblings, 1 reply; 13+ messages in thread
From: Peter Ujfalusi @ 2015-12-03 15:42 UTC (permalink / raw)
To: linux-arm-kernel
On 12/03/2015 05:32 PM, Arnd Bergmann wrote:
> On Thursday 03 December 2015 16:33:11 Peter Ujfalusi wrote:
>> +
>> +/**
>> + * dma_request_chan - try to allocate an exclusive slave channel
>> + * @dev: pointer to client device structure
>> + * @name: slave channel name
>> + *
>> + * Returns pointer to appropriate DMA channel on success or an error pointer.
>> + */
>> +struct dma_chan *dma_request_chan(struct device *dev, const char *name)
>> +{
>> + struct dma_device *d, *_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 (has_acpi_companion(dev) && !chan)
>> + chan = acpi_dma_request_slave_chan_by_name(dev, name);
>
> I just noticed that you are creating this as a new interface, rather than
> changing dma_request_slave_channel_reason() and making the old interface
> a static inline wrapper. What is the reasoning behind that?
Oh, it was in my plans. Will do it for v02
> I think if we make both interfaces do the same thing, it's easier
> to do the migration.
>
>> + if (chan) {
>> + /* Valid channel found */
>> + if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
>> + return chan;
>> +
>> + pr_warn("%s: %s DMA request failed, falling back to legacy\n",
>> + __func__, dev->of_node ? "OF" : "ACPI");
>> + }
>
> Maybe print the error code as well?
Or remove the print altogether?
In a healthy system we will either get the channel or the EPROBE_DEFER, in
case of the platforms where the DT lookup does not work we expect errors and
it is 'normal'.
I think if we fail via DT/ACPI and we fail with legacy also then the client
driver will say something about it anyways, or deal with it as it see fits.
--
P?ter
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 3/4] dmaengine: core: Introduce new, universal API to request a channel
2015-12-03 15:42 ` Peter Ujfalusi
@ 2015-12-03 15:45 ` Arnd Bergmann
0 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2015-12-03 15:45 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 03 December 2015 17:42:31 Peter Ujfalusi wrote:
> >
> >> + if (chan) {
> >> + /* Valid channel found */
> >> + if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
> >> + return chan;
> >> +
> >> + pr_warn("%s: %s DMA request failed, falling back to legacy\n",
> >> + __func__, dev->of_node ? "OF" : "ACPI");
> >> + }
> >
> > Maybe print the error code as well?
>
> Or remove the print altogether?
> In a healthy system we will either get the channel or the EPROBE_DEFER, in
> case of the platforms where the DT lookup does not work we expect errors and
> it is 'normal'.
> I think if we fail via DT/ACPI and we fail with legacy also then the client
> driver will say something about it anyways, or deal with it as it see fits.
>
Right, that works too. It took me a while to figure out that we only get
there on systems that have ACPI or DT enabled for a particular device,
but where the normal method failed, rather than also systems with traditional
board files. Without the pr_warn, I would not have needed to think about
this ;-)
Arnd
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] dmaengine: core: Introduce new, universal API to request a channel
2015-12-03 14:33 ` [PATCH 3/4] dmaengine: core: Introduce new, universal API to request a channel Peter Ujfalusi
2015-12-03 15:32 ` Arnd Bergmann
@ 2015-12-03 16:30 ` Andy Shevchenko
1 sibling, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2015-12-03 16:30 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 3, 2015 at 4:33 PM, Peter Ujfalusi <peter.ujfalusi@ti.com> 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_slave_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 const struct dma_slave_map da830_edma_map[] = {
> { "davinci-mcasp.0", "rx", EDMA_FILTER_PARAM(0, 0) },
> { "davinci-mcasp.0", "tx", EDMA_FILTER_PARAM(0, 1) },
> { "davinci-mcasp.1", "rx", EDMA_FILTER_PARAM(0, 2) },
> { "davinci-mcasp.1", "tx", EDMA_FILTER_PARAM(0, 3) },
> { "davinci-mcasp.2", "rx", EDMA_FILTER_PARAM(0, 4) },
> { "davinci-mcasp.2", "tx", EDMA_FILTER_PARAM(0, 5) },
> { "spi_davinci.0", "rx", EDMA_FILTER_PARAM(0, 14) },
> { "spi_davinci.0", "tx", EDMA_FILTER_PARAM(0, 15) },
> { "da830-mmc.0", "rx", EDMA_FILTER_PARAM(0, 16) },
> { "da830-mmc.0", "tx", EDMA_FILTER_PARAM(0, 17) },
> { "spi_davinci.1", "rx", EDMA_FILTER_PARAM(0, 18) },
> { "spi_davinci.1", "tx", EDMA_FILTER_PARAM(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.slave_map = da830_edma_map;
> da8xx_edma0_pdata.slavecnt = ARRAY_SIZE(da830_edma_map);
>
> The DMA driver then needs to configure the needed device -> filter_fn
> mapping before it registers with dma_async_device_register() :
>
> if (info->slave_map) {
> ecc->dma_slave.filter_map.map = info->slave_map;
> ecc->dma_slave.filter_map.mapcnt = info->slavecnt;
> ecc->dma_slave.filter_map.filter_fn = edma_filter_fn;
One nitpick here.
I think filter_map.filter_fn naming is duplicate.
What about
dma_device
.filter -> filter data (or .filter_data)
.map -> mappings
.map_count -> # of entries in mappings
.fn -> filter function
What do you think?
> }
>
> 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_slave_map to get the channel with the dma_get_channel() internal
> function.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] dmaengine: edma: Add support for DMA filter mapping to slave devices
2015-12-03 14:33 [PATCH 0/4] dmaengine: New 'universal' API for requesting channel Peter Ujfalusi
` (2 preceding siblings ...)
2015-12-03 14:33 ` [PATCH 3/4] dmaengine: core: Introduce new, universal API to request a channel Peter Ujfalusi
@ 2015-12-03 14:33 ` Peter Ujfalusi
2015-12-03 15:38 ` Arnd Bergmann
2015-12-03 16:32 ` [PATCH 0/4] dmaengine: New 'universal' API for requesting channel Andy Shevchenko
4 siblings, 1 reply; 13+ messages in thread
From: Peter Ujfalusi @ 2015-12-03 14:33 UTC (permalink / raw)
To: linux-arm-kernel
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 <peter.ujfalusi@ti.com>
---
drivers/dma/edma.c | 6 ++++++
include/linux/platform_data/edma.h | 7 +++++++
2 files changed, 13 insertions(+)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 0675e268d577..46b305ea0d21 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -2297,6 +2297,12 @@ static int edma_probe(struct platform_device *pdev)
edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot);
}
+ if (info->slave_map) {
+ ecc->dma_slave.filter_map.map = info->slave_map;
+ ecc->dma_slave.filter_map.mapcnt = info->slavecnt;
+ ecc->dma_slave.filter_map.filter_fn = edma_filter_fn;
+ }
+
ret = dma_async_device_register(&ecc->dma_slave);
if (ret) {
dev_err(dev, "slave ddev registration failed (%d)\n", ret);
diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h
index e2878baeb90e..105700e62ea1 100644
--- a/include/linux/platform_data/edma.h
+++ b/include/linux/platform_data/edma.h
@@ -53,12 +53,16 @@ enum dma_event_q {
#define EDMA_CTLR(i) ((i) >> 16)
#define EDMA_CHAN_SLOT(i) ((i) & 0xffff)
+#define EDMA_FILTER_PARAM(ctlr, chan) ((int[]) { EDMA_CTLR_CHAN(ctlr, chan) })
+
struct edma_rsv_info {
const s16 (*rsv_chans)[2];
const s16 (*rsv_slots)[2];
};
+struct dma_slave_map;
+
/* platform_data for EDMA driver */
struct edma_soc_info {
/*
@@ -76,6 +80,9 @@ struct edma_soc_info {
s8 (*queue_priority_mapping)[2];
const s16 (*xbar_chans)[2];
+
+ const struct dma_slave_map *slave_map;
+ int slavecnt;
};
#endif
--
2.6.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/4] dmaengine: edma: Add support for DMA filter mapping to slave devices
2015-12-03 14:33 ` [PATCH 4/4] dmaengine: edma: Add support for DMA filter mapping to slave devices Peter Ujfalusi
@ 2015-12-03 15:38 ` Arnd Bergmann
2015-12-03 15:46 ` Peter Ujfalusi
0 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2015-12-03 15:38 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 03 December 2015 16:33:12 Peter Ujfalusi wrote:
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 0675e268d577..46b305ea0d21 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -2297,6 +2297,12 @@ static int edma_probe(struct platform_device *pdev)
> edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot);
> }
>
> + if (info->slave_map) {
> + ecc->dma_slave.filter_map.map = info->slave_map;
> + ecc->dma_slave.filter_map.mapcnt = info->slavecnt;
> + ecc->dma_slave.filter_map.filter_fn = edma_filter_fn;
> + }
> +
>
Just a minor comment here: I think all three assignments can be done
unconditionally. As I mentioned before, I'd also remove 'struct dma_filter'
and put the three members in struct dma_device directly. In fact, the
filter function can go with the other function pointers for consistency.
Arnd
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 4/4] dmaengine: edma: Add support for DMA filter mapping to slave devices
2015-12-03 15:38 ` Arnd Bergmann
@ 2015-12-03 15:46 ` Peter Ujfalusi
2015-12-08 13:15 ` Peter Ujfalusi
0 siblings, 1 reply; 13+ messages in thread
From: Peter Ujfalusi @ 2015-12-03 15:46 UTC (permalink / raw)
To: linux-arm-kernel
On 12/03/2015 05:38 PM, Arnd Bergmann wrote:
> On Thursday 03 December 2015 16:33:12 Peter Ujfalusi wrote:
>> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
>> index 0675e268d577..46b305ea0d21 100644
>> --- a/drivers/dma/edma.c
>> +++ b/drivers/dma/edma.c
>> @@ -2297,6 +2297,12 @@ static int edma_probe(struct platform_device *pdev)
>> edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot);
>> }
>>
>> + if (info->slave_map) {
>> + ecc->dma_slave.filter_map.map = info->slave_map;
>> + ecc->dma_slave.filter_map.mapcnt = info->slavecnt;
>> + ecc->dma_slave.filter_map.filter_fn = edma_filter_fn;
>> + }
>> +
>>
>
> Just a minor comment here: I think all three assignments can be done
> unconditionally.
True.
> As I mentioned before, I'd also remove 'struct dma_filter'
> and put the three members in struct dma_device directly. In fact, the
> filter function can go with the other function pointers for consistency.
I just like to keep things in one place ;)
I don't have strong stand on keeping the intermediate 'struct dma_filter'
Let's hear from Vinod regarding to this
--
P?ter
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 4/4] dmaengine: edma: Add support for DMA filter mapping to slave devices
2015-12-03 15:46 ` Peter Ujfalusi
@ 2015-12-08 13:15 ` Peter Ujfalusi
0 siblings, 0 replies; 13+ messages in thread
From: Peter Ujfalusi @ 2015-12-08 13:15 UTC (permalink / raw)
To: linux-arm-kernel
On 12/03/2015 05:46 PM, Peter Ujfalusi wrote:
> On 12/03/2015 05:38 PM, Arnd Bergmann wrote:
>> On Thursday 03 December 2015 16:33:12 Peter Ujfalusi wrote:
>>> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
>>> index 0675e268d577..46b305ea0d21 100644
>>> --- a/drivers/dma/edma.c
>>> +++ b/drivers/dma/edma.c
>>> @@ -2297,6 +2297,12 @@ static int edma_probe(struct platform_device *pdev)
>>> edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot);
>>> }
>>>
>>> + if (info->slave_map) {
>>> + ecc->dma_slave.filter_map.map = info->slave_map;
>>> + ecc->dma_slave.filter_map.mapcnt = info->slavecnt;
>>> + ecc->dma_slave.filter_map.filter_fn = edma_filter_fn;
>>> + }
>>> +
>>>
>>
>> Just a minor comment here: I think all three assignments can be done
>> unconditionally.
>
> True.
>
>> As I mentioned before, I'd also remove 'struct dma_filter'
>> and put the three members in struct dma_device directly. In fact, the
>> filter function can go with the other function pointers for consistency.
>
> I just like to keep things in one place ;)
> I don't have strong stand on keeping the intermediate 'struct dma_filter'
> Let's hear from Vinod regarding to this
One remaining design issue is on how and where to place the filter related
variables/pointers:
Keep it separated as it was in the RFC and v01 series:
struct dma_slave_map {
const char *devname;
const char *slave;
void *param;
};
struct dma_filter {
dma_filter_fn fn;
int mapcnt;
const struct dma_slave_map *map;
};
struct dma_device {
...
struct dma_filter filter;
...
};
Or to have them under the dma_device directly:
struct dma_device {
...
int filter_mapcnt;
const struct dma_slave_map *filter_map;
...
dma_filter_fn filter_fn;
...
};
Vinod: what is your preference for this?
Thanks,
P?ter
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/4] dmaengine: New 'universal' API for requesting channel
2015-12-03 14:33 [PATCH 0/4] dmaengine: New 'universal' API for requesting channel Peter Ujfalusi
` (3 preceding siblings ...)
2015-12-03 14:33 ` [PATCH 4/4] dmaengine: edma: Add support for DMA filter mapping to slave devices Peter Ujfalusi
@ 2015-12-03 16:32 ` Andy Shevchenko
4 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2015-12-03 16:32 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 3, 2015 at 4:33 PM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> Hi,
> As it has been discussed in the following thread:
> http://www.gossamer-threads.com/lists/linux/kernel/2181487#2181487
>
> 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_slave_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 const struct dma_slave_map da830_edma_map[] = {
> { "davinci-mcasp.0", "rx", EDMA_FILTER_PARAM(0, 0) },
> { "davinci-mcasp.0", "tx", EDMA_FILTER_PARAM(0, 1) },
> { "davinci-mcasp.1", "rx", EDMA_FILTER_PARAM(0, 2) },
> { "davinci-mcasp.1", "tx", EDMA_FILTER_PARAM(0, 3) },
> { "davinci-mcasp.2", "rx", EDMA_FILTER_PARAM(0, 4) },
> { "davinci-mcasp.2", "tx", EDMA_FILTER_PARAM(0, 5) },
> { "spi_davinci.0", "rx", EDMA_FILTER_PARAM(0, 14) },
> { "spi_davinci.0", "tx", EDMA_FILTER_PARAM(0, 15) },
> { "da830-mmc.0", "rx", EDMA_FILTER_PARAM(0, 16) },
> { "da830-mmc.0", "tx", EDMA_FILTER_PARAM(0, 17) },
> { "spi_davinci.1", "rx", EDMA_FILTER_PARAM(0, 18) },
> { "spi_davinci.1", "tx", EDMA_FILTER_PARAM(0, 19) },
> };
>
> This information is going to be used 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.slave_map = da830_edma_map;
> da8xx_edma0_pdata.slavecnt = ARRAY_SIZE(da830_edma_map);
>
> The DMA driver then needs to configure the needed device -> filter_fn
> mapping before it registers with dma_async_device_register() :
>
> if (info->slave_map) {
> ecc->dma_slave.filter_map.map = info->slave_map;
> ecc->dma_slave.filter_map.mapcnt = info->slavecnt;
> ecc->dma_slave.filter_map.filter_fn = edma_filter_fn;
> }
>
> 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_slave_map to get the channel with the dma_get_channel() internal
> function.
For patches 1-3 with regard to comment on 3:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 13+ messages in thread