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

[Stefano Babic]

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.

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

> @@ -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).

> --- 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.

> 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.

> 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 ?

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
=====================================================================