[U-Boot] [PATCH u-boot-marvell v3 09/10] board: turris_mox: Support 1 GB version of Turris Mox

[Stefan Roese <sr@denx.de>]

Hi Marek,

On 11.12.18 14:59, Marek Behún wrote:
> get_ram_size does not work correctly on Mox. On a 512 MiB board it
> detects 1024 MiB of RAM, because on the 512 MiB RAM chip the topmost
> address bit is simply ignored and the RAM wraps - on
> 0x20000000-0x40000000 CPU sees the same data as on 0x0-0x20000000.

That's what get_ram_size() does: It does detect such aliases when
the same memory is mapped at multiple areas (power of 2). Did you
give it a try with a max value of 1024 MiB? It should return
512 on such boards.
  
> ATF does not run RAM size determining code either, it just gets RAM
> size from a register, this register is written before ATF by BootROM
> and we have done it so that there is always 1 GB so that we could use
> same secure firmware image for all Moxes. I tried to change this
> register in secure firmware, but this lead to Synchornous Abort events
> in U-Boot.
> 
> Maybe we could move the dram_init funcitons from arm64-common.c to
> specific board files, or maybe we could declare them __weak in
> arm64-common.c and turris_mox can then redefine them.
> 
> Would that be OK with you?

Please fist check if get_ram_size() can't be used.

Thanks,
Stefan
  
> Marek
> 
> On Thu, 29 Nov 2018 14:07:59 +0100
> Stefan Roese <sr@denx.de> wrote:
> 
>> On 20.11.18 13:04, Marek Behún wrote:
>>> Depending on the data in the OTP memory, differentiate between the
>>> 512 MiB and 1 GiB versions of Turris Mox and report these RAM sizes
>>> in dram_init and dram_init_banksize.
>>>
>>> Signed-off-by: Marek Behún <marek.behun@nic.cz>
>>> ---
>>>    arch/arm/mach-mvebu/arm64-common.c   |  7 ++++++-
>>>    board/CZ.NIC/turris_mox/turris_mox.c | 27
>>> +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1
>>> deletion(-)
>>>
>>> diff --git a/arch/arm/mach-mvebu/arm64-common.c
>>> b/arch/arm/mach-mvebu/arm64-common.c index f47273fde9..5e6ac9fc4a
>>> 100644 --- a/arch/arm/mach-mvebu/arm64-common.c
>>> +++ b/arch/arm/mach-mvebu/arm64-common.c
>>> @@ -43,8 +43,12 @@ const struct mbus_dram_target_info
>>> *mvebu_mbus_dram_info(void) return NULL;
>>>    }
>>>    
>>> -/* DRAM init code ... */
>>> +/*
>>> + * DRAM init code ...
>>> + * Turris Mox defines this itself, depending on data in burned
>>> eFuses
>>> + */
>>>    
>>> +#ifndef CONFIG_TARGET_TURRIS_MOX
>>>    int dram_init_banksize(void)
>>>    {
>>>    	fdtdec_setup_memory_banksize();
>>> @@ -59,6 +63,7 @@ int dram_init(void)
>>>    
>>>    	return 0;
>>>    }
>>> +#endif /* !CONFIG_TARGET_TURRIS_MOX */
>>
>> 2 Problems with this:
>>
>> a)
>> This does not apply any more with the latest changes in mainline.
>>
>> b)
>> I really don't like #ifdef's here in this common code. Can you not
>> get rid of this somehow? Isn't the turris_mox also using ATF and
>> will read the RAM size from there?
>>
>> U-Boot still has the good old get_ram_size() function, which can
>> easily auto-detect 512MiB vs 1GiB when run with 1GiB as parameter.
>>
>> Thanks,
>> Stefan
>>
>>>    
>>>    int arch_cpu_init(void)
>>>    {
>>> diff --git a/board/CZ.NIC/turris_mox/turris_mox.c
>>> b/board/CZ.NIC/turris_mox/turris_mox.c index 89b3cd2ce0..9aa2fc004d
>>> 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c
>>> +++ b/board/CZ.NIC/turris_mox/turris_mox.c
>>> @@ -14,6 +14,7 @@
>>>    #include <linux/string.h>
>>>    #include <linux/libfdt.h>
>>>    #include <fdt_support.h>
>>> +#include <environment.h>
>>>    
>>>    #ifdef CONFIG_WDT_ARMADA_37XX
>>>    #include <wdt.h>
>>> @@ -40,6 +41,32 @@
>>>    
>>>    DECLARE_GLOBAL_DATA_PTR;
>>>    
>>> +int dram_init(void)
>>> +{
>>> +	int ret, ram_size;
>>> +
>>> +	gd->ram_base = 0;
>>> +	gd->ram_size = (phys_size_t)0x20000000;
>>> +
>>> +	ret = mbox_sp_get_board_info(NULL, NULL, NULL, NULL,
>>> &ram_size);
>>> +	if (ret < 0) {
>>> +		puts("Cannot read RAM size from OTP, defaulting to
>>> 512 MiB");
>>> +	} else {
>>> +		if (ram_size == 1024)
>>> +			gd->ram_size = (phys_size_t)0x40000000;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +int dram_init_banksize(void)
>>> +{
>>> +	gd->bd->bi_dram[0].start = (phys_addr_t)0;
>>> +	gd->bd->bi_dram[0].size = gd->ram_size;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    #if defined(CONFIG_OF_BOARD_FIXUP)
>>>    int board_fix_fdt(void *blob)
>>>    {
>>>    
>>
>> Viele Grüße,
>> Stefan
>>
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de