From: Russell King - ARM Linux <hidden> Date: 2012-06-22 23:12:22
Before this goes much further... one fairly obvious and important point
must be made.
You're designing an API here. You're designing it *WITHOUT* involving
the two most important people in its design that there are. The
DMA engine maintainers. Is this how we go about designing APIs - behind
maintainers backs and then presenting it to the maintainers as a fait
accompli?
There's 86 messages in this thread, none of which have been copied to
them in any way. Why aren't they involved?
On Fri, Jun 22, 2012 at 05:52:08PM -0500, Jon Hunter wrote:
Hi Arnd,
On 06/14/2012 06:48 AM, Arnd Bergmann wrote:
[snip]
quoted
This would let us handle the following cases very easily:
1. one read-write channel
dmas = <&dmac 0x3 match>;
2. a choice of two read-write channels:
dmas = <&dmacA 0x3 matchA>, <&dmacB 0x3 matchB>;
3. one read-channel, one write channel:
dmas = <&dmac 0x1 match-read>, <&dmac 0x2 match-write>;
4. a choice of two read channels and one write channel:
dmas = <&dmacA 0x1 match-readA>, <&dmacA 0x2 match-write>
<&dmacB match-readB>;
And only the cases where we have more multiple channels that differ
in more aspects would require named properties:
5. two different channels
dmas = <&dmac 0x3 match-rwdata>, <&dmac 0x1 match-status>;
dma-names = "rwdata", "status";
6. same as 5, but with a choice of channels:
dmas = <&dmacA 0x3 match-rwdataA>, <&dmacA 0x1 match-status>;
<dmacB 0x3 match-rwdataB>;
dma-names = "rwdata", "status", "rwdata";
With a definition like that, we can implement a very simple device
driver interface for the common cases, and a slightly more complex
one for the more complex cases:
1. chan = of_dma_request_channel(dev->of_node, 0);
2. chan = of_dma_request_channel(dev->of_node, 0);
3. rxchan = of_dma_request_channel(dev->of_node, DMA_MEM_TO_DEV);
txchan = of_dma_request_channel(dev->of_node, DMA_DEV_TO_MEM);
4. rxchan = of_dma_request_channel(dev->of_node, DMA_MEM_TO_DEV);
txchan = of_dma_request_channel(dev->of_node, DMA_DEV_TO_MEM);
5. chan = of_dma_request_named_channel(dev->of_node, "rwdata", 0);
auxchan = of_dma_request_named_channel(dev->of_node, "status", DMA_DEV_TO_MEM);
6. chan = of_dma_request_named_channel(dev->of_node, "rwdata", 0);
auxchan = of_dma_request_named_channel(dev->of_node, "status", DMA_DEV_TO_MEM);
In the above examples, did you imply that the of_dma_request_channel()
function would return a type of "struct dma_chan" and so be calling
dma_request_channel() underneath?
I am been prototyping something, but wanted to make sure I am completely
aligned on this :-)
Cheers
Jon
From: Jon Hunter <hidden> Date: 2012-06-25 16:51:05
Hi Russell,
On 06/22/2012 06:12 PM, Russell King - ARM Linux wrote:
Before this goes much further... one fairly obvious and important point
must be made.
You're designing an API here. You're designing it *WITHOUT* involving
the two most important people in its design that there are. The
DMA engine maintainers. Is this how we go about designing APIs - behind
maintainers backs and then presenting it to the maintainers as a fait
accompli?
Absolutely not, this was not the intent and your point is well
understood. I have added Dan and Vinod, and will ensure that he is added
in future.
There's 86 messages in this thread, none of which have been copied to
them in any way. Why aren't they involved?
Initially this binding was not dma-engine centric. However, I should
have included them in this version from the beginning as I had evolved
it in that direction.
Dan, Vinod, in this thread we have been discussing the addition of a
generic device-tree binding for DMA controllers. In the below, we were
discussing the addition of a device-tree API, which would work as a
wrapper to the dma-engine dma_request_channel() API. I apologise for
adding you late into the discussion. If you have any questions/comments
let me know.
Jon
On Fri, Jun 22, 2012 at 05:52:08PM -0500, Jon Hunter wrote:
quoted
Hi Arnd,
On 06/14/2012 06:48 AM, Arnd Bergmann wrote:
[snip]
quoted
This would let us handle the following cases very easily:
1. one read-write channel
dmas = <&dmac 0x3 match>;
2. a choice of two read-write channels:
dmas = <&dmacA 0x3 matchA>, <&dmacB 0x3 matchB>;
3. one read-channel, one write channel:
dmas = <&dmac 0x1 match-read>, <&dmac 0x2 match-write>;
4. a choice of two read channels and one write channel:
dmas = <&dmacA 0x1 match-readA>, <&dmacA 0x2 match-write>
<&dmacB match-readB>;
And only the cases where we have more multiple channels that differ
in more aspects would require named properties:
5. two different channels
dmas = <&dmac 0x3 match-rwdata>, <&dmac 0x1 match-status>;
dma-names = "rwdata", "status";
6. same as 5, but with a choice of channels:
dmas = <&dmacA 0x3 match-rwdataA>, <&dmacA 0x1 match-status>;
<dmacB 0x3 match-rwdataB>;
dma-names = "rwdata", "status", "rwdata";
With a definition like that, we can implement a very simple device
driver interface for the common cases, and a slightly more complex
one for the more complex cases:
1. chan = of_dma_request_channel(dev->of_node, 0);
2. chan = of_dma_request_channel(dev->of_node, 0);
3. rxchan = of_dma_request_channel(dev->of_node, DMA_MEM_TO_DEV);
txchan = of_dma_request_channel(dev->of_node, DMA_DEV_TO_MEM);
4. rxchan = of_dma_request_channel(dev->of_node, DMA_MEM_TO_DEV);
txchan = of_dma_request_channel(dev->of_node, DMA_DEV_TO_MEM);
5. chan = of_dma_request_named_channel(dev->of_node, "rwdata", 0);
auxchan = of_dma_request_named_channel(dev->of_node, "status", DMA_DEV_TO_MEM);
6. chan = of_dma_request_named_channel(dev->of_node, "rwdata", 0);
auxchan = of_dma_request_named_channel(dev->of_node, "status", DMA_DEV_TO_MEM);
In the above examples, did you imply that the of_dma_request_channel()
function would return a type of "struct dma_chan" and so be calling
dma_request_channel() underneath?
I am been prototyping something, but wanted to make sure I am completely
aligned on this :-)
Cheers
Jon
On Mon, 2012-06-25 at 11:51 -0500, Jon Hunter wrote:
Hi Russell,
On 06/22/2012 06:12 PM, Russell King - ARM Linux wrote:
quoted
Before this goes much further... one fairly obvious and important point
must be made.
You're designing an API here. You're designing it *WITHOUT* involving
the two most important people in its design that there are. The
DMA engine maintainers. Is this how we go about designing APIs - behind
maintainers backs and then presenting it to the maintainers as a fait
accompli?
Absolutely not, this was not the intent and your point is well
understood. I have added Dan and Vinod, and will ensure that he is added
in future.
quoted
There's 86 messages in this thread, none of which have been copied to
them in any way. Why aren't they involved?
Initially this binding was not dma-engine centric. However, I should
have included them in this version from the beginning as I had evolved
it in that direction.
Dan, Vinod, in this thread we have been discussing the addition of a
generic device-tree binding for DMA controllers. In the below, we were
discussing the addition of a device-tree API, which would work as a
wrapper to the dma-engine dma_request_channel() API. I apologise for
adding you late into the discussion. If you have any questions/comments
let me know.
Looks like this a long discussion, I will try to go through archives.
But am still unsure about about dmaengine part. If we have DT binding
for dma controllers, why it they worry about dma_request_channel()
IMO, the problem of channel mapping is not DT specific, it need to be
solved at dmaengine and possibly the required mapping can come from DT
among other mechanisms for various platforms.
This is what google told me about this patch set:
Design of DMA helpers
1. Supporting devices with multiple DMA controllers
In the case of DMA controllers that are using DMA Engine, requesting a
channel is performed by calling the following function.
struct dma_chan *dma_request_channel(dma_cap_mask_t mask,
dma_filter_fn filter_fn,
void *filter_param);
The mask variable is used to identify the device controller in a list of
controllers. The filter_fn and filter_param are used to identify the
required dma channel and return a handle to the dma channel of type
dma_chan. From the examples I have seen, the mask and filter_fn are constant
for a given DMA controller. Therefore, when registering a DMA controller with
device tree we can pass these parameters and store them so that a device can
request them when requesting a channel. Hence, based upon this our register
function for the DMA controller now looks like this.
int of_dma_controller_register(struct device_node *np,
dma_cap_mask_t *mask, dma_filter_fn fn);
IMO we should do away with filer functions.
If we solve the mapping problem, then we don't need a filer.
2. Supporting legacy devices not using DMA Engine
These devices present a problem, as there may not be a uniform way to easily
support them with regard to device tree. However, _IF_ legacy devices that
are not using DMA Engine, only have a single DMA controller, then this
problem is a lot simpler. For example, if we look at the previously proposed
API for registering a DMA controller (where we pass the mask and function
pointer to the DMA Engine filter function) we can simply pass NULL and hence,
a driver requesting the DMA channel information would receive NULL for the
DMA Engine specific parameters. Then for legacy devices we simply need a
means to return the channel information (more on this later). If there are
legacy devices that do have multiple DMA controllers, then maybe they need to
be converted to support DMA Engine. I am not sure if this is unreasonable???
Why should these be supported? They should be converted to use dmaengine
over a reasonable amount of time.
3. Representing and requesting channel information
From a hardware perspective, a DMA channel could be represented as ...
i. channel index/number
ii. channel transfer type (optional)
iii. DMA interrupt mapping (optional)
Please note that the transfer type is used to indicate if the transfer is to
device from memory, to memory from device, to memory from memory, etc. This
can be useful when there is a device such as an MMC device that uses two DMA
channels, one for reading (RX) and one for writing (TX).
From a dma controller perspective, it can service both with single
channel.
I have dma controller which can talk to three peripherals on both
transmit and receive direction. The point is that 1:1 mapping of dma
channel does not exist. So any representation which tries to do this may
not work.
Forgetting device tree for now, some drivers use strings to represent a
DMA channel instead of using an integer. I assume that these drivers then
employ some sort of look-up table to convert the string into a channel
number/index that the hardware understands. If this assumption is correct
then when moving to a device tree implementation having such look-up tables
in the driver should no longer be necessary as the device tree will provide
the mapping of channel index/number to the device. Furthermore, it makes
sense that device tree uses integers to represent channel as opposed to
strings to save the driver having to convert the string into a integer at
some later stage.
Next we need to think about how the DMA controller and channels are described
in the device tree itself. The following device tree node example describes
the properties of the DMA controller that includes, the register address
range, number of interrupt supported, number of channels and number of request
signals. This has been enhanced from the previous versions by adding number of
channels and number of request signals.
sdma: dma-controller at 4A056000 {
compatible = "ti,omap4-sdma";
reg = <0x4A056000 0x1000>;
interrupts = <4>;
#dma-cells = <2>;
#dma-channels = <32>;
#dma-requests = <127>;
};
Given the above controller definition, DMA resources for a device, such as an
MMC that uses two DMA channels, can be declared as follows.
mmc1: mmc at 4809c000 {
...
dma = <&sdma 61 1 &sdma 62 2>;
...
};
The above syntax to represent each DMA resource is "controller-phandle +
dma-channel + transfer-type". The transfer-type here is defined to match the
types in DMA Engine dma_transfer_direction enumeration
(see include/linux/dmaengine.h). Hence, a "1" means DMA_MEM_TO_DEV and a "2"
means "DMA_DEV_TO_MEM". This will be helpful later when requesting the
channel information. You will also notice here that there is no entry for
representing the interrupt used by the DMA channel and this is because this
version has not added this capability. I believe that this will be important
to define in the device tree, but at the moment this has been left out purely
because passing this information was not supported for the device (OMAP) that
I was using to implement this. This could be easily added if people find this
implementation acceptable.
A driver can now request the DMA channel information by calling the following
function.
int of_get_dma_channel_info(struct device_node *np, int type,
struct of_dma_channel_info *info)
Where type represents the transfer-type (again the DMA Engine
dma_transfer_direction enumeration can be used here regardless of if DMA
Engine is used) and of_dma_channel_info is defined as follows.
struct of_dma_channel_info {
int dma_channel;
dma_cap_mask_t dma_cap;
dma_filter_fn dma_filter_func;
};
Here dma_channel will always be valid and the other fields are optional
depending on whether DMA Engine is used.
This implementation has been tested on OMAP4430 using Russell King's latest
DMA Engine series for OMAP [3] and with Benoit Cousson latest DT changes for
OMAP4 [4]. I have validated that MMC is working on the PANDA board with this
implementation. I have not included all the changes for PANDA board here but
just wished to share the implementation.
I am still unclear on how this attempts to solve mapping problem? Maybe
i need more coffee at midnight break!
quoted
On Fri, Jun 22, 2012 at 05:52:08PM -0500, Jon Hunter wrote:
quoted
Hi Arnd,
On 06/14/2012 06:48 AM, Arnd Bergmann wrote:
[snip]
quoted
This would let us handle the following cases very easily:
1. one read-write channel
dmas = <&dmac 0x3 match>;
2. a choice of two read-write channels:
dmas = <&dmacA 0x3 matchA>, <&dmacB 0x3 matchB>;
3. one read-channel, one write channel:
dmas = <&dmac 0x1 match-read>, <&dmac 0x2 match-write>;
4. a choice of two read channels and one write channel:
dmas = <&dmacA 0x1 match-readA>, <&dmacA 0x2 match-write>
<&dmacB match-readB>;
And only the cases where we have more multiple channels that differ
in more aspects would require named properties:
5. two different channels
dmas = <&dmac 0x3 match-rwdata>, <&dmac 0x1 match-status>;
dma-names = "rwdata", "status";
6. same as 5, but with a choice of channels:
dmas = <&dmacA 0x3 match-rwdataA>, <&dmacA 0x1 match-status>;
<dmacB 0x3 match-rwdataB>;
dma-names = "rwdata", "status", "rwdata";
With a definition like that, we can implement a very simple device
driver interface for the common cases, and a slightly more complex
one for the more complex cases:
1. chan = of_dma_request_channel(dev->of_node, 0);
2. chan = of_dma_request_channel(dev->of_node, 0);
3. rxchan = of_dma_request_channel(dev->of_node, DMA_MEM_TO_DEV);
txchan = of_dma_request_channel(dev->of_node, DMA_DEV_TO_MEM);
4. rxchan = of_dma_request_channel(dev->of_node, DMA_MEM_TO_DEV);
txchan = of_dma_request_channel(dev->of_node, DMA_DEV_TO_MEM);
5. chan = of_dma_request_named_channel(dev->of_node, "rwdata", 0);
auxchan = of_dma_request_named_channel(dev->of_node, "status", DMA_DEV_TO_MEM);
6. chan = of_dma_request_named_channel(dev->of_node, "rwdata", 0);
auxchan = of_dma_request_named_channel(dev->of_node, "status", DMA_DEV_TO_MEM);
In the above examples, did you imply that the of_dma_request_channel()
function would return a type of "struct dma_chan" and so be calling
dma_request_channel() underneath?
I am been prototyping something, but wanted to make sure I am completely
aligned on this :-)
Cheers
Jon
On Mon, 2012-06-25 at 11:51 -0500, Jon Hunter wrote:
quoted
Hi Russell,
quoted
Dan, Vinod, in this thread we have been discussing the addition of a
generic device-tree binding for DMA controllers. In the below, we were
discussing the addition of a device-tree API, which would work as a
wrapper to the dma-engine dma_request_channel() API. I apologise for
adding you late into the discussion. If you have any questions/comments
let me know.
Looks like this a long discussion, I will try to go through archives.
But am still unsure about about dmaengine part. If we have DT binding
for dma controllers, why it they worry about dma_request_channel()
IMO, the problem of channel mapping is not DT specific, it need to be
solved at dmaengine and possibly the required mapping can come from DT
among other mechanisms for various platforms.
This is what google told me about this patch set:
dma_request_channel is called with some information about the channel
provided in its arguments, and the driver might get that from a number
of places.
In the case of having a fully populated device tree with this binding,
the driver calling (of_)dma_request_channel does not need to know about
any of that information because we should be able to encapsulate that
completely in device tree data. It does not replace the regular interface
but wraps around it to provide a higher abstraction level where possible.
Of course if you think we should not be doing that but instead
have of_dma_request_channel() live besides dma_request_channel()
rather than calling it, that should be absolutely fine too.
quoted
In the case of DMA controllers that are using DMA Engine, requesting a
channel is performed by calling the following function.
struct dma_chan *dma_request_channel(dma_cap_mask_t mask,
dma_filter_fn filter_fn,
void *filter_param);
The mask variable is used to identify the device controller in a list of
controllers. The filter_fn and filter_param are used to identify the
required dma channel and return a handle to the dma channel of type
dma_chan. From the examples I have seen, the mask and filter_fn are constant
for a given DMA controller. Therefore, when registering a DMA controller with
device tree we can pass these parameters and store them so that a device can
request them when requesting a channel. Hence, based upon this our register
function for the DMA controller now looks like this.
int of_dma_controller_register(struct device_node *np,
dma_cap_mask_t *mask, dma_filter_fn fn);
IMO we should do away with filter functions.
If we solve the mapping problem, then we don't need a filer.
The channel data in the device tree is still in a format
that is specific to that dmaengine driver and interpreted
by it. Using the regular dma_filter_fn prototype is not
necessary, but it would be convenient because the dmaengine
code already knows how to deal with it. If we don't use this
method, how about adding another callback to struct dma_device
like
bool (*device_match)(struct dma_chan *chan, struct property *req);
quoted
2. Supporting legacy devices not using DMA Engine
These devices present a problem, as there may not be a uniform way to easily
support them with regard to device tree. However, _IF_ legacy devices that
are not using DMA Engine, only have a single DMA controller, then this
problem is a lot simpler. For example, if we look at the previously proposed
API for registering a DMA controller (where we pass the mask and function
pointer to the DMA Engine filter function) we can simply pass NULL and hence,
a driver requesting the DMA channel information would receive NULL for the
DMA Engine specific parameters. Then for legacy devices we simply need a
means to return the channel information (more on this later). If there are
legacy devices that do have multiple DMA controllers, then maybe they need to
be converted to support DMA Engine. I am not sure if this is unreasonable???
Why should these be supported? They should be converted to use dmaengine
over a reasonable amount of time.
I agree, at least for the long run. However, that is a separate issue to work on.
Right now we need a generic way to represent dma requests independent of how
they are used in the kernel. The device tree binding is supposed to be
operating system independent so there should be nothing in it that requires
the use of the linux dmaengine code.
For drivers that do not use dmaengine, we have to make a decision whether
it's worth adding support for the DT binding first and converting the driver
and its users to dmaengine later, or whether it's better to use the dmaengine
API right away to avoid having to do changes twice.
quoted
3. Representing and requesting channel information
From a hardware perspective, a DMA channel could be represented as ...
i. channel index/number
ii. channel transfer type (optional)
iii. DMA interrupt mapping (optional)
Please note that the transfer type is used to indicate if the transfer is to
device from memory, to memory from device, to memory from memory, etc. This
can be useful when there is a device such as an MMC device that uses two DMA
channels, one for reading (RX) and one for writing (TX).
From a dma controller perspective, it can service both with single
channel.
I have dma controller which can talk to three peripherals on both
transmit and receive direction. The point is that 1:1 mapping of dma
channel does not exist. So any representation which tries to do this may
not work.
In the device tree, we know both the dmaengine and its slave, so we also know
whether to put one or more requests in there.
quoted
struct of_dma_channel_info {
int dma_channel;
dma_cap_mask_t dma_cap;
dma_filter_fn dma_filter_func;
};
Here dma_channel will always be valid and the other fields are optional
depending on whether DMA Engine is used.
This implementation has been tested on OMAP4430 using Russell King's latest
DMA Engine series for OMAP [3] and with Benoit Cousson latest DT changes for
OMAP4 [4]. I have validated that MMC is working on the PANDA board with this
implementation. I have not included all the changes for PANDA board here but
just wished to share the implementation.
I am still unclear on how this attempts to solve mapping problem? Maybe
i need more coffee at midnight break!
I believe we have moved on from this proposal to a simpler one,
doing away with the of_dma_channel_info.
quoted
quoted
On Fri, Jun 22, 2012 at 05:52:08PM -0500, Jon Hunter wrote:
In the above examples, did you imply that the of_dma_request_channel()
function would return a type of "struct dma_chan" and so be calling
dma_request_channel() underneath?
I am been prototyping something, but wanted to make sure I am completely
aligned on this :-)
This is what I think we need to be heading to.
Arnd
On Mon, 2012-06-25 at 20:30 +0000, Arnd Bergmann wrote:
dma_request_channel is called with some information about the channel
provided in its arguments, and the driver might get that from a number
of places.
Today, we just ask for a channel with specific mask. Further filtering
is done in filter function as we request a channel, not a specific one.
In most slave cases, we need a specific channel from a specific
controller, and that is where DT can play a role. In addition to DMA
resources for dma and client driver, I would expect DT to provide the
channel mapping information, which is anyway known only by platform.
That should be a dmaengine binding and not client or controller
specific. For different platforms this information can come from DT or
something else.
Then, once a channel is requested dmaengine knows what to provide.
And as you see the filter becomes redundant.
In the case of having a fully populated device tree with this binding,
the driver calling (of_)dma_request_channel does not need to know about
any of that information because we should be able to encapsulate that
completely in device tree data. It does not replace the regular interface
but wraps around it to provide a higher abstraction level where possible.
Of course if you think we should not be doing that but instead
have of_dma_request_channel() live besides dma_request_channel()
rather than calling it, that should be absolutely fine too.
quoted
quoted
In the case of DMA controllers that are using DMA Engine, requesting a
channel is performed by calling the following function.
struct dma_chan *dma_request_channel(dma_cap_mask_t mask,
dma_filter_fn filter_fn,
void *filter_param);
The mask variable is used to identify the device controller in a list of
controllers. The filter_fn and filter_param are used to identify the
required dma channel and return a handle to the dma channel of type
dma_chan. From the examples I have seen, the mask and filter_fn are constant
for a given DMA controller. Therefore, when registering a DMA controller with
device tree we can pass these parameters and store them so that a device can
request them when requesting a channel. Hence, based upon this our register
function for the DMA controller now looks like this.
int of_dma_controller_register(struct device_node *np,
dma_cap_mask_t *mask, dma_filter_fn fn);
IMO we should do away with filter functions.
If we solve the mapping problem, then we don't need a filer.
The channel data in the device tree is still in a format
that is specific to that dmaengine driver and interpreted
by it. Using the regular dma_filter_fn prototype is not
necessary, but it would be convenient because the dmaengine
code already knows how to deal with it. If we don't use this
method, how about adding another callback to struct dma_device
like
bool (*device_match)(struct dma_chan *chan, struct property *req);
quoted
quoted
2. Supporting legacy devices not using DMA Engine
These devices present a problem, as there may not be a uniform way to easily
support them with regard to device tree. However, _IF_ legacy devices that
are not using DMA Engine, only have a single DMA controller, then this
problem is a lot simpler. For example, if we look at the previously proposed
API for registering a DMA controller (where we pass the mask and function
pointer to the DMA Engine filter function) we can simply pass NULL and hence,
a driver requesting the DMA channel information would receive NULL for the
DMA Engine specific parameters. Then for legacy devices we simply need a
means to return the channel information (more on this later). If there are
legacy devices that do have multiple DMA controllers, then maybe they need to
be converted to support DMA Engine. I am not sure if this is unreasonable???
Why should these be supported? They should be converted to use dmaengine
over a reasonable amount of time.
I agree, at least for the long run. However, that is a separate issue to work on.
Right now we need a generic way to represent dma requests independent of how
they are used in the kernel. The device tree binding is supposed to be
operating system independent so there should be nothing in it that requires
the use of the linux dmaengine code.
For drivers that do not use dmaengine, we have to make a decision whether
it's worth adding support for the DT binding first and converting the driver
and its users to dmaengine later, or whether it's better to use the dmaengine
API right away to avoid having to do changes twice.
Latter please :)
quoted
quoted
3. Representing and requesting channel information
From a hardware perspective, a DMA channel could be represented as ...
i. channel index/number
ii. channel transfer type (optional)
iii. DMA interrupt mapping (optional)
Please note that the transfer type is used to indicate if the transfer is to
device from memory, to memory from device, to memory from memory, etc. This
can be useful when there is a device such as an MMC device that uses two DMA
channels, one for reading (RX) and one for writing (TX).
From a dma controller perspective, it can service both with single
channel.
I have dma controller which can talk to three peripherals on both
transmit and receive direction. The point is that 1:1 mapping of dma
channel does not exist. So any representation which tries to do this may
not work.
In the device tree, we know both the dmaengine and its slave, so we also know
whether to put one or more requests in there.
quoted
quoted
struct of_dma_channel_info {
int dma_channel;
dma_cap_mask_t dma_cap;
dma_filter_fn dma_filter_func;
};
Here dma_channel will always be valid and the other fields are optional
depending on whether DMA Engine is used.
This implementation has been tested on OMAP4430 using Russell King's latest
DMA Engine series for OMAP [3] and with Benoit Cousson latest DT changes for
OMAP4 [4]. I have validated that MMC is working on the PANDA board with this
implementation. I have not included all the changes for PANDA board here but
just wished to share the implementation.
I am still unclear on how this attempts to solve mapping problem? Maybe
i need more coffee at midnight break!
I believe we have moved on from this proposal to a simpler one,
doing away with the of_dma_channel_info.
quoted
quoted
quoted
On Fri, Jun 22, 2012 at 05:52:08PM -0500, Jon Hunter wrote:
In the above examples, did you imply that the of_dma_request_channel()
function would return a type of "struct dma_chan" and so be calling
dma_request_channel() underneath?
I am been prototyping something, but wanted to make sure I am completely
aligned on this :-)
This is what I think we need to be heading to.
I am not sure about this, maybe haven't understood all details yet.
But, I was hoping the DT binding will be "hidden". DMAengine driver will
get resource information from DT. Clients will get DMA resource
information from DT. And if possible dmaengine gets mapping information
from DT.
So then why should we have xx_dma_xxx apis?
Today, we just ask for a channel with specific mask. Further filtering
is done in filter function as we request a channel, not a specific one.
In most slave cases, we need a specific channel from a specific
controller, and that is where DT can play a role. In addition to DMA
resources for dma and client driver, I would expect DT to provide the
channel mapping information, which is anyway known only by platform.
Can you describe what you mean by "channel mapping information"?
Is that not what we pass into the filter function?
That should be a dmaengine binding and not client or controller
specific. For different platforms this information can come from DT or
something else.
Then, once a channel is requested dmaengine knows what to provide.
And as you see the filter becomes redundant.
But what code interprets the channel mapping then?
On Mon, 2012-06-25 at 20:30 +0000, Arnd Bergmann wrote:
quoted
I agree, at least for the long run. However, that is a separate issue to work on.
Right now we need a generic way to represent dma requests independent of how
they are used in the kernel. The device tree binding is supposed to be
operating system independent so there should be nothing in it that requires
the use of the linux dmaengine code.
For drivers that do not use dmaengine, we have to make a decision whether
it's worth adding support for the DT binding first and converting the driver
and its users to dmaengine later, or whether it's better to use the dmaengine
API right away to avoid having to do changes twice.
Latter please :)
I'd always leave that decision up to the author of each driver that gets
converted. Fortunately there are very few left that are not already using
the dmaengine interfaces.
quoted
quoted
quoted
quoted
On Fri, Jun 22, 2012 at 05:52:08PM -0500, Jon Hunter wrote:
In the above examples, did you imply that the of_dma_request_channel()
function would return a type of "struct dma_chan" and so be calling
dma_request_channel() underneath?
I am been prototyping something, but wanted to make sure I am completely
aligned on this :-)
This is what I think we need to be heading to.
I am not sure about this, maybe haven't understood all details yet.
But, I was hoping the DT binding will be "hidden". DMAengine driver will
get resource information from DT. Clients will get DMA resource
information from DT. And if possible dmaengine gets mapping information
from DT.
So then why should we have xx_dma_xxx apis?
I think encoding a description for a dma request in a single number is
the last thing we want to do here. We've tried that with IRQ and GPIO
numbers and it got us into a huge mess that will need a long time to
get out of.
Some platforms actually use IORESOURCE_DMA, which was useful to describe
ISA DMA channels, for encoding some form of channel or request number,
but this causes all sorts of problems. These are almost exclusively
used by those platforms that don't have a dmaengine driver yet, so I'd
hope that we can remove this as we convert those platforms over to
dmaengine and device tree.
The representation in device tree as we have it now is a combination of
a pointer to the dmaengine and a description of the request line in it,
typically a single small integer number local to the dmaengine. We should
not try to make that a global integer number again that just serves the
purpose of looking up the dmaengine and local number again.
IMHO no device driver should be bothered with any artificial resource
information, but instead I want all the DT parsing to happen in the
dmaengine code (or some wrapper around it) where it gets used. The only
thing that a device driver needs to know is that it wants to use a
channel based on what is described in the device tree.
Arnd
On Tue, 2012-06-26 at 14:59 +0000, Arnd Bergmann wrote:
On Tuesday 26 June 2012, Vinod Koul wrote:
quoted
Today, we just ask for a channel with specific mask. Further filtering
is done in filter function as we request a channel, not a specific one.
In most slave cases, we need a specific channel from a specific
controller, and that is where DT can play a role. In addition to DMA
resources for dma and client driver, I would expect DT to provide the
channel mapping information, which is anyway known only by platform.
Can you describe what you mean by "channel mapping information"?
Is that not what we pass into the filter function?
Today many dmaengine drivers have a filter function which is exported
and then used by clients to filter out the channel. That is not a right
way to do, so any future plan which is based on filter is not correct.
IMHO dmaengine driver should *not* know anything about mapping. By
mapping I refer to platform information which tells me which client can
use which channel from which dmac.
If the dmaengine hardware has mux then we have flexible mapping, other
cases would be where request lines are hard wired, so mapping is pretty
much static.
This information if provided to dmaengine can result in dmaengine doing
proper allocation of dma channels.
One of the proposals we discussed sometime back:
https://lkml.org/lkml/2012/3/8/26
quoted
That should be a dmaengine binding and not client or controller
specific. For different platforms this information can come from DT or
something else.
Then, once a channel is requested dmaengine knows what to provide.
And as you see the filter becomes redundant.
But what code interprets the channel mapping then?
only dmaengine. In this case the mapping comes from DT.
I think encoding a description for a dma request in a single number is
the last thing we want to do here. We've tried that with IRQ and GPIO
numbers and it got us into a huge mess that will need a long time to
get out of.
No i wasn't thinking of a number. The mapping shouldn't be a global
number at all, though that is a very easy but not very scalable
solution.
We need to take care of 1:1 mapping of client and channels as well as
many:1 cases as well. A single global number cannot represent that
properly.
My idea is platform gives this information to dmaengine. Clients and
dmaengine driver do not worry about it. That also paves way for arch
independent clients and drivers.
Some platforms actually use IORESOURCE_DMA, which was useful to describe
ISA DMA channels, for encoding some form of channel or request number,
but this causes all sorts of problems. These are almost exclusively
used by those platforms that don't have a dmaengine driver yet, so I'd
hope that we can remove this as we convert those platforms over to
dmaengine and device tree.
The representation in device tree as we have it now is a combination of
a pointer to the dmaengine and a description of the request line in it,
typically a single small integer number local to the dmaengine. We should
not try to make that a global integer number again that just serves the
purpose of looking up the dmaengine and local number again.
IMHO no device driver should be bothered with any artificial resource
information, but instead I want all the DT parsing to happen in the
dmaengine code (or some wrapper around it) where it gets used. The only
thing that a device driver needs to know is that it wants to use a
channel based on what is described in the device tree.
Sure, but I would expect the clients and dmacs to find information about
their devices from DT?
dmaengine should get only mapping information used for allocating
channel to client.
--
~Vinod
On Tue, 2012-06-26 at 14:59 +0000, Arnd Bergmann wrote:
quoted
On Tuesday 26 June 2012, Vinod Koul wrote:
quoted
Today, we just ask for a channel with specific mask. Further filtering
is done in filter function as we request a channel, not a specific one.
In most slave cases, we need a specific channel from a specific
controller, and that is where DT can play a role. In addition to DMA
resources for dma and client driver, I would expect DT to provide the
channel mapping information, which is anyway known only by platform.
Can you describe what you mean by "channel mapping information"?
Is that not what we pass into the filter function?
Today many dmaengine drivers have a filter function which is exported
and then used by clients to filter out the channel. That is not a right
way to do, so any future plan which is based on filter is not correct.
I agree that exporting a filter function from the dmaengine driver is
very wrong and should not be done because it requires that the device
driver knows which engine is used and that is contrary to the idea
of having an abstraction layer.
We were talking about using a filter function because that would be
easy to do without changing the core dmaengine code. However, in the
proposal, there would actually just be a single filter function that
gets used by all drivers that have DT bindings, and it can be
completely encapsulated in the of_dma_request_channel() function
so it does not have to be a global symbol.
If we instead modify the dmaengine code itself to know about DT
rather than wrapping around it, we would not need this filter
function, but we should still have a probe() function that is
called by dmaengine code to interpret the data that is specific
to one dmaengine driver.
IMHO dmaengine driver should *not* know anything about mapping. By
mapping I refer to platform information which tells me which client can
use which channel from which dmac.
Agreed too. That information shoudd be part of the slave device-node
in DT, as I have argued in this thread already. The slave device driver
does not need to care about the format or the contents of it,
but we need some code to interpret the contents. From all I can tell,
the structure of this data cannot be completely generic because of
all the special cases, so the code to interpret it would live in the
probe() function I mentioned about that the dmaengine driver provides.
quoted
I think encoding a description for a dma request in a single number is
the last thing we want to do here. We've tried that with IRQ and GPIO
numbers and it got us into a huge mess that will need a long time to
get out of.
No i wasn't thinking of a number. The mapping shouldn't be a global
number at all, though that is a very easy but not very scalable
solution.
We need to take care of 1:1 mapping of client and channels as well as
many:1 cases as well. A single global number cannot represent that
properly.
My idea is platform gives this information to dmaengine. Clients and
dmaengine driver do not worry about it. That also paves way for arch
independent clients and drivers.
IMO the platform should have no part in this. I absolutely want to
get rid of any platform-specific hardcoded tables in the kernel for
stuff that can easily be run-time detected from the device tree.
There are cases where hard-coding in the kernel is easier, but I don't
think this is one of them.
quoted
Some platforms actually use IORESOURCE_DMA, which was useful to describe
ISA DMA channels, for encoding some form of channel or request number,
but this causes all sorts of problems. These are almost exclusively
used by those platforms that don't have a dmaengine driver yet, so I'd
hope that we can remove this as we convert those platforms over to
dmaengine and device tree.
The representation in device tree as we have it now is a combination of
a pointer to the dmaengine and a description of the request line in it,
typically a single small integer number local to the dmaengine. We should
not try to make that a global integer number again that just serves the
purpose of looking up the dmaengine and local number again.
IMHO no device driver should be bothered with any artificial resource
information, but instead I want all the DT parsing to happen in the
dmaengine code (or some wrapper around it) where it gets used. The only
thing that a device driver needs to know is that it wants to use a
channel based on what is described in the device tree.
Sure, but I would expect the clients and dmacs to find information about
their devices from DT?
dmaengine should get only mapping information used for allocating
channel to client.
Let's take a look at a concrete example. The
arch/arm/mach-ux500/board-mop500-sdi.c file defines dma channel
configuration for the mmc devices on the ux500 platform that looks
like
static struct stedma40_chan_cfg mop500_sdi2_dma_cfg_tx = {
.mode = STEDMA40_MODE_LOGICAL,
.dir = STEDMA40_MEM_TO_PERIPH,
.src_dev_type = STEDMA40_DEV_SRC_MEMORY,
.dst_dev_type = DB8500_DMA_DEV28_SD_MM2_TX,
.src_info.data_width = STEDMA40_WORD_WIDTH,
.dst_info.data_width = STEDMA40_WORD_WIDTH,
};
I want to move this information to the device tree in a way that the
device driver does not have to care about it. With the proposed
binding, this would mean we get an mmci device node with a property
containing something like
dma-requests = <&dma40 /* pointer to dma engine */
0x01 /* logical, mem to dev */
28 /* DEV28_SD_MM2_TX */
32>, /* 32 bit width */
<&dma40 0x02 32 32>; /* dev to mem channel */
The fact that this dmaengine driver requires two cells (request number
and data width) should only be known to the code that deals with that
one driver and that should interpret those two cells, while the first
two cells (pointer to dma-engine and direction) can be handled
by the common dmaengine layer.
In order to do that, we need some code in the dmaengine driver that
gets the property data as its argument.
Arnd
On Tue, 2012-06-26 at 20:27 +0000, Arnd Bergmann wrote:
On Tuesday 26 June 2012, Vinod Koul wrote:
quoted
On Tue, 2012-06-26 at 14:59 +0000, Arnd Bergmann wrote:
quoted
On Tuesday 26 June 2012, Vinod Koul wrote:
quoted
Today, we just ask for a channel with specific mask. Further filtering
is done in filter function as we request a channel, not a specific one.
In most slave cases, we need a specific channel from a specific
controller, and that is where DT can play a role. In addition to DMA
resources for dma and client driver, I would expect DT to provide the
channel mapping information, which is anyway known only by platform.
Can you describe what you mean by "channel mapping information"?
Is that not what we pass into the filter function?
Today many dmaengine drivers have a filter function which is exported
and then used by clients to filter out the channel. That is not a right
way to do, so any future plan which is based on filter is not correct.
I agree that exporting a filter function from the dmaengine driver is
very wrong and should not be done because it requires that the device
driver knows which engine is used and that is contrary to the idea
of having an abstraction layer.
We were talking about using a filter function because that would be
easy to do without changing the core dmaengine code. However, in the
proposal, there would actually just be a single filter function that
gets used by all drivers that have DT bindings, and it can be
completely encapsulated in the of_dma_request_channel() function
so it does not have to be a global symbol.
I kind of like this idea.
If we instead modify the dmaengine code itself to know about DT
rather than wrapping around it, we would not need this filter
function, but we should still have a probe() function that is
called by dmaengine code to interpret the data that is specific
to one dmaengine driver.
I was hoping we can have dmaengine binding, that way dmaengine core code
knows about what to do when some client requests a channel.
quoted
IMHO dmaengine driver should *not* know anything about mapping. By
mapping I refer to platform information which tells me which client can
use which channel from which dmac.
Agreed too. That information shoudd be part of the slave device-node
in DT, as I have argued in this thread already. The slave device driver
does not need to care about the format or the contents of it,
but we need some code to interpret the contents. From all I can tell,
the structure of this data cannot be completely generic because of
all the special cases, so the code to interpret it would live in the
probe() function I mentioned about that the dmaengine driver provides.
rather than slave driver, why dont we keep this binding within
dmaengine. That would make slave and clients completely independent.
quoted
quoted
I think encoding a description for a dma request in a single number is
the last thing we want to do here. We've tried that with IRQ and GPIO
numbers and it got us into a huge mess that will need a long time to
get out of.
No i wasn't thinking of a number. The mapping shouldn't be a global
number at all, though that is a very easy but not very scalable
solution.
We need to take care of 1:1 mapping of client and channels as well as
many:1 cases as well. A single global number cannot represent that
properly.
My idea is platform gives this information to dmaengine. Clients and
dmaengine driver do not worry about it. That also paves way for arch
independent clients and drivers.
IMO the platform should have no part in this. I absolutely want to
get rid of any platform-specific hardcoded tables in the kernel for
stuff that can easily be run-time detected from the device tree.
There are cases where hard-coding in the kernel is easier, but I don't
think this is one of them.
Again, you got me wrong. We don't want any hardcoded table is kernel.
The information in table should come from whatever way that platform can
give me. For your case it should be DT.
We can have the map of which client can use which channel as DT binding
of dmaengine core. So dmaengine can easily arbitrate about channel
requests. Again this mapping information is not some even linux
independent
quoted
quoted
Some platforms actually use IORESOURCE_DMA, which was useful to describe
ISA DMA channels, for encoding some form of channel or request number,
but this causes all sorts of problems. These are almost exclusively
used by those platforms that don't have a dmaengine driver yet, so I'd
hope that we can remove this as we convert those platforms over to
dmaengine and device tree.
The representation in device tree as we have it now is a combination of
a pointer to the dmaengine and a description of the request line in it,
typically a single small integer number local to the dmaengine. We should
not try to make that a global integer number again that just serves the
purpose of looking up the dmaengine and local number again.
IMHO no device driver should be bothered with any artificial resource
information, but instead I want all the DT parsing to happen in the
dmaengine code (or some wrapper around it) where it gets used. The only
thing that a device driver needs to know is that it wants to use a
channel based on what is described in the device tree.
Sure, but I would expect the clients and dmacs to find information about
their devices from DT?
dmaengine should get only mapping information used for allocating
channel to client.
Let's take a look at a concrete example. The
arch/arm/mach-ux500/board-mop500-sdi.c file defines dma channel
configuration for the mmc devices on the ux500 platform that looks
like
static struct stedma40_chan_cfg mop500_sdi2_dma_cfg_tx = {
.mode = STEDMA40_MODE_LOGICAL,
.dir = STEDMA40_MEM_TO_PERIPH,
.src_dev_type = STEDMA40_DEV_SRC_MEMORY,
.dst_dev_type = DB8500_DMA_DEV28_SD_MM2_TX,
.src_info.data_width = STEDMA40_WORD_WIDTH,
.dst_info.data_width = STEDMA40_WORD_WIDTH,
};
I want to move this information to the device tree in a way that the
device driver does not have to care about it. With the proposed
binding, this would mean we get an mmci device node with a property
containing something like
dma-requests = <&dma40 /* pointer to dma engine */
0x01 /* logical, mem to dev */
28 /* DEV28_SD_MM2_TX */
32>, /* 32 bit width */
<&dma40 0x02 32 32>; /* dev to mem channel */
The fact that this dmaengine driver requires two cells (request number
and data width) should only be known to the code that deals with that
one driver and that should interpret those two cells, while the first
two cells (pointer to dma-engine and direction) can be handled
by the common dmaengine layer.
In order to do that, we need some code in the dmaengine driver that
gets the property data as its argument.
On Tue, 2012-06-26 at 20:27 +0000, Arnd Bergmann wrote:
quoted
On Tuesday 26 June 2012, Vinod Koul wrote:
quoted
On Tue, 2012-06-26 at 14:59 +0000, Arnd Bergmann wrote:
If we instead modify the dmaengine code itself to know about DT
rather than wrapping around it, we would not need this filter
function, but we should still have a probe() function that is
called by dmaengine code to interpret the data that is specific
to one dmaengine driver.
I was hoping we can have dmaengine binding, that way dmaengine core code
knows about what to do when some client requests a channel.
quoted
quoted
IMHO dmaengine driver should *not* know anything about mapping. By
mapping I refer to platform information which tells me which client can
use which channel from which dmac.
Agreed too. That information shoudd be part of the slave device-node
in DT, as I have argued in this thread already. The slave device driver
does not need to care about the format or the contents of it,
but we need some code to interpret the contents. From all I can tell,
the structure of this data cannot be completely generic because of
all the special cases, so the code to interpret it would live in the
probe() function I mentioned about that the dmaengine driver provides.
rather than slave driver, why dont we keep this binding within
dmaengine. That would make slave and clients completely independent.
Sorry, I believe I was just using the wrong terminology, and what I named
the slave here would just be the client.
This may have contributed to a lot of confusion before, so let's make
sure I use the right terms now:
a) slave == dmac == dmaengine driver
b) client == device driver, e.g. mmc
c) common code == dmaengine layer
Is this correct?
quoted
quoted
quoted
I think encoding a description for a dma request in a single number is
the last thing we want to do here. We've tried that with IRQ and GPIO
numbers and it got us into a huge mess that will need a long time to
get out of.
No i wasn't thinking of a number. The mapping shouldn't be a global
number at all, though that is a very easy but not very scalable
solution.
We need to take care of 1:1 mapping of client and channels as well as
many:1 cases as well. A single global number cannot represent that
properly.
My idea is platform gives this information to dmaengine. Clients and
dmaengine driver do not worry about it. That also paves way for arch
independent clients and drivers.
IMO the platform should have no part in this. I absolutely want to
get rid of any platform-specific hardcoded tables in the kernel for
stuff that can easily be run-time detected from the device tree.
There are cases where hard-coding in the kernel is easier, but I don't
think this is one of them.
Again, you got me wrong. We don't want any hardcoded table is kernel.
The information in table should come from whatever way that platform can
give me. For your case it should be DT.
Ok, good.
We can have the map of which client can use which channel as DT binding
of dmaengine core. So dmaengine can easily arbitrate about channel
requests. Again this mapping information is not some even linux
independent.
Why can't it be OS independent?
Do you mean there must be a global table, or are you ok with putting
the information about a channel into the device that uses the channel,
as we do for most other subsystems (IRQ, GPIO, pinctrl, ...).
If not, what is the problem with that approach?
Arnd
The channel data in the device tree is still in a format
that is specific to that dmaengine driver and interpreted
by it. Using the regular dma_filter_fn prototype is not
necessary, but it would be convenient because the dmaengine
code already knows how to deal with it. If we don't use this
method, how about adding another callback to struct dma_device
like
bool (*device_match)(struct dma_chan *chan, struct property *req);
I like this idea, but why don't we extend it to also cover the non-DT
case? I.e., why don't we add the above callback (call it "match" or
"filter" or anything else) to dmaengine operations and inside (the
extended) dma_request_channel(), instead of calling the filter function,
passed as a parameter, we loop over all registered DMAC devices and call
their filter callbacks, until one of them returns true? In fact, it goes
back to my earlier proposal from
http://thread.gmane.org/gmane.linux.kernel/1246957
which I, possibly, failed to explain properly. So, the transformation
chain from today's API would be (all code is approximate):
(today)
<client driver>
dma_request_channel(mask, filter, filter_arg);
<dmaengine_core>
for_each_channel() {
ret = (*filter)(chan, filter_arg);
if (ret) {
ret = chan->device->device_alloc_chan_resources(chan);
if (!ret)
return chan;
else
return NULL;
}
}
(can be transformed to)
<client driver>
dma_request_channel(mask, filter_arg);
<dmaengine_core>
for_each_channel() {
ret = chan->device->filter(chan, filter_arg);
if (ret) {
<same as above>
}
}
(which further could be simplified to)
<client driver>
dma_request_channel(mask, filter_arg);
<dmaengine_core>
for_each_channel() {
ret = chan->device->device_alloc_chan_resources(chan, filter_arg);
if (!ret)
return chan;
else if (ret != -ENODEV)
return ret;
/* -ENODEV - try the next channel */
}
Which is quite similar to my above mentioned proposal. Wouldn't this both
improve the present API and prepare it for DT?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
On Wed, 2012-06-27 at 15:20 +0000, Arnd Bergmann wrote:
Back from vacation... so restart the pending discussion
Sorry, I believe I was just using the wrong terminology, and what I named
the slave here would just be the client.
This may have contributed to a lot of confusion before, so let's make
sure I use the right terms now:
a) slave == dmac == dmaengine driver
b) client == device driver, e.g. mmc
c) common code == dmaengine layer
Is this correct?
Yup, that is what i use.
quoted
quoted
quoted
quoted
I think encoding a description for a dma request in a single number is
the last thing we want to do here. We've tried that with IRQ and GPIO
numbers and it got us into a huge mess that will need a long time to
get out of.
No i wasn't thinking of a number. The mapping shouldn't be a global
number at all, though that is a very easy but not very scalable
solution.
We need to take care of 1:1 mapping of client and channels as well as
many:1 cases as well. A single global number cannot represent that
properly.
My idea is platform gives this information to dmaengine. Clients and
dmaengine driver do not worry about it. That also paves way for arch
independent clients and drivers.
IMO the platform should have no part in this. I absolutely want to
get rid of any platform-specific hardcoded tables in the kernel for
stuff that can easily be run-time detected from the device tree.
There are cases where hard-coding in the kernel is easier, but I don't
think this is one of them.
Again, you got me wrong. We don't want any hardcoded table is kernel.
The information in table should come from whatever way that platform can
give me. For your case it should be DT.
Ok, good.
quoted
We can have the map of which client can use which channel as DT binding
of dmaengine core. So dmaengine can easily arbitrate about channel
requests. Again this mapping information is not some even linux
independent.
Why can't it be OS independent?
I meant OS independent, didn't come out well :(
Do you mean there must be a global table, or are you ok with putting
the information about a channel into the device that uses the channel,
as we do for most other subsystems (IRQ, GPIO, pinctrl, ...).
If not, what is the problem with that approach?
Today, we simple ask, "give me dma channel with DMA_SLAVE capability".
If we change it to "give me dma channel which suits my need" and have
additional information in dmaengine to handle this request effectively.
What that would mean is
a) DMA channel either knows which channel to provide, Or
b) Additional arguments provided to dmaengine API to help it find out
which channel to provide.
It would be good to have client ask for a specific channel. But in order
to build generic clients, we face a problem that clients may not know
how they mapped to dmac by SoC designer. Or the mux maybe entirely
flexible on which channel.
If we add this as DT property (which I assume should be platform
specific), then client will know which channel to request.
It can have two levels, dmac and channel. In case mux is flexible enough
then client gets a channel and program the mux for this mapping.
I think this is the most simplistic solution I have for this, thoughts?
--
~Vinod
On Fri, 2012-07-06 at 13:36 +0200, Guennadi Liakhovetski wrote:
On Mon, 25 Jun 2012, Arnd Bergmann wrote:
[snip]
quoted
The channel data in the device tree is still in a format
that is specific to that dmaengine driver and interpreted
by it. Using the regular dma_filter_fn prototype is not
necessary, but it would be convenient because the dmaengine
code already knows how to deal with it. If we don't use this
method, how about adding another callback to struct dma_device
like
bool (*device_match)(struct dma_chan *chan, struct property *req);
I like this idea, but why don't we extend it to also cover the non-DT
case? I.e., why don't we add the above callback (call it "match" or
"filter" or anything else) to dmaengine operations and inside (the
extended) dma_request_channel(), instead of calling the filter function,
passed as a parameter, we loop over all registered DMAC devices and call
their filter callbacks,
And I have told you many times, that dmacs should not know anything
about clients. They should be totally agnostic to it.
Clients need to request a specific channel, and that is where changes
should come for. Not having dmac provide one.
--
~Vinod
On Wed, 2012-06-27 at 15:20 +0000, Arnd Bergmann wrote:
[snip]
quoted
Do you mean there must be a global table, or are you ok with putting
the information about a channel into the device that uses the channel,
as we do for most other subsystems (IRQ, GPIO, pinctrl, ...).
If not, what is the problem with that approach?
Today, we simple ask, "give me dma channel with DMA_SLAVE capability".
If we change it to "give me dma channel which suits my need" and have
additional information in dmaengine to handle this request effectively.
What that would mean is
a) DMA channel either knows which channel to provide, Or
b) Additional arguments provided to dmaengine API to help it find out
which channel to provide.
It would be good to have client ask for a specific channel. But in order
to build generic clients, we face a problem that clients may not know
how they mapped to dmac by SoC designer. Or the mux maybe entirely
flexible on which channel.
If we add this as DT property (which I assume should be platform
specific), then client will know which channel to request.
It can have two levels, dmac and channel. In case mux is flexible enough
then client gets a channel and program the mux for this mapping.
I think this is the most simplistic solution I have for this, thoughts?
How about this my idea:
http://thread.gmane.org/gmane.linux.ports.arm.omap/75828/focus=15501
A small correction to it would be, that it shouldn't (necessarily) be a
separate driver, because in some cases the mux resides on the DMAC, they
share registers, so, it shouldn't really be a separate device and a
separate driver, don't think it's worth an MFD set up or anything similar.
So, I am trying ATM to implement something along the lines of
struct dma_chan *dma_request_slave_channel(struct device *dev,
enum dma_transfer_direction direction, const char *name);
The connection between clients and the mux is always static, so, the
dmaengine core can always just pass to the mux a client-side "pad"
specifier (dev + direction + (optionally) name). The mux call-back will
then see, where it can connect that pad and return a suitable channel
descriptor - possibly with the help of the DMAC driver proper.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
Do you mean there must be a global table, or are you ok with putting
the information about a channel into the device that uses the channel,
as we do for most other subsystems (IRQ, GPIO, pinctrl, ...).
If not, what is the problem with that approach?
Today, we simple ask, "give me dma channel with DMA_SLAVE capability".
If we change it to "give me dma channel which suits my need" and have
additional information in dmaengine to handle this request effectively.
What that would mean is
a) DMA channel either knows which channel to provide, Or
b) Additional arguments provided to dmaengine API to help it find out
which channel to provide.
It would be good to have client ask for a specific channel. But in order
to build generic clients, we face a problem that clients may not know
how they mapped to dmac by SoC designer. Or the mux maybe entirely
flexible on which channel.
If we add this as DT property (which I assume should be platform
specific), then client will know which channel to request.
It can have two levels, dmac and channel. In case mux is flexible enough
then client gets a channel and program the mux for this mapping.
I think this is the most simplistic solution I have for this, thoughts?
I think we're basically on the same page. Let's see if I have covered
all the cases we discussed so far. I've tried to update the binding that
Jon sent out initially with everything we've discussed, so please review
this to see if I understood you correctly.
Arnd
* Generic DMA Controller and DMA request bindings
Generic binding to provide a way for a driver using DMA Engine to retrieve the
DMA request or channel information that goes from a hardware device to a DMA
controller.
* DMA controller
Required property:
- #dma-cells: Number elements to describe DMA channel information. Must be
at least 2, allowing a phandle and a flags cell, but usually
is larger so a client can also specify a request or channel
number and/or some configuration.
Optional properties:
- #dma-channels: Number of DMA channels supported by the controller.
- #dma-requests: Number of DMA requests signals supported by the controller.
Example:
sdma: dmaengine at 48000000 {
compatible = "ti,omap4-sdma"
reg = <0x48000000 0x1000>;
interrupts = <4>;
#dma-cells = <3>;
#dma-channels = <32>;
#dma-requests = <127>;
};
* DMA client
Client drivers should specify the DMA property using a phandle to the controller
followed by the number of DMA request/channel and the transfer type of the
channel (eg. device-to-memory, memory-to-device, memory-to-memory, etc).
Required property:
dmas: list of one or more dma specifiers, each consisting of
- phandle pointing to dma controller node
- flags word, a bit map that can hold these flags
* 0x00000001 channel can be used for transfer from device
* 0x00000002 channel can be user for transfer to device
- zero or more cells in a format specific to the the dma controller
node listed in the phandle. This typically contains a dma request
line number or a channel number, but can contain any data that
is used required for configuring a channel.
Optional property:
dma-names: when present, this shall contain one identifier string
for each dma specifier in the dmas property. The specific strings
that can be used are defined in the binding of the DMA client
device. When multiple dma specifiers can be used as alternatives,
the dma-names for those dma specifiers must be identical.
Any dma specifiers that have identical flags and identical dma-names
(if present) shall refer to different dma controllers that can be
used as alternatives, e.g. when a request line is connected to
multiple dma controllers. If multiple dma specifiers are listed that
have the same flags but refer to different functional channels,
the dma-names property must be used to distinguish them.
Examples:
1. One DMA write channel, one DMA read/write channel:
i2c1: i2c at 1 {
...
dmas = <&sdma 2 1 &sdma 3 2>;
...
};
2. A single read-write channel with two alternative dma controllers
providing it:
dmas = <&dma0 3 5
&dma1 3 7
&dma2 3 2>;
3. A device with three channels, one of which has two alternatives:
dmas = <&dma0 1 4 /* data read */
&dma0 2 6 /* data write */
&dma1 1 0 /* error read */
&dma2 1 0>; /* alternative error read */
dma-names = "data", "data", "error", "error";
4. A dma controller requiring complex configuration:
dma: dmaengine at 48000000 {
compatible = "foo,foo-sdma"
reg = <0x48000000 0x1000>;
interrupts = <4>;
#dma-cells = <6>; /* phandle, flag, request, channel,
input-width, output-width */
#dma-channels = <32>;
#dma-requests = <127>;
};
mmc at 49000000 {
...
dmas = <&dma 1 /* read */
2 /* request line */
5 /* channel */
16 /* 16 bit bus width on read */
8> /* 8 bit bus width on write */
<&dma 2 /* write */
3 /* request line */
6 /* channel */
8 /* 8 bit bus width on read */
16> /* 16 bit bus width on write */
On Tue, 2012-07-17 at 19:24 +0000, Arnd Bergmann wrote:
On Friday 13 July 2012, Vinod Koul wrote:
quoted
quoted
Do you mean there must be a global table, or are you ok with putting
the information about a channel into the device that uses the channel,
as we do for most other subsystems (IRQ, GPIO, pinctrl, ...).
If not, what is the problem with that approach?
Today, we simple ask, "give me dma channel with DMA_SLAVE capability".
If we change it to "give me dma channel which suits my need" and have
additional information in dmaengine to handle this request effectively.
What that would mean is
a) DMA channel either knows which channel to provide, Or
b) Additional arguments provided to dmaengine API to help it find out
which channel to provide.
It would be good to have client ask for a specific channel. But in order
to build generic clients, we face a problem that clients may not know
how they mapped to dmac by SoC designer. Or the mux maybe entirely
flexible on which channel.
If we add this as DT property (which I assume should be platform
specific), then client will know which channel to request.
It can have two levels, dmac and channel. In case mux is flexible enough
then client gets a channel and program the mux for this mapping.
I think this is the most simplistic solution I have for this, thoughts?
I think we're basically on the same page. Let's see if I have covered
all the cases we discussed so far. I've tried to update the binding that
Jon sent out initially with everything we've discussed, so please review
this to see if I understood you correctly.
I think this looks fine to me. Few comments below on client side
Arnd
* Generic DMA Controller and DMA request bindings
Generic binding to provide a way for a driver using DMA Engine to retrieve the
DMA request or channel information that goes from a hardware device to a DMA
controller.
* DMA controller
Required property:
- #dma-cells: Number elements to describe DMA channel information. Must be
at least 2, allowing a phandle and a flags cell, but usually
is larger so a client can also specify a request or channel
number and/or some configuration.
Optional properties:
- #dma-channels: Number of DMA channels supported by the controller.
- #dma-requests: Number of DMA requests signals supported by the controller.
Example:
sdma: dmaengine at 48000000 {
compatible = "ti,omap4-sdma"
reg = <0x48000000 0x1000>;
interrupts = <4>;
#dma-cells = <3>;
#dma-channels = <32>;
#dma-requests = <127>;
};
* DMA client
Client drivers should specify the DMA property using a phandle to the controller
followed by the number of DMA request/channel and the transfer type of the
channel (eg. device-to-memory, memory-to-device, memory-to-memory, etc).
Required property:
dmas: list of one or more dma specifiers, each consisting of
- phandle pointing to dma controller node
- flags word, a bit map that can hold these flags
* 0x00000001 channel can be used for transfer from device
* 0x00000002 channel can be user for transfer to device
Is this for identifying which channel is for TX and RX? If not I am not
sure I understood it well
- zero or more cells in a format specific to the the dma controller
node listed in the phandle. This typically contains a dma request
line number or a channel number, but can contain any data that
is used required for configuring a channel.
Optional property:
dma-names: when present, this shall contain one identifier string
for each dma specifier in the dmas property. The specific strings
that can be used are defined in the binding of the DMA client
device. When multiple dma specifiers can be used as alternatives,
the dma-names for those dma specifiers must be identical.
Any dma specifiers that have identical flags and identical dma-names
(if present) shall refer to different dma controllers that can be
used as alternatives, e.g. when a request line is connected to
multiple dma controllers. If multiple dma specifiers are listed that
have the same flags but refer to different functional channels,
the dma-names property must be used to distinguish them.
Examples:
1. One DMA write channel, one DMA read/write channel:
i2c1: i2c at 1 {
...
dmas = <&sdma 2 1 &sdma 3 2>;
...
};
2. A single read-write channel with two alternative dma controllers
providing it:
dmas = <&dma0 3 5
&dma1 3 7
&dma2 3 2>;
3. A device with three channels, one of which has two alternatives:
dmas = <&dma0 1 4 /* data read */
&dma0 2 6 /* data write */
&dma1 1 0 /* error read */
&dma2 1 0>; /* alternative error read */
dma-names = "data", "data", "error", "error";
4. A dma controller requiring complex configuration:
dma: dmaengine at 48000000 {
compatible = "foo,foo-sdma"
reg = <0x48000000 0x1000>;
interrupts = <4>;
#dma-cells = <6>; /* phandle, flag, request, channel,
input-width, output-width */
Why would we want the widths to be here?
Assuming a DMA from System memory to a peripheral, source width should
be system memory width and destination the peripheral width. IMO these
should not be in dma property even if we need these
#dma-channels = <32>;
#dma-requests = <127>;
};
mmc at 49000000 {
...
dmas = <&dma 1 /* read */
2 /* request line */
5 /* channel */
16 /* 16 bit bus width on read */
8> /* 8 bit bus width on write */
<&dma 2 /* write */
3 /* request line */
6 /* channel */
8 /* 8 bit bus width on read */
16> /* 16 bit bus width on write */
From this looks like flag is for TX/RX, so maybe i read correct :)
Required property:
dmas: list of one or more dma specifiers, each consisting of
- phandle pointing to dma controller node
- flags word, a bit map that can hold these flags
* 0x00000001 channel can be used for transfer from device
* 0x00000002 channel can be user for transfer to device
Is this for identifying which channel is for TX and RX? If not I am not
sure I understood it well
Yes, but we can potentially add more flags here.
The argument we had when coming up with this was roughly:
* we need to identify which specifiers are referring to the same
conceptual channel and can be used as alternatives
* this could be done just using the dma-names property, but making
dma-names mandatory adds complexity for everyone.
* Most devices have just one or two channels, and if they have two,
there is usually one input and one output.
=> if the common dmaengine code can find out whether a channel is
input or output without looking at the dmac driver specific configuration,
we don't need to add dma-names in most cases, but just let the client
driver ask for "give me a channel with these flags".
Why would we want the widths to be here?
Assuming a DMA from System memory to a peripheral, source width should
be system memory width and destination the peripheral width. IMO these
should not be in dma property even if we need these
I was just trying to come up with an example of something we might put
into the additional configuration fields. This may or may not be a
realistic one, I have no idea. If you know something else that one
of the dma controllers might want to put in there, we should change the
example.
I took the example of data width from 'struct stedma40_chan_cfg', which
is used in some places to configure this from platform data. My
impression was that if we want to move that data from board files into
the device tree, it has to be here, but it can well be that there is
a better place for it, e.g. in the global (channel independent)
configuration of the dmac.
Arnd
Why would we want the widths to be here?
Assuming a DMA from System memory to a peripheral, source width should
be system memory width and destination the peripheral width. IMO these
should not be in dma property even if we need these
Hi Vinod,
I know at least one peripheral which accepts 2 widths, 8bit and 16bit, namely the
M-Systems DiskOnChip G3 NAND chip.
This device has to configured to choose either 8bit data bus access or 16bit
data bus access.
I'm just wondering if that usecase will fit in without the width information
embedded, and how will the driver choose the width to use ?
Cheers.
--
Robert
On Fri, 2012-07-20 at 08:39 +0000, Arnd Bergmann wrote:
On Friday 20 July 2012, Vinod Koul wrote:
quoted
quoted
Required property:
dmas: list of one or more dma specifiers, each consisting of
- phandle pointing to dma controller node
- flags word, a bit map that can hold these flags
* 0x00000001 channel can be used for transfer from device
* 0x00000002 channel can be user for transfer to device
Is this for identifying which channel is for TX and RX? If not I am not
sure I understood it well
Yes, but we can potentially add more flags here.
The argument we had when coming up with this was roughly:
* we need to identify which specifiers are referring to the same
conceptual channel and can be used as alternatives
* this could be done just using the dma-names property, but making
dma-names mandatory adds complexity for everyone.
* Most devices have just one or two channels, and if they have two,
there is usually one input and one output.
=> if the common dmaengine code can find out whether a channel is
input or output without looking at the dmac driver specific configuration,
we don't need to add dma-names in most cases, but just let the client
driver ask for "give me a channel with these flags".
No we don't export the direction of the channel and usually channel can
be configured either way.
But from a client POV it makes sense as with the given direction you
would need a specific request line for a channel. So this is right.
But direction is something I don't expect to be used for "give me a
channel"
Why would we want the widths to be here?
Assuming a DMA from System memory to a peripheral, source width should
be system memory width and destination the peripheral width. IMO these
should not be in dma property even if we need these
I was just trying to come up with an example of something we might put
into the additional configuration fields. This may or may not be a
realistic one, I have no idea. If you know something else that one
of the dma controllers might want to put in there, we should change the
example.
I took the example of data width from 'struct stedma40_chan_cfg', which
is used in some places to configure this from platform data. My
impression was that if we want to move that data from board files into
the device tree, it has to be here, but it can well be that there is
a better place for it, e.g. in the global (channel independent)
configuration of the dmac.
Arnd
Why would we want the widths to be here?
Assuming a DMA from System memory to a peripheral, source width should
be system memory width and destination the peripheral width. IMO these
should not be in dma property even if we need these
Hi Vinod,
I know at least one peripheral which accepts 2 widths, 8bit and 16bit, namely the
M-Systems DiskOnChip G3 NAND chip.
This device has to configured to choose either 8bit data bus access or 16bit
data bus access.
That would be configured by the client (peripheral) driver and passed to
dmaengine driver using the slave config. The point is that it has
nothing to do with dma.
I'm just wondering if that usecase will fit in without the width information
embedded, and how will the driver choose the width to use ?
It you need, this should be the client property and passed as argument
to dma, not a dma property :)
--
~Vinod
From: Stephen Warren <hidden> Date: 2012-07-23 21:29:57
On 07/17/2012 01:24 PM, Arnd Bergmann wrote:
...
I think we're basically on the same page. Let's see if I have covered
all the cases we discussed so far. I've tried to update the binding that
Jon sent out initially with everything we've discussed, so please review
this to see if I understood you correctly.
...
* DMA client
...
Examples:
...
3. A device with three channels, one of which has two alternatives:
s/three/four/ s/one of which/both of which/
This binding doc seems reasonable to me.
3. A device with three channels, one of which has two alternatives:
s/three/four/ s/one of which/both of which/
This binding doc seems reasonable to me.
I asked a linguist about it who said that you can't have "both" together
with "four". She also mentioned that my text is rather confusing, so maybe
you also got it wrong. I'll try adding some explanation:
3. A device with three channels, one of which has two alternatives:
dmas = <&dma0 1 4 /* first channel, data read */
&dma0 2 6 /* second channel, data write */
&dma1 1 0 /* third channel, error read */
&dma2 1 0>; /* third channel, ernative error read */
dma-names = "data", "data", "error", "error";
The first two channels are identified by having a unique direction
flag in combination with the "data" string. For the third channel,
there are two dma specifiers with identical flags (1) and strings
("error"), so only one specifier may be used at a time.
Arnd
From: Sergei Shtylyov <hidden> Date: 2012-07-24 12:54:27
Hello.
On 24-07-2012 1:29, Stephen Warren wrote:
quoted
I think we're basically on the same page. Let's see if I have covered
all the cases we discussed so far. I've tried to update the binding that
Jon sent out initially with everything we've discussed, so please review
this to see if I understood you correctly.
...
quoted
* DMA client
...
quoted
Examples:
...
quoted
3. A device with three channels, one of which has two alternatives:
s/three/four/ s/one of which/both of which/
You can't say "both" of 4 channels, only "all". :-)
WBR, Sergei
From: Stephen Warren <hidden> Date: 2012-07-24 16:04:53
On 07/24/2012 01:19 AM, Arnd Bergmann wrote:
On Monday 23 July 2012, Stephen Warren wrote:
quoted
quoted
3. A device with three channels, one of which has two alternatives:
s/three/four/ s/one of which/both of which/
This binding doc seems reasonable to me.
I asked a linguist about it who said that you can't have "both" together
with "four". She also mentioned that my text is rather confusing, so maybe
you also got it wrong. I'll try adding some explanation:
Oops, I guess I meant s/three/two/ :-)
It seems that given there are two values for dma-names, there really are
two channels; it's just that one channel is bi-directional, and the
second has two alternatives.
Still, I guess you could also view this as three separate channels
instead. In which case, the text below makes sense.
3. A device with three channels, one of which has two alternatives:
dmas = <&dma0 1 4 /* first channel, data read */
&dma0 2 6 /* second channel, data write */
&dma1 1 0 /* third channel, error read */
&dma2 1 0>; /* third channel, ernative error read */
dma-names = "data", "data", "error", "error";
The first two channels are identified by having a unique direction
flag in combination with the "data" string. For the third channel,
there are two dma specifiers with identical flags (1) and strings
("error"), so only one specifier may be used at a time.
It seems that given there are two values for dma-names, there really are
two channels; it's just that one channel is bi-directional, and the
second has two alternatives.
Still, I guess you could also view this as three separate channels
instead. In which case, the text below makes sense.
quoted
3. A device with three channels, one of which has two alternatives:
dmas = <&dma0 1 4 /* first channel, data read */
&dma0 2 6 /* second channel, data write */
&dma1 1 0 /* third channel, error read */
&dma2 1 0>; /* third channel, ernative error read */
dma-names = "data", "data", "error", "error";
A bidirectional channel would have only one request line, not two,
and we would write that as
dmas = <&dma0 3 4>; /* one channel on dmarq 4, read-write */
Arnd
From: Jon Hunter <hidden> Date: 2012-07-24 19:07:43
Hi Vinod,
On 07/20/2012 04:37 AM, Vinod Koul wrote:
On Fri, 2012-07-20 at 08:39 +0000, Arnd Bergmann wrote:
quoted
On Friday 20 July 2012, Vinod Koul wrote:
quoted
quoted
Required property:
dmas: list of one or more dma specifiers, each consisting of
- phandle pointing to dma controller node
- flags word, a bit map that can hold these flags
* 0x00000001 channel can be used for transfer from device
* 0x00000002 channel can be user for transfer to device
Is this for identifying which channel is for TX and RX? If not I am not
sure I understood it well
Yes, but we can potentially add more flags here.
The argument we had when coming up with this was roughly:
* we need to identify which specifiers are referring to the same
conceptual channel and can be used as alternatives
* this could be done just using the dma-names property, but making
dma-names mandatory adds complexity for everyone.
* Most devices have just one or two channels, and if they have two,
there is usually one input and one output.
=> if the common dmaengine code can find out whether a channel is
input or output without looking at the dmac driver specific configuration,
we don't need to add dma-names in most cases, but just let the client
driver ask for "give me a channel with these flags".
No we don't export the direction of the channel and usually channel can
be configured either way.
So yes I can see that a channel itself could be configured to support a
given direction, but when we ask for a channel via dma_request_channel()
we are going to get a channel that matches the criteria we pass using
the filter parameter. So here the thinking was that "flags" is a filter
parameter that the user could specify and one example being direction
but it could be something else too.
But from a client POV it makes sense as with the given direction you
would need a specific request line for a channel. So this is right.
But direction is something I don't expect to be used for "give me a
channel"
Ok. The thought was that the user would have the following means of
requesting a channel ...
1. By name
2. By a filter parameter (flags)
3. By name and a filter parameter
So we would have the following APIs ...
struct dma_chan
*of_dma_request_channel(struct device_node *node, unsigned int flags);
struct dma_chan
*of_dma_request_named channel(struct device_node *node, char *name,
unsigned int flags);
In both of these the filter parameter flags is optional.
Let me know your thoughts on this.
Cheers
Jon
Ok. The thought was that the user would have the following means of
requesting a channel ...
1. By name
2. By a filter parameter (flags)
3. By name and a filter parameter
So we would have the following APIs ...
struct dma_chan
*of_dma_request_channel(struct device_node *node, unsigned int flags);
struct dma_chan
*of_dma_request_named channel(struct device_node *node, char *name,
unsigned int flags);
In both of these the filter parameter flags is optional.
Let me know your thoughts on this.
I definitely like this version. I was thinking of a different variant
where we have separate functions for each flag value, but I think yours
is actually better.
Arnd
On Fri, Jul 20, 2012 at 12:00 PM, Vinod Koul [off-list ref] wrote:
On Tue, 2012-07-17 at 19:24 +0000, Arnd Bergmann wrote:
quoted
On Friday 13 July 2012, Vinod Koul wrote:
quoted
quoted
Do you mean there must be a global table, or are you ok with putting
the information about a channel into the device that uses the channel,
as we do for most other subsystems (IRQ, GPIO, pinctrl, ...).
If not, what is the problem with that approach?
Today, we simple ask, "give me dma channel with DMA_SLAVE capability".
If we change it to "give me dma channel which suits my need" and have
additional information in dmaengine to handle this request effectively.
What that would mean is
a) DMA channel either knows which channel to provide, Or
b) Additional arguments provided to dmaengine API to help it find out
which channel to provide.
It would be good to have client ask for a specific channel. But in order
to build generic clients, we face a problem that clients may not know
how they mapped to dmac by SoC designer. Or the mux maybe entirely
flexible on which channel.
If we add this as DT property (which I assume should be platform
specific), then client will know which channel to request.
It can have two levels, dmac and channel. In case mux is flexible enough
then client gets a channel and program the mux for this mapping.
I think this is the most simplistic solution I have for this, thoughts?
I think we're basically on the same page. Let's see if I have covered
all the cases we discussed so far. I've tried to update the binding that
Jon sent out initially with everything we've discussed, so please review
this to see if I understood you correctly.
I think this looks fine to me. Few comments below on client side
quoted
Arnd
* Generic DMA Controller and DMA request bindings
Generic binding to provide a way for a driver using DMA Engine to retrieve the
DMA request or channel information that goes from a hardware device to a DMA
controller.
* DMA controller
Required property:
- #dma-cells: Number elements to describe DMA channel information. Must be
at least 2, allowing a phandle and a flags cell, but usually
is larger so a client can also specify a request or channel
number and/or some configuration.
Optional properties:
- #dma-channels: Number of DMA channels supported by the controller.
- #dma-requests: Number of DMA requests signals supported by the controller.
Example:
sdma: dmaengine at 48000000 {
compatible = "ti,omap4-sdma"
reg = <0x48000000 0x1000>;
interrupts = <4>;
#dma-cells = <3>;
#dma-channels = <32>;
#dma-requests = <127>;
};
* DMA client
Client drivers should specify the DMA property using a phandle to the controller
followed by the number of DMA request/channel and the transfer type of the
channel (eg. device-to-memory, memory-to-device, memory-to-memory, etc).
Required property:
dmas: list of one or more dma specifiers, each consisting of
- phandle pointing to dma controller node
- flags word, a bit map that can hold these flags
* 0x00000001 channel can be used for transfer from device
* 0x00000002 channel can be user for transfer to device
Is this for identifying which channel is for TX and RX? If not I am not
sure I understood it well
quoted
- zero or more cells in a format specific to the the dma controller
node listed in the phandle. This typically contains a dma request
line number or a channel number, but can contain any data that
is used required for configuring a channel.
How about extend struct dma_slave_config, adding one member of
"request_line", if
"request line" is must config in most platform.
struct dma_slave_config {
~
u32 request_line;
}
The advantage is request_line can be get directly from
dmaengine_slave_config regardless of DT.
quoted
Optional property:
dma-names: when present, this shall contain one identifier string
for each dma specifier in the dmas property. The specific strings
that can be used are defined in the binding of the DMA client
device. When multiple dma specifiers can be used as alternatives,
the dma-names for those dma specifiers must be identical.
Any dma specifiers that have identical flags and identical dma-names
(if present) shall refer to different dma controllers that can be
used as alternatives, e.g. when a request line is connected to
multiple dma controllers. If multiple dma specifiers are listed that
have the same flags but refer to different functional channels,
the dma-names property must be used to distinguish them.
Examples:
1. One DMA write channel, one DMA read/write channel:
i2c1: i2c at 1 {
...
dmas = <&sdma 2 1 &sdma 3 2>;
...
};
2. A single read-write channel with two alternative dma controllers
providing it:
dmas = <&dma0 3 5
&dma1 3 7
&dma2 3 2>;
3. A device with three channels, one of which has two alternatives:
dmas = <&dma0 1 4 /* data read */
&dma0 2 6 /* data write */
&dma1 1 0 /* error read */
&dma2 1 0>; /* alternative error read */
dma-names = "data", "data", "error", "error";
4. A dma controller requiring complex configuration:
dma: dmaengine at 48000000 {
compatible = "foo,foo-sdma"
reg = <0x48000000 0x1000>;
interrupts = <4>;
#dma-cells = <6>; /* phandle, flag, request, channel,
input-width, output-width */
Why would we want the widths to be here?
Assuming a DMA from System memory to a peripheral, source width should
be system memory width and destination the peripheral width. IMO these
should not be in dma property even if we need these
quoted
#dma-channels = <32>;
#dma-requests = <127>;
};
mmc at 49000000 {
...
dmas = <&dma 1 /* read */
2 /* request line */
5 /* channel */
16 /* 16 bit bus width on read */
8> /* 8 bit bus width on write */
<&dma 2 /* write */
3 /* request line */
6 /* channel */
8 /* 8 bit bus width on read */
16> /* 16 bit bus width on write */
From this looks like flag is for TX/RX, so maybe i read correct :)
On Tue, 2012-07-24 at 14:07 -0500, Jon Hunter wrote:
Hi Vinod,
quoted
quoted
quoted
quoted
Required property:
dmas: list of one or more dma specifiers, each consisting of
- phandle pointing to dma controller node
- flags word, a bit map that can hold these flags
* 0x00000001 channel can be used for transfer from device
* 0x00000002 channel can be user for transfer to device
Is this for identifying which channel is for TX and RX? If not I am not
sure I understood it well
Yes, but we can potentially add more flags here.
The argument we had when coming up with this was roughly:
* we need to identify which specifiers are referring to the same
conceptual channel and can be used as alternatives
* this could be done just using the dma-names property, but making
dma-names mandatory adds complexity for everyone.
* Most devices have just one or two channels, and if they have two,
there is usually one input and one output.
=> if the common dmaengine code can find out whether a channel is
input or output without looking at the dmac driver specific configuration,
we don't need to add dma-names in most cases, but just let the client
driver ask for "give me a channel with these flags".
No we don't export the direction of the channel and usually channel can
be configured either way.
So yes I can see that a channel itself could be configured to support a
given direction, but when we ask for a channel via dma_request_channel()
we are going to get a channel that matches the criteria we pass using
the filter parameter. So here the thinking was that "flags" is a filter
parameter that the user could specify and one example being direction
but it could be something else too.
Yes that can be done, but I am leaning towards clients not have to do
anything :) DMAEngine needs to know mapping and when
dma_request_channel() is called it _always_ gives you the right channel.
Maybe for slave case we need to create dma_request_slave_channel() which
has additional arguments for dmaengine to do the filtering.
quoted
But from a client POV it makes sense as with the given direction you
would need a specific request line for a channel. So this is right.
But direction is something I don't expect to be used for "give me a
channel"
Ok. The thought was that the user would have the following means of
requesting a channel ...
1. By name
Bare name maynot be enough. In a dmac we have many channels which one to
choose?
2. By a filter parameter (flags)
Even with direction same problem can arise
3. By name and a filter parameter
Additionally we need to say which channel, or making dmaengine already
aware will help here
So we would have the following APIs ...
struct dma_chan
*of_dma_request_channel(struct device_node *node, unsigned int flags);
struct dma_chan
*of_dma_request_named channel(struct device_node *node, char *name,
unsigned int flags);
In both of these the filter parameter flags is optional.
Let me know your thoughts on this.
I would call them dma_request_slave_channel and try to add to it for
additional filtering
--
~Vinod
But from a client POV it makes sense as with the given direction you
would need a specific request line for a channel. So this is right.
But direction is something I don't expect to be used for "give me a
channel"
Ok. The thought was that the user would have the following means of
requesting a channel ...
1. By name
Bare name maynot be enough. In a dmac we have many channels which one to
choose?
The name is what is associated with the property in the client device
node, which describes everything the dmac driver needs to know.
If the dmac needs to pick a specific channel, it can find out from the
"dmas" property in combination with that name. If it is allowed to
pick any channel, it doesn't need to bother.
quoted
2. By a filter parameter (flags)
Even with direction same problem can arise
Again this is just identifying which dma specifier from the "dmas"
property to pick. The use case is the very common one that there
is at most one "read" and one "write" channel. In this case all
the client has to know is that it wants a channel that fits the
description given in DT for the direction it's looking for.
quoted
3. By name and a filter parameter
Additionally we need to say which channel, or making dmaengine already
aware will help here.
The channel is still described in the specifier, the client should
not care about it.
Arnd
On Thu, 2012-07-26 at 07:14 +0000, Arnd Bergmann wrote:
On Thursday 26 July 2012, Vinod Koul wrote:
quoted
quoted
quoted
But from a client POV it makes sense as with the given direction you
would need a specific request line for a channel. So this is right.
But direction is something I don't expect to be used for "give me a
channel"
Ok. The thought was that the user would have the following means of
requesting a channel ...
1. By name
Bare name maynot be enough. In a dmac we have many channels which one to
choose?
The name is what is associated with the property in the client device
node, which describes everything the dmac driver needs to know.
If the dmac needs to pick a specific channel, it can find out from the
"dmas" property in combination with that name. If it is allowed to
pick any channel, it doesn't need to bother.
dmac doesn't pick a channel. They don't come into picture till dmaengine
and client have agreed on channel. And then channel callback in invoked,
still it doesn't know which client.
quoted
quoted
2. By a filter parameter (flags)
Even with direction same problem can arise
Again this is just identifying which dma specifier from the "dmas"
property to pick. The use case is the very common one that there
is at most one "read" and one "write" channel. In this case all
the client has to know is that it wants a channel that fits the
description given in DT for the direction it's looking for.
client knows but that needs to be propagated to dmaengine (not dmac) and
dmaengine filters this based on new information it has
quoted
quoted
3. By name and a filter parameter
Additionally we need to say which channel, or making dmaengine already
aware will help here.
The channel is still described in the specifier, the client should
not care about it.
Arnd
From: Jon Hunter <hidden> Date: 2012-07-26 15:53:46
On 07/26/2012 06:28 AM, Vinod Koul wrote:
On Thu, 2012-07-26 at 07:14 +0000, Arnd Bergmann wrote:
quoted
On Thursday 26 July 2012, Vinod Koul wrote:
quoted
quoted
quoted
But from a client POV it makes sense as with the given direction you
would need a specific request line for a channel. So this is right.
But direction is something I don't expect to be used for "give me a
channel"
Ok. The thought was that the user would have the following means of
requesting a channel ...
1. By name
Bare name maynot be enough. In a dmac we have many channels which one to
choose?
The name is what is associated with the property in the client device
node, which describes everything the dmac driver needs to know.
If the dmac needs to pick a specific channel, it can find out from the
"dmas" property in combination with that name. If it is allowed to
pick any channel, it doesn't need to bother.
dmac doesn't pick a channel. They don't come into picture till dmaengine
and client have agreed on channel. And then channel callback in invoked,
still it doesn't know which client.
I think what Arnd meant was that dmaengine (not the dmac) would use the
DT node and name information to extract the dma mapping information from
the device tree and provide a channel back to the client. So yes the
dmac is not involved here.
By the way, when I said "by name" above (and probably this was not
clear) but it should have been "DT node and name". So really a channel
is requested by ...
1. DT node and a name
2. DT node and a filter parameter (flags)
3. DT node, a name and a filter parameter (flags)
The DT node points us to the specific device in the DT, say an MMC node,
and the MMC node then contains the DMA mapping info. The device node may
have the mapping information have one or more DMA requests/channels and
so then the name and/or flags is used to determine which the client needs.
Sorry hope that this is a little clearer.
Cheers
Jon
From: Jon Hunter <hidden> Date: 2012-07-26 17:43:31
On 07/26/2012 01:42 AM, Vinod Koul wrote:
On Tue, 2012-07-24 at 14:07 -0500, Jon Hunter wrote:
quoted
Hi Vinod,
quoted
quoted
quoted
quoted
Required property:
dmas: list of one or more dma specifiers, each consisting of
- phandle pointing to dma controller node
- flags word, a bit map that can hold these flags
* 0x00000001 channel can be used for transfer from device
* 0x00000002 channel can be user for transfer to device
Is this for identifying which channel is for TX and RX? If not I am not
sure I understood it well
Yes, but we can potentially add more flags here.
The argument we had when coming up with this was roughly:
* we need to identify which specifiers are referring to the same
conceptual channel and can be used as alternatives
* this could be done just using the dma-names property, but making
dma-names mandatory adds complexity for everyone.
* Most devices have just one or two channels, and if they have two,
there is usually one input and one output.
=> if the common dmaengine code can find out whether a channel is
input or output without looking at the dmac driver specific configuration,
we don't need to add dma-names in most cases, but just let the client
driver ask for "give me a channel with these flags".
No we don't export the direction of the channel and usually channel can
be configured either way.
So yes I can see that a channel itself could be configured to support a
given direction, but when we ask for a channel via dma_request_channel()
we are going to get a channel that matches the criteria we pass using
the filter parameter. So here the thinking was that "flags" is a filter
parameter that the user could specify and one example being direction
but it could be something else too.
Yes that can be done, but I am leaning towards clients not have to do
anything :) DMAEngine needs to know mapping and when
dma_request_channel() is called it _always_ gives you the right channel.
Ok, so are you proposing to remove the filter function and parameter
from the dma_request_channel()?
Maybe for slave case we need to create dma_request_slave_channel() which
has additional arguments for dmaengine to do the filtering.
Ok, so what is not clear to me is if you envision that
dma_request_slave_channel() is using a mapping table based look-up or
the DT scheme or both.
As Arnd highlighted the DT convention is to store the DMA info in each
of the device nodes and not store in a global mapping table which
conflicts with having a mapping table approach for non-DT usage. So I am
still not sure how you envision this function working for both the
non-DT and DT use-cases.
Jon
On Thu, 2012-07-26 at 12:43 -0500, Jon Hunter wrote:
quoted
quoted
So yes I can see that a channel itself could be configured to
support a
quoted
quoted
given direction, but when we ask for a channel via
dma_request_channel()
quoted
quoted
we are going to get a channel that matches the criteria we pass
using
quoted
quoted
the filter parameter. So here the thinking was that "flags" is a
filter
quoted
quoted
parameter that the user could specify and one example being
direction
quoted
quoted
but it could be something else too.
Yes that can be done, but I am leaning towards clients not have to
do
quoted
anything :) DMAEngine needs to know mapping and when
dma_request_channel() is called it _always_ gives you the right
channel.
Ok, so are you proposing to remove the filter function and parameter
from the dma_request_channel()?
No. But add a new request call, dma_request_slave_channel() which is
exclusive for slave usages and takes into account the mapping to be done
for channels
quoted
Maybe for slave case we need to create dma_request_slave_channel()
which
quoted
has additional arguments for dmaengine to do the filtering.
Yup
Ok, so what is not clear to me is if you envision that
dma_request_slave_channel() is using a mapping table based look-up or
the DT scheme or both.
The API should not worry about it. It would be good to have DT/ other be
behind this API, so it is transparent to users. They just request a
slave channel.
As Arnd highlighted the DT convention is to store the DMA info in each
of the device nodes and not store in a global mapping table which
conflicts with having a mapping table approach for non-DT usage. So I
am
still not sure how you envision this function working for both the
non-DT and DT use-cases.
I expect the clients to pass the mapping information to dmaengine in way
dmaengine understands. This information can come from DT or other
places. That way dmaengine gets info from any system being used and be
able to allocate slave channel properly.
--
~Vinod
From: Jon Hunter <hidden> Date: 2012-08-01 20:43:02
Hi Vinod,
On 07/31/2012 06:12 AM, Vinod Koul wrote:
On Thu, 2012-07-26 at 12:43 -0500, Jon Hunter wrote:
quoted
quoted
quoted
So yes I can see that a channel itself could be configured to
support a
quoted
quoted
given direction, but when we ask for a channel via
dma_request_channel()
quoted
quoted
we are going to get a channel that matches the criteria we pass
using
quoted
quoted
the filter parameter. So here the thinking was that "flags" is a
filter
quoted
quoted
parameter that the user could specify and one example being
direction
quoted
quoted
but it could be something else too.
Yes that can be done, but I am leaning towards clients not have to
do
quoted
anything :) DMAEngine needs to know mapping and when
dma_request_channel() is called it _always_ gives you the right
channel.
Ok, so are you proposing to remove the filter function and parameter
from the dma_request_channel()?
No. But add a new request call, dma_request_slave_channel() which is
exclusive for slave usages and takes into account the mapping to be done
for channels
quoted
quoted
Maybe for slave case we need to create dma_request_slave_channel()
which
quoted
has additional arguments for dmaengine to do the filtering.
Yup
quoted
Ok, so what is not clear to me is if you envision that
dma_request_slave_channel() is using a mapping table based look-up or
the DT scheme or both.
The API should not worry about it. It would be good to have DT/ other be
behind this API, so it is transparent to users. They just request a
slave channel.
So would you envision something like (copying from Guennadi's API but
changing direction to flags) ...
struct dma_chan *dma_request_slave_channel(struct device *dev,
char *name, unsigned int flags)
{
/* If device-tree is present get slave info from here */
if (dev->of_node)
return of_dma_request_slave_channel(dev, name, flags);
return NULL;
}
Ok, so right now the above is nothing more than a simple wrapper around
a DT dma function to extract the slave info. However, it would allow us
to add another means for getting the slave info in the future if
necessary by adding an else part to the above.
Cheers
Jon
On Wed, 2012-08-01 at 15:43 -0500, Jon Hunter wrote:
Hi Vinod,
On 07/31/2012 06:12 AM, Vinod Koul wrote:
quoted
On Thu, 2012-07-26 at 12:43 -0500, Jon Hunter wrote:
quoted
quoted
quoted
So yes I can see that a channel itself could be configured to
support a
quoted
quoted
given direction, but when we ask for a channel via
dma_request_channel()
quoted
quoted
we are going to get a channel that matches the criteria we pass
using
quoted
quoted
the filter parameter. So here the thinking was that "flags" is a
filter
quoted
quoted
parameter that the user could specify and one example being
direction
quoted
quoted
but it could be something else too.
Yes that can be done, but I am leaning towards clients not have to
do
quoted
anything :) DMAEngine needs to know mapping and when
dma_request_channel() is called it _always_ gives you the right
channel.
Ok, so are you proposing to remove the filter function and parameter
from the dma_request_channel()?
No. But add a new request call, dma_request_slave_channel() which is
exclusive for slave usages and takes into account the mapping to be done
for channels
quoted
quoted
Maybe for slave case we need to create dma_request_slave_channel()
which
quoted
has additional arguments for dmaengine to do the filtering.
Yup
quoted
Ok, so what is not clear to me is if you envision that
dma_request_slave_channel() is using a mapping table based look-up or
the DT scheme or both.
The API should not worry about it. It would be good to have DT/ other be
behind this API, so it is transparent to users. They just request a
slave channel.
So would you envision something like (copying from Guennadi's API but
changing direction to flags) ...
struct dma_chan *dma_request_slave_channel(struct device *dev,
char *name, unsigned int flags)
{
/* If device-tree is present get slave info from here */
if (dev->of_node)
return of_dma_request_slave_channel(dev, name, flags);
return NULL;
}
Ok, so right now the above is nothing more than a simple wrapper around
a DT dma function to extract the slave info. However, it would allow us
to add another means for getting the slave info in the future if
necessary by adding an else part to the above.
Yup, something like above should work well. But without any dependency
from dmac's (unlike the RFC propsed)
--
~Vinod