Hello Wolfgang,
Wolfgang Denk wrote:
> In message <4d2d7122.60...@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
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
