[PATCH V4 1/2] of: Add generic device tree DMA helpers
From: Jon Hunter <hidden>
Date: 2012-09-14 14:03:18
Also in:
linux-devicetree, linux-omap
Subsystem:
open firmware and flattened device tree, the rest · Maintainers:
Rob Herring, Saravana Kannan, Linus Torvalds
On 09/14/2012 08:32 AM, Arnd Bergmann wrote:
On Friday 14 September 2012, Jon Hunter wrote:quoted
On 09/14/2012 04:43 AM, Arnd Bergmann wrote:quoted
quoted
+ +Client drivers should specify the DMA property using a phandle to the controller +followed by DMA controller specific data. + +Required property: +- dmas: List of one or more DMA specifiers, each consisting of + - A phandle pointing to DMA controller node + - A single integer cell containing DMA controller + specific information. This typically contains a dma + request line number or a channel number, but can + contain any data that is used required for configuring + a channel. +- dma-names: Contains one identifier string for each dma specifier in + the dmas property. The specific strings that can be used + are defined in the binding of the DMA client device.I think here we need to clarify that listing the same name multiple times implies having multiple alternatives for the same channel.Ok, however, the way it works right now is that we will use the first specifier that matches the name. So if there are multiple with the same name that would imply that someone will need call the xxx_request_slave_channel() multiple times to extract these. Is that ok?I would expect a driver to only call the function once, and get something back from the dmaengine layer that works. If there are two controllers to choose from and one is busy, then it should definitely give a channel from the non-busy one. It's not much of an issue if the code doesn't handle all corner cases at first, but I would expect that the binding correctly describes how to write a device tree that will work once the code implements it correctly.
Gotcha, may be something like the following should work then ...
diff --git a/drivers/of/dma.c b/drivers/of/dma.c
index 4025f2f..de59611 100644
--- a/drivers/of/dma.c
+++ b/drivers/of/dma.c@@ -127,13 +127,15 @@ static int of_dma_find_channel(struct device_node *np, char *name, return count; for (i = 0; i < count; i++) { - of_property_read_string_index(np, "dma-names", i, &s); + if (of_property_read_string_index(np, "dma-names", i, &s)) + continue; if (strcmp(name, s)) continue; - return of_parse_phandle_with_args(np, "dmas", "#dma-cells", - i, dma_spec); + if (!of_parse_phandle_with_args(np, "dmas", "#dma-cells", i, + dma_spec)) + return 0; } return -ENODEV;
@@ -159,33 +161,34 @@ struct dma_chan *of_dma_request_slave_channel(struct device_node *np, return NULL; } - r = of_dma_find_channel(np, name, &dma_spec); - - if (r) { - pr_err("%s: can't find DMA channel\n", np->full_name); - return NULL; - } + do { + r = of_dma_find_channel(np, name, &dma_spec); + if (r) { + pr_err("%s: can't find DMA channel\n", np->full_name); + return NULL; + } + + ofdma = of_dma_find_controller(dma_spec.np); + if (!ofdma) { + pr_debug("%s: can't find DMA controller %s\n", + np->full_name, dma_spec.np->full_name); + continue; + } - ofdma = of_dma_find_controller(dma_spec.np); - if (!ofdma) { - pr_err("%s: can't find DMA controller %s\n", np->full_name, - dma_spec.np->full_name); - return NULL; - } + if (dma_spec.args_count != ofdma->of_dma_nbcells) { + pr_debug("%s: wrong #dma-cells for %s\n", np->full_name, + dma_spec.np->full_name); + continue; + } - if (dma_spec.args_count != ofdma->of_dma_nbcells) { - pr_err("%s: wrong #dma-cells for %s\n", np->full_name, - dma_spec.np->full_name); - return NULL; - } + chan = ofdma->of_dma_xlate(&dma_spec, ofdma); - chan = ofdma->of_dma_xlate(&dma_spec, ofdma); + of_node_put(dma_spec.np); - of_node_put(dma_spec.np); + } while (!chan); return chan; } -EXPORT_SYMBOL_GPL(of_dma_request_slave_channel);
Cheers Jon