of: Add generic device tree DMA helpers
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Add some basic helpers to retrieve a DMA controller device_node and the DMA request line number.
For legacy reason another API will export the DMA request number into a Linux resource of type IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.
--- Documentation/devicetree/bindings/dma/dma.txt | 44 +++++++++ drivers/of/Kconfig | 5 + drivers/of/Makefile | 1 + drivers/of/dma.c | 130 +++++++++++++++++++++++++ include/linux/of_dma.h | 49 +++++++++ 5 files changed, 229 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/dma.txt create mode 100644 drivers/of/dma.c create mode 100644 include/linux/of_dma.h
Documentation/devicetree/bindings/dma/dma.txt new file +44 -0
@@ -0,0 +1,44 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller + +Required properties: + - dma-controller: Mark the device as a DMA controller + - #dma-cells: Number of cell for each DMA line, must be one. + + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg = <0x48000000 0x1000>; + interrupts = <12>; + dma-controller; + #dma-cells = <1>; + }; + + + +* DMA client + +Client drivers should specify the DMA request numbers using a phandle to +the controller + the DMA request number on that controller. + +Required properties: + - dma-request: List of pair phandle + dma-request per line + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property. + + +Example: + + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };
drivers/of/Kconfig +5 -0
@@ -90,4 +90,9 @@ config OF_PCI_IRQ help OpenFirmware PCI IRQ routing helpers +config OF_DMA + def_bool y + help + Device Tree DMA routing helpers + endmenu # OF
drivers/of/Makefile +1 -0
@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SELFTEST) += selftest.o obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o +obj-$(CONFIG_OF_DMA) += dma.o
drivers/of/dma.c new file +130 -0
130 lines not on this page · raw message
include/linux/of_dma.h new file +49 -0
49 lines not on this page · raw message
Signature · 1 line
1.7.0.4
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Cousson, Benoit wrote at Friday, January 27, 2012 10:29 AM:
Quoted 6 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.
This binding looks reasonable to me; it's pretty much exactly what I was expecting the custom Tegra I2S binding might evolve into. A couple comments inline...
Documentation/devicetree/bindings/dma/dma.txt line 34
Quoted 11 lines
Documentation/devicetree/bindings/dma/dma.txt new file +9
-0+* DMA client + +Client drivers should specify the DMA request numbers using a phandle to +the controller + the DMA request number on that controller. + +Required properties: + - dma-request: List of pair phandle + dma-request per line + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property.
I think property dma-request-names should be optional; some drives will simply request by index and hence the property won't be needed. Same as the common clock binding.
Quoted 7 lines
drivers/of/dma.c +5
-0+int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np) +{ +int ret = -EINVAL ; +struct of_phandle_args dma_spec;
drivers/of/dma.c line 44
Quoted 10 lines
drivers/of/dma.c +10
-0+ + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index, &dma_spec); +if (ret) { +pr_debug ("%s: can't parse dma property\n" , __func__); + goto err0; + } + +if (dma_spec.args_count >0 ) + ret = dma_spec.args[0 ];
Doesn't that force #dma-cells > 0? What about a DMA controller that only supports a single DMA request, in which case you might want #dma-cells==0?
Having an of_xlate for the controller would be useful here in general:
Is a single int really enough here?
Some DMA controllers have N channels that can be used for arbitrary things. You may need to specify the DMA request once when allocating the channel, or maybe individual for each transfer if it can vary. Other controllers may need a special channel ID for each type of request. There are probably more cases I haven't thought of.
So in general, I think this API should return some kind of cookie that's meaningful to the DMA controller, and this should be provided back to the DMA controller with every API that might need it; channel allocation, DMA transfer request, etc.
I suppose a controller may be able to represent that cookie in a single int, but perhaps not, e.g. if a binding needs #dma-cells=4 for some reason. Perhaps returning a void* here would be better, coupled with of_put_dma_request() to allow the controller to free it if required?
That'd allow individual controller bindings to specify e.g. HW FIFO width, wrap, FIFO levels for HW flow control, ...
(I vaguely recall that dmaengine addresses some of this already, but unfortunately I'm not very familiar with it yet)
include/linux/of_dma.h
Quoted 3 lines
include/linux/of_dma.h +1
-0+#ifdef CONFIG_OF_GPIO
CONFIG_OF_DMA
Signature · 1 line
nvpublic
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Quoted 11 lines · Stephen Warren
Cousson, Benoit wrote at Friday, January 27, 2012 10:29 AM:
Quoted 6 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.This binding looks reasonable to me; it's pretty much exactly what I was expecting the custom Tegra I2S binding might evolve into. A couple comments inline...
Well, that does not surprise me since this is your Tegra I2S binding that made me think that it should be time to define that generic DMA binding we've been waiting for a while :-)
Quoted 15 lines
Documentation/devicetree/bindings/dma/dma.txt line 34
Quoted 11 lines
Documentation/devicetree/bindings/dma/dma.txt new file +9
-0+* DMA client + +Client drivers should specify the DMA request numbers using a phandle to +the controller + the DMA request number on that controller. + +Required properties: + - dma-request: List of pair phandle + dma-request per line + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property.I think property dma-request-names should be optional; some drives will simply request by index and hence the property won't be needed. Same as the common clock binding.
Yes, indeed, and it is optional in the code. I wrongly added that in the "required" paragraph. I'll fix that.
Quoted 12 lines
Quoted 12 lines
drivers/of/dma.c +10
-0+int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np) +{ +int ret = -EINVAL ; +struct of_phandle_args dma_spec; + + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index,&dma_spec); +if (ret) { +pr_debug ("%s: can't parse dma property\n" , __func__);
Quoted 8 lines
drivers/of/dma.c line 44
Quoted 5 lines
drivers/of/dma.c +5
-0+ goto err0; + } + +if (dma_spec.args_count>0 ) + ret = dma_spec.args[0 ];Doesn't that force #dma-cells> 0? What about a DMA controller that only supports a single DMA request, in which case you might want #dma-cells==0?
OK, if such DMA controller exist, it will make sense.
Quoted 9 lines
Having an
of_xlatefor the controller would be useful here in general:Is a single int really enough here?
Some DMA controllers have N channels that can be used for arbitrary things. You may need to specify the DMA request once when allocating the channel, or maybe individual for each transfer if it can vary. Other controllers may need a special channel ID for each type of request. There are probably more cases I haven't thought of.
In my naive view of the DMA controller the channel is something that will be handled and allocated by the DMA controller upon driver request. So this is not something that is necessarily HW related, and thus should not be described in DT.
But, maybe I'm missing your point.
Quoted 4 lines
So in general, I think this API should return some kind of cookie that's meaningful to the DMA controller, and this should be provided back to the DMA controller with every API that might need it; channel allocation, DMA transfer request, etc.
Forgive my ignorance, but the only DMA controller I've been using for a while is the OMAP2+ SDMA, hence this simplistic API that fit perfectly the OMAP need.
So I guess that having an xlate function like GPIO and IRQ will make sense if we have to handle these kind of variants.
Quoted 9 lines
I suppose a controller may be able to represent that cookie in a single int, but perhaps not, e.g. if a binding needs #dma-cells=4 for some reason. Perhaps returning a void* here would be better, coupled with
of_put_dma_request()to allow the controller to free it if required?That'd allow individual controller bindings to specify e.g. HW FIFO width, wrap, FIFO levels for HW flow control, ...
(I vaguely recall that dmaengine addresses some of this already, but unfortunately I'm not very familiar with it yet)
Me neither, but do we have to handle all this parameters in DT? I guess that most of the configuration will still be under driver responsibility for the channel. The DMA controller itself can clearly defined a bunch of parameters to expose its capabilities, but again so far I do not have any clue about all these fancy DMA controller variants. Some flexibility will clearly be needed, but how far should we go?
The goal of that RFC was to start gathering some feedback to understand the requirements for the DMA bindings, so at least I have a couple of new ones now :-).
Quoted 5 lines
include/linux/of_dma.h
Quoted 3 lines
include/linux/of_dma.h +1
-0+#ifdef CONFIG_OF_GPIO
CONFIG_OF_DMA
Oops, some leftover from the of_gpio.h copy-paste.
Thanks for the good feedback,
Benoit
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Quoted 35 lines · Benoit
Quoted 13 lines · Stephen Warren
Cousson, Benoit wrote at Friday, January 27, 2012 10:29 AM:
Quoted 12 lines
drivers/of/dma.c +10
-0+int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np) +{ +int ret = -EINVAL ; +struct of_phandle_args dma_spec; + + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index,&dma_spec); +if (ret) { +pr_debug ("%s: can't parse dma property\n" , __func__);Quoted 8 lines
drivers/of/dma.c line 44
Quoted 5 lines
drivers/of/dma.c +5
-0+ goto err0; + } + +if (dma_spec.args_count>0 ) + ret = dma_spec.args[0 ];Doesn't that force #dma-cells> 0? What about a DMA controller that only supports a single DMA request, in which case you might want #dma-cells==0?
OK, if such DMA controller exist, it will make sense.
Quoted 9 lines
Having an
of_xlatefor the controller would be useful here in general:Is a single int really enough here?
Some DMA controllers have N channels that can be used for arbitrary things. You may need to specify the DMA request once when allocating the channel, or maybe individual for each transfer if it can vary. Other controllers may need a special channel ID for each type of request. There are probably more cases I haven't thought of.
Quoted 5 lines
In my naive view of the DMA controller the channel is something that will be handled and allocated by the DMA controller upon driver request. So this is not something that is necessarily HW related, and thus should not be described in DT.
But, maybe I'm missing your point.
The binding for each dma controller gets to decide the format and size of the dma spec (just like for irq and gpio specs). The data in the spec is opaque to the DMA users, and only has meaning after code with knowledge of the specific dma controller binding .xlates it into something useful.
So for OMAP, the binding may trivially map, but other DMA controllers may require flags or other configuration encoded into the DMA spec. The generic binding must take that into account.
Quoted 10 lines
Quoted 9 lines
I suppose a controller may be able to represent that cookie in a single int, but perhaps not, e.g. if a binding needs #dma-cells=4 for some reason. Perhaps returning a void* here would be better, coupled with
of_put_dma_request()to allow the controller to free it if required?That'd allow individual controller bindings to specify e.g. HW FIFO width, wrap, FIFO levels for HW flow control, ...
(I vaguely recall that dmaengine addresses some of this already, but unfortunately I'm not very familiar with it yet)
Quoted 7 lines
Me neither, but do we have to handle all this parameters in DT? I guess that most of the configuration will still be under driver responsibility for the channel. The DMA controller itself can clearly defined a bunch of parameters to expose its capabilities, but again so far I do not have any clue about all these fancy DMA controller variants. Some flexibility will clearly be needed, but how far should we go?
It goes as far as the binding author deems is appropriate. For your case it sounds like 1 cell is sufficient, so go with that. Just don't restrict other binding authors from encoding more (or less) data. Especially when it is trivial to allow #dma-cells != 1.
g.
RFC PATCH 1/2 of: Add generic device tree DMA helpers
In reply to Cousson, BenoitQuoted 10 lines · Benoit
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.Signed-off-by Benoit Cousson <[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>
Hey Benoit.
Good start to this. Comments below.
Quoted 17 lines
--- Documentation/devicetree/bindings/dma/dma.txt | 44 +++++++++ drivers/of/Kconfig | 5 + drivers/of/Makefile | 1 + drivers/of/dma.c | 130 +++++++++++++++++++++++++ include/linux/of_dma.h | 49 +++++++++ 5 files changed, 229 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/dma.txt create mode 100644 drivers/of/dma.c create mode 100644 include/linux/of_dma.hDocumentation/devicetree/bindings/dma/dma.txt new file +0
-0
Documentation/devicetree/bindings/dma/dma.txt line 10
Quoted 10 lines
Documentation/devicetree/bindings/dma/dma.txt new file +10
-0@@ -0,0 +1,44 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller + +Required properties: + - dma-controller: Mark the device as a DMA controller
I know gpio and interrupt controllers do this, but I'm not convinced it is necessary. The compatible binding for the device will implicitly state that the device is a dma controller and adopts the generic dma binding.
Documentation/devicetree/bindings/dma/dma.txt line 11
Quoted 1 line
Documentation/devicetree/bindings/dma/dma.txt new file +1
-0+ - #dma-cells: Number of cell for each DMA line, must be one.
Must be one? Then why bother with the property?
Documentation/devicetree/bindings/dma/dma.txt line 21
Quoted 10 lines
Documentation/devicetree/bindings/dma/dma.txt new file +10
-0+ + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg = <0x48000000 0x1000>; + interrupts = <12>; + dma-controller; + #dma-cells = <1>;
Nit: inconsistent indentation (tabs/spaces)... and it looks like you've got your tabs set to 4 characters. All rightstanding coders know that the only holy and blessed tab stop is 8 characters, so remind me to ridicule that 4 character nonsense out of you the next time we're in the same room. >:-}
Documentation/devicetree/bindings/dma/dma.txt line 34
Quoted 13 lines
Documentation/devicetree/bindings/dma/dma.txt new file +13
-0+ }; + + + +* DMA client + +Client drivers should specify the DMA request numbers using a phandle to +the controller + the DMA request number on that controller. + +Required properties: + - dma-request: List of pair phandle + dma-request per line + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property.
As Stephen mentioned, the -names should not be required.
Quoted 84 lines
Documentation/devicetree/bindings/dma/dma.txt new file +10
-0+ + +Example: + + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/Kconfig +5
-0@@ -90,4 +90,9 @@ config OF_PCI_IRQ help OpenFirmware PCI IRQ routing helpers +config OF_DMA + def_bool y + help + Device Tree DMA routing helpers + endmenu # OFdrivers/of/Makefile +1
-0@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SELFTEST) += selftest.o obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o +obj-$(CONFIG_OF_DMA) += dma.odrivers/of/dma.c new file +45
-0@@ -0,0 +1,130 @@ +/* +* Device tree helpers for DMA request / controller +* +* Based on of_gpio.c +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_dma.h> + +/** +* of_get_dma_request() - Get a DMA request number and dma-controller node +* @np: device node to get DMA request from +* @propname: property name containing DMA specifier(s) +* @index: index of the DMA request +* @ctrl_np: a device_node pointer to fill in +* +* Returns DMA number along to the dma controller node, or one of the errno +* value on the error condition. If @ctrl_np is not NULL the function also +* fills in the DMA controller device_node pointer. +*/ +int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np) +{ +int ret = -EINVAL ; +struct of_phandle_args dma_spec; + + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index, &dma_spec); +if (ret) { +pr_debug ("%s: can't parse dma property\n" , __func__); + goto err0; + } + +if (dma_spec.args_count >0 ) + ret = dma_spec.args[0 ]; +
drivers/of/dma.c line 55
Quoted 10 lines
drivers/of/dma.c new file +10
-0+if (ctrl_np) + *ctrl_np = dma_spec.np; +else +of_node_put (dma_spec.np); + +err0: +pr_debug ("%s exited with status %d\n" , __func__, ret); +return ret; +} +EXPORT_SYMBOL (of_get_dma_request);
This makes the assumption that dma specifiers will only ever be 1 cell. I think to be generally useful, the full dma specifier needs to be either handed to the dma controller to get a cookie or passed back to the caller in its entirety.
I think this function is fine if you want to use it internally in OMAP-only device drivers, but doing that way probably ties the OMAP drivers to directly access the OMAP dma controllers API instead of through dmaengine.
To do it generically will require some form of .xlate function like with irqs so that the dma controller driver can perform its own interpretation of the dma spec.
Quoted 24 lines
drivers/of/dma.c new file +24
-0+ +/** +* of_dma_count - Count DMA requests for a device +* @np: device node to count DMAs for +* +* The function returns the count of DMA requests specified for a node. +* +* Note that the empty DMA specifiers counts too. For example, +* +* dma-request = <0 +* &sdma 1 +* 0 +* &sdma 3>; +* +* defines four DMAs (so this function will return 4), two of which +* are not specified. +*/ +unsigned int of_dma_count (struct device_node *np) +{ +unsigned int cnt =0 ; + + do { + int ret; +
drivers/of/dma.c line 89
Quoted 10 lines
drivers/of/dma.c new file +10
-0+ ret =of_parse_phandle_with_args (np,"dma-request" , +"#dma-cells" , cnt,NULL ); +/* A hole in the dma-request = <> counts anyway. */ +if (ret <0 && ret != -EEXIST ) +break ; + }while (++cnt); + +return cnt; +} +EXPORT_SYMBOL (of_dma_count);
This is identical to what needs to be done to count gpios and clks. Create a generic of_count_phandle_with_args() please.
However, after writing my comments on the function below, this function may not actually be needed either. of_gpio_count isn't used by any core code, it is only currently used by spi drivers that want to know how many chip selects they have. Since I don't want to resolve DMA references at device registration time, I wonder if there is any need for an of_dma_count() function.
Quoted 31 lines
drivers/of/dma.c new file +31
-0+ +/** +* of_dma_to_resource - Decode a node's DMA and return it as a resource +* @dev: pointer to device tree node +* @index: zero-based index of the DMA request +* @r: pointer to resource structure to return result into. +* +* Using a resource to export a DMA request number to a driver should +* be used only for legacy purpose and on system when only one DMA controller +* is present. +* The proper and only scalable way is to use the native of_get_dma_request API +* in order retrieve both the DMA controller device node and the DMA request +* line for that controller. +*/ +int of_dma_to_resource (struct device_node *dev,int index,struct resource *r) +{ +const char *name =NULL ; +int dma; + +if (!r) +return -EINVAL ; + + dma =of_get_dma_request (dev, index,NULL ); +if (dma <0 ) +return dma; + +/* +* Get optional "dma-request-names" property to add a name +* to the resource. +*/ + of_property_read_string_index(dev, "dma-request-names", index,
drivers/of/dma.c line 130
Quoted 10 lines
drivers/of/dma.c new file +10
-0+ &name); + + r->start = dma; + r->end = dma; + r->flags =IORESOURCE_DMA ; + r->name = name ? name : dev->full_name; + +return dma; +} +EXPORT_SYMBOL_GPL (of_dma_to_resource);
How do you see this function being used? I ask because I don't want to add it to the DT device registration code (of_platform_populate()). I actually want to reduce the amount of work being done at device registration time and push resolving irqs out to the device drivers. The reason for this is so that drivers can resolve irqs supplied by other device drivers once I've got deferred probe merged.
This isn't currently possible because a lot of drivers parse the resources table directly. Those users first need to be migrated to use the platform_get_irq*() APIs.
DMAs have the same issue, so it is important that device drivers resolve the dma specifier at .probe() time so that it can use dma channels supplied by a dma device driver.
I suspect having a common of_parse_named_phandle_with_args() will useful when needing to resolve named dma resources from device drivers.
g.
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Hi Grant,
Quoted 40 lines · Grant Likely
Quoted 10 lines · Benoit
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.Signed-off-by Benoit Cousson<[email protected]>Cc Grant Likely<[email protected]>Cc Rob Herring<[email protected]>Hey Benoit.
Good start to this. Comments below.
Quoted 24 lines
--- Documentation/devicetree/bindings/dma/dma.txt | 44 +++++++++ drivers/of/Kconfig | 5 + drivers/of/Makefile | 1 + drivers/of/dma.c | 130 +++++++++++++++++++++++++ include/linux/of_dma.h | 49 +++++++++ 5 files changed, 229 insertions(+), 0 deletions(-)create mode 100644
Documentation/devicetree/bindings/dma/dma.txtcreate mode 100644drivers/of/dma.c
create mode 100644include/linux/of_dma.hDocumentation/devicetree/bindings/dma/dma.txt new file +7
-0@@ -0,0 +1,44 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller
Quoted 8 lines
Documentation/devicetree/bindings/dma/dma.txt line 10
Quoted 3 lines
Documentation/devicetree/bindings/dma/dma.txt new file +3
-0+ +Required properties: + - dma-controller: Mark the device as a DMA controllerI know gpio and interrupt controllers do this, but I'm not convinced it is necessary. The compatible binding for the device will implicitly state that the device is a dma controller and adopts the generic dma binding.
That's fine by me, I was just thinking of a mechanism similar to the IRQ controller to iterate over the dma controllers at early boot. But I guess such approach should become useless with the deferred probe mechanism.
Quoted 3 lines
Documentation/devicetree/bindings/dma/dma.txt line 11
Quoted 1 line
Documentation/devicetree/bindings/dma/dma.txt new file +1
-0+ - #dma-cells: Number of cell for each DMA line, must be one.Must be one? Then why bother with the property?
Must be higher that one was the idea, but it looks like from Stephen's comment that some simple DMA controller with only one line might even exist.
Quoted 17 lines
Documentation/devicetree/bindings/dma/dma.txt
Quoted 11 lines
Documentation/devicetree/bindings/dma/dma.txt new file +10
-0+ + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg =<0x48000000 0x1000>; + interrupts =<12>; + dma-controller; + #dma-cells =<1>;Nit: inconsistent indentation (tabs/spaces)... and it looks like you've got your tabs set to 4 characters. All rightstanding coders know that the only holy and blessed tab stop is 8 characters, so remind me to ridicule that 4 character nonsense out of you the next time we're in the same room.>:-}
Outch, I got caught!
I might have argued that this is my Python setting... but in fact the PEP8 recommend the usage of 4 spaces... so I do not have any valid excuse :-)
Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/dma/dma.txt new file +7
-0+ }; + + + +* DMA client + +Client drivers should specify the DMA request numbers using a phandle to
Quoted 8 lines
Documentation/devicetree/bindings/dma/dma.txt line 34
Quoted 6 lines
Documentation/devicetree/bindings/dma/dma.txt new file +6
-0+the controller + the DMA request number on that controller. + +Required properties: + - dma-request: List of pair phandle + dma-request per line + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property.As Stephen mentioned, the -names should not be required.
Yep, the code is already doing that, the documentation was wrong.
BTW, should it be dma-request or dma-requests?
Quoted 101 lines
Quoted 85 lines
Documentation/devicetree/bindings/dma/dma.txt new file +10
-0+ + +Example: + + i2c1: i2c at 1 { + ... + dma-request =<&sdma 2&sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/Kconfig +5
-0@@ -90,4 +90,9 @@ config OF_PCI_IRQ help OpenFirmware PCI IRQ routing helpers +config OF_DMA + def_bool y + help + Device Tree DMA routing helpers + endmenu # OFdrivers/of/Makefile +1
-0@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SELFTEST) += selftest.o obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o +obj-$(CONFIG_OF_DMA) += dma.odrivers/of/dma.c new file +45
-0@@ -0,0 +1,130 @@ +/* +* Device tree helpers for DMA request / controller +* +* Based on of_gpio.c +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_dma.h> + +/** +* of_get_dma_request() - Get a DMA request number and dma-controller node +* @np: device node to get DMA request from +* @propname: property name containing DMA specifier(s) +* @index: index of the DMA request +* @ctrl_np: a device_node pointer to fill in +* +* Returns DMA number along to the dma controller node, or one of the errno +* value on the error condition. If @ctrl_np is not NULL the function also +* fills in the DMA controller device_node pointer. +*/ +int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np) +{ +int ret = -EINVAL ; +struct of_phandle_args dma_spec; + + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index,&dma_spec); +if (ret) { +pr_debug ("%s: can't parse dma property\n" , __func__); + goto err0; + } + +if (dma_spec.args_count>0 ) + ret = dma_spec.args[0 ]; +drivers/of/dma.c line 55
Quoted 10 lines
drivers/of/dma.c new file +10
-0+if (ctrl_np) + *ctrl_np = dma_spec.np; +else +of_node_put (dma_spec.np); + +err0: +pr_debug ("%s exited with status %d\n" , __func__, ret); +return ret; +} +EXPORT_SYMBOL (of_get_dma_request);This makes the assumption that dma specifiers will only ever be 1 cell. I think to be generally useful, the full dma specifier needs to be either handed to the dma controller to get a cookie or passed back to the caller in its entirety.
Quoted 4 lines
I think this function is fine if you want to use it internally in OMAP-only device drivers, but doing that way probably ties the OMAP drivers to directly access the OMAP dma controllers API instead of through dmaengine.
Well it was meant to be generic, but knowing only the OMAP case, I did the minimal support to make OMAP, and the two custom DMA bindings I found, working.
Quoted 3 lines
To do it generically will require some form of .xlate function like with irqs so that the dma controller driver can perform its own interpretation of the dma spec.
OK, I'll make that more generic.
Quoted 38 lines
Quoted 24 lines
drivers/of/dma.c new file +24
-0+ +/** +* of_dma_count - Count DMA requests for a device +* @np: device node to count DMAs for +* +* The function returns the count of DMA requests specified for a node. +* +* Note that the empty DMA specifiers counts too. For example, +* +* dma-request =<0 +*&sdma 1 +* 0 +*&sdma 3>; +* +* defines four DMAs (so this function will return 4), two of which +* are not specified. +*/ +unsigned int of_dma_count (struct device_node *np) +{ +unsigned int cnt =0 ; + + do { + int ret; +drivers/of/dma.c line 89
Quoted 10 lines
drivers/of/dma.c new file +10
-0+ ret =of_parse_phandle_with_args (np,"dma-request" , +"#dma-cells" , cnt,NULL ); +/* A hole in the dma-request =<> counts anyway. */ +if (ret<0 && ret != -EEXIST ) +break ; + }while (++cnt); + +return cnt; +} +EXPORT_SYMBOL (of_dma_count);This is identical to what needs to be done to count gpios and clks. Create a generic
of_count_phandle_with_args()please.
Quoted 6 lines
However, after writing my comments on the function below, this function may not actually be needed either.
of_gpio_countisn't used by any core code, it is only currently used by spi drivers that want to know how many chip selects they have. Since I don't want to resolve DMA references at device registration time, I wonder if there is any need for anof_dma_count()function.
Today, I'm using that to build the resource array that will contain the IORESOURCE_DMA resources for legacy OMAP drivers.
Quoted 36 lines
Quoted 36 lines
drivers/of/dma.c new file +36
-0+ +/** +* of_dma_to_resource - Decode a node's DMA and return it as a resource +* @dev: pointer to device tree node +* @index: zero-based index of the DMA request +* @r: pointer to resource structure to return result into. +* +* Using a resource to export a DMA request number to a driver should +* be used only for legacy purpose and on system when only one DMA controller +* is present. +* The proper and only scalable way is to use the native of_get_dma_request API +* in order retrieve both the DMA controller device node and the DMA request +* line for that controller. +*/ +int of_dma_to_resource (struct device_node *dev,int index,struct resource *r) +{ +const char *name =NULL ; +int dma; + +if (!r) +return -EINVAL ; + + dma =of_get_dma_request (dev, index,NULL ); +if (dma<0 ) +return dma; + +/* +* Get optional "dma-request-names" property to add a name +* to the resource. +*/ +of_property_read_string_index (dev,"dma-request-names" , index, + &name); + + r->start = dma; + r->end = dma; + r->flags =IORESOURCE_DMA ;
Quoted 8 lines
drivers/of/dma.c line 130
Quoted 5 lines
drivers/of/dma.c new file +5
-0+ r->name = name ? name : dev->full_name; + +return dma; +} +EXPORT_SYMBOL_GPL (of_dma_to_resource);How do you see this function being used? I ask because I don't want to add it to the DT device registration code (
of_platform_populate()).
Yep, that was the plan :-)
Quoted 4 lines
I actually want to reduce the amount of work being done at device registration time and push resolving irqs out to the device drivers. The reason for this is so that drivers can resolve irqs supplied by other device drivers once I've got deferred probe merged.
That make senses, but that will break all the drivers that are expecting IRQ and DMA resources to be already there at probe time. These drivers are still relying on the good old platform_get_resource() API. That will add some extra effort to the drivers migration to DT:-(
Quoted 3 lines
This isn't currently possible because a lot of drivers parse the resources table directly. Those users first need to be migrated to use the platform_get_irq*() APIs.
But even in that case, the device should still have the resources populated before probe. I'm not sure I fully understand what you mean here.
Quoted 7 lines
DMAs have the same issue, so it is important that device drivers resolve the dma specifier at .probe() time so that it can use dma channels supplied by a dma device driver.
I suspect having a common
of_parse_named_phandle_with_args()will useful when needing to resolve named dma resources from device drivers.
So it looks like you really have a plan to deprecate all the platform_get_resource APIs usage from driver? At least for DMA and IRQ?
Thanks for the feedback,
Benoit
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Quoted 16 lines · Benoit
Hi Grant,
Quoted 5 lines · Grant Likely
Quoted 1 line · Benoit
+EXPORT_SYMBOL_GPL(of_dma_to_resource);
How do you see this function being used? I ask because I don't want to add it to the DT device registration code (
of_platform_populate()).Yep, that was the plan :-)
Quoted 4 lines
I actually want to reduce the amount of work being done at device registration time and push resolving irqs out to the device drivers. The reason for this is so that drivers can resolve irqs supplied by other device drivers once I've got deferred probe merged.
Quoted 5 lines
That make senses, but that will break all the drivers that are expecting IRQ and DMA resources to be already there at probe time. These drivers are still relying on the good old
platform_get_resource()API.
That will add some extra effort to the drivers migration to DT:-(
They will be broken anyway because there isn't any way for the core code to decode DMA properties. There isn't a consistent way that drivers use DMA resources or any complete data about what dma engine those resources refer to. Unlike irqs, there is no global DMA channel number space available to driver, which makes it really hard to generically populate DMA channels into the resource table.
It really is much better to resolve it at .probe() time where the driver can get unambiguous information. Plus, as I mentioned, it plays better with deferred probe.
Quoted 7 lines
Quoted 3 lines
This isn't currently possible because a lot of drivers parse the resources table directly. Those users first need to be migrated to use the platform_get_irq*() APIs.
But even in that case, the device should still have the resources populated before probe. I'm not sure I fully understand what you mean here.
Ah, but once it is in an API, hooks can be implemented in the platform_get_*() APIs to resolve relevant things at call time instead of at device registration time.
Quoted 11 lines
Quoted 7 lines
DMAs have the same issue, so it is important that device drivers resolve the dma specifier at .probe() time so that it can use dma channels supplied by a dma device driver.
I suspect having a common
of_parse_named_phandle_with_args()will useful when needing to resolve named dma resources from device drivers.So it looks like you really have a plan to deprecate all the
platform_get_resourceAPIs usage from driver? At least for DMA and IRQ?
I definitely would like to be rid of (or refactor) the DMA get APIs from drivers because I don't think they provide enough information about how DMA channels are hooked up (as described above).
However, I actually prefer the platform_get_ API for irqs over accessing the resource table directly. I can hook into it if/when I change irqs to be resolved at .probe time.
g.
RFC PATCH 1/2 of: Add generic device tree DMA helpers
In reply to Grant LikelyQuoted 4 lines · Grant Likely
This makes the assumption that dma specifiers will only ever be 1 cell. I think to be generally useful, the full dma specifier needs to be either handed to the dma controller to get a cookie or passed back to the caller in its entirety.
More to the point, who says that the DMA specifier is even an integer? When people are using DMA engines, it (probably) isn't an integer at all. Several platforms I know of use strings for this.
Some platforms can even select between two or more DMA engines for handling the same peripheral - I believe Samsung do this depending on their individual workloads.
However, the opaque DMA engine API for requesting a channel doesn't lend itself well to DT, as the match data and match function are entirely left to the individual DMA engine driver and/or platform itself.
As far as creating another linear number space for DMA stuff, I'd really suggest against that - you're going to need some additional code in place to manage that numberspace. If you at least use a two- paid cookie, eg 'dma controller phandle + request signal' then that makes all the stuff we're starting to see with the IRQ subsystem, IRQ domains etc become completely unnecessary.
I guess what I'm saying is ignore the flat number space, and go straight to some kind of 'dma domains' solution from the start.
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Hi Russell,
Quoted 26 lines · Russell King - ARM Linux
Quoted 4 lines · Grant Likely
This makes the assumption that dma specifiers will only ever be 1 cell. I think to be generally useful, the full dma specifier needs to be either handed to the dma controller to get a cookie or passed back to the caller in its entirety.
More to the point, who says that the DMA specifier is even an integer? When people are using DMA engines, it (probably) isn't an integer at all. Several platforms I know of use strings for this.
Some platforms can even select between two or more DMA engines for handling the same peripheral - I believe Samsung do this depending on their individual workloads.
However, the opaque DMA engine API for requesting a channel doesn't lend itself well to DT, as the match data and match function are entirely left to the individual DMA engine driver and/or platform itself.
As far as creating another linear number space for DMA stuff, I'd really suggest against that - you're going to need some additional code in place to manage that numberspace. If you at least use a two- paid cookie, eg 'dma controller phandle + request signal' then that makes all the stuff we're starting to see with the IRQ subsystem, IRQ domains etc become completely unnecessary.
Quoted 2 lines
I guess what I'm saying is ignore the flat number space, and go straight to some kind of 'dma domains' solution from the start.
Fully agree, and this is exactly the idea of this DMA binding: First argument is always a DMA controller phandle and then you can add 0, 1 or more cells to define extra specifiers dependent of the DMA controller driver expectation. The one cell Grant was referring was just the extra specifier that is needed for a simple DMA engine like the SDMA we have inside OMAP. But the whole idea is to have a flexible enough mechanism to allow any kind of specifier.
No more global linear number space like for IRQ!
Regards,
Benoit
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Quoted 40 lines · Benoit
Hi Russell,
Quoted 26 lines · Russell King - ARM Linux
Quoted 4 lines · Grant Likely
This makes the assumption that dma specifiers will only ever be 1 cell. I think to be generally useful, the full dma specifier needs to be either handed to the dma controller to get a cookie or passed back to the caller in its entirety.
More to the point, who says that the DMA specifier is even an integer? When people are using DMA engines, it (probably) isn't an integer at all. Several platforms I know of use strings for this.
Some platforms can even select between two or more DMA engines for handling the same peripheral - I believe Samsung do this depending on their individual workloads.
However, the opaque DMA engine API for requesting a channel doesn't lend itself well to DT, as the match data and match function are entirely left to the individual DMA engine driver and/or platform itself.
As far as creating another linear number space for DMA stuff, I'd really suggest against that - you're going to need some additional code in place to manage that numberspace. If you at least use a two- paid cookie, eg 'dma controller phandle + request signal' then that makes all the stuff we're starting to see with the IRQ subsystem, IRQ domains etc become completely unnecessary.
Quoted 2 lines
I guess what I'm saying is ignore the flat number space, and go straight to some kind of 'dma domains' solution from the start.
Fully agree, and this is exactly the idea of this DMA binding: First argument is always a DMA controller phandle and then you can add 0, 1 or more cells to define extra specifiers dependent of the DMA controller driver expectation. The one cell Grant was referring was just the extra specifier that is needed for a simple DMA engine like the SDMA we have inside OMAP. But the whole idea is to have a flexible enough mechanism to allow any kind of specifier.
Quoted 1 line
No more global linear number space like for IRQ!
How does this work when you're stuffing a number into a struct resource as a plain DMA number? That looks very much like a linear number space, as you don't have a way to associate that number with anything else.
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Quoted 41 lines · Russell King - ARM Linux
Quoted 40 lines · Benoit
Hi Russell,
Quoted 26 lines · Russell King - ARM Linux
Quoted 4 lines · Grant Likely
This makes the assumption that dma specifiers will only ever be 1 cell. I think to be generally useful, the full dma specifier needs to be either handed to the dma controller to get a cookie or passed back to the caller in its entirety.
More to the point, who says that the DMA specifier is even an integer? When people are using DMA engines, it (probably) isn't an integer at all. Several platforms I know of use strings for this.
Some platforms can even select between two or more DMA engines for handling the same peripheral - I believe Samsung do this depending on their individual workloads.
However, the opaque DMA engine API for requesting a channel doesn't lend itself well to DT, as the match data and match function are entirely left to the individual DMA engine driver and/or platform itself.
As far as creating another linear number space for DMA stuff, I'd really suggest against that - you're going to need some additional code in place to manage that numberspace. If you at least use a two- paid cookie, eg 'dma controller phandle + request signal' then that makes all the stuff we're starting to see with the IRQ subsystem, IRQ domains etc become completely unnecessary.
Quoted 2 lines
I guess what I'm saying is ignore the flat number space, and go straight to some kind of 'dma domains' solution from the start.
Fully agree, and this is exactly the idea of this DMA binding: First argument is always a DMA controller phandle and then you can add 0, 1 or more cells to define extra specifiers dependent of the DMA controller driver expectation. The one cell Grant was referring was just the extra specifier that is needed for a simple DMA engine like the SDMA we have inside OMAP. But the whole idea is to have a flexible enough mechanism to allow any kind of specifier.
Quoted 5 lines
Quoted 1 line
No more global linear number space like for IRQ!
How does this work when you're stuffing a number into a struct resource as a plain DMA number? That looks very much like a linear number space, as you don't have a way to associate that number with anything else.
As explained in the kernel doc header and in the changelog, that one is for legacy purpose only.
+/**
+ * of_dma_to_resource - Decode a node's DMA and return it as a resource
+ * @dev: pointer to device tree node
+ * @index: zero-based index of the DMA request
+ * @r: pointer to resource structure to return result into.
+ *
+ * Using a resource to export a DMA request number to a driver should
+ * be used only for legacy purpose and on system when only one DMA controller
+ * is present.
+ * The proper and only scalable way is to use the native of_get_dma_request API
+ * in order retrieve both the DMA controller device node and the DMA request
+ * line for that controller.
+ */
There is indeed no way to put information about DMA controller and DMA line in the resource structure except if we start hacking the start and end attributes. But I'm not sure anyone will want to do that.
The only way to take benefit of the extra information DT can provide is to use the of_get_dma_request API.
Regards,
Benoit
RFC PATCH 1/2 of: Add generic device tree DMA helpers
In reply to Cousson, BenoitOn 01/27/2012 06:29 PM, Cousson, Benoit :
Quoted 6 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.
Hi,
I followed that discussion and I like very much the biding that Benoit is proposing. It will help me a lot with my current work on Atmel DMA controller.
If I understand correctly, some rework is needed before it can be integrated in a stable git tree (I mean before we can base our work on top of it). So, in the meantime, what should I do to help and make things go forward? to be quite frank, I would be interested to have a working DMA enabled device soon ;-)
Do you think that 3.4 is out of reach?
Best regards,
Quoted 278 lines
Signed-off-by Benoit Cousson <[email protected]>Cc Grant Likely <[email protected]>Cc Rob Herring <[email protected]>--- Documentation/devicetree/bindings/dma/dma.txt | 44 +++++++++ drivers/of/Kconfig | 5 + drivers/of/Makefile | 1 + drivers/of/dma.c | 130 +++++++++++++++++++++++++ include/linux/of_dma.h | 49 +++++++++ 5 files changed, 229 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/dma.txt create mode 100644 drivers/of/dma.c create mode 100644 include/linux/of_dma.hDocumentation/devicetree/bindings/dma/dma.txt new file +44
-0@@ -0,0 +1,44 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller + +Required properties: + - dma-controller: Mark the device as a DMA controller + - #dma-cells: Number of cell for each DMA line, must be one. + + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg = <0x48000000 0x1000>; + interrupts = <12>; + dma-controller; + #dma-cells = <1>; + }; + + + +* DMA client + +Client drivers should specify the DMA request numbers using a phandle to +the controller + the DMA request number on that controller. + +Required properties: + - dma-request: List of pair phandle + dma-request per line + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property. + + +Example: + + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/Kconfig +5
-0@@ -90,4 +90,9 @@ config OF_PCI_IRQ help OpenFirmware PCI IRQ routing helpers +config OF_DMA + def_bool y + help + Device Tree DMA routing helpers + endmenu # OFdrivers/of/Makefile +1
-0@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SELFTEST) += selftest.o obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o +obj-$(CONFIG_OF_DMA) += dma.odrivers/of/dma.c new file +130
-0@@ -0,0 +1,130 @@ +/* +* Device tree helpers for DMA request / controller +* +* Based on of_gpio.c +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_dma.h> + +/** +* of_get_dma_request() - Get a DMA request number and dma-controller node +* @np: device node to get DMA request from +* @propname: property name containing DMA specifier(s) +* @index: index of the DMA request +* @ctrl_np: a device_node pointer to fill in +* +* Returns DMA number along to the dma controller node, or one of the errno +* value on the error condition. If @ctrl_np is not NULL the function also +* fills in the DMA controller device_node pointer. +*/ +int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np) +{ +int ret = -EINVAL ; +struct of_phandle_args dma_spec; + + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index, &dma_spec); +if (ret) { +pr_debug ("%s: can't parse dma property\n" , __func__); + goto err0; + } + +if (dma_spec.args_count >0 ) + ret = dma_spec.args[0 ]; + +if (ctrl_np) + *ctrl_np = dma_spec.np; +else +of_node_put (dma_spec.np); + +err0: +pr_debug ("%s exited with status %d\n" , __func__, ret); +return ret; +} +EXPORT_SYMBOL (of_get_dma_request); + +/** +* of_dma_count - Count DMA requests for a device +* @np: device node to count DMAs for +* +* The function returns the count of DMA requests specified for a node. +* +* Note that the empty DMA specifiers counts too. For example, +* +* dma-request = <0 +* &sdma 1 +* 0 +* &sdma 3>; +* +* defines four DMAs (so this function will return 4), two of which +* are not specified. +*/ +unsigned int of_dma_count (struct device_node *np) +{ +unsigned int cnt =0 ; + +do { +int ret; + + ret =of_parse_phandle_with_args (np,"dma-request" , +"#dma-cells" , cnt,NULL ); +/* A hole in the dma-request = <> counts anyway. */ +if (ret <0 && ret != -EEXIST ) +break ; + }while (++cnt); + +return cnt; +} +EXPORT_SYMBOL (of_dma_count); + +/** +* of_dma_to_resource - Decode a node's DMA and return it as a resource +* @dev: pointer to device tree node +* @index: zero-based index of the DMA request +* @r: pointer to resource structure to return result into. +* +* Using a resource to export a DMA request number to a driver should +* be used only for legacy purpose and on system when only one DMA controller +* is present. +* The proper and only scalable way is to use the native of_get_dma_request API +* in order retrieve both the DMA controller device node and the DMA request +* line for that controller. +*/ +int of_dma_to_resource (struct device_node *dev,int index,struct resource *r) +{ +const char *name =NULL ; +int dma; + +if (!r) +return -EINVAL ; + + dma =of_get_dma_request (dev, index,NULL ); +if (dma <0 ) +return dma; + +/* +* Get optional "dma-request-names" property to add a name +* to the resource. +*/ +of_property_read_string_index (dev,"dma-request-names" , index, + &name); + + r->start = dma; + r->end = dma; + r->flags =IORESOURCE_DMA ; + r->name = name ? name : dev->full_name; + +return dma; +} +EXPORT_SYMBOL_GPL (of_dma_to_resource);include/linux/of_dma.h new file +49
-0@@ -0,0 +1,49 @@ +/* +* OF helpers for DMA request / controller +* +* Based on of_gpio.h +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#ifndef __LINUX_OF_DMA_H +#define __LINUX_OF_DMA_H + +#include <linux/of.h> + +struct device_node ; + +#ifdef CONFIG_OF_GPIO + +extern int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np); +extern unsigned int of_dma_count (struct device_node *np); +extern int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); + +#else /* CONFIG_OF_DMA */ + +static int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np); +{ +return -ENOSYS ; +} + +static inline unsigned int of_dma_count (struct device_node *np) +{ +return 0 ; +} + +static int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); +{ +return -ENOSYS ; +} + +#endif /* CONFIG_OF_DMA */ + +#endif /* __LINUX_OF_DMA_H */
Signature · 1 line
Nicolas Ferre
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Salut Nico,
Quoted 14 lines · Nicolas Ferre
On 01/27/2012 06:29 PM, Cousson, Benoit :
Quoted 6 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.Hi,
I followed that discussion and I like very much the biding that Benoit is proposing. It will help me a lot with my current work on Atmel DMA controller.
Quoted 5 lines
If I understand correctly, some rework is needed before it can be integrated in a stable git tree (I mean before we can base our work on top of it). So, in the meantime, what should I do to help and make things go forward? to be quite frank, I would be interested to have a working DMA enabled device soon ;-)
Me too, but unfortunately, I was busy trying to add irq_domain and fixing issues with SPARSE_IRQ on OMAP :-(
Quoted 1 line
Do you think that 3.4 is out of reach?
Maybe not, from the comments, it looks like we should add a .xlate callback to allow any custom parsing of the DMA nodes attributes.
I'll be more than happy, if you can finalize that patch :-)
Thanks,
Benoit
Quoted 283 lines
Best regards,
Quoted 278 lines
Signed-off-by Benoit Cousson<[email protected]>Cc Grant Likely<[email protected]>Cc Rob Herring<[email protected]>--- Documentation/devicetree/bindings/dma/dma.txt | 44 +++++++++ drivers/of/Kconfig | 5 + drivers/of/Makefile | 1 + drivers/of/dma.c | 130 +++++++++++++++++++++++++ include/linux/of_dma.h | 49 +++++++++ 5 files changed, 229 insertions(+), 0 deletions(-)create mode 100644
Documentation/devicetree/bindings/dma/dma.txtcreate mode 100644drivers/of/dma.c
create mode 100644include/linux/of_dma.hDocumentation/devicetree/bindings/dma/dma.txt new file +44
-0@@ -0,0 +1,44 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller + +Required properties: + - dma-controller: Mark the device as a DMA controller + - #dma-cells: Number of cell for each DMA line, must be one. + + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg =<0x48000000 0x1000>; + interrupts =<12>; + dma-controller; + #dma-cells =<1>; + }; + + + +* DMA client + +Client drivers should specify the DMA request numbers using a phandle to +the controller + the DMA request number on that controller. + +Required properties: + - dma-request: List of pair phandle + dma-request per line + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property. + + +Example: + + i2c1: i2c at 1 { + ... + dma-request =<&sdma 2&sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/Kconfig +5
-0@@ -90,4 +90,9 @@ config OF_PCI_IRQ help OpenFirmware PCI IRQ routing helpers +config OF_DMA + def_bool y + help + Device Tree DMA routing helpers + endmenu # OFdrivers/of/Makefile +1
-0@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SELFTEST) += selftest.o obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o +obj-$(CONFIG_OF_DMA) += dma.odrivers/of/dma.c new file +130
-0@@ -0,0 +1,130 @@ +/* +* Device tree helpers for DMA request / controller +* +* Based on of_gpio.c +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_dma.h> + +/** +* of_get_dma_request() - Get a DMA request number and dma-controller node +* @np: device node to get DMA request from +* @propname: property name containing DMA specifier(s) +* @index: index of the DMA request +* @ctrl_np: a device_node pointer to fill in +* +* Returns DMA number along to the dma controller node, or one of the errno +* value on the error condition. If @ctrl_np is not NULL the function also +* fills in the DMA controller device_node pointer. +*/ +int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np) +{ +int ret = -EINVAL ; +struct of_phandle_args dma_spec; + + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index,&dma_spec); +if (ret) { +pr_debug ("%s: can't parse dma property\n" , __func__); + goto err0; + } + +if (dma_spec.args_count>0 ) + ret = dma_spec.args[0 ]; + +if (ctrl_np) + *ctrl_np = dma_spec.np; +else +of_node_put (dma_spec.np); + +err0: +pr_debug ("%s exited with status %d\n" , __func__, ret); +return ret; +} +EXPORT_SYMBOL (of_get_dma_request); + +/** +* of_dma_count - Count DMA requests for a device +* @np: device node to count DMAs for +* +* The function returns the count of DMA requests specified for a node. +* +* Note that the empty DMA specifiers counts too. For example, +* +* dma-request =<0 +*&sdma 1 +* 0 +*&sdma 3>; +* +* defines four DMAs (so this function will return 4), two of which +* are not specified. +*/ +unsigned int of_dma_count (struct device_node *np) +{ +unsigned int cnt =0 ; + +do { +int ret; + + ret =of_parse_phandle_with_args (np,"dma-request" , +"#dma-cells" , cnt,NULL ); +/* A hole in the dma-request =<> counts anyway. */ +if (ret<0 && ret != -EEXIST ) +break ; + }while (++cnt); + +return cnt; +} +EXPORT_SYMBOL (of_dma_count); + +/** +* of_dma_to_resource - Decode a node's DMA and return it as a resource +* @dev: pointer to device tree node +* @index: zero-based index of the DMA request +* @r: pointer to resource structure to return result into. +* +* Using a resource to export a DMA request number to a driver should +* be used only for legacy purpose and on system when only one DMA controller +* is present. +* The proper and only scalable way is to use the native of_get_dma_request API +* in order retrieve both the DMA controller device node and the DMA request +* line for that controller. +*/ +int of_dma_to_resource (struct device_node *dev,int index,struct resource *r) +{ +const char *name =NULL ; +int dma; + +if (!r) +return -EINVAL ; + + dma =of_get_dma_request (dev, index,NULL ); +if (dma<0 ) +return dma; + +/* +* Get optional "dma-request-names" property to add a name +* to the resource. +*/ +of_property_read_string_index (dev,"dma-request-names" , index, + &name); + + r->start = dma; + r->end = dma; + r->flags =IORESOURCE_DMA ; + r->name = name ? name : dev->full_name; + +return dma; +} +EXPORT_SYMBOL_GPL (of_dma_to_resource);include/linux/of_dma.h new file +49
-0@@ -0,0 +1,49 @@ +/* +* OF helpers for DMA request / controller +* +* Based on of_gpio.h +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#ifndef __LINUX_OF_DMA_H +#define __LINUX_OF_DMA_H + +#include <linux/of.h> + +struct device_node ; + +#ifdef CONFIG_OF_GPIO + +extern int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np); +extern unsigned int of_dma_count (struct device_node *np); +extern int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); + +#else /* CONFIG_OF_DMA */ + +static int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np); +{ +return -ENOSYS ; +} + +static inline unsigned int of_dma_count (struct device_node *np) +{ +return 0 ; +} + +static int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); +{ +return -ENOSYS ; +} + +#endif /* CONFIG_OF_DMA */ + +#endif /* __LINUX_OF_DMA_H */
RFC PATCH 1/2 of: Add generic device tree DMA helpers
On 02/23/2012 11:03 AM, Cousson, Benoit :
Quoted 1 line
Salut Nico,
Coucou Benoit ;-)
Quoted 15 lines · Nicolas Ferre
Quoted 14 lines
On 01/27/2012 06:29 PM, Cousson, Benoit :
Quoted 6 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.Hi,
I followed that discussion and I like very much the biding that Benoit is proposing. It will help me a lot with my current work on Atmel DMA controller.
Quoted 8 lines
Quoted 5 lines
If I understand correctly, some rework is needed before it can be integrated in a stable git tree (I mean before we can base our work on top of it). So, in the meantime, what should I do to help and make things go forward? to be quite frank, I would be interested to have a working DMA enabled device soon ;-)
Me too, but unfortunately, I was busy trying to add
irq_domainand fixing issues withSPARSE_IRQon OMAP :-(
Been there, loved that ;-)
Quoted 6 lines
Quoted 1 line
Do you think that 3.4 is out of reach?
Maybe not, from the comments, it looks like we should add a .xlate callback to allow any custom parsing of the DMA nodes attributes.
I'll be more than happy, if you can finalize that patch :-)
I will try to figure out what I can understand from the irq mechanism of .xlate and try to see if I can implement it on top of your patch.
I will keep you informed...
Bye,
Quoted 297 lines
Quoted 295 lines
Best regards,
Quoted 291 lines
Signed-off-by Benoit Cousson<[email protected]>Cc Grant Likely<[email protected]>Cc Rob Herring<[email protected]>--- Documentation/devicetree/bindings/dma/dma.txt | 44 +++++++++ drivers/of/Kconfig | 5 + drivers/of/Makefile | 1 + drivers/of/dma.c | 130 +++++++++++++++++++++++++ include/linux/of_dma.h | 49 +++++++++ 5 files changed, 229 insertions(+), 0 deletions(-)create mode 100644
Documentation/devicetree/bindings/dma/dma.txtcreate mode 100644drivers/of/dma.c
create mode 100644include/linux/of_dma.hDocumentation/devicetree/bindings/dma/dma.txt new file +28
-0@@ -0,0 +1,44 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller + +Required properties: + - dma-controller: Mark the device as a DMA controller + - #dma-cells: Number of cell for each DMA line, must be one. + + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg =<0x48000000 0x1000>; + interrupts =<12>; + dma-controller; + #dma-cells =<1>; + }; + + + +* DMA client + +Client drivers should specify the DMA request numbers using aphandle to
Documentation/devicetree/bindings/dma/dma.txt new file +5
-0+the controller + the DMA request number on that controller. + +Required properties: + - dma-request: List of pair phandle + dma-request per line + - dma-request-names: list of strings in the same order as thedma-request
Documentation/devicetree/bindings/dma/dma.txt new file +11
-0+ in the dma-request property. + + +Example: + + i2c1: i2c at 1 { + ... + dma-request =<&sdma 2&sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/Kconfig +5
-0@@ -90,4 +90,9 @@ config OF_PCI_IRQ help OpenFirmware PCI IRQ routing helpers +config OF_DMA + def_bool y + help + Device Tree DMA routing helpers + endmenu # OFdrivers/of/Makefile +1
-0@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SELFTEST) += selftest.o obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o +obj-$(CONFIG_OF_DMA) += dma.odrivers/of/dma.c new file +6
-0@@ -0,0 +1,130 @@ +/* +* Device tree helpers for DMA request / controller +* +* Based on of_gpio.c +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */drivers/of/dma.c new file +9
-0+ +#include <linux/device.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_dma.h> + +/** +* of_get_dma_request() - Get a DMA request number and dma-controller node
+ * @np: device node to get DMA request from
+ * @propname: property name containing DMA specifier(s)
+ * @index: index of the DMA request
+ * @ctrl_np: adevice_nodepointer to fill in
+ *
+ * Returns DMA number along to the dma controller node, or one of the errno
+ * value on the error condition. If @ctrl_np is not NULL the function also
+ * fills in the DMA controllerdevice_nodepointer.
+ */drivers/of/dma.c new file +69
-0+int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np) +{ +int ret = -EINVAL ; +struct of_phandle_args dma_spec; + + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index,&dma_spec); +if (ret) { +pr_debug ("%s: can't parse dma property\n" , __func__); + goto err0; + } + +if (dma_spec.args_count>0 ) + ret = dma_spec.args[0 ]; + +if (ctrl_np) + *ctrl_np = dma_spec.np; +else +of_node_put (dma_spec.np); + +err0: +pr_debug ("%s exited with status %d\n" , __func__, ret); +return ret; +} +EXPORT_SYMBOL (of_get_dma_request); + +/** +* of_dma_count - Count DMA requests for a device +* @np: device node to count DMAs for +* +* The function returns the count of DMA requests specified for a node. +* +* Note that the empty DMA specifiers counts too. For example, +* +* dma-request =<0 +*&sdma 1 +* 0 +*&sdma 3>; +* +* defines four DMAs (so this function will return 4), two of which +* are not specified. +*/ +unsigned int of_dma_count (struct device_node *np) +{ +unsigned int cnt =0 ; + +do { +int ret; + + ret =of_parse_phandle_with_args (np,"dma-request" , +"#dma-cells" , cnt,NULL ); +/* A hole in the dma-request =<> counts anyway. */ +if (ret<0 && ret != -EEXIST ) +break ; + }while (++cnt); + +return cnt; +} +EXPORT_SYMBOL (of_dma_count); + +/** +* of_dma_to_resource - Decode a node's DMA and return it as a resource +* @dev: pointer to device tree node +* @index: zero-based index of the DMA request +* @r: pointer to resource structure to return result into. +* +* Using a resource to export a DMA request number to a driver should +* be used only for legacy purpose and on system when only one DMA controller
+ * is present.
+ * The proper and only scalable way is to use the nativeof_get_dma_requestAPI
+ * in order retrieve both the DMA controller device node and the DMA request
+ * line for that controller.
+ */
+intof_dma_to_resource(struct device_node *dev, int index, struct resource *r)drivers/of/dma.c new file +26
-0+{ +const char *name =NULL ; +int dma; + +if (!r) +return -EINVAL ; + + dma =of_get_dma_request (dev, index,NULL ); +if (dma<0 ) +return dma; + +/* +* Get optional "dma-request-names" property to add a name +* to the resource. +*/ +of_property_read_string_index (dev,"dma-request-names" , index, + &name); + + r->start = dma; + r->end = dma; + r->flags =IORESOURCE_DMA ; + r->name = name ? name : dev->full_name; + +return dma; +} +EXPORT_SYMBOL_GPL (of_dma_to_resource);include/linux/of_dma.h new file +6
-0@@ -0,0 +1,49 @@ +/* +* OF helpers for DMA request / controller +* +* Based on of_gpio.h +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */include/linux/of_dma.h new file +38
-0+ +#ifndef __LINUX_OF_DMA_H +#define __LINUX_OF_DMA_H + +#include <linux/of.h> + +struct device_node ; + +#ifdef CONFIG_OF_GPIO + +extern int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np); +extern unsigned int of_dma_count (struct device_node *np); +extern int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); + +#else /* CONFIG_OF_DMA */ + +static int of_get_dma_request (struct device_node *np,int index, +struct device_node **ctrl_np); +{ +return -ENOSYS ; +} + +static inline unsigned int of_dma_count (struct device_node *np) +{ +return 0 ; +} + +static int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); +{ +return -ENOSYS ; +} + +#endif /* CONFIG_OF_DMA */ + +#endif /* __LINUX_OF_DMA_H */
Signature · 1 line
Nicolas Ferre
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Quoted 33 lines · Nicolas Ferre
On 02/23/2012 11:03 AM, Cousson, Benoit :
Quoted 1 line
Salut Nico,
Coucou Benoit ;-)
Quoted 15 lines · Nicolas Ferre
Quoted 14 lines
On 01/27/2012 06:29 PM, Cousson, Benoit :
Quoted 6 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.Hi,
I followed that discussion and I like very much the biding that Benoit is proposing. It will help me a lot with my current work on Atmel DMA controller.
Quoted 8 lines
Quoted 5 lines
If I understand correctly, some rework is needed before it can be integrated in a stable git tree (I mean before we can base our work on top of it). So, in the meantime, what should I do to help and make things go forward? to be quite frank, I would be interested to have a working DMA enabled device soon ;-)
Me too, but unfortunately, I was busy trying to add
irq_domainand fixing issues withSPARSE_IRQon OMAP :-(Been there, loved that ;-)
Quoted 2 lines
Quoted 1 line
Do you think that 3.4 is out of reach?
Quoted 7 lines
Quoted 4 lines
Maybe not, from the comments, it looks like we should add a .xlate callback to allow any custom parsing of the DMA nodes attributes.
I'll be more than happy, if you can finalize that patch :-)
I will try to figure out what I can understand from the irq mechanism of .xlate and try to see if I can implement it on top of your patch.
In fact that dma code is a big copy/paste of the of/gpio one and gpio was already managing .xlate function.
I removed it because I thought it was useless for the DMA :-)
You might just have to copy the original code...
Quoted 1 line
I will keep you informed...
Thanks for the help,
Benoit
RFC PATCH 1/2 of: Add generic device tree DMA helpers
On 02/23/2012 04:57 PM, Cousson, Benoit :
Quoted 39 lines · Nicolas Ferre
Quoted 38 lines
On 02/23/2012 11:03 AM, Cousson, Benoit :
Quoted 1 line
Salut Nico,
Coucou Benoit ;-)
Quoted 15 lines · Nicolas Ferre
Quoted 14 lines
On 01/27/2012 06:29 PM, Cousson, Benoit :
Quoted 6 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.Hi,
I followed that discussion and I like very much the biding that Benoit is proposing. It will help me a lot with my current work on Atmel DMA controller.
Quoted 8 lines
Quoted 5 lines
If I understand correctly, some rework is needed before it can be integrated in a stable git tree (I mean before we can base our work on top of it). So, in the meantime, what should I do to help and make things go forward? to be quite frank, I would be interested to have a working DMA enabled device soon ;-)
Me too, but unfortunately, I was busy trying to add
irq_domainand fixing issues withSPARSE_IRQon OMAP :-(Been there, loved that ;-)
Quoted 6 lines
Quoted 1 line
Do you think that 3.4 is out of reach?
Maybe not, from the comments, it looks like we should add a .xlate callback to allow any custom parsing of the DMA nodes attributes.
I'll be more than happy, if you can finalize that patch :-)
Quoted 8 lines
Quoted 2 lines
I will try to figure out what I can understand from the irq mechanism of .xlate and try to see if I can implement it on top of your patch.
In fact that dma code is a big copy/paste of the of/gpio one and gpio was already managing .xlate function.
I removed it because I thought it was useless for the DMA :-)You might just have to copy the original code...
The thing is that with gpio, we can rely on the gpio_chip structure for having a common storage location of such .xlate callbacks. In our DMA case, I guess that we should not cling to dmaengine infrastructure because not all of us are using it right now.
So, maybe we should provide some simple "xlate" helpers like the irqdomain ones. But I fear that a "callback" directly called from the of_get_dma_request() function is not possible (the gpio code model).
On the other hand, the "cookie" based infrastructure seems a little overkill to me. It seems that, in this case, we must establish a relation between the DMA controller code and the device tree DMA helpers. Please correct me if I am wrong.
Best regards,
Signature · 1 line
Nicolas Ferre
RFC PATCH 1/2 of: Add generic device tree DMA helpers
Quoted 47 lines · Nicolas Ferre
On 02/23/2012 04:57 PM, Cousson, Benoit :
Quoted 46 lines · Nicolas Ferre
Quoted 33 lines
On 02/23/2012 11:03 AM, Cousson, Benoit :
Quoted 1 line
Salut Nico,
Coucou Benoit ;-)
Quoted 15 lines · Nicolas Ferre
Quoted 14 lines
On 01/27/2012 06:29 PM, Cousson, Benoit :
Quoted 6 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.Hi,
I followed that discussion and I like very much the biding that Benoit is proposing. It will help me a lot with my current work on Atmel DMA controller.
Quoted 8 lines
Quoted 5 lines
If I understand correctly, some rework is needed before it can be integrated in a stable git tree (I mean before we can base our work on top of it). So, in the meantime, what should I do to help and make things go forward? to be quite frank, I would be interested to have a working DMA enabled device soon ;-)
Me too, but unfortunately, I was busy trying to add
irq_domainand fixing issues withSPARSE_IRQon OMAP :-(Been there, loved that ;-)
Quoted 2 lines
Quoted 1 line
Do you think that 3.4 is out of reach?
Quoted 7 lines
Quoted 4 lines
Maybe not, from the comments, it looks like we should add a .xlate callback to allow any custom parsing of the DMA nodes attributes.
I'll be more than happy, if you can finalize that patch :-)
I will try to figure out what I can understand from the irq mechanism of .xlate and try to see if I can implement it on top of your patch.
In fact that dma code is a big copy/paste of the of/gpio one and gpio was already managing .xlate function.
I removed it because I thought it was useless for the DMA :-)
Quoted 6 lines
Quoted 1 line
You might just have to copy the original code...
The thing is that with gpio, we can rely on the
gpio_chipstructure for having a common storage location of such .xlate callbacks. In our DMA case, I guess that we should not cling to dmaengine infrastructure because not all of us are using it right now.
Yep, that's the main issue compared to GPIO or IRQ.
Quoted 3 lines
So, maybe we should provide some simple "xlate" helpers like the irqdomain ones. But I fear that a "callback" directly called from the
of_get_dma_request()function is not possible (the gpio code model).
I think a well that we cannot much but a simple very abstract callback since we do not have any formal DMA representation?
I'm not sure to understand you concern?
Quoted 4 lines
On the other hand, the "cookie" based infrastructure seems a little overkill to me. It seems that, in this case, we must establish a relation between the DMA controller code and the device tree DMA helpers. Please correct me if I am wrong.
Yes, and in our case the DMA controller cannot be specified with anything else than a Linux device for the moment.
Regards,
Benoit
RFC PATCH 1/2 of: Add generic device tree DMA helpers
On 02/27/2012 03:22 PM, Cousson, Benoit :
Quoted 60 lines · Nicolas Ferre
Quoted 50 lines
On 02/23/2012 04:57 PM, Cousson, Benoit :
Quoted 49 lines · Nicolas Ferre
Quoted 35 lines
On 02/23/2012 11:03 AM, Cousson, Benoit :
Quoted 1 line
Salut Nico,
Coucou Benoit ;-)
Quoted 23 lines · Nicolas Ferre
Quoted 15 lines
On 01/27/2012 06:29 PM, Cousson, Benoit :
Quoted 6 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request line number.For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This API is usable only on system with an unique DMA controller.Hi,
I followed that discussion and I like very much the biding that Benoit
is proposing. It will help me a lot with my current work on Atmel DMA controller.Quoted 6 lines
If I understand correctly, some rework is needed before it can be integrated in a stable git tree (I mean before we can base our work on
top of it). So, in the meantime, what should I do to help and make things go forward? to be quite frank, I would be interested to have a working DMA enabled device soon ;-)Quoted 2 lines
Me too, but unfortunately, I was busy trying to add
irq_domainand fixing issues withSPARSE_IRQon OMAP :-(Been there, loved that ;-)
Quoted 2 lines
Quoted 1 line
Do you think that 3.4 is out of reach?
Quoted 8 lines
Quoted 4 lines
Maybe not, from the comments, it looks like we should add a .xlate callback to allow any custom parsing of the DMA nodes attributes.
I'll be more than happy, if you can finalize that patch :-)
I will try to figure out what I can understand from the irq mechanism of
.xlate and try to see if I can implement it on top of your patch.In fact that dma code is a big copy/paste of the of/gpio one and gpio was already managing .xlate function.
I removed it because I thought it was useless for the DMA :-)Quoted 6 lines
Quoted 1 line
You might just have to copy the original code...
The thing is that with gpio, we can rely on the
gpio_chipstructure for having a common storage location of such .xlate callbacks. In our DMA case, I guess that we should not cling to dmaengine infrastructure because not all of us are using it right now.Yep, that's the main issue compared to GPIO or IRQ.
Quoted 8 lines
Quoted 3 lines
So, maybe we should provide some simple "xlate" helpers like the irqdomain ones. But I fear that a "callback" directly called from the
of_get_dma_request()function is not possible (the gpio code model).I think a well that we cannot much but a simple very abstract callback since we do not have any formal DMA representation?
I'm not sure to understand you concern?
Well, in fact I cannot find a place where to store a "xlate" callback:
- it have to be provided by the DMA controller (the one pointed out by the phandle) and not the caller of the of_get_dma_request() funtion
- it has to be generic enough to match the dmaengine/non-dmaengine cases (so it cannot be stored in the dmaengine "struct dma_device").
So I guess that the very basic idea of returning the full DMA specifier (struct of_phandle_args) is the best... My only concern is that the "helper" becomes very short and empty in this case...
Bye,
Signature · 1 line
Nicolas Ferre
RFC PATCH 1/1 of: DMA helpers: manage generic requests specification
In reply to Cousson, BenoitBy making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of an of_dma_xlate() function that will be implemented by the DMA controller can carry any type of "dma-request" argument.
The DMA client will search its associated DMA controller in the list and call the registered of_dam_xlate() function to retrieve the request values.
One simple xlate function is provided for the "single number" type of request biding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
---
Hi all,
Here are my thoughts about the DMA helpers for device tree. This patch goes on top of Benoit's ones. My goal was to keep this separated from any DMA infrastructure (dmaengine not needed, nor any other DMA implementation).
It is to keep the ball rolling, so do not hesitate to comment.
Best regards,
Documentation/devicetree/bindings/dma/dma.txt | 29 +++-- drivers/of/dma.c | 161 ++++++++++++++++++++++--- include/linux/of_dma.h | 33 +++++- 3 files changed, 186 insertions(+), 37 deletions(-)
Documentation/devicetree/bindings/dma/dma.txt +15 -14
@@ -6,9 +6,8 @@ DMA request line that goes from an IP to a DMA controller. * DMA controller -Required properties: - - dma-controller: Mark the device as a DMA controller - - #dma-cells: Number of cell for each DMA line, must be one. +Required property: + - #dma-cells: Number of cells for each DMA line. Example:
@@ -17,7 +16,6 @@ Example: compatible = "ti,sdma-omap4" reg = <0x48000000 0x1000>; interrupts = <12>; - dma-controller; #dma-cells = <1>; };
@@ -25,20 +23,23 @@ Example: * DMA client -Client drivers should specify the DMA request numbers using a phandle to -the controller + the DMA request number on that controller. +Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. -Required properties: - - dma-request: List of pair phandle + dma-request per line +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: - dma-request-names: list of strings in the same order as the dma-request in the dma-request property. Example: - i2c1: i2c at 1 { - ... - dma-request = <&sdma 2 &sdma 3>; - dma-request-names = "tx", "rx"; - ... - }; + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };
drivers/of/dma.c +142 -19
202 lines not on this page · raw message
include/linux/of_dma.h +29 -4
@@ -17,18 +17,38 @@struct device_node ; -#ifdef CONFIG_OF_GPIO +#ifdef CONFIG_OF_DMA +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +}; + +extern int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)); +extern void of_dma_controller_free (struct device_node *np);extern int of_get_dma_request (struct device_node *np,int index, -struct device_node **ctrl_np); +void *out_data);extern unsigned int of_dma_count (struct device_node *np);extern int of_dma_to_resource (struct device_node *dev,int index,struct resource *r); +extern int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *req_nbr);#else /* CONFIG_OF_DMA */ -static int of_get_dma_request (struct device_node *np,int index, -struct device_node **ctrl_np); +static int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +return -ENOSYS ; +} + +static void of_dma_controller_free (struct device_node *np) {} + +extern int of_get_dma_request (struct device_node *np,int index, +void *out_data) {return -ENOSYS ; }
@@ -44,6 +64,11 @@ static int of_dma_to_resource(struct device_node *dev, int index,return -ENOSYS ; } +static int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *req_nbr) +{ +return -ENOSYS ; +}#endif /* CONFIG_OF_DMA */ #endif /* __LINUX_OF_DMA_H */
Signature · 1 line
1.7.9
RFC PATCH 1/1 of: DMA helpers: manage generic requests specification
Nicolas Ferre wrote at Wednesday, February 29, 2012 7:54 AM:
Quoted 10 lines
By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.
Quoted 5 lines
One simple xlate function is provided for the "single number" type of request biding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
At a high level this seems along the right lines.
The big issue I see is the lack of type-safety in of_get_dma_request()'s out_data pointer: The DMA xlate function will assume it points at some particular type, and the client is responsible for allocating that correct type. However, it's "user"-supplied device tree data that hooks the two together, and there could easily be mismatches, thus causing the xlate function to trash memory.
Could each DMA controller (or type of out_data) have some enum in the kernel that the client passes in, and the DMA engine validates matches its expectations, to prevent this kind of thing?
enum DMA_PARAMS_TYPE {
FOO_DMA_PARAMS,
BAR_DMA_PARAMS,
...
};struct foo_dma_params { ... };
struct foo_dma_params params;
of_get_dma_request(np, 0, ¶ms, FOO_DMA_PARAMS);
and inside xlate():
if (params_type != FOO_DMA_PARAMS)
return -EINVAL;
For the dmaengine case, I assume there'd be a single DMA_PARAMS_TYPE value for the dmaengine specifier type, and the dmaengine core would take care of making sure everything matched somehow.
Signature · 1 line
nvpublic
RFC PATCH 1/1 of: DMA helpers: manage generic requests specification
Quoted 26 lines · Stephen Warren
Nicolas Ferre wrote at Wednesday, February 29, 2012 7:54 AM:
Quoted 10 lines
By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.Quoted 5 lines
One simple xlate function is provided for the "single number" type of request biding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
At a high level this seems along the right lines.
The big issue I see is the lack of type-safety in of_get_dma_request()'s
out_datapointer: The DMA xlate function will assume it points at some particular type, and the client is responsible for allocating that correct type. However, it's "user"-supplied device tree data that hooks the two together, and there could easily be mismatches, thus causing the xlate function to trash memory.
Quoted 3 lines
Could each DMA controller (or type of
out_data) have some enum in the kernel that the client passes in, and the DMA engine validates matches its expectations, to prevent this kind of thing?
Beside u32 and string, do we know what kind of parameters we can expect to retrieve?
Because we can potentially reduce the scope to these two types only.
Quoted 12 lines
enum DMA_PARAMS_TYPE { FOO_DMA_PARAMS, BAR_DMA_PARAMS, ... };struct
foo_dma_params{ ... };struct
foo_dma_paramsparams;
of_get_dma_request(np, 0,¶ms, FOO_DMA_PARAMS);
Quoted 8 lines
and inside
xlate():if (
params_type!=FOO_DMA_PARAMS)return -EINVAL;For the dmaengine case, I assume there'd be a single
DMA_PARAMS_TYPEvalue for the dmaengine specifier type, and the dmaengine core would take care of making sure everything matched somehow.
Regards,
Benoit
RFC PATCH 1/1 of: DMA helpers: manage generic requests specification
Cousson, Benoit wrote at Monday, March 05, 2012 6:14 AM:
Quoted 27 lines · Stephen Warren
Quoted 26 lines
Nicolas Ferre wrote at Wednesday, February 29, 2012 7:54 AM:
Quoted 10 lines
By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.Quoted 5 lines
One simple xlate function is provided for the "single number" type of request biding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
At a high level this seems along the right lines.
The big issue I see is the lack of type-safety in of_get_dma_request()'s
out_datapointer: The DMA xlate function will assume it points at some particular type, and the client is responsible for allocating that correct type. However, it's "user"-supplied device tree data that hooks the two together, and there could easily be mismatches, thus causing the xlate function to trash memory.
Quoted 7 lines
Quoted 3 lines
Could each DMA controller (or type of
out_data) have some enum in the kernel that the client passes in, and the DMA engine validates matches its expectations, to prevent this kind of thing?Beside u32 and string, do we know what kind of parameters we can expect to retrieve?
Because we can potentially reduce the scope to these two types only.
I wondered whether different drivers would return arbitrary driver-specific structs, so that the DT specific would encode not only the DMA request number, but perhaps some/all of the DMA parameters like transfer size, wrap, even DMA FIFO address, ...
Still, there might not be much point in doing that, since drivers (at least on Tegra) are already manually passing this information to the DMA driver in the non-DT case, so we can just continue doing that same thing.
Signature · 1 line
nvpublic
RFC PATCH 1/1 of: DMA helpers: manage generic requests specification
Quoted 9 lines · Stephen Warren
Cousson, Benoit wrote at Monday, March 05, 2012 6:14 AM:
Quoted 3 lines
Beside u32 and string, do we know what kind of parameters we can expect to retrieve?
Because we can potentially reduce the scope to these two types only.I wondered whether different drivers would return arbitrary driver-specific structs, so that the DT specific would encode not only the DMA request number, but perhaps some/all of the DMA parameters like transfer size, wrap, even DMA FIFO address, ...
Right. The thing to consider here is: what information does the driver already have about the device?
- physical address of the data register
- size of the data register
- it certainly should know the fifo depth (think - if it has to do PIO then it needs this information already.)
so, there's no point putting that information into DT because it must be known by the driver for systems not using DMA.
We have a standardized way to pass this information to DMA engine drivers from the peripheral device driver: struct dma_slave_config.
What is outside of the scope of the driver is knowing how a peripheral device is connected to a set of DMA engines. That's a much harder problem to solve because there's many varied ways to do that in hardware, from a static mapping between the peripheral and a DMA engine request signal, to implementations which have some kind of front end MUX, to there being several choices of DMA engine for the peripheral (I believe some Samsung platforms fall into this category.)
Some of those problems can be solved by redesigning the DMA engine drivers to have a clear way to handle DMA engines with multiple request inputs shared between multiple channels: you treat the individual request inputs as separate DMA engine channels, and when there is work to be done, you allocate the DMA engine channel to a physical DMA engine and physical channel.
That's not going to be easy to do at the moment, because every DMA engine driver is custom written from the ground up with no commonality between them (I've mentioned this in previous email threads.)
I think, before trying to design some kind of DT based representation of how to bind DMA stuff to peripheral drivers, the very first thing that needs to happen is the DMA engine drivers need sorting so that they all work in mostly the same way in terms of how channels are allocated to peripherals. Only once we have a consistent way that's done can we then start looking at what kind of representation is needed to bind the two together.
Otherwise, we're going to end up with the OMAP way of binding channels, the PL08x way, the SA11x0 way, etc and its going to become very very unfunny to sort this out later.
RFC PATCH 1/1 of: DMA helpers: manage generic requests specification
In reply to Nicolas FerreHi Nico,
Quoted 22 lines · Nicolas Ferre
By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request biding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
Signed-off-by Nicolas Ferre<[email protected]>Cc Benoit Cousson<[email protected]>Cc Stephen Warren<[email protected]>Cc Grant Likely<[email protected]>Cc Russell King<[email protected]>Cc Rob Herring<[email protected]>
Quoted 7 lines
---
Hi all,Here are my thoughts about the DMA helpers for device tree. This patch goes on top of Benoit's ones. My goal was to keep this separated from any DMA infrastructure (dmaengine not needed, nor any other DMA implementation).
Thanks for taking the ball on that.
Quoted 1 line
It is to keep the ball rolling, so do not hesitate to comment.
That looks pretty good.
My only question to the whole audience who care about that is:
Do we want to enforce the usage of dmaengine and thus add the DT support into dmaengine or do we want to go add some infrastructure in DT core like it is done here?
To be honest, I'm fine with that approach because OMAP DMA is still not using dmaengine. But potentially I can easily add a dummy dmaengine into the OMAP DMA driver just to support the OF code if needed.
Quoted 97 lines
Documentation/devicetree/bindings/dma/dma.txt | 29 +++-- drivers/of/dma.c | 161 ++++++++++++++++++++++--- include/linux/of_dma.h | 33 +++++- 3 files changed, 186 insertions(+), 37 deletions(-)Documentation/devicetree/bindings/dma/dma.txt +15
-14@@ -6,9 +6,8 @@ DMA request line that goes from an IP to a DMA controller. * DMA controller -Required properties: - - dma-controller: Mark the device as a DMA controller - - #dma-cells: Number of cell for each DMA line, must be one. +Required property: + - #dma-cells: Number of cells for each DMA line. Example:@@ -17,7 +16,6 @@ Example: compatible = "ti,sdma-omap4" reg =<0x48000000 0x1000>; interrupts =<12>; - dma-controller; #dma-cells =<1>; };@@ -25,20 +23,23 @@ Example: * DMA client -Client drivers should specify the DMA request numbers using a phandle to -the controller + the DMA request number on that controller. +Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. -Required properties: - - dma-request: List of pair phandle + dma-request per line +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: - dma-request-names: list of strings in the same order as the dma-request in the dma-request property. Example: - i2c1: i2c at 1 { - ... - dma-request =<&sdma 2&sdma 3>; - dma-request-names = "tx", "rx"; - ... - }; + i2c1: i2c at 1 { + ... + dma-request =<&sdma 2&sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/dma.c +21
-1@@ -13,41 +13,145 @@#include <linux/device.h> #include <linux/err.h> #include <linux/module.h> +#include <linux/rculist.h> +#include <linux/slab.h> #include <linux/of.h> #include <linux/of_dma.h> +static LIST_HEAD (of_dma_list); + +/** +* of_dma_find_controller() - Find a DMA controller in DT DMA helpers list +* @np: device node of DMA controller +*/ +static struct of_dma *of_dma_find_controller (struct device_node *np) +{ +struct of_dma *ofdma; + +list_for_each_entry_rcu (ofdma,&of_dma_list, of_dma_controllers) { +if (ofdma->of_node == np) +return ofdma; + } + +return NULL ; +} +/** -* of_get_dma_request() - Get a DMA request number and dma-controller node +* of_dma_controller_register() - Register a DMA controller to DT DMA helpers
drivers/of/dma.c line 50
Quoted 10 lines
drivers/of/dma.c +10
-0+* @np: device node of DMA controller +* @of_dma_xlate: generic translation function which converts a phandle +* arguments list into a generic output value +* +* Returns 0 on success or appropriate errno value on error. +* +* If #dma-cells is not specified in DMA controller device tree node, we assume +* that the DMA controller phandle will come without argument. +* +* Allocated memory sould be freed with apropriate of_dma_controller_free()
Nit: typos: should appropriate
drivers/of/dma.c line 60
Quoted 10 lines
drivers/of/dma.c +10
-0+* call. +*/ +int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +struct of_dma *ofdma; +int *nbcells; + +if (!np || !of_dma_xlate) { +pr_err ("%s: not enouth information provided\n" , __func__);
Nit: typo: enough
Quoted 113 lines
drivers/of/dma.c +79
-15+return -EINVAL ; + } + + ofdma =kzalloc (sizeof (*ofdma),GFP_KERNEL ); +if (!ofdma) +return -ENOMEM ; + + nbcells = (int *)of_get_property (np,"#dma-cells" ,NULL ); +if (!nbcells) +/* +* no #dma-cells properties: assume no argument to +* dma-request property on slave side +*/ + ofdma->of_dma_n_cells =0 ; +else + ofdma->of_dma_n_cells =be32_to_cpup (nbcells); + + ofdma->of_node = np; + ofdma->of_dma_xlate = of_dma_xlate; + +/* Now queue of_dma controller structure in list */ +list_add_tail_rcu (&ofdma->of_dma_controllers,&of_dma_list); + +return 0 ; +} +EXPORT_SYMBOL (of_dma_controller_register); + +/** +* of_dma_controller_free() - Remove a DMA controller from DT DMA helpers list +* @np: device node of DMA controller +* +* Memory allocated by of_dma_controller_register() is freed here. +*/ +void of_dma_controller_free (struct device_node *np) +{ +struct of_dma *ofdma; + + ofdma =of_dma_find_controller (np); +if (ofdma) { +list_del_rcu (&ofdma->of_dma_controllers); +kfree (ofdma); + } +} +EXPORT_SYMBOL (of_dma_controller_free); + +/** +* of_get_dma_request() - Get the associated DMA request data * @np: device node to get DMA request from -* @propname: property name containing DMA specifier(s) * @index: index of the DMA request -* @ctrl_np: a device_node pointer to fill in +* @out_data: a output that can be filled in by the of_dma_xlate() function * -* Returns DMA number along to the dma controller node, or one of the errno -* value on the error condition. If @ctrl_np is not NULL the function also -* fills in the DMA controller device_node pointer. +* Returns return value of of_dma_xlate() and fills out_data (if provided). +* On error returns the appropriate errno value. */ int of_get_dma_request (struct device_node *np,int index, -struct device_node **ctrl_np) +void *out_data) { -int ret = -EINVAL ; -struct of_phandle_args dma_spec; +struct of_phandle_args dma_spec; +struct of_dma *ofdma; +int ret; ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , index,&dma_spec);if (ret) { -pr_debug ("%s: can't parse dma property\n" , __func__); +pr_err ("%s: can't parse dma property\n" , np->full_name); goto err0; } -if (dma_spec.args_count>0 ) - ret = dma_spec.args[0 ]; +if (list_empty (&of_dma_list)) { +pr_debug ("%s: empty DMA controller list\n" , + np->full_name); + ret = -ENOENT ; + goto err1; + } -if (ctrl_np) - *ctrl_np = dma_spec.np; -else -of_node_put (dma_spec.np); + ofdma =of_dma_find_controller (dma_spec.np); +if (!ofdma) { +pr_debug ("%s: DMA controller %s isn't registered\n" , + np->full_name, dma_spec.np->full_name); + ret = -ENODEV ; + goto err1; + } +if (dma_spec.args_count != ofdma->of_dma_n_cells) { +pr_debug ("%s: wrong #dma-cells for %s\n" , + np->full_name, dma_spec.np->full_name); + ret = -EINVAL ; + goto err1; + } + + ret = ofdma->of_dma_xlate (&dma_spec, out_data); + +err1: +of_node_put (dma_spec.np); err0:pr_debug ("%s exited with status %d\n" , __func__, ret);return ret;
drivers/of/dma.c err0: · line 168
Quoted 10 lines
drivers/of/dma.c +7
-0@@ -55,6 +159,24 @@ err0:EXPORT_SYMBOL (of_get_dma_request);/** +* of_dma_xlate_onenumbercell() - Generic DMA xlate for direct one cell bindings +* +* Device Tree DMA translation function which works with one cell bindings +* where the cell values map directly to the hardware request number understood +* by the DMA controller. +*/ +int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec,void *req_nbr)
Nit: I'd rather use dma_req instead of req_nbr that can be understood as number of request lines.
Quoted 81 lines
drivers/of/dma.c +15
-3+{ +if (!dma_spec) +return -EINVAL ; +if (WARN_ON (dma_spec->args_count !=1 )) +return -EINVAL ; + *(int *)req_nbr = dma_spec->args[0 ]; +return 0 ; +} +EXPORT_SYMBOL (of_dma_xlate_onenumbercell); + +/** * of_dma_count - Count DMA requests for a device * @np: device node to count DMAs for * @@ -105,13 +227,14 @@ int of_dma_to_resource(struct device_node *dev, int index, struct resource *r) {const char *name =NULL ;int dma; +int ret;if (!r)return -EINVAL ; - dma =of_get_dma_request (dev, index,NULL ); -if (dma<0 ) -return dma; + ret =of_get_dma_request (dev, index,&dma); +if (ret<0 ) +return ret;/* * Get optional "dma-request-names" property to add a name include/linux/of_dma.h +24
-4@@ -17,18 +17,38 @@struct device_node ; -#ifdef CONFIG_OF_GPIO +#ifdef CONFIG_OF_DMA +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +}; + +extern int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)); +extern void of_dma_controller_free (struct device_node *np);extern int of_get_dma_request (struct device_node *np,int index, -struct device_node **ctrl_np); +void *out_data);extern unsigned int of_dma_count (struct device_node *np);extern int of_dma_to_resource (struct device_node *dev,int index,struct resource *r); +extern int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *req_nbr);#else /* CONFIG_OF_DMA */ -static int of_get_dma_request (struct device_node *np,int index, -struct device_node **ctrl_np); +static int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +return -ENOSYS ; +} + +static void of_dma_controller_free (struct device_node *np) {} + +extern int of_get_dma_request (struct device_node *np,int index, +void *out_data) {return -ENOSYS ; }@@ -44,6 +64,11 @@ static int of_dma_to_resource(struct device_node *dev, int index,return -ENOSYS ;
include/linux/of_dma.h static int of_dma_to_resource(struct device_node *dev, int index, · line 74
Quoted 10 lines
include/linux/of_dma.h +5
-0} +static int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *req_nbr) +{ +return -ENOSYS ; +}#endif /* CONFIG_OF_DMA */ #endif /* __LINUX_OF_DMA_H */
Regards,
Benoit
RFC PATCH 1/1 of: DMA helpers: manage generic requests specification
In reply to Nicolas FerreQuoted 150 lines · Nicolas Ferre
By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request biding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
Signed-off-by Nicolas Ferre <[email protected]>Cc Benoit Cousson <[email protected]>Cc Stephen Warren <[email protected]>Cc Grant Likely <[email protected]>Cc Russell King <[email protected]>Cc Rob Herring <[email protected]>---
Hi all,Here are my thoughts about the DMA helpers for device tree. This patch goes on top of Benoit's ones. My goal was to keep this separated from any DMA infrastructure (dmaengine not needed, nor any other DMA implementation).
It is to keep the ball rolling, so do not hesitate to comment.
Best regards,
Documentation/devicetree/bindings/dma/dma.txt | 29 +++-- drivers/of/dma.c | 161 ++++++++++++++++++++++--- include/linux/of_dma.h | 33 +++++- 3 files changed, 186 insertions(+), 37 deletions(-)Documentation/devicetree/bindings/dma/dma.txt +15
-14@@ -6,9 +6,8 @@ DMA request line that goes from an IP to a DMA controller. * DMA controller -Required properties: - - dma-controller: Mark the device as a DMA controller - - #dma-cells: Number of cell for each DMA line, must be one. +Required property: + - #dma-cells: Number of cells for each DMA line. Example:@@ -17,7 +16,6 @@ Example: compatible = "ti,sdma-omap4" reg = <0x48000000 0x1000>; interrupts = <12>; - dma-controller; #dma-cells = <1>; };@@ -25,20 +23,23 @@ Example: * DMA client -Client drivers should specify the DMA request numbers using a phandle to -the controller + the DMA request number on that controller. +Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. -Required properties: - - dma-request: List of pair phandle + dma-request per line +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: - dma-request-names: list of strings in the same order as the dma-request in the dma-request property. Example: - i2c1: i2c at 1 { - ... - dma-request = <&sdma 2 &sdma 3>; - dma-request-names = "tx", "rx"; - ... - }; + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/dma.c +39
-1@@ -13,41 +13,145 @@#include <linux/device.h> #include <linux/err.h> #include <linux/module.h> +#include <linux/rculist.h> +#include <linux/slab.h> #include <linux/of.h> #include <linux/of_dma.h> +static LIST_HEAD (of_dma_list); + +/** +* of_dma_find_controller() - Find a DMA controller in DT DMA helpers list +* @np: device node of DMA controller +*/ +static struct of_dma *of_dma_find_controller (struct device_node *np) +{ +struct of_dma *ofdma; + +list_for_each_entry_rcu (ofdma, &of_dma_list, of_dma_controllers) { +if (ofdma->of_node == np) +return ofdma; + } + +return NULL ; +} +/** -* of_get_dma_request() - Get a DMA request number and dma-controller node +* of_dma_controller_register() - Register a DMA controller to DT DMA helpers +* @np: device node of DMA controller +* @of_dma_xlate: generic translation function which converts a phandle +* arguments list into a generic output value +* +* Returns 0 on success or appropriate errno value on error. +* +* If #dma-cells is not specified in DMA controller device tree node, we assume +* that the DMA controller phandle will come without argument. +* +* Allocated memory sould be freed with apropriate of_dma_controller_free() +* call. +*/ +int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +struct of_dma *ofdma; +int *nbcells; +
drivers/of/dma.c line 68
Quoted 10 lines
drivers/of/dma.c +10
-0+if (!np || !of_dma_xlate) { +pr_err ("%s: not enouth information provided\n" , __func__); +return -EINVAL ; + } + + ofdma =kzalloc (sizeof (*ofdma),GFP_KERNEL ); +if (!ofdma) +return -ENOMEM ; + + nbcells = (int *)of_get_property (np,"#dma-cells" ,NULL );
No cast required; but nbcells should be a (const __be32 *) instead of an int*.
Quoted 8 lines
drivers/of/dma.c +8
-0+if (!nbcells) +/* +* no #dma-cells properties: assume no argument to +* dma-request property on slave side +*/ + ofdma->of_dma_n_cells =0 ; +else + ofdma->of_dma_n_cells =be32_to_cpup (nbcells);
drivers/of/dma.c line 86
Quoted 10 lines
drivers/of/dma.c +10
-0+ + ofdma->of_node = np; + ofdma->of_dma_xlate = of_dma_xlate; + +/* Now queue of_dma controller structure in list */ +list_add_tail_rcu (&ofdma->of_dma_controllers, &of_dma_list); + +return 0 ; +} +EXPORT_SYMBOL (of_dma_controller_register);
EXPORT_SYMBOL_GPL
Quoted 136 lines
drivers/of/dma.c +75
-18+ +/** +* of_dma_controller_free() - Remove a DMA controller from DT DMA helpers list +* @np: device node of DMA controller +* +* Memory allocated by of_dma_controller_register() is freed here. +*/ +void of_dma_controller_free (struct device_node *np) +{ +struct of_dma *ofdma; + + ofdma =of_dma_find_controller (np); +if (ofdma) { +list_del_rcu (&ofdma->of_dma_controllers); +kfree (ofdma); + } +} +EXPORT_SYMBOL (of_dma_controller_free); + +/** +* of_get_dma_request() - Get the associated DMA request data * @np: device node to get DMA request from -* @propname: property name containing DMA specifier(s) * @index: index of the DMA request -* @ctrl_np: a device_node pointer to fill in +* @out_data: a output that can be filled in by the of_dma_xlate() function * -* Returns DMA number along to the dma controller node, or one of the errno -* value on the error condition. If @ctrl_np is not NULL the function also -* fills in the DMA controller device_node pointer. +* Returns return value of of_dma_xlate() and fills out_data (if provided). +* On error returns the appropriate errno value. */ int of_get_dma_request (struct device_node *np,int index, -struct device_node **ctrl_np) +void *out_data) { -int ret = -EINVAL ; -struct of_phandle_args dma_spec; +struct of_phandle_args dma_spec; +struct of_dma *ofdma; +int ret; ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , index, &dma_spec);if (ret) { -pr_debug ("%s: can't parse dma property\n" , __func__); +pr_err ("%s: can't parse dma property\n" , np->full_name); goto err0; } -if (dma_spec.args_count >0 ) - ret = dma_spec.args[0 ]; +if (list_empty (&of_dma_list)) { +pr_debug ("%s: empty DMA controller list\n" , + np->full_name); + ret = -ENOENT ; + goto err1; + } -if (ctrl_np) - *ctrl_np = dma_spec.np; -else -of_node_put (dma_spec.np); + ofdma =of_dma_find_controller (dma_spec.np); +if (!ofdma) { +pr_debug ("%s: DMA controller %s isn't registered\n" , + np->full_name, dma_spec.np->full_name); + ret = -ENODEV ; + goto err1; + } +if (dma_spec.args_count != ofdma->of_dma_n_cells) { +pr_debug ("%s: wrong #dma-cells for %s\n" , + np->full_name, dma_spec.np->full_name); + ret = -EINVAL ; + goto err1; + } + + ret = ofdma->of_dma_xlate (&dma_spec, out_data); + +err1: +of_node_put (dma_spec.np); err0:pr_debug ("%s exited with status %d\n" , __func__, ret);return ret;@@ -55,6 +159,24 @@ err0:EXPORT_SYMBOL (of_get_dma_request);/** +* of_dma_xlate_onenumbercell() - Generic DMA xlate for direct one cell bindings +* +* Device Tree DMA translation function which works with one cell bindings +* where the cell values map directly to the hardware request number understood +* by the DMA controller. +*/ +int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec,void *req_nbr) +{ +if (!dma_spec) +return -EINVAL ; +if (WARN_ON (dma_spec->args_count !=1 )) +return -EINVAL ; + *(int *)req_nbr = dma_spec->args[0 ]; +return 0 ; +} +EXPORT_SYMBOL (of_dma_xlate_onenumbercell); + +/** * of_dma_count - Count DMA requests for a device * @np: device node to count DMAs for * @@ -105,13 +227,14 @@ int of_dma_to_resource(struct device_node *dev, int index, struct resource *r) {const char *name =NULL ;int dma; +int ret;if (!r)return -EINVAL ; - dma =of_get_dma_request (dev, index,NULL ); -if (dma <0 ) -return dma; + ret =of_get_dma_request (dev, index, &dma); +if (ret <0 ) +return ret;/* * Get optional "dma-request-names" property to add a name include/linux/of_dma.h +0
-0@@ -17,18 +17,38 @@struct device_node ;
include/linux/of_dma.h line 27
Quoted 10 lines
include/linux/of_dma.h +7
-1-#ifdef CONFIG_OF_GPIO +#ifdef CONFIG_OF_DMA +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};
The api makes struct of_dma private; so this should be moved to the .c file.
Otherwise, this looks okay to me.
g.
Quoted 45 lines
include/linux/of_dma.h +22
-3+ +extern int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)); +extern void of_dma_controller_free (struct device_node *np);extern int of_get_dma_request (struct device_node *np,int index, -struct device_node **ctrl_np); +void *out_data);extern unsigned int of_dma_count (struct device_node *np);extern int of_dma_to_resource (struct device_node *dev,int index,struct resource *r); +extern int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *req_nbr);#else /* CONFIG_OF_DMA */ -static int of_get_dma_request (struct device_node *np,int index, -struct device_node **ctrl_np); +static int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +return -ENOSYS ; +} + +static void of_dma_controller_free (struct device_node *np) {} + +extern int of_get_dma_request (struct device_node *np,int index, +void *out_data) {return -ENOSYS ; }@@ -44,6 +64,11 @@ static int of_dma_to_resource(struct device_node *dev, int index,return -ENOSYS ; } +static int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *req_nbr) +{ +return -ENOSYS ; +}#endif /* CONFIG_OF_DMA */ #endif /* __LINUX_OF_DMA_H */ Signature · 1 line
1.7.9
RFC PATCH 1/1 of: DMA helpers: manage generic requests specification
On 03/05/2012 04:36 PM, Grant Likely :
Quoted 335 lines · Nicolas Ferre
Quoted 150 lines
By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request biding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
Signed-off-by Nicolas Ferre <[email protected]>Cc Benoit Cousson <[email protected]>Cc Stephen Warren <[email protected]>Cc Grant Likely <[email protected]>Cc Russell King <[email protected]>Cc Rob Herring <[email protected]>---
Hi all,Here are my thoughts about the DMA helpers for device tree. This patch goes on top of Benoit's ones. My goal was to keep this separated from any DMA infrastructure (dmaengine not needed, nor any other DMA implementation).
It is to keep the ball rolling, so do not hesitate to comment.
Best regards,
Documentation/devicetree/bindings/dma/dma.txt | 29 +++-- drivers/of/dma.c | 161 ++++++++++++++++++++++--- include/linux/of_dma.h | 33 +++++- 3 files changed, 186 insertions(+), 37 deletions(-)Documentation/devicetree/bindings/dma/dma.txt +15
-14@@ -6,9 +6,8 @@ DMA request line that goes from an IP to a DMA controller. * DMA controller -Required properties: - - dma-controller: Mark the device as a DMA controller - - #dma-cells: Number of cell for each DMA line, must be one. +Required property: + - #dma-cells: Number of cells for each DMA line. Example:@@ -17,7 +16,6 @@ Example: compatible = "ti,sdma-omap4" reg = <0x48000000 0x1000>; interrupts = <12>; - dma-controller; #dma-cells = <1>; };@@ -25,20 +23,23 @@ Example: * DMA client -Client drivers should specify the DMA request numbers using a phandle to -the controller + the DMA request number on that controller. +Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. -Required properties: - - dma-request: List of pair phandle + dma-request per line +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: - dma-request-names: list of strings in the same order as the dma-request in the dma-request property. Example: - i2c1: i2c at 1 { - ... - dma-request = <&sdma 2 &sdma 3>; - dma-request-names = "tx", "rx"; - ... - }; + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/dma.c +39
-1@@ -13,41 +13,145 @@#include <linux/device.h> #include <linux/err.h> #include <linux/module.h> +#include <linux/rculist.h> +#include <linux/slab.h> #include <linux/of.h> #include <linux/of_dma.h> +static LIST_HEAD (of_dma_list); + +/** +* of_dma_find_controller() - Find a DMA controller in DT DMA helpers list +* @np: device node of DMA controller +*/ +static struct of_dma *of_dma_find_controller (struct device_node *np) +{ +struct of_dma *ofdma; + +list_for_each_entry_rcu (ofdma, &of_dma_list, of_dma_controllers) { +if (ofdma->of_node == np) +return ofdma; + } + +return NULL ; +} +/** -* of_get_dma_request() - Get a DMA request number and dma-controller node +* of_dma_controller_register() - Register a DMA controller to DT DMA helpers +* @np: device node of DMA controller +* @of_dma_xlate: generic translation function which converts a phandle +* arguments list into a generic output value +* +* Returns 0 on success or appropriate errno value on error. +* +* If #dma-cells is not specified in DMA controller device tree node, we assume +* that the DMA controller phandle will come without argument. +* +* Allocated memory sould be freed with apropriate of_dma_controller_free() +* call. +*/ +int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +struct of_dma *ofdma; +int *nbcells; +drivers/of/dma.c line 68
Quoted 10 lines
drivers/of/dma.c +10
-0+if (!np || !of_dma_xlate) { +pr_err ("%s: not enouth information provided\n" , __func__); +return -EINVAL ; + } + + ofdma =kzalloc (sizeof (*ofdma),GFP_KERNEL ); +if (!ofdma) +return -ENOMEM ; + + nbcells = (int *)of_get_property (np,"#dma-cells" ,NULL );No cast required; but nbcells should be a (const
__be32*) instead of an int*.Quoted 8 lines
drivers/of/dma.c +8
-0+if (!nbcells) +/* +* no #dma-cells properties: assume no argument to +* dma-request property on slave side +*/ + ofdma->of_dma_n_cells =0 ; +else + ofdma->of_dma_n_cells =be32_to_cpup (nbcells);drivers/of/dma.c line 86
Quoted 10 lines
drivers/of/dma.c +10
-0+ + ofdma->of_node = np; + ofdma->of_dma_xlate = of_dma_xlate; + +/* Now queue of_dma controller structure in list */ +list_add_tail_rcu (&ofdma->of_dma_controllers, &of_dma_list); + +return 0 ; +} +EXPORT_SYMBOL (of_dma_controller_register);
EXPORT_SYMBOL_GPLQuoted 136 lines
drivers/of/dma.c +75
-18+ +/** +* of_dma_controller_free() - Remove a DMA controller from DT DMA helpers list +* @np: device node of DMA controller +* +* Memory allocated by of_dma_controller_register() is freed here. +*/ +void of_dma_controller_free (struct device_node *np) +{ +struct of_dma *ofdma; + + ofdma =of_dma_find_controller (np); +if (ofdma) { +list_del_rcu (&ofdma->of_dma_controllers); +kfree (ofdma); + } +} +EXPORT_SYMBOL (of_dma_controller_free); + +/** +* of_get_dma_request() - Get the associated DMA request data * @np: device node to get DMA request from -* @propname: property name containing DMA specifier(s) * @index: index of the DMA request -* @ctrl_np: a device_node pointer to fill in +* @out_data: a output that can be filled in by the of_dma_xlate() function * -* Returns DMA number along to the dma controller node, or one of the errno -* value on the error condition. If @ctrl_np is not NULL the function also -* fills in the DMA controller device_node pointer. +* Returns return value of of_dma_xlate() and fills out_data (if provided). +* On error returns the appropriate errno value. */ int of_get_dma_request (struct device_node *np,int index, -struct device_node **ctrl_np) +void *out_data) { -int ret = -EINVAL ; -struct of_phandle_args dma_spec; +struct of_phandle_args dma_spec; +struct of_dma *ofdma; +int ret; ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , index, &dma_spec);if (ret) { -pr_debug ("%s: can't parse dma property\n" , __func__); +pr_err ("%s: can't parse dma property\n" , np->full_name); goto err0; } -if (dma_spec.args_count >0 ) - ret = dma_spec.args[0 ]; +if (list_empty (&of_dma_list)) { +pr_debug ("%s: empty DMA controller list\n" , + np->full_name); + ret = -ENOENT ; + goto err1; + } -if (ctrl_np) - *ctrl_np = dma_spec.np; -else -of_node_put (dma_spec.np); + ofdma =of_dma_find_controller (dma_spec.np); +if (!ofdma) { +pr_debug ("%s: DMA controller %s isn't registered\n" , + np->full_name, dma_spec.np->full_name); + ret = -ENODEV ; + goto err1; + } +if (dma_spec.args_count != ofdma->of_dma_n_cells) { +pr_debug ("%s: wrong #dma-cells for %s\n" , + np->full_name, dma_spec.np->full_name); + ret = -EINVAL ; + goto err1; + } + + ret = ofdma->of_dma_xlate (&dma_spec, out_data); + +err1: +of_node_put (dma_spec.np); err0:pr_debug ("%s exited with status %d\n" , __func__, ret);return ret;@@ -55,6 +159,24 @@ err0:EXPORT_SYMBOL (of_get_dma_request);/** +* of_dma_xlate_onenumbercell() - Generic DMA xlate for direct one cell bindings +* +* Device Tree DMA translation function which works with one cell bindings +* where the cell values map directly to the hardware request number understood +* by the DMA controller. +*/ +int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec,void *req_nbr) +{ +if (!dma_spec) +return -EINVAL ; +if (WARN_ON (dma_spec->args_count !=1 )) +return -EINVAL ; + *(int *)req_nbr = dma_spec->args[0 ]; +return 0 ; +} +EXPORT_SYMBOL (of_dma_xlate_onenumbercell); + +/** * of_dma_count - Count DMA requests for a device * @np: device node to count DMAs for * @@ -105,13 +227,14 @@ int of_dma_to_resource(struct device_node *dev, int index, struct resource *r) {const char *name =NULL ;int dma; +int ret;if (!r)return -EINVAL ; - dma =of_get_dma_request (dev, index,NULL ); -if (dma <0 ) -return dma; + ret =of_get_dma_request (dev, index, &dma); +if (ret <0 ) +return ret;/* * Get optional "dma-request-names" property to add a name include/linux/of_dma.h +0
-0@@ -17,18 +17,38 @@struct device_node ;include/linux/of_dma.h line 27
Quoted 10 lines
include/linux/of_dma.h +7
-1-#ifdef CONFIG_OF_GPIO +#ifdef CONFIG_OF_DMA +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};The api makes struct
of_dmaprivate; so this should be moved to the .c file.
Quoted 1 line
Otherwise, this looks okay to me.
Hi Grant and Stephen,
I plan to resend a squashed version of Benoit's patch and mine about those DMA helpers (we talked about this with Benoit). I tried to address all remarks by Benoit and Grant.
I do have the will to avoid the treats of memory corruption in case of malformed DT data, as Stephen was saying. But, on the other hand I do not know really if this can happen: if the .xlate() function which is provided by the DMA controller is well written, it should check for proper args_count or maximum string size. I do not have the feeling that adding an enum will enforce the security here.
Do you know a way to enforce security of this "void *" parameter or the check of number of cells + the due diligence of .xlate() function writers will be enough?
Best regards,
Signature · 1 line
Nicolas Ferre
RFC PATCH 1/1 of: DMA helpers: manage generic requests specification
On 03/14/2012 11:47 AM, Nicolas Ferre wrote:
...
Quoted 10 lines
I do have the will to avoid the treats of memory corruption in case of malformed DT data, as Stephen was saying. But, on the other hand I do not know really if this can happen: if the .xlate() function which is provided by the DMA controller is well written, it should check for proper
args_countor maximum string size. I do not have the feeling that adding an enum will enforce the security here.Do you know a way to enforce security of this "void *" parameter or the check of number of cells + the due diligence of .xlate() function writers will be enough?
I guess if the only source of the data is a driver's of_xlate function, and it's only being passed back to that same driver and never interpreted elsewhere, then its probably reasonable to assume that's enough for safety.
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Cousson, BenoitAdd some basic helpers to retrieve a DMA controller device_node and the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of an of_dma_xlate() function that will be implemented by the DMA controller can carry any type of "dma-request" argument.
The DMA client will search its associated DMA controller in the list and call the registered of_dam_xlate() function to retrieve the request values.
One simple xlate function is provided for the "single number" type of request binding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type IORESOURCE_DMA.
--- Documentation/devicetree/bindings/dma/dma.txt | 45 +++++ drivers/of/Kconfig | 5 + drivers/of/Makefile | 1 + drivers/of/dma.c | 260 +++++++++++++++++++++++++ include/linux/of_dma.h | 67 +++++++ 5 files changed, 378 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/dma.txt create mode 100644 drivers/of/dma.c create mode 100644 include/linux/of_dma.h
Documentation/devicetree/bindings/dma/dma.txt new file +45 -0
@@ -0,0 +1,45 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller + +Required property: + - #dma-cells: Number of cells for each DMA line. + + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg = <0x48000000 0x1000>; + interrupts = <12>; + #dma-cells = <1>; + }; + + + +* DMA client + +Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. + +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property. + + +Example: + + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };
drivers/of/Kconfig +5 -0
@@ -90,4 +90,9 @@ config OF_PCI_IRQ help OpenFirmware PCI IRQ routing helpers +config OF_DMA + def_bool y + help + Device Tree DMA routing helpers + endmenu # OF
drivers/of/Makefile +1 -0
@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SELFTEST) += selftest.o obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o +obj-$(CONFIG_OF_DMA) += dma.o
drivers/of/dma.c new file +260 -0
260 lines not on this page · raw message
include/linux/of_dma.h new file +67 -0
67 lines not on this page · raw message
Signature · 1 line
1.7.9
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 10 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.
Quoted 8 lines
One simple xlate function is provided for the "single number" type of request binding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
This looks very good. I missed the first version of this patch, but was thinking of very similar bindings.
Quoted 10 lines
Documentation/devicetree/bindings/dma/dma.txt new file +10
-0+Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. + +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property.
Documentation/devicetree/bindings/dma/dma.txt line 45
Quoted 10 lines
Documentation/devicetree/bindings/dma/dma.txt new file +10
-0+ + +Example: + + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };
This is slightly different from how the proposed pwm binding works that Thierry is working on, which uses an arbitrary property name instead of requiring the use of a specific property but then allowing to give names in another property.
I don't care much which way it's done, but please try to agree on one approach that is used for both.
The one you have here is already used by reg and irq, while the other one is used in gpio.
Quoted 9 lines
+/**
+ *of_get_dma_request()- Get the associated DMA request data
+ * @np: device node to get DMA request from
+ * @index: index of the DMA request
+ * @out_data: a output that can be filled in by theof_dma_xlate()function
+ *
+ * Returns return value ofof_dma_xlate()and fillsout_data(if provided).
+ * On error returns the appropriate errno value.
+ */
I would suggest provinding a helper function around this one that directly calls __dma_request_channel(). AFAICT, you should be able to let the dma controller provide a generic filter function in struct of_dma that the driver can use by default.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 14 lines · Arnd Bergmann
Quoted 13 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request binding.
Quoted 8 lines
Quoted 5 lines
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.This looks very good. I missed the first version of this patch, but was thinking of very similar bindings.
There's one issue which is concerning me - when we switch OMAP to use DMA engine, it won't use numeric stuff anymore but the DMA engine way of requesting a channel (match function + data).
How does that fit into this scheme?
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 21 lines · Russell King - ARM Linux
Quoted 20 lines · Arnd Bergmann
Quoted 18 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request binding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
Quoted 8 lines
Quoted 2 lines
This looks very good. I missed the first version of this patch, but was thinking of very similar bindings.
There's one issue which is concerning me - when we switch OMAP to use DMA engine, it won't use numeric stuff anymore but the DMA engine way of requesting a channel (match function + data).
How does that fit into this scheme?
I haven't looked at the omap dma driver much, but if it just uses a channel number, then I'd assume the data would be a single u32 cell with that number, and the match function would be trivial.
Is that what you are asking about or am I missing the point?
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Russell King - ARM LinuxQuoted 21 lines · Russell King - ARM Linux
Quoted 20 lines · Arnd Bergmann
Quoted 18 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request binding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
Quoted 8 lines
Quoted 2 lines
This looks very good. I missed the first version of this patch, but was thinking of very similar bindings.
There's one issue which is concerning me - when we switch OMAP to use DMA engine, it won't use numeric stuff anymore but the DMA engine way of requesting a channel (match function + data).
How does that fit into this scheme?
Not well as the current patch set stands. The xlate function doesn't return any context for the dma channel number that it returns, so the driver cannot figure out which DMA controller to use if there are multiple.
g.
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 29 lines · Grant Likely
Quoted 28 lines · Russell King - ARM Linux
Quoted 14 lines · Arnd Bergmann
Quoted 13 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request binding.
Quoted 8 lines
Quoted 5 lines
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.This looks very good. I missed the first version of this patch, but was thinking of very similar bindings.
There's one issue which is concerning me - when we switch OMAP to use DMA engine, it won't use numeric stuff anymore but the DMA engine way of requesting a channel (match function + data).
Quoted 5 lines
Quoted 1 line
How does that fit into this scheme?
Not well as the current patch set stands. The xlate function doesn't return any context for the dma channel number that it returns, so the driver cannot figure out which DMA controller to use if there are multiple.
Shouldn't that be part of the data returned by xlate? I was under the impression that this would be the data that you would pass into dma_request_channel, together with a filter function that find the instance from the data.
That's why I suggested adding another helper function that would provide a generic filter function for the dt binding doing just that.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 32 lines · Arnd Bergmann
Quoted 31 lines · Grant Likely
Quoted 29 lines · Russell King - ARM Linux
Quoted 14 lines · Arnd Bergmann
Quoted 13 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request binding.
Quoted 8 lines
Quoted 5 lines
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.This looks very good. I missed the first version of this patch, but was thinking of very similar bindings.
There's one issue which is concerning me - when we switch OMAP to use DMA engine, it won't use numeric stuff anymore but the DMA engine way of requesting a channel (match function + data).
How does that fit into this scheme?
Quoted 7 lines
Quoted 3 lines
Not well as the current patch set stands. The xlate function doesn't return any context for the dma channel number that it returns, so the driver cannot figure out which DMA controller to use if there are multiple.
Shouldn't that be part of the data returned by xlate? I was under the impression that this would be the data that you would pass into
dma_request_channel, together with a filter function that find the instance from the data.
Yes, that's my point. The patch as it is currently written doesn't do what it needs to.
g.
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Arnd BergmannQuoted 1 line · Arnd Bergmann
On Thursday 15 March 2012, Nicolas Ferre wrote:
[...]
Quoted 12 lines
Documentation/devicetree/bindings/dma/dma.txt line 45
Quoted 6 lines
Documentation/devicetree/bindings/dma/dma.txt new file +6
-0+ i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };This is slightly different from how the proposed pwm binding works that Thierry is working on, which uses an arbitrary property name instead of requiring the use of a specific property but then allowing to give names in another property.
Quoted 5 lines
I don't care much which way it's done, but please try to agree on one approach that is used for both.
The one you have here is already used by reg and irq, while the other one is used in gpio.
I think we can just use pwm as the fixed property name. Or alternatively do something along the lines of the regulator bindings, where we use "-pwm" as the suffix for specifying PWM devices. For instance if a named PWM is requested, the OF support code would look for a <name>-pwm property, while requesting an unnamed PWM would simply look at the pwm property.
When it comes to the labelling of PWM devices, I don't think both variants are exclusive. Currently the PWM framework uses name of the user OF device node for the PWM label. That is, if I have the following in the DTS:
pwm {
...
};
backlight {
compatible = "pwm-backlight";
pwm = <&pwm 0 5000000>;
...
};Then the PWM will be labelled "backlight":
$ cat cat /sys/kernel/debug/pwm platform/tegra-pwm, 4 PWM devices pwm-0 (backlight ): requested enabled pwm-1 ((null) ): pwm-2 ((null) ): pwm-3 ((null) ):
So if we decide to explicitly allow specifying names, then we can always add a pwm-names property (or <name>-pwm-names respectively) to use as label and fallback to the user OF device node name if that property is not present.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120315/ffaa7df4/attachment.sig>
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 50 lines · Thierry Reding
Quoted 1 line · Arnd Bergmann
On Thursday 15 March 2012, Nicolas Ferre wrote:
[...]
Quoted 12 lines
Documentation/devicetree/bindings/dma/dma.txt line 45
Quoted 6 lines
Documentation/devicetree/bindings/dma/dma.txt new file +6
-0+ i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };This is slightly different from how the proposed pwm binding works that Thierry is working on, which uses an arbitrary property name instead of requiring the use of a specific property but then allowing to give names in another property.
Quoted 5 lines
I don't care much which way it's done, but please try to agree on one approach that is used for both.
The one you have here is already used by reg and irq, while the other one is used in gpio.
I think we can just use pwm as the fixed property name. Or alternatively do something along the lines of the regulator bindings, where we use "-pwm" as the suffix for specifying PWM devices. For instance if a named PWM is requested, the OF support code would look for a <name>-pwm property, while requesting an unnamed PWM would simply look at the pwm property.
When it comes to the labelling of PWM devices, I don't think both variants are exclusive. Currently the PWM framework uses name of the user OF device node for the PWM label. That is, if I have the following in the DTS:
pwm { .. }; backlight { compatible = "pwm-backlight"; pwm = <&pwm 0 5000000>; ... };Then the PWM will be labelled "backlight":
$ cat cat /sys/kernel/debug/pwm platform/tegra-pwm, 4 PWM devices pwm-0 (backlight ): requested enabled pwm-1 ((null) ): pwm-2 ((null) ): pwm-3 ((null) ):
Quoted 3 lines
So if we decide to explicitly allow specifying names, then we can always add a pwm-names property (or <name>-pwm-names respectively) to use as label and fallback to the user OF device node name if that property is not present.
After implementing both schemes (ie. interrupts+interrupt-names && [*-]gpios) I definitely prefer the fixed property name plus a separate names property. It is easier to use common code with that scheme, and easier to statically check for correctness.
g.
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 9 lines · Grant Likely
Quoted 3 lines · Thierry Reding
So if we decide to explicitly allow specifying names, then we can always add a pwm-names property (or <name>-pwm-names respectively) to use as label and fallback to the user OF device node name if that property is not present.
After implementing both schemes (ie. interrupts+interrupt-names && [*-]gpios) I definitely prefer the fixed property name plus a separate names property. It is easier to use common code with that scheme, and easier to statically check for correctness.
Okay. Would everyone be happy with "pwms" and "pwm-names"?
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120318/eedcc819/attachment.sig>
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 7 lines · Thierry Reding
Not enough information to check signature validity. Show Details
Quoted 5 lines · Grant Likely
Quoted 3 lines · Thierry Reding
So if we decide to explicitly allow specifying names, then we can always add a pwm-names property (or <name>-pwm-names respectively) to use as label and fallback to the user OF device node name if that property is not present.
Quoted 6 lines
Quoted 4 lines
After implementing both schemes (ie. interrupts+interrupt-names && [*-]gpios) I definitely prefer the fixed property name plus a separate names property. It is easier to use common code with that scheme, and easier to statically check for correctness.
Okay. Would everyone be happy with "pwms" and "pwm-names"?
Sounds good. I would have suggested "pwm", but the plural also works since that is used for "interrupts", too.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Thierry RedingQuoted 6 lines · Thierry Reding
Quoted 5 lines · Grant Likely
Quoted 3 lines · Thierry Reding
So if we decide to explicitly allow specifying names, then we can always add a pwm-names property (or <name>-pwm-names respectively) to use as label and fallback to the user OF device node name if that property is not present.
Quoted 6 lines
Quoted 4 lines
After implementing both schemes (ie. interrupts+interrupt-names && [*-]gpios) I definitely prefer the fixed property name plus a separate names property. It is easier to use common code with that scheme, and easier to statically check for correctness.
Okay. Would everyone be happy with "pwms" and "pwm-names"?
okay.
g.
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Grant LikelyQuoted 4 lines · Grant Likely
After implementing both schemes (ie. interrupts+interrupt-names && [*-]gpios) I definitely prefer the fixed property name plus a separate names property. It is easier to use common code with that scheme, and easier to statically check for correctness.
It's not a fantastic experience when using the bindings as the arrays grow large, though - keeping things matched up isn't much fun especially if any of the elements in the array are optional.
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 10 lines · Mark Brown
Quoted 4 lines · Grant Likely
After implementing both schemes (ie. interrupts+interrupt-names && [*-]gpios) I definitely prefer the fixed property name plus a separate names property. It is easier to use common code with that scheme, and easier to statically check for correctness.
It's not a fantastic experience when using the bindings as the arrays grow large, though - keeping things matched up isn't much fun especially if any of the elements in the array are optional.
Maybe one can use named properties in a new device node in that case, like this:
bus {
dma: dma-controller {
#dma-cells = <1>;
};
device {
compatible = "device";
channel: dma-channel {
type = <0x1>;
name = "foo";
number = <23>;
direction = <3>;
};
dma-requests = <&dma &channel>;
};
};In this case, the dma engine's filter function would look up the dma-channel device node from the phandle passed in the argument to the dma-requests property, while a simpler dma engine would just use a single number to identify a channel there.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 15 lines · Arnd Bergmann
Quoted 10 lines · Mark Brown
Quoted 4 lines · Grant Likely
After implementing both schemes (ie. interrupts+interrupt-names && [*-]gpios) I definitely prefer the fixed property name plus a separate names property. It is easier to use common code with that scheme, and easier to statically check for correctness.
It's not a fantastic experience when using the bindings as the arrays grow large, though - keeping things matched up isn't much fun especially if any of the elements in the array are optional.
Maybe one can use named properties in a new device node in that case, like this:
Quoted 16 lines
bus {
dma: dma-controller {
#dma-cells = <1>;
};
device {
compatible = "device";
channel: dma-channel {
type = <0x1>;
name = "foo";
number = <23>;
direction = <3>;
};
dma-requests = <&dma &channel>;
};
};For reference, this is very similar to how the pinctrl bindings work, except that they require the "channel" node to be a child of the DMA controller, and hence "dma-requests" doesn't contain <&dma &channel>, just <&channel>, since "dma" is the parent (or grand-parent) of "channel".
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 20 lines · Stephen Warren
Quoted 19 lines
Maybe one can use named properties in a new device node in that case, like this:
bus { dma: dma-controller { #dma-cells = <1>; }; device { compatible = "device"; channel: dma-channel { type = <0x1>; name = "foo"; number = <23>; direction = <3>; }; dma-requests = <&dma &channel>; }; };
Quoted 4 lines
For reference, this is very similar to how the pinctrl bindings work, except that they require the "channel" node to be a child of the DMA controller, and hence "dma-requests" doesn't contain <&dma &channel>, just <&channel>, since "dma" is the parent (or grand-parent) of "channel".
Right, but the difference beytween the pinctrl binding and what I describe here is that the channel description would be part of the dma engine driver specific binding, not the generic binding that is visible to device drivers.
As I said, one dmaengine could use a phandle while another one could just use a single number or nothing at all. If it chooses to use a phandle (or possible a combination of phandle and number), the binding could mandate for the device node to be either a child of the dma controller or the device using it. In either case it would be sensible to do it the same way for all dmaengine drivers that want to have a phandle in the argument.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 26 lines · Arnd Bergmann
Quoted 20 lines · Stephen Warren
Quoted 19 lines
Maybe one can use named properties in a new device node in that case, like this:
bus { dma: dma-controller { #dma-cells = <1>; }; device { compatible = "device"; channel: dma-channel { type = <0x1>; name = "foo"; number = <23>; direction = <3>; }; dma-requests = <&dma &channel>; }; };Quoted 4 lines
For reference, this is very similar to how the pinctrl bindings work, except that they require the "channel" node to be a child of the DMA controller, and hence "dma-requests" doesn't contain <&dma &channel>, just <&channel>, since "dma" is the parent (or grand-parent) of "channel".
Quoted 4 lines
Right, but the difference beytween the pinctrl binding and what I describe here is that the channel description would be part of the dma engine driver specific binding, not the generic binding that is visible to device drivers.
That's actually true for pinctrl as well: Common pinctrl bindings cover the content/format of "dma-requests" and a requirement for a "dma-channel" node, whereas the per-pin-controller bindings define the content of node "dma-channel".
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Arnd BergmannQuoted 49 lines · Arnd Bergmann
Quoted 10 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.Quoted 8 lines
One simple xlate function is provided for the "single number" type of request binding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.This looks very good. I missed the first version of this patch, but was thinking of very similar bindings.
Quoted 10 lines
Documentation/devicetree/bindings/dma/dma.txt new file +10
-0+Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. + +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property.Documentation/devicetree/bindings/dma/dma.txt line 45
Quoted 10 lines
Documentation/devicetree/bindings/dma/dma.txt new file +10
-0+ + +Example: + + i2c1: i2c at 1 { + ... + dma-request =<&sdma 2&sdma 3>; + dma-request-names = "tx", "rx"; + ... + };This is slightly different from how the proposed pwm binding works that Thierry is working on, which uses an arbitrary property name instead of requiring the use of a specific property but then allowing to give names in another property.
Quoted 5 lines
I don't care much which way it's done, but please try to agree on one approach that is used for both.
The one you have here is already used by reg and irq, while the other one is used in gpio.
This was done like IRQ on purpose, because an Interrupt ReQuest line and a DMA Request line are really similar for the HW point of view at IP level. I'm not sure what Thierry have done for pwm, but I thing that having the same scheme for reg, irq and dma was what we agreed with Grant during Plumbers.
GPIO scheme will be probably good enough as well, but the idea was to be consistent in the binding for similar information.
Regards,
Benoit
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 3 lines · Benoit
This was done like IRQ on purpose, because an Interrupt ReQuest line and a DMA Request line are really similar for the HW point of view at IP level.
I'm not sure about that at all levels. Sure, at hardware level they're the same, but I think the flat numeric namespace for IRQs has been proven to be a problem when there's multiple IRQ controllers in the system.
Quoted 3 lines
I'm not sure what Thierry have done for pwm, but I thing that having the same scheme for reg, irq and dma was what we agreed with Grant during Plumbers.
I really don't like these behind-the-scenes discussions which never then get followed up in public, and people then start quoting what was "agreed" as that's the way things must be done.
It's a bit like folk at the recent Linaro Connect apparantly discussing my machine registry and deciding that it should be shut down. No one has yet talked to me about that or even done the curtesy of asking me what my view is.
As far as I'm concerned for DMA stuff, there is currently no real solution for a DT representation; TI have asked me to take over the conversion of OMAP DMA support to the DMA engine API, and I'm not yet convinced that the existing numbering system is the right solution - especially as there's several overlapping numberspaces for OMAP DMA numbers which are SoC specific.
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 9 lines · Russell King - ARM Linux
Quoted 3 lines · Benoit
This was done like IRQ on purpose, because an Interrupt ReQuest line and a DMA Request line are really similar for the HW point of view at IP level.
I'm not sure about that at all levels. Sure, at hardware level they're the same, but I think the flat numeric namespace for IRQs has been proven to be a problem when there's multiple IRQ controllers in the system.
In the DT bindings, both IRQ and the suggested DMA are not flat number spaces, but instead can be of arbitrarly length defined by the controller.
Quoted 6 lines
As far as I'm concerned for DMA stuff, there is currently no real solution for a DT representation; TI have asked me to take over the conversion of OMAP DMA support to the DMA engine API, and I'm not yet convinced that the existing numbering system is the right solution - especially as there's several overlapping numberspaces for OMAP DMA numbers which are SoC specific.
The numbers definitely need to become local to each of the controllers, but that is the case pretty much automatically using the proposed binding, because each dma request identifier starts with the phandle of the controller.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 21 lines · Arnd Bergmann
Quoted 9 lines · Russell King - ARM Linux
Quoted 3 lines · Benoit
This was done like IRQ on purpose, because an Interrupt ReQuest line and a DMA Request line are really similar for the HW point of view at IP level.
I'm not sure about that at all levels. Sure, at hardware level they're the same, but I think the flat numeric namespace for IRQs has been proven to be a problem when there's multiple IRQ controllers in the system.
In the DT bindings, both IRQ and the suggested DMA are not flat number spaces, but instead can be of arbitrarly length defined by the controller.
Quoted 6 lines
As far as I'm concerned for DMA stuff, there is currently no real solution for a DT representation; TI have asked me to take over the conversion of OMAP DMA support to the DMA engine API, and I'm not yet convinced that the existing numbering system is the right solution - especially as there's several overlapping numberspaces for OMAP DMA numbers which are SoC specific.
Quoted 4 lines
The numbers definitely need to become local to each of the controllers, but that is the case pretty much automatically using the proposed binding, because each dma request identifier starts with the phandle of the controller.
Indeed, and in the case of the OMAP SDMA controller, it can handle up to 127 DMA request lines numbered from 0 to 126... So a local number seems to be a good representation... especially for a number. I'm not sure to understand the issue with this binding.
And AFAIK, there is the only one general purpose DMA controller in OMAP so far. The other ones are private to some IPs like MMC or USB, so they do not need necessarily need any DT representation.
But anyway, since the controller phandle is mandatory, it will be able to handle even several instances of this DMA controller without any issue.
Regards
Benoit
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 32 lines · Benoit
Quoted 21 lines · Arnd Bergmann
Quoted 9 lines · Russell King - ARM Linux
Quoted 3 lines · Benoit
This was done like IRQ on purpose, because an Interrupt ReQuest line and a DMA Request line are really similar for the HW point of view at IP level.
I'm not sure about that at all levels. Sure, at hardware level they're the same, but I think the flat numeric namespace for IRQs has been proven to be a problem when there's multiple IRQ controllers in the system.
In the DT bindings, both IRQ and the suggested DMA are not flat number spaces, but instead can be of arbitrarly length defined by the controller.
Quoted 6 lines
As far as I'm concerned for DMA stuff, there is currently no real solution for a DT representation; TI have asked me to take over the conversion of OMAP DMA support to the DMA engine API, and I'm not yet convinced that the existing numbering system is the right solution - especially as there's several overlapping numberspaces for OMAP DMA numbers which are SoC specific.
Quoted 4 lines
The numbers definitely need to become local to each of the controllers, but that is the case pretty much automatically using the proposed binding, because each dma request identifier starts with the phandle of the controller.
Indeed, and in the case of the OMAP SDMA controller, it can handle up to 127 DMA request lines numbered from 0 to 126... So a local number seems to be a good representation... especially for a number. I'm not sure to understand the issue with this binding.
Quoted 6 lines
And AFAIK, there is the only one general purpose DMA controller in OMAP so far. The other ones are private to some IPs like MMC or USB, so they do not need necessarily need any DT representation.
But anyway, since the controller phandle is mandatory, it will be able to handle even several instances of this DMA controller without any issue.
Thank you both for missing my point.
#define OMAP24XX_DMA_SHA1MD5_RX 13 /* S_DMA_12 */ #define OMAP34XX_DMA_SHA2MD5_RX 13 /* S_DMA_12 */
#define OMAP242X_DMA_EXT_DMAREQ2 14 /* S_DMA_13 */ #define OMAP243X_DMA_EXT_DMAREQ3 14 /* S_DMA_13 */
#define OMAP242X_DMA_EXT_DMAREQ3 15 /* S_DMA_14 */ #define OMAP24XX_DMA_SPI3_TX0 15 /* S_DMA_14 */
#define OMAP242X_DMA_EXT_DMAREQ4 16 /* S_DMA_15 */ #define OMAP24XX_DMA_SPI3_RX0 16 /* S_DMA_15 */
#define OMAP242X_DMA_EAC_BT_DL_RD 25 /* S_DMA_24 */ #define OMAP243X_DMA_EXT_DMAREQ4 25 /* S_DMA_24 */ #define OMAP34XX_DMA_I2C3_TX 25 /* S_DMA_24 */
Notice the overlap between the different SoCs for the same number on the same DMA controller.
This shouldn't cause problems when all users are within the SoC specific file, but those EXT ones would probably be platform specific, and so you immediately have a dependence between the platform and the SoC there.
That dependence can be completely eliminated by other matching schemes which are supportable via the DMA engine API.
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 21 lines · Russell King - ARM Linux
Thank you both for missing my point.
#define OMAP24XX_DMA_SHA1MD5_RX 13 /* S_DMA_12 */ #define OMAP34XX_DMA_SHA2MD5_RX 13 /* S_DMA_12 */#define OMAP242X_DMA_EXT_DMAREQ2 14 /* S_DMA_13 */ #define OMAP243X_DMA_EXT_DMAREQ3 14 /* S_DMA_13 */#define OMAP242X_DMA_EXT_DMAREQ3 15 /* S_DMA_14 */ #define OMAP24XX_DMA_SPI3_TX0 15 /* S_DMA_14 */#define OMAP242X_DMA_EXT_DMAREQ4 16 /* S_DMA_15 */ #define OMAP24XX_DMA_SPI3_RX0 16 /* S_DMA_15 */#define OMAP242X_DMA_EAC_BT_DL_RD 25 /* S_DMA_24 */ #define OMAP243X_DMA_EXT_DMAREQ4 25 /* S_DMA_24 */ #define OMAP34XX_DMA_I2C3_TX 25 /* S_DMA_24 */Notice the overlap between the different SoCs for the same number on the same DMA controller.
Quoted 6 lines
This shouldn't cause problems when all users are within the SoC specific file, but those EXT ones would probably be platform specific, and so you immediately have a dependence between the platform and the SoC there.
That dependence can be completely eliminated by other matching schemes which are supportable via the DMA engine API.
Ok, so I guess you are worried about the case where you have one .dtsi include file for the soc and another .dtsi file for the platform, and you want to generically encode e.g. DMA_EXT_DMAREQ3 in the platform file so you don't need to write a new .dts file for each combination of that platform wiht a different soc. Does that describe where you see the issue?
If so, I would recommend not using a flat numbering scheme for omap, but have something slightly more complex like a lot of the other dmagenine drivers do. This binding does not make any assumption about the meaning of the values, so you can do e.g. three cells for each channel with a definition like this:
Cell 1: channel class
Possible values are:
0 -- raw channel number
1 -- external channel
2 -- spi
3 -- i2c
4 -- mmc
to be extended when necessary
Cell 2: instance of this channel
When Cell 1 is zero, this is just the channel number local to the controller, for other values it defines the instance, e.g. ext0, ext1, ext2, ...
Cell 3: DMA direction
The following values are defined:
0 -- invalid
1 -- read
2 -- write
3 -- read/write
This specific binding for the omap dma would let you put either a simple channel into the device tree, or have a high-level description in there. On an OMAP243X, these two would have the same meaning and a user could put either one in there:
dma-request = <&dmaengine 0 14 1 /* physical channels 14 (read) */ &dmaengine 0 15 2>; /* and 15 (write) */ dma-request = <&dmaengine 1 3 1 /* external DMA 3 (read) and 4 (write) */ &dmaengine 1 4 2>;
The cell layout above is just a made-up example, you can basically put everything in there that you would put into the arguments to the dma match function. I believe this is not limited to numbers but can also include phandles and strings. The mapping from dma classes to physical channels could either be hardcoded in the source for the dmaengine driver, or get passed in properties of the dmaengine's device_node.
A completely different way to get around the same problem is to define a tree of virtual dmaengine device nodes, still within the same binding:
dmaengine-phys: dmaengine {
compatible = "ti,omap2430-dmaengine", "ti,omap2-dmaengine";
reg = <0x4a056000 0x1000>;
#address-cells = <1>; /* physical dma channel number space */
#size-cells = <0>;
#dma-cells = 1;
dmaengine-ext: dmaengine at 2 {
ranges = <0 2>, <1 3>, <2 7>, <3 14>, <4 25>,
<5 25> <6 64>;
#dma-cells = 1;
};
};This way, a device driver could either refer to the physical dmaengine device and pass
a physical number like
dma-requests = <&dmaengine-phys 14>;
or to the same effect refer to a child of node of that engine passing a local number like
dma-requests = <&dmaengine-ext 3>;
The dmaengine driver in this case can simply use of_translate_address() to get from channel 3 to 14 using the ranges in the child dmaengine device node.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Cousson, BenoitQuoted 6 lines · Benoit
Quoted 4 lines · Arnd Bergmann
The numbers definitely need to become local to each of the controllers, but that is the case pretty much automatically using the proposed binding, because each dma request identifier starts with the phandle of the controller.
Quoted 8 lines
Indeed, and in the case of the OMAP SDMA controller, it can handle up to 127 DMA request lines numbered from 0 to 126... So a local number seems to be a good representation... especially for a number. I'm not sure to understand the issue with this binding.
And AFAIK, there is the only one general purpose DMA controller in OMAP so far. The other ones are private to some IPs like MMC or USB, so they do not need necessarily need any DT representation.
AM335x has an EDMA controller instead of SDMA. This is the same EDMA found on mach-davinci/ parts.
-Matt
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Russell King - ARM LinuxQuoted 8 lines · Russell King - ARM Linux
I really don't like these behind-the-scenes discussions which never then get followed up in public, and people then start quoting what was "agreed" as that's the way things must be done.
It's a bit like folk at the recent Linaro Connect apparantly discussing my machine registry and deciding that it should be shut down. No one has yet talked to me about that or even done the curtesy of asking me what my view is.
Let me catch the ball in flight here as the responsible folk was me.
For the transition to DT to be effective for new boards on ARM, I suggested that the machine registry would need to be closed at this point i.e. no new additions to it. As far as I remember, Vincent volunteered to talk to you about it, which he apparently did within the same hour. That's all there was to it.
Nicolas
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Nicolas FerreQuoted 2 lines · Nicolas Ferre
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.
Can you explain which legacy scenarios you think this is going to help with?
Quoted 5 lines
drivers/of/dma.c new file +5
-0+/** +* of_dma_to_resource - Decode a node's DMA and return it as a resource +* @dev: pointer to device tree node +* @index: zero-based index of the DMA request +* @r: pointer to resource structure to return result into.
drivers/of/dma.c line 234
Quoted 10 lines
drivers/of/dma.c new file +10
-0+* +* Using a resource to export a DMA request number to a driver should +* be used only for legacy purpose and on system when only one DMA controller +* is present. +* The proper and only scalable way is to use the native of_get_dma_request API +* in order retrieve both the DMA controller device node and the DMA request +* line for that controller. +*/ +int of_dma_to_resource (struct device_node *dev,int index,struct resource *r) +{
I think a better way to discourage the use of IORESOURCE_DMA is to not provide this function at all, especially given that it's not really well-defined what it will do when you have more than one cell for the dma channel identifier or when you have multiple DMA engines, so any driver relying on it is inherently nonportable.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 5 lines · Arnd Bergmann
Quoted 2 lines · Nicolas Ferre
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.Can you explain which legacy scenarios you think this is going to help with?
We do have a bunch of drivers that are using IORESOURCE_DMA today to retrieve the DMA request. Since we do have an unique DMA controller it was easy and convenient.
And it seems that other ARM SoCs are using it for the same purpose. There are at least 230+ IORESOURCE_DMA instances in the kernel today.
Quoted 13 lines
Quoted 13 lines
+/**
+ *of_dma_to_resource- Decode a node's DMA and return it as a resource
+ * @dev: pointer to device tree node
+ * @index: zero-based index of the DMA request
+ * @r: pointer to resource structure to return result into.
+ *
+ * Using a resource to export a DMA request number to a driver should
+ * be used only for legacy purpose and on system when only one DMA controller
+ * is present.
+ * The proper and only scalable way is to use the nativeof_get_dma_requestAPI
+ * in order retrieve both the DMA controller device node and the DMA request
+ * line for that controller.
+ */
Quoted 8 lines
drivers/of/dma.c line 234
Quoted 2 lines
drivers/of/dma.c new file +2
-0+int of_dma_to_resource (struct device_node *dev,int index,struct resource *r) +{I think a better way to discourage the use of
IORESOURCE_DMAis to not provide this function at all, especially given that it's not really well-defined what it will do when you have more than one cell for the dma channel identifier or when you have multiple DMA engines, so any driver relying on it is inherently nonportable.
For the moment it is still used in a lot of places, and without any other better API yet, it is still useful. As written it is there only for simple single DMA controller case.
By maintaining that IORESOURCE_DMA for the moment we can adapt some driver to DT without having to change the way they are retrieving information. By using IORESOURCE_IRQ and IORESOURCE_MEM, we had the same advantage.
Otherwise how are we supposed to get the DMA channel for non-DT boot until we have migrated everything to DT? A bunch of ARM SoC are using IORESOURCE_DMA for the same purpose.
The goal here is really to maintain that during the transition phase only. As soon as the full DT support is there, we can switch to the of_ API.
Ideally, we should define and use a generic API non dependent of DT. AFAIK, that does not exist so far.
And since most drivers are not using dmaengine, we do not even have a common DMA fmwk to define an API on top.
I fully agree that this IORESOURCE_DMA is not scalable for multiple controller, ugly, and must be avoided like the plague. But what other options do we have during the transition?
Regards,
Benoit
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 2 lines · Benoit
And it seems that other ARM SoCs are using it for the same purpose. There are at least 230+
IORESOURCE_DMAinstances in the kernel today.
These tend to be the ones that don't use dmaengine but either the ISA dma api or a platform specific variant of that, right?
Also, I think that most of those definitions are for the same few devices. The number that I see is much lower:
$ git grep -l IORESOURCE_DMA drivers/ sound/ | wc -l
51Out of those, a quite number are mips or blackfin or xtensa based, or are for legacy ISA devices, which leaves 29 drivers for ARM.
Quoted 8 lines
For the moment it is still used in a lot of places, and without any other better API yet, it is still useful. As written it is there only for simple single DMA controller case.
By maintaining that
IORESOURCE_DMAfor the moment we can adapt some driver to DT without having to change the way they are retrieving information. By usingIORESOURCE_IRQandIORESOURCE_MEM, we had the same advantage.
The main difference to IORESOURCE_IRQ and IORESOURCE_MEM that I see is that those are going to start for any forseeable time and are actually helpful in a lot of ways. We are not going to remove the single number space for interrupts in the next few years. For DMA, the dmaengine API has already moved away from the flat number space of the ISA API.
Quoted 7 lines
Otherwise how are we supposed to get the DMA channel for non-DT boot until we have migrated everything to DT? A bunch of ARM SoC are using
IORESOURCE_DMAfor the same purpose.The goal here is really to maintain that during the transition phase only. As soon as the full DT support is there, we can switch to the
of_API.
Quoted 8 lines
Ideally, we should define and use a generic API non dependent of DT. AFAIK, that does not exist so far.
And since most drivers are not using dmaengine, we do not even have a common DMA fmwk to define an API on top.I fully agree that this
IORESOURCE_DMAis not scalable for multiple controller, ugly, and must be avoided like the plague. But what other options do we have during the transition?
We could use the same binding for the nonstandard controllers, but keep the dma channel number lookup separate for those, and let them call of_get_dma_request directly.
Since there are not too many drivers using those controllers with dma resources today, it's fairly easy to go through those as we write the driver bindings and just do
err = of_get_dma_request(pdev->dev.of_node, 0, &dma);
if (err) {
struct resource *r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
if (r)
dma = r->start;
}For the drivers that we convert to DT before we convert them to dmaengine, and not do anything if we convert them to dmaengine first.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 4 lines · Arnd Bergmann
Quoted 2 lines · Benoit
And it seems that other ARM SoCs are using it for the same purpose. There are at least 230+
IORESOURCE_DMAinstances in the kernel today.
Quoted 8 lines
These tend to be the ones that don't use dmaengine but either the ISA dma api or a platform specific variant of that, right?
Also, I think that most of those definitions are for the same few devices. The number that I see is much lower:
$ git grep -l IORESOURCE_DMA drivers/ sound/ | wc -l 51
Gosh, good point... I've just done a dumb grep, but most of them are in the device creation inside arch code:-(
Assuming OMAP driver does contains omap in the name, the result is indeed way smaller.
git grep -l IORESOURCE_DMA drivers/ sound/ | grep omap
drivers/crypto/omap-aes.cdrivers/crypto/omap-sham.cdrivers/spi/spi-omap2-mcspi.cdrivers/tty/serial/omap-serial.csound/soc/omap/omap-dmic.csound/soc/omap/omap-hdmi.c
We do have 127 DMA requests and only 6 drivers are using them, that's a pity...
Quoted 49 lines
Out of those, a quite number are mips or blackfin or xtensa based, or are for legacy ISA devices, which leaves 29 drivers for ARM.
Quoted 8 lines
For the moment it is still used in a lot of places, and without any other better API yet, it is still useful. As written it is there only for simple single DMA controller case.
By maintaining that
IORESOURCE_DMAfor the moment we can adapt some driver to DT without having to change the way they are retrieving information. By usingIORESOURCE_IRQandIORESOURCE_MEM, we had the same advantage.The main difference to
IORESOURCE_IRQandIORESOURCE_MEMthat I see is that those are going to start for any forseeable time and are actually helpful in a lot of ways. We are not going to remove the single number space for interrupts in the next few years. For DMA, the dmaengine API has already moved away from the flat number space of the ISA API.Quoted 7 lines
Otherwise how are we supposed to get the DMA channel for non-DT boot until we have migrated everything to DT? A bunch of ARM SoC are using
IORESOURCE_DMAfor the same purpose.The goal here is really to maintain that during the transition phase only. As soon as the full DT support is there, we can switch to the
of_API.Quoted 8 lines
Ideally, we should define and use a generic API non dependent of DT. AFAIK, that does not exist so far.
And since most drivers are not using dmaengine, we do not even have a common DMA fmwk to define an API on top.I fully agree that this
IORESOURCE_DMAis not scalable for multiple controller, ugly, and must be avoided like the plague. But what other options do we have during the transition?We could use the same binding for the nonstandard controllers, but keep the dma channel number lookup separate for those, and let them call
of_get_dma_requestdirectly.Since there are not too many drivers using those controllers with dma resources today, it's fairly easy to go through those as we write the driver bindings and just do
err = of_get_dma_request(pdev->dev.of_node, 0,&dma); if (err) { struct resource *r = platform_get_resource(pdev, IORESOURCE_DMA, 0); if (r) dma = r->start; }
Quoted 2 lines
For the drivers that we convert to DT before we convert them to dmaengine, and not do anything if we convert them to dmaengine first.
Considering the small amount of OMAP drivers really using IORESOURCE_DMA, I guess this is fair enough.
Bottom-line, I do not have any more issue removing this of_dma_to_resource.
Nico,
Is that OK for you to repost the patch without this function?
Thanks,
Benoit
PATCH 1/1 of: Add generic device tree DMA helpers
On 03/16/2012 02:28 PM, Cousson, Benoit :
Quoted 87 lines · Arnd Bergmann
Quoted 12 lines · Benoit
Quoted 2 lines
And it seems that other ARM SoCs are using it for the same purpose. There are at least 230+
IORESOURCE_DMAinstances in the kernel today.These tend to be the ones that don't use dmaengine but either the ISA dma api or a platform specific variant of that, right?
Also, I think that most of those definitions are for the same few devices. The number that I see is much lower:
$ git grep -l IORESOURCE_DMA drivers/ sound/ | wc -l 51Gosh, good point... I've just done a dumb grep, but most of them are in the device creation inside arch code:-(
Assuming OMAP driver does contains omap in the name, the result is indeed way smaller.
git grep -l
IORESOURCE_DMAdrivers/sound/| grep omap
drivers/crypto/omap-aes.cdrivers/crypto/omap-sham.cdrivers/spi/spi-omap2-mcspi.cdrivers/tty/serial/omap-serial.csound/soc/omap/omap-dmic.csound/soc/omap/omap-hdmi.cWe do have 127 DMA requests and only 6 drivers are using them, that's a pity...
Quoted 51 lines
Out of those, a quite number are mips or blackfin or xtensa based, or are for legacy ISA devices, which leaves 29 drivers for ARM.
Quoted 8 lines
For the moment it is still used in a lot of places, and without any other better API yet, it is still useful. As written it is there only for simple single DMA controller case.
By maintaining that
IORESOURCE_DMAfor the moment we can adapt some driver to DT without having to change the way they are retrieving information. By usingIORESOURCE_IRQandIORESOURCE_MEM, we had the same advantage.The main difference to
IORESOURCE_IRQandIORESOURCE_MEMthat I see is that those are going to start for any forseeable time and are actually helpful in a lot of ways. We are not going to remove the single number space for interrupts in the next few years. For DMA, the dmaengine API has already moved away from the flat number space of the ISA API.Quoted 8 lines
Otherwise how are we supposed to get the DMA channel for non-DT boot until we have migrated everything to DT? A bunch of ARM SoC are using
IORESOURCE_DMAfor the same purpose.The goal here is really to maintain that during the transition phase only. As soon as the full DT support is there, we can switch to the
of_API.Quoted 8 lines
Ideally, we should define and use a generic API non dependent of DT. AFAIK, that does not exist so far.
And since most drivers are not using dmaengine, we do not even have a common DMA fmwk to define an API on top.I fully agree that this
IORESOURCE_DMAis not scalable for multiple controller, ugly, and must be avoided like the plague. But what other options do we have during the transition?We could use the same binding for the nonstandard controllers, but keep the dma channel number lookup separate for those, and let them call
of_get_dma_requestdirectly.Since there are not too many drivers using those controllers with dma resources today, it's fairly easy to go through those as we write the driver bindings and just do
err = of_get_dma_request(pdev->dev.of_node, 0,&dma); if (err) { struct resource *r = platform_get_resource(pdev, IORESOURCE_DMA, 0); if (r) dma = r->start; }Quoted 3 lines
For the drivers that we convert to DT before we convert them to dmaengine,
and not do anything if we convert them to dmaengine first.
Quoted 8 lines
Considering the small amount of OMAP drivers really using
IORESOURCE_DMA, I guess this is fair enough.Bottom-line, I do not have any more issue removing this
of_dma_to_resource.Nico,
Is that OK for you to repost the patch without this function?
Yes sure, I will repost a V2 soon.
I also try to have an integration in DT selftest.c:
- that will show some possibilities of the API
- that will give some examples of usage (1 or 2 xlate() functions)
- that made me review my code and find a weakness
- that will allow to monitor non-regression (obviously) => WIP...
Bye,
Signature · 1 line
Nicolas Ferre
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Nicolas FerreQuoted 13 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request binding.
Quoted 8 lines
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.Signed-off-by Nicolas Ferre <[email protected]>Signed-off-by Benoit Cousson <[email protected]>
Hi Nicolas,
Comments below.
Quoted 71 lines
Cc Stephen Warren <[email protected]>Cc Grant Likely <[email protected]>Cc Russell King <[email protected]>Cc Rob Herring <[email protected]>--- Documentation/devicetree/bindings/dma/dma.txt | 45 +++++ drivers/of/Kconfig | 5 + drivers/of/Makefile | 1 + drivers/of/dma.c | 260 +++++++++++++++++++++++++ include/linux/of_dma.h | 67 +++++++ 5 files changed, 378 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/dma.txt create mode 100644 drivers/of/dma.c create mode 100644 include/linux/of_dma.hDocumentation/devicetree/bindings/dma/dma.txt new file +45
-0@@ -0,0 +1,45 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller + +Required property: + - #dma-cells: Number of cells for each DMA line. + + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg = <0x48000000 0x1000>; + interrupts = <12>; + #dma-cells = <1>; + }; + + + +* DMA client + +Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. + +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property. + + +Example: + + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/Kconfig +0
-0
drivers/of/Kconfig config OF_PCI_IRQ · line 96
Quoted 7 lines
drivers/of/Kconfig +4
-0@@ -90,4 +90,9 @@ config OF_PCI_IRQ help OpenFirmware PCI IRQ routing helpers +config OF_DMA + def_bool y + help + Device Tree DMA routing helpers
Not really any point in having this config symbol at all if it is always enabled.
Quoted 35 lines
drivers/of/Kconfig +1
-0+ endmenu # OFdrivers/of/Makefile +1
-0@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SELFTEST) += selftest.o obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o +obj-$(CONFIG_OF_DMA) += dma.odrivers/of/dma.c new file +18
-0@@ -0,0 +1,260 @@ +/* +* Device tree helpers for DMA request / controller +* +* Based on of_gpio.c +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/rculist.h> +#include <linux/slab.h> +#include <linux/of.h>
drivers/of/dma.c line 28
Quoted 10 lines
drivers/of/dma.c new file +10
-0+#include <linux/of_dma.h> + +static LIST_HEAD (of_dma_list); + +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};
This _xlate is nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.
The void *data pointer must be replaced with a typed structure so that context can be returned.
Quoted 45 lines
drivers/of/dma.c new file +45
-0+ +/** +* of_dma_find_controller() - Find a DMA controller in DT DMA helpers list +* @np: device node of DMA controller +*/ +static struct of_dma *of_dma_find_controller (struct device_node *np) +{ +struct of_dma *ofdma; + +list_for_each_entry_rcu (ofdma, &of_dma_list, of_dma_controllers) { +if (ofdma->of_node == np) +return ofdma; + } + +return NULL ; +} + +/** +* of_dma_controller_register() - Register a DMA controller to DT DMA helpers +* @np: device node of DMA controller +* @of_dma_xlate: generic translation function which converts a phandle +* arguments list into a generic output value +* +* Returns 0 on success or appropriate errno value on error. +* +* If #dma-cells is not specified in DMA controller device tree node, we assume +* that the DMA controller phandle will come without argument. +* +* Allocated memory should be freed with appropriate of_dma_controller_free() +* call. +*/ +int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +struct of_dma *ofdma; +const __be32 *nbcells; + +if (!np || !of_dma_xlate) { +pr_err ("%s: not enough information provided\n" , __func__); +return -EINVAL ; + } + + ofdma =kzalloc (sizeof (*ofdma),GFP_KERNEL ); +if (!ofdma) +return -ENOMEM ;
drivers/of/dma.c line 83
Quoted 10 lines
drivers/of/dma.c new file +10
-0+ + nbcells =of_get_property (np,"#dma-cells" ,NULL ); +if (!nbcells) +/* +* no #dma-cells properties: assume no argument to +* dma-request property on slave side +*/ + ofdma->of_dma_n_cells =0 ; +else + ofdma->of_dma_n_cells =be32_to_cpup (nbcells);
Make #dma-cells a required property
Quoted 88 lines
drivers/of/dma.c new file +88
-0+ + ofdma->of_node = np; + ofdma->of_dma_xlate = of_dma_xlate; + +/* Now queue of_dma controller structure in list */ +list_add_tail_rcu (&ofdma->of_dma_controllers, &of_dma_list); + +return 0 ; +} +EXPORT_SYMBOL_GPL (of_dma_controller_register); + +/** +* of_dma_controller_free() - Remove a DMA controller from DT DMA helpers list +* @np: device node of DMA controller +* +* Memory allocated by of_dma_controller_register() is freed here. +*/ +void of_dma_controller_free (struct device_node *np) +{ +struct of_dma *ofdma; + + ofdma =of_dma_find_controller (np); +if (ofdma) { +list_del_rcu (&ofdma->of_dma_controllers); +kfree (ofdma); + } +} +EXPORT_SYMBOL_GPL (of_dma_controller_free); + +/** +* of_get_dma_request() - Get the associated DMA request data +* @np: device node to get DMA request from +* @index: index of the DMA request +* @out_data: a output that can be filled in by the of_dma_xlate() function +* +* Returns return value of of_dma_xlate() and fills out_data (if provided). +* On error returns the appropriate errno value. +*/ +int of_get_dma_request (struct device_node *np,int index, +void *out_data) +{ +struct of_phandle_args dma_spec; +struct of_dma *ofdma; +int ret; + + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index, &dma_spec); +if (ret) { +pr_err ("%s: can't parse dma property\n" , np->full_name); + goto err0; + } + +if (list_empty (&of_dma_list)) { +pr_debug ("%s: empty DMA controller list\n" , + np->full_name); + ret = -ENOENT ; + goto err1; + } + + ofdma =of_dma_find_controller (dma_spec.np); +if (!ofdma) { +pr_debug ("%s: DMA controller %s isn't registered\n" , + np->full_name, dma_spec.np->full_name); + ret = -ENODEV ; + goto err1; + } + +if (dma_spec.args_count != ofdma->of_dma_n_cells) { +pr_debug ("%s: wrong #dma-cells for %s\n" , + np->full_name, dma_spec.np->full_name); + ret = -EINVAL ; + goto err1; + } + + ret = ofdma->of_dma_xlate (&dma_spec, out_data); + +err1: +of_node_put (dma_spec.np); +err0: +pr_debug ("%s exited with status %d\n" , __func__, ret); +return ret; +} +EXPORT_SYMBOL_GPL (of_get_dma_request); + +/** +* of_dma_xlate_onenumbercell() - Generic DMA xlate for direct one cell bindings +* +* Device Tree DMA translation function which works with one cell bindings
drivers/of/dma.c line 181
Quoted 10 lines
drivers/of/dma.c new file +10
-0+* where the cell values map directly to the hardware request number understood +* by the DMA controller. +*/ +int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec,void *dma_req) +{ +if (!dma_spec) +return -EINVAL ; +if (WARN_ON (dma_spec->args_count !=1 )) +return -EINVAL ; + *(int *)dma_req = dma_spec->args[0 ];
Following on from comment above; the void *dma_req parameter is dangerous. How does this function know that it has been passed an int* pointer?
Quoted 65 lines
drivers/of/dma.c new file +65
-0+return 0 ; +} +EXPORT_SYMBOL_GPL (of_dma_xlate_onenumbercell); + +/** +* of_dma_count - Count DMA requests for a device +* @np: device node to count DMAs for +* +* The function returns the count of DMA requests specified for a node. +* +* Note that the empty DMA specifiers counts too. For example, +* +* dma-request = <0 +* &sdma 1 +* 0 +* &sdma 3>; +* +* defines four DMAs (so this function will return 4), two of which +* are not specified. +*/ +unsigned int of_dma_count (struct device_node *np) +{ +unsigned int cnt =0 ; + +do { +int ret; + + ret =of_parse_phandle_with_args (np,"dma-request" , +"#dma-cells" , cnt,NULL ); +/* A hole in the dma-request = <> counts anyway. */ +if (ret <0 && ret != -EEXIST ) +break ; + }while (++cnt); + +return cnt; +} +EXPORT_SYMBOL_GPL (of_dma_count); + +/** +* of_dma_to_resource - Decode a node's DMA and return it as a resource +* @dev: pointer to device tree node +* @index: zero-based index of the DMA request +* @r: pointer to resource structure to return result into. +* +* Using a resource to export a DMA request number to a driver should +* be used only for legacy purpose and on system when only one DMA controller +* is present. +* The proper and only scalable way is to use the native of_get_dma_request API +* in order retrieve both the DMA controller device node and the DMA request +* line for that controller. +*/ +int of_dma_to_resource (struct device_node *dev,int index,struct resource *r) +{ +const char *name =NULL ; +int dma; +int ret; + +if (!r) +return -EINVAL ; + + ret =of_get_dma_request (dev, index, &dma); +if (ret <0 ) +return ret; + +/*
drivers/of/dma.c line 256
Quoted 10 lines
drivers/of/dma.c new file +10
-0+* Get optional "dma-request-names" property to add a name +* to the resource. +*/ +of_property_read_string_index (dev,"dma-request-names" , index, + &name); + + r->start = dma; + r->end = dma; + r->flags =IORESOURCE_DMA ; + r->name = name ? name : dev->full_name;
I'll reiterate my concern here. The dual meaning of r->name depending on the presence of a dma-request-names property is a real problem. I know this patch set hasn't introduced the problem since it already exists for register regions and irqs, but it is expanding it.
Quoted 85 lines
drivers/of/dma.c new file +4
-0+ +return dma; +} +EXPORT_SYMBOL_GPL (of_dma_to_resource);include/linux/of_dma.h new file +67
-0@@ -0,0 +1,67 @@ +/* +* OF helpers for DMA request / controller +* +* Based on of_gpio.h +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#ifndef __LINUX_OF_DMA_H +#define __LINUX_OF_DMA_H + +#include <linux/of.h> + +struct device_node ; + +#ifdef CONFIG_OF_DMA + +extern int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)); +extern void of_dma_controller_free (struct device_node *np); +extern int of_get_dma_request (struct device_node *np,int index, +void *out_data); +extern unsigned int of_dma_count (struct device_node *np); +extern int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); + +extern int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *dma_req); +#else /* CONFIG_OF_DMA */ + +static int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +return -ENOSYS ; +} + +static void of_dma_controller_free (struct device_node *np) {} + +extern int of_get_dma_request (struct device_node *np,int index, +void *out_data) +{ +return -ENOSYS ; +} + +static inline unsigned int of_dma_count (struct device_node *np) +{ +return 0 ; +} + +static int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); +{ +return -ENOSYS ; +} + +static int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *dma_req) +{ +return -ENOSYS ; +} +#endif /* CONFIG_OF_DMA */ + +#endif /* __LINUX_OF_DMA_H */ Signature · 7 lines
1.7.9 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 2 lines
Grant Likely, B.Sc, P.Eng. Secret Lab Technologies,Ltd.
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 14 lines · Grant Likely
drivers/of/dma.c line 28
Quoted 8 lines
drivers/of/dma.c new file +8
-0+static LIST_HEAD (of_dma_list); + +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};This
_xlateis nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.
Quoted 2 lines
The void *data pointer must be replaced with a typed structure so that context can be returned.
I've read up a bit more on how the existing drivers use the filter functions, it seems there are multiple classes of them, the classes that I've encountered are:
1. matches on chan->device pointer and/or chan_id
(8 drivers)
2. will match anything
(6 drivers)
3. requires specific dma engine driver, then behaves like 1 or 2 (8 drivers, almost all freescale)
4. one of a kind, matches resource name string or device->dev_id (two drivers)
5. filter function and data both provided by platform code, platform picks dmaengine driver.
(4 amba pl* drivers, used on ARM, ux500, ...)
The last category is interesting because here, the dmaengine driver (pl330, coh901318, sirf-dma, ste_dma40) provides the filter function while in the other cases that is provided by the device driver! Out of these, the ste_dma40 is special because it's the only one where the data is a complex data structure describing the constraints on the driver, while all others just find the right channel.
Some drivers also pass assign driver specific data to chan->private.
I would hope that we can all make them use something like
struct dma_channel *of_dma_request_channel(struct of_node*,
int index, void *driver_data);
with an appropriate common definition behind it. In the cases where the driver can just match anything, I'd assume that all channels are equal, so #dma-cells would be 0. For the ste_dma40, #dma-cells needs to cover all of stedma40_chan_cfg. In most other cases, #dma-cells can be 1 and just enumerate the channels, unless we want to simplify the cases that Russell mentioned where we want to keep a two stage mapping channel identifiers and physical channel numbers.
How about an implementation like this:?
typedef bool dma_filter_simple(struct dma_chan *chan, void *filter_param)
{
/* zero #dma-cells, accept anything */
return true;
}struct dma_channel *of_dma_request_channel(struct of_node*, int index,
dma_cap_mask_t *mask, void *driver_data)
{
struct of_phandle_args dma_spec;
struct dma_device *device;
struct dma_chan *chan = NULL;
dma_filter_fn *filter;
ret = of_parse_phandle_with_args(np, "dma-request", "#dma-cells",
index, &dma_spec);
device = dma_find_device(dma_spec->np);
if (!device)
goto out;
if (dma_spec->args_count == 0)
filter = dma_filter_simple;
else
filter = device->dma_dt_filter; /* new member */
chan = private_candidate(mask, device, filter, dma_spec->args);
if (chan && !chan->private)
chan->private = driver_data;
out:
return chan;
}Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 42 lines · Arnd Bergmann
Quoted 14 lines · Grant Likely
drivers/of/dma.c line 28
Quoted 8 lines
drivers/of/dma.c new file +8
-0+static LIST_HEAD (of_dma_list); + +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};This
_xlateis nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.Quoted 2 lines
The void *data pointer must be replaced with a typed structure so that context can be returned.
I've read up a bit more on how the existing drivers use the filter functions, it seems there are multiple classes of them, the classes that I've encountered are:
1. matches on
chan->devicepointer and/orchan_id
(8 drivers)
2. will match anything
(6 drivers)
3. requires specific dma engine driver, then behaves like 1 or 2 (8 drivers, almost all freescale)
4. one of a kind, matches resource name string ordevice->dev_id(two drivers)
5. filter function and data both provided by platform code, platform picks dmaengine driver.
(4 amba pl* drivers, used on ARM, ux500, ...)The last category is interesting because here, the dmaengine driver (pl330, coh901318, sirf-dma,
ste_dma40) provides the filter function while in the other cases that is provided by the device driver! Out of these, theste_dma40is special because it's the only one where the data is a complex data structure describing the constraints on the driver, while all others just find the right channel.
Quoted 5 lines
Some drivers also pass assign driver specific data to
chan->private.I would hope that we can all make them use something like
structdma_channel*of_dma_request_channel(struct of_node*,
int index, void*driver_data);
certainly my hope too!
Quoted 20 lines
with an appropriate common definition behind it. In the cases where the driver can just match anything, I'd assume that all channels are equal, so #dma-cells would be 0. For the
ste_dma40, #dma-cells needs to cover all ofstedma40_chan_cfg. In most other cases, #dma-cells can be 1 and just enumerate the channels, unless we want to simplify the cases that Russell mentioned where we want to keep a two stage mapping channel identifiers and physical channel numbers.How about an implementation like this:?
typedef bool
dma_filter_simple(struct dma_chan *chan, void *filter_param){ /* zero #dma-cells, accept anything */ return true; }struct
dma_channel*of_dma_request_channel(struct of_node*, int index,
dma_cap_mask_t*mask, void*driver_data)
Quoted 10 lines
{
struct of_phandle_args dma_spec;
struct dma_device *device;
struct dma_chan *chan = NULL;
dma_filter_fn *filter;
ret = of_parse_phandle_with_args(np, "dma-request", "#dma-cells",
index, &dma_spec);
device = dma_find_device(dma_spec->np);Is dma_find_device() a new function? How does it look up the dma device?
Quoted 2 lines
if (!device) goto out;
Just return NULL here.
Quoted 5 lines
if (dma_spec->args_count == 0) filter = dma_filter_simple; else filter = device->dma_dt_filter; /* new member */
I'm not thrilled with this if/else hunk; even the case of #dma-cells=<0> should provide a hook; even it if is the stock simple filter. Leaving filter as NULL is the same as accepting everything anyway.
Quoted 7 lines
chan = private_candidate(mask, device, filter, dma_spec->args); if (chan && !chan->private) chan->private = driver_data; out: return chan;
I think this looks right, except for the comments above.
g.
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 9 lines · Grant Likely
Quoted 9 lines
struct
dma_channel*of_dma_request_channel(struct of_node*, int index,
dma_cap_mask_t*mask, void*driver_data){ struct of_phandle_args dma_spec; struct dma_device *device; struct dma_chan *chan = NULL; dma_filter_fn *filter;
Quoted 7 lines
Quoted 4 lines
ret = of_parse_phandle_with_args(np, "dma-request", "#dma-cells", index, &dma_spec); device = dma_find_device(dma_spec->np);Is
dma_find_device()a new function? How does it look up the dma device?
Yes, it would be similar to the proposed function in Benoit's patch
Quoted 10 lines
Quoted 5 lines
if (dma_spec->args_count == 0) filter = dma_filter_simple; else filter = device->dma_dt_filter; /* new member */I'm not thrilled with this if/else hunk; even the case of #dma-cells=<0> should provide a hook; even it if is the stock simple filter. Leaving filter as NULL is the same as accepting everything anyway.
Right, good point. So a dmaengine driver would either register a trivial filter if it wants to do anything here, or it would just leave it as a NULL pointer otherwise.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 6 lines · Arnd Bergmann
Quoted 3 lines
Is
dma_find_device()a new function? How does it look up the dma device?Yes, it would be similar to the proposed function in Benoit's patch
Well, actually we would not even need a new list with all the devices, it can simply become
/* must be called with dma_list_mutex held */
static struct dma_device *dma_find_device(struct of_node *node)
{
struct dma_device *device;
list_for_each_entry(device, &dma_device_list, global_node) {
if (device->dev.of_node == node)
break;
}
return device;
}Given that dma_device_list is most likely be a very short list, this will be a fast operation.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Arnd BergmannOn 03/18/2012 09:13 PM, Arnd Bergmann :
Quoted 15 lines · Grant Likely
Quoted 14 lines
drivers/of/dma.c line 28
Quoted 8 lines
drivers/of/dma.c new file +8
-0+static LIST_HEAD (of_dma_list); + +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};This
_xlateis nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.
Quoted 8 lines
Quoted 2 lines
The void *data pointer must be replaced with a typed structure so that context can be returned.
I've read up a bit more on how the existing drivers use the filter functions, it seems there are multiple classes of them, the classes that I've encountered are:
1. matches on
chan->devicepointer and/orchan_id
I have the impression that we are now talking about *channel* selection. It is not the purpose of those helper functions. It is just to retrieve a *request line* for a particular slave interface. The selection/filtering of a channel is a different topic (that we can address in the future).
Maybe some DMA controllers are able to handle several "request lines" among several DMA controller instances. But I suspect that this case should be handled in another place, before calling this API.
Quoted 44 lines
(8 drivers)2. will match anything
(6 drivers)
3. requires specific dma engine driver, then behaves like 1 or 2 (8 drivers, almost all freescale)
4. one of a kind, matches resource name string ordevice->dev_id(two drivers)
5. filter function and data both provided by platform code, platform picks dmaengine driver.
(4 amba pl* drivers, used on ARM, ux500, ...)The last category is interesting because here, the dmaengine driver (pl330, coh901318, sirf-dma,
ste_dma40) provides the filter function while in the other cases that is provided by the device driver! Out of these, theste_dma40is special because it's the only one where the data is a complex data structure describing the constraints on the driver, while all others just find the right channel.Some drivers also pass assign driver specific data to
chan->private.I would hope that we can all make them use something like
structdma_channel*of_dma_request_channel(struct of_node*,
int index, void*driver_data);
with an appropriate common definition behind it. In the cases where the driver can just match anything, I'd assume that all channels are equal, so #dma-cells would be 0. For theste_dma40, #dma-cells needs to cover all ofstedma40_chan_cfg. In most other cases, #dma-cells can be 1 and just enumerate the channels, unless we want to simplify the cases that Russell mentioned where we want to keep a two stage mapping channel identifiers and physical channel numbers.How about an implementation like this:?
typedef bool
dma_filter_simple(struct dma_chan *chan, void *filter_param){ /* zero #dma-cells, accept anything */ return true; }struct
dma_channel*of_dma_request_channel(struct of_node*, int index,
dma_cap_mask_t*mask, void*driver_data)
Quoted 8 lines
{
struct of_phandle_args dma_spec;
struct dma_device *device;
struct dma_chan *chan = NULL;
dma_filter_fn *filter;
ret = of_parse_phandle_with_args(np, "dma-request", "#dma-cells",
index, &dma_spec);Well, this property handles "request lines" not "channels". So if we move the selection/filtering of channels in DT, we may need to create a new property of this purpose...
Quoted 20 lines
device = dma_find_device(dma_spec->np); if (!device) goto out; if (dma_spec->args_count == 0) filter = dma_filter_simple; else filter = device->dma_dt_filter; /* new member */ chan = private_candidate(mask, device, filter, dma_spec->args); if (chan && !chan->private) chan->private = driver_data; out: return chan; }Arnd
Signature · 1 line
Nicolas Ferre
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 23 lines · Nicolas Ferre
On 03/18/2012 09:13 PM, Arnd Bergmann :
Quoted 22 lines · Grant Likely
Quoted 14 lines
drivers/of/dma.c line 28
Quoted 8 lines
drivers/of/dma.c new file +8
-0+static LIST_HEAD (of_dma_list); + +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};This
_xlateis nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.Quoted 2 lines
The void *data pointer must be replaced with a typed structure so that context can be returned.
I've read up a bit more on how the existing drivers use the filter functions, it seems there are multiple classes of them, the classes that I've encountered are:
Quoted 5 lines
Quoted 1 line
1. matches on
chan->devicepointer and/orchan_idI have the impression that we are now talking about *channel* selection. It is not the purpose of those helper functions. It is just to retrieve a *request line* for a particular slave interface.
The request line on what though? The DMA controller, or something else?
The reason I ask is because on ARM boards, the DMA controller has N request lines. Three of those request lines are multiplexed in an external FPGA to select between N different peripherals.
For example, the UART and MMCI may be routed to the FPGA multiplexer, and either could be routed to DMA controller request signal 0, 1, or 2, whereas a different UART might be routed directly to DMA controller request signal 3 and 4.
At the moment, we handle this via platform data and callbacks passed into the PL080 DMA driver, by matching virtual DMA channels based upon strings looked up in the platform data. This gives the range of DMA request signal numbers on the controller, and calls a platform provided function to setup the multiplexer.
Clearly, if your proposal is all about DMA controller request signals only, the above scheme can't be represented in DT... and that's what worries me - the fact is that the DMA engine API can and does cope with this but it seems that we're building restrictions in with DT on what can be represented.
That makes me wonder whether the model that's being chosen is anywhere near correct for the DMA engine API.
Now, as I've said before, I'm trying to work on creating an OMAP DMA engine implementation, and I'd _really_ like to have the freedom to do what's necessary there without having to think one bit about DT getting in the way. To put it another way, I don't want to be constrained by any weirdo DT representations at the moment, and I really don't want to waste time discussing them at the moment, rather than getting on with that job. Because if I end up discussing this at length, I'm not going to be able to do anything on the OMAP DMA engine stuff.
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 24 lines · Russell King - ARM Linux
Quoted 23 lines · Nicolas Ferre
On 03/18/2012 09:13 PM, Arnd Bergmann :
Quoted 22 lines · Grant Likely
Quoted 14 lines
drivers/of/dma.c line 28
Quoted 8 lines
drivers/of/dma.c new file +8
-0+static LIST_HEAD (of_dma_list); + +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};This
_xlateis nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.Quoted 2 lines
The void *data pointer must be replaced with a typed structure so that context can be returned.
I've read up a bit more on how the existing drivers use the filter functions, it seems there are multiple classes of them, the classes that I've encountered are:
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
1. matches on
chan->devicepointer and/orchan_idI have the impression that we are now talking about *channel* selection. It is not the purpose of those helper functions. It is just to retrieve a *request line* for a particular slave interface.
The request line on what though? The DMA controller, or something else?
The idea is just to represent the request line from the IP to the DMA controller. This is exactly the same pattern than for the IRQ line toward the IRQ controller.
Quoted 9 lines
The reason I ask is because on ARM boards, the DMA controller has N request lines. Three of those request lines are multiplexed in an external FPGA to select between N different peripherals.
For example, the UART and MMCI may be routed to the FPGA multiplexer, and either could be routed to DMA controller request signal 0, 1, or 2, whereas a different UART might be routed directly to DMA controller request signal 3 and 4.
Quoted 8 lines
At the moment, we handle this via platform data and callbacks passed into the PL080 DMA driver, by matching virtual DMA channels based upon strings looked up in the platform data. This gives the range of DMA request signal numbers on the controller, and calls a platform provided function to setup the multiplexer.
Clearly, if your proposal is all about DMA controller request signals only, the above scheme can't be represented in DT...
Yes, this is the goal. I guess in your case you do need some DMA nexus node like DT is already providing for the IRQ. But you still should be able to describe how the HW blocks are interconnected.
Quoted 4 lines
and that's what
worries me - the fact is that the DMA engine API can and does cope with this but it seems that we're building restrictions in with DT on what can be represented.
But then you are mixing the SW representation and the HW one. DMAengine is the Linux way of managing DMA channel. DT is supposed to be OS agnostic and just describe the HW.
So you cannot take into account some DMAengine specificity to define the DT binding. At least in theory.
Quoted 2 lines
That makes me wonder whether the model that's being chosen is anywhere near correct for the DMA engine API.
That's not the goal. The goal is to describe how the DMA request lines are connected from the IP to the controller.
Quoted 8 lines
Now, as I've said before, I'm trying to work on creating an OMAP DMA engine implementation, and I'd _really_ like to have the freedom to do what's necessary there without having to think one bit about DT getting in the way. To put it another way, I don't want to be constrained by any weirdo DT representations at the moment, and I really don't want to waste time discussing them at the moment, rather than getting on with that job. Because if I end up discussing this at length, I'm not going to be able to do anything on the OMAP DMA engine stuff.
The current way of representing the DMA requests for OMAP SDMA is pretty straightforward and fit well with the current binding. It looks to me that this is the ARM boards that will be tricky to handle.
Regards,
Benoit
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Grant LikelyOn 03/17/2012 10:40 AM, Grant Likely :
Quoted 101 lines · Nicolas Ferre
Quoted 13 lines
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request binding.
Quoted 8 lines
This implementation is independent from dmaengine so it can also be used by legacy drivers.
For legacy reason another API will export the DMA request number into a Linux resource of type
IORESOURCE_DMA.Signed-off-by Nicolas Ferre <[email protected]>Signed-off-by Benoit Cousson <[email protected]>Hi Nicolas,
Comments below.
Quoted 74 lines
Cc Stephen Warren <[email protected]>Cc Grant Likely <[email protected]>Cc Russell King <[email protected]>Cc Rob Herring <[email protected]>--- Documentation/devicetree/bindings/dma/dma.txt | 45 +++++ drivers/of/Kconfig | 5 + drivers/of/Makefile | 1 + drivers/of/dma.c | 260 +++++++++++++++++++++++++ include/linux/of_dma.h | 67 +++++++ 5 files changed, 378 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/dma.txt create mode 100644 drivers/of/dma.c create mode 100644 include/linux/of_dma.hDocumentation/devicetree/bindings/dma/dma.txt new file +45
-0@@ -0,0 +1,45 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller + +Required property: + - #dma-cells: Number of cells for each DMA line. + + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg = <0x48000000 0x1000>; + interrupts = <12>; + #dma-cells = <1>; + }; + + + +* DMA client + +Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. + +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property. + + +Example: + + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };drivers/of/Kconfig +0
-0@@ -90,4 +90,9 @@ config OF_PCI_IRQ help OpenFirmware PCI IRQ routing helpers
Quoted 7 lines
drivers/of/Kconfig config OF_PCI_IRQ · line 96
Quoted 4 lines
drivers/of/Kconfig +4
-0+config OF_DMA + def_bool y + help + Device Tree DMA routing helpersNot really any point in having this config symbol at all if it is always enabled.
Well, it is the same for:
config OF_DEVICE
But for sure, I can remove it: so I will add it to obj-y = base.o <here>
in the Makefile.
Quoted 41 lines
Quoted 41 lines
drivers/of/Kconfig +0
-0endmenu # OFdrivers/of/Makefile +1
-0@@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SELFTEST) += selftest.o obj-$(CONFIG_OF_MDIO) += of_mdio.o obj-$(CONFIG_OF_PCI) += of_pci.o obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o +obj-$(CONFIG_OF_DMA) += dma.odrivers/of/dma.c new file +25
-0@@ -0,0 +1,260 @@ +/* +* Device tree helpers for DMA request / controller +* +* Based on of_gpio.c +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/rculist.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <linux/of_dma.h> + +static LIST_HEAD (of_dma_list); + +struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node;
Quoted 8 lines
drivers/of/dma.c line 28
Quoted 3 lines
drivers/of/dma.c new file +3
-0+int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};This
_xlateis nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.
You mean, if there is no DMA controller phandle specified in the property? I think that it is not the purpose of this API to choose a DMA controller, Nor to provide a channel. The only purpose of this API is to give a HW request to be used by a DMA slave driver. This slave should already have a channel to use and a controller to talk to.
Quoted 2 lines
The void *data pointer must be replaced with a typed structure so that context can be returned.
I am not sure to follow you entirely... How do I address the fact that several types of request value can be returned then?
BTW, can we imagine a phandle property with a sting as a argument? should it be written this way?
dma-request = <&testdmac1>, "slave-rx", "slave-tx";
If yes, the of_parse_phandle_with_args() is not working on this type...
(I realize that there seems to be no way out for a generic API: maybe we should move to one or two cases to address and concentrate on them).
Quoted 49 lines
Quoted 49 lines
drivers/of/dma.c new file +49
-0+ +/** +* of_dma_find_controller() - Find a DMA controller in DT DMA helpers list +* @np: device node of DMA controller +*/ +static struct of_dma *of_dma_find_controller (struct device_node *np) +{ +struct of_dma *ofdma; + +list_for_each_entry_rcu (ofdma, &of_dma_list, of_dma_controllers) { +if (ofdma->of_node == np) +return ofdma; + } + +return NULL ; +} + +/** +* of_dma_controller_register() - Register a DMA controller to DT DMA helpers +* @np: device node of DMA controller +* @of_dma_xlate: generic translation function which converts a phandle +* arguments list into a generic output value +* +* Returns 0 on success or appropriate errno value on error. +* +* If #dma-cells is not specified in DMA controller device tree node, we assume +* that the DMA controller phandle will come without argument. +* +* Allocated memory should be freed with appropriate of_dma_controller_free() +* call. +*/ +int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +struct of_dma *ofdma; +const __be32 *nbcells; + +if (!np || !of_dma_xlate) { +pr_err ("%s: not enough information provided\n" , __func__); +return -EINVAL ; + } + + ofdma =kzalloc (sizeof (*ofdma),GFP_KERNEL ); +if (!ofdma) +return -ENOMEM ; + + nbcells =of_get_property (np,"#dma-cells" ,NULL ); +if (!nbcells) +/*
Quoted 8 lines
drivers/of/dma.c line 83
Quoted 6 lines
drivers/of/dma.c new file +6
-0+* no #dma-cells properties: assume no argument to +* dma-request property on slave side +*/ + ofdma->of_dma_n_cells =0 ; + else + ofdma->of_dma_n_cells = be32_to_cpup(nbcells);Make #dma-cells a required property
Ok.
Quoted 93 lines
Quoted 93 lines
drivers/of/dma.c new file +93
-0+ + ofdma->of_node = np; + ofdma->of_dma_xlate = of_dma_xlate; + +/* Now queue of_dma controller structure in list */ +list_add_tail_rcu (&ofdma->of_dma_controllers, &of_dma_list); + +return 0 ; +} +EXPORT_SYMBOL_GPL (of_dma_controller_register); + +/** +* of_dma_controller_free() - Remove a DMA controller from DT DMA helpers list +* @np: device node of DMA controller +* +* Memory allocated by of_dma_controller_register() is freed here. +*/ +void of_dma_controller_free (struct device_node *np) +{ +struct of_dma *ofdma; + + ofdma =of_dma_find_controller (np); +if (ofdma) { +list_del_rcu (&ofdma->of_dma_controllers); +kfree (ofdma); + } +} +EXPORT_SYMBOL_GPL (of_dma_controller_free); + +/** +* of_get_dma_request() - Get the associated DMA request data +* @np: device node to get DMA request from +* @index: index of the DMA request +* @out_data: a output that can be filled in by the of_dma_xlate() function +* +* Returns return value of of_dma_xlate() and fills out_data (if provided). +* On error returns the appropriate errno value. +*/ +int of_get_dma_request (struct device_node *np,int index, +void *out_data) +{ +struct of_phandle_args dma_spec; +struct of_dma *ofdma; +int ret; + + ret =of_parse_phandle_with_args (np,"dma-request" ,"#dma-cells" , + index, &dma_spec); +if (ret) { +pr_err ("%s: can't parse dma property\n" , np->full_name); + goto err0; + } + +if (list_empty (&of_dma_list)) { +pr_debug ("%s: empty DMA controller list\n" , + np->full_name); + ret = -ENOENT ; + goto err1; + } + + ofdma =of_dma_find_controller (dma_spec.np); +if (!ofdma) { +pr_debug ("%s: DMA controller %s isn't registered\n" , + np->full_name, dma_spec.np->full_name); + ret = -ENODEV ; + goto err1; + } + +if (dma_spec.args_count != ofdma->of_dma_n_cells) { +pr_debug ("%s: wrong #dma-cells for %s\n" , + np->full_name, dma_spec.np->full_name); + ret = -EINVAL ; + goto err1; + } + + ret = ofdma->of_dma_xlate (&dma_spec, out_data); + +err1: +of_node_put (dma_spec.np); +err0: +pr_debug ("%s exited with status %d\n" , __func__, ret); +return ret; +} +EXPORT_SYMBOL_GPL (of_get_dma_request); + +/** +* of_dma_xlate_onenumbercell() - Generic DMA xlate for direct one cell bindings +* +* Device Tree DMA translation function which works with one cell bindings +* where the cell values map directly to the hardware request number understood +* by the DMA controller. +*/ +int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec,void *dma_req) +{
Quoted 8 lines
drivers/of/dma.c line 181
Quoted 5 lines
drivers/of/dma.c new file +5
-0+if (!dma_spec) +return -EINVAL ; +if (WARN_ON (dma_spec->args_count !=1 )) +return -EINVAL ; + *(int *)dma_req = dma_spec->args[0 ];Following on from comment above; the void
*dma_reqparameter is dangerous. How does this function know that it has been passed an int* pointer?
Well, that is a drawback that comes from having to address generic cases. But anyway, if the DMA controller decide to register a .xlate() function that returns an integer, the slave driver that will ask a "request line" to this DMA controller will be aware that an integer has to be passed to of_get_dma_request().
(
but I realize that a test on dma_req is certainly missing:
if (!dma_req) return -ENOBUFS;
)
It is true also that if we can find something generic and cleaner, I will vote for it...
Quoted 72 lines
Quoted 72 lines
drivers/of/dma.c new file +72
-0+return 0 ; +} +EXPORT_SYMBOL_GPL (of_dma_xlate_onenumbercell); + +/** +* of_dma_count - Count DMA requests for a device +* @np: device node to count DMAs for +* +* The function returns the count of DMA requests specified for a node. +* +* Note that the empty DMA specifiers counts too. For example, +* +* dma-request = <0 +* &sdma 1 +* 0 +* &sdma 3>; +* +* defines four DMAs (so this function will return 4), two of which +* are not specified. +*/ +unsigned int of_dma_count (struct device_node *np) +{ +unsigned int cnt =0 ; + +do { +int ret; + + ret =of_parse_phandle_with_args (np,"dma-request" , +"#dma-cells" , cnt,NULL ); +/* A hole in the dma-request = <> counts anyway. */ +if (ret <0 && ret != -EEXIST ) +break ; + }while (++cnt); + +return cnt; +} +EXPORT_SYMBOL_GPL (of_dma_count); + +/** +* of_dma_to_resource - Decode a node's DMA and return it as a resource +* @dev: pointer to device tree node +* @index: zero-based index of the DMA request +* @r: pointer to resource structure to return result into. +* +* Using a resource to export a DMA request number to a driver should +* be used only for legacy purpose and on system when only one DMA controller +* is present. +* The proper and only scalable way is to use the native of_get_dma_request API +* in order retrieve both the DMA controller device node and the DMA request +* line for that controller. +*/ +int of_dma_to_resource (struct device_node *dev,int index,struct resource *r) +{ +const char *name =NULL ; +int dma; +int ret; + +if (!r) +return -EINVAL ; + + ret =of_get_dma_request (dev, index, &dma); +if (ret <0 ) +return ret; + +/* +* Get optional "dma-request-names" property to add a name +* to the resource. +*/ +of_property_read_string_index (dev,"dma-request-names" , index, + &name); + + r->start = dma;
Quoted 8 lines
drivers/of/dma.c line 256
Quoted 3 lines
drivers/of/dma.c new file +3
-0+ r->end = dma; + r->flags =IORESOURCE_DMA ; + r->name = name ? name : dev->full_name;I'll reiterate my concern here. The dual meaning of
r->namedepending on the presence of a dma-request-names property is a real problem. I know this patch set hasn't introduced the problem since it already exists for register regions and irqs, but it is expanding it.
After discussion between Benoit and Arnd it is clear now that the of_dma_to_resource() API will go away.
Quoted 86 lines
Quoted 85 lines
drivers/of/dma.c new file +4
-0+ +return dma; +} +EXPORT_SYMBOL_GPL (of_dma_to_resource);include/linux/of_dma.h new file +67
-0@@ -0,0 +1,67 @@ +/* +* OF helpers for DMA request / controller +* +* Based on of_gpio.h +* +* Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#ifndef __LINUX_OF_DMA_H +#define __LINUX_OF_DMA_H + +#include <linux/of.h> + +struct device_node ; + +#ifdef CONFIG_OF_DMA + +extern int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)); +extern void of_dma_controller_free (struct device_node *np); +extern int of_get_dma_request (struct device_node *np,int index, +void *out_data); +extern unsigned int of_dma_count (struct device_node *np); +extern int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); + +extern int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *dma_req); +#else /* CONFIG_OF_DMA */ + +static int of_dma_controller_register (struct device_node *np, +int (*of_dma_xlate)(struct of_phandle_args *,void *)) +{ +return -ENOSYS ; +} + +static void of_dma_controller_free (struct device_node *np) {} + +extern int of_get_dma_request (struct device_node *np,int index, +void *out_data) +{ +return -ENOSYS ; +} + +static inline unsigned int of_dma_count (struct device_node *np) +{ +return 0 ; +} + +static int of_dma_to_resource (struct device_node *dev,int index, +struct resource *r); +{ +return -ENOSYS ; +} + +static int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec, +void *dma_req) +{ +return -ENOSYS ; +} +#endif /* CONFIG_OF_DMA */ + +#endif /* __LINUX_OF_DMA_H */ Signature · 7 lines
1.7.9 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 1 line
Nicolas Ferre
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 10 lines · Nicolas Ferre
Quoted 4 lines
This
_xlateis nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.You mean, if there is no DMA controller phandle specified in the property? I think that it is not the purpose of this API to choose a DMA controller, Nor to provide a channel. The only purpose of this API is to give a HW request to be used by a DMA slave driver. This slave should already have a channel to use and a controller to talk to.
I don't think there is consensus on this point. I would expect that the device driver requests the channel with the same operation as passing the request data.
Contrast the two ways this is done in atmel-mci.c and mmci.c:
==== atmel ====
/* interface between platform and driver */
struct at_dma_slave {
struct device *dma_dev;
dma_addr_t tx_reg;
dma_addr_t rx_reg;
enum at_dma_slave_width reg_width;
u32 cfg;
u32 ctrla;
};
struct mci_dma_data {
struct at_dma_slave sdata;
};#define slave_data_ptr(s) (&(s)->sdata)
#define find_slave_dev(s) ((s)->sdata.dma_dev)
/* in atmel-mci.c */
static bool atmci_filter(struct dma_chan *chan, void *slave)
{
struct mci_dma_data *sl = slave;
if (sl && find_slave_dev(sl) == chan->device->dev) {
chan->private = slave_data_ptr(sl);
return true;
} else {
return false;
}
}
static bool atmci_configure_dma(struct atmel_mci *host)
{
...
host->dma.chan = dma_request_channel(mask, atmci_filter, pdata->dma_slave);
...
}==== mmci ====
/* in drivers/dma/ste_dma40.c, others in pl330.c, coh901318.c, ... */ bool stedma40_filter(struct dma_chan *chan, void *data)
{
struct stedma40_chan_cfg *info = data;
struct d40_chan *d40c = container_of(chan, struct d40_chan, chan);
int err;
err = d40_validate_conf(d40c, info);
if (!err)
d40c->dma_cfg = *info;
d40c->configured = true;
return err == 0;
}EXPORT_SYMBOL(stedma40_filter);
/* in mmci.h */
struct mmci_platform_data {
...
bool (*dma_filter)(struct dma_chan *chan, void *filter_param);
void *dma_rx_param;
void *dma_tx_param;
};/* in mmci.c */
static void __devinit mmci_dma_setup(struct mmci_host *host)
{
...
host->dma_rx_channel = dma_request_channel(mask, plat->dma_filter, plat->dma_rx_param);
...
}====
Whatever we come up with obviously needs to work with both drivers. I think we will end up with something closer to the second one, except that the dma parameters do not come from platform_data but from the #dma-request property, which also identifies the controller and channel.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 33 lines · Arnd Bergmann
==== mmci ====
/* indrivers/dma/ste_dma40.c, others inpl330.c,coh901318.c, ... */ boolstedma40_filter(struct dma_chan *chan, void *data){ struct stedma40_chan_cfg *info = data; struct d40_chan *d40c = container_of(chan, struct d40_chan, chan); int err; err = d40_validate_conf(d40c, info); if (!err) d40c->dma_cfg = *info; d40c->configured = true; return err == 0; }
EXPORT_SYMBOL(stedma40_filter);/* in
mmci.h*/struct mmci_platform_data { ... bool (*dma_filter)(struct dma_chan *chan, void *filter_param); void *dma_rx_param; void *dma_tx_param; };/* in
mmci.c*/
static void__devinitmmci_dma_setup(struct mmci_host *host){ ... host->dma_rx_channel = dma_request_channel(mask, plat->dma_filter, plat->dma_rx_param); ... }
Quoted 6 lines
====
Whatever we come up with obviously needs to work with both drivers. I think we will end up with something closer to the second one, except that the dma parameters do not come from
platform_databut from the #dma-request property, which also identifies the controller and channel.
Firstly, define what you mean by "the DMA parameters". Things like burst size, register width, register address? That should all be known by the peripheral driver and _not_ be encoded into DT in any way - and this information should be passed to the DMA engine driver for the well defined API for that purpose.
Secondly, there are platforms (Samsung) where peripherals are connected to more than one DMA controller, and either DMA controller can be used - I'm told by Jassi that there's reasons why you'd want to select one or other as the target at runtime.
Whether it's appropriate for DT to know that or not, I'm not certain at the moment - I suspect the _right_ solution would be a radical DMA engine redesign which moves _all_ slave DMA to a virtual channel representation, and for the virtual channels to be scheduled onto a set of physical DMA channels over a range of DMA devices. Obviously, there's restrictions on which virtual channels could be scheduled onto which physical channels of which DMA devices.
It seems to me, therefore, that any attempt to come up with some kind of DT based representation of the current scheme is doomed to fail, and will at some point become obsolete.
I think instead, rather than trying to fix this now, we persist with what we have, but organize an effort to clean up and restructure the DMA engine code so that:
(a) things like the Samsung, and ARM board oddities can be easily dealt with in a driver independent manner.
(b) get rid of all the duplicated functionality between each different DMA engine implementation, separating this out into core code, and simplifying the drivers.
The _big_ problem I see is that if we try to represent the existing solution in DT, we're going to get locked into that way of doing things and then any major restructuring of the DMA engine stuff will become an impossibility - especially if we start specifying things by DMA request signal numbers on DMA engines.
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 5 lines · Russell King - ARM Linux
Firstly, define what you mean by "the DMA parameters". Things like burst size, register width, register address? That should all be known by the peripheral driver and not be encoded into DT in any way - and this information should be passed to the DMA engine driver for the well defined API for that purpose.
This seems to be different for each controller, I tried to use a generic term here. In case of stedma40 (which is probably the most complex one today), it would be this structure:
struct stedma40_chan_cfg {
enum stedma40_xfer_dir dir;
bool high_priority;
bool realtime;
enum stedma40_mode mode;
enum stedma40_mode_opt mode_opt;
int src_dev_type;
int dst_dev_type;
struct stedma40_half_channel_info src_info;
struct stedma40_half_channel_info dst_info;
bool use_fixed_channel;
int phy_channel;
};All these fields are set up by the platform code today, not by the device driver, so it would be logical to have them represented in the device tree, though some of the fields might turn out to be in the wrong place already.
Quoted 4 lines
Secondly, there are platforms (Samsung) where peripherals are connected to more than one DMA controller, and either DMA controller can be used - I'm told by Jassi that there's reasons why you'd want to select one or other as the target at runtime.
How common is that? If there are only a few drivers that have this requirement, we could have this represented in the driver binding, by listing multiple channels and giving the device the choice.
A possible way to deal with this is to list all the alternatives in the device tree and using the dma-names property to name them, giving the same name to those that we need to pick one of.
Quoted 9 lines
I think instead, rather than trying to fix this now, we persist with what we have, but organize an effort to clean up and restructure the DMA engine code so that:
(a) things like the Samsung, and ARM board oddities can be easily dealt with in a driver independent manner.
(b) get rid of all the duplicated functionality between each different DMA engine implementation, separating this out into core code, and simplifying the drivers.
At that point, we definitely need to involve the dmaenigne maintainers who have unfortunately not been on Cc for the whole discussion so far.
I actually hope that a lot of the issues go away if we come up with good device tree bindings that can cover the existing corner cases while providing a simple interface for device drivers to use.
Quoted 5 lines
The big problem I see is that if we try to represent the existing solution in DT, we're going to get locked into that way of doing things and then any major restructuring of the DMA engine stuff will become an impossibility - especially if we start specifying things by DMA request signal numbers on DMA engines.
Note that Thomas Abraham has already introduced a DT support for pl330, which is sadly seems to be anything but generic.
Arnd
PATCH 1/1 of: Add generic device tree DMA helpers
Quoted 10 lines · Arnd Bergmann
Quoted 5 lines
Secondly, there are platforms (Samsung) where peripherals are connected to more than one DMA controller, and either DMA controller can be used - I'm told by Jassi that there's reasons why you'd want to select one or other as the target at runtime.
How common is that? If there are only a few drivers that have this requirement, we could have this represented in the driver binding, by listing multiple channels and giving the device the choice.
All Samsung SoCs (last ~4yrs) with PL330 have some peripherals that could be served by more than one PL330 instance.
The requirement is not from any driver and it is nothing Samsung specific. Any platform with 1:n client:dmac might want to do stuff like serving a new channel request from an already active DMAC, rather than invoking otherwise fully idle one.
Similarly for Mem->Mem, the PL330 distributes its cycles between active channels, so a platform might want channel allocation uniformly across PL330 instances.
-jassi
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Russell King - ARM LinuxOn 03/19/2012 04:33 PM, Russell King - ARM Linux :
Quoted 52 lines · Arnd Bergmann
Quoted 33 lines
==== mmci ====
/* indrivers/dma/ste_dma40.c, others inpl330.c,coh901318.c, ... */ boolstedma40_filter(struct dma_chan *chan, void *data){ struct stedma40_chan_cfg *info = data; struct d40_chan *d40c = container_of(chan, struct d40_chan, chan); int err; err = d40_validate_conf(d40c, info); if (!err) d40c->dma_cfg = *info; d40c->configured = true; return err == 0; }
EXPORT_SYMBOL(stedma40_filter);/* in
mmci.h*/struct mmci_platform_data { ... bool (*dma_filter)(struct dma_chan *chan, void *filter_param); void *dma_rx_param; void *dma_tx_param; };/* in
mmci.c*/
static void__devinitmmci_dma_setup(struct mmci_host *host){ ... host->dma_rx_channel = dma_request_channel(mask, plat->dma_filter, plat->dma_rx_param); ... }Quoted 6 lines
====
Whatever we come up with obviously needs to work with both drivers. I think we will end up with something closer to the second one, except that the dma parameters do not come from
platform_databut from the #dma-request property, which also identifies the controller and channel.Firstly, define what you mean by "the DMA parameters". Things like burst size, register width, register address? That should all be known by the peripheral driver and _not_ be encoded into DT in any way - and this information should be passed to the DMA engine driver for the well defined API for that purpose.
Secondly, there are platforms (Samsung) where peripherals are connected to more than one DMA controller, and either DMA controller can be used - I'm told by Jassi that there's reasons why you'd want to select one or other as the target at runtime.
Quoted 7 lines
Whether it's appropriate for DT to know that or not, I'm not certain at the moment - I suspect the _right_ solution would be a radical DMA engine redesign which moves _all_ slave DMA to a virtual channel representation, and for the virtual channels to be scheduled onto a set of physical DMA channels over a range of DMA devices. Obviously, there's restrictions on which virtual channels could be scheduled onto which physical channels of which DMA devices.
OMG! the dmaengine drivers are already not so obvious to understand. I fear that trying to mimic some special behaviors within relatively simple dmaengine drivers will bring confusion and prevent people to read/contribute and fix those simple ones...
Quoted 14 lines
It seems to me, therefore, that any attempt to come up with some kind of DT based representation of the current scheme is doomed to fail, and will at some point become obsolete.
I think instead, rather than trying to fix this now, we persist with what we have, but organize an effort to clean up and restructure the DMA engine code so that:
(a) things like the Samsung, and ARM board oddities can be easily dealt with in a driver independent manner.
(b) get rid of all the duplicated functionality between each different DMA engine implementation, separating this out into core code, and simplifying the drivers.
Quoted 5 lines
The _big_ problem I see is that if we try to represent the existing solution in DT, we're going to get locked into that way of doing things and then any major restructuring of the DMA engine stuff will become an impossibility - especially if we start specifying things by DMA request signal numbers on DMA engines.
Even if I understand your point, I wonder what is the solution for us that have a pretty simple representation of *hardware* to code into DT:
should we wait for a big rework? Should we go for a private DMA binding for each of us that have just the need for a quite simple representation?
I must say that I am puzzled by recent discussion on the topic (mix of "channel" and "request" notions, plan for a complete rework of dmaengine that can delay DT representation of DMA slave-controller relationship, even my own doubts on the "void *" output parameter, etc.). It seems that I am not familiar with all those cases and that I cannot go further with a generic DT representation...
Best regards,
Signature · 1 line
Nicolas Ferre
PATCH 1/1 of: Add generic device tree DMA helpers
Hi Nico,
Quoted 91 lines · Nicolas Ferre
On 03/19/2012 04:33 PM, Russell King - ARM Linux :
Quoted 52 lines · Arnd Bergmann
Quoted 33 lines
==== mmci ====
/* indrivers/dma/ste_dma40.c, others inpl330.c,coh901318.c, ... */ boolstedma40_filter(struct dma_chan *chan, void *data){ struct stedma40_chan_cfg *info = data; struct d40_chan *d40c = container_of(chan, struct d40_chan, chan); int err; err = d40_validate_conf(d40c, info); if (!err) d40c->dma_cfg = *info; d40c->configured = true; return err == 0; }
EXPORT_SYMBOL(stedma40_filter);/* in
mmci.h*/struct mmci_platform_data { ... bool (*dma_filter)(struct dma_chan *chan, void *filter_param); void *dma_rx_param; void *dma_tx_param; };/* in
mmci.c*/
static void__devinitmmci_dma_setup(struct mmci_host *host){ ... host->dma_rx_channel = dma_request_channel(mask, plat->dma_filter, plat->dma_rx_param); ... }Quoted 6 lines
====
Whatever we come up with obviously needs to work with both drivers. I think we will end up with something closer to the second one, except that the dma parameters do not come from
platform_databut from the #dma-request property, which also identifies the controller and channel.Firstly, define what you mean by "the DMA parameters". Things like burst size, register width, register address? That should all be known by the peripheral driver and _not_ be encoded into DT in any way - and this information should be passed to the DMA engine driver for the well defined API for that purpose.
Secondly, there are platforms (Samsung) where peripherals are connected to more than one DMA controller, and either DMA controller can be used - I'm told by Jassi that there's reasons why you'd want to select one or other as the target at runtime.
Quoted 7 lines
Whether it's appropriate for DT to know that or not, I'm not certain at the moment - I suspect the _right_ solution would be a radical DMA engine redesign which moves _all_ slave DMA to a virtual channel representation, and for the virtual channels to be scheduled onto a set of physical DMA channels over a range of DMA devices. Obviously, there's restrictions on which virtual channels could be scheduled onto which physical channels of which DMA devices.
OMG! the dmaengine drivers are already not so obvious to understand. I fear that trying to mimic some special behaviors within relatively simple dmaengine drivers will bring confusion and prevent people to read/contribute and fix those simple ones...
Quoted 14 lines
It seems to me, therefore, that any attempt to come up with some kind of DT based representation of the current scheme is doomed to fail, and will at some point become obsolete.
I think instead, rather than trying to fix this now, we persist with what we have, but organize an effort to clean up and restructure the DMA engine code so that:
(a) things like the Samsung, and ARM board oddities can be easily dealt with in a driver independent manner.
(b) get rid of all the duplicated functionality between each different DMA engine implementation, separating this out into core code, and simplifying the drivers.Quoted 5 lines
The _big_ problem I see is that if we try to represent the existing solution in DT, we're going to get locked into that way of doing things and then any major restructuring of the DMA engine stuff will become an impossibility - especially if we start specifying things by DMA request signal numbers on DMA engines.
Even if I understand your point, I wonder what is the solution for us that have a pretty simple representation of *hardware* to code into DT:
should we wait for a big rework? Should we go for a private DMA binding for each of us that have just the need for a quite simple representation?
Quoted 6 lines
I must say that I am puzzled by recent discussion on the topic (mix of "channel" and "request" notions, plan for a complete rework of dmaengine that can delay DT representation of DMA slave-controller relationship, even my own doubts on the "void *" output parameter, etc.). It seems that I am not familiar with all those cases and that I cannot go further with a generic DT representation...
I do agree that it appears that some DMA controllers are way more complex that the one we were trying to support.
On OMAP, the DMA channels and the DMA requests are completely orthogonal, and thus I do not have to provide any information about the channel in the DT binding since the SW can affect any request to any channel.
Reading the various points from Russell and Arnd, it seems that this is not always the case :-(
I guess this is probably the main cause of confusion in this discussion.
So I do not know either how we can progress further on that without a clear understanding of what all these DMA controllers are capable of.
Regards,
Benoit
PATCH 1/1 of: Add generic device tree DMA helpers
In reply to Nicolas FerreQuoted 14 lines · Nicolas Ferre
On 03/17/2012 10:40 AM, Grant Likely :
Quoted 12 lines · Nicolas Ferre
drivers/of/dma.c line 28
Quoted 6 lines
drivers/of/dma.c new file +6
-0+struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};This
_xlateis nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.
Quoted 2 lines
You mean, if there is no DMA controller phandle specified in the property?
No; I'm assuming that dma-channel properties will alwasy have a phandle to the dma controller node.
Quoted 4 lines
I think that it is not the purpose of this API to choose a DMA controller, Nor to provide a channel. The only purpose of this API is to give a HW request to be used by a DMA slave driver. This slave should already have a channel to use and a controller to talk to.
Then where is the function that finds the reference to the DMA controller? I don't understand why it would be useful to decode that separately.
Quoted 9 lines
Quoted 2 lines
The void *data pointer must be replaced with a typed structure so that context can be returned.
I am not sure to follow you entirely... How do I address the fact that several types of request value can be returned then?
BTW, can we imagine a phandle property with a sting as a argument? should it be written this way?
dma-request = <&testdmac1>, "slave-rx", "slave-tx";
No, I'm not suggesting that. Mixing phandles and strings in a single property is possible but ugly. The phandle-args pattern which uses zero or more cells as arguments should be used.
Quoted 2 lines
If yes, the
of_parse_phandle_with_args()is not working on this type...
Right; of_parse_phandle_with_args() should do the job.
Quoted 3 lines
(I realize that there seems to be no way out for a generic API: maybe we should move to one or two cases to address and concentrate on them).
The way I read this patch, the xlate function returns a single integer representing the DMA request number, but it doesn't provide any data about *which* dma controller is associated with that channel. The result of xlate needs to be something like a dma_chan reference that identifies both the DMA engine and the channel/request on that dma engine.
[...]
Quoted 18 lines
Quoted 9 lines
Quoted 9 lines
drivers/of/dma.c new file +9
-0+/** +* of_dma_xlate_onenumbercell() - Generic DMA xlate for direct one cell bindings +* +* Device Tree DMA translation function which works with one cell bindings +* where the cell values map directly to the hardware request number understood +* by the DMA controller. +*/ +int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec,void *dma_req) +{Quoted 8 lines
drivers/of/dma.c line 181
Quoted 5 lines
drivers/of/dma.c new file +5
-0+if (!dma_spec) +return -EINVAL ; +if (WARN_ON (dma_spec->args_count !=1 )) +return -EINVAL ; + *(int *)dma_req = dma_spec->args[0 ];Following on from comment above; the void
*dma_reqparameter is dangerous. How does this function know that it has been passed an int* pointer?
Quoted 2 lines
Well, that is a drawback that comes from having to address generic cases.
Not if you do it right. If a specific data structure is returned, then there can be context attached as to what the data means and which dma controller knows how to parse it.
Quoted 4 lines
But anyway, if the DMA controller decide to register a .xlate() function that returns an integer, the slave driver that will ask a "request line" to this DMA controller will be aware that an integer has to be passed to
of_get_dma_request().
The problem still remains; I don't see how the dma slave can figure out *which* dma controller it needs to talk to.
g.
Signature · 2 lines
Grant Likely, B.Sc, P.Eng. Secret Lab Technologies,Ltd.
PATCH 1/1 of: Add generic device tree DMA helpers
On 03/19/2012 03:45 PM, Grant Likely :
Quoted 86 lines · Nicolas Ferre
Quoted 14 lines
On 03/17/2012 10:40 AM, Grant Likely :
Quoted 12 lines · Nicolas Ferre
drivers/of/dma.c line 28
Quoted 6 lines
drivers/of/dma.c new file +6
-0+struct of_dma { +struct list_head of_dma_controllers; +struct device_node *of_node; +int of_dma_n_cells; +int (*of_dma_xlate)(struct of_phandle_args *dma_spec,void *data); +};This
_xlateis nearly useless as a generic API. It solves the problem for the specific case where the driver is hard-coded to know which DMA engine to talk to, but since the returned data doesn't provide any context, it isn't useful if there are multiple DMA controllers to choose from.Quoted 2 lines
You mean, if there is no DMA controller phandle specified in the property?
No; I'm assuming that dma-channel properties will alwasy have a phandle to the dma controller node.
Quoted 4 lines
I think that it is not the purpose of this API to choose a DMA controller, Nor to provide a channel. The only purpose of this API is to give a HW request to be used by a DMA slave driver. This slave should already have a channel to use and a controller to talk to.
Then where is the function that finds the reference to the DMA controller? I don't understand why it would be useful to decode that separately.
Quoted 9 lines
Quoted 2 lines
The void *data pointer must be replaced with a typed structure so that context can be returned.
I am not sure to follow you entirely... How do I address the fact that several types of request value can be returned then?
BTW, can we imagine a phandle property with a sting as a argument? should it be written this way?
dma-request = <&testdmac1>, "slave-rx", "slave-tx";No, I'm not suggesting that. Mixing phandles and strings in a single property is possible but ugly. The phandle-args pattern which uses zero or more cells as arguments should be used.
Quoted 2 lines
If yes, the
of_parse_phandle_with_args()is not working on this type...Right;
of_parse_phandle_with_args()should do the job.Quoted 3 lines
(I realize that there seems to be no way out for a generic API: maybe we should move to one or two cases to address and concentrate on them).
The way I read this patch, the xlate function returns a single integer representing the DMA request number, but it doesn't provide any data about *which* dma controller is associated with that channel. The result of xlate needs to be something like a
dma_chanreference that identifies both the DMA engine and the channel/request on that dma engine.[...]
Quoted 18 lines
Quoted 9 lines
Quoted 9 lines
drivers/of/dma.c new file +9
-0+/** +* of_dma_xlate_onenumbercell() - Generic DMA xlate for direct one cell bindings +* +* Device Tree DMA translation function which works with one cell bindings +* where the cell values map directly to the hardware request number understood +* by the DMA controller. +*/ +int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec,void *dma_req) +{Quoted 8 lines
drivers/of/dma.c line 181
Quoted 5 lines
drivers/of/dma.c new file +5
-0+if (!dma_spec) +return -EINVAL ; +if (WARN_ON (dma_spec->args_count !=1 )) +return -EINVAL ; + *(int *)dma_req = dma_spec->args[0 ];Following on from comment above; the void
*dma_reqparameter is dangerous. How does this function know that it has been passed an int* pointer?Quoted 2 lines
Well, that is a drawback that comes from having to address generic cases.
Not if you do it right. If a specific data structure is returned, then there can be context attached as to what the data means and which dma controller knows how to parse it.
Quoted 7 lines
Quoted 4 lines
But anyway, if the DMA controller decide to register a .xlate() function that returns an integer, the slave driver that will ask a "request line" to this DMA controller will be aware that an integer has to be passed to
of_get_dma_request().The problem still remains; I don't see how the dma slave can figure out *which* dma controller it needs to talk to.
Is not it what the phandle to the dma controller is made for?
Bye,
Signature · 1 line
Nicolas Ferre
PATCH v2 1/2 of: Add generic device tree DMA helpers
In reply to Nicolas FerreAdd some basic helpers to retrieve a DMA controller device_node and the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of an of_dma_xlate() function that will be implemented by the DMA controller can carry any type of "dma-request" argument.
The DMA client will search its associated DMA controller in the list and call the registered of_dam_xlate() function to retrieve the request values.
One simple xlate function is provided for the "single number" type of request binding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
---
Hi all,
v2: - remove of_dma_to_resource API
- make property #dma-cells required (no fallback anymore)
- another check in of_dma_xlate_onenumbercell() function
This patch is just for the record as it seems that those helpers will not cover every people expectations. So I let interested people build on top of this or redo the whole stuff from the gound up.
Bye,
Documentation/devicetree/bindings/dma/dma.txt | 45 +++++ drivers/of/Makefile | 2 +- drivers/of/dma.c | 215 +++++++++++++++++++++++++ include/linux/of_dma.h | 60 +++++++ 4 files changed, 321 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/dma.txt create mode 100644 drivers/of/dma.c create mode 100644 include/linux/of_dma.h
Documentation/devicetree/bindings/dma/dma.txt new file +45 -0
@@ -0,0 +1,45 @@ +* Generic DMA Controller and DMA request bindings + +Generic binding to provide a way for a driver to retrieve the +DMA request line that goes from an IP to a DMA controller. + + +* DMA controller + +Required property: + - #dma-cells: Number of cells for each DMA line. + + +Example: + + sdma: dma-controller at 48000000 { + compatible = "ti,sdma-omap4" + reg = <0x48000000 0x1000>; + interrupts = <12>; + #dma-cells = <1>; + }; + + + +* DMA client + +Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. + +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line". +Optional property: + - dma-request-names: list of strings in the same order as the dma-request + in the dma-request property. + + +Example: + + i2c1: i2c at 1 { + ... + dma-request = <&sdma 2 &sdma 3>; + dma-request-names = "tx", "rx"; + ... + };
drivers/of/Makefile +1 -1
@@ -1,4 +1,4 @@ -obj-y = base.o +obj-y = base.o dma.o obj-$(CONFIG_OF_FLATTREE) += fdt.o obj-$(CONFIG_OF_PROMTREE) += pdt.o obj-$(CONFIG_OF_ADDRESS) += address.o
drivers/of/dma.c new file +215 -0
215 lines not on this page · raw message
include/linux/of_dma.h new file +60 -0
60 lines not on this page · raw message
Signature · 1 line
1.7.9
PATCH 2/2 of: selftest/dma: Add selftest for new DT DMA request helpers
Those selftests are covering the new DT DMA helpers. They will test both error and normal cases.
A custom .xlate() function is also provided to show the use of this API in case of a different need than the single cell argument.
--- arch/arm/boot/dts/at91sam9m10g45ek.dts | 2 + arch/arm/boot/dts/testcases/tests-dma.dtsi | 24 +++++++ arch/arm/boot/dts/testcases/tests.dtsi | 1 + drivers/of/selftest.c | 100 ++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 0 deletions(-) create mode 100644 arch/arm/boot/dts/testcases/tests-dma.dtsi
arch/arm/boot/dts/at91sam9m10g45ek.dts +2 -0
@@ -38,3 +38,5 @@ }; }; }; + +/include/ "testcases/tests.dtsi"
arch/arm/boot/dts/testcases/tests-dma.dtsi new file +24 -0
@@ -0,0 +1,24 @@ + +/ { + testcase-data { + dma-tests { + testdmac0: test-dma-controller0 { + #dma-cells = <1>; + }; + + testdmac1: test-dma-controller1 { + #dma-cells = <2>; + }; + + dma-slave-a { + dma-request = <&testdmac0 42>; + }; + dma-slave-b { + dma-request = <&testdmac1 24>; /* wrong number of values */ + }; + dma-slave-c { + dma-request = <&testdmac1 24 25>; + }; + }; + }; +};
arch/arm/boot/dts/testcases/tests.dtsi +1 -0
@@ -1 +1,2 @@ /include/ "tests-phandle.dtsi" +/include/ "tests-dma.dtsi"
drivers/of/selftest.c +100 -0
118 lines not on this page · raw message
Signature · 1 line
1.7.9
PATCH 2/2 of: selftest/dma: Add selftest for new DT DMA request helpers
Quoted 4 lines · Nicolas Ferre
Those selftests are covering the new DT DMA helpers. They will test both error and normal cases.
A custom .xlate() function is also provided to show the use of this API in case of a different need than the single cell argument.
I'm very happy to see test case code. Thanks.
However, since we both understand that this series doesn't address my major concerns, I'm not spending any time reviewing this version.
g.
Quoted 196 lines
Signed-off-by Nicolas Ferre <[email protected]>Cc Benoit Cousson <[email protected]>Cc Stephen Warren <[email protected]>Cc Grant Likely <[email protected]>Cc Russell King <[email protected]>Cc Rob Herring <[email protected]>Cc Arnd Bergmann <[email protected]>--- arch/arm/boot/dts/at91sam9m10g45ek.dts | 2 + arch/arm/boot/dts/testcases/tests-dma.dtsi | 24 +++++++ arch/arm/boot/dts/testcases/tests.dtsi | 1 + drivers/of/selftest.c | 100 ++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 0 deletions(-) create mode 100644 arch/arm/boot/dts/testcases/tests-dma.dtsiarch/arm/boot/dts/at91sam9m10g45ek.dts +2
-0@@ -38,3 +38,5 @@ }; }; }; + +/include/ "testcases/tests.dtsi"arch/arm/boot/dts/testcases/tests-dma.dtsi new file +24
-0@@ -0,0 +1,24 @@ + +/ { + testcase-data { + dma-tests { + testdmac0: test-dma-controller0 { + #dma-cells = <1>; + }; + + testdmac1: test-dma-controller1 { + #dma-cells = <2>; + }; + + dma-slave-a { + dma-request = <&testdmac0 42>; + }; + dma-slave-b { + dma-request = <&testdmac1 24>; /* wrong number of values */ + }; + dma-slave-c { + dma-request = <&testdmac1 24 25>; + }; + }; + }; +};arch/arm/boot/dts/testcases/tests.dtsi +1
-0@@ -1 +1,2 @@ /include/ "tests-phandle.dtsi" +/include/ "tests-dma.dtsi"drivers/of/selftest.c +100
-0@@ -9,6 +9,7 @@#include <linux/errno.h> #include <linux/module.h> #include <linux/of.h> +#include <linux/of_dma.h> #include <linux/list.h> #include <linux/mutex.h> #include <linux/slab.h> @@ -148,6 +149,104 @@ static void __init of_selftest_property_match_string(void)selftest (rc == -EILSEQ ,"unterminated string; rc=%i" , rc); } +struct two_cells { +int cell1; +int cell2; +}; + +static int of_dma_xlate_twonumbercell (struct of_phandle_args *dma_spec,void *cells) +{ +struct two_cells *tc; + +if (!dma_spec) +return -EINVAL ; +if (!cells) +return -ENOBUFS ; +if (WARN_ON (dma_spec->args_count !=2 )) +return -EINVAL ; + + tc = (struct two_cells *)cells; + + tc->cell1 = dma_spec->args[0 ]; + tc->cell2 = dma_spec->args[1 ]; +return 0 ; +} + +static void __init of_selftest_dma (void ) +{ +struct device_node *dma_controller0_np; +struct device_node *dma_controller1_np; +struct device_node *dma_slave_np; +int rc; +int dma_req; +struct two_cells tc; + +pr_info ("start\n" ); + dma_controller0_np =of_find_node_by_path ("/testcase-data/dma-tests/test-dma-controller0" ); + dma_slave_np =of_find_node_by_path ("/testcase-data/dma-tests/dma-slave-a" ); +if (!dma_controller0_np || !dma_slave_np) { +pr_err ("No testcase data in device tree\n" ); +return ; + } + +/* wrong usage: DMA controller not registered yet! */ + rc =of_get_dma_request (dma_slave_np,0 , &dma_req); +selftest (rc == -ENODEV ,"selftest DMA slave request expected:-ENODEV got:%i\n" , rc); + +/* test DMA controller registration */ + rc =of_dma_controller_register (dma_controller0_np, of_dma_xlate_onenumbercell); +selftest (rc ==0 ,"selftest DMA controller not found: got:%i\n" , rc); + +/* wrong usage and error code tests */ + rc =of_get_dma_request (dma_slave_np,1 , &dma_req); +selftest (rc == -EINVAL ,"selftest DMA slave request expected:-EINVAL got:%i\n" , rc); + + rc =of_get_dma_request (dma_slave_np,0 ,NULL ); +selftest (rc == -ENOBUFS ,"selftest DMA slave request expected:-ENOBUFS got:%i\n" , rc); + +/* proper use of the API */ + rc =of_get_dma_request (dma_slave_np,0 , &dma_req); +selftest (rc ==0 ,"selftest DMA slave request error: got:%i\n" , rc); +selftest (dma_req ==42 ,"selftest DMA slave request line expected:42 got:%i\n" , dma_req); + +of_dma_controller_free (dma_controller0_np); + +/* wrong usage: DMA controller not registered anymore! */ + rc =of_get_dma_request (dma_slave_np,0 , &dma_req); +selftest (rc == -ENODEV ,"selftest DMA slave request expected:-ENODEV got:%i\n" , rc); + +/* use with a two values DMA request specification */ + dma_controller1_np =of_find_node_by_path ("/testcase-data/dma-tests/test-dma-controller1" ); + dma_slave_np =of_find_node_by_path ("/testcase-data/dma-tests/dma-slave-b" ); +if (!dma_controller1_np || !dma_slave_np) { +pr_err ("No testcase data in device tree\n" ); +return ; + } + +/* DMA controller registration with custom .xlate() function */ + rc =of_dma_controller_register (dma_controller1_np, of_dma_xlate_twonumbercell); +selftest (rc ==0 ,"selftest DMA controller not found: got:%i\n" , rc); + +/* wrong number of cells */ + rc =of_get_dma_request (dma_slave_np,0 , &tc); +selftest (rc == -EINVAL ,"selftest DMA slave request expected:-EINVAL got:%i\n" , rc); + + dma_slave_np =of_find_node_by_path ("/testcase-data/dma-tests/dma-slave-c" ); +if (!dma_slave_np) { +pr_err ("No dma-slave-c data in device tree\n" ); +return ; + } + +/* proper use of the API */ + rc =of_get_dma_request (dma_slave_np,0 , &tc); +selftest (rc ==0 ,"selftest DMA slave request error: got:%i\n" , rc); +selftest (tc.cell1 ==24 ,"selftest DMA slave request line expected:24 got:%i\n" , tc.cell1); +selftest (tc.cell2 ==25 ,"selftest DMA slave request line expected:25 got:%i\n" , tc.cell2); + +of_dma_controller_free (dma_controller1_np); +pr_info ("end - %s\n" , selftest_passed ?"PASS" :"FAIL" ); +} +static int __init of_selftest (void ) {struct device_node *np;@@ -162,6 +261,7 @@ static int __init of_selftest(void)pr_info ("start of selftest - you will see error messages\n" );of_selftest_parse_phandle_with_args ();of_selftest_property_match_string (); +of_selftest_dma ();pr_info ("end of selftest - %s\n" , selftest_passed ?"PASS" :"FAIL" );return 0 ; }Signature · 7 lines
1.7.9 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 2 lines
Grant Likely, B.Sc, P.Eng. Secret Lab Technologies,Ltd.
PATCH 1/1 of: dma/fixup
In reply to Nicolas Ferre--- include/linux/of_dma.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
include/linux/of_dma.h +3 -3
@@ -18,7 +18,7 @@struct device_node ; -#ifdef CONFIG_OF_DMA +#ifdef CONFIG_OF extern int of_dma_controller_register (struct device_node *np,int (*of_dma_xlate)(struct of_phandle_args *,void *));
@@ -29,7 +29,7 @@ extern unsigned int of_dma_count(struct device_node *np);extern int of_dma_xlate_onenumbercell (struct of_phandle_args *dma_spec,void *dma_req); -#else /* CONFIG_OF_DMA */ +#else /* CONFIG_OF */ static int of_dma_controller_register (struct device_node *np,int (*of_dma_xlate)(struct of_phandle_args *,void *))
@@ -55,6 +55,6 @@ static int of_dma_xlate_onenumbercell(struct of_phandle_args *dma_spec, {return -ENOSYS ; } -#endif /* CONFIG_OF_DMA */ +#endif /* CONFIG_OF */ #endif /* __LINUX_OF_DMA_H */
Signature · 1 line
1.7.9
PATCH v2 1/2 of: Add generic device tree DMA helpers
In reply to Nicolas FerreQuoted 8 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.
Quoted 8 lines
The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request binding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
Thanks a lot of posting this updated version, I think it will be very useful as we proceed. I still hope that in the end we find that the differences are not all that big and we can do it all with bindings very much like what you have here.
Arnd
PATCH v2 1/2 of: Add generic device tree DMA helpers
In reply to Nicolas FerreQuoted 18 lines · Nicolas Ferre
Add some basic helpers to retrieve a DMA controller
device_nodeand the DMA request specifications. By making DMA controllers register a generic translation function, we allow the management of any type of DMA requests specification.
The void * output of anof_dma_xlate()function that will be implemented by the DMA controller can carry any type of "dma-request" argument.The DMA client will search its associated DMA controller in the list and call the registered
of_dam_xlate()function to retrieve the request values.One simple xlate function is provided for the "single number" type of request binding.
This implementation is independent from dmaengine so it can also be used by legacy drivers.
+++
b/Documentation/devicetree/bindings/dma/dma.txt
Documentation/devicetree/bindings/dma/dma.txt line 32
Quoted 9 lines
Documentation/devicetree/bindings/dma/dma.txt new file +9
-0+* DMA client + +Client drivers should specify the DMA request property using a phandle to +the controller. If needed, the DMA request identity on that controller is then +added followed by optional request specifications. + +Required property: + - dma-request: List of phandle + dma-request + request specifications, + one group per request "line".
Should the property name be plural, i.e. "dma-requests"? For PWM I believe we settled on "pwms" not "pwm", and that was for consistency with other bindings like interrupts and GPIOs.