Hi > > > > +#ifdef CFG_SDR_CONFIG > > > > mov r1, #CFG_SDR_CONFIG > > > > + mov r9, #0xff > > > > +#else > > > > + mov r1, #0x1d /* 256 MB, two banks of 128 MB */ > > > > + mov r9, #0 > > > > +#endif > > > > > > I don't want such $ifdef's in global code. Why do you > thinkthat 2 x > > > 128 MB would be a default configuration for all IXP based boards? > > > > When CFG_SDR_CONFIG is defined, it holds the memory size, > which is set > > in the config.h > > When this value is not set, autodetection is assumed. > > Setting up the board as 2 x 128Mb is used for the autodetection. > > Understood. Nevertheless the resulting code is quite complex hard to > read.
Not complex, just hard to read because of assembly :) > > > > > +sdr_init: > > > > /* snip */ > > > > > > +sdr_init_done: > > > > > > This whole test makes not much sense to me. I think the > code should be > > > changed to use the standard get_ram_size() funciton instead (see > > > common/memsize.c). > > > > get_ram_size will nog set the memory controller to the correct size. > > The ixp can not run C code when memory is not initialized. > > Because of the missing C environment (stack etc)? We could probably > use a part of the QueueManager SRAM for a temorary initial stack, so > that we could run C code very early. And do the SDRAM initialization > and autodetection in C instead of assembler (as done for PPC). This would involve a large change in the code, compared to this patch. Even the relocation code is in assembler right now (because of this reason) > If we could run this SDRAM setup code in C, you could implement a weak > default init function that could be overwritten by a board specific > version (in your Yes, ofcource, in C life is much simpler. > case). This way we wouldn't add more custom stuff into the > common/generic source files. > > What do you think? I think that this code could be used as generic code, it is not board specific code. I could change de ifdef structure to enable/disable the autodetection part, instead of using the CFG_SDR_CONFIG define, but a ifdef would remain. > > > Maybe it could be used as a replacement for the code in pdnb.c yes. > > Yes. This should be replaced. Ok, makes sense. Regards, Martijn de Gouw Disclaimer: The information contained in this email, including any attachments is confidential and is for the sole use of the intended recipient(s). Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please notify the sender immediately by replying to this message and destroy all copies of this message and any attachments. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
