of: Add generic device tree DMA helpers

From: Jon Hunter <[email protected]>

This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2] to add some basic helpers to retrieve a DMA controller device_node and the DMA request/channel information.

+++ b/Documentation/devicetree/bindings/dma/dma.txt

Documentation/devicetree/bindings/dma/dma.txt new file +3 -0

+* Generic DMA Controller and DMA request bindings
+
+Generic binding to provide a way for a driver using DMA Engine to retrieve the

Documentation/devicetree/bindings/dma/dma.txt new file +6 -0

+Example:
+
+	sdma: dmaengine at 48000000 {
+		compatible = "ti,omap4-sdma"
+		reg = <0x48000000 0x1000>;
+		interrupts = <4>;

v3: - avoid passing an xlate function and instead pass DMA engine parameters
- define number of dma channels and requests in dma-controller node

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)

Quoted 12 lines · Jon Hunter

Documentation/devicetree/bindings/dma/dma.txt new file +12 -0

+Example:
+
+	sdma: dmaengine at 48000000 {
+		compatible = "ti,omap4-sdma"
+		reg = <0x48000000 0x1000>;
+		interrupts = <4>;
+		#dma-cells = <2>;
+		#dma-channels = <32>;
+		#dma-requests = <127>;
+	};
+
+

Documentation/devicetree/bindings/dma/dma.txt line 32

Quoted 4 lines

Documentation/devicetree/bindings/dma/dma.txt new file +4 -0

+* 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

What exactly is "request/channel"? Is it a request ID, a channel ID, both packed into a single integer (which bits are used for each field), etc.?

Documentation/devicetree/bindings/dma/dma.txt line 35

Quoted 3 lines

Documentation/devicetree/bindings/dma/dma.txt new file +3 -0

+channel (eg. device-to-memory, memory-to-device, memory-to-memory, etc). Drivers
+should use the DMA Engine enum dma_transfer_direction (eg. DMA_DEV_TO_MEM,
+DMA_MEM_TO_DEV, etc) for specifying the transfer type.

The binding document should stand alone; you shouldn't need to go look up values in any particular OS/driver's source code. In other words, this should explicitly enumerate the values for DMA_DEV_TO_MEM etc. here. That said, I'm not sure it's appropriate to mandate that every DMA controller's DMA specifier actually include this information in DT (a controller might only support DEV_TO_MEM for example, and hence not need this information in DT at all).

The DMA client's binding should specify how many entries it needs in the dma property and what they mean, e.g. it should know that dma property index 0 is the TX DMA specifier, and index 1 is the RX specifier.

Documentation/devicetree/bindings/dma/dma.txt line 15

Quoted 2 lines

Documentation/devicetree/bindings/dma/dma.txt new file +2 -0

+    - #dma-channels: Number of DMA channels supported by the controller.
+    - #dma-requests: Number of DMA requests signals supported by the controller.

I don't see why those properties would be mandatory in device tree. They don't appear to be involved in parsing a DMA client's DMA specifier. Shouldn't this information be provided at run-time by the DMA controller driver. If it supports different HW models with different capabilities, then it certainly could represent those values in DT, but I don't think this should be required.

Quoted 5 lines · Jon Hunter

From: Jon Hunter <[email protected]>

This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2] to add some basic helpers to retrieve a DMA controller device_node and the DMA request/channel information.

I'll reply to the binding documentation first; perhaps that'll set the scene for my questions better.

Quoted 4 lines

+++ b/Documentation/devicetree/bindings/dma/dma.txt

Documentation/devicetree/bindings/dma/dma.txt new file +3 -0

+* Generic DMA Controller and DMA request bindings
+
+Generic binding to provide a way for a driver using DMA Engine to retrieve the

Documentation/devicetree/bindings/dma/dma.txt line 13

Quoted 10 lines

Documentation/devicetree/bindings/dma/dma.txt new file +10 -0

+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
+                  2 with current implementation but in future we could allow
+		  more than 2 to describe interrupt mapping as well.

That makes sense.

Documentation/devicetree/bindings/dma/dma.txt line 15

Quoted 2 lines

Documentation/devicetree/bindings/dma/dma.txt new file +2 -0

+    - #dma-channels: Number of DMA channels supported by the controller.
+    - #dma-requests: Number of DMA requests signals supported by the controller.

I don't see why those properties would be mandatory in device tree. They don't appear to be involved in parsing a DMA client's DMA specifier. Shouldn't this information be provided at run-time by the DMA controller driver. If it supports different HW models with different capabilities, then it certainly could represent those values in DT, but I don't think this should be required.

Quoted 12 lines

Documentation/devicetree/bindings/dma/dma.txt new file +12 -0

+Example:
+
+	sdma: dmaengine at 48000000 {
+		compatible = "ti,omap4-sdma"
+		reg = <0x48000000 0x1000>;
+		interrupts = <4>;
+		#dma-cells = <2>;
+		#dma-channels = <32>;
+		#dma-requests = <127>;
+	};
+
+

Documentation/devicetree/bindings/dma/dma.txt line 32

Quoted 4 lines

Documentation/devicetree/bindings/dma/dma.txt new file +4 -0

+* 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

What exactly is "request/channel"? Is it a request ID, a channel ID, both packed into a single integer (which bits are used for each field), etc.?

If this is up to the individual controller binding, then I think the description should just specify that there is a phandle followed by a DMA specifier, and the DMA specifier contains #dma-cells from the phandle node. See other similar bindings for suitable wording.

Documentation/devicetree/bindings/dma/dma.txt line 35

Quoted 3 lines

Documentation/devicetree/bindings/dma/dma.txt new file +3 -0

+channel (eg. device-to-memory, memory-to-device, memory-to-memory, etc). Drivers
+should use the DMA Engine enum dma_transfer_direction (eg. DMA_DEV_TO_MEM,
+DMA_MEM_TO_DEV, etc) for specifying the transfer type.

The binding document should stand alone; you shouldn't need to go look up values in any particular OS/driver's source code. In other words, this should explicitly enumerate the values for DMA_DEV_TO_MEM etc. here. That said, I'm not sure it's appropriate to mandate that every DMA controller's DMA specifier actually include this information in DT (a controller might only support DEV_TO_MEM for example, and hence not need this information in DT at all).

The DMA client's binding should specify how many entries it needs in the dma property and what they mean, e.g. it should know that dma property index 0 is the TX DMA specifier, and index 1 is the RX specifier.

Quoted 4 lines

Documentation/devicetree/bindings/dma/dma.txt new file +4 -0

+Required property:
+    - dma: List of phandle + dma-request/channel + transfer-type,
+      one group per request "line".
+

Documentation/devicetree/bindings/dma/dma.txt line 45

Quoted 5 lines

Documentation/devicetree/bindings/dma/dma.txt new file +5 -0

+Example:
+
+	i2c1: i2c at 1 {
+		...
+		dma = <&sdma 2 1 &sdma 3 2>;

Should that be "dmas" (plural) to be consistency with interrupts, gpios, ...?

Back to pieces of your patch description:

Quoted 2 lines

v3: - avoid passing an xlate function and instead pass DMA engine parameters
- define number of dma channels and requests in dma-controller node

Perhaps I lack context of previous discussion, but that seems the wrong direction, and doesn't allow DMA controllers to define the format of their DMA specifiers.

I'd expect the process to work just like other DT bindings:

* Client calls dma_request(index)
* OF DMA core gets property, does the following until it hits index:
** Get phandle
** Get provider node for phandle
** Parses #dma-cells from provider node
** If at appropriate index:
*** call xlate/get function to convert the DMA specifier to something that could be passed to e.g. dma_request_channel.
*** else skip that many cells and loop

Now, the structure returned from the xlate function could encompass filter_fn and filter_param if required.

In fact, I'd expect that all xlate return something like:

struct of_dma_filter_param {
    struct device_node *node;
    dma_filter_fn filter_fn;
    void *filter_param;
};

and the following filter function to always be used for the DT case:

bool of_dma_filter_func(...)

{
    struct of_dma_filter_param *p = ...;

    /* First, only match the controller of the DT node that parsed this
       filter data */
    if (p->node != xxx->node)
        return false;

    /* Then let the individual controller do whatever match it needs */
    return p->filter_fn(xxx, p->filter_param);
}

Quoted 1 line

3. Representing and requesting channel information

Quoted 2 lines

From a hardware perspective, a DMA channel could be represented as ...

From a hardware perspective, a DMA _resource_ could be represented as ...

Quoted 2 lines

i. channel index/number

Quoted 2 lines

ii. channel transfer type (optional)
iii. DMA interrupt mapping (optional)

I think the representation should be down to the individual DMA controller.

For example, Tegra has n identical DMA channels, and each can be used with any peripheral. However, when allocating the channel, you have to specify which peripheral request to use on that channel. So, you need to specify DMA request ID for Tegra, not DMA channel ID.

I can imagine another DMA controller where each channel is dedicated to one particular piece of HW (e.g. I2S controller 0 TX FIFO). In this case, you'd need to specify DMA channel ID here, not request ID (well, I guess they're equivalent 1:1)

This is why I think DMA controller should specify the format of their own DMA specifier in DT, and why they should provide an xlate function to parse that.

The thought here was that some DMAs may distinguish between devices by a request ID or a channel ID or both. In the case of say an OMAP4, we have 32 channels and 127 request IDs. From a h/w perspective we need to know both. Each of the 32 channels can be programmed to use any one of the 127 h/w request signals.

Other DMAs may only need to specify requests or channels. However, the thought was you could specific either or both depending on your needs. Again these should have been optional parameters.

Hi Stephen,

Quoted 6 lines · Stephen Warren

Quoted 5 lines · Jon Hunter

From: Jon Hunter <[email protected]>

This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2] to add some basic helpers to retrieve a DMA controller device_node and the DMA request/channel information.

Quoted 11 lines

Quoted 11 lines

Documentation/devicetree/bindings/dma/dma.txt new file +11 -0

+
+	sdma: dmaengine at 48000000 {
+		compatible = "ti,omap4-sdma"
+		reg = <0x48000000 0x1000>;
+		interrupts = <4>;
+		#dma-cells = <2>;
+		#dma-channels = <32>;
+		#dma-requests = <127>;
+	};
+
+

Quoted 8 lines

Documentation/devicetree/bindings/dma/dma.txt line 32

Quoted 4 lines

Documentation/devicetree/bindings/dma/dma.txt new file +4 -0

+* 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

What exactly is "request/channel"? Is it a request ID, a channel ID, both packed into a single integer (which bits are used for each field), etc.?

The thought here was that some DMAs may distinguish between devices by a request ID or a channel ID or both. In the case of say an OMAP4, we have 32 channels and 127 request IDs. From a h/w perspective we need to know both. Each of the 32 channels can be programmed to use any one of the 127 h/w request signals.

From a HW perspective, do you actually need to know both? I think the HW

Yes this is the same as OMAPs SDMA. In the case of such DMA controllers you only really care about the request ID and total number of channels that are available to you.

Quoted 3 lines

This is why I think DMA controller should specify the format of their own DMA specifier in DT, and why they should provide an xlate function to parse that.

Ok fair enough. However, it seems that at a minimum we could provide one or two xlate functions in the generic binding for people to use. One could be the DMA engine xlate binding and another could be the simple xlate binding Nicolas proposed for a DMA controller that returns a single channel/request ID.

However, at the same time, may be people would prefer that devices such as tegra, omap, at91, etc, offer their own xlate function for DMA engine. I am not sure, but we could go either way.

Quoted 9 lines · Jon Hunter

Hi Stephen,

Quoted 6 lines · Stephen Warren

Quoted 5 lines · Jon Hunter

From: Jon Hunter <[email protected]>

This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2] to add some basic helpers to retrieve a DMA controller device_node and the DMA request/channel information.

...

Quoted 20 lines

Quoted 11 lines

Quoted 11 lines

Documentation/devicetree/bindings/dma/dma.txt new file +11 -0

+
+	sdma: dmaengine at 48000000 {
+		compatible = "ti,omap4-sdma"
+		reg = <0x48000000 0x1000>;
+		interrupts = <4>;
+		#dma-cells = <2>;
+		#dma-channels = <32>;
+		#dma-requests = <127>;
+	};
+
+

Quoted 8 lines

Documentation/devicetree/bindings/dma/dma.txt line 32

Quoted 4 lines

Documentation/devicetree/bindings/dma/dma.txt new file +4 -0

+* 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

What exactly is "request/channel"? Is it a request ID, a channel ID, both packed into a single integer (which bits are used for each field), etc.?

Quoted 5 lines

The thought here was that some DMAs may distinguish between devices by a request ID or a channel ID or both. In the case of say an OMAP4, we have 32 channels and 127 request IDs. From a h/w perspective we need to know both. Each of the 32 channels can be programmed to use any one of the 127 h/w request signals.

From a HW perspective, do you actually need to know both? I think the HW description must specify the request ID, but because any channel can be programmed to any request ID, you don't actually care about the channel ID; it can be dynamically allocated by the DMA driver when the client driver calls dma_request_channel().

From the perspective of a driver using the DMA, no. However, I thought

Actually, you say the following below, so I guess you already agree here:

Quoted 3 lines

Yes this is the same as OMAPs SDMA. In the case of such DMA controllers you only really care about the request ID and total number of channels that are available to you.

...

Quoted 10 lines

Quoted 3 lines

This is why I think DMA controller should specify the format of their own DMA specifier in DT, and why they should provide an xlate function to parse that.

Ok fair enough. However, it seems that at a minimum we could provide one or two xlate functions in the generic binding for people to use. One could be the DMA engine xlate binding and another could be the simple xlate binding Nicolas proposed for a DMA controller that returns a single channel/request ID.

Quoted 3 lines

However, at the same time, may be people would prefer that devices such as tegra, omap, at91, etc, offer their own xlate function for DMA engine. I am not sure, but we could go either way.

I'd expect the bindings to be written to allow the individual DMA controllers to have complete control over the meaning of the DMA specifier.

That said, I would certainly expect some common patterns to emerge, such as a single cell to specify the DMA channel ID, or a single cell to specify the DMA request/selector ID. We should certainly make sure that where different controllers need the same information, they use the same way to represent it, and use common utility code to implement the xlate functionality. We could perhaps even write these common cases into the core DMA bindings as examples to help ensure uniformity. However, we just need to do this in a way that allows other cases too.

From: Jon Hunter <[email protected]>

This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2] to add some basic helpers to retrieve a DMA controller device_node and the DMA request/channel information.

Aim of DMA helpers
- The purpose of device-tree (as far as I understand), is to describe the ?capabilites of the hardware. Thinking about DMA controllers purely from the ?context of the hardware to begin with, we can describe a device in terms of ?a DMA controller as follows ...
? ? ? ?1. Number of DMA controllers
? ? ? ?2. Number of channels (maybe physical or logical)
? ? ? ?3. Mapping of DMA requests signals to DMA controller
? ? ? ?4. Number of DMA interrupts
? ? ? ?5. Mapping of DMA interrupts to channels
- With the above in mind the aim of the DT DMA helper functions is to extract ?the above information from the DT and provide to the appropriate driver. ?However, due to the vast number of DMA controllers and not all are using a ?common driver (such as DMA Engine) it has been seen that this is not a ?trivial task.

Quoted 6 lines · Jon Hunter

From: Jon Hunter <[email protected]>

This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2] to add some basic helpers to retrieve a DMA controller device_node and the DMA request/channel information.

Quoted 16 lines

Aim of DMA helpers
- The purpose of device-tree (as far as I understand), is to describe the capabilites of the hardware. Thinking about DMA controllers purely from the context of the hardware to begin with, we can describe a device in terms of a DMA controller as follows ...
1. Number of DMA controllers
2. Number of channels (maybe physical or logical)
3. Mapping of DMA requests signals to DMA controller
4. Number of DMA interrupts
5. Mapping of DMA interrupts to channels
- With the above in mind the aim of the DT DMA helper functions is to extract the above information from the DT and provide to the appropriate driver. However, due to the vast number of DMA controllers and not all are using a common driver (such as DMA Engine) it has been seen that this is not a trivial task.

Sorry for being slow, but so far I thought DT is only to provide _h/w_ specific info
to the _controller_ drivers. It was not supposed to provide any info pertaining to
some API (dmaengine in this case).

And I believe this is one of few situations where we are better off not generalizing
the interface - pass controller specific info in the controller driver's specified format.
Generalizing only seems to complicate things here, when we anyway have machine specific dtb, which could always have clients requesting and the controllers given
dma's info in controller driver's specific format.

Perhaps I am overlooking the need to generalize. If you think so, please help me understand by pointing out some use case for it.

Hi Jassi,

Quoted 38 lines · Jassi Brar

Quoted 6 lines · Jon Hunter

From: Jon Hunter <[email protected]>

This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2] to add some basic helpers to retrieve a DMA controller device_node and the DMA request/channel information.

Quoted 16 lines

Aim of DMA helpers
- The purpose of device-tree (as far as I understand), is to describe the ?capabilites of the hardware. Thinking about DMA controllers purely from the ?context of the hardware to begin with, we can describe a device in terms of ?a DMA controller as follows ...
? ? ? ?1. Number of DMA controllers
? ? ? ?2. Number of channels (maybe physical or logical)
? ? ? ?3. Mapping of DMA requests signals to DMA controller
? ? ? ?4. Number of DMA interrupts
? ? ? ?5. Mapping of DMA interrupts to channels
- With the above in mind the aim of the DT DMA helper functions is to extract ?the above information from the DT and provide to the appropriate driver. ?However, due to the vast number of DMA controllers and not all are using a ?common driver (such as DMA Engine) it has been seen that this is not a ?trivial task.

Sorry for being slow, but so far I thought DT is only to provide _h/w_ specific info
to the _controller_ drivers. It was not supposed to provide any info pertaining to
some API (dmaengine in this case).

And I believe this is one of few situations where we are better off not generalizing
the interface - pass controller specific info in the controller driver's specified format.
Generalizing only seems to complicate things here, when we anyway have machine specific dtb, which could always have clients requesting and the controllers given
dma's info in controller driver's specific format.

Quoted 2 lines

Perhaps I am overlooking the need to generalize. If you think so, please help me understand by pointing out some use case for it.

No not really, your points are valid. From reading the previous discussions one of the items that was clearly lacking was the ability to represent and identify a device having more than one DMA controller. In other words, when you request the DMA resource, how do you identify which DMA controller in the system that resource belongs too. With DMA engine there are ways we can do this.

I see this requires a client driver to specify a particular req_line on a particular dma controller. I am not sure if this is most optimal.

I think such client->req_line map should be provided to the dmac controller driver via its dt node in some format. The dmac driver could then populate a dma_chan, or similar, only for that req_line and not for the unused one which otherwise could also have served the same client.

Ideally the I2C driver should simply ask, say, a channel for TX and another for RX, everything else should already be setup via dmac's dt nodes.

Quoted 10 lines · Jassi Brar

I see this requires a client driver to specify a particular req_line on a particular dma controller. I am not sure if this is most optimal.

I think such client->req_line map should be provided to the dmac controller driver via its dt node in some format. The dmac driver could then populate a dma_chan, or similar, only for that req_line and not for the unused one which otherwise could also have served the same client.

Ideally the I2C driver should simply ask, say, a channel for TX and another for RX, everything else should already be setup via dmac's dt nodes.

If I understand you correctly, you think we can generalize the dma-request data in a way that we don't need to pass a specific dma engine phandle. I very much doubt this is possible, given how different the requirements are for each of the engines we support. You really need to pass a specific phandle so that we can find the code that can interpret the data in a meaningful way.

Quoted 12 lines · Arnd Bergmann

Quoted 10 lines · Jassi Brar

I see this requires a client driver to specify a particular req_line on a particular dma controller. I am not sure if this is most optimal.

I think such client->req_line map should be provided to the dmac controller driver via its dt node in some format. The dmac driver could then populate a dma_chan, or similar, only for that req_line and not for the unused one which otherwise could also have served the same client.

Ideally the I2C driver should simply ask, say, a channel for TX and another for RX, everything else should already be setup via dmac's dt nodes.

Quoted 7 lines

If I understand you correctly, you think we can generalize the dma-request data in a way that we don't need to pass a specific dma engine phandle. I very much doubt this is possible, given how different the requirements are for each of the engines we support. You really need to pass a specific phandle so that we can find the code that can interpret the data in a meaningful way.

Hmm... there ought to be a way by which a client is handed a random 'token' via its dt node and similarly the dmac informed which channel (and with what capabilities) to allocate should it see a request coming with that token.

That way dmac and client drivers using DT could do away with the filter_fn.

Roughly speaking (I am not very well versed with DT syntax)

Client Node:-

mmc1: mmc at 13002000 {
...
dma_tx = <891> //some platform-wide unique value
dma_rx = <927> //some platform-wide unique value
...
};

Quoted 3 lines · Jassi Brar

Hmm... there ought to be a way by which a client is handed a random 'token' via its dt node and similarly the dmac informed which channel (and with what capabilities) to allocate should it see a request coming with that token.

I think that's the whole point of the patch.

However, the token needs to be some driver-specific struct, since the required information may be more than just a single channel or request ID in general.

Quoted 4 lines

That way dmac and client drivers using DT could do away with the filter_fn.

Roughly speaking (I am not very well versed with DT syntax)

Quoted 8 lines

Client Node:-

mmc1: mmc at 13002000 {
? ? ? ? ...
? ? ? ? dma_tx = <891> ? //some platform-wide unique value ? ? ? ? dma_rx = <927> ? //some platform-wide unique value ? ? ? ? ...
?};

I believe we specifically don't want to introduce any global concept of DMA channel ID, either within the kernel, or at the device tree level.

Quoted 6 lines · Stephen Warren

Quoted 3 lines · Jassi Brar

Hmm... there ought to be a way by which a client is handed a random 'token' via its dt node and similarly the dmac informed which channel (and with what capabilities) to allocate should it see a request coming with that token.

Quoted 6 lines

I think that's the whole point of the patch.

However, the token needs to be some driver-specific struct, since the required information may be more than just a single channel or request ID in general.

Well, what I call 'token' could just as well be some numerical hash of what you call 'driver-specific struct'.
And I never thought we could do without h/w specific information, just that peculiarities lie with the dmac controllers and that's where the discerning should happen.

Our opinions differ in that, I believe client side shouldn't need to parse/decode the h/w specific parameters from data gotten via DT (what I call 'token' and you 'driver-specific struct') - because that is the key to having common client drivers working with different dma controllers. And yes, I don't think we could find a future proof generic and simple enough representation of dma resource specification in a DT node :)

Instead, I suggest we encode the finer details of each channel-request in node of the dma controller matched against the 'token' assigned to the respective clients.

That encoding("channel_id") would be dma
controller specific and if we also manage to contain it within fixed number of bytes we could also have common helpers for fetching it,

though
it still would need to be decoded by dmac controller driver - which I think we can't do without, considering the variety of dma floating around.

The data doesn't need to be part of the DMA controller node in order for the DMA controller driver to be the entity interpreting it.

The advantage here is that:

* The specifier is stored in the client, not the IRQ/GPIO/DMA controller's node, so it's located right at the usage site, which typically makes working out what resources a client uses easier.

This also keeps client-specific information out of the provider node, allowing it to be fully generic.

Quoted 4 lines

That encoding("channel_id") would be dma
controller specific and if we also manage to contain it within fixed number of bytes we could also have common helpers for fetching it,

I don't think there's any need for it to fit into a fixed number of bytes

Quoted 4 lines · Stephen Warren

The data doesn't need to be part of the DMA controller node in order for the DMA controller driver to be the entity interpreting it.

I rather say, if the dma controller driver is the entity going to interpret the data, why not provide the data directly through its node at one go?

There is important difference between providing data via clients during runtime vs providing info about every client to the dmac driver at one go during its probe.

Quoted 6 lines

Quoted 3 lines

That encoding("channel_id") would be dma
controller specific and if we also manage to contain it within fixed number of bytes we could also have common helpers for fetching it,

I don't think there's any need for it to fit into a fixed number of bytes

Yeah, I realized that soon after posting.

I think I have run out of ways to explain myself better. FWIW, I won't object to whatever mechanism folks decide after knowing my concerns.

Quoted 4 lines · Stephen Warren

The data doesn't need to be part of the DMA controller node in order for the DMA controller driver to be the entity interpreting it.

I rather say, if the dma controller driver is the entity going to interpret the data, why not provide the data directly through its node at one go?

There is important difference between providing data via clients during runtime vs providing info about every client to the dmac driver at one go during its probe.

Quoted 5 lines

The advantage here is that:

* The specifier is stored in the client, not the IRQ/GPIO/DMA controller's node, so it's located right at the usage site, which typically makes working out what resources a client uses easier.

A client doesn't really need some n'th channel of some m'th dma controller. A client simply needs one channel for transmitting data whichever platform it be.
So {n,m}_channel isn't really a required resource, dma_tx is, which I still specify in the client's node.
The fact that this information is simply fwd by the client as such to the dmac
(via utility code), makes the ritual even more pointless.

Quoted 2 lines

