[RFC 11/14] efi_loader: move distro_efi_get_fdt_name()
Caleb Connolly
caleb.connolly at linaro.org
Fri Apr 26 16:52:11 CEST 2024
Hi Heinrich,
On 26/04/2024 16:13, Heinrich Schuchardt wrote:
> Move distro_efi_get_fdt_name() to a separate C module
> and rename it to efi_get_distro_fdt_name().
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
> boot/bootmeth_efi.c | 60 ++-------------------------------
> include/efi_loader.h | 2 ++
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_fdt.c | 73 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 78 insertions(+), 58 deletions(-)
> create mode 100644 lib/efi_loader/efi_fdt.c
>
> diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> index aebc5207fc0..40da77c497b 100644
> --- a/boot/bootmeth_efi.c
> +++ b/boot/bootmeth_efi.c
> @@ -144,62 +144,6 @@ static int distro_efi_check(struct udevice *dev, struct bootflow_iter *iter)
> return 0;
> }
>
> -/**
> - * distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
> - *
> - * @fname: Place to put filename
> - * @size: Max size of filename
> - * @seq: Sequence number, to cycle through options (0=first)
> - * Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
> - * -EINVAL if there are no more options, -EALREADY if the control FDT should be
> - * used
> - */
> -static int distro_efi_get_fdt_name(char *fname, int size, int seq)
> -{
> - const char *fdt_fname;
> - const char *prefix;
> -
> - /* select the prefix */
> - switch (seq) {
> - case 0:
> - /* this is the default */
> - prefix = "/dtb";
> - break;
> - case 1:
> - prefix = "";
> - break;
> - case 2:
> - prefix = "/dtb/current";
> - break;
This is a bit of a tangential point (and shouldn't block this series at
all). But where do we find a balance with this search algorithm? The
distro I work on (postmarketOS) actually installs DTB files to "/dtbs"
(not "/dtb").
No doubt we can come up with a bunch of clever ideas for optimising this
with wildcards or whatever, but is that something we want to implement?
What about distros that support falling back to previous kernel versions
(and consequently have the DTB dir named after the kernel version).
Presumably in these situations the distro would have systemd-boot, GRUB,
etc. Would it make sense to just inform the bootloader what the .dtb
file name is so that they can handle the path building?
(I understand there are other usecases and reasons to load the DTB ahead
of time in U-Boot).
> - default:
> - return log_msg_ret("pref", -EINVAL);
> - }
> -
> - fdt_fname = env_get("fdtfile");
> - if (fdt_fname) {
> - snprintf(fname, size, "%s/%s", prefix, fdt_fname);
> - log_debug("Using device tree: %s\n", fname);
> - } else if (IS_ENABLED(CONFIG_OF_HAS_PRIOR_STAGE)) {
> - strcpy(fname, "<prior>");
> - return log_msg_ret("pref", -EALREADY);
> - /* Use this fallback only for 32-bit ARM */
> - } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) {
> - const char *soc = env_get("soc");
> - const char *board = env_get("board");
> - const char *boardver = env_get("boardver");
> -
> - /* cf the code in label_boot() which seems very complex */
> - snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
> - soc ? soc : "", soc ? "-" : "", board ? board : "",
> - boardver ? boardver : "");
> - log_debug("Using default device tree: %s\n", fname);
> - } else {
> - return log_msg_ret("env", -ENOENT);
> - }
> -
> - return 0;
> -}
> -
> /*
> * distro_efi_try_bootflow_files() - Check that files are present
> *
> @@ -241,7 +185,7 @@ static int distro_efi_try_bootflow_files(struct udevice *dev,
> ret = -ENOENT;
> *fname = '\0';
> for (seq = 0; ret == -ENOENT; seq++) {
> - ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);
> + ret = efi_get_distro_fdt_name(fname, sizeof(fname), seq);
> if (ret == -EALREADY)
> bflow->flags = BOOTFLOWF_USE_PRIOR_FDT;
> if (!ret) {
> @@ -340,7 +284,7 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow)
> sprintf(file_addr, "%lx", fdt_addr);
>
> /* We only allow the first prefix with PXE */
> - ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0);
> + ret = efi_get_distro_fdt_name(fname, sizeof(fname), 0);
> if (ret)
> return log_msg_ret("nam", ret);
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 0ac04990b8e..ed2b517b130 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -1187,4 +1187,6 @@ efi_status_t efi_disk_get_device_name(const efi_handle_t handle, char *buf, int
> */
> void efi_add_known_memory(void);
>
> +int efi_get_distro_fdt_name(char *fname, int size, int seq);
> +
> #endif /* _EFI_LOADER_H */
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 034e366967f..2af6f2066b5 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -59,6 +59,7 @@ obj-y += efi_device_path.o
> obj-$(CONFIG_EFI_DEVICE_PATH_TO_TEXT) += efi_device_path_to_text.o
> obj-$(CONFIG_EFI_DEVICE_PATH_UTIL) += efi_device_path_utilities.o
> obj-y += efi_dt_fixup.o
> +obj-y += efi_fdt.o
> obj-y += efi_file.o
> obj-$(CONFIG_EFI_LOADER_HII) += efi_hii.o
> obj-y += efi_image_loader.o
> diff --git a/lib/efi_loader/efi_fdt.c b/lib/efi_loader/efi_fdt.c
> new file mode 100644
> index 00000000000..0edf0c1e2fc
> --- /dev/null
> +++ b/lib/efi_loader/efi_fdt.c
> @@ -0,0 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Bootmethod for distro boot via EFI
> + *
> + * Copyright 2021 Google LLC
> + * Written by Simon Glass <sjg at chromium.org>
> + */
> +
> +#include <efi_loader.h>
> +#include <env.h>
> +#include <errno.h>
> +#include <log.h>
> +#include <string.h>
> +#include <vsprintf.h>
> +
> +/**
> + * distro_efi_get_fdt_name() - get the filename for reading the .dtb file
> + *
> + * @fname: buffer for filename
> + * @size: buffer size
> + * @seq: sequence number, to cycle through options (0=first)
> + *
> + * Returns:
> + * 0 on success,
> + * -ENOENT if the "fdtfile" env var does not exist,
> + * -EINVAL if there are no more options,
> + * -EALREADY if the control FDT should be used
> + */
> +int efi_get_distro_fdt_name(char *fname, int size, int seq)
> +{
> + const char *fdt_fname;
> + const char *prefix;
> +
> + /* select the prefix */
> + switch (seq) {
> + case 0:
> + /* this is the default */
> + prefix = "/dtb";
> + break;
> + case 1:
> + prefix = "";
> + break;
> + case 2:
> + prefix = "/dtb/current";
> + break;
> + default:
> + return log_msg_ret("pref", -EINVAL);
> + }
> +
> + fdt_fname = env_get("fdtfile");
> + if (fdt_fname) {
> + snprintf(fname, size, "%s/%s", prefix, fdt_fname);
> + log_debug("Using device tree: %s\n", fname);
> + } else if (IS_ENABLED(CONFIG_OF_HAS_PRIOR_STAGE)) {
> + strcpy(fname, "<prior>");
> + return log_msg_ret("pref", -EALREADY);
> + /* Use this fallback only for 32-bit ARM */
> + } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) {
> + const char *soc = env_get("soc");
> + const char *board = env_get("board");
> + const char *boardver = env_get("boardver");
> +
> + /* cf the code in label_boot() which seems very complex */
> + snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
> + soc ? soc : "", soc ? "-" : "", board ? board : "",
> + boardver ? boardver : "");
> + log_debug("Using default device tree: %s\n", fname);
> + } else {
> + return log_msg_ret("env", -ENOENT);
> + }
> +
> + return 0;
> +}
--
// Caleb (they/them)
More information about the U-Boot
mailing list