<br><br><div class="gmail_quote">On Wed, May 6, 2009 at 3:07 PM, Grant Likely <span dir="ltr">&lt;<a href="mailto:grant.likely@secretlab.ca">grant.likely@secretlab.ca</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">On Wed, May 6, 2009 at 2:15 PM, Wolfgang Denk &lt;<a href="mailto:wd@denx.de">wd@denx.de</a>&gt; wrote:<br>
&gt; From: Piotr Ziecik &lt;<a href="mailto:kosmo@semihalf.com">kosmo@semihalf.com</a>&gt;<br>
&gt;<br>
&gt; This patch adds initial version of MPC512x DMA driver.<br>
&gt; Only memory to memory transfers are currenly supported.<br>
&gt;<br>
&gt; Signed-off-by: Piotr Ziecik &lt;<a href="mailto:kosmo@semihalf.com">kosmo@semihalf.com</a>&gt;<br>
&gt; Signed-off-by: Wolfgang Denk &lt;<a href="mailto:wd@denx.de">wd@denx.de</a>&gt;<br>
&gt; Cc: Grant Likely &lt;<a href="mailto:grant.likely@secretlab.ca">grant.likely@secretlab.ca</a>&gt;<br>
&gt; Cc: John Rigby &lt;<a href="mailto:jcrigby@gmail.com">jcrigby@gmail.com</a>&gt;<br>
<br>
</div>Don&#39;t have time to review this in detail right now, but three quick comments:<br>
<div class="im"><br>
&gt;  drivers/dma/mpc512x_dma.c                    |  642 ++++++++++++++++++++++++++<br>
&gt;  drivers/dma/mpc512x_dma.h                    |  192 ++++++++<br>
<br>
</div>It looks to me like these two files should be merged.<br>
<div class="im"><br>
&gt; diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts b/arch/powerpc/boot/dts/mpc5121ads.dts<br>
&gt; index c2d9de9..e7f0e09 100644<br>
&gt; --- a/arch/powerpc/boot/dts/mpc5121ads.dts<br>
&gt; +++ b/arch/powerpc/boot/dts/mpc5121ads.dts<br>
&gt; @@ -373,7 +373,7 @@<br>
&gt;                };<br>
&gt;<br>
&gt;                dma@14000 {<br>
&gt; -                       compatible = &quot;fsl,mpc5121-dma2&quot;;<br>
&gt; +                       compatible = &quot;fsl,mpc512x-dma&quot;;<br>
<br>
</div>Nack.  Compatible values should not use wildcards.  Be specific.  And<br>
be specific about what it is compatible to if another part implements<br>
the same device.</blockquote><div>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. <br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
<div class="im"><br>
&gt;                        reg = &lt;0x14000 0x1800&gt;;<br>
&gt;                        interrupts = &lt;65 0x8&gt;;<br>
&gt;                        interrupt-parent = &lt; &amp;ipic &gt;;<br>
&gt; diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c<br>
&gt; index b776e45..135fd6b 100644<br>
&gt; --- a/arch/powerpc/platforms/512x/mpc512x_shared.c<br>
&gt; +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c<br>
&gt; @@ -95,6 +95,7 @@ void __init mpc512x_init_i2c(void)<br>
&gt;  static struct of_device_id __initdata of_bus_ids[] = {<br>
&gt;        { .compatible = &quot;fsl,mpc5121-immr&quot;, },<br>
&gt;        { .compatible = &quot;fsl,mpc5121-localbus&quot;, },<br>
&gt; +       { .compatible = &quot;fsl,mpc5121-dma&quot;, },<br>
<br>
</div>This doesn&#39;t look right either.  Shouldn&#39;t the dma device hang off the<br>
IMMR node?</blockquote><div>Yes dma is part of IMMR. <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
<br>
g.<br>
<font color="#888888"><br>
--<br>
Grant Likely, B.Sc., P.Eng.<br>
Secret Lab Technologies Ltd.<br>
</font></blockquote></div><br>