[U-Boot] [PATCH v2 1/2] splash: add support for loading splash from a FIT image

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@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(-)
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; } + 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"); + 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; +} +#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 {

Sort include files in accordance to u-boot coding style.
Signed-off-by: Tomas Melin tomas.melin@vaisala.com --- common/splash_source.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/common/splash_source.c b/common/splash_source.c index 9ca523d..4115730 100644 --- a/common/splash_source.c +++ b/common/splash_source.c @@ -7,17 +7,17 @@ */
#include <common.h> -#include <nand.h> -#include <errno.h> -#include <splash.h> -#include <spi_flash.h> -#include <spi.h> -#include <usb.h> -#include <sata.h> #include <bmp_layout.h> -#include <fs.h> +#include <errno.h> #include <fdt_support.h> +#include <fs.h> #include <image.h> +#include <nand.h> +#include <sata.h> +#include <spi.h> +#include <spi_flash.h> +#include <splash.h> +#include <usb.h>
DECLARE_GLOBAL_DATA_PTR;

On 25 November 2016 at 02:45, Tomas Melin tomas.melin@vaisala.com wrote:
Sort include files in accordance to u-boot coding style.
Signed-off-by: Tomas Melin tomas.melin@vaisala.com
common/splash_source.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Tomas,
On 25 November 2016 at 02:45, Tomas Melin tomas.melin@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@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

Hi Simon,
On 11/29/2016 11:40 PM, Simon Glass wrote:
Hi Tomas,
On 25 November 2016 at 02:45, Tomas Melin tomas.melin@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@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)
Yes, the patch is here: http://patchwork.ozlabs.org/patch/699136/ I changed the state of that patch to superseded.
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 ?
Sorry for not noticing this before posting. Fixing those issues and adding documentation for next round.
}
Please drop this blank line.
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.
You are right that these are more or less debug level information. Changing to use debug().
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.
The reason I kept it like this is that fit_image_load() seems to assume the FIT image is already loaded to memory. This loads only the splash image data from storage (not all FIT data). Also, it looks as fit_image_load() doesn't support data in FIT images with data-offset properties (?).
spl_load_simple_fit() does similar things, but cannot really be used as such for getting the data needed here.
I'll send out a new version of this patch-series.
BR, Tomas
participants (2)
-
Simon Glass
-
Tomas Melin