Re: [PATCH] arm64: zynqmp: Do not describe u-boot.itb if SPL is disabled

[Michal Simek <michal.simek@amd.com>]

On 3/12/24 13:29, Heinrich Schuchardt wrote:
> On 05.03.24 16:47, Ilias Apalodimas wrote:
>> On Fri, Feb 23, 2024 at 05:18:42PM +0100, Michal Simek wrote:
>>> There is no reason to describe u-boot.itb on system without SPL. Pretty
>>> much this is cover all systems which are using only boot.bin which contains
>>> all images inside.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>>> ---
>>>
>>>   board/xilinx/common/board.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
>>> index 9641ed307b75..4f38b7d27684 100644
>>> --- a/board/xilinx/common/board.c
>>> +++ b/board/xilinx/common/board.c
>>> @@ -43,7 +43,7 @@ struct efi_fw_image fw_images[] = {
>>>           .image_index = 1,
>>>       },
>>>   #endif
>>> -#if defined(XILINX_UBOOT_IMAGE_GUID)
>>> +#if defined(XILINX_UBOOT_IMAGE_GUID) && 
>>> defined(CONFIG_SPL_FS_LOAD_PAYLOAD_NAME)
>>
>> What happens if this is defined with CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="" ?
> 
> CONFIG_SPL_FS_LOAD_PAYLOAD_NAME depends on SPL_FS_EXT4 || SPL_FS_FAT ||
> SPL_FS_SQUASHFS || SPL_SEMIHOSTING. So it is only defined if SPL could
> load a file.
> 
> CONFIG_SPL_FS_LOAD_PAYLOAD_NAME defaults to a non-blank name. If a user
> provides an invalid name, SPL will not be able to load the file.
> 
> What is wrong here is to assume that *.itb has to be load as a file. We
> can configure U-Boot SPL to load the itb from a raw partition.
> 
> The check might be too restrictive.

I am not sure I fully follow what you are saying here.
All current dfu rules via set_dfu_alt_info() (board/xilinx/zynqmp/zynqmp.c) are 
using CONFIG_SPL_FS_LOAD_PAYLOAD_NAME for second location description.
Obviously in qspi boot mode it is description for RAW. In sd one it is 
description with using fat.
Description for two guids make only sense in connection to using U-Boot SPL 
only. Because if SPL is not used likely you are going to use standard Xilinx 
solution which is pretty much all in one inside boot.bin which is described 
already.
This patch is just trying to get rid of description for second firmware if non 
SPL bootflow is used because pointer is likely not correct.

Thanks,
Michal