Re: [PATCHv4 6/7] S5P DRAM support

[Alexey Galakhov <agalakhov@gmail.com>]

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.

Of course, it is possible to get rid of #ifdefs in s5p_init_dram_bank()
but #ifdef is still needed for init_seq(). Ot is there any better solution?

Regards,
--
Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox