Patchwork [U-Boot,2/5] ARM: vexpress: create A9 specific board config

login
register
mail settings
Submitter Andre Przywara
Date April 3, 2013, 1:44 p.m.
Message ID <1364996676-25361-3-git-send-email-andre.przywara@linaro.org>
Download mbox | patch
Permalink /patch/233468/
State Superseded
Delegated to: Albert ARIBAUD
Headers show

Comments

Andre Przywara - April 3, 2013, 1:44 p.m.
From: Ryan Harkin <ryan.harkin@linaro.org>

This patch creates a new config for the A9 quad core tile that includes the
generic config for the Versatile Express platform.

Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
 MAINTAINERS                       |  2 +-
 boards.cfg                        |  2 +-
 include/configs/vexpress_ca9x4.h  | 34 ++++++++++++++++++++++++++++++++++
 include/configs/vexpress_common.h |  1 -
 4 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100644 include/configs/vexpress_ca9x4.h
Albert ARIBAUD - April 4, 2013, 10:09 a.m.
Hi Andre,

On Wed,  3 Apr 2013 15:44:33 +0200, Andre Przywara
<andre.przywara@linaro.org> wrote:

> From: Ryan Harkin <ryan.harkin@linaro.org>
> 
> This patch creates a new config for the A9 quad core tile that includes the
> generic config for the Versatile Express platform.
> 
> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
>  MAINTAINERS                       |  2 +-
>  boards.cfg                        |  2 +-
>  include/configs/vexpress_ca9x4.h  | 34 ++++++++++++++++++++++++++++++++++
>  include/configs/vexpress_common.h |  1 -
>  4 files changed, 36 insertions(+), 3 deletions(-)
>  create mode 100644 include/configs/vexpress_ca9x4.h

Wait, so patch 1/5 renames ca9x4_ct_vxp as vexpress_common, then patch
2/5 renames vexpress_common as vexpress_ca9x4? If so then please make
this a single patch without the intermediary/temporary step.

Amicalement,
Andre Przywara - April 4, 2013, 10:31 a.m.
On 04/04/2013 12:09 PM, Albert ARIBAUD wrote:
> Hi Andre,
>
> On Wed,  3 Apr 2013 15:44:33 +0200, Andre Przywara
> <andre.przywara@linaro.org> wrote:
>
>> From: Ryan Harkin <ryan.harkin@linaro.org>
>>
>> This patch creates a new config for the A9 quad core tile that includes the
>> generic config for the Versatile Express platform.
>>
>> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>>   MAINTAINERS                       |  2 +-
>>   boards.cfg                        |  2 +-
>>   include/configs/vexpress_ca9x4.h  | 34 ++++++++++++++++++++++++++++++++++
>>   include/configs/vexpress_common.h |  1 -
>>   4 files changed, 36 insertions(+), 3 deletions(-)
>>   create mode 100644 include/configs/vexpress_ca9x4.h
>
> Wait, so patch 1/5 renames ca9x4_ct_vxp as vexpress_common, then patch
> 2/5 renames vexpress_common as vexpress_ca9x4? If so then please make
> this a single patch without the intermediary/temporary step.

But that would not mark the actual file copy (vexpress_common.h is 
almost the same as vexpress_ca9x4.h) as such, right?
So you would end up with a completely new file (_common.h) and
a file with almost all content deleted (_ca9x4.h). The fact that the 
code just moved wouldn't be obvious.
That would be extra pity with the nice -M move features in the previous 
patch.

However I have no problems with merging these two, if you insist.

Regards,
Andre.
Albert ARIBAUD - April 4, 2013, 11:13 a.m.
Hi Andre,

On Thu, 04 Apr 2013 12:31:08 +0200, Andre Przywara
<andre.przywara@linaro.org> wrote:

> On 04/04/2013 12:09 PM, Albert ARIBAUD wrote:
> > Hi Andre,
> >
> > On Wed,  3 Apr 2013 15:44:33 +0200, Andre Przywara
> > <andre.przywara@linaro.org> wrote:
> >
> >> From: Ryan Harkin <ryan.harkin@linaro.org>
> >>
> >> This patch creates a new config for the A9 quad core tile that includes the
> >> generic config for the Versatile Express platform.
> >>
> >> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
> >> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> >> ---
> >>   MAINTAINERS                       |  2 +-
> >>   boards.cfg                        |  2 +-
> >>   include/configs/vexpress_ca9x4.h  | 34 ++++++++++++++++++++++++++++++++++
> >>   include/configs/vexpress_common.h |  1 -
> >>   4 files changed, 36 insertions(+), 3 deletions(-)
> >>   create mode 100644 include/configs/vexpress_ca9x4.h
> >
> > Wait, so patch 1/5 renames ca9x4_ct_vxp as vexpress_common, then patch
> > 2/5 renames vexpress_common as vexpress_ca9x4? If so then please make
> > this a single patch without the intermediary/temporary step.
> 
> But that would not mark the actual file copy (vexpress_common.h is 
> almost the same as vexpress_ca9x4.h) as such, right?
> So you would end up with a completely new file (_common.h) and
> a file with almost all content deleted (_ca9x4.h). The fact that the 
> code just moved wouldn't be obvious.
> That would be extra pity with the nice -M move features in the previous 
> patch.

