[PATCHv4 6/7] S5P DRAM support

[Sascha Hauer]

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 |