Re: Re: [PATCH 11/12] mpc5121: Added MPC512x DMA driver.
Hi John, Did you use DMA driver in our FEC driver before? Best Regards, Hongjun Chen 2009-05-20 发件人： Piotr Zięcik 发送时间： 2009-05-19 16:05:05 收件人： Hongjun Chen 抄送： Wolfgang Denk; linuxppc-dev@ozlabs.org 主题： Re: [PATCH 11/12] mpc5121: Added MPC512x DMA driver. Tuesday 19 May 2009 03:37:47 Hongjun Chen wrote: > Since you have reuse most part of our BSP, why should you reinvent wheel > for MPC512x DMA? We have one ready DMA driver, which has been used by AC97, > ATA, VIU etc. > Answer is simple. The old one does not fit Linux DMA API. New one uses existing infrastructure which makes DMA engine aviable for existing consumers in Linux kernel. For example network stack. Support for I/O <- > memory transfers will be added when more consumers arrive. -- Best Regards. Piotr Zięcik ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 11/12] mpc5121: Added MPC512x DMA driver.
Tuesday 19 May 2009 03:37:47 Hongjun Chen wrote: > Since you have reuse most part of our BSP, why should you reinvent wheel > for MPC512x DMA? We have one ready DMA driver, which has been used by AC97, > ATA, VIU etc. > Answer is simple. The old one does not fit Linux DMA API. New one uses existing infrastructure which makes DMA engine aviable for existing consumers in Linux kernel. For example network stack. Support for I/O <-> memory transfers will be added when more consumers arrive. -- Best Regards. Piotr Zięcik ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 11/12] mpc5121: Added MPC512x DMA driver.
Since you have reuse most part of our BSP, why should you reinvent wheel for MPC512x DMA? We have one ready DMA driver, which has been used by AC97, ATA, VIU etc. Hongjun Chen 2009-05-19 发件人： Wolfgang Denk 发送时间： 2009-05-07 04:15:18 收件人： linuxppc-dev@ozlabs.org 抄送： Piotr Ziecik; Wolfgang Denk 主题： [PATCH 11/12] mpc5121: Added MPC512x DMA driver. From: Piotr Ziecik This patch adds initial version of MPC512x DMA driver. Only memory to memory transfers are currenly supported. Signed-off-by: Piotr Ziecik Signed-off-by: Wolfgang Denk Cc: Grant Likely Cc: John Rigby --- arch/powerpc/boot/dts/mpc5121ads.dts |2 +- arch/powerpc/platforms/512x/mpc512x_shared.c |1 + drivers/dma/Kconfig |7 + drivers/dma/Makefile |1 + drivers/dma/mpc512x_dma.c| 642 ++ drivers/dma/mpc512x_dma.h| 192 6 files changed, 844 insertions(+), 1 deletions(-) create mode 100644 drivers/dma/mpc512x_dma.c create mode 100644 drivers/dma/mpc512x_dma.h ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 11/12] mpc5121: Added MPC512x DMA driver.
On Wed, May 6, 2009 at 3:07 PM, Grant Likely wrote:
> On Wed, May 6, 2009 at 2:15 PM, Wolfgang Denk wrote:
> > From: Piotr Ziecik
> >
> > This patch adds initial version of MPC512x DMA driver.
> > Only memory to memory transfers are currenly supported.
> >
> > Signed-off-by: Piotr Ziecik
> > Signed-off-by: Wolfgang Denk
> > Cc: Grant Likely
> > Cc: John Rigby
>
> 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
Re: [PATCH 11/12] mpc5121: Added MPC512x DMA driver.
On Wed, May 6, 2009 at 2:15 PM, Wolfgang Denk wrote:
> From: Piotr Ziecik
>
> This patch adds initial version of MPC512x DMA driver.
> Only memory to memory transfers are currenly supported.
>
> Signed-off-by: Piotr Ziecik
> Signed-off-by: Wolfgang Denk
> Cc: Grant Likely
> Cc: John Rigby
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 @@
> };
>
> ...@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.
> 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?
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
