[U-Boot] [PATCH 1/1] efi_loader: pass address to efi_install_fdt()

[Alexander Graf]

On 03.12.19 08:27, Heinrich Schuchardt wrote:
> As part of moving the parsing of command line arguments to do_bootefi()
> call efi_install_fdt() with the address of the device tree instead of a
> string.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>   cmd/bootefi.c | 30 ++++++++++++++++--------------
>   1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index f613cce7e2..3cf190889e 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -196,40 +196,37 @@ static void *get_config_table(const efi_guid_t *guid)
>   #endif /* !CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE) */
>
>   /**
> - * efi_install_fdt() - install fdt passed by a command argument
> + * efi_install_fdt() - install device tree
>    *
> - * If fdt_opt is available, the device tree located at that memory address will
> + * If fdt_addr is available, the device tree located at that memory address will
>    * will be installed as configuration table, otherwise the device tree located
>    * at the address indicated by environment variable fdtcontroladdr will be used.
>    *
> - * On architectures (x86) using ACPI tables device trees shall not be installed
> - * as configuration table.
> + * On architectures using ACPI tables device trees shall not be installed as
> + * configuration table.
>    *
> - * @fdt_opt:	pointer to argument
> + * @fdt_addr:	address of device tree
>    * Return:	status code
>    */
> -static efi_status_t efi_install_fdt(const char *fdt_opt)
> +static efi_status_t efi_install_fdt(uintptr_t fdt_addr)
>   {
>   	/*
>   	 * The EBBR spec requires that we have either an FDT or an ACPI table
>   	 * but not both.
>   	 */
>   #if CONFIG_IS_ENABLED(GENERATE_ACPI_TABLE)
> -	if (fdt_opt) {
> +	if (fdt_addr) {


Why check for fdt_addr != 0 here? Since you do the parsing outside of 
this function now, just make 0 a valid pointer and check for the 
validity outside of this function.


>   		printf("ERROR: can't have ACPI table and device tree.\n");
>   		return EFI_LOAD_ERROR;
>   	}
>   #else
> -	unsigned long fdt_addr;
>   	void *fdt;
>   	bootm_headers_t img = { 0 };
>   	efi_status_t ret;
>
> -	if (fdt_opt) {
> -		fdt_addr = simple_strtoul(fdt_opt, NULL, 16);
> -		if (!fdt_addr)
> -			return EFI_INVALID_PARAMETER;
> -	} else {
> +	if (!fdt_addr) {
> +		const char* fdt_opt;
> +
>   		/* Look for device tree that is already installed */
>   		if (get_config_table(&efi_guid_fdt))
>   			return EFI_SUCCESS;
> @@ -556,6 +553,7 @@ static int do_efi_selftest(void)
>   static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   {
>   	efi_status_t ret;
> +	uintptr_t fdt_addr;
>
>   	if (argc < 2)
>   		return CMD_RET_USAGE;
> @@ -568,7 +566,11 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   		return CMD_RET_FAILURE;
>   	}
>
> -	ret = efi_install_fdt(argc > 2 ? argv[2] : NULL);
> +	if (argc > 2)
> +		fdt_addr = simple_strtoul(argv[2], NULL, 16);
> +	else
> +		fdt_addr = 0UL;
> +	ret = efi_install_fdt(fdt_addr);


So here:


if (fdt_addr)
     efi_install_fdt(fdt_addr);

And then you can remove all of the conditional code (and indentation) in 
efi_install_fdt() above.


Alex