[PATCH] xilinx: board: Add support to pick bootscr address from DT

[Michal Simek]

Hi Simon,

On 6/15/23 11:14, Simon Glass wrote:
> Hi Michal,
> 
> On Tue, 13 Jun 2023 at 15:13, Michal Simek <michal.simek at amd.com> wrote:
>>
>> From: Algapally Santosh Sagar <santoshsagar.algapally at amd.com>
>>
>> The bootscript is expected at a default address specific to each
>> platform.
> 
> Who puts it there?

Distro boot is using that variable. As of today in board_late_init_xilinx() we 
are doing simple calculation that it is added to certain offset from ram start.

This code.
                scriptaddr = env_get_hex("scriptaddr", 0);
                ret |= env_set_hex("scriptaddr", gd->ram_base + scriptaddr);

And because of system allocation DDR for non secure doesn't need to start at 
offset 0. But we don't want to keep recompiling u-boot to change default 
variables location that's why we are updating it at run time because detected 
DDR location.

And distro boot expects this address to be physical address not any offset where 
boot script is loaded.

Logic around is include/config_distro_bootcmd.h

> 
>> When high speed memory like Programmable Logic Double Data Rate RAM
>> (PL DDR RAM) or Higher Bandwidth Memory RAM (HBM) is used the boot.scr
>> may be loaded at a different offset. The offset needs to be set through
>> setenv. Due to the default values in some cases the boot.scr is falling
>> in between the kernel partition.
>>
>> The bootscript address or the bootscript offset is fetched directly from
>> the DT and updated in the environment making it easier for automated
>> flows.
>>
>> Signed-off-by: Algapally Santosh Sagar <santoshsagar.algapally at amd.com>
>> Signed-off-by: Michal Simek <michal.simek at amd.com>
>> ---
>>
>>   board/xilinx/common/board.c | 43 ++++++++++++++++++++++++++++++++++---
>>   1 file changed, 40 insertions(+), 3 deletions(-)
>>
>> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
>> index d071ebfb9cc2..bdd4113b0916 100644
>> --- a/board/xilinx/common/board.c
>> +++ b/board/xilinx/common/board.c
>> @@ -405,6 +405,31 @@ static int env_set_by_index(const char *name, int index, char *data)
>>          return env_set(var, data);
>>   }
>>
>> +static int get_bootscript_address(u64 *bootscr_address, u64 *bootscr_offset)
>> +{
>> +       int ret;
>> +       ofnode uboot;
>> +
>> +       *bootscr_address = 0;
>> +       *bootscr_offset = 0;
>> +
>> +       uboot = ofnode_path("/options/u-boot");
>> +       if (!ofnode_valid(uboot)) {
>> +               printf("%s: Missing /u-boot node\n", __func__);
>> +               return -EINVAL;
>> +       }
>> +
>> +       ret = ofnode_read_u64(uboot, "bootscr-address", bootscr_address);
> 
> Normally we would have the size under the control of a #address-cells
> property. Do you think we should do that here?

address/size cells are used for reg/ranges properties. I don't think we should 
be using it here. That's why it is also said in binding that value is 64bit to 
cover both 32/64bit architectures which are used today.

And the same problem is there also with all of these because they have fixed 
address but that addresses should be different based on boards or HW design 
configuration or system partitioning.

  53         "fdt_addr_r=0x40000000\0" \
  54         "fdt_size_r=0x400000\0" \
  55         "pxefile_addr_r=0x10000000\0" \
  56         "kernel_addr_r=0x18000000\0" \
  57         "kernel_size_r=0x10000000\0" \
  58         "kernel_comp_addr_r=0x30000000\0" \
  59         "kernel_comp_size=0x3C00000\0" \
  60         "scriptaddr=0x20000000\0" \
  61         "ramdisk_addr_r=0x02100000\0" \

Thanks,
Michal