On 06/18/2015 11:57 AM, Tom Rini wrote:
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 <vita...@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?


On EVM that has SODIMMs with SPD eeprom we already read SPD data and detected the size. That happen before relocation. Why do we need to repeat that step again after relocation. Also it make the "after-relocation" code conditional. The EVM w/o SPD info has to get the size by some different means. Having one simple variable and the global data structure simply solve the issue. This variable is Keystone2 specific and doesn't affect any other architecture. If you don't like that the size is represented in GB we can make the variable 64 bit.

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.


We have situations when not all EVM DDR3 memory is available for Linux kernel. Some portion (sometimes more that 50%) of the memory can be dedicated for DSPs. In that case kernel even doesn't have to know about that memory. In that case u-boot is responsible to configure DDR3 controller for the entire DDR3 memory, but Linux shouldn't see the entire memory. We still have to "play some game". Some customers ask for board specific environment variables to override default memory configuration in fdt.

The board_calc_memory_size(), which you offered is an excellent solution to perform the custom arch_fdt_fixup. It is even possible to use it for Keystone2 requirements. But it will require significant redesign and testing of existing code, and still "playing some game".

So, if you strongly disagree to add one (for Keystone only) variable to global data, forget about it. I'll work on using your approach later.

Thanks,
-Vitaly


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to