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

[Grant Likely]

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.

> Signed-off-by: Piotr Ziecik <kosmo at semihalf.com>
> Signed-off-by: Wolfgang Denk <wd at denx.de>
> Signed-off-by: Anatolij Gustschin <agust at denx.de>
> Cc: Dan Williams <dan.j.williams at intel.com>
> Cc: Grant Likely <grant.likely at secretlab.ca>
> Cc: John Rigby <jcrigby at gmail.com>
> ---
>  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

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

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

> +
> +static struct of_device_id mpc_dma_match[] = {
> +       { .compatible = "fsl,mpc5121-dma", },
> +       {},
> +};
> +
> +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()

> +       .driver         = {
> +               .name   = DRV_NAME,
> +               .owner  = THIS_MODULE,
> +       },
> +};
> +
> +static int __init mpc_dma_init(void)
> +{
> +       return of_register_platform_driver(&mpc_dma_driver);
> +}
> +module_init(mpc_dma_init);
> +
> +static void __exit mpc_dma_exit(void)
> +{
> +       of_unregister_platform_driver(&mpc_dma_driver);
> +}
> +module_exit(mpc_dma_exit);
> +
> +/* MODULE API */

Meaningless comment.

> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Piotr Ziecik <kosmo at semihalf.com>");

Thanks,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.