Hello Wolfgang,
Wolfgang Denk wrote:
In message 4D2D7122.60909@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?
Good question ... as I know, there is no possibility to detect the hardware on an other way then over the RTC ... and I don;t want to go this way ... what if the RTC is not responding?
Maybe you can ask the hardware guys again?
Ok. done. As Detlev mentioned in the meantime the "LOWBOOT" cases can be removed, done.
- 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 ?
It is set in arch/powerpc/lib/board.c before calling update_flash_size(), so this adaption is only needed, if the baseaddr is changing on different flash sizes, what is valid for the rev5 board ...
It may not be needed, but I think it should compute the same result, so you should be able to omit the "#if" and the "#endif".
If you should compute a different result, then I'd wonder if the code is actually correct?
Yep, you are right, the "#if ... #endif" is not needed here.
-#endif /* CONFIG_CMD_IDE */
Ah! So this is a bug fix?
Yep. Should I seperate this in an extra patch?
At least mention it in the commit message.
Ok, add this to the commit message.
Hmm.. I have only one flash on the !rev5 board, so I need this differentiation here, or?
Yes.
Hmm.. after looking in code, can your proposal work?: Is flash_info[0].size valid, when the cfi driver detects the flash?
For the probing, a temporary address can be used. You set up the final mapping only after the sizes are knows, similar to what we do when we have several banks of RAM.
But thats exactly what I do with the defines in include/configs/digsy_mtc.h
#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}
and after detecting the real size through the cfi driver on this temporary addresses, the flash_info gets updated with the new baseaddr through the call:
int update_flash_size (int flash_size) [...] if (dev->size) { /* calculate new base addr for this chipselect */ base -= dev->size; [...] /* recalculate the sectoraddr in the cfi driver */ size += flash_get_size(base, i);
so after that, flash_info has the right values and I update the chip selects in board specific code ...
bye, Heiko