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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to