Hi, Stefano,
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: Friday, October 15, 2010 7:40 PM To: Liu Hui-R64343 Cc: sbabic@denx.de; u-boot@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@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
at91jadecpu 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@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@denx.de =====================================================================