This also keeps client-specific information out of the provider node, allowing it to be fully generic.

A typical Samsung SoC has 2 peripheral DMA controllers and
about 40 possible clients. Of which ~20 clients could be served by either of the two dmacs. Of course hardly ever a machine has 10 clients. It would be desirable if the dma driver doesn't populate the unused 54(32+32-10) channels,
presumably also reserving limited resources for each, on the machine.

Consider this example ...
A PL330 instance has 8 physical channels and 32 client i/f.
So only 8 client reqs could be active at any time.
PL330 doesn't provide a way to program a true LLI transfer(very useful for audio) using a single physical channel. However, we could emulate true LLI if we employ 2 physical channels for one audio channel request.

Obviously, if a machine had 7 or lesser clients, one would freely employ 2 physical channels for audio and yet not worry about starving other clients. How would we achieve this if the dmac driver was initialized as "fully generic" ?

Quoted 4 lines · Jassi Brar

There is important difference between providing data via clients during runtime vs providing info about every client to the dmac driver at one go during its probe.

I certainly see that there is a difference.

I don't understand why it's an important difference; I think everything needs to be handled at run-time no matter what:

Just because there is information in the DT that "this client DT node uses this channel/request-ID/... of this DMA controller" doesn't mean that the driver for the client is going to be loaded, or that SW running on the system is going to cause that driver to actually be opened and do any DMA. As such, any resource allocation rules etc. must be evaluated only if/when a driver actually requests/uses a DMA channel, so having "all the information up front" doesn't seem like a theoretically possible thing anyway.

Very similar to this, in order to support your point that a given client could potentially use a channel from either of 2 different DMA controller instances, you don't know until run-time which controller will be used, so can't have up-front information in this scenario either, even if the DMA does actually take place.

Solving (b) seems to require something very roughly like:

dma-controller at 0 {
? ?compatible = "foo,dmac-xxx";
? ?dma-requests = <
? ? ? ? ? ? ? ?&client0 0 // DMAC req 0 is client0's 0th req output ? ? ? ? ? ? ? ?&client0 1
? ? ? ? ? ? ? ?&client1 0
? ? ? ? ? ? ? ?&client1 1
? ? ? ? ? ? ? ?&client2 0
? ? ? ? ? ? ? ?...>;
};

dma-controller at 1 {
? ?compatible = "bar,dmac-yyy";
? ?dma-requests = <
? ? ? ? ? ? ? ?// Supports some client modules, but not the same ones ? ? ? ? ? ? ? ?&client0 0
? ? ? ? ? ? ? ?&client1 0
? ? ? ? ? ? ? ?&client3 0
? ? ? ? ? ? ? ?...>;
};

client0: i2s {
? ?/* has 2 DMA request output signals: 0, 1 */
};

client1: spdif {
? ?/* has 2 DMA request signals: 0, 1 */
};

client2: foo {
? ?/* has 1 DMA request signal: 0 */
};

client3: bar {
? ?/* has 1 DMA request signal: 0 */
};

and then having the core DMA code have an API like:

channel = dma_request(clients_dma_req_output_id)

which looks in each DMA controller's dma-requests list for the client's phandle and matching dma_req_output_id.

Quoted 11 lines · Stephen Warren

Quoted 4 lines · Jassi Brar

There is important difference between providing data via clients during runtime vs providing info about every client to the dmac driver at one go during its probe.

I certainly see that there is a difference.

I don't understand why it's an important difference; I think everything needs to be handled at run-time no matter what:

Quoted 8 lines

Just because there is information in the DT that "this client DT node uses this channel/request-ID/... of this DMA controller" doesn't mean that the driver for the client is going to be loaded, or that SW running on the system is going to cause that driver to actually be opened and do any DMA. As such, any resource allocation rules etc. must be evaluated only if/when a driver actually requests/uses a DMA channel, so having "all the information up front" doesn't seem like a theoretically possible thing anyway.

One point is about 'qos' here.... something like bandwidth allocation. If the dmac driver knew up-front the max possible clients that could be active simultaneously, it could "divide the bandwidth" accordingly. Again, the example of PL330 employing 2physical channels for better service - LLI (you got it right), where even 1 physical channel would also have served only not as reliably. I believe there would be more such scenarios.

Another minor benefit could be that the dmac driver populate only as many "struct dma_chan" as there are actually clients on the machine (and this population has to be done during probe). It could mean ~8 instead of ~40 channels populated on a machine. Note, a PL330 dmac can have 32 channels, OMAP's has 128....

Most importantly, I just think it's a cleaner approach.

Quoted 5 lines

Very similar to this, in order to support your point that a given client could potentially use a channel from either of 2 different DMA controller instances, you don't know until run-time which controller will be used, so can't have up-front information in this scenario either, even if the DMA does actually take place.

Sorry, perhaps I wasn't clear enough.
A client sees _no_ difference between the two channels that could serve it. And it can't start using two, if two are available.
Client just needs one suitable channel on whichever dmac that might be. If the channel for a client is to be switched runtime, that decision should lie with the dmac driver, not the client. And I am not really after implementing that runtime decision support.

Quoted 39 lines

Solving (b) seems to require something very roughly like:

dma-controller at 0 {
   compatible = "foo,dmac-xxx";
   dma-requests = <
               &client0 0 // DMAC req 0 is client0's 0th req output
               &client0 1
               &client1 0
               &client1 1
               &client2 0
               ...>;
};

dma-controller at 1 {
   compatible = "bar,dmac-yyy";
   dma-requests = <
               // Supports some client modules, but not the same ones
               &client0 0
               &client1 0
               &client3 0
               ...>;
};

client0: i2s {
   /* has 2 DMA request output signals: 0, 1 */
};

client1: spdif {
   /* has 2 DMA request signals: 0, 1 */
};

client2: foo {
   /* has 1 DMA request signal: 0 */
};

client3: bar {
   /* has 1 DMA request signal: 0 */
};

Quoted 6 lines

and then having the core DMA code have an API like:

channel = dma_request(clients_dma_req_output_id)

which looks in each DMA controller's dma-requests list for the client's phandle and matching dma_req_output_id.

Almost what I proposed in my third post in this thread !!

The minor difference being, you use the {request-signal, phandle} pair to find the right channel, I used only 'token'.

Please note, with this method we specify info about every client at one go and via dmac's DT node - hence those benefits.

Also note that, a client's dma specifier becomes perfectly general and future-proof

   client1: spdif {
          dma_tx = <278>
          dma_rx = <723>
    };

otherwise the following representation

    client1: spdif {
               dma = <&sdma 2 1 &sdma 3 2>;
     };

could break with some weird dma setups (IIANW Russell already finds it wouldn't work for him).

Quoted 7 lines · Jassi Brar

One point is about 'qos' here.... something like bandwidth allocation. If the dmac driver knew up-front the max possible clients that could be active simultaneously, it could "divide the bandwidth" accordingly. Again, the example of PL330 employing 2physical channels for better service - LLI (you got it right), where even 1 physical channel would also have served only not as reliably. I believe there would be more such scenarios.

QoS seems like policy to me, whereas DT is more about describing the HW. Is DT the correct place to describe QoS policy?

I guess you are talking more about deriving policy from the description of HW, i.e. how many client described in DT.

However, for some reason that seems dangerous to me; what if clients can be instantiated some other way?

For a 1:1 mapping (or 1:n mapping in HW with static selection specified in the DT) between DMA client and DMA controller, perhaps the controller can indeed make QoS decisions based on which (how many) clients are connected to it.

However, if a DMA client can be serviced by more than 1 DMA engine, and the decision as to which DMA engine to use occurs at run-time by the DMA driver core, rather than being statically configured in the DT, then the DMA controller drivers cannot know ahead of time which will be used

Quoted 3 lines

The minor difference being, you use the {request-signal, phandle} pair to find the right channel, I used only 'token'.

That's a pretty big difference, I think.

In your proposal, every token was globally unique (well, within the 1 DT file). I'd rather not see any more global numbering schemes.

Now, DMA requests are signals /from/ a DMA client to a DMA controller ("send more data please", or "pull more data please"). Hence, I assert that the phandle should refer to the DMA client, not the DMA controller.

Quoted 9 lines

Also note that, a client's dma specifier becomes perfectly general and future-proof

? ?client1: spdif {
? ? ? ? ? dma_tx = <278>
? ? ? ? ? dma_rx = <723>
? ? };

Quoted 8 lines

otherwise the following representation

? ? client1: spdif {
? ? ? ? ? ? ? ?dma = <&sdma 2 1 &sdma 3 2>;
? ? ?};

could break with some weird dma setups (IIANW Russell already finds it wouldn't work for him).

To solve Russell's HW, we need some way of representing the mux directly in DT irrespective of how the DMA controller or DMA client specify what they're connected to. Anything else isn't representing the HW in DT.

Also, who knows how to control the mux? We need that to be fully general, and so the mux itself really needs some kind of driver which the DMA core or DMA controller can call into when the channel is allocated in order to set up the mux. Right now, Russell's driver calls in the a platform-/board-provided callback, but we should really architect a generic driver framework for this.

client0: i2s {
/* has 2 DMA request output signals: 0, 1 */
};

client1: spdif {
/* has 2 DMA request signals: 0, 1 */
};

Quoted 10 lines · Stephen Warren

Quoted 7 lines · Jassi Brar

One point is about 'qos' here.... something like bandwidth allocation. If the dmac driver knew up-front the max possible clients that could be active simultaneously, it could "divide the bandwidth" accordingly. Again, the example of PL330 employing 2physical channels for better service - LLI (you got it right), where even 1 physical channel would also have served only not as reliably. I believe there would be more such scenarios.

Quoted 5 lines

QoS seems like policy to me, whereas DT is more about describing the HW. Is DT the correct place to describe QoS policy?

I guess you are talking more about deriving policy from the description of HW, i.e. how many client described in DT.

Yeah, that's what I meant.

Quoted 2 lines

However, for some reason that seems dangerous to me; what if clients can be instantiated some other way?

The other way could be hotplug ?

Anyway in those machines every channel would be populated and dmac driver would always account for the all-ports-plugged case.

Quoted 9 lines

For a 1:1 mapping (or 1:n mapping in HW with static selection specified in the DT) between DMA client and DMA controller, perhaps the controller can indeed make QoS decisions based on which (how many) clients are connected to it.

However, if a DMA client can be serviced by more than 1 DMA engine, and the decision as to which DMA engine to use occurs at run-time by the DMA driver core, rather than being statically configured in the DT, then the DMA controller drivers cannot know ahead of time which will be used

I think the dmac driver would make use of the routing flexibility to sustain its 'qos', and not the other way around (decide qos based on which one of the two dmacs would provide a channel to a client in future).
Anyways, so far only Samsung SoCs seem to have that flexibility/redundancy and I have never had anyone asking for that runtime decision making.

Quoted 3 lines

Quoted 2 lines

The minor difference being, you use the {request-signal, phandle} pair to find the right channel, I used only 'token'.

Quoted 4 lines

That's a pretty big difference, I think.

In your proposal, every token was globally unique (well, within the 1 DT file). I'd rather not see any more global numbering schemes.

Probably my shallow experience, but "globally unique, within a file" sounds like an oxymoron :)

I think arbitrary numerical tokens are a reasonable price to pay for the robustness and simplicity they bring.

Quoted 3 lines

Now, DMA requests are signals /from/ a DMA client to a DMA controller ("send more data please", or "pull more data please"). Hence, I assert that the phandle should refer to the DMA client, not the DMA controller.

OK, though we may just want to convey information about the h/w setup from the s/w POV, rather than info to simulate the h/w ;)

For ex, the dma api and controller drivers don't really care about the fact that the client's driver must set some bit to trigger operation, whereas some simulator would need to care about that.

Anyways, I am OK with whatever works well and make things simpler.

Quoted 21 lines

Quoted 8 lines

Also note that, a client's dma specifier becomes perfectly general and future-proof

   client1: spdif {
          dma_tx = <278>
          dma_rx = <723>
    };

Quoted 8 lines

otherwise the following representation

    client1: spdif {
               dma = <&sdma 2 1 &sdma 3 2>;
     };

