[PATCH 07/11] dma: Add MPC512x DMA driver

[Anatolij Gustschin]

On Thu, 21 Jan 2010 10:22:27 -0700
Grant Likely <grant.likely at secretlab.ca> wrote:

> On Tue, Jan 19, 2010 at 1:24 PM, Anatolij Gustschin <agust at denx.de> wrote:
> > From: Piotr Ziecik <kosmo at semihalf.com>
> >
> > Adds initial version of MPC512x DMA driver.
> > Only memory to memory transfers are currenly supported.
> 
> Comments below on brief review.  I've not looked at the code in-depth.

> > ...
> >  drivers/dma/Kconfig       |    7 +
> >  drivers/dma/Makefile      |    1 +
> >  drivers/dma/mpc512x_dma.c |  636 +++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/dma/mpc512x_dma.h |  192 ++++++++++++++
> 
> Unless the stuff in the .h file is used by other .c files, it really
> belongs in mpc512x_dma.c

Ok, will be moved to .c file in next version.

> 
> > +static int __init mpc_dma_probe(struct of_device *op,
> > +                                       const struct of_device_id *match)
> 
> __devinit
> 
> > +static void __exit mpc_dma_remove(struct of_device *op)
> 
> __devexit

Ok.

> > +{
> > +       struct device *dev = &op->dev;
> > +       struct mpc_dma *mdma = dev_get_drvdata(dev);
> > +
> > +       devm_free_irq(dev, mdma->irq, mdma);
> > +}
> 
> No unregistration of the dma device?  No unmapping?  No kfree()?

I will add unregistration of the dma device. Unmapping and
freeing should be done automatically on driver detach, mapping
and allocation is done by devm_ioremap() and devm_kzalloc().

> > +static struct of_platform_driver mpc_dma_driver = {
> > +       .match_table    = mpc_dma_match,
> > +       .probe          = mpc_dma_probe,
> > +       .remove         = __exit_p(mpc_dma_remove),
> 
> __devexit_p()

Ok.

> > +module_exit(mpc_dma_exit);
> > +
> > +/* MODULE API */
> 
> Meaningless comment.

Ok, will remove it.

Thanks,
Anatolij