[U-Boot] [PATCH] keystone2: use detected ddr3a size

[Tom Rini <trini@konsulko.com>]

On Mon, Jun 15, 2015 at 12:42:49PM -0400, Vitaly Andrianov wrote:
> 
> 
> On 06/15/2015 10:17 AM, Tom Rini wrote:
> >On Mon, Jun 15, 2015 at 08:48:01AM -0400, Vitaly Andrianov wrote:
> >
> >>KS2 u-boot detects the ddr3a size installed to EVM. The detected size can
> >>be used instead of environment variable. Because the ddr3 configuration is
> >>done before relocation we cannot use a global variable to pass the
> >>ddr3_size to ft_board_setup(). Instead we have to use the global data
> >>structure.
> >>
> >>Because KS2 u-boot works in 32 bit address space the existing ram_size
> >>global data filed cannot be used. The maximum, which the get_ram_size()
> >>can detect is 2GB only. This patch creates the ddr3_size filed in the
> >>arch_global_data structure, which is used for that purpose.
> >>
> >>Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
> >
> >So we've got a few possibilities here, yes?  Since we have the ability
> >to change the DDR modules on the board and read the sizes in the SPD
> >information U-Boot is the place where the board can find out if we have
> >say 1GB or 2GB of memory and thus has to be the one to correctly
> >populate the device tree.  So the "fix" that we're talking about for
> >Calxeda can't be applied here.
> >
> >But this also brings up http://patchwork.ozlabs.org/patch/281094/ (and
> >the follow-up of http://patchwork.ozlabs.org/patch/291219/ and
> >http://patchwork.ozlabs.org/patch/291247/) where no, we have a problem
> >that we need to fix.
> >
> Hi Tom,
> 
> If I understand correctly the patches above are about changing long
> to unsigned long to accommodate possible 2GB of DDR size. Or to use
> phys_addr_t for 64bit architecture. Did I miss something?

No, but that is part of your actual problem.  You have 2GB of DDR (or
more in some cases) that you want to report.

> The problem with KS2 platforms is that it is a 32 bit architecture
> which uses LPAE. So, the EVMs may have more than 2GB memory
> (typically 4 or 8 GB), but u-boot sees only 2GB maximum. That is
> what get_ram_size() can detect.

Right.  So you're in the same problem area as the highbank board (and
some Tegra boards too I think).

> Also it is not always possible to use SPD data to detect the DDR
> size because not all EVMs use SODIMM. Some of them use DDR3 chips
> populated to the main board.

Right.  But on the ones you added support for the SPD data to, you do,
right?

> Even if we uses SPD data, we detect the DDR3 size before relocation.
> So, I believe, instead of reading the SPD EEPROM and calculate the
> size again, it is easier just to pass the ddr3 size through the
> global_data.

Well we need to do something, certainly.  The problem is that we need to
populate the device tree for the kernel with the correct amount of
memory.  Today we have a system that essentially forces what we have
stored today in gd to be what we populate.  This is wrong in the LPAE
case. In the case of highbank, something else has already correctly
populated the DT with the memory sizes and a patch has been made to say
"lets just set CONFIG_NR_DRAM_BANKS to 0 so we can avoid that 'fixup'".
This won't help Keystone as U-Boot is where we somehow know how much
memory there really is.

Today, a bit further down in board/ti/ks2_evm/board.c than this patch
shows you play some games to correct the DT node.  And part of the
problem is that if we add "ddr3_size" to just the keystone DT we've made
a very specific work-around for this general problem.  You're still
having to play games to know that you shoved a >32bit value into a 32bit
variable.

So yes, I think you need to structure the code such that you can call a
function to read the SPD information and see how big your memory is, and
then go poke arch/arm/lib/bootm-fdt.c::arch_fixup_fdt() to be something
like:
__weak void board_calc_memory_size(u64 *start, u64 *end)
{
  .. current for-loop
}

int arch_fixup_fdt(void *blob)
{
  board_calc_memory_size(&start, &end);
  fdt_fixup_memory_banks(...);
  ...
}

And then have keystone fill in a board_calc_memory_size() and even
populate the real # of banks and such if you want.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150618/14fa53ec/attachment.sig>