could break with some weird dma setups (IIANW Russell already finds it wouldn't work for him).

To solve Russell's HW, we need some way of representing the mux directly in DT irrespective of how the DMA controller or DMA client specify what they're connected to. Anything else isn't representing the HW in DT.

Quoted 6 lines

Also, who knows how to control the mux? We need that to be fully general, and so the mux itself really needs some kind of driver which the DMA core or DMA controller can call into when the channel is allocated in order to set up the mux. Right now, Russell's driver calls in the a platform-/board-provided callback, but we should really architect a generic driver framework for this.

Well, I doubt if there would ever be enough such platforms to warrant a new generic framework. For now, I would leave that to be a matter between the dmac driver and its DT node.

Similarly let every dmac, being unique, require DT data in it's own custom format - I don't believe we can find a generic DT format for every kind of dmac that does exist or would exist. (For ex, you found a way for RMK's mux'ed req_lines, but what about assigning priorities to clients which is possible with PL08X dmacs but not most others?)

So, I would strive only to make clients' dma specifier generic.

Quoted 4 lines

client0: i2s {
/* has 2 DMA request output signals: 0, 1 */
};

Quoted 4 lines

client1: spdif {
/* has 2 DMA request signals: 0, 1 */
};

Do we also need to somehow tag these signals for the client to differentiate between TX and RX channel ?

Quoted 2 lines · Jassi Brar

I think arbitrary numerical tokens are a reasonable price to pay for the robustness and simplicity they bring.

I have to disagree here.

Using phandle+ID is almost as simple, and much more flexible. Global IDs have a number of disadvantages:

a) You have to somehow keep them unique. Even with just a single .dts file, that's going to be a little painful since there's no central table of these IDs.

What if the DT is composed of a bunch of chunks that represent pluggable boards, which may be mixed/matched together depending on what the user actually plugged in? Then, you have to be very careful to keep the n different files' numbering ranges segregated, or conflicts will occur.

Quoted 5 lines

Well, I doubt if there would ever be enough such platforms to warrant a new generic framework. For now, I would leave that to be a matter between the dmac driver and its DT node.

Quoted 5 lines

Similarly let every dmac, being unique, require DT data in it's own custom format - I don't believe we can find a generic DT format for every kind of dmac that does exist or would exist. (For ex, you found a way for RMK's mux'ed req_lines, but what about assigning priorities to clients which is possible with PL08X dmacs but not most others?)

Good question. Again thought that sounds a little like policy, so perhaps should be negotiated at runtime rather than described in DT?

Quoted 9 lines

Quoted 9 lines

client0: i2s {
? /* has 2 DMA request output signals: 0, 1 */
};

client1: spdif {
? /* has 2 DMA request signals: 0, 1 */
};

Quoted 2 lines

Do we also need to somehow tag these signals for the client to differentiate between TX and RX channel ?

Yes, the client's DT binding would certainly need to describe how many DMA request signals its HW generates, and give a unique ID to each. The driver would need to request a DMA channel for a specific one of its DMA requests.

Quoted 1 line · Stephen Warren

On 05/10/2012 01:59 PM, Jassi Brar wrote:

Quoted 11 lines

Quoted 10 lines

Quoted 8 lines

client0: i2s {
/* has 2 DMA request output signals: 0, 1 */
};

client1: spdif {
/* has 2 DMA request signals: 0, 1 */
};

Do we also need to somehow tag these signals for the client to differentiate between TX and RX channel ?

Quoted 5 lines

Yes, the client's DT binding would certainly need to describe how many DMA request signals its HW generates, and give a unique ID to each. The driver would need to request a DMA channel for a specific one of its DMA requests.

Did I read "give a unique ID to each" correctly ?

Could you please take some time out to jot down an example of how a typical client's dma specifier should look.

FWIW, I think I can live with what you propose. Let us go for the kill.

Quoted 2 lines · Jassi Brar

Quoted 1 line · Stephen Warren

On 05/10/2012 01:59 PM, Jassi Brar wrote:

...

Quoted 16 lines

Quoted 16 lines

Quoted 10 lines

Quoted 8 lines

client0: i2s {
? /* has 2 DMA request output signals: 0, 1 */
};

client1: spdif {
? /* has 2 DMA request signals: 0, 1 */
};

Do we also need to somehow tag these signals for the client to differentiate between TX and RX channel ?

Yes, the client's DT binding would certainly need to describe how many DMA request signals its HW generates, and give a unique ID to each. The driver would need to request a DMA channel for a specific one of its DMA requests.

Quoted 1 line

Did I read "give a unique ID to each" correctly ?

It'd be unique relative to that individual device or DT node, not at any larger scope.

Quoted 2 lines

Could you please take some time out to jot down an example of how a typical client's dma specifier should look.

With this proposal, I'm not sure that the client DT node would need any DMA information at all, at least nothing that identifies which DMA controllers, channels, or requests are required to service this node/device's DMA requests - that routing information is all represented in the DMA controller itself.

(I think Arnd made the following point earlier in this thread):

If you did need to put any other information in DT, then that probably would go in the DMA client node, since it'd presumably be the same irrespective of which DMA controller got used. However, that information presumably wouldn't be needed in DT at all, since the driver would know it, since it'd be a facet of the HW.

Note: I'm thinking things like DMA physical address (presumably an offset from the reg property), DMA access size (presumably a fixed property of the HW), DMA burst size (presumably a property of the HW, although at least some HW can be programmed to raise the DMA request signal with a varying number of FIFO entries free, so not fixed), etc.

Quoted 20 lines · Stephen Warren

Quoted 2 lines · Jassi Brar

Quoted 1 line · Stephen Warren

On 05/10/2012 01:59 PM, Jassi Brar wrote:

...

Quoted 16 lines

Quoted 16 lines

Quoted 10 lines

Quoted 8 lines

client0: i2s {
/* has 2 DMA request output signals: 0, 1 */
};

client1: spdif {
/* has 2 DMA request signals: 0, 1 */
};

Do we also need to somehow tag these signals for the client to differentiate between TX and RX channel ?

Yes, the client's DT binding would certainly need to describe how many DMA request signals its HW generates, and give a unique ID to each. The driver would need to request a DMA channel for a specific one of its DMA requests.

Quoted 5 lines

Quoted 1 line

Did I read "give a unique ID to each" correctly ?

It'd be unique relative to that individual device or DT node, not at any larger scope.

OK.

Quoted 17 lines

Quoted 2 lines

Could you please take some time out to jot down an example of how a typical client's dma specifier should look.

With this proposal, I'm not sure that the client DT node would need any DMA information at all, at least nothing that identifies which DMA controllers, channels, or requests are required to service this node/device's DMA requests - that routing information is all represented in the DMA controller itself.

(I think Arnd made the following point earlier in this thread):

If you did need to put any other information in DT, then that probably would go in the DMA client node, since it'd presumably be the same irrespective of which DMA controller got used. However, that information presumably wouldn't be needed in DT at all, since the driver would know it, since it'd be a facet of the HW.

Quoted 6 lines

Note: I'm thinking things like DMA physical address (presumably an offset from the reg property), DMA access size (presumably a fixed property of the HW), DMA burst size (presumably a property of the HW, although at least some HW can be programmed to raise the DMA request signal with a varying number of FIFO entries free, so not fixed), etc.

Yeah, neither did I think a client node should tell more than IDs of its channels
- which we now decide to simply mention in its binding.

How do I know if you or someone is interested in reworking the patchset or want me to give it a try?

To solve Russell's HW, we need some way of representing the mux directly in DT irrespective of how the DMA controller or DMA client specify what they're connected to. Anything else isn't representing the HW in DT.

Quoted 4 lines

That way dmac and client drivers using DT could do away with the filter_fn.

Roughly speaking (I am not very well versed with DT syntax)

Quoted 8 lines

Client Node:-

mmc1: mmc at 13002000 {
...
dma_tx = <891> //some platform-wide unique value
dma_rx = <927> //some platform-wide unique value
...
};

I believe we specifically don't want to introduce any global concept of DMA channel ID, either within the kernel, or at the device tree level.

While we do have global interrupt and GPIO IDs within the kernel, this has caused problems, I think primarily due to the need to manage this unified namespace and allocate a single global ID for a bunch of different ranges of controller-specific IDs.

Within device tree, everything is always represented as an ID relative-to or within a particular controller, so it's easy to namespace things separately.

Quoted 44 lines · Jon Hunter

Hi Jassi,

Quoted 38 lines · Jassi Brar

Quoted 6 lines · Jon Hunter

From: Jon Hunter <[email protected]>

This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2] to add some basic helpers to retrieve a DMA controller device_node and the DMA request/channel information.

Quoted 16 lines

Aim of DMA helpers
- The purpose of device-tree (as far as I understand), is to describe the capabilites of the hardware. Thinking about DMA controllers purely from the context of the hardware to begin with, we can describe a device in terms of a DMA controller as follows ...
1. Number of DMA controllers
2. Number of channels (maybe physical or logical)
3. Mapping of DMA requests signals to DMA controller
4. Number of DMA interrupts
5. Mapping of DMA interrupts to channels
- With the above in mind the aim of the DT DMA helper functions is to extract the above information from the DT and provide to the appropriate driver. However, due to the vast number of DMA controllers and not all are using a common driver (such as DMA Engine) it has been seen that this is not a trivial task.

Sorry for being slow, but so far I thought DT is only to provide _h/w_ specific info
to the _controller_ drivers. It was not supposed to provide any info pertaining to
some API (dmaengine in this case).

And I believe this is one of few situations where we are better off not generalizing
the interface - pass controller specific info in the controller driver's specified format.
Generalizing only seems to complicate things here, when we anyway have machine specific dtb, which could always have clients requesting and the controllers given
dma's info in controller driver's specific format.

Quoted 2 lines

Perhaps I am overlooking the need to generalize. If you think so, please help me understand by pointing out some use case for it.

Quoted 7 lines

No not really, your points are valid. From reading the previous discussions one of the items that was clearly lacking was the ability to represent and identify a device having more than one DMA controller. In other words, when you request the DMA resource, how do you identify which DMA controller in the system that resource belongs too. With DMA engine there are ways we can do this.

Well, if we really can't have dmac drivers make 'intelligent' runtime decision of request mapping on more than one capable controllers, we still can make it simpler than the proposed scheme.

Documentation/devicetree/bindings/dma/dma.txt new file +5 -0

+       i2c1: i2c at 1 {
+               ...
+               dma = <&sdma 2 1 &sdma 3 2>;
+               ...
+       };

Quoted 1 line

I see this requires a client driver to specify a particular req_line on a particular dma controller. I am not sure if this is most optimal.

I think such client->req_line map should be provided to the dmac controller driver via its dt node in some format. The dmac driver could then populate a dma_chan, or similar, only for that req_line and not for the unused one which otherwise could also have served the same client.

Ideally the I2C driver should simply ask, say, a channel for TX and another for RX, everything else should already be setup via dmac's dt nodes.

Quoted 7 lines · Jassi Brar

+ ? ? ? i2c1: i2c at 1 {
+ ? ? ? ? ? ? ? ...
+ ? ? ? ? ? ? ? dma = <&sdma 2 1 &sdma 3 2>;
+ ? ? ? ? ? ? ? ...
+ ? ? ? };

Quoted 1 line

Quoted 2 lines

I see this requires a client driver to specify a particular req_line on a particular dma controller. I am not sure if this is most optimal.

Actually, no. The phandle in the DT specifies the DMA controller to use. Then the client simply asks for a channel with a particular property, for example, DMA_MEM_TO_DEV (ie. TX) and the channel information is return.

Quoted 7 lines

I think such client->req_line map should be provided to the dmac controller driver via its dt node in some format. The dmac driver could then populate a dma_chan, or similar, only for that req_line and not for the unused one which otherwise could also have served the same client.

Ideally the I2C driver should simply ask, say, a channel for TX and another for RX, everything else should already be setup via dmac's dt nodes.

Yes that is the intention here.

Hi Jon,

Quoted 8 lines · Jon Hunter

Quoted 7 lines · Jassi Brar

Documentation/devicetree/bindings/dma/dma.txt new file +5 -0

+       i2c1: i2c at 1 {
+               ...
+               dma = <&sdma 2 1 &sdma 3 2>;
+               ...
+       };

Quoted 1 line

Quoted 7 lines

Quoted 2 lines

I see this requires a client driver to specify a particular req_line on a particular dma controller. I am not sure if this is most optimal.

Actually, no. The phandle in the DT specifies the DMA controller to use. Then the client simply asks for a channel with a particular property, for example, DMA_MEM_TO_DEV (ie. TX) and the channel information is return.

See below.

Quoted 5 lines

Quoted 5 lines

I think such client->req_line map should be provided to the dmac controller driver via its dt node in some format. The dmac driver could then populate a dma_chan, or similar, only for that req_line and not for the unused one which otherwise could also have served the same client.

Quoted 5 lines

Quoted 2 lines

Ideally the I2C driver should simply ask, say, a channel for TX and another for RX, everything else should already be setup via dmac's dt nodes.

Yes that is the intention here.

But the client is required to specify the dmac that would serve it. Which is more
than simply asking for "some suitable channel".

