Hi Igor, On 11 December 2016 at 10:37, Igor Grinberg <grinb...@compulab.co.il> wrote: > Hi Tomas, Simon, > > Sorry, to break in that late... > I have a quick question below. > > On 12/05/16 09:36, Tomas Melin 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.me...@vaisala.com> > > [...] > >> diff --git a/common/splash_source.c b/common/splash_source.c >> index 70d724f..94b46d3 100644 >> --- a/common/splash_source.c >> +++ b/common/splash_source.c > > [...] > >> +#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) { >> + debug("Could not find valid FIT image\n"); >> + return -EINVAL; >> + } >> + >> + node_offset = fit_image_get_node(fit_header, location->name); >> + if (node_offset < 0) { >> + debug("Could not find splash image '%s' in FIT\n", >> + location->name); >> + return -ENOENT; >> + } >> + > > I think two above debug() are very legitimate - no need to shout if no FIT > image > or no splash in it... > >> + res = fit_image_get_data_offset(fit_header, node_offset, >> + &splash_offset); >> + if (res < 0) { >> + debug("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) { >> + debug("Could not find 'data-size' property in FIT\n"); >> + return res; >> + } > > Now regarding these two, I'm not sure. > Since we have found a valid FIT and also a node with a correct splash name, > probably the intent is that we show the splash, right? > But in the two above checks, we find inconsistencies that do not allow us to > show the splash - meaning the FIT is not actually good (am I right here?). > So may be we should report it to the 'user' and allow correcting the FIT? > Otherwise, it is impossible to debug the image w/o a debug version of > U-Boot... > Do I make sense, or do I miss something?
Yes that makes some sense, but the problem is that then you are including error messages always which would never happen in a working system (i.e. it just bloats the code). So long as the error is reported (even if it is not a very specific error), people can add DEBUG and track it down. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot