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

From: Ovidiu Panait <ovidiu.panait@windriver.com>
To: u-boot@lists.denx.de
Subject: [PATCH v3 11/17] board_f: ppc: Factor out ppc-specific bdinfo setup
Date: Wed, 29 Jul 2020 09:41:01 +0300	[thread overview]
Message-ID: <172aa31f-4f2b-0585-b0f5-bea0036f14de@windriver.com> (raw)
In-Reply-To: <CAPnjgZ0GnfMz5Pae2wuozxoSZrKT+xCe3K0QV2BhH1cDy0VG2w@mail.gmail.com>

Hi Simon,

On 28.07.2020 21:58, Simon Glass wrote:
> Hi Ovidiu,
>
> On Tue, 21 Jul 2020 at 10:43, Ovidiu Panait <ovidiu.panait@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@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@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@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.

In the meantime, I have sent the v4 for this series, containing the 
above mentioned changes:

https://lists.denx.de/pipermail/u-boot/2020-July/421283.html

Thanks,

Ovidiu

> Regards,
> Simon