Hi Sascha,<div><br><br><div class="gmail_quote">On Mon, Oct 18, 2010 at 4:31 PM, Sascha Hauer <span dir="ltr">&lt;<a href="mailto:s.hauer@pengutronix.de">s.hauer@pengutronix.de</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi Yong,<br>
<br>
On Mon, Oct 18, 2010 at 01:43:43PM +0800, Yong Shen wrote:<br>
&gt; Hi Sascha,<br>
&gt;<br>
&gt; Thanks for your thorough review. I have two feedbacks to your commends.<br>
&gt; Sorry for delayed response, cause I had a hard time due to my computer crash<br>
&gt; and data loss.<br>
<div class="im">&gt;<br>
&gt; &gt; diff --git a/arch/arm/mach-mx5/cpu.c b/arch/arm/mach-mx5/cpu.c<br>
</div>&gt; &gt; &gt; index 2d37785..83add9c 100644<br>
<div class="im">&gt; &gt; &gt; --- a/arch/arm/mach-mx5/cpu.c<br>
&gt; &gt; &gt; +++ b/arch/arm/mach-mx5/cpu.c<br>
&gt; &gt; &gt; @@ -22,6 +22,8 @@ static int cpu_silicon_rev = -1;<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt;  #define SI_REV 0x48<br>
&gt; &gt; &gt;<br>
</div>&gt; &gt; &gt; +struct cpu_wp *(*get_cpu_wp)(int *wp);<br>
&gt; &gt; &gt; +<br>
&gt; &gt;<br>
&gt; &gt; This is not needed.<br>
&gt; &gt;<br>
&gt; This is needed, otherwise it does not pass compile.<br>
<br>
This hunk is the only change to arch/arm/mach-mx5/cpu.c and get_cpu_wp<br>
is introduced with this patch, so how can this break compilation?<br>
Also, you should move this to a header file. Otherwise you risk of<br>
having multiple (and possibly different) declarations of the same<br>
function which can lead to hard to find bugs.<br></blockquote><div>IMHO,  get_cpu_wp is definition rather than a declaration and without it there will be errors in link phase. its declaration is in arch/arm/plat-mxc/include/mach/mxc.h.</div>
<div><br></div></div></div>