[U-Boot] [PATCH v1 1/3] add boot_get_loadables() to load listed images
Simon Glass
sjg at chromium.org
Fri May 15 15:57:40 CEST 2015
Hi Karl,
On 13 May 2015 at 06:53, Karl Apsite <Karl.Apsite at dornerworks.com> wrote:
> From: Karl Apsite <karl.apsite at dornerworks.com>
>
> Added a trimmed down instance of boot_get_<thing>() to satisfy the
> minimum requierments of the added feature. The function follows the
> normal patterns set by other boot_get<thing>'s, which should make it a
> bit easier to combine them all together into one boot_get_image()
> function in a later refactor.
>
> Documentation for the new function can be found in source:
> common/image.c
>
> Signed-off-by: Karl Apsite <Karl.Apsite at dornerworks.com>
> ---
>
> common/bootm.c | 22 +++++++++++++
> common/image-fit.c | 8 ++++-
> common/image.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/bootstage.h | 1 +
> include/image.h | 5 ++-
> 5 files changed, 127 insertions(+), 2 deletions(-)
>
This looks fine to me. Can you please add documentation - doc/uImage.FIT
A few bits below.
> diff --git a/common/bootm.c b/common/bootm.c
> index 6842029..f04e49b 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -240,6 +240,23 @@ static int bootm_find_fdt(int flag, int argc, char * const argv[])
> }
> #endif
>
> +#if defined(CONFIG_FIT)
> +static int bootm_find_loadables(int flag, int argc, char * const argv[])
> +{
> + int ret;
> +
> + /* find all of the loadables */
> + ret = boot_get_loadable(argc, argv, &images, IH_ARCH_DEFAULT,
> + NULL, NULL);
> + if (ret) {
> + puts("Loadable(s) is corrupt or invalid\n");
printf() here (we are trying to avoid using puts() so that one day we
can make it compatible with the C library puts in that it
automatically outputs \n).
> + return 1;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> int bootm_find_ramdisk_fdt(int flag, int argc, char * const argv[])
> {
> if (bootm_find_ramdisk(flag, argc, argv))
> @@ -250,6 +267,11 @@ int bootm_find_ramdisk_fdt(int flag, int argc, char * const argv[])
> return 1;
> #endif
>
> +#if defined(CONFIG_FIT)
> + if (bootm_find_loadables(flag, argc, argv))
> + return 1;
> +#endif
> +
> return 0;
> }
>
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 245264d..fe23c13 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -1544,6 +1544,8 @@ static const char *fit_get_image_type_property(int type)
> return FIT_RAMDISK_PROP;
> case IH_TYPE_X86_SETUP:
> return FIT_SETUP_PROP;
> + case IH_TYPE_LOADABLE:
> + return FIT_LOADABLE_PROP;
> }
>
> return "unknown";
> @@ -1661,7 +1663,11 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
> os_ok = image_type == IH_TYPE_FLATDT ||
> fit_image_check_os(fit, noffset, IH_OS_LINUX) ||
> fit_image_check_os(fit, noffset, IH_OS_OPENRTOS);
> - if (!type_ok || !os_ok) {
> +
> + /* If either of the checks fail, we should report an error, but
/*
* If either ...
* what it is
*/
> + * if the image type is coming from the "loadables" field, we
> + * don't care about what it is */
> + if ((!type_ok || !os_ok) && image_type != IH_TYPE_LOADABLE) {
> fit_image_get_os(fit, noffset, &os);
> printf("No %s %s %s Image\n",
> genimg_get_os_name(os),
> diff --git a/common/image.c b/common/image.c
> index fdec496..dc7e795 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -1165,6 +1165,99 @@ int boot_get_setup(bootm_headers_t *images, uint8_t arch,
> #endif
> }
>
> +/**
> + * boot_get_loadable - routine to load a list of binaries to memory
> + * @argc: Ignored Argument
> + * @argv: Ignored Argument
> + * @images: pointer to the bootm images structure
> + * @arch: expected architecture for the image
> + * @ld_start: Ignored Argument
> + * @ld_len: Ignored Argument
> + *
> + * boot_get_loadable() will take the given FIT configuration, and look
> + * for a field named "loadables". Loadables, is a list of elements in
> + * the FIT given as strings. exe:
> + * loadables = "linux_kernel at 1", "fdt at 2";
> + * this function will attempt to parse each string, and load the
> + * corresponding element from the FIT into memory. Once placed,
> + * no aditional actions are taken.
> + *
> + * returns:
@return
> + * 0, if only valid images or no images are found
> + * error code, if an error occurs during fit_image_load
Great docs.
> + */
> +#if defined(CONFIG_FIT)
> +int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,
> + uint8_t arch, const ulong *ld_start, ulong * const ld_len)
> +{
> + /*
> + * These variabels are used to hold the current image location
> + * in system memory.
> + */
> + ulong tmp_img_addr;
> + /* These are not used */
> + ulong img_data, img_len;
> + void *buf;
> + char *uname;
> + char *uname_end;
> + int len, fit_img_result;
blank line
> + /* Check to see if the images struct has a FIT configuration */
> + if (genimg_has_config(images)) {
Can we do...
if (!genimg_has_config(images)) {
debug("## FIT configuration was not specified\n");
return 0;
}
just to limit the indent level?
> + /*
> + * Obtain the os FIT header from the images struct
> + * copy from dataflash if needed
> + */
> + tmp_img_addr = map_to_sysmem(images->fit_hdr_os);
> + tmp_img_addr = genimg_get_image(tmp_img_addr);
> + buf = map_sysmem(tmp_img_addr, 0);
> + /*
> + * Check image type. For FIT images get FIT node
> + * and attempt to locate a generic binary.
> + */
> + switch (genimg_get_format(buf)) {
> + case IMAGE_FORMAT_FIT:
> + /*
> + * Try to obtain the 'loadables' field from the
> + * configuration If it exists, then try and load
> + * all of the listed strings
> + *
> + * This logic assumes a character is one byte
> + */
> + uname = (char *)fdt_getprop(buf,
do you need the cast?
> + fit_conf_get_node(buf, images->fit_uname_cfg),
> + FIT_LOADABLE_PROP, &len);
> + uname_end = uname + len;
> + if (NULL != uname) {
if (uname)
but perhaps you should use:
for (index = 0; !fdt_get_string_index(...index...); index++) {
process string
> + do {
> + fit_img_result = fit_image_load(images,
> + tmp_img_addr,
> + (const char **)&uname,
> + &(images->fit_uname_cfg), arch,
> + IH_TYPE_LOADABLE,
> + BOOTSTAGE_ID_FIT_LOADS_START,
> + FIT_LOAD_OPTIONAL_NON_ZERO,
> + &img_data, &img_len);
> + if (fit_img_result < 0) {
> + /* Something went wrong! */
> + return fit_img_result;
> + }
> + uname += strnlen(uname,
> + uname_end - uname) + 1;
> + } while (uname < uname_end);
> + }
> + break;
> + default:
> + puts("The given image format is not supported (corrupt?)\n");
> + return 1;
> + }
> + } else {
> + debug("## FIT configuration was not specified\n");
> + }
> +
> + return 0;
> +}
> +#endif
> +
> #ifdef CONFIG_SYS_BOOT_GET_CMDLINE
> /**
> * boot_get_cmdline - allocate and initialize kernel cmdline
> diff --git a/include/bootstage.h b/include/bootstage.h
> index be44014..4e2e0fb 100644
> --- a/include/bootstage.h
> +++ b/include/bootstage.h
> @@ -168,6 +168,7 @@ enum bootstage_id {
> BOOTSTAGE_ID_NAND_FIT_READ = 150,
> BOOTSTAGE_ID_NAND_FIT_READ_OK,
>
> + BOOTSTAGE_ID_FIT_LOADS_START = 160, /* for Loadable Images */
FIT_LOADABLE_START?
> /*
> * These boot stages are new, higher level, and not directly related
> * to the old boot progress numbers. They are useful for recording
> diff --git a/include/image.h b/include/image.h
> index 97b96b3..0b0b55b 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -244,6 +244,7 @@ struct lmb;
> #define IH_TYPE_SOCFPGAIMAGE 19 /* Altera SOCFPGA Preloader */
> #define IH_TYPE_X86_SETUP 20 /* x86 setup.bin Image */
> #define IH_TYPE_LPC32XXIMAGE 21 /* x86 setup.bin Image */
> +#define IH_TYPE_LOADABLE 22 /* A list of typeless images */
>
> /*
> * Compression Types
> @@ -455,7 +456,9 @@ ulong genimg_get_image(ulong img_addr);
>
> int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
> uint8_t arch, ulong *rd_start, ulong *rd_end);
> -#endif
> +int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,
> + uint8_t arch, const ulong *ld_start, ulong * const ld_len);
Function comment should go here since this is the API definition (move
it from the C file).
> +#endif /* !USE_HOSTCC */
>
> int boot_get_setup_fit(bootm_headers_t *images, uint8_t arch,
> ulong *setup_start, ulong *setup_len);
> --
> 2.3.7
>
Regards,
Simon
More information about the U-Boot
mailing list