If you read the whole exchange between I and Stephen, we converged on a scheme of clients' node having nothing to specify and DMAC told all about every client in one palce. Which resembles closer to reality and is much simpler.
I already started on a patchset and should be able submit for review in a day or two.

Hi Jassi,

Quoted 28 lines · Jassi Brar

Hi Jon,

Quoted 8 lines · Jon Hunter

Quoted 7 lines · Jassi Brar

(quoted diff) +5 -0

+       i2c1: i2c at 1 {
+               ...
+               dma = <&sdma 2 1 &sdma 3 2>;
+               ...
+       };

Quoted 1 line

Quoted 7 lines

Quoted 2 lines

I see this requires a client driver to specify a particular req_line on a particular dma controller. I am not sure if this is most optimal.

Actually, no. The phandle in the DT specifies the DMA controller to use. Then the client simply asks for a channel with a particular property, for example, DMA_MEM_TO_DEV (ie. TX) and the channel information is return.

See below.

Quoted 8 lines

Quoted 7 lines

I think such client->req_line map should be provided to the dmac controller driver via its dt node in some format. The dmac driver could then populate a dma_chan, or similar, only for that req_line and not for the unused one which otherwise could also have served the same client.

Ideally the I2C driver should simply ask, say, a channel for TX and another for RX, everything else should already be setup via dmac's dt nodes.

Quoted 5 lines

Quoted 2 lines

Yes that is the intention here.

But the client is required to specify the dmac that would serve it. Which is more
than simply asking for "some suitable channel".

No this is not the case with what I propose. The client knows nothing about the dmac.

Quoted 37 lines · Jon Hunter

Hi Jassi,

Quoted 33 lines · Jassi Brar

Hi Jon,

Quoted 8 lines · Jon Hunter

Quoted 7 lines · Jassi Brar

(quoted diff) +5 -0

+       i2c1: i2c at 1 {
+               ...
+               dma = <&sdma 2 1 &sdma 3 2>;
+               ...
+       };

Quoted 1 line

Quoted 7 lines

Quoted 2 lines

I see this requires a client driver to specify a particular req_line on a particular dma controller. I am not sure if this is most optimal.

Actually, no. The phandle in the DT specifies the DMA controller to use. Then the client simply asks for a channel with a particular property, for example, DMA_MEM_TO_DEV (ie. TX) and the channel information is return.

See below.

Quoted 10 lines

Quoted 7 lines

I think such client->req_line map should be provided to the dmac controller driver via its dt node in some format. The dmac driver could then populate a dma_chan, or similar, only for that req_line and not for the unused one which otherwise could also have served the same client.

Ideally the I2C driver should simply ask, say, a channel for TX and another for RX, everything else should already be setup via dmac's dt nodes.

Yes that is the intention here.

But the client is required to specify the dmac that would serve it. Which is more
than simply asking for "some suitable channel".

Quoted 2 lines

No this is not the case with what I propose. The client knows nothing about the dmac.

I think you're both talking about slightly different things.

Jon: You're talking about the driver code.
Jassi: You're talking about the device tree.

By including a property in the DMA client's DT node that describes which DMA controller services it, the device (the DT node) "knows" about the DMA controllers.

By moving the information to the DMA controller and specifying which DMA clients can be serviced, that DMA client DT node no longer contains any information about the DMA controller that serves it, and hence it's not bound to a single controller, and hence can select from 1 of n controllers at run-time if applicable given the HW.

Hi Jassi,

Quoted 28 lines · Jassi Brar

Hi Jon,

Quoted 8 lines · Jon Hunter

Quoted 7 lines · Jassi Brar

(quoted diff) +5 -0

+       i2c1: i2c at 1 {
+               ...
+               dma = <&sdma 2 1 &sdma 3 2>;
+               ...
+       };

Quoted 1 line

Quoted 7 lines

Quoted 2 lines

I see this requires a client driver to specify a particular req_line on a particular dma controller. I am not sure if this is most optimal.

Actually, no. The phandle in the DT specifies the DMA controller to use. Then the client simply asks for a channel with a particular property, for example, DMA_MEM_TO_DEV (ie. TX) and the channel information is return.

See below.

Quoted 8 lines

Quoted 7 lines

I think such client->req_line map should be provided to the dmac controller driver via its dt node in some format. The dmac driver could then populate a dma_chan, or similar, only for that req_line and not for the unused one which otherwise could also have served the same client.

Ideally the I2C driver should simply ask, say, a channel for TX and another for RX, everything else should already be setup via dmac's dt nodes.

Quoted 5 lines

Quoted 2 lines

Yes that is the intention here.

But the client is required to specify the dmac that would serve it. Which is more
than simply asking for "some suitable channel".

No this is not the case with what I propose. The client knows nothing about the dmac.

By the way, I do see your point. You wish to describe the all the mappings available to all dma controllers and then set a mapping in the device tree. Where as I am simply setting a mapping and do not list all other possibilities (assuming that there some).

What is still unclear to me, is if you use this token approach how readable is the device-tree? For example, if you have a client that can use one of two dmac and for each dmac the request/channel number is different, then by using a global token how can I determine what the options available for this client are?

Take your example ...

mmc1: mmc at 13002000 {
...
dma_tx = <891> //some platform-wide unique value
dma_rx = <927> //some platform-wide unique value
...
};

DMAC's Node:-

pdma2: pdma at 10800000 {
         .......
	dma_map = <891, 7>,       // Map mmc1_tx onto i/f 7
		  <927, 8>,       // Map mmc1_rx onto i/f 8
	.......
};

But now I have another dmac which has the following options ...

pdma1: pdma at 10000000 {
         .......
	dma_map = <598, 2>,       // Map mmc1_tx onto i/f 2
		  <230, 3>,       // Map mmc1_rx onto i/f 3
	.......
};

Other than using a comment or yet another token to represent the client, it is not clear from the arbitrary token value itself what my options are.

One way around this would be to have an enable/disable flag along with the token such as ...

mmc1: mmc at 13002000 {
        ...
        dma_tx = <891, 1>   // default tx channel
        dma_rx = <927, 1>   // default rx channel
        dma_tx = <598, 0>   // other available tx channel
        dma_rx = <230, 0>   // other available rx channel
        ...
};

That being said, we could take the same approach with using the dmac phandle instead of the token. So you would have ...

mmc1: mmc at 13002000 {
...
// phandle + channel + enable/disable
dma_tx = <pdma0, 7, 1> // default tx channel
dma_rx = <pdma0, 8, 1> // default rx channel
dma_tx = <pdma1, 2, 0> // other available tx channel
dma_rx = <pdma1, 3, 0> // other available rx channel
...
};

Then you could eliminate the random token and dma map from the dmac. Seems easier to read too.

On 05/16/2012 10:01 AM, Jon Hunter wrote:
...

Quoted 11 lines

By the way, I do see your point. You wish to describe the all the mappings available to all dma controllers and then set a mapping in the device tree. Where as I am simply setting a mapping and do not list all other possibilities (assuming that there some).

What is still unclear to me, is if you use this token approach how readable is the device-tree? For example, if you have a client that can use one of two dmac and for each dmac the request/channel number is different, then by using a global token how can I determine what the options available for this client are?

Quoted 8 lines

Take your example ...

mmc1: mmc at 13002000 {
? ? ? ? ...
? ? ? ? dma_tx = <891> ? //some platform-wide unique value ? ? ? ? dma_rx = <927> ? //some platform-wide unique value ? ? ? ? ...
};

I believe those properties (in the DMA client) should be completely omitted; there's no need for them.

Also, we definitely should not be using "some platform-wide unique value", but rather the phandle of the DMA client, plus some client-defined client channel ID. ...

(oh, and - rather than _ is idiomatic for DT property names)

Quoted 8 lines

DMAC's Node:-

pdma2: pdma at 10800000 {
? ? ? ? ?.......
? ? ? dma_map = <891, 7>, ? ? ? // Map mmc1_tx onto i/f 7 ? ? ? ? ? ? ? ? <927, 8>, ? ? ? // Map mmc1_rx onto i/f 8 ? ? ? .......
};

So this would become:

pdma2: pdma at 10800000 {
? ? ? ?.......
? ? ? ?dma-map =
? ? ? ? ? ? ? ?... entries for channels 0.. 6
? ? ? ? ? ? ? ?<&mmc1, 0>, ? ? ? // Map mmc1_tx onto i/f 7 ? ? ? ? ? ? ? ?<&mmc1, 1>, ? ? ? // Map mmc1_rx onto i/f 8 ? ? ? ? ? ? ? ?... ;
? ? ? ?.......
};

This (a) follows existing DT practice of using phandle + specifier, and
(b) makes it easy to know exactly what clients you're talking about, since all you need to do is search for the label "mmc1" throughout the DT.

Quoted 8 lines

But now I have another dmac which has the following options ...

pdma1: pdma at 10000000 {
? ? ? ? ?.......
? ? ? dma_map = <598, 2>, ? ? ? // Map mmc1_tx onto i/f 2 ? ? ? ? ? ? ? ? <230, 3>, ? ? ? // Map mmc1_rx onto i/f 3 ? ? ? .......
};

Which would become something very similar:

pdma1: pdma at 10000000 {
? ? ? ?.......
? ? ? ?dma-map =
? ? ? ? ? ? ? ?... entries for channels 0.. 1
? ? ? ? ? ? ? ?<&mmc1, 0>, ? ? ? // Map mmc1_tx onto i/f 2 ? ? ? ? ? ? ? ?<&mmc1, 1>, ? ? ? // Map mmc1_rx onto i/f 3 ? ? ? ? ? ? ? ?... ;
? ? ? ?.......
};

Note that dma-map here is describing the list of DMA requests that the DMA controller knows about. As far as the binding goes, these are irrelevant to channels; only the driver for the DMAC knows whether it needs to use a specific channel ID to service a particular DMA request signal, or not.

OK, my guts feel people might be interested in what's cooking on my side. I started with the binding text first and then would write code based upon that approach.

The following might be tweaked as I look deeper into client and DMAC drivers while deciding upon what the helper functions should be optimally...

-------------------- 8< --------------------

Generic binding to provide a way to provide the client-channel map and other dmac specific parameters to the dma controller driver

DMA Model:-
Only the most common characteristics of a dma setup are assumed
in this binding.
Client: Some h/w controller that could request a DMA controller in the system to perform data transfer on its behalf. Example SPI, MMC, I2S etc.
DMAC: A DMA Controller instance. Example, PL330, PL08X, SDMA etc.

The assumed model of the DMAC, in this binding, has P peripheral
interfaces (P request signals) that could request a data transfer and C physical channels that actually do the data transfers, hence, at most C out of P peripherals could be served by the DMAC at any point of time. Usually C := P, but not always. Usually, any of the physical channels could be employed by the DMAC driver to serve any client.
The DMAC driver identifies a client by its i/f number, 'peri_id'
on the given DMAC. For example, TX for SPI has 7th while RX_TX (half-duplex) for MMC has 10th peripheral interface (request-signal) on a given DMAC. Usually, any of the physical channels could be employed by the DMAC driver to serve a client.

* DMA Controller

Required property:

- #map-cells: Number of elements in each chan-map entry. At least 3 elements are required by this binding.

- chan-map: List of entries that specify clients' 'peri_id'. and also possibly DMAC specific per-client data. The first element of each entry being the client's phandle. The second the direction of data transfer w.r.t the client 1 for RX, 2 for TX and 3 for RX|TX. The third the 'peri_id' of the client's request signal on the DMAC.

Optional properties:

- #dma-peri-ifs: P, usually the DMAC driver would simply assume the number of entries in the 'chan-map' property to be the effective number of peripheral request interfaces on the DMAC. If specified, it should be at least the number of entries in the 'chan-map' property.

- #dma-channels: C, if specified, it should neither be more than the value of 'dma-peri-ifs' nor equal to zero.
If unspecified, it is assumed to be equal to the value of
'dma-peri-ifs', i.e, C := P

- #private-data: Peculiarities of the DMAC setup, not taken into account by this generic model. The decoding of it would be private to the DMAC's driver. For ex, some DMAC drivers for dmaengine would specify dma_cap_mask_t for the DMAC, if they don't need to specify it on a per-client basis (i.e, via 4th element of a 'chan-map' entry).

Example:

	dma-controller at 0 {
		compatible = "foo,dmac-xxx";
		#private-data = <0x80808080>;
		#dma-peri-ifs = <32>;
		#dma-channels = <8>;
		#map-cells = <3>;
		chan-map =
			<&i2s1 1 2>,     /* peri_id of I2S's RX is 2 */
			<&i2s1 2 3>,     /* peri_id of I2S's TX is 3 */
			<&mmc1 3 5>,  /* peri_id of MMC's RX_TX is 5 */
			<&spi1 1 6>,
			<&spi1 2 8>,
			...;
	};

