diff mbox

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

Message ID 1448133669-10245-1-git-send-email-yorksun@freescale.com
State Superseded
Delegated to: York Sun
Headers show

Commit Message

York Sun Nov. 21, 2015, 7:21 p.m. UTC
MC and debug server are not board-specific. Instead of reserving
memory in each board file, a weak function is introduced in board_f.c
to replace macro CONFIG_SYS_MEM_TOP_HIDE for more flexibility.
Legacy use of this macro is still supported. Move the reservation
calculation to SoC file. Reduce debug server memory by 2MB to
make room for secure memory.

In the system with MC and debug server, the top of u-boot memory
is not the end of memory. PRAM is not used for this reservation.

Signed-off-by: York Sun <yorksun@freescale.com>

---

Changes in v3:
  Rename CONFIG_SYS_MC_RESERV_MEM_ALIGN to CONFIG_SYS_MC_RSV_MEM_ALIGN
  Check for unused CONFIG_SYS_MEM_TOP_HIDE if board_reserve_ram_top is used
  Use gd->ram_size = board_reserve_ram_top(gd->ram_size) format
  Use phys_size_t instead of ulong for ram_size calculation

Changes in v2:
  Revise commit message.

 README                                  |    6 +++---
 arch/arm/cpu/armv8/fsl-layerscape/cpu.c |   21 +++++++++++++++++++++
 board/freescale/ls2085a/ls2085a.c       |   17 -----------------
 board/freescale/ls2085aqds/ls2085aqds.c |   17 -----------------
 board/freescale/ls2085ardb/ls2085ardb.c |   17 -----------------
 common/board_f.c                        |   23 +++++++++++++++--------
 include/configs/ls2085a_common.h        |    5 ++---
 7 files changed, 41 insertions(+), 65 deletions(-)

Comments

Simon Glass Nov. 22, 2015, 4:11 p.m. UTC | #1
Hi York,

On 21 November 2015 at 12:21, York Sun <yorksun@freescale.com> wrote:
> MC and debug server are not board-specific. Instead of reserving
> memory in each board file, a weak function is introduced in board_f.c
> to replace macro CONFIG_SYS_MEM_TOP_HIDE for more flexibility.
> Legacy use of this macro is still supported. Move the reservation
> calculation to SoC file. Reduce debug server memory by 2MB to
> make room for secure memory.
>
> In the system with MC and debug server, the top of u-boot memory
> is not the end of memory. PRAM is not used for this reservation.
>
> Signed-off-by: York Sun <yorksun@freescale.com>
>
> ---
>
> Changes in v3:
>   Rename CONFIG_SYS_MC_RESERV_MEM_ALIGN to CONFIG_SYS_MC_RSV_MEM_ALIGN
>   Check for unused CONFIG_SYS_MEM_TOP_HIDE if board_reserve_ram_top is used
>   Use gd->ram_size = board_reserve_ram_top(gd->ram_size) format
>   Use phys_size_t instead of ulong for ram_size calculation
>
> Changes in v2:
>   Revise commit message.
>
>  README                                  |    6 +++---
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c |   21 +++++++++++++++++++++
>  board/freescale/ls2085a/ls2085a.c       |   17 -----------------
>  board/freescale/ls2085aqds/ls2085aqds.c |   17 -----------------
>  board/freescale/ls2085ardb/ls2085ardb.c |   17 -----------------
>  common/board_f.c                        |   23 +++++++++++++++--------
>  include/configs/ls2085a_common.h        |    5 ++---
>  7 files changed, 41 insertions(+), 65 deletions(-)
>
> diff --git a/README b/README
> index 61cbc82..3b80fe6 100644
> --- a/README
> +++ b/README
> @@ -3889,7 +3889,7 @@ Configuration Settings:
>                 the RAM base is not zero, or RAM is divided into banks,
>                 this variable needs to be recalcuated to get the address.
>
> -- CONFIG_SYS_MEM_TOP_HIDE (PPC only):
> +- CONFIG_SYS_MEM_TOP_HIDE:
>                 If CONFIG_SYS_MEM_TOP_HIDE is defined in the board config header,
>                 this specified memory area will get subtracted from the top
>                 (end) of RAM and won't get "touched" at all by U-Boot. By
> @@ -5068,8 +5068,8 @@ This firmware often needs to be loaded during U-Boot booting.
>  - CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE
>         Define minimum DDR size required for debug server image
>
> -- CONFIG_SYS_MEM_TOP_HIDE_MIN
> -       Define minimum DDR size to be hided from top of the DDR memory
> +- CONFIG_SYS_MC_RSV_MEM_ALIGN
> +       Define alignment of reserved memory MC requires
>
>  Reproducible builds
>  -------------------
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> index 501feb3..788e95d 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> @@ -628,3 +628,24 @@ void reset_cpu(ulong addr)
>         val |= 0x02;
>         scfg_out32(rstcr, val);
>  }
> +
> +phys_size_t board_reserve_ram_top(phys_size_t ram_size)
> +{
> +       phys_size_t ram_top = ram_size;
> +
> +#ifdef CONFIG_SYS_MEM_TOP_HIDE
> +#error CONFIG_SYS_MEM_TOP_HIDE not to be used together with this function
> +#endif
> +/* Carve the Debug Server private DRAM block from the end of DRAM */
> +#ifdef CONFIG_FSL_DEBUG_SERVER
> +       ram_top -= debug_server_get_dram_block_size();
> +#endif
> +
> +/* Carve the MC private DRAM block from the end of DRAM */
> +#ifdef CONFIG_FSL_MC_ENET
> +       ram_top -= mc_get_dram_block_size();
> +       ram_top &= ~(CONFIG_SYS_MC_RSV_MEM_ALIGN - 1);
> +#endif
> +
> +       return ram_top;
> +}
> diff --git a/board/freescale/ls2085a/ls2085a.c b/board/freescale/ls2085a/ls2085a.c
> index 27481e2..6f4c3d4 100644
> --- a/board/freescale/ls2085a/ls2085a.c
> +++ b/board/freescale/ls2085a/ls2085a.c
> @@ -66,23 +66,6 @@ int arch_misc_init(void)
>  }
>  #endif
>
> -unsigned long get_dram_size_to_hide(void)
> -{
> -       unsigned long dram_to_hide = 0;
> -
> -/* Carve the Debug Server private DRAM block from the end of DRAM */
> -#ifdef CONFIG_FSL_DEBUG_SERVER
> -       dram_to_hide += debug_server_get_dram_block_size();
> -#endif
> -
> -/* Carve the MC private DRAM block from the end of DRAM */
> -#ifdef CONFIG_FSL_MC_ENET
> -       dram_to_hide += mc_get_dram_block_size();
> -#endif
> -
> -       return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
> -}
> -
>  int board_eth_init(bd_t *bis)
>  {
>         int error = 0;
> diff --git a/board/freescale/ls2085aqds/ls2085aqds.c b/board/freescale/ls2085aqds/ls2085aqds.c
> index b02d6e8..8898cc3 100644
> --- a/board/freescale/ls2085aqds/ls2085aqds.c
> +++ b/board/freescale/ls2085aqds/ls2085aqds.c
> @@ -251,23 +251,6 @@ int arch_misc_init(void)
>  }
>  #endif
>
> -unsigned long get_dram_size_to_hide(void)
> -{
> -       unsigned long dram_to_hide = 0;
> -
> -/* Carve the Debug Server private DRAM block from the end of DRAM */
> -#ifdef CONFIG_FSL_DEBUG_SERVER
> -       dram_to_hide += debug_server_get_dram_block_size();
> -#endif
> -
> -/* Carve the MC private DRAM block from the end of DRAM */
> -#ifdef CONFIG_FSL_MC_ENET
> -       dram_to_hide += mc_get_dram_block_size();
> -#endif
> -
> -       return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
> -}
> -
>  #ifdef CONFIG_FSL_MC_ENET
>  void fdt_fixup_board_enet(void *fdt)
>  {
> diff --git a/board/freescale/ls2085ardb/ls2085ardb.c b/board/freescale/ls2085ardb/ls2085ardb.c
> index 18953b8..efddf74 100644
> --- a/board/freescale/ls2085ardb/ls2085ardb.c
> +++ b/board/freescale/ls2085ardb/ls2085ardb.c
> @@ -217,23 +217,6 @@ int arch_misc_init(void)
>  }
>  #endif
>
> -unsigned long get_dram_size_to_hide(void)
> -{
> -       unsigned long dram_to_hide = 0;
> -
> -/* Carve the Debug Server private DRAM block from the end of DRAM */
> -#ifdef CONFIG_FSL_DEBUG_SERVER
> -       dram_to_hide += debug_server_get_dram_block_size();
> -#endif
> -
> -/* Carve the MC private DRAM block from the end of DRAM */
> -#ifdef CONFIG_FSL_MC_ENET
> -       dram_to_hide += mc_get_dram_block_size();
> -#endif
> -
> -       return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
> -}
> -
>  #ifdef CONFIG_FSL_MC_ENET
>  void fdt_fixup_board_enet(void *fdt)
>  {
> 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?

Also I think the common code change should go in its own patch.

> diff --git a/include/configs/ls2085a_common.h b/include/configs/ls2085a_common.h
> index 0011e72..aba878e 100644
> --- a/include/configs/ls2085a_common.h
> +++ b/include/configs/ls2085a_common.h
> @@ -193,10 +193,9 @@ unsigned long long get_qixis_addr(void);
>   * 512MB aligned, so the min size to hide is 512MB.
>   */
>  #if defined(CONFIG_FSL_MC_ENET) || defined(CONFIG_FSL_DEBUG_SERVER)
> -#define CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE    (256UL * 1024 * 1024)
> +#define CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE    (254UL * 1024 * 1024)
>  #define CONFIG_SYS_LS_MC_DRAM_BLOCK_MIN_SIZE           (256UL * 1024 * 1024)
> -#define CONFIG_SYS_MEM_TOP_HIDE_MIN                    (512UL * 1024 * 1024)
> -#define CONFIG_SYS_MEM_TOP_HIDE                get_dram_size_to_hide()
> +#define CONFIG_SYS_MC_RSV_MEM_ALIGN                    (512UL * 1024 * 1024)
>  #endif
>
>  /* PCIe */
> --
> 1.7.9.5
>

Regards,
Simon
York Sun Nov. 22, 2015, 5:53 p.m. UTC | #2
On 11/22/2015 08:11 AM, Simon Glass wrote:
> Hi York,
> 
> On 21 November 2015 at 12:21, York Sun <yorksun@freescale.com> wrote:
>> MC and debug server are not board-specific. Instead of reserving
>> memory in each board file, a weak function is introduced in board_f.c
>> to replace macro CONFIG_SYS_MEM_TOP_HIDE for more flexibility.
>> Legacy use of this macro is still supported. Move the reservation
>> calculation to SoC file. Reduce debug server memory by 2MB to
>> make room for secure memory.
>>
>> In the system with MC and debug server, the top of u-boot memory
>> is not the end of memory. PRAM is not used for this reservation.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
>>
>> ---
>>
>> Changes in v3:
>>   Rename CONFIG_SYS_MC_RESERV_MEM_ALIGN to CONFIG_SYS_MC_RSV_MEM_ALIGN
>>   Check for unused CONFIG_SYS_MEM_TOP_HIDE if board_reserve_ram_top is used
>>   Use gd->ram_size = board_reserve_ram_top(gd->ram_size) format
>>   Use phys_size_t instead of ulong for ram_size calculation
>>
>> Changes in v2:
>>   Revise commit message.
>>
>>  README                                  |    6 +++---
>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c |   21 +++++++++++++++++++++
>>  board/freescale/ls2085a/ls2085a.c       |   17 -----------------
>>  board/freescale/ls2085aqds/ls2085aqds.c |   17 -----------------
>>  board/freescale/ls2085ardb/ls2085ardb.c |   17 -----------------
>>  common/board_f.c                        |   23 +++++++++++++++--------
>>  include/configs/ls2085a_common.h        |    5 ++---
>>  7 files changed, 41 insertions(+), 65 deletions(-)
>>
>> diff --git a/README b/README
>> index 61cbc82..3b80fe6 100644
>> --- a/README
>> +++ b/README
>> @@ -3889,7 +3889,7 @@ Configuration Settings:
>>                 the RAM base is not zero, or RAM is divided into banks,
>>                 this variable needs to be recalcuated to get the address.
>>
>> -- CONFIG_SYS_MEM_TOP_HIDE (PPC only):
>> +- CONFIG_SYS_MEM_TOP_HIDE:
>>                 If CONFIG_SYS_MEM_TOP_HIDE is defined in the board config header,
>>                 this specified memory area will get subtracted from the top
>>                 (end) of RAM and won't get "touched" at all by U-Boot. By
>> @@ -5068,8 +5068,8 @@ This firmware often needs to be loaded during U-Boot booting.
>>  - CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE
>>         Define minimum DDR size required for debug server image
>>
>> -- CONFIG_SYS_MEM_TOP_HIDE_MIN
>> -       Define minimum DDR size to be hided from top of the DDR memory
>> +- CONFIG_SYS_MC_RSV_MEM_ALIGN
>> +       Define alignment of reserved memory MC requires
>>
>>  Reproducible builds
>>  -------------------
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> index 501feb3..788e95d 100644
>> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
>> @@ -628,3 +628,24 @@ void reset_cpu(ulong addr)
>>         val |= 0x02;
>>         scfg_out32(rstcr, val);
>>  }
>> +
>> +phys_size_t board_reserve_ram_top(phys_size_t ram_size)
>> +{
>> +       phys_size_t ram_top = ram_size;
>> +
>> +#ifdef CONFIG_SYS_MEM_TOP_HIDE
>> +#error CONFIG_SYS_MEM_TOP_HIDE not to be used together with this function
>> +#endif
>> +/* Carve the Debug Server private DRAM block from the end of DRAM */
>> +#ifdef CONFIG_FSL_DEBUG_SERVER
>> +       ram_top -= debug_server_get_dram_block_size();
>> +#endif
>> +
>> +/* Carve the MC private DRAM block from the end of DRAM */
>> +#ifdef CONFIG_FSL_MC_ENET
>> +       ram_top -= mc_get_dram_block_size();
>> +       ram_top &= ~(CONFIG_SYS_MC_RSV_MEM_ALIGN - 1);
>> +#endif
>> +
>> +       return ram_top;
>> +}
>> diff --git a/board/freescale/ls2085a/ls2085a.c b/board/freescale/ls2085a/ls2085a.c
>> index 27481e2..6f4c3d4 100644
>> --- a/board/freescale/ls2085a/ls2085a.c
>> +++ b/board/freescale/ls2085a/ls2085a.c
>> @@ -66,23 +66,6 @@ int arch_misc_init(void)
>>  }
>>  #endif
>>
>> -unsigned long get_dram_size_to_hide(void)
>> -{
>> -       unsigned long dram_to_hide = 0;
>> -
>> -/* Carve the Debug Server private DRAM block from the end of DRAM */
>> -#ifdef CONFIG_FSL_DEBUG_SERVER
>> -       dram_to_hide += debug_server_get_dram_block_size();
>> -#endif
>> -
>> -/* Carve the MC private DRAM block from the end of DRAM */
>> -#ifdef CONFIG_FSL_MC_ENET
>> -       dram_to_hide += mc_get_dram_block_size();
>> -#endif
>> -
>> -       return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
>> -}
>> -
>>  int board_eth_init(bd_t *bis)
>>  {
>>         int error = 0;
>> diff --git a/board/freescale/ls2085aqds/ls2085aqds.c b/board/freescale/ls2085aqds/ls2085aqds.c
>> index b02d6e8..8898cc3 100644
>> --- a/board/freescale/ls2085aqds/ls2085aqds.c
>> +++ b/board/freescale/ls2085aqds/ls2085aqds.c
>> @@ -251,23 +251,6 @@ int arch_misc_init(void)
>>  }
>>  #endif
>>
>> -unsigned long get_dram_size_to_hide(void)
>> -{
>> -       unsigned long dram_to_hide = 0;
>> -
>> -/* Carve the Debug Server private DRAM block from the end of DRAM */
>> -#ifdef CONFIG_FSL_DEBUG_SERVER
>> -       dram_to_hide += debug_server_get_dram_block_size();
>> -#endif
>> -
>> -/* Carve the MC private DRAM block from the end of DRAM */
>> -#ifdef CONFIG_FSL_MC_ENET
>> -       dram_to_hide += mc_get_dram_block_size();
>> -#endif
>> -
>> -       return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
>> -}
>> -
>>  #ifdef CONFIG_FSL_MC_ENET
>>  void fdt_fixup_board_enet(void *fdt)
>>  {
>> diff --git a/board/freescale/ls2085ardb/ls2085ardb.c b/board/freescale/ls2085ardb/ls2085ardb.c
>> index 18953b8..efddf74 100644
>> --- a/board/freescale/ls2085ardb/ls2085ardb.c
>> +++ b/board/freescale/ls2085ardb/ls2085ardb.c
>> @@ -217,23 +217,6 @@ int arch_misc_init(void)
>>  }
>>  #endif
>>
>> -unsigned long get_dram_size_to_hide(void)
>> -{
>> -       unsigned long dram_to_hide = 0;
>> -
>> -/* Carve the Debug Server private DRAM block from the end of DRAM */
>> -#ifdef CONFIG_FSL_DEBUG_SERVER
>> -       dram_to_hide += debug_server_get_dram_block_size();
>> -#endif
>> -
>> -/* Carve the MC private DRAM block from the end of DRAM */
>> -#ifdef CONFIG_FSL_MC_ENET
>> -       dram_to_hide += mc_get_dram_block_size();
>> -#endif
>> -
>> -       return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
>> -}
>> -
>>  #ifdef CONFIG_FSL_MC_ENET
>>  void fdt_fixup_board_enet(void *fdt)
>>  {
>> 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.

York
York Sun Dec. 7, 2015, 5:43 p.m. UTC | #3
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.

York
Simon Glass Dec. 7, 2015, 5:54 p.m. UTC | #4
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.

Regards,
Simon
York Sun Dec. 7, 2015, 6:03 p.m. UTC | #5
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
Simon Glass Dec. 7, 2015, 6:08 p.m. UTC | #6
Hi York,

On 7 December 2015 at 11:03, York Sun <yorksun@freescale.com> wrote:
>
>
> 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.

OK thanks. Please send an email or a patch when you have some ideas.

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

Regards,
Simon
diff mbox

Patch

diff --git a/README b/README
index 61cbc82..3b80fe6 100644
--- a/README
+++ b/README
@@ -3889,7 +3889,7 @@  Configuration Settings:
 		the RAM base is not zero, or RAM is divided into banks,
 		this variable needs to be recalcuated to get the address.
 
-- CONFIG_SYS_MEM_TOP_HIDE (PPC only):
+- CONFIG_SYS_MEM_TOP_HIDE:
 		If CONFIG_SYS_MEM_TOP_HIDE is defined in the board config header,
 		this specified memory area will get subtracted from the top
 		(end) of RAM and won't get "touched" at all by U-Boot. By
@@ -5068,8 +5068,8 @@  This firmware often needs to be loaded during U-Boot booting.
 - CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE
 	Define minimum DDR size required for debug server image
 
-- CONFIG_SYS_MEM_TOP_HIDE_MIN
-	Define minimum DDR size to be hided from top of the DDR memory
+- CONFIG_SYS_MC_RSV_MEM_ALIGN
+	Define alignment of reserved memory MC requires
 
 Reproducible builds
 -------------------
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
index 501feb3..788e95d 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
@@ -628,3 +628,24 @@  void reset_cpu(ulong addr)
 	val |= 0x02;
 	scfg_out32(rstcr, val);
 }
