Patchwork [U-Boot,2/2] arm/km: fix u-boot.kwb build breakage

login
register
mail settings
Submitter Holger Brunck
Date June 14, 2011, 7:11 a.m.
Message ID <1308035513-7462-2-git-send-email-holger.brunck@keymile.com>
Download mbox | patch
Permalink /patch/100287/
State Superseded
Headers show

Comments

Holger Brunck - June 14, 2011, 7:11 a.m.
commit 010a958b
(arm/km: remove CONFIG_SYS_KWD_CONFIG from keymile-common.h)
breaks building keymile arm targets, when u-boot.kwb tries to
generate the binary with mkimage. A simple make <board> or MAKEALL
succeeded because it don't try to build the kirwood binary at the end.

Due this commit we use the CONFIG_SYS_KWD_CONFIG from the
arch-kirkwood/config.h and it was removed from the board config.
But it was forgotten to include the header. Now the header is included
in km_arm.h. Some other defines were obsolete due to this include,
these are also removed in this commit.

Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
cc: Valentin Longchamp <valentin.longchamp@keymile.com>
cc: Prafulla Wadaskar <prafulla@marvell.com>
cc: Heiko Schocher <hs@denx.de>
---
 include/configs/km/km_arm.h |   30 +++++++-----------------------
 include/configs/mgcoge3un.h |    1 +
 2 files changed, 8 insertions(+), 23 deletions(-)
Prafulla Wadaskar - June 15, 2011, 7:18 a.m.
> -----Original Message-----
> From: Holger Brunck [mailto:holger.brunck@keymile.com]
> Sent: Tuesday, June 14, 2011 12:42 PM
> To: u-boot@lists.denx.de
> Cc: Holger Brunck; Valentin Longchamp; Prafulla Wadaskar; Heiko Schocher
> Subject: [PATCH 2/2] arm/km: fix u-boot.kwb build breakage
> 
> commit 010a958b
> (arm/km: remove CONFIG_SYS_KWD_CONFIG from keymile-common.h)
> breaks building keymile arm targets, when u-boot.kwb tries to
> generate the binary with mkimage. A simple make <board> or MAKEALL
> succeeded because it don't try to build the kirwood binary at the end.
> 
> Due this commit we use the CONFIG_SYS_KWD_CONFIG from the
> arch-kirkwood/config.h and it was removed from the board config.
> But it was forgotten to include the header. Now the header is included
> in km_arm.h. Some other defines were obsolete due to this include,
> these are also removed in this commit.
> 
> Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
> cc: Valentin Longchamp <valentin.longchamp@keymile.com>
> cc: Prafulla Wadaskar <prafulla@marvell.com>
> cc: Heiko Schocher <hs@denx.de>
> ---
>  include/configs/km/km_arm.h |   30 +++++++-----------------------
>  include/configs/mgcoge3un.h |    1 +
>  2 files changed, 8 insertions(+), 23 deletions(-)
> 
> diff --git a/include/configs/km/km_arm.h b/include/configs/km/km_arm.h
> index 20ee6ea..29815be 100644
> --- a/include/configs/km/km_arm.h
> +++ b/include/configs/km/km_arm.h
> @@ -40,15 +40,20 @@
>   * High Level Configuration Options (easy to change)
>   */
>  #define CONFIG_MARVELL
> -#define CONFIG_ARM926EJS		/* Basic Architecture */
>  #define CONFIG_FEROCEON_88FR131		/* CPU Core subversion */
>  #define CONFIG_KIRKWOOD			/* SOC Family Name */
> -#define CONFIG_KW88F6281		/* SOC Name */
>  #define CONFIG_MACH_KM_KIRKWOOD		/* Machine type */
> 
>  /* include common defines/options for all Keymile boards */
>  #include "keymile-common.h"
> 
> +#define CONFIG_KW88F6281		/* SOC Name */

Why did you moved this from top to here?

> +#define CONFIG_CMD_NAND
> +#define CONFIG_CMD_SF

I think CONFIG_CMD_XXX should ideally below #include <config_cmd_default.h> and that too in <board_config>.h file.

Rest Ack of the patch

Regards..
Prafulla . .
Holger Brunck - June 15, 2011, 7:38 a.m.
Hi Prafulla,

