On Wed, May 6, 2009 at 3:07 PM, Grant Likely <grant.lik...@secretlab.ca>wrote:
> On Wed, May 6, 2009 at 2:15 PM, Wolfgang Denk <w...@denx.de> wrote: > > From: Piotr Ziecik <ko...@semihalf.com> > > > > This patch adds initial version of MPC512x DMA driver. > > Only memory to memory transfers are currenly supported. > > > > Signed-off-by: Piotr Ziecik <ko...@semihalf.com> > > Signed-off-by: Wolfgang Denk <w...@denx.de> > > Cc: Grant Likely <grant.lik...@secretlab.ca> > > Cc: John Rigby <jcri...@gmail.com> > > Don't have time to review this in detail right now, but three quick > comments: > > > drivers/dma/mpc512x_dma.c | 642 > ++++++++++++++++++++++++++ > > drivers/dma/mpc512x_dma.h | 192 ++++++++ > > It looks to me like these two files should be merged. > > > diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts > b/arch/powerpc/boot/dts/mpc5121ads.dts > > index c2d9de9..e7f0e09 100644 > > --- a/arch/powerpc/boot/dts/mpc5121ads.dts > > +++ b/arch/powerpc/boot/dts/mpc5121ads.dts > > @@ -373,7 +373,7 @@ > > }; > > > > d...@14000 { > > - compatible = "fsl,mpc5121-dma2"; > > + compatible = "fsl,mpc512x-dma"; > > Nack. Compatible values should not use wildcards. Be specific. And > be specific about what it is compatible to if another part implements > the same device. The internal name for the dma module was dma2 that is where the orginal name came from. There is a dma2 in some 8xxx but last I looked it is not at all the same. I would vote for fsl,mpc5121-dma. > > > > reg = <0x14000 0x1800>; > > interrupts = <65 0x8>; > > interrupt-parent = < &ipic >; > > diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c > b/arch/powerpc/platforms/512x/mpc512x_shared.c > > index b776e45..135fd6b 100644 > > --- a/arch/powerpc/platforms/512x/mpc512x_shared.c > > +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c > > @@ -95,6 +95,7 @@ void __init mpc512x_init_i2c(void) > > static struct of_device_id __initdata of_bus_ids[] = { > > { .compatible = "fsl,mpc5121-immr", }, > > { .compatible = "fsl,mpc5121-localbus", }, > > + { .compatible = "fsl,mpc5121-dma", }, > > This doesn't look right either. Shouldn't the dma device hang off the > IMMR node? Yes dma is part of IMMR. > > > g. > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. >
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev