From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Babic Date: Mon, 14 Nov 2011 09:34:00 +0100 Subject: [U-Boot] [PATCH 1/6] i.mx: i.mx5: Move some files to imx-common folder In-Reply-To: <1321094190-8108-2-git-send-email-jason.hui@linaro.org> References: <1321094190-8108-1-git-send-email-jason.hui@linaro.org> <1321094190-8108-2-git-send-email-jason.hui@linaro.org> Message-ID: <4EC0D278.4020408@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > --- 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. > +######################################################################### > 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. > +++ 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. 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 =====================================================================