[PATCHv2] cpufreq for freescale mx51

[s.hauer@pengutronix.de (Sascha Hauer)]

On Mon, Oct 18, 2010 at 05:08:14PM +0800, Yong Shen wrote:
> Hi Sascha,
> 
> 
> On Mon, Oct 18, 2010 at 4:31 PM, Sascha Hauer <s.hauer@pengutronix.de>wrote:
> 
> > Hi Yong,
> >
> > On Mon, Oct 18, 2010 at 01:43:43PM +0800, Yong Shen wrote:
> > > Hi Sascha,
> > >
> > > Thanks for your thorough review. I have two feedbacks to your commends.
> > > Sorry for delayed response, cause I had a hard time due to my computer
> > crash
> > > and data loss.
> > >
> > > > diff --git a/arch/arm/mach-mx5/cpu.c b/arch/arm/mach-mx5/cpu.c
> > > > > index 2d37785..83add9c 100644
> > > > > --- a/arch/arm/mach-mx5/cpu.c
> > > > > +++ b/arch/arm/mach-mx5/cpu.c
> > > > > @@ -22,6 +22,8 @@ static int cpu_silicon_rev = -1;
> > > > >
> > > > >  #define SI_REV 0x48
> > > > >
> > > > > +struct cpu_wp *(*get_cpu_wp)(int *wp);
> > > > > +
> > > >
> > > > This is not needed.
> > > >
> > > This is needed, otherwise it does not pass compile.
> >
> > This hunk is the only change to arch/arm/mach-mx5/cpu.c and get_cpu_wp
> > is introduced with this patch, so how can this break compilation?
> > Also, you should move this to a header file. Otherwise you risk of
> > having multiple (and possibly different) declarations of the same
> > function which can lead to hard to find bugs.
> >
> 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.

Of course, you are right, my bad. Isn't this function common to al
i.MXs? In this case it should be somewhere in plat-mxc.
Anyway, it seems very odd to me to provide a global function pointer
which gets overwritten by the boards. For me it is more logical to
provide a mxc_register_workpoints() function along with a
mxc_for_each_workpoint() iterator.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |