[U-Boot] [PATCH v3] armv8: fsl-layerscale: Rewrite reserving memory for MC and debug server

[York Sun <yorksun@freescale.com>]

On 12/07/2015 09:54 AM, Simon Glass wrote:
> Hi York,
> 
> On 7 December 2015 at 10:43, York Sun <yorksun@freescale.com> wrote:
>>
>>
>>
>> On 11/22/2015 09:53 AM, York Sun wrote:
>>>
>>>
>>> On 11/22/2015 08:11 AM, Simon Glass wrote:
>>>> Hi York,
>>>>
>>
>> <snip>
>>
>>>>> diff --git a/common/board_f.c b/common/board_f.c
>>>>> index 8061105..2fd1c21 100644
>>>>> --- a/common/board_f.c
>>>>> +++ b/common/board_f.c
>>>>> @@ -316,6 +316,15 @@ __weak ulong board_get_usable_ram_top(ulong total_size)
>>>>>         return gd->ram_top;
>>>>>  }
>>>>>
>>>>> +__weak phys_size_t board_reserve_ram_top(phys_size_t ram_size)
>>>>> +{
>>>>> +#ifdef CONFIG_SYS_MEM_TOP_HIDE
>>>>> +       return ram_size - CONFIG_SYS_MEM_TOP_HIDE;
>>>>> +#else
>>>>> +       return ram_size;
>>>>> +#endif
>>>>> +}
>>>>> +
>>>>>  static int setup_dest_addr(void)
>>>>>  {
>>>>>         debug("Monitor len: %08lX\n", gd->mon_len);
>>>>> @@ -332,19 +341,17 @@ static int setup_dest_addr(void)
>>>>>          */
>>>>>         gd->secure_ram = gd->ram_size;
>>>>>  #endif
>>>>> -#if defined(CONFIG_SYS_MEM_TOP_HIDE)
>>>>>         /*
>>>>>          * Subtract specified amount of memory to hide so that it won't
>>>>>          * get "touched" at all by U-Boot. By fixing up gd->ram_size
>>>>>          * the Linux kernel should now get passed the now "corrected"
>>>>> -        * memory size and won't touch it either. This should work
>>>>> -        * for arch/ppc and arch/powerpc. Only Linux board ports in
>>>>> -        * arch/powerpc with bootwrapper support, that recalculate the
>>>>> -        * memory size from the SDRAM controller setup will have to
>>>>> -        * get fixed.
>>>>> +        * memory size and won't touch it either. This has been used
>>>>> +        * by arch/powerpc exclusively. Now ARMv8 takes advantage of
>>>>> +        * thie mechanism. If memory is split into banks, addresses
>>>>> +        * need to be calculated.
>>>>>          */
>>>>> -       gd->ram_size -= CONFIG_SYS_MEM_TOP_HIDE;
>>>>> -#endif
>>>>> +       gd->ram_size = board_reserve_ram_top(gd->ram_size);
>>>>> +
>>>>>  #ifdef CONFIG_SYS_SDRAM_BASE
>>>>>         gd->ram_top = CONFIG_SYS_SDRAM_BASE;
>>>>>  #endif
>>>>
>>>> Sorry I didn't notice this patch before...
>>>>
>>>> Can you use the existing board_get_usable_ram_top() for this?
>>>>
>>>
>>> Simon,
>>>
>>> No. The "top" is not necessarily the end of memory. It is the top of
>>> CONFIG_SYS_SDRAM_BASE + get_effective_memsize(). I am trying to avoid reserving
>>> memory in the middle.
>>>
>>> I am using the same way as CONFIG_SYS_MEM_TOP_HIDE, but rewriting it with a weak
>>> function.
>>>
>>
>> Simon,
>>
>> If you are satisfied with my explanation, I am considering to merge this patch.
> 
> I think this function is getting pretty complicated. Probably we need
> to create a struct with the various parameters in it, and have a
> single board function that is called to possibly make adjustments. At
> present it is really hard to figure out what is going on and this
> patch makes things worse.
> 
> I'm fine with that happening later if you want to merge this patch
> now. But I think it needs a look.
> 

Simon,

I agree we need look into this. It started to get complicated when we have more
than 32-bit physical address. It got more complicated when the memory is divided
into several banks (for ARMv8 case). This patch is needed if I merge another two
patches to make DDR non-secure, which fixes some issue with non-secure masters.
Let's spend some time after the holidays to restructure this.

York