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

[Michal Simek]

Hi Simon and Tom,

On 19.7.2017 11:05, Simon Glass wrote:
> 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>

Simon: thanks.

Tom: Any comment or do you know better way how to support this?

Thanks,
Michal