On Thu, 21 Jan 2010 10:22:27 -0700 Grant Likely <grant.lik...@secretlab.ca> wrote:
> On Tue, Jan 19, 2010 at 1:24 PM, Anatolij Gustschin <ag...@denx.de> wrote: > > From: Piotr Ziecik <ko...@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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev