On Friday 28 February 2014 06:14 PM, Arnd Bergmann wrote:

On Friday 28 February 2014 17:56:40 Santosh Shilimkar wrote:

quoted

The Packet DMA driver sets up the dma channels and flows for the
QMSS(Queue Manager SubSystem) who triggers the actual data movements
across clients using destination queues. Every client modules like
NETCP(Network Coprocessor), SRIO(Serial Rapid IO) and CRYPTO
Engines has its own instance of packet dma hardware. QMSS has also
an internal packet DMA module which is used as an infrastructure
DMA with zero copy.

Patch adds DMAEngine driver for Keystone Packet DMA hardware.
The specifics on the device tree bindings for Packet DMA can be
found in:
        Documentation/devicetree/bindings/dma/keystone-pktdma.txt

The driver implements the configuration functions using standard DMAEngine
apis. The data movement is managed by QMSS device driver.

The description of this sounds a bit like the x-gene queue manager/tag
manager, rather than a traditional DMA engine. For that driver, we
decided to use the mailbox subsystem instead of the DMA subsystem.

Can you explain what kind of data is actually getting transferred
in by your hardware here? You say the DMA is performed by the QMSS,
so do you use the pktdma driver to drive the data transfers of the
QMSS or do you just use it for flow control by passing short (a few
bytes) messages and also have to program the pktdma?

I don't know what x-gene stuff is, but the keystone packet dma
is hardware DMA controller(per client subystem) which has all
the traditional DMA controller properties. The packet DMA needs
to be setup just like any slave dma configuration. The actual
data transfers are triggered by operations on Queue Manager
SubSystem.
 
I just posted the QMSS driver as well which explains how the system
work. 

quoted

+
+bool dma_keystone_filter_fn(struct dma_chan *chan, void *param)
+{
+       if (chan->device->dev->driver == &keystone_dma_driver.driver) {
+               struct keystone_dma_chan *kdma_chan = from_achan(chan);
+               unsigned chan_id = *(u32 *)param & KEYSTONE_DMA_CHAN_ID_MASK;
+               unsigned chan_type = *(u32 *)param >> KEYSTONE_DMA_TYPE_SHIFT;
+
+               if (chan_type == KEYSTONE_DMA_TX_CHAN_TYPE &&
+                   kdma_chan->direction == DMA_MEM_TO_DEV)
+                       return chan_id  == kdma_chan->channel;
+
+               if (chan_type == KEYSTONE_DMA_RX_FLOW_TYPE &&
+                   kdma_chan->direction == DMA_DEV_TO_MEM)
+                       return chan_id  == kdma_chan->flow;
+       }
+       return false;
+}
+EXPORT_SYMBOL_GPL(dma_keystone_filter_fn);

The filter function should be removed here and replaced with a simpler
custom xlate function calling dma_get_slave_chan() on the matching
channel.

Can you please expand your comment please ? I though the filter function
is the one should implement for specific channel allocations.

quoted

diff --git a/include/linux/keystone-pktdma.h b/include/linux/keystone-pktdma.h
new file mode 100644
index 0000000..e6a66c8
--- /dev/null
+++ b/include/linux/keystone-pktdma.h

@@ -0,0 +1,168 @@

A DMA engine driver should not have a public API. Please move all the
contents of the two header files into the driver itself to ensure none
of this is visible to slave drivers. If it turns out that you use
declarations from this header in slave drivers at the moment, please
explain what they are so we can discuss alternative solutions.

Yes the slave drivers make use of these. The dt-binding header is created
because some the macro's are shared between DTS and code.

keystone-pktdma.h carriers information about the packet dma config
which is done by slave drivers. It also contains the dma descriptor
format and its associate macro's used by slave drivers. Since
the hardware perspective, the descriptor represent the packet
format dma expects and hence its left in the dma header header.

Regards,
Santosh