[PATCH v3 11/17] board_f: ppc: Factor out ppc-specific bdinfo setup

[Simon Glass]

Hi Ovidiu,

On Tue, 21 Jul 2020 at 10:43, Ovidiu Panait <ovidiu.panait at windriver.com> wrote:
>
> Hi Simon,
>
> On 21.07.2020 17:17, Simon Glass wrote:
> > On Mon, 20 Jul 2020 at 08:20, Ovidiu Panait <ovidiu.panait at windriver.com> wrote:
> >> Factor out ppc-specific bdinfo setup from generic init sequence to
> >> arch_setup_bdinfo in arch/powerpc/lib/bdinfo.c.
> >>
> >> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
> >> ---
> >>
> >>   arch/powerpc/lib/bdinfo.c | 25 +++++++++++++++++++++++++
> >>   common/board_f.c          | 36 ++----------------------------------
> >>   2 files changed, 27 insertions(+), 34 deletions(-)
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >
> > Could this be a way to remove the weak SDRAM function?
>
> I think the SDRAM weak function can be eliminated by overwriting the
> values set in common/board_f.c using the xtensa-specific
> arch_setup_bdinfo (arch/xtensa/lib/bdinfo.c), as you suggested.
>
>
> I plan to replace the weak SDRAM function patch with this one:
>
> iff --git a/arch/Kconfig b/arch/Kconfig
> index 9be02d1319..4d9557959c 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -10,7 +10,6 @@ choice
>
>   config ARC
>       bool "ARC architecture"
> -    select ARCH_EARLY_INIT_R
>       select ARC_TIMER
>       select CLK
>       select HAVE_PRIVATE_LIBGCC
> diff --git a/arch/arc/lib/cpu.c b/arch/arc/lib/cpu.c
> index 27b5832a0c..d66a8c867a 100644
> --- a/arch/arc/lib/cpu.c
> +++ b/arch/arc/lib/cpu.c
> @@ -25,13 +25,6 @@ int arch_cpu_init(void)
>       return 0;
>   }
>
> -int arch_early_init_r(void)
> -{
> -    gd->bd->bi_memstart = CONFIG_SYS_SDRAM_BASE;
> -    gd->bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
> -    return 0;
> -}
> -
>   /* This is a dummy function on arc */
>   int dram_init(void)
>   {
> diff --git a/arch/xtensa/lib/bdinfo.c b/arch/xtensa/lib/bdinfo.c
> new file mode 100644
> index 0000000000..b3ab7227aa
> --- /dev/null
> +++ b/arch/xtensa/lib/bdinfo.c
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * XTENSA-specific information for the 'bd' command
> + *
> + * (C) Copyright 2003
> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +int arch_setup_bdinfo(void)
> +{
> +    struct bd_info *bd = gd->bd;
> +
> +    gd->bd->bi_memstart = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
> +    gd->bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
> +}
> diff --git a/board/cadence/xtfpga/xtfpga.c b/board/cadence/xtfpga/xtfpga.c
> index 2869e5cf68..4b49b6e5c8 100644
> --- a/board/cadence/xtfpga/xtfpga.c
> +++ b/board/cadence/xtfpga/xtfpga.c
> @@ -51,9 +51,6 @@ int checkboard(void)
>
>   int dram_init_banksize(void)
>   {
> -    gd->bd->bi_memstart = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
> -    gd->bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
> -
>       return 0;
>   }
>
> diff --git a/common/board_f.c b/common/board_f.c
> index 4356431488..3b11f08725 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -605,6 +605,11 @@ __weak int arch_setup_bdinfo(void)
>
>   int setup_bdinfo(void)
>   {
> +    struct bd_info *bd = gd->bd;
> +
> +    bd->bi_memstart = gd->ram_base;  /* start of memory */
> +    bd->bi_memsize = gd->ram_size;   /* size in bytes */
> +
>       return arch_setup_bdinfo();
>   }
>
> @@ -614,12 +619,6 @@ static int setup_board_part1(void)
>   {
>       struct bd_info *bd = gd->bd;
>
> -    /*
> -     * Save local variables to board info struct
> -     */
> -    bd->bi_memstart = CONFIG_SYS_SDRAM_BASE;    /* start of memory */
> -    bd->bi_memsize = gd->ram_size;            /* size in bytes */
> -
>   #ifdef CONFIG_SYS_SRAM_BASE
>       bd->bi_sramstart = CONFIG_SYS_SRAM_BASE;    /* start of SRAM */
>       bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE;        /* size  of SRAM */
>
> Do you think this is a better approach?

Yes it looks better to me.

Regards,
Simon