Try -C too, for copies, and possibly --find-copies-harder.

I don't see how eliminating the intermediate target naming would
prevent git from detecting moves and copies; and it will simplify the
changes undergone by non-header files such as baords.cfg and
MAINTAINERS.

> However I have no problems with merging these two, if you insist.

I am fine with either of the two following solutions:

a) If patch 1/5 commonalizes vexpress code, then it should not rename
any target, and patch 2/5 should do the renaming.

or

b) if patch 1/5 commonalizes and renames the target, it should give it
its final name and patch 2 should be merged in.

> Regards,
> Andre.

Amicalement,

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index b4d9a35..761c36c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -960,7 +960,7 @@  Hugo Villeneuve <hugo.villeneuve@lyrtech.com>
 
 Matt Waddel <matt.waddel@linaro.org>
 
-	vexpress_common	ARM ARMV7 (Quad Core)
+	vexpress_ca9x4	ARM ARMV7 (Quad Core)
 
 Otavio Salvador <otavio@ossystems.com.br>
 
diff --git a/boards.cfg b/boards.cfg
index 3c971d4..908e3bc 100644
--- a/boards.cfg
+++ b/boards.cfg
@@ -232,7 +232,7 @@  versatilepb                  arm         arm926ejs   versatile           armltd
 versatileqemu                arm         arm926ejs   versatile           armltd         versatile   versatile:ARCH_VERSATILE_QEMU,ARCH_VERSATILE_PB
 integratorap_cm946es         arm         arm946es    integrator          armltd         -               integratorap:CM946ES
 integratorcp_cm946es         arm         arm946es    integrator          armltd         -               integratorcp:CM946ES
-vexpress_common              arm         armv7       vexpress            armltd
+vexpress_ca9x4               arm         armv7       vexpress            armltd
 am335x_evm                   arm         armv7       am335x              ti             am33xx      am335x_evm:SERIAL1,CONS_INDEX=1
 am335x_evm_spiboot           arm         armv7       am335x              ti             am33xx      am335x_evm:SERIAL1,CONS_INDEX=1,SPI_BOOT
 am335x_evm_uart1             arm         armv7       am335x              ti             am33xx      am335x_evm:SERIAL2,CONS_INDEX=2
diff --git a/include/configs/vexpress_ca9x4.h b/include/configs/vexpress_ca9x4.h
new file mode 100644
index 0000000..c3b6986
--- /dev/null
+++ b/include/configs/vexpress_ca9x4.h
@@ -0,0 +1,34 @@ 
+/*
+ * (C) Copyright 2011 Linaro
+ * Ryan Harkin, <ryan.harkin@linaro.org>
+ *
+ * Configuration for Versatile Express. Parts were derived from other ARM
+ *   configurations.
+ *
+ * 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
+ */
+
+#ifndef __VEXPRESS_CA9X4_H
+#define __VEXPRESS_CA9X4_H
+
+#define CONFIG_VEXPRESS_ORIGINAL_MEMORY_MAP
+#include "vexpress_common.h"
+#define CONFIG_BOOTP_VCI_STRING     "U-boot.armv7.vexpress_ca9x4"
+
+#endif /* VEXPRESS_CA9X4_H */
diff --git a/include/configs/vexpress_common.h b/include/configs/vexpress_common.h
index a7cd1d4..9a3431e 100644
--- a/include/configs/vexpress_common.h
+++ b/include/configs/vexpress_common.h
@@ -97,7 +97,6 @@ 
 #define CONFIG_BOOTP_HOSTNAME
 #define CONFIG_BOOTP_PXE
 #define CONFIG_BOOTP_PXE_CLIENTARCH	0x100
-#define CONFIG_BOOTP_VCI_STRING		"U-boot.armv7.ca9x4_ct_vxp"
 
 /* Miscellaneous configurable options */
 #undef	CONFIG_SYS_CLKS_IN_HZ