On Tue, May 22, 2012 at 03:17:32PM +0600, Alexey Galakhov wrote:
> On 22.05.2012 02:02, Sascha Hauer wrote:
> > On Fri, May 18, 2012 at 03:43:30PM +0600, Alexey Galakhov wrote:
> >> +void __bare_init s5p_init_dram_bank(uint32_t base, uint32_t mc0, uint32_t mc1)
> >> +{
> >> +#ifdef S5P_DRAM_LPDDR
> >> + uint32_t reg = 0x100;
> >> +#endif
> >> +#ifdef S5P_DRAM_LPDDR2
> >> + uint32_t reg = 0x200;
> >> +#endif
> >> +#ifdef S5P_DRAM_DDR2
> >> + uint32_t reg = 0x400;
> >> +#endif
> >> + reg |= (S5P_DRAM_BURST) << 20;
> >> +#ifdef S5P_DRAM_16BIT
> >> + reg |= 0x1000;
> >> +#else /* 32-bit */
> >> + reg |= 0x2000;
> >> +#endif
> >
> > Since this function is called from board specific code, can we pass in
> > the DDR/DDR2 16BIT/32BIT settings as arguments to this function and
> > get rid of these ifdefs?
> >
> > Sascha
>
> Thank you for the question. Most likely not.
>
> According to the datasheet, LPDDR/LPDDR2/DDR2 initialization sequence is
> completely different. It is in init_seq() function. Since I have no
> LPDDR(2) hardware to test, I coded the DDR2 version only. A runtime
> switch like that:
>
> switch(mode) {
> case LPDDR:
> lpddr_init_seq();
> break;
> case LPDDR2:
> lpddr2_init_seq();
> break;
> case DDR2:
> ddr2_init_seq();
> break;
> }
>
> is too expensive for S5PV210's __bare_init which is very limited in
> space. Thus it is better to have only one function compiled in.
I see. In this case you could provide a s5p_init_dram_bank_ddr2 which
does the setup for ddr2 ram. The linker will then throw away any other
eventually compiled in s5p_init_dram_bank_* function. Code common to
all sdram setup functions could still be shared without overhead.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