* Client Controller

Required property: None.
The client's DT node doesn't need any DMA specifier. Typically it would only comment about the data transfer direction associated with each of its request signal. Preferably also mentioned in the binding.

Optional property: None.

On 16 May 2012 21:45, Stephen Warren <[email protected]> wrote:

Generic binding to provide a way to provide the client-channel map and other dmac specific parameters to the dma controller driver

DMA Model:-
Only the most common characteristics of a dma setup are assumed
in this binding.
Client: Some h/w controller that could request a DMA controller in the system to perform data transfer on its behalf. Example SPI, MMC, I2S etc.
DMAC: A DMA Controller instance. Example, PL330, PL08X, SDMA etc.

The assumed model of the DMAC, in this binding, has P peripheral
interfaces (P request signals) that could request a data transfer and C physical channels that actually do the data transfers, hence, at most C out of P peripherals could be served by the DMAC at any point of time. Usually C := P, but not always. Usually, any of the physical channels could be employed by the DMAC driver to serve any client.
The DMAC driver identifies a client by its i/f number, 'peri_id'
on the given DMAC. For example, TX for SPI has 7th while RX_TX (half-duplex) for MMC has 10th peripheral interface (request-signal) on a given DMAC. Usually, any of the physical channels could be employed by the DMAC driver to serve a client.

Quoted 12 lines

Generic binding to provide a way to provide the client-channel map and other dmac specific parameters to the dma controller driver

DMA Model:-
? Only the most common characteristics of a dma setup are assumed in this binding.
Client: Some h/w controller that could request a DMA controller in the system to perform data transfer on its behalf. Example SPI, MMC, I2S etc.
DMAC: A DMA Controller instance. Example, PL330, PL08X, SDMA etc.

Quoted 4 lines

?The assumed model of the DMAC, in this binding, has P peripheral interfaces (P request signals) that could request a data transfer and C physical channels that actually do the data transfers, hence, at most C out of P peripherals could be served by the DMAC at any

Quoted 8 lines

point of time. Usually C := P, but not always. Usually, any of the physical channels could be employed by the DMAC driver to serve any client.
?The DMAC driver identifies a client by its i/f number, 'peri_id' on the given DMAC. For example, TX for SPI has 7th while RX_TX (half-duplex) for MMC has 10th peripheral interface (request-signal) on a given DMAC. Usually, any of the physical channels could be employed by the DMAC driver to serve a client.

I'm still anything but convinced by this model. Basically it's the exact opposite of what we do for every other subsystem (irq, pinctrl, regulator, gpio, ...), where the device using some infrastructure contains the information about who provides it, whereas here you move all the information into the device that provides the functionality, and have no handle in the device using it by which the driver can identify it.

I believe that it can work and that it solves the problem you are faced with at minimal complexity, but you put the burden of this complexity on everybody who does not have this issue, and make the general binding confusing and harder to read.

It also adds much more data to the device tree (in source and binary form) because you need to describe every device using a dma controller and have a label to reference it.

More importantly, you make it very hard to add devices in a board file to a dma controller that already has descriptions for some channels, because you cannot easily extend the chan-map unless you rewrite all of it.

We really need something simpler than this for the common case. I have already made suggestions for how to make it still possible to cover the corner case of multiple dma engines connected to the same slave, which would keep the complexity local to those devices that actually need it.

On 17 May 2012 01:12, Arnd Bergmann <[email protected]> wrote:

Quoted 4 lines

More importantly, you make it very hard to add devices in a board file to a dma controller that already has descriptions for some channels, because you cannot easily extend the chan-map unless you rewrite all of it.

I am not sure I understand the point.

Now, the DMA node for an on-SoC DMAC would be in soc.dtsi. Typically, the DMAC is connected to many on-SoC devices, and hence soc.dtsi would need to specify the routing information for all those devices to avoid duplicating it in every board.dts. Now, if you have some DMA requests that go off-SoC, the board.dts file might want to add to the routing table to indicate what clients connect to those DMA requests. However, there's no way in the device tree compiler right now to add to a property; you can only completely replace it. That would entail duplicating the entire routing information from soc.dtsi into each board.dts that wanted to add to it - a bad situation. Splitting the routing information into chunks in the client nodes avoids this issue entirely.

Quoted 8 lines · Arnd Bergmann

I'm still anything but convinced by this model. Basically it's the exact opposite of what we do for every other subsystem (irq, pinctrl, regulator, gpio, ...), where the device using some infrastructure contains the information about who provides it, whereas here you move all the information into the device that provides the functionality, and have no handle in the device using it by which the driver can identify it.

The idea was that a client shouldn't need to know/tell which dma controller serves it or which peripheral interface of a dma controller.
I think in future third-party device IPs, like ARM's Primecell, are only gonna get more common so it makes even more sense.

Quoted 5 lines

I believe that it can work and that it solves the problem you are faced with at minimal complexity, but you put the burden of this complexity on everybody who does not have this issue, and make the general binding confusing and harder to read.

I am sorry if I gave you the wrong impression, but I didn't intend to just scratch my personal itch. I truly believed I and Stephen reached a generic solution i.e, as much as it could be.

Quoted 14 lines · Jassi Brar

On 16 May 2012 21:45, Stephen Warren <[email protected]> wrote:

Generic binding to provide a way to provide the client-channel map and other dmac specific parameters to the dma controller driver

DMA Model:-
Only the most common characteristics of a dma setup are assumed
in this binding.
Client: Some h/w controller that could request a DMA controller in the system to perform data transfer on its behalf. Example SPI, MMC, I2S etc.
DMAC: A DMA Controller instance. Example, PL330, PL08X, SDMA etc.

Quoted 12 lines

The assumed model of the DMAC, in this binding, has P peripheral
interfaces (P request signals) that could request a data transfer and C physical channels that actually do the data transfers, hence, at most C out of P peripherals could be served by the DMAC at any point of time. Usually C := P, but not always. Usually, any of the physical channels could be employed by the DMAC driver to serve any client.
The DMAC driver identifies a client by its i/f number, 'peri_id'
on the given DMAC. For example, TX for SPI has 7th while RX_TX (half-duplex) for MMC has 10th peripheral interface (request-signal) on a given DMAC. Usually, any of the physical channels could be employed by the DMAC driver to serve a client.

I'm still anything but convinced by this model. Basically it's the exact opposite of what we do for every other subsystem (irq, pinctrl, regulator, gpio, ...), where the device using some infrastructure contains the information about who provides it, whereas here you move all the information into the device that provides the functionality, and have no handle in the device using it by which the driver can identify it.

Quoted 25 lines

Quoted 12 lines

Generic binding to provide a way to provide the client-channel map and other dmac specific parameters to the dma controller driver

DMA Model:-
? Only the most common characteristics of a dma setup are assumed in this binding.
Client: Some h/w controller that could request a DMA controller in the system to perform data transfer on its behalf. Example SPI, MMC, I2S etc.
DMAC: A DMA Controller instance. Example, PL330, PL08X, SDMA etc.

Quoted 12 lines

?The assumed model of the DMAC, in this binding, has P peripheral interfaces (P request signals) that could request a data transfer and C physical channels that actually do the data transfers, hence, at most C out of P peripherals could be served by the DMAC at any point of time. Usually C := P, but not always. Usually, any of the physical channels could be employed by the DMAC driver to serve any client.
?The DMAC driver identifies a client by its i/f number, 'peri_id' on the given DMAC. For example, TX for SPI has 7th while RX_TX (half-duplex) for MMC has 10th peripheral interface (request-signal) on a given DMAC. Usually, any of the physical channels could be employed by the DMAC driver to serve a client.

Quoted 7 lines

I'm still anything but convinced by this model. Basically it's the exact opposite of what we do for every other subsystem (irq, pinctrl, regulator, gpio, ...), where the device using some infrastructure contains the information about who provides it, whereas here you move all the information into the device that provides the functionality, and have no handle in the device using it by which the driver can identify it.

Yes, I guess this is backwards. But, the HW is a little different too; GPIOs (and probably interrupts) don't have multiple places they could come from.

The problem is that if we did something like this in the DMA client:

dma-reqs = <&dmac1 DMAC1_DMA_REQ_6 &dmac1 DMAC1_DMA_REQ_8>;

how do we know if the client is emitting 2 DMA request signals that get routed to two different inputs on the DMA controller, or whether this is two alternatives for the same signal.

Simple case:

/* e.g. FIFO TX DMA req - 2 DMACs possible */
dma-req-0 = <&dmac1 DMAC1_DMA_REQ_6>;
/* e.g. FIFO RX DMA req 1 DMAC possible */
dma-req-1 = <&dmac1 DMAC1_DMA_REQ_8>;

Multiple DMAC case:

/* e.g. FIFO TX DMA req - 2 DMACs possible */ dma-req-0 = <&dmac1 DMAC1_DMA_REQ_6 &dmac2 DMA_2_DMA_REQ_8>; /* e.g. FIFO RX DMA req 1 DMAC possible */ dma-req-1 = <&dmac1 DMAC1_DMA_REQ_8>;

Then, when the DMA client calls the standard API to "get DMA channel for my outbound DMA request "n", the core code will kasprintf("dma-req-%d", n); to generate the property name. That's how pinctrl works.

Does that seem better?

Quoted 14 lines · Stephen Warren

Simple case:

/* e.g. FIFO TX DMA req - 2 DMACs possible */
dma-req-0 = <&dmac1 DMAC1_DMA_REQ_6>;
/* e.g. FIFO RX DMA req 1 DMAC possible */
dma-req-1 = <&dmac1 DMAC1_DMA_REQ_8>;

Multiple DMAC case:

/* e.g. FIFO TX DMA req - 2 DMACs possible */ dma-req-0 = <&dmac1 DMAC1_DMA_REQ_6 &dmac2 DMA_2_DMA_REQ_8>; /* e.g. FIFO RX DMA req 1 DMAC possible */ dma-req-1 = <&dmac1 DMAC1_DMA_REQ_8>;

Quoted 5 lines

Then, when the DMA client calls the standard API to "get DMA channel for my outbound DMA request "n", the core code will kasprintf("dma-req-%d", n); to generate the property name. That's how pinctrl works.

Does that seem better?

Yes, that is one way that I suggested at an earlier point. After some discussion, I would use a different syntax for these though, to the exact same effect, writing it as

	dmas = <&dmac1 DMAC1_DMA_REQ_6>,  <&dmac2 DMA_2_DMA_REQ_8>,  <&dmac1 DMAC1_DMA_REQ_8>;
	dma-names = "tx", "tx", "rx";

The driver can still request the dma line by name "tx" and the subsystem would pick one out of those with the same name.

For the majority of cases, we would only need a single dma request line

and could leave out the dma-names property, so when you don't ask for a specific name, you just get any dma line out of the dmas array.

Arnd

Quoted 4 lines

The driver can still request the dma line by name "tx" and the subsystem would pick one out of those with the same name.

For the majority of cases, we would only need a single dma request line

Hmm. Many devices have multiple different FIFOs, and hence multiple DMA request signals (e.g. Tegra I2S has separate RX and TX FIFO, Tegra S/PDIF has 2 FIFOs in each direction). That would require the driver to always use get_by_name() to differentiate between 2/4 options for the same DMA req or 2/4 different DMA requests. Most other bindings allow use of get_by_id() or get_by_name() interchangeably.

Quoted 11 lines · Stephen Warren

Quoted 4 lines

The driver can still request the dma line by name "tx" and the subsystem would pick one out of those with the same name.

For the majority of cases, we would only need a single dma request line

Hmm. Many devices have multiple different FIFOs, and hence multiple DMA request signals (e.g. Tegra I2S has separate RX and TX FIFO, Tegra S/PDIF has 2 FIFOs in each direction). That would require the driver to always use get_by_name() to differentiate between 2/4 options for the same DMA req or 2/4 different DMA requests. Most other bindings allow use of get_by_id() or get_by_name() interchangeably.

Ok, good point. So we could make the common case that they are numbered but not named and require all drivers to use named properties when there is the possibility that the device might be connected to multiple controllers, but that seems tricky because a driver can start being used on a platform that has only one controller and have no dma-name property in the device tree but then get used on a different soc that actually has multiple controllers.

So if we do that, we might want to make the dma-names property mandatory for every device, and document what the names are.

Another option would be to encode the direction in the property in a generic way and provide an API that lets you ask specifically for a read, write or readwrite channel out of the ones that are available, rather than assuming there is only one kind. Consequently, any device that uses more than one read channel or more than one write channel would still have to use names to identify them.

