[U-Boot] [PATCH v3 01/11] armv8: Add global variable resv_ram

[Simon Glass]

Hi York,

On 3 March 2017 at 09:38, york sun <york.sun at nxp.com> wrote:
>
> On 03/02/2017 08:53 PM, Simon Glass wrote:
> > Hi York,
> >
> > On 1 March 2017 at 12:32, York Sun <york.sun at nxp.com> wrote:
> >> Use gd->arch.resv_ram to track reserved memory allocation.
> >>
> >> Signed-off-by: York Sun <york.sun at nxp.com>
> >> ---
> >>
> >> Changes in v3: None
> >> Changes in v2: None
> >>
> >>  arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 6 ++++++
> >>  arch/arm/include/asm/global_data.h        | 3 +++
> >>  cmd/bdinfo.c                              | 4 ++++
> >>  3 files changed, 13 insertions(+)
> >>
> >> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> >> index adccdf1..a40556f 100644
> >> --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> >> +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> >> @@ -273,6 +273,12 @@ config SYS_FSL_SDHC_CLK_DIV
> >>           clock, in another word SDHC_clk = Platform_clk / this_divider.
> >>  endmenu
> >>
> >> +config RESV_RAM_TOP
> >> +       bool
> >> +       help
> >> +         Reserve memory from the top, tracked by gd->arch.resv_ram. It's up
> >> +         to implementation to allow access to this reserved memory or not.
> >
> > This is not sufficiently descriptive IMO. What is it used for? What do
> > you mean by 'from the top'? What is the top?
>
> Simon,
>
> How about renaming it to RESV_RAM?
>
> config RESV_RAM
>          bool
>         help
>           Reserve memory from the RAM, tracked by gd->arch.resv_ram.
>            This reserved RAM can be used by special driver that resides
>            in memory after U-Boot exits. It's up to the implementation
>            to allocate and allow access to this reserved memory. For
>            example, the reserved RAM can be at the high end of physical
>            memory. The reserved RAM may be excluded from the memory
>            bank(s) passed to OS, or marked as reserved.

OK, or perhaps RESERVED_RAM.

In the description you say 'can be' and 'may me'. What determines
whether it is or not?

>
> >
> >> +
> >>  config SYS_FSL_ERRATUM_A008336
> >>         bool
> >>
> >> diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
> >> index aee87cd..b1fc410 100644
> >> --- a/arch/arm/include/asm/global_data.h
> >> +++ b/arch/arm/include/asm/global_data.h
> >> @@ -59,6 +59,9 @@ struct arch_global_data {
> >>         phys_addr_t secure_ram;
> >>         unsigned long tlb_allocated;
> >>  #endif
> >> +#ifdef CONFIG_RESV_RAM_TOP
> >> +       phys_addr_t resv_ram;
> >
> > Please add a comment here explaining what it is for, or referencing something.
>
> I will add a comment as
> /*
>   * Reserved RAM for memory resident eg. Management Complex (MC) driver
>   * which continues to run after U-Boot exits.
>   */
>
> >
> >> +#endif
> >>
> >>  #ifdef CONFIG_ARCH_OMAP2
> >>         u32 omap_boot_device;
> >> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
> >> index ae3027a..0c5fa56 100644
> >> --- a/cmd/bdinfo.c
> >> +++ b/cmd/bdinfo.c
> >> @@ -392,6 +392,10 @@ static int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc,
> >>                           gd->arch.secure_ram & MEM_RESERVE_SECURE_ADDR_MASK);
> >>         }
> >>  #endif
> >> +#ifdef CONFIG_RESV_RAM_TOP
> >> +       if (gd->arch.resv_ram)
> >> +               print_num("Reserved ram", gd->arch.resv_ram);
> >> +#endif
> >>  #if defined(CONFIG_CMD_NET) && !defined(CONFIG_DM_ETH)
> >>         print_eths();
> >>  #endif
> >> --
>
> This patch set includes the effort to rewrite the reservation code we
> discussed a year ago
>
> https://lists.denx.de/pipermail/u-boot/2015-December/236974.html
> https://lists.denx.de/pipermail/u-boot/2015-December/236979.html
> https://lists.denx.de/pipermail/u-boot/2015-December/236996.html
>
> York

Regards,
Simon