[U-Boot] [PATCH 1/2] KW: Move the memory register definitions into kirkwood.h

[Gerlando Falauto]

Dear Marek Vasut,


On 06/29/2012 12:08 PM, Marek Vasut wrote:
> Dear Gerlando Falauto,
>
>> Dear Maruk,

sorry about my misspelling!

>>
>> I am currently trying to address the exact same issue (SDRAM size
>> detection and fixup).
>> My idea was however (as opposed to moving register definition as you
>> did), to add a fixup function to dram.c, say:
>>
>> /*
>>    * kw_sdram_bs - writes SDRAM Bank size
>>    */
>> void kw_sdram_bs_set(enum memory_bank bank, u32 size)
>> {
>> 	/* Read current register value */
>> 	u32 reg = readl(KW_REG_CPUCS_WIN_SZ(bank));
>>
>> 	printf("Current value: %x\n", reg);
>> 	/* Clear window size */
>> 	reg&= ~KW_REG_CPUCS_WIN_SIZE(0xFF);
>>
>> 	/* Set new window size */
>> 	reg |= KW_REG_CPUCS_WIN_SIZE((size - 1)>>  24);
>>
>> 	printf("Writing: %x\n", reg);
>> 	writel(reg, KW_REG_CPUCS_WIN_SZ(bank));
>> }
>>
>> which would then be called to fix the window size according to the total
>> memory size as reported by get_ram_size().
>
> Read up at [1].
>
> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/133991

Did read up, not sure I understand.
If I get it right, *this* thread and patchset follows the feedback from 
the above thread.

What I am trying to say is: we have a function (available to all 
kirkwood boards) which reads the value of the window size register for a 
given memory bank and returns its size.
Now we want to do the opposite, write the window size register according 
to the detected SDRAM size.
Why should we make up our own (board-specific) register manipulation 
tweaks, as opposed to having a function do it for us in dram.c as its 
companion?
Reasons behind it, are board-specific, so in your case it would turn into:

+int board_early_init_f(void)
+{
+	unsigned long size = get_ram_size(PHYS_SDRAM_1, PHYS_SDRAM_1_SIZE);
+
+	/* 256MB module, adjust BAR register */
+	if (size == 256 * 1024 * 1024)
+		kw_sdram_bs_set(0, size);

Which I find cleaner.
Even the if part could be also removed.
And would work as-is on any board needing it (ours, for instance).

What do you think?

Thank you for your patience,
Gerlando