
Dear Heiko Schocher,
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?
- 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?
-#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.
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.
Best regards,
Wolfgang Denk