Dear Axel Beierlein,
In message 1248471861-8085-1-git-send-email-beierlein@goetting.de you wrote:
Signed-off-by: Axel Beierlein beierlein@goetting.de
Makefile | 7 +++++++ board/tqc/tqm5200/tqm5200.c | 9 ++++++++- include/configs/TQM5200.h | 33 ++++++++++++++++++++++++++------- 3 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile index 2a06440..7f197dd 100644 --- a/Makefile +++ b/Makefile @@ -764,6 +764,7 @@ Total5200_Rev2_lowboot_config: unconfig } @$(MKCONFIG) -a Total5200 ppc mpc5xxx total5200
+HG43630_config \ cam5200_config \ cam5200_niosflash_config \ fo300_config \
Please use a lower case board name, and keep list sorted.
@@ -808,6 +809,12 @@ TQM5200_STK100_config: unconfig @[ -z "$(findstring HIGHBOOT,$@)" ] || \ { echo "TEXT_BASE = 0xFFF00000" >$(obj)board/tqm5200/config.tmp ; \ }
- @[ -z "$(findstring HG43630,$@)" ] || \
{ echo "#define CONFIG_HG43630" >>$(obj)include/config.h ; \echo "#define CONFIG_TQM5200S" >>$(obj)include/config.h ; \echo "#define CONFIG_TQM5200_B" >>$(obj)include/config.h ; \$(XECHO) "... TQM5200S on Goetting HG43630 Board" ; \}
Please don't add board specific configuration to the top level Makefile; do thi sin your board config file instead.
diff --git a/board/tqc/tqm5200/tqm5200.c b/board/tqc/tqm5200/tqm5200.c index faa2e02..0d69e86 100644 --- a/board/tqc/tqm5200/tqm5200.c +++ b/board/tqc/tqm5200/tqm5200.c @@ -255,6 +255,10 @@ int checkboard (void) # error "UNKNOWN" #endif
+#ifdef CONFIG_HG43630 +#define CARRIER_NAME "HG43630" +#endif
Please add as "#elif" in the "#if" construct above. And move the block out of checkboard(), please.
{ if (line_number == 1) { strcpy (info, " Board: TQM5200 (TQ-Components GmbH)"); -#if defined (CONFIG_STK52XX) || defined (CONFIG_TB5200) || defined(CONFIG_FO300) +#if defined (CONFIG_STK52XX) || defined (CONFIG_TB5200) || defined(CONFIG_FO300) || defined(CONFIG_HG43630)
Line too long.
#if defined (CONFIG_STK52XX) strcpy (info, " on a STK52xx carrier board"); @@ -657,6 +661,9 @@ void video_get_info_str (int line_number, char *info) #if defined (CONFIG_FO300) strcpy (info, " on a FO300 carrier board"); #endif +#if defined (CONFIG_HG43630)
strcpy (info, " on a HG43630 carrier board");+#endif
It's time to get rid of this "#if" here and use something like
strcpy (info, " on a " CARRIER_NAME " carrier board");
instead, as it has been done in checkboard() long ago.
diff --git a/include/configs/TQM5200.h b/include/configs/TQM5200.h index a4336a7..0f5ff77 100644 --- a/include/configs/TQM5200.h +++ b/include/configs/TQM5200.h @@ -71,7 +71,7 @@ /* switch is open */ #endif /* CONFIG_FO300 */
-#ifdef CONFIG_STK52XX +#if defined(CONFIG_STK52XX) && !defined(CONFIG_HG43630)
Is CONFIG_STK52XX really defined for CONFIG_HG43630? where?
#define CONFIG_PS2KBD /* AT-PS/2 Keyboard */ #define CONFIG_PS2MULT /* .. on PS/2 Multiplexer */ #define CONFIG_PS2SERIAL 6 /* .. on PSC6 */ @@ -85,6 +85,7 @@
- 0x50000000 - 0x50ffffff - PCI IO Space
*/ #ifdef CONFIG_STK52XX +#ifndef CONFIG_HG43630 #define CONFIG_PCI 1 #define CONFIG_PCI_PNP 1 /* #define CONFIG_PCI_SCAN_SHOW 1 */ @@ -96,7 +97,7 @@ #define CONFIG_PCI_IO_BUS 0x50000000 #define CONFIG_PCI_IO_PHYS CONFIG_PCI_IO_BUS #define CONFIG_PCI_IO_SIZE 0x01000000
+#endif/* ifndef(CONFIG_HG43630)*/
Add blank line here. But - isn;t it sufficient to only not define CONFIG_PCI, while leaving the rest unchanged?
-#if defined(CONFIG_STK52XX) || defined(CONFIG_FO300) +#if defined(CONFIG_STK52XX) || defined(CONFIG_FO300) && !defined(CONFIG_HG43630)
Are you sure this logic is correct, and necessary?
@@ -196,7 +197,7 @@ #define CONFIG_PCIAUTO_SKIP_HOST_BRIDGE 1 #endif
-#if defined(CONFIG_MINIFAP) || defined(CONFIG_STK52XX) || defined(CONFIG_FO300) +#if defined(CONFIG_MINIFAP) || defined(CONFIG_STK52XX) || defined(CONFIG_FO300)
Please do not add trailing white space.
#ifdef CONFIG_STK52XX # if defined(CONFIG_TQM5200_B) # if defined(CONFIG_SYS_LOWBOOT) +# if defined(CONFIG_HG43630) +# define MTDPARTS_DEFAULT "mtdparts=TQM5200-0:512k(u-boot)," \
"512k(env)," \"2m(kernel)," \"16m(ramfs1)," \"5m(ramfs2)," \"8m(jffs2)" \+# else # define MTDPARTS_DEFAULT "mtdparts=TQM5200-0:1m(firmware)," \ "256k(dtb)," \ "2304k(kernel)," \ @@ -422,6 +438,7 @@ "2m(initrd)," \ "8m(misc)," \ "16m(big-fs)" +# endif /* CONFIG_HG43630 */ # else /* highboot */ # define MTDPARTS_DEFAULT "mtdparts=TQM5200-0:2560k(kernel)," \
This #ifdef block is becoming an unreadable mess. Please split up into separate definitions for each board plus a default config, something like
#define MTDPARTS_DEFAULT ... #define MTDPARTS_HG43630 ... #define MTDPARTS_CAM5200 ... #define MTDPARTS_FO300 ... #define MTDPARTS_STK5200 MTDPARTS_DEFAULT
and then use reference the needed version based on the CARRIER_NAME setting.
#undef CONFIG_IDE_8xx_DIRECT /* Direct IDE not supported */ #undef CONFIG_IDE_LED /* LED for ide not supported */
Don't delete this blank line.
#define CONFIG_IDE_RESET /* reset for ide supported */ #define CONFIG_IDE_PREINIT
+#ifndef CONFIG_HG43630 #define CONFIG_SYS_IDE_MAXBUS 1 /* max. 1 IDE bus */ #define CONFIG_SYS_IDE_MAXDEVICE 2 /* max. 2 drives per IDE bus */
+#else +#define CONFIG_SYS_IDE_MAXBUS 0 +#define CONFIG_SYS_IDE_MAXDEVICE 0 +#endif /* CONFIG_HG43630 */ #define CONFIG_SYS_ATA_IDE0_OFFSET 0x0000
Is it not sufficient to not enable IDE support?
#define CONFIG_SYS_ATA_BASE_ADDR MPC5XXX_ATA @@ -725,7 +745,6 @@
/* Support ATAPI devices */ #define CONFIG_ATAPI 1
/*-----------------------------------------------------------------------
Don't delete this blank line.
Best regards,
Wolfgang Denk