Quoted 2 lines · Arnd Bergmann

So if we do that, we might want to make the dma-names property mandatory for every device, and document what the names are.

We could do that, but one more proposal: Add the client's ID/index into the dmas property, so:

simple 1 req:

dmas = <0 &dmac1 xxx>;

simple 2 req:

dmas = <0 &dmac1 xxx 1 &dmac1 yyy>;

multiple dmacs:

dmas = <0 &dmac1 xxx 0 &dmac2 zzz 1 &dmac1 yyy>;

(i.e. dmas = [client_dma_req_id phandle dma-specifier]*)

(where 0==TX_FIFO, 1=RX_FIFO for example, but could also have 2=TX_FIFO_B, 3=RX_FIFO_B, ...)

Then dma-names would map name to ID, but you'd still need to search all through the dmas property to find the ID match.

Quoted 6 lines

Another option would be to encode the direction in the property in a generic way and provide an API that lets you ask specifically for a read, write or readwrite channel out of the ones that are available, rather than assuming there is only one kind. Consequently, any device that uses more than one read channel or more than one write channel would still have to use names to identify them.

I'm not sure that just direction cuts it; Tegra's SPDIF has 2 TX DMAs ("PCM" data and control data) and same for RX. The format above is roughly the same as what you proposed, but with an explicit ID rather than direction in the dmas property.

Quoted 20 lines · Stephen Warren

Quoted 2 lines · Arnd Bergmann

So if we do that, we might want to make the dma-names property mandatory for every device, and document what the names are.

We could do that, but one more proposal: Add the client's ID/index into the dmas property, so:

simple 1 req:

dmas = <0 &dmac1 xxx>;

simple 2 req:

dmas = <0 &dmac1 xxx 1 &dmac1 yyy>;

multiple dmacs:

dmas = <0 &dmac1 xxx 0 &dmac2 zzz 1 &dmac1 yyy>;

Quoted 7 lines

(i.e. dmas = [client_dma_req_id phandle dma-specifier]*)

(where 0==TX_FIFO, 1=RX_FIFO for example, but could also have 2=TX_FIFO_B, 3=RX_FIFO_B, ...)

Then dma-names would map name to ID, but you'd still need to search all through the dmas property to find the ID match.

Again, this would probably work, but it adds complexity and inconsistency for the common case, as nothing else requires these. I think it's much better than putting the information into the dma controller node, but not ideal.

Quoted 11 lines

Quoted 6 lines

Another option would be to encode the direction in the property in a generic way and provide an API that lets you ask specifically for a read, write or readwrite channel out of the ones that are available, rather than assuming there is only one kind. Consequently, any device that uses more than one read channel or more than one write channel would still have to use names to identify them.

I'm not sure that just direction cuts it; Tegra's SPDIF has 2 TX DMAs ("PCM" data and control data) and same for RX. The format above is roughly the same as what you proposed, but with an explicit ID rather than direction in the dmas property.

The point with the direction was that it covers most cases and makes them rather simple, while for the rare case where you need more than two channels, you just use the otherwise optional named interface rather than the numbered one. My feeling is that this also makes a lot of sense at the driver API level: most dirvers just ask for the read and write channels using a very simple interface, and those drivers that have more than two will want to name them anyway.

Quoted 8 lines

The point with the direction was that it covers most cases and makes them rather simple, while for the rare case where you need more than two channels, you just use the otherwise optional named interface rather than the numbered one. My feeling is that this also makes a lot of sense at the driver API level: most dirvers just ask for the read and write channels using a very simple interface, and those drivers that have more than two will want to name them anyway.

How are you thinking of representing the direction in DT - as part of the DMA request specifier, so in a DMAC-specific way?

If so, that seems a little odd; you have to request a DMA channel for "TX", but then end up having the common code check all the entries in the dmas property since it can't know which are TX, and then have the wrong ones almost accidentally fail, since the DMAC will then determine that it can't support TX on the RX DMA request IDs.

Quoted 9 lines · Stephen Warren

Quoted 8 lines

The point with the direction was that it covers most cases and makes them rather simple, while for the rare case where you need more than two channels, you just use the otherwise optional named interface rather than the numbered one. My feeling is that this also makes a lot of sense at the driver API level: most dirvers just ask for the read and write channels using a very simple interface, and those drivers that have more than two will want to name them anyway.

Quoted 8 lines

How are you thinking of representing the direction in DT - as part of the DMA request specifier, so in a DMAC-specific way?

If so, that seems a little odd; you have to request a DMA channel for "TX", but then end up having the common code check all the entries in the dmas property since it can't know which are TX, and then have the wrong ones almost accidentally fail, since the DMAC will then determine that it can't support TX on the RX DMA request IDs.

I think the direction must be encoded in a way that does not depend on the binding for the specific controller. There are two ways I can see how we might do it:

1. have separate property names for tx and rx channels, and probably one for combined rx/tx channels

2. define the second cell in each channel specifier to be the direction in a predefined format, followed by the other (controller specific) attributes, if any.

Quoted 23 lines · Arnd Bergmann

Quoted 9 lines · Stephen Warren

Quoted 8 lines

The point with the direction was that it covers most cases and makes them rather simple, while for the rare case where you need more than two channels, you just use the otherwise optional named interface rather than the numbered one. My feeling is that this also makes a lot of sense at the driver API level: most dirvers just ask for the read and write channels using a very simple interface, and those drivers that have more than two will want to name them anyway.

Quoted 8 lines

How are you thinking of representing the direction in DT - as part of the DMA request specifier, so in a DMAC-specific way?

If so, that seems a little odd; you have to request a DMA channel for "TX", but then end up having the common code check all the entries in the dmas property since it can't know which are TX, and then have the wrong ones almost accidentally fail, since the DMAC will then determine that it can't support TX on the RX DMA request IDs.

I think the direction must be encoded in a way that does not depend on the binding for the specific controller. There are two ways I can see how we might do it:

Quoted 6 lines

1. have separate property names for tx and rx channels, and probably one for combined rx/tx channels

2. define the second cell in each channel specifier to be the direction in a predefined format, followed by the other (controller specific) attributes, if any.

In this option, lets say bit 0 is 0==TX, 1==RX (or perhaps 2 bits if combined TX/RX is needed)

Then, we could reserve say bits 7:1 as client DMA request ID.

And then have bits 31:8 as DMAC specific data.

Wouldn't that allow us to have our cake and eat it?

Quoted 20 lines · Stephen Warren

Quoted 12 lines · Arnd Bergmann

Quoted 6 lines · Stephen Warren

If so, that seems a little odd; you have to request a DMA channel for "TX", but then end up having the common code check all the entries in the dmas property since it can't know which are TX, and then have the wrong ones almost accidentally fail, since the DMAC will then determine that it can't support TX on the RX DMA request IDs.

I think the direction must be encoded in a way that does not depend on the binding for the specific controller. There are two ways I can see how we might do it:

Quoted 6 lines

1. have separate property names for tx and rx channels, and probably one for combined rx/tx channels

2. define the second cell in each channel specifier to be the direction in a predefined format, followed by the other (controller specific) attributes, if any.

Quoted 8 lines

In this option, lets say bit 0 is 0==TX, 1==RX (or perhaps 2 bits if combined TX/RX is needed)

Then, we could reserve say bits 7:1 as client DMA request ID.

And then have bits 31:8 as DMAC specific data.

Wouldn't that allow us to have our cake and eat it?

Sorry for not responding sooner.

It seems to me we were pretty close on alignment. In fact, I was quite happy with Steven's 2nd to last proposal of ...

simple 1 req:
dmas = <0 &dmac1 xxx>;

simple 2 req:
dmas = <0 &dmac1 xxx 1 &dmac1 yyy>;

multiple dmacs:
dmas = <0 &dmac1 xxx 0 &dmac2 zzz 1 &dmac1 yyy>;

Arnd, I know that you had some concerns. However, in the above case I would expect that the 3rd parameter be optional and it can be some sort of match variable. In other words, we don't care how the 32-bits are encoded or what they represent but they would be used appropriately by the xlate function being used. So the xlate and this "match" variable would have to speak the same language.

I think that I prefer the idea of having a 3rd optional match variable than encoding the DMA request ID and match data together in 1 32-bit variable. However, not a big deal either way.

Quoted 44 lines · Jon Hunter

Quoted 20 lines · Stephen Warren

Quoted 12 lines · Arnd Bergmann

Quoted 6 lines · Stephen Warren

If so, that seems a little odd; you have to request a DMA channel for "TX", but then end up having the common code check all the entries in the dmas property since it can't know which are TX, and then have the wrong ones almost accidentally fail, since the DMAC will then determine that it can't support TX on the RX DMA request IDs.

I think the direction must be encoded in a way that does not depend on the binding for the specific controller. There are two ways I can see how we might do it:

Quoted 6 lines

1. have separate property names for tx and rx channels, and probably one for combined rx/tx channels

2. define the second cell in each channel specifier to be the direction in a predefined format, followed by the other (controller specific) attributes, if any.

Quoted 8 lines

In this option, lets say bit 0 is 0==TX, 1==RX (or perhaps 2 bits if combined TX/RX is needed)

Then, we could reserve say bits 7:1 as client DMA request ID.

And then have bits 31:8 as DMAC specific data.

Wouldn't that allow us to have our cake and eat it?

Sorry for not responding sooner.

It seems to me we were pretty close on alignment. In fact, I was quite happy with Steven's 2nd to last proposal of ...

simple 1 req:
dmas = <0 &dmac1 xxx>;

simple 2 req:
dmas = <0 &dmac1 xxx 1 &dmac1 yyy>;

multiple dmacs:
dmas = <0 &dmac1 xxx 0 &dmac2 zzz 1 &dmac1 yyy>;

Quoted 6 lines

Arnd, I know that you had some concerns. However, in the above case I would expect that the 3rd parameter be optional and it can be some sort of match variable. In other words, we don't care how the 32-bits are encoded or what they represent but they would be used appropriately by the xlate function being used. So the xlate and this "match" variable would have to speak the same language.

I would at least put the &dmac part first to be consistent with other bindings that refer to some node. That controller should then be able to specify the number of additional cells used to describe the dma request. We can define that the first cell after the controller phandle is always the same thing, e.g. the direction (in/out/inout) or a client-specific ID or a combination of that with other predefined bits that are not dependent on dma controller specifics.

As I said previously, I think just encoding the direction but not the client specific ID (meaning we would have to disambiguate the more complex cases that Stephen mentioned using a dma-names property) would be the best because it keeps the common case simple, but I could live with other things going in there too.

Quoted 5 lines · Arnd Bergmann

As I said previously, I think just encoding the direction but not the client specific ID (meaning we would have to disambiguate the more complex cases that Stephen mentioned using a dma-names property) would be the best because it keeps the common case simple, but I could live with other things going in there too.

Ok, so you are saying that there are some DMA controllers where there is no channel/request ID and only a direction is needed? So an even simpler case than what I had imagined.

Quoted 5 lines

As I said previously, I think just encoding the direction but not the client specific ID (meaning we would have to disambiguate the more complex cases that Stephen mentioned using a dma-names property) would be the best because it keeps the common case simple, but I could live with other things going in there too.

Ok, so you are saying that there are some DMA controllers where there is no channel/request ID and only a direction is needed? So an even simpler case than what I had imagined.

So in that case, I don't see why the first cell after the phandle could not be an index which could be either a direction or request-id and then the next cell after that be a secondary match variable.

So simple case with just a index (either req-id or direction) ...

dmas = <&dmac0 index>

More complex case ...

dmas = <&dmac0 index match>

For example, for OMAP index = req-id and match = direction ...

dmas = <&dmac0 req-id direction>

Or am I way off the page?

Quoted 6 lines · Jon Hunter

Quoted 5 lines

As I said previously, I think just encoding the direction but not the client specific ID (meaning we would have to disambiguate the more complex cases that Stephen mentioned using a dma-names property) would be the best because it keeps the common case simple, but I could live with other things going in there too.

Quoted 3 lines

Ok, so you are saying that there are some DMA controllers where there is no channel/request ID and only a direction is needed? So an even simpler case than what I had imagined.

No, that was not the case I was thinking about.

Quoted 16 lines

So in that case, I don't see why the first cell after the phandle could not be an index which could be either a direction or request-id and then the next cell after that be a secondary match variable.

So simple case with just a index (either req-id or direction) ...

dmas = <&dmac0 index>

More complex case ...

dmas = <&dmac0 index match>

For example, for OMAP index = req-id and match = direction ...

dmas = <&dmac0 req-id direction>