+
+phys_size_t board_reserve_ram_top(phys_size_t ram_size)
+{
+	phys_size_t ram_top = ram_size;
+
+#ifdef CONFIG_SYS_MEM_TOP_HIDE
+#error CONFIG_SYS_MEM_TOP_HIDE not to be used together with this function
+#endif
+/* Carve the Debug Server private DRAM block from the end of DRAM */
+#ifdef CONFIG_FSL_DEBUG_SERVER
+	ram_top -= debug_server_get_dram_block_size();
+#endif
+
+/* Carve the MC private DRAM block from the end of DRAM */
+#ifdef CONFIG_FSL_MC_ENET
+	ram_top -= mc_get_dram_block_size();
+	ram_top &= ~(CONFIG_SYS_MC_RSV_MEM_ALIGN - 1);
+#endif
+
+	return ram_top;
+}
diff --git a/board/freescale/ls2085a/ls2085a.c b/board/freescale/ls2085a/ls2085a.c
index 27481e2..6f4c3d4 100644
--- a/board/freescale/ls2085a/ls2085a.c
+++ b/board/freescale/ls2085a/ls2085a.c
@@ -66,23 +66,6 @@  int arch_misc_init(void)
 }
 #endif
 
-unsigned long get_dram_size_to_hide(void)
-{
-	unsigned long dram_to_hide = 0;
-
-/* Carve the Debug Server private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_DEBUG_SERVER
-	dram_to_hide += debug_server_get_dram_block_size();
-#endif
-
-/* Carve the MC private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_MC_ENET
-	dram_to_hide += mc_get_dram_block_size();
-#endif
-
-	return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
-}
-
 int board_eth_init(bd_t *bis)
 {
 	int error = 0;
diff --git a/board/freescale/ls2085aqds/ls2085aqds.c b/board/freescale/ls2085aqds/ls2085aqds.c
index b02d6e8..8898cc3 100644
--- a/board/freescale/ls2085aqds/ls2085aqds.c
+++ b/board/freescale/ls2085aqds/ls2085aqds.c
@@ -251,23 +251,6 @@  int arch_misc_init(void)
 }
 #endif
 
-unsigned long get_dram_size_to_hide(void)
-{
-	unsigned long dram_to_hide = 0;
-
-/* Carve the Debug Server private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_DEBUG_SERVER
-	dram_to_hide += debug_server_get_dram_block_size();
-#endif
-
-/* Carve the MC private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_MC_ENET
-	dram_to_hide += mc_get_dram_block_size();
-#endif
-
-	return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
-}
-
 #ifdef CONFIG_FSL_MC_ENET
 void fdt_fixup_board_enet(void *fdt)
 {
diff --git a/board/freescale/ls2085ardb/ls2085ardb.c b/board/freescale/ls2085ardb/ls2085ardb.c
index 18953b8..efddf74 100644
--- a/board/freescale/ls2085ardb/ls2085ardb.c
+++ b/board/freescale/ls2085ardb/ls2085ardb.c
@@ -217,23 +217,6 @@  int arch_misc_init(void)
 }
 #endif
 
-unsigned long get_dram_size_to_hide(void)
-{
-	unsigned long dram_to_hide = 0;
-
-/* Carve the Debug Server private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_DEBUG_SERVER
-	dram_to_hide += debug_server_get_dram_block_size();
-#endif
-
-/* Carve the MC private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_MC_ENET
-	dram_to_hide += mc_get_dram_block_size();
-#endif
-
-	return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
-}
-
 #ifdef CONFIG_FSL_MC_ENET
 void fdt_fixup_board_enet(void *fdt)
 {
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
diff --git a/include/configs/ls2085a_common.h b/include/configs/ls2085a_common.h
index 0011e72..aba878e 100644
--- a/include/configs/ls2085a_common.h
+++ b/include/configs/ls2085a_common.h
@@ -193,10 +193,9 @@  unsigned long long get_qixis_addr(void);
  * 512MB aligned, so the min size to hide is 512MB.
  */
 #if defined(CONFIG_FSL_MC_ENET) || defined(CONFIG_FSL_DEBUG_SERVER)
-#define CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE	(256UL * 1024 * 1024)
+#define CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE	(254UL * 1024 * 1024)
 #define CONFIG_SYS_LS_MC_DRAM_BLOCK_MIN_SIZE		(256UL * 1024 * 1024)
-#define CONFIG_SYS_MEM_TOP_HIDE_MIN			(512UL * 1024 * 1024)
-#define CONFIG_SYS_MEM_TOP_HIDE		get_dram_size_to_hide()
+#define CONFIG_SYS_MC_RSV_MEM_ALIGN			(512UL * 1024 * 1024)
 #endif
 
 /* PCIe */