[U-Boot] [PATCH 1/1] IMX: rename mx51 to mx5

[Liu Hui-R64343]

Hi, Stefano,

> -----Original Message-----
> From: Stefano Babic [mailto:sbabic at denx.de]
> Sent: Friday, October 15, 2010 7:40 PM
> To: Liu Hui-R64343
> Cc: sbabic at denx.de; u-boot at lists.denx.de
> Subject: Re: [U-Boot][PATCH 1/1] IMX: rename mx51 to mx5
> 
> On 10/15/2010 04:22 AM, Jason Liu wrote:
> > Rename mx51 to mx5 in order to support more mx51
> > like-style SOCs such as MX53 and the followings.
> >
> > Signed-off-by: Jason Liu <r64343 at freescale.com>
> 
> Hi Jason,
> 
> a little feedback. This patch is well-formed and I do not see the
> corruption problems as with the former one.
> 
> However, the patch does not apply:
> 
> Applying: IMX: rename mx51 to mx5
> error: patch failed: arch/arm/include/asm/arch-mx51/sys_proto.h:24
> error: arch/arm/include/asm/arch-mx51/sys_proto.h: patch does not
apply
> error: patch failed: boards.cfg:46
> error: boards.cfg: patch does not apply
> Patch failed at 0001 IMX: rename mx51 to mx5
> 
> Have you applied the patch on the current u-boot.git tree ? It seems
you
> have to to rebase your patch.

Yes, correct.

> 
> Please increment the version of your patch to allow everybody to track
> easier the changes. Something like [PATCH V2 1/1] makes the job.

Yes, I will do it.

> 
> > @@ -2,7 +2,7 @@
> >   * (C) Copyright 2007
> >   * Sascha Hauer, Pengutronix
> >   *
> > - * (C) Copyright 2009 Freescale Semiconductor, Inc.
> > + * (C) Copyright 2009-2010 Freescale Semiconductor, Inc.
> 
> I let someone with more legal experience as me to judge if this change
> is allowed or not. Normally, a new Copyright is added in case there is
> some important improvement that are not covered by the original file.
In
> this case, only a define was changed (CONFIG_MX51_HCLK_FREQ ->
> CONFIG_HCLK_FREQ).

Make sense, I will modify it.

> 
> > --- a/arch/arm/cpu/armv7/mx51/soc.c
> > +++ b/arch/arm/cpu/armv7/mx5/soc.c
> > @@ -2,7 +2,7 @@
> >   * (C) Copyright 2007
> >   * Sascha Hauer, Pengutronix
> >   *
> > - * (C) Copyright 2009 Freescale Semiconductor, Inc.
> > + * (C) Copyright 2009-2010 Freescale Semiconductor, Inc.
> >   *
> >   * See file CREDITS for list of people who contributed to this
> >   * project.
> > @@ -35,26 +35,25 @@
> >
> >  u32 get_cpu_rev(void)
> >  {
> > -	int reg;
> > -	int system_rev;
> > +	int system_rev = CONFIG_CPU_TYPE << 8;
> 
> CONFIG_CPU_TYPE is a new CONFIG_ switch, that should be not needed.
See
> my comments later.

Good point.

> 
> > diff --git a/boards.cfg b/boards.cfg
> > index 9226424..e144281 100644
> > --- a/boards.cfg
> > +++ b/boards.cfg
> > @@ -46,7 +46,7 @@ pm9263		arm	arm926ejs	-
ronetix
> 		at91
> >  jadecpu		arm	arm926ejs	jadecpu		syteco
> 	mb86r0x
> >  suen3		arm	arm926ejs	km_arm		keymile
> 	kirkwood
> >  rd6281a		arm	arm926ejs	-		Marvell
> 	kirkwood
> > -mx51evk		arm	armv7		mx51evk
freescale	mx51
> > +mx51evk		arm	armv7		mx51evk
freescale	mx5
> 
> It makes sense to change other boards with MX51 inside this patch and
> not with a separate patch. So add changes for the other board, too.

Agree.
 
> 
> > diff --git a/include/configs/mx51evk.h b/include/configs/mx51evk.h
> > old mode 100644
> > new mode 100755
> > index 86a4731..363af3d
> > --- a/include/configs/mx51evk.h
> > +++ b/include/configs/mx51evk.h
> > @@ -1,7 +1,7 @@
> >  /*
> >   * Copyright (C) 2007, Guennadi Liakhovetski <lg at denx.de>
> >   *
> > - * (C) Copyright 2009 Freescale Semiconductor, Inc.
> > + * (C) Copyright 2009-2010 Freescale Semiconductor, Inc.
> >   *
> >   * Configuration settings for the MX51EVK Board
> >   *
> > @@ -28,10 +28,11 @@
> >   /* High Level Configuration Options */
> >
> >  #define CONFIG_MX51	/* in a mx51 */
> > +#define CONFIG_CPU_TYPE	51
> 
> Why do we have CONFIG_MX51 and CONFIG_CPU_TYPE ? It seems redundant. A
> board maintainer must set both and this makes no great sense. Can we
> derive CONFIG_CPU_TYPE (or its meaning) from CONFIG_MX51 when we need
?

Good point, I will modify it.

The v2 patch will be sent out for review soon. :)

BR,
Jason
> 
> Best regards,
> Stefano Babic
> 
> --
> =====================================================================
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
> =====================================================================