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

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

[Vitaly Andrianov]

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>
---
 arch/arm/include/asm/global_data.h |  3 +++
 board/ti/ks2_evm/board.c           | 10 +++-------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index bb24f33..c6c0cdf 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -51,6 +51,9 @@ struct arch_global_data {
 #ifdef CONFIG_FSL_LSCH3
 	unsigned long mem2_clk;
 #endif
+#ifdef CONFIG_ARCH_KEYSTONE
+	int	ddr3_size;
+#endif
 };
 
 #include <asm-generic/global_data.h>
diff --git a/board/ti/ks2_evm/board.c b/board/ti/ks2_evm/board.c
index 8892a28..a5c4b60 100644
--- a/board/ti/ks2_evm/board.c
+++ b/board/ti/ks2_evm/board.c
@@ -35,14 +35,12 @@ static struct aemif_config aemif_configs[] = {
 
 int dram_init(void)
 {
-	u32 ddr3_size;
-
-	ddr3_size = ddr3_init();
+	gd->arch.ddr3_size = ddr3_init();
 
 	gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
 				    CONFIG_MAX_RAM_BANK_SIZE);
 	aemif_init(ARRAY_SIZE(aemif_configs), aemif_configs);
-	ddr3_init_ecc(KS2_DDR3A_EMIF_CTRL_BASE, ddr3_size);
+	ddr3_init_ecc(KS2_DDR3A_EMIF_CTRL_BASE, gd->arch.ddr3_size);
 	return 0;
 }
 
@@ -135,9 +133,7 @@ int ft_board_setup(void *blob, bd_t *bd)
 
 	ddr3a_size = 0;
 	if (lpae) {
-		env = getenv("ddr3a_size");
-		if (env)
-			ddr3a_size = simple_strtol(env, NULL, 10);
+		ddr3a_size = gd->arch.ddr3_size;
 		if ((ddr3a_size != 8) && (ddr3a_size != 4))
 			ddr3a_size = 0;
 	}
-- 
1.9.1

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

[Tom Rini]

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.

-- 
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/20150615/611e746c/attachment.sig>

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

[Vitaly Andrianov]

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?

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.

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.

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.

Thanks,
Vitaly

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

[York Sun]

On 06/15/2015 09:42 AM, 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?
> 
> 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.
> 
> 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.
> 
> 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.
> 

My 2 cents.

For powerpc, we have been using SPD for a long time. We use gd->ram_size to hold
the total size. For 32-bit u-boot, only 2GB memory can be used by u-boot. But
gd->ram_size still hold the total size and tells Linux.

For arm, we reuse the driver and update gd->ram_size in dram_init().

For either case, we have found it to be more complex than using the SPD data
directly. We need to take into account the actual bus width. One can put a
standard DIMM but only use half the width.

York

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

[Vitaly Andrianov]

On 06/15/2015 12:56 PM, York Sun wrote:
>
>
> On 06/15/2015 09:42 AM, 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?
>>
>> 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.
>>
>> 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.
>>
>> 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.
>>
>
> My 2 cents.
>
> For powerpc, we have been using SPD for a long time. We use gd->ram_size to hold
> the total size. For 32-bit u-boot, only 2GB memory can be used by u-boot. But
> gd->ram_size still hold the total size and tells Linux.
>
> For arm, we reuse the driver and update gd->ram_size in dram_init().
>
> For either case, we have found it to be more complex than using the SPD data
> directly. We need to take into account the actual bus width. One can put a
> standard DIMM but only use half the width.
>
> York
>
Hi York,

Thank you for the reply. Actually Keystone2 platforms detect ddr3 size 
and that is done in ddr3_init(). The function is called before 
relocation. The size detection itself can be done in many different 
ways. It can be done by reading SPD data (if it is presend), or just 
simply read from board configuration file (if DDR3 chips are just 
soldered to the board). This patch is not about this.
This patch is how to make the ddr3 size available to other patch of the 
code after relocation. The code which uses the size is common for all 
KS2 SOCs and EVMs, while the ddr3_init() implementation may be different.

Regards,
-Vitaly

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

[Tom Rini]

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>

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

[Vitaly Andrianov]

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 <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?
>

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] [PATCH] keystone2: use detected ddr3a size

[Tom Rini]

On Tue, Jun 23, 2015 at 08:24:23AM -0400, Vitaly Andrianov wrote:
> 
> 
> 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 <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?
> >
> 
> 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.

I want to answer your last two points together.  First, gd is nominally
used for things like "I cannot get this data again after relocation" or
"I must have this information passed around prior to relocation".

Next, there's already an infrastructure in the generic code and PowerPC
code for saying that we have a 32bit CPU but extra large memory.  Given
how the arch/*/lib/board.c consolidation went I'm not sure that there's
much more work needed to enable this for ARM than fixing up
arch/arm/include/asm/types.h (based on the PowerPC one) and setting a
few CONFIG options (which need to be Kconfig'd).

All that would then leave is needing to add a generic way to make sure
that we carve up the memory node in the DT to exclude what needs
excluding.  And this too is a very generic problem as many SoCs need
some way to cut out DDR and use it for some DSP or another.

So yes, I'd like to see this problem solved in a way that will benefit
not just Keystone 2 but other TI parts (I have vague recollections of
other parts having large amounts of memory being possible) and other
vendors too (which is good, since you can leverage a large amount of
work they already did, to make it happen :)).  Thanks!

-- 
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/20150623/0d43522e/attachment.sig>