[U-Boot] [RFC PATCH] common: board_f: Make reserve_mmu a weak function

[Simon Glass]

Hi Michal,

On 18 July 2017 at 07:59, Michal Simek <michal.simek at xilinx.com> wrote:
> Hi Simon,
>
> On 18.7.2017 16:50, Simon Glass wrote:
>> Hi Michal,
>>
>> On 18 July 2017 at 07:23, Michal Simek <michal.simek at xilinx.com> wrote:
>>> Hi Simon,
>>>
>>> On 18.7.2017 16:01, Simon Glass wrote:
>>>> Hi Michal,
>>>>
>>>> On 13 July 2017 at 06:50, Michal Simek <michal.simek at xilinx.com> wrote:
>>>>> From: Siva Durga Prasad Paladugu <siva.durga.paladugu at xilinx.com>
>>>>>
>>>>> Make reserve_mmu a weak so that it provides an option
>>>>> to customize this routine as per platform need
>>>>
>>>> Why do you need this? Can we instead have the generic code do the
>>>> right thing? Or can we use reserve_arch() to handle this?
>>>
>>> reserve_mmu is called just for ARM.
>>
>> What arch are you using?
>
> arm64 - a53s
>
>>
>>> What we need to do is support configuration where we have small memory
>>> OCM and we need to put mmu table to different location (TCM) because it
>>> is simply big and it can't be placed where it is placed now.
>>
>> What is TCM? Is this the internal SRAM?
>
> It is internal sram which is used mostly by R5s in the system but a53
> has also access to it.
>
>>
>> I don't understand 'simply big'. Are you saying it is too big to go
>> into main memory?
>
> These configuration can't use DDR. One of that configuration which we
> are using is DDR memory test. It means you run from internal sram (256kB
> OCM + 256k TCM).
> Another configuration is for loading stuff to EMMC on different boards.
> It means you don't need to setup DDR for generic emmc programming.
>
> All these configurations are using console over DCC - arm jtag uart.

OK I see.

>
>>
>>>
>>> Based on looking at code the key question is if we can call reserve_mmu
>>> at that time when reserve_arch is called now.
>>> On zynqmp I can't see any issue to allocate tlb on the stack and call it
>>> from it. The question is if this is valid for all arms.
>>>
>>> This is just a small hack I have created to move it to stack.
>>>
>>>  diff --git a/common/board_f.c b/common/board_f.c
>>>  index 88e20e0168b2..32386c957962 100644
>>>  --- a/common/board_f.c
>>>  +++ b/common/board_f.c
>>>  @@ -433,13 +433,13 @@ __weak int reserve_mmu(void)
>>>   {
>>>          /* reserve TLB table */
>>>          gd->arch.tlb_size = PGTABLE_SIZE;
>>>  -       gd->relocaddr -= gd->arch.tlb_size;
>>>  +       gd->start_addr_sp -= gd->arch.tlb_size;
>>>
>>>          /* round down to next 64 kB limit */
>>>  -       gd->relocaddr &= ~(0x10000 - 1);
>>>  +       gd->start_addr_sp &= ~(0x10000 - 1);
>>>
>>>  -       gd->arch.tlb_addr = gd->relocaddr;
>>>  -       debug("TLB table from %08lx to %08lx\n", gd->arch.tlb_addr,
>>>  +       gd->arch.tlb_addr = gd->start_addr_sp;
>>>  +       printf("TLB table from %08lx to %08lx\n", gd->arch.tlb_addr,
>>>                gd->arch.tlb_addr + gd->arch.tlb_size);
>>>
>>>   #ifdef CONFIG_SYS_MEM_RESERVE_SECURE
>>>  @@ -837,7 +837,7 @@ static int initf_dm(void)
>>>   /* Architecture-specific memory reservation */
>>>   __weak int reserve_arch(void)
>>>   {
>>>  -       return 0;
>>>  +       return reserve_mmu();
>>>   }
>>>
>>>   __weak int arch_cpu_init_dm(void)
>>>  @@ -998,10 +998,6 @@ static init_fnc_t init_sequence_f[] = {
>>>          reserve_pram,
>>>   #endif
>>>          reserve_round_4k,
>>>  -#if !(defined(CONFIG_SYS_ICACHE_OFF) &&
>>> defined(CONFIG_SYS_DCACHE_OFF)) && \
>>>  -               defined(CONFIG_ARM)
>>>  -       reserve_mmu,
>>>  -#endif
>>>   #ifdef CONFIG_DM_VIDEO
>>>          reserve_video,
>>>   #else
>>>
>>> For us we can't just rewrite tlb addresses as we want to do because we
>>> can't allow reserve_mmu to allocate 64kB alied PGTABLE_SIZE in
>>> relocation structure.
>>
>> Why not?
>
> Because we need to fit to 256kB of memory also with relocation and also
> space for data. And IIRC MMU table size is 64kB.
> Another option could be to disable relocation.

I don't have a better option for this case unfortunately. I did send a
board uclass series a while back which could have handled this, but
for now I think what you have is reasonable.

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon