[U-Boot] [PATCH v2 1/2] splash: add support for loading splash from a FIT image
Simon Glass
sjg at chromium.org
Tue Nov 29 22:40:22 CET 2016
Hi Tomas,
On 25 November 2016 at 02:45, Tomas Melin <tomas.melin at vaisala.com> wrote:
> Enable support for loading a splash image from within a FIT image.
> The image is assumed to be generated with mkimage -E flag to hold
> the data external to the FIT.
>
> Signed-off-by: Tomas Melin <tomas.melin at vaisala.com>
> ---
> common/image-fit.c | 49 ++++++++++++++++++++++++++++++++++
> common/splash_source.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/image.h | 4 +++
> include/splash.h | 5 ++--
> 4 files changed, 127 insertions(+), 2 deletions(-)
Sorry, a few comments...
(is there a 2/2 patch - is that the one to sort the includes? If so it
would be better to put that one first I think, so this one just adds
the new includes in the right place)
Please can you use patman/checkpatch to check the patch?
4 errors, 2 warnings, 0 checks for
0001-splash-add-support-for-loading-splash-from-a-FIT-ima.patch:
error: common/splash_source.c,317: "(foo*)" should be "(foo *)"
error: common/splash_source.c,321: "(foo*)" should be "(foo *)"
warning: common/splash_source.c,339: line over 80 characters
error: common/splash_source.c,350: trailing whitespace
error: common/splash_source.c,406: code indent should use tabs where possible
warning: common/splash_source.c,406: please, no spaces at the start of a line
Also, can you add some docs to README.splashprepare ?
>
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 77dc011..6f4e9e6 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -777,6 +777,54 @@ int fit_image_get_data(const void *fit, int noffset,
> }
>
> /**
> + * Get 'data-offset' property from a given image node.
> + *
> + * @fit: pointer to the FIT image header
> + * @noffset: component image node offset
> + * @data_offset: holds the data-offset property
> + *
> + * returns:
> + * 0, on success
> + * -ENOENT if the property could not be found
> + */
> +int fit_image_get_data_offset(const void *fit, int noffset, int *data_offset)
> +{
> + const fdt32_t *val;
> +
> + val = fdt_getprop(fit, noffset, FIT_DATA_OFFSET_PROP, NULL);
> + if (!val)
> + return -ENOENT;
> +
> + *data_offset = fdt32_to_cpu(*val);
> +
> + return 0;
> +}
> +
> +/**
> + * Get 'data-size' property from a given image node.
> + *
> + * @fit: pointer to the FIT image header
> + * @noffset: component image node offset
> + * @data_size: holds the data-size property
> + *
> + * returns:
> + * 0, on success
> + * -ENOENT if the property could not be found
> + */
> +int fit_image_get_data_size(const void *fit, int noffset, int *data_size)
> +{
> + const fdt32_t *val;
> +
> + val = fdt_getprop(fit, noffset, FIT_DATA_SIZE_PROP, NULL);
> + if (!val)
> + return -ENOENT;
> +
> + *data_size = fdt32_to_cpu(*val);
> +
> + return 0;
> +}
> +
> +/**
> * fit_image_hash_get_algo - get hash algorithm name
> * @fit: pointer to the FIT format image header
> * @noffset: hash node offset
> @@ -1825,3 +1873,4 @@ int boot_get_setup_fit(bootm_headers_t *images, uint8_t arch,
>
> return ret;
> }
> +
Please drop this blank line.
> diff --git a/common/splash_source.c b/common/splash_source.c
> index 72df2c1..9ca523d 100644
> --- a/common/splash_source.c
> +++ b/common/splash_source.c
> @@ -16,6 +16,8 @@
> #include <sata.h>
> #include <bmp_layout.h>
> #include <fs.h>
> +#include <fdt_support.h>
> +#include <image.h>
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -295,6 +297,71 @@ static struct splash_location *select_splash_location(
> return NULL;
> }
>
> +#ifdef CONFIG_FIT
> +static int splash_load_fit(struct splash_location *location, u32 bmp_load_addr)
> +{
> + int res;
> + int node_offset;
> + int splash_offset;
> + int splash_size;
> + struct image_header *img_header;
> + const u32 *fit_header;
> + u32 fit_size;
> + const size_t header_size = sizeof(struct image_header);
> +
> + /* Read in image header */
> + res = splash_storage_read_raw(location, bmp_load_addr, header_size);
> + if (res < 0)
> + return res;
> +
> + img_header = (struct image_header*)bmp_load_addr;
> + fit_size = fdt_totalsize(img_header);
> +
> + /* Read in entire FIT */
> + fit_header = (const u32*)(bmp_load_addr + header_size);
> + res = splash_storage_read_raw(location, (u32)fit_header, fit_size);
> + if (res < 0)
> + return res;
> +
> + res = fit_check_format(fit_header);
> + if (!res) {
> + printf("Could not find valid FIT-image\n");
> + return -EINVAL;
> + }
> +
> + node_offset = fit_image_get_node(fit_header, location->name);
> + if (node_offset < 0) {
> + printf("Could not find splash image '%s' in FIT\n",
> + location->name);
> + return -ENOENT;
> + }
> +
> + res = fit_image_get_data_offset(fit_header, node_offset, &splash_offset);
> + if (res < 0) {
> + printf("Could not find 'data-offset' property in FIT\n");
> + return res;
> + }
> +
> + res = fit_image_get_data_size(fit_header, node_offset, &splash_size);
> + if (res < 0) {
> + printf("Could not find 'data-size' property in FIT\n");
Should any of these be debug(), or do you really expect these to be
useful to the user? If we can avoid printf() it does cut the code
size.
> + return res;
> + }
> +
> + /* Align data offset to 4-byte boundrary */
> + fit_size = fdt_totalsize(fit_header);
> + fit_size = (fit_size + 3) & ~3;
> +
> + /* Read in the splash data */
> + location->offset = (location->offset + fit_size + splash_offset);
> + res = splash_storage_read_raw(location, bmp_load_addr , splash_size);
> + if (res < 0)
> + return res;
> +
> + return 0;
> +}
I suppose this is fine. But really I would prefer to use something like:
fit_image_load(, ..., &data, &len)
memcpy(dst, src_addr, len);
but that would presumably require quite a few changes, right? At least
it would be good to know what is missing.
> +#endif /* CONFIG_FIT */
> +
> /**
> * splash_source_load - load splash image from a supported location.
> *
> @@ -331,5 +398,9 @@ int splash_source_load(struct splash_location *locations, uint size)
> return splash_load_raw(splash_location, bmp_load_addr);
> else if (splash_location->flags == SPLASH_STORAGE_FS)
> return splash_load_fs(splash_location, bmp_load_addr);
> +#ifdef CONFIG_FIT
> + else if (splash_location->flags == SPLASH_STORAGE_FIT)
> + return splash_load_fit(splash_location, bmp_load_addr);
> +#endif
> return -EINVAL;
> }
> diff --git a/include/image.h b/include/image.h
> index 2b1296c..94df589 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -870,6 +870,8 @@ int bootz_setup(ulong image, ulong *start, ulong *end);
>
> /* image node */
> #define FIT_DATA_PROP "data"
> +#define FIT_DATA_OFFSET_PROP "data-offset"
> +#define FIT_DATA_SIZE_PROP "data-size"
> #define FIT_TIMESTAMP_PROP "timestamp"
> #define FIT_DESC_PROP "description"
> #define FIT_ARCH_PROP "arch"
> @@ -948,6 +950,8 @@ int fit_image_get_load(const void *fit, int noffset, ulong *load);
> int fit_image_get_entry(const void *fit, int noffset, ulong *entry);
> int fit_image_get_data(const void *fit, int noffset,
> const void **data, size_t *size);
> +int fit_image_get_data_offset(const void *fit, int noffset, int *data_offset);
> +int fit_image_get_data_size(const void *fit, int noffset, int *data_size);
>
> int fit_image_hash_get_algo(const void *fit, int noffset, char **algo);
> int fit_image_hash_get_value(const void *fit, int noffset, uint8_t **value,
> diff --git a/include/splash.h b/include/splash.h
> index 136eac7..228aff4 100644
> --- a/include/splash.h
> +++ b/include/splash.h
> @@ -33,8 +33,9 @@ enum splash_storage {
> };
>
> enum splash_flags {
> - SPLASH_STORAGE_RAW,
> - SPLASH_STORAGE_FS,
> + SPLASH_STORAGE_RAW, /* Stored in raw memory */
> + SPLASH_STORAGE_FS, /* Stored within a file system */
> + SPLASH_STORAGE_FIT, /* Stored inside a FIT image */
> };
>
> struct splash_location {
> --
> 2.1.4
Regards,
Simon
More information about the U-Boot
mailing list