[PATCH v4 5/9] xtensa: Remove arch_setup_bdinfo()
Stefan Roese
sr at denx.de
Thu Aug 13 10:15:46 CEST 2020
Hi Ovidiu,
On 13.08.20 10:09, Stefan Roese wrote:
> Hi Ovidiu,
>
> On 13.08.20 09:57, Ovidiu Panait wrote:
>> Hi Stefan,
>>
>> On 13.08.2020 08:47, Stefan Roese wrote:
>>> arch_setup_bdinfo() only configures the deprecated bi_memstart &
>>> bi_memsize values, which should not be needed any more. Lets remove
>>> this file completely.
>>>
>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>>
>>> ---
>>>
>>> Changes in v4:
>>> - New patch
>>>
>>> arch/xtensa/lib/Makefile | 2 +-
>>> arch/xtensa/lib/bdinfo.c | 22 ----------------------
>>> 2 files changed, 1 insertion(+), 23 deletions(-)
>>> delete mode 100644 arch/xtensa/lib/bdinfo.c
>>>
>>> diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
>>> index ceee59b9bd..c59df7d372 100644
>>> --- a/arch/xtensa/lib/Makefile
>>> +++ b/arch/xtensa/lib/Makefile
>>> @@ -5,4 +5,4 @@
>>> obj-$(CONFIG_CMD_BOOTM) += bootm.o
>>> -obj-y += cache.o misc.o relocate.o time.o bdinfo.o
>>> +obj-y += cache.o misc.o relocate.o time.o
>>> diff --git a/arch/xtensa/lib/bdinfo.c b/arch/xtensa/lib/bdinfo.c
>>> deleted file mode 100644
>>> index 4ec8529521..0000000000
>>> --- a/arch/xtensa/lib/bdinfo.c
>>> +++ /dev/null
>>> @@ -1,22 +0,0 @@
>>> -// 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;
>>> -
>>> - bd->bi_memstart = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
>>> - bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
>>> -
>>
>> When I did the setup_bdinfo refactoring, I realized that xtensa was
>> the only arch handling bi_{memstart,memsize} in a special way:
>>
>> bd->bi_memstart = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
>> bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
>>
>> Because xtensa uses PHYSADDR(CONFIG_SYS_SDRAM_BASE) for bi_memstart, I
>> was not able to replace it with gd->ram_base, as for all the other
>> arches.
>
> Please note that bi_memstart/size is probably not used at all on
> xtensa - only for the bdinfo cmd AFAICT.
>
>> Currently, gd->ram_base is defined in common/board_f.c, function
>> setup_dest_addr as:
>>
>> #ifdef CONFIG_SYS_SDRAM_BASE
>>> -------gd->ram_base = CONFIG_SYS_SDRAM_BASE;
>> #endif
>
> Yes, this #ifdef is ugly - I also noticed it while working on this
> patchset.
>
>> So I think the PHYSADDR() logic needs to be preserved so that the
>> patchset would not change the memory start/end logic in
>> arch/xtensa/lib/bootm.c:
>>
>> diff --git a/arch/xtensa/lib/bootm.c b/arch/xtensa/lib/bootm.c
>> index 458eaf95c0..6fcbfc4292 100644
>> --- a/arch/xtensa/lib/bootm.c
>> +++ b/arch/xtensa/lib/bootm.c
>> @@ -41,15 +41,14 @@ static struct bp_tag *setup_last_tag(struct bp_tag
>> *params)
>>
>> static struct bp_tag *setup_memory_tag(struct bp_tag *params)
>> {
>> - struct bd_info *bd = gd->bd;
>> struct meminfo *mem;
>>
>> params->id = BP_TAG_MEMORY;
>> params->size = sizeof(struct meminfo);
>> mem = (struct meminfo *)params->data;
>> mem->type = MEMORY_TYPE_CONVENTIONAL;
>> - mem->start = bd->bi_memstart;
>> - mem->end = bd->bi_memstart + bd->bi_memsize;
>> + mem->start = gd->ram_base;
>> + mem->end = gd->ram_base + gd->ram_size;
>
> I see. Why not add this instead as well to this new patchset:
>
> diff --git a/arch/xtensa/cpu/cpu.c b/arch/xtensa/cpu/cpu.c
> index 85d3464607..a4ba71a301 100644
> --- a/arch/xtensa/cpu/cpu.c
> +++ b/arch/xtensa/cpu/cpu.c
> @@ -45,6 +45,7 @@ int print_cpuinfo(void)
>
> int arch_cpu_init(void)
> {
> + gd->ram_base = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
> gd->ram_size = CONFIG_SYS_SDRAM_SIZE;
> return 0;
> }
>
> and keep the bi_memstart -> ram_base conversion above? Looks more
> consistant to me.
Thinking a bit more about this, its probably better to use this patch:
diff --git a/arch/xtensa/lib/bootm.c b/arch/xtensa/lib/bootm.c
index 6fcbfc4292..0e564507f9 100644
--- a/arch/xtensa/lib/bootm.c
+++ b/arch/xtensa/lib/bootm.c
@@ -47,8 +47,8 @@ static struct bp_tag *setup_memory_tag(struct bp_tag
*params)
params->size = sizeof(struct meminfo);
mem = (struct meminfo *)params->data;
mem->type = MEMORY_TYPE_CONVENTIONAL;
- mem->start = gd->ram_base;
- mem->end = gd->ram_base + gd->ram_size;
+ mem->start = PHYSADDR(gd->ram_base);
+ mem->end = PHYSADDR(gd->ram_base + gd->ram_size);
printf(" MEMORY: tag:0x%04x, type:0X%lx,
start:0X%lx, end:0X%lx\n",
BP_TAG_MEMORY, mem->type, mem->start, mem->end);
What do you think?
Thanks,
Stefan
More information about the U-Boot
mailing list