Quoted 1 line

Or am I way off the page?

The intention was instead to remove the need for the /index/ in those cases, because having a client-specific index in here makes it inconsistent with other similar bindings (reg, interrupts, gpios, ...) that people are familiar with. They use an implicit index by counting the fields in the respective properties.

The existing method we have for avoiding index numbers is to use named fields, like

	dmas = <&dmac0 matchA>, <dmac1 matchB>, <dmac2 matchC>;
	dma-names = "rx", "rx", "tx";

This is similar to how we use named interrupt and mmio resources, but it requires that we always request the dma lines by name, which is slightly more complex than we might want it to be.

Because the vast majority of cases just use a single channel, or one channel per direction, my idea was to encode the direction in the dmas property, which lets us request a dma channel by direction from the driver side, without explicitly encoding the name.

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);

Quoted 17 lines · Arnd Bergmann

Quoted 16 lines · Jon Hunter

So in that case, I don't see why the first cell after the phandle could not be an index which could be either a direction or request-id and then the next cell after that be a secondary match variable.

So simple case with just a index (either req-id or direction) ...

dmas = <&dmac0 index>

More complex case ...

dmas = <&dmac0 index match>

For example, for OMAP index = req-id and match = direction ...

dmas = <&dmac0 req-id direction>

Quoted 7 lines

Quoted 1 line

Or am I way off the page?

The intention was instead to remove the need for the /index/ in those cases, because having a client-specific index in here makes it inconsistent with other similar bindings (reg, interrupts, gpios, ...) that people are familiar with. They use an implicit index by counting the fields in the respective properties.

So maybe "index" was not the right term to use here. What I meant was that this is really the req/chan-id associated with this device. It is not an arbitrary index that in turn gets resolved into the req-id. So in other words, just like gpio where you have the gpio number, here you have the dma req-id.

Quoted 6 lines

The existing method we have for avoiding index numbers is to use named fields, like

	dmas = <&dmac0 matchA>, <dmac1 matchB>, <dmac2 matchC>;
	dma-names = "rx", "rx", "tx";

Quoted 3 lines

This is similar to how we use named interrupt and mmio resources, but it requires that we always request the dma lines by name, which is slightly more complex than we might want it to be.

Ok, but how do you get the req-id from the above binding? Doesn't it need to be stored there somewhere even for the most simplest case? Or are you saying that in this case you are just returning a name and the dma driver resolves that into a req-id?

Quoted 4 lines

Because the vast majority of cases just use a single channel, or one channel per direction, my idea was to encode the direction in the dmas property, which lets us request a dma channel by direction from the driver side, without explicitly encoding the name.

Yes, thats fine and so the direction is really the match criteria in this case.

Quoted 5 lines

This would let us handle the following cases very easily:

1. one read-write channel

	dmas = <&dmac 0x3 match>;

Where 0x3 is the req-id? Just to confirm ;-)

Why not have match after the phandle to be consistent with the simple example using name fields above?

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);

Hi Arnd,

On 06/14/2012 06:48 AM, Arnd Bergmann wrote:

[snip]

Quoted 40 lines

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);

Quoted 8 lines

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

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?

Quoted 64 lines

Hi Arnd,

On 06/14/2012 06:48 AM, Arnd Bergmann wrote:

[snip]

Quoted 40 lines

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);

Quoted 8 lines

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

Hi Russell,

Quoted 8 lines · Russell King - ARM Linux

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 2 lines

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.

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);

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???

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

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.

Quoted 60 lines · Jon Hunter

Quoted 59 lines

Hi Arnd,

On 06/14/2012 06:48 AM, Arnd Bergmann wrote:

[snip]

Quoted 40 lines

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);

Quoted 8 lines

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?

Quoted 5 lines

Quoted 5 lines

I am been prototyping something, but wanted to make sure I am completely aligned on this :-)

Cheers
Jon

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 8 lines · Jon Hunter

Hi Russell,

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:

Quoted 16 lines

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.

Quoted 2 lines

	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.

Quoted 13 lines

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.

Quoted 8 lines

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)

Quoted 5 lines

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.

Quoted 10 lines

	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.

Quoted 5 lines

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 13 lines

Quoted 13 lines · Jon Hunter

Quoted 12 lines

Quoted 11 lines

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);

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

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 :-)

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 16 lines

Quoted 16 lines

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.

Quoted 4 lines

Quoted 2 lines

	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 14 lines

Quoted 13 lines

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???

Quoted 2 lines

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 14 lines

Quoted 9 lines

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)

Quoted 5 lines

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

Quoted 5 lines

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 10 lines

Quoted 10 lines

	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.

Quoted 7 lines

Quoted 5 lines

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 13 lines

Quoted 13 lines

Quoted 13 lines · Jon Hunter

Quoted 12 lines

Quoted 11 lines

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);

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

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

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.

Quoted 6 lines

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.

Quoted 4 lines

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 20 lines

Quoted 19 lines

Quoted 19 lines

Quoted 19 lines · Jon Hunter

Quoted 18 lines

Quoted 11 lines

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 :-)

Quoted 1 line

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?

Quoted 6 lines · Vinod Koul

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?

Quoted 5 lines

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?

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.

Quoted 8 lines · Arnd Bergmann

Quoted 6 lines · Vinod Koul

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.

Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 13 lines

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.

Quoted 5 lines

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.

Quoted 9 lines · Vinod Koul

Quoted 8 lines · Arnd Bergmann

Quoted 6 lines · Vinod Koul

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.

Quoted 6 lines

Quoted 2 lines

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.

Quoted 4 lines

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 12 lines

Quoted 5 lines

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.

Quoted 3 lines

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 19 lines

Quoted 14 lines

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.

Quoted 5 lines

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.

Quoted 4 lines

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.

Quoted 7 lines · Arnd Bergmann

Quoted 1 line · Vinod Koul

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 5 lines

Quoted 4 lines

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.

Quoted 7 lines

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 15 lines

Quoted 11 lines

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 5 lines

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.

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 22 lines

Quoted 16 lines

Quoted 12 lines

Quoted 5 lines

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.

Quoted 3 lines

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.

Quoted 5 lines

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 3 lines

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 4 lines

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?

On Wed, 2012-06-27 at 15:20 +0000, Arnd Bergmann wrote:

Quoted 4 lines

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?

Quoted 4 lines

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?

Quoted 25 lines · Vinod Koul

Quoted 4 lines

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.

Quoted 1 line

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@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@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@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@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 :)

Quoted 6 lines

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 8 lines

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 9 lines · Vinod Koul

Quoted 6 lines

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".

Quoted 9 lines

Quoted 8 lines

4. A dma controller requiring complex configuration:

dma: dmaengine@48000000 {
compatible = "foo,foo-sdma"
reg = <0x48000000 0x1000>;
interrupts = <4>;
#dma-cells = <6>; /* phandle, flag, request, channel,
input-width, output-width */

Quoted 4 lines

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

Quoted 22 lines · Arnd Bergmann

Quoted 9 lines · Vinod Koul

Quoted 6 lines

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.

Quoted 4 lines

=> 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"

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.

Hi Vinod,

Quoted 21 lines

Quoted 21 lines

Quoted 9 lines

Quoted 6 lines

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.

Quoted 6 lines

Quoted 4 lines

=> 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.

Quoted 4 lines

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.

Quoted 9 lines

Quoted 4 lines

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?

Quoted 1 line

2. By a filter parameter (flags)

Even with direction same problem can arise

Quoted 1 line

3. By name and a filter parameter

Additionally we need to say which channel, or making dmaengine already aware will help here.

Quoted 9 lines · Vinod Koul

Quoted 9 lines

Quoted 4 lines

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

Quoted 2 lines

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 3 lines

Quoted 2 lines

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 4 lines

Quoted 2 lines

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

Quoted 12 lines · Arnd Bergmann

Quoted 11 lines · Vinod Koul

Quoted 9 lines

Quoted 4 lines

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?

Quoted 5 lines

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 18 lines · Vinod Koul

Quoted 17 lines · Arnd Bergmann

Quoted 11 lines · Vinod Koul

Quoted 9 lines

Quoted 4 lines

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 3 lines

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.

Quoted 29 lines · Jon Hunter

Hi Vinod,

Quoted 21 lines

Quoted 21 lines

Quoted 9 lines

Quoted 6 lines

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.

Quoted 6 lines

Quoted 4 lines

=> 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.

Quoted 6 lines

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 1 line

Quoted 1 line

So yes I can see that a channel itself could be configured to

support a

Quoted 1 line

Quoted 1 line

given direction, but when we ask for a channel via

dma_request_channel()

Quoted 1 line

Quoted 1 line

we are going to get a channel that matches the criteria we pass

using

Quoted 1 line

Quoted 1 line

the filter parameter. So here the thinking was that "flags" is a

filter

Quoted 1 line

Quoted 1 line

parameter that the user could specify and one example being

direction

Quoted 1 line

Quoted 1 line

but it could be something else too.

Quoted 1 line

Yes that can be done, but I am leaning towards clients not have to

do

Quoted 2 lines

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()?

Quoted 2 lines

Maybe for slave case we need to create dma_request_slave_channel()

which

Quoted 1 line

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.

Quoted 11 lines · Jon Hunter

Quoted 1 line

Quoted 1 line

So yes I can see that a channel itself could be configured to

support a

Quoted 1 line

Quoted 1 line

given direction, but when we ask for a channel via

dma_request_channel()

Quoted 1 line

Quoted 1 line

we are going to get a channel that matches the criteria we pass

using

Quoted 1 line

Quoted 1 line

the filter parameter. So here the thinking was that "flags" is a

filter

Quoted 1 line

Quoted 1 line

parameter that the user could specify and one example being

direction

Quoted 1 line

Quoted 1 line

but it could be something else too.

Quoted 8 lines

Quoted 1 line

Yes that can be done, but I am leaning towards clients not have to

do

Quoted 2 lines

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 4 lines

Quoted 2 lines

Maybe for slave case we need to create dma_request_slave_channel()

which

Quoted 1 line

has additional arguments for dmaengine to do the filtering.

Yup

Quoted 4 lines

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.

Hi Vinod,

Quoted 27 lines · Vinod Koul

Quoted 11 lines · Jon Hunter

Quoted 1 line

Quoted 1 line

So yes I can see that a channel itself could be configured to

support a

Quoted 1 line

Quoted 1 line

given direction, but when we ask for a channel via

dma_request_channel()

Quoted 1 line

Quoted 1 line

we are going to get a channel that matches the criteria we pass

using

Quoted 1 line

Quoted 1 line

the filter parameter. So here the thinking was that "flags" is a

filter

Quoted 1 line

Quoted 1 line

parameter that the user could specify and one example being

direction

Quoted 1 line

Quoted 1 line

but it could be something else too.

Quoted 8 lines

Quoted 1 line

Yes that can be done, but I am leaning towards clients not have to

do

Quoted 2 lines

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 4 lines

Quoted 2 lines

Maybe for slave case we need to create dma_request_slave_channel()

which

Quoted 1 line

has additional arguments for dmaengine to do the filtering.

Quoted 8 lines

Yup

Quoted 4 lines

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.

Quoted 8 lines

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 8 lines · Vinod Koul

Quoted 8 lines

4. A dma controller requiring complex configuration:

dma: dmaengine@48000000 {
compatible = "foo,foo-sdma"
reg = <0x48000000 0x1000>;
interrupts = <4>;
#dma-cells = <6>; /* phandle, flag, request, channel,
input-width, output-width */

Quoted 4 lines

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 ?

Quoted 26 lines · Arnd Bergmann

Quoted 25 lines · Vinod Koul

Quoted 4 lines

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.

Quoted 6 lines

Quoted 1 line

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 40 lines

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).

Quoted 6 lines

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 4 lines

- 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.

Quoted 40 lines

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";

Quoted 8 lines

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 4 lines

#dma-channels = <32>;
#dma-requests = <127>;
};

Quoted 13 lines

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 :)

--
~Vinod

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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:

Quoted 1 line

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.

Quoted 5 lines · Stephen Warren

Quoted 1 line

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.

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 8 lines

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";

Quoted 4 lines

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 1 line

* DMA client

...

Quoted 1 line

Examples:

...

Quoted 1 line

3. A device with three channels, one of which has two alternatives:

s/three/four/ s/one of which/both of which/

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);

On Mon, 25 Jun 2012, Arnd Bergmann wrote:

[snip]

Quoted 9 lines

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):

...
<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?

Quoted 36 lines · Guennadi Liakhovetski

On Mon, 25 Jun 2012, Arnd Bergmann wrote:

[snip]

Quoted 9 lines

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):

...
<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 */
	}