On 06/15/2011 09:18 AM, Prafulla Wadaskar wrote:
> 
> 
>> -----Original Message-----
>> From: Holger Brunck [mailto:holger.brunck@keymile.com]
>> Sent: Tuesday, June 14, 2011 12:42 PM
>> To: u-boot@lists.denx.de
>> Cc: Holger Brunck; Valentin Longchamp; Prafulla Wadaskar; Heiko Schocher
>> Subject: [PATCH 2/2] arm/km: fix u-boot.kwb build breakage
>>
>> commit 010a958b
>> (arm/km: remove CONFIG_SYS_KWD_CONFIG from keymile-common.h)
>> breaks building keymile arm targets, when u-boot.kwb tries to
>> generate the binary with mkimage. A simple make <board> or MAKEALL
>> succeeded because it don't try to build the kirwood binary at the end.
>>
>> Due this commit we use the CONFIG_SYS_KWD_CONFIG from the
>> arch-kirkwood/config.h and it was removed from the board config.
>> But it was forgotten to include the header. Now the header is included
>> in km_arm.h. Some other defines were obsolete due to this include,
>> these are also removed in this commit.
>>
>> Signed-off-by: Holger Brunck <holger.brunck@keymile.com>
>> cc: Valentin Longchamp <valentin.longchamp@keymile.com>
>> cc: Prafulla Wadaskar <prafulla@marvell.com>
>> cc: Heiko Schocher <hs@denx.de>
>> ---
>>  include/configs/km/km_arm.h |   30 +++++++-----------------------
>>  include/configs/mgcoge3un.h |    1 +
>>  2 files changed, 8 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/configs/km/km_arm.h b/include/configs/km/km_arm.h
>> index 20ee6ea..29815be 100644
>> --- a/include/configs/km/km_arm.h
>> +++ b/include/configs/km/km_arm.h
>> @@ -40,15 +40,20 @@
>>   * High Level Configuration Options (easy to change)
>>   */
>>  #define CONFIG_MARVELL
>> -#define CONFIG_ARM926EJS		/* Basic Architecture */
>>  #define CONFIG_FEROCEON_88FR131		/* CPU Core subversion */
>>  #define CONFIG_KIRKWOOD			/* SOC Family Name */
>> -#define CONFIG_KW88F6281		/* SOC Name */
>>  #define CONFIG_MACH_KM_KIRKWOOD		/* Machine type */
>>
>>  /* include common defines/options for all Keymile boards */
>>  #include "keymile-common.h"
>>
>> +#define CONFIG_KW88F6281		/* SOC Name */
> 
> Why did you moved this from top to here?
> 

Indeed this is unneeded. I fix this and resubmit.

>> +#define CONFIG_CMD_NAND
>> +#define CONFIG_CMD_SF
> 
> I think CONFIG_CMD_XXX should ideally below #include <config_cmd_default.h> and that too in <board_config>.h file.
>

I agree for a single board support, but this is not possible in our approach. We
include config_cmd_default.h in keymile-common.h. And keymile-common.h is valid
for our powerpc and arm boards. In km_arm.h we add the CMD settings which are
only additionaly needed on our arm boards.

> Rest Ack of the patch
>

Ok.

Best regards
Holger
Albert ARIBAUD - July 16, 2011, 10:51 a.m.
Le 15/06/2011 09:38, Holger Brunck a écrit :

> I agree for a single board support, but this is not possible in our approach. We
> include config_cmd_default.h in keymile-common.h. And keymile-common.h is valid
> for our powerpc and arm boards. In km_arm.h we add the CMD settings which are
> only additionaly needed on our arm boards.

You can #undef things just as you can #define them after including 
default command headers -- I personally see no issue with this.

Amicalement,
Holger Brunck - July 18, 2011, 9:41 a.m.
Hi Albert,

On 07/16/2011 12:51 PM, Albert ARIBAUD wrote:
> Le 15/06/2011 09:38, Holger Brunck a écrit :
> 
>> I agree for a single board support, but this is not possible in our approach. We
>> include config_cmd_default.h in keymile-common.h. And keymile-common.h is valid
>> for our powerpc and arm boards. In km_arm.h we add the CMD settings which are
>> only additionaly needed on our arm boards.
> 
> You can #undef things just as you can #define them after including default
> command headers -- I personally see no issue with this.
> 

yes you are right it could be done this way. But I think it's more clearly
arranged if we only define the commands in keymile-common.h wich are common for
all keymile boards rather to define a superset there and undef them later on in
km-powerpc.h or km_arm.h.

Best regards
Holger Brunck

Patch

diff --git a/include/configs/km/km_arm.h b/include/configs/km/km_arm.h
index 20ee6ea..29815be 100644
--- a/include/configs/km/km_arm.h
+++ b/include/configs/km/km_arm.h
@@ -40,15 +40,20 @@ 
  * High Level Configuration Options (easy to change)
  */
 #define CONFIG_MARVELL
-#define CONFIG_ARM926EJS		/* Basic Architecture */
 #define CONFIG_FEROCEON_88FR131		/* CPU Core subversion */
 #define CONFIG_KIRKWOOD			/* SOC Family Name */
