On Mon, Nov 14, 2011 at 4:34 PM, Stefano Babic <[email protected]> wrote: > On 11/12/2011 11:36 AM, Jason Liu wrote: >> In order to support the coming MX6 platform and to reducde >> the duplicated code, we had better move some common files >> to the imx-common folder for sharing. >> >> Signed-off-by: Jason Liu <[email protected]> >> --- > > Hi jason, > >> Makefile | 7 ++ >> arch/arm/cpu/armv7/imx-common/Makefile | 47 ++++++++++ >> arch/arm/cpu/armv7/imx-common/cpu_info.c | 108 >> ++++++++++++++++++++++++ >> arch/arm/cpu/armv7/{mx5 => imx-common}/speed.c | 0 >> arch/arm/cpu/armv7/{mx5 => imx-common}/timer.c | 17 ++-- >> arch/arm/cpu/armv7/mx5/Makefile | 2 +- >> arch/arm/cpu/armv7/mx5/soc.c | 77 ----------------- >> 7 files changed, 172 insertions(+), 86 deletions(-) > > I agree completely to add a common directory to share code between MX5 > and MX6. However, your patch mixes new code with code moved from MX5. > This is at least not coherent with the commit message, where you say you > are only moving files. I understand what you did, I think it is enough > to extend the commit message to better explain it.
Thanks for review. I will extend the commit message to reflect other changes. > > >> +######################################################################### >> diff --git a/arch/arm/cpu/armv7/imx-common/cpu_info.c >> b/arch/arm/cpu/armv7/imx-common/cpu_info.c > > The name of the file is quite confusing - I am expecting to see only > function to retrieve information about the revision, but there are other > common functions. It is better to use another name. Yes, agree. what about cpu.c? if you dislike, could you do me a favor to name it. :) > >> +++ b/arch/arm/cpu/armv7/imx-common/cpu_info.c >> @@ -0,0 +1,108 @@ >> +/* >> + * (C) Copyright 2007 >> + * Sascha Hauer, Pengutronix >> + * >> + * (C) Copyright 2009 Freescale Semiconductor, Inc. >> + * >> + * See file CREDITS for list of people who contributed to this >> + * project. >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation; either version 2 of >> + * the License, or (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, >> + * MA 02111-1307 USA >> + */ >> + > > Ok - all these functions are taken from the original soc.c. > >> diff --git a/arch/arm/cpu/armv7/mx5/speed.c >> b/arch/arm/cpu/armv7/imx-common/speed.c >> similarity index 100% >> rename from arch/arm/cpu/armv7/mx5/speed.c >> rename to arch/arm/cpu/armv7/imx-common/speed.c >> diff --git a/arch/arm/cpu/armv7/mx5/timer.c >> b/arch/arm/cpu/armv7/imx-common/timer.c >> old mode 100644 >> new mode 100755 >> similarity index 84% >> rename from arch/arm/cpu/armv7/mx5/timer.c >> rename to arch/arm/cpu/armv7/imx-common/timer.c >> index 2544b08..98e9f4a >> --- a/arch/arm/cpu/armv7/mx5/timer.c >> +++ b/arch/arm/cpu/armv7/imx-common/timer.c > > You mix here two things - you move the files and you change it adding > new features. Split into two patches. In fact, I did not do any function change. I just do the followings two changes: - fix the checkpatch warnings with the original timer.c file in mx5 folder. - change the CONFIG_SYS_MX5_CLK32 to CLK_32KHZ I can add the changes to the commit message. Do I still need split into two patches? Jason Liu > > 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: [email protected] > ===================================================================== > _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
