[v5,03/10] board_f: Add default values for bi_dram[] in dram_init_banksize()

Remove the bi_memstart / bi_memsize assignment in setup_bdinfo() and
make sure, that bd_dram[] is always configured in the weak default
implementation of dram_init_banksize(), when CONFIG_SYS_SDRAM_BASE is
not set.

Signed-off-by: Stefan Roese <sr@denx.de>

---

(no changes since v4)

Changes in v4:
- New patch

 common/board_f.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Am Freitag, den 14.08.2020, 08:33 +0200 schrieb Stefan Roese:
> Remove the bi_memstart / bi_memsize assignment in setup_bdinfo() and
> make sure, that bd_dram[] is always configured in the weak default
> implementation of dram_init_banksize(), when CONFIG_SYS_SDRAM_BASE is
> not set.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> 
> ---
> 
> (no changes since v4)
> 
> Changes in v4:
> - New patch
> 
>  common/board_f.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/common/board_f.c b/common/board_f.c
> index dd9a5220e1..bfbeda29b2 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -240,6 +240,9 @@ __weak int dram_init_banksize(void)
>  #if defined(CONFIG_SYS_SDRAM_BASE)
>  	gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>  	gd->bd->bi_dram[0].size = get_effective_memsize();
> +#else
> +	gd->bd->bi_dram[0].start = gd->ram_base;
> +	gd->bd->bi_dram[0].size = gd->ram_size;
>  #endif

hm, I can only find four locations where gd->ram_base is assigned:

$ git grep -C1 -n 'gd->ram_base ='

arch/arm/mach-uniphier/dram_init.c-254-         if (!valid_bank_found)
arch/arm/mach-uniphier/dram_init.c:255:                 gd->ram_base =
dram_map[i].base;
arch/arm/mach-uniphier/dram_init.c-256-
--
board/broadcom/bcmns3/ns3.c-168-         */
board/broadcom/bcmns3/ns3.c:169:        gd->ram_base =
(phys_size_t)(BCM_NS3_MEM_END - SZ_16M);
board/broadcom/bcmns3/ns3.c-170-        gd->ram_size = (unsigned
long)SZ_16M;
--
common/board_f.c-350-#ifdef CONFIG_SYS_SDRAM_BASE
common/board_f.c:351:   gd->ram_base = CONFIG_SYS_SDRAM_BASE;
common/board_f.c-352-#endif
--
lib/fdtdec.c-1050-      gd->ram_size = (phys_size_t)(res.end -
res.start + 1);
lib/fdtdec.c:1051:      gd->ram_base = (unsigned long)res.start;
lib/fdtdec.c-1052-      debug("%s: Initial DRAM size %llx\n", __func__,

So you already have the check for CONFIG_SYS_SDRAM_BASE in
common/board_f.c. And without CONFIG_SYS_SDRAM_BASE defined gd-
>ram_base should be 0.


Shouldn't it be enough to implement just this? 

__weak int dram_init_banksize(void)
{
	gd->bd->bi_dram[0].start = gd->ram_base;
	gd->bd->bi_dram[0].size = get_effective_memsize();

	return 0;
}

>  
>  	return 0;
> @@ -602,9 +605,6 @@ 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 */
> -
>  	if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) {
>  		bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
>  		bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE;  /* size  of SRAM */

Hi Daniel,

On 14.08.20 12:47, Daniel Schwierzeck wrote:
> Am Freitag, den 14.08.2020, 08:33 +0200 schrieb Stefan Roese:
>> Remove the bi_memstart / bi_memsize assignment in setup_bdinfo() and
>> make sure, that bd_dram[] is always configured in the weak default
>> implementation of dram_init_banksize(), when CONFIG_SYS_SDRAM_BASE is
>> not set.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>>
>> ---
>>
>> (no changes since v4)
>>
>> Changes in v4:
>> - New patch
>>
>>   common/board_f.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/board_f.c b/common/board_f.c
>> index dd9a5220e1..bfbeda29b2 100644
>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -240,6 +240,9 @@ __weak int dram_init_banksize(void)
>>   #if defined(CONFIG_SYS_SDRAM_BASE)
>>   	gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>>   	gd->bd->bi_dram[0].size = get_effective_memsize();
>> +#else
>> +	gd->bd->bi_dram[0].start = gd->ram_base;
>> +	gd->bd->bi_dram[0].size = gd->ram_size;
>>   #endif
> 
> hm, I can only find four locations where gd->ram_base is assigned:
> 
> $ git grep -C1 -n 'gd->ram_base ='
> 
> arch/arm/mach-uniphier/dram_init.c-254-         if (!valid_bank_found)
> arch/arm/mach-uniphier/dram_init.c:255:                 gd->ram_base =
> dram_map[i].base;
> arch/arm/mach-uniphier/dram_init.c-256-
> --
> board/broadcom/bcmns3/ns3.c-168-         */
> board/broadcom/bcmns3/ns3.c:169:        gd->ram_base =
> (phys_size_t)(BCM_NS3_MEM_END - SZ_16M);
> board/broadcom/bcmns3/ns3.c-170-        gd->ram_size = (unsigned
> long)SZ_16M;
> --
> common/board_f.c-350-#ifdef CONFIG_SYS_SDRAM_BASE
> common/board_f.c:351:   gd->ram_base = CONFIG_SYS_SDRAM_BASE;
> common/board_f.c-352-#endif
> --
> lib/fdtdec.c-1050-      gd->ram_size = (phys_size_t)(res.end -
> res.start + 1);
> lib/fdtdec.c:1051:      gd->ram_base = (unsigned long)res.start;
> lib/fdtdec.c-1052-      debug("%s: Initial DRAM size %llx\n", __func__,
> 
> So you already have the check for CONFIG_SYS_SDRAM_BASE in
> common/board_f.c. And without CONFIG_SYS_SDRAM_BASE defined gd-
>> ram_base should be 0.
> 
> 
> Shouldn't it be enough to implement just this?
> 
> __weak int dram_init_banksize(void)
> {
> 	gd->bd->bi_dram[0].start = gd->ram_base;
> 	gd->bd->bi_dram[0].size = get_effective_memsize();
> 
> 	return 0;
> }

I also though about changing (simplifying) it in a similar way. But I
was not brave enough to go this far. ;)

With all your reasoning and when no further objections follow from
other developers, I'll change this patch in the next version
accordingly.

Thanks,
Stefan