-#define CONFIG_KW88F6281		/* SOC Name */
 #define CONFIG_MACH_KM_KIRKWOOD		/* Machine type */
 
 /* include common defines/options for all Keymile boards */
 #include "keymile-common.h"
 
+#define CONFIG_KW88F6281		/* SOC Name */
+#define CONFIG_CMD_NAND
+#define CONFIG_CMD_SF
+#define CONFIG_SOFT_I2C		/* I2C bit-banged	*/
+
+#include "asm/arch/config.h"
+
 #define CONFIG_SYS_TEXT_BASE	0x04000000	/* code address after reloc */
 #define CONFIG_ENV_SIZE		(128 << 10)	/* NAND chip block size	*/
 #define CONFIG_SYS_MEMTEST_START 0x00400000	/* 4M */
@@ -75,12 +80,7 @@ 
 
 #define CONFIG_KM_ARCH_DBG_FILE		"scripts/debug-arm-env.txt"
 
-#define CONFIG_MD5	/* get_random_hex on krikwood needs MD5 support */
 #define CONFIG_SKIP_LOWLEVEL_INIT	/* disable board lowlevel_init */
-#define CONFIG_KIRKWOOD_EGIGA_INIT	/* Enable GbePort0/1 for kernel */
-#undef  CONFIG_KIRKWOOD_PCIE_INIT	/* Disable PCIE Port0 for kernel */
-#define CONFIG_KIRKWOOD_RGMII_PAD_1V8	/* Set RGMII Pad voltage to 1.8V */
-
 #define CONFIG_MISC_INIT_R
 
 /*
@@ -116,7 +116,6 @@ 
  */
 #define CONFIG_CMD_ELF
 #define CONFIG_CMD_MTDPARTS
-#define CONFIG_CMD_NAND
 #define CONFIG_CMD_NFS
 
 /*
@@ -131,8 +130,6 @@ 
  */
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
 #define NAND_MAX_CHIPS			1
-#define CONFIG_NAND_KIRKWOOD
-#define CONFIG_SYS_NAND_BASE		0xd8000000
 
 #define BOOTFLASH_START		0x0
 
@@ -175,8 +172,6 @@ 
 /*
  * I2C related stuff
  */
-#define	CONFIG_SOFT_I2C		/* I2C bit-banged	*/
-
 #define	CONFIG_KIRKWOOD_GPIO		/* Enable GPIO Support */
 #if defined(CONFIG_SOFT_I2C)
 #ifndef __ASSEMBLY__
@@ -200,8 +195,6 @@  int get_scl(void);
 #define I2C_DELAY	udelay(3)	/* 1/4 I2C clock duration */
 #define I2C_SOFT_DECLARATIONS
 
-#define	CONFIG_SYS_I2C_SLAVE		0x0
-#define	CONFIG_SYS_I2C_SPEED		100000
 #endif
 
 #define CONFIG_SYS_I2C_EEPROM_ADDR	0x50
@@ -224,15 +217,8 @@  int get_scl(void);
 #define CONFIG_ENV_OFFSET_REDUND	0x2000 /* no bracets! */
 #define CONFIG_ENV_SIZE_REDUND		(CONFIG_ENV_SIZE)
 
-#define CONFIG_CMD_SF
-
 #define CONFIG_SPI_FLASH
-#define CONFIG_HARD_SPI
-#define CONFIG_KIRKWOOD_SPI
 #define CONFIG_SPI_FLASH_STMICRO
-#define CONFIG_ENV_SPI_BUS		0
-#define CONFIG_ENV_SPI_CS		0
-#define CONFIG_ENV_SPI_MAX_HZ		50000000	/* 50Mhz */
 
 #define FLASH_GPIO_PIN			0x00010000
 
@@ -272,8 +258,6 @@  int get_scl(void);
 
 /* additions for new relocation code, must be added to all boards */
 #define CONFIG_SYS_SDRAM_BASE		0x00000000
-/* Kirkwood has 2k of Security SRAM, use it for SP */
-#define CONFIG_SYS_INIT_SP_ADDR		0xC8012000
 /* Do early setups now in board_init_f() */
 #define CONFIG_BOARD_EARLY_INIT_F
 
diff --git a/include/configs/mgcoge3un.h b/include/configs/mgcoge3un.h
index 6d56d7d..8d1b61f 100644
--- a/include/configs/mgcoge3un.h
+++ b/include/configs/mgcoge3un.h
@@ -48,6 +48,7 @@ 
 #define KM_ENV_BUS	"pca9547:70:d" /* I2C2 (Mux-Port 5)*/
 
 /* we use a new RAM type on mgcoge3un board */
+#undef CONFIG_SYS_KWD_CONFIG
 #define CONFIG_SYS_KWD_CONFIG $(SRCTREE)/$(CONFIG_BOARDDIR)/kwbimage-memphis.cfg
 
 /*