On 09/14/2012 08:32 AM, Arnd Bergmann wrote:
> On Friday 14 September 2012, Jon Hunter wrote:
>> On 09/14/2012 04:43 AM, Arnd Bergmann wrote:
>>>> +
>>>> +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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
