Dear Heiko Schocher,
In message 1294816806-32614-2-git-send-email-hs@denx.de you wrote:
Global question: do we really need an CONFIG_DIGSY_REV5? Is there not a way to determine the revision by probing the hardware? For example, the RTC's show up at different addresses on the bus - but ther emight be even easier ways?
diff --git a/board/digsy_mtc/digsy_mtc.c b/board/digsy_mtc/digsy_mtc.c index cc6087b..2b0c574 100644 --- a/board/digsy_mtc/digsy_mtc.c +++ b/board/digsy_mtc/digsy_mtc.c
....
#endif /* CONFIG_IDE_RESET */ +#endif /* CONFIG_CMD_IDE */
This looks wrong to me. You did not add a matching "#if" or "#ifdef" anywhere?
+/* Update the Flash Baseaddr settings */ +int update_flash_size (int flash_size) +{
- volatile struct mpc5xxx_mmap_ctl *mm =
(struct mpc5xxx_mmap_ctl *) CONFIG_SYS_MBAR;- flash_info_t *dev;
- int i;
- int size = 0;
- unsigned long base = 0x0;
- u32 *cs_reg = (u32 *)&mm->cs0_start;
- for (i = 0; i < 2; i++) {
dev = &flash_info[i];if (dev->size) {/* calculate new base addr for this chipselect */base -= dev->size;out_be32(cs_reg, START_REG(base));cs_reg++;out_be32(cs_reg, STOP_REG(base, dev->size));cs_reg++;/* recalculate the sectoraddr in the cfi driver */size += flash_get_size(base, i);}- }
+#if defined(CONFIG_DIGSY_REV5)
- gd->bd->bi_flashstart = base;
+#endif
Why is this #if needed? Why not always set bi_flashstart ?
void ft_board_setup(void *blob, bd_t *bd) { ft_cpu_setup(blob, bd);
- /* remove RTC */
+#if defined(CONFIG_DIGSY_REV5)
- ft_delete_node(blob, "dallas,ds1339");
+#else
- ft_delete_node(blob, "mc,rv3029c2");
+#endif
You should add a comment here what you are doing, and why.
ft_delete_node() returns int - why do you ignore the return codes?
+#if defined(CONFIG_SYS_UPDATE_FLASH_SIZE)
- ft_adapt_flash_base(blob);
+#endif
ft_adapt_flash_base() returns int - why do you ignore the return code?
} #endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */
-#endif /* CONFIG_CMD_IDE */
Ah! So this is a bug fix?
diff --git a/board/digsy_mtc/is45s16800a2.h b/board/digsy_mtc/is45s16800a2.h new file mode 100644 index 0000000..029e6cd --- /dev/null +++ b/board/digsy_mtc/is45s16800a2.h @@ -0,0 +1,27 @@ +/*
- (C) Copyright 2004-2009
- Mark Jonas, Freescale Semiconductor, mark.jonas@motorola.com.
Are you sure that Mark wrote any of this code?
diff --git a/boards.cfg b/boards.cfg index 94b8745..9e1fc14 100644 --- a/boards.cfg +++ b/boards.cfg @@ -241,6 +241,9 @@ cm5200 powerpc mpc5xxx digsy_mtc powerpc mpc5xxx digsy_mtc digsy_mtc_LOWBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0xFF000000 digsy_mtc_RAMBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0x00100000 +digsy_mtc_rev5 powerpc mpc5xxx digsy_mtc - - digsy_mtc:DIGSY_REV5 +digsy_mtc_rev5_LOWBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0xFF000000,DIGSY_REV5 +digsy_mtc_rev5_RAMBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0x00100000,DIGSY_REV5
Do we really need all these 6 configurations for the digsy_mtc board? Are all of them actively being used?
diff --git a/doc/README.cfi b/doc/README.cfi new file mode 100644 index 0000000..fa35108 --- /dev/null +++ b/doc/README.cfi @@ -0,0 +1,15 @@ +known issues:
+using M29W128GH from Numonyx:
+You need to add a board specific flash_cmd_reset() function +for this chip to work correctly. Something like this should +work (tested on the digsy_mtc board):
+void flash_cmd_reset(flash_info_t *info) +{
flash_write_cmd(info, 0, 0, AMD_CMD_RESET);+}
Stefan, can you please send an explicit ACK for this part?
+#if defined(CONFIG_DIGSY_REV5) +#define CONFIG_SYS_FLASH_BASE 0xFE000000 +#define CONFIG_SYS_FLASH_BASE_CS1 0xFC000000 +#define CONFIG_SYS_MAX_FLASH_BANKS 2 +#define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE_CS1, \
CONFIG_SYS_FLASH_BASE}+#define CONFIG_SYS_UPDATE_FLASH_SIZE +#define CONFIG_FDT_FIXUP_NOR_FLASH_SIZE +#else #define CONFIG_SYS_FLASH_BASE 0xFF000000 -#define CONFIG_SYS_FLASH_SIZE 0x01000000
#define CONFIG_SYS_MAX_FLASH_BANKS 1 +#define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE } +#endif
Is this really needed? I understand you map the flash at the end of the address space? I used to use code like this in similar situations:
#define CONFIG_SYS_FLASH_BASE (0-flash_info[0].size)
This will auto-adjust depending on the actual size of the flash, and avoids all these ifdef's. Maybe you can do something similar?
Best regards,
Wolfgang Denk