Re: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.

Hi, Dan,

Does I have followed your new API? :-)

[..]

quoted

quoted

+static struct dma_chan

*of_find_dma_chan_by_phandle(phandle phandle)

quoted

+{
+       struct device_node *np;
+       struct dma_chan *chan;
+       struct fsl_dma_device *fdev;
+
+       np = of_find_node_by_phandle(phandle);
+       if (!np || !of_device_is_compatible(np->parent, "fsl,dma"))
+               return NULL;
+
+       fdev =

dev_get_drvdata(&of_find_device_by_node(np->parent)->dev);

quoted

+
+       list_for_each_entry(chan, &fdev->common.channels,

device_node)

quoted

+               if

(to_of_device(to_fsl_chan(chan)->chan_dev)->node == np)

quoted

+                       return chan;
+       return NULL;
+}
+EXPORT_SYMBOL(of_find_dma_chan_by_phandle);

This routine implies that there is a piece of code somewhere that
wants to select which channels it can use.  A similar effect can be
achieved by registering a dma_client with the dmaengine interface
('dma_async_client_register').  Then when the client code makes a call
to 'dma_async_client_chan_request' it receives a 'dma_event_callback'
for each channel in the system.  It will also be asynchronously
notified of channels entering and leaving the system.  The goal is to
share a common infrastructure for channel management.

It's speacial codes for our processors. Some device need the speacial DMA channel, such as must be DMA channel 0. So I add these codes. Or, is it possible to add a API for the special DMA channel getting?

Timur had mentioned that this is for device tree support, but your
comment makes me think that this is for client support.  This sounds
like a case where you can define a dma_client to find a specific
channel, something like:
http://marc.info/?l=linux-kernel&m=118417229619156&w=2

quoted

quoted

+
+static int __devinit of_fsl_dma_probe(struct of_device *dev,
+                       const struct of_device_id *match)
+{
+       int err;
+       unsigned int irq;
+       struct fsl_dma_device *fdev;
+
+       fdev = kzalloc(sizeof(struct fsl_dma_device), GFP_KERNEL);
+       if (!fdev) {
+               dev_err(&dev->dev, "No enough memory for 'priv'\n");
+               err = -ENOMEM;
+               goto err;
+       }
+       fdev->dev = &dev->dev;
+       INIT_LIST_HEAD(&fdev->common.channels);
+
+       /* get DMA controller register base */
+       err = of_address_to_resource(dev->node, 0, &fdev->reg);
+       if (err) {
+               dev_err(&dev->dev, "Can't get %s property 'reg'\n",
+                               dev->node->full_name);
+               goto err;
+       }
+
+       dev_info(&dev->dev, "Probe the Freescale DMA driver for %s "
+                       "controller at 0x%08x...\n",
+                       match->compatible, fdev->reg.start);
+       fdev->reg_base = ioremap(fdev->reg.start, fdev->reg.end
+                                               -

fdev->reg.start + 1);

quoted

+
+       dma_cap_set(DMA_MEMCPY, fdev->common.cap_mask);
+       fdev->common.device_alloc_chan_resources =

fsl_dma_alloc_chan_resources;

quoted

+       fdev->common.device_free_chan_resources =

fsl_dma_free_chan_resources;

quoted

+       fdev->common.device_prep_dma_memcpy = fsl_dma_prep_memcpy;
+       fdev->common.device_is_tx_complete = fsl_dma_is_complete;
+       fdev->common.device_issue_pending =

fsl_dma_memcpy_issue_pending;

quoted

+       fdev->common.device_dependency_added =

fsl_dma_dependency_added;

quoted

+       fdev->common.dev = &dev->dev;
+

If this driver adds:

dma_cap_set(DMA_INTERRUPT, fdev->common.cap_mask);
fdev->common.device_prep_dma_interrupt = fsl_dma_prep_interrupt;

It will be able to take advantage of interrupt triggered callbacks in
async_tx.  Without these changes async_tx will poll for the completion
of each transaction.

The new API have lacking documents :) I'll make some study here.

Yes, I will address that...

Thanks!
- zw

